Skip to content

MCO-2275: Part2 Migrate MCO OCB #6340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ptalgulk01 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

MCO-2275: Part2 Migrate MCO OCB #6340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4646c67
Migrate 17 OCB test cases from openshift-tests-private
ptalgulk01 May 21, 2026
61fc8ec
Add OCB remaning tc and exclude the failing tc until they are resolved
ptalgulk01 Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
236 changes: 122 additions & 114 deletions test/extended-priv/machineconfigpool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,6 @@ func (mcp *MachineConfigPool) SetMaxUnavailable(maxUnavailable int) {
o.Expect(err).NotTo(o.HaveOccurred())
}

// RemoveMaxUnavailable removes spec.maxUnavailable attribute from the pool config
func (mcp *MachineConfigPool) RemoveMaxUnavailable() {
logger.Infof("patch mcp %v, removing spec.maxUnavailable", mcp.name)
err := mcp.Patch("json", `[{ "op": "remove", "path": "/spec/maxUnavailable" }]`)
o.Expect(err).NotTo(o.HaveOccurred())
}

// SetOsImageStream sets the osImageStream name for the MCP
func (mcp *MachineConfigPool) SetOsImageStream(streamName string) error {
logger.Infof("patch mcp %v, change spec.osImageStream.name to %s", mcp.name, streamName)
Expand All @@ -158,6 +151,13 @@ func (mcp *MachineConfigPool) GetStatusOsImageStream() (string, error) {
return mcp.Get(`{.status.osImageStream.name}`)
}

// RemoveMaxUnavailable removes spec.maxUnavailable attribute from the pool config
func (mcp *MachineConfigPool) RemoveMaxUnavailable() {
logger.Infof("patch mcp %v, removing spec.maxUnavailable", mcp.name)
err := mcp.Patch("json", `[{ "op": "remove", "path": "/spec/maxUnavailable" }]`)
o.Expect(err).NotTo(o.HaveOccurred())
}

func (mcp *MachineConfigPool) getConfigNameOfSpec() (string, error) {
output, err := mcp.Get(`{.spec.configuration.name}`)
logger.Infof("spec.configuration.name of mcp/%v is %v", mcp.name, output)
Expand Down Expand Up @@ -276,6 +276,7 @@ func (mcp *MachineConfigPool) estimateWaitDuration() time.Duration {
totalNodes int
guessedNodes = 3 // the number of nodes that we will use if we cannot get the actual number of nodes in the cluster
masterAdjust = 1.0
proxyModifier = 1.0
snoModifier = 0.0
emptyMCPWaitDuration = 2.0
minutesDuration = 1 * time.Minute
Expand Down Expand Up @@ -324,7 +325,14 @@ func (mcp *MachineConfigPool) estimateWaitDuration() time.Duration {
snoModifier = 3
}
}
return time.Duration(((float64(totalNodes*mcp.MinutesWaitingPerNode) * masterAdjust) + snoModifier) * float64(minutesDuration))
proxy := NewResource(mcp.GetOC(), "proxy", "cluster")
httpProxy, proxyErr := proxy.Get(`{.status.httpProxy}`)
if proxyErr == nil && httpProxy != "" {
logger.Infof("Increase waiting time because the cluster is proxied")
proxyModifier = 1.3
}

return time.Duration(((float64(totalNodes*mcp.MinutesWaitingPerNode) * masterAdjust * proxyModifier) + snoModifier) * float64(minutesDuration))
}

// SetWaitingTimeForKernelChange increases the time that the MCP will wait for the update to be executed
Expand Down Expand Up @@ -537,6 +545,80 @@ func (mcp *MachineConfigPool) GetSortedNodesOrFail() []*Node {
return nodes
}

// GetSortedUpdatedNodes returns the list of the UpdatedNodes sorted by the time when they started to be updated.
// If maxUnavailable>0, then the function will fail if more that maxUpdatingNodes are being updated at the same time
func (mcp *MachineConfigPool) GetSortedUpdatedNodes(maxUnavailable int) []*Node {
timeToWait := mcp.estimateWaitDuration()
logger.Infof("Waiting %s in pool %s for all nodes to start updating.", timeToWait, mcp.name)

poolNodes, errget := mcp.GetNodes()
o.Expect(errget).NotTo(o.HaveOccurred(), fmt.Sprintf("Cannot get nodes in pool %s", mcp.GetName()))

pendingNodes := poolNodes
updatedNodes := []*Node{}
immediate := false
err := wait.PollUntilContextTimeout(context.TODO(), 20*time.Second, timeToWait, immediate, func(_ context.Context) (bool, error) {
// If there are degraded machines, stop polling, directly fail
degradedstdout, degradederr := mcp.getDegradedMachineCount()
if degradederr != nil {
logger.Errorf("the err:%v, and try next round", degradederr)
return false, nil
}

if degradedstdout != 0 {
logger.Errorf("Degraded MC:\n%s", mcp.PrettyString())
exutil.AssertWaitPollNoErr(fmt.Errorf("Degraded machines"), fmt.Sprintf("mcp %s has degraded %d machines", mcp.name, degradedstdout))
}

// Check that there aren't more thatn maxUpdatingNodes updating at the same time
if maxUnavailable > 0 {
totalUpdating := 0
for _, node := range poolNodes {
isUpdating, err := node.IsUpdating()
if err != nil {
logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err)
return false, err
}
if isUpdating {
totalUpdating++
}
}
if totalUpdating > maxUnavailable {
// print nodes for debug
mcp.oc.Run("get").Args("nodes").Execute()

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C2 --type go 'func\s*\([^)]*\)\s*Execute\s*\(' .
rg -n -C2 --type go '\.Run\("get"\)\.Args\("nodes"\)\.Execute\(\)' test/extended-priv/machineconfigpool.go

Repository: openshift/machine-config-operator

Length of output: 8796

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## client.go around CLI Execute"
sed -n '820,885p' test/extended-priv/util/client.go

echo
echo "## machineconfigpool.go around diagnostic call"
sed -n '560,605p' test/extended-priv/machineconfigpool.go

echo
echo "## same repository calls to CLI Execute without error check"
rg -n --type go '\.+Execute\(\)' test/extended-priv -g '*.go'

Repository: openshift/machine-config-operator

Length of output: 10952

Handle the diagnostic command failure.

CLI.Execute() returns the underlying oc get error, but line 588 discards it and the test still asserts the polling error. Capture the error, keep it in the context or prepend it to the final assertion, so node-dump failures surface useful diagnostics instead of being absorbed.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended-priv/machineconfigpool.go` at line 588, Update the
node-diagnostic command in the surrounding polling/error-handling flow to
capture the error returned by CLI.Execute() for
mcp.oc.Run("get").Args("nodes").Execute(). Preserve that error in the test
context or include it in the final assertion so failures from oc get nodes are
surfaced alongside the polling error.

Source: Path instructions

exutil.AssertWaitPollNoErr(fmt.Errorf("maxUnavailable Not Honored. Pool %s, error: %d nodes were updating at the same time. Only %d nodes should be updating at the same time", mcp.GetName(), totalUpdating, maxUnavailable), "")
}
}

remainingNodes := []*Node{}
for _, node := range pendingNodes {
isUpdating, err := node.IsUpdating()
if err != nil {
logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err)
return false, err
}
if isUpdating {
logger.Infof("Node %s is UPDATING", node.GetName())
updatedNodes = append(updatedNodes, node)
} else {
remainingNodes = append(remainingNodes, node)
}
}

if len(remainingNodes) == 0 {
logger.Infof("All nodes have started to be updated on mcp %s", mcp.name)
return true, nil

}
logger.Infof(" %d remaining nodes", len(remainingNodes))
pendingNodes = remainingNodes
return false, nil
})

exutil.AssertWaitPollNoErr(err, fmt.Sprintf("Could not get the list of updated nodes on mcp %s", mcp.name))
return updatedNodes
}

// GetCordonedNodes get cordoned nodes (if maxUnavailable > 1 ) otherwise return the 1st cordoned node
func (mcp *MachineConfigPool) GetCordonedNodes() []*Node {

Expand Down Expand Up @@ -1288,6 +1370,38 @@ func CreateCustomMCPWithStreamByLabel(oc *exutil.CLI, name, label, osstream stri
return CreateCustomMCPWithStreamByNodes(oc, name, osstream, customMcpNodes)
}

// AddNodesToMachineConfigPool adds the given nodes to the specified MCP by labeling them
// and waits for the MCP to report the expected machine count and updated status
func AddNodesToMachineConfigPool(oc *exutil.CLI, mcpName string, nodes []*Node) error {
mcp := NewMachineConfigPool(oc, mcpName)

currentNodes, err := mcp.GetNodes()
if err != nil {
return fmt.Errorf("error getting current nodes from %s: %w", mcpName, err)
}
expectedCount := len(currentNodes) + len(nodes)

for _, n := range nodes {
err := n.AddLabel(fmt.Sprintf("node-role.kubernetes.io/%s", mcpName), "")
if err != nil {
return fmt.Errorf("error labeling node %s to add it to pool %s: %w", n.GetName(), mcpName, err)
}
logger.Infof("Node %s added to pool %s", n.GetName(), mcpName)
}

err = mcp.WaitForMachineCount(expectedCount, 5*time.Minute)
if err != nil {
return fmt.Errorf("the %s MCP is not reporting the expected machine count: %w", mcpName, err)
}

err = mcp.WaitImmediateForUpdatedStatus()
if err != nil {
return fmt.Errorf("the %s MCP is not updated: %w", mcpName, err)
}

return nil
}

// CreateCustomMCP creates a new custom MCP with the given name and number of nodes
// No osstream field is set - MCP will inherit from worker pool automatically
// Nodes will be taken from the worker pool
Expand Down Expand Up @@ -1644,112 +1758,6 @@ func GetPoolWithArchDifferentFromOrFail(oc *exutil.CLI, arch architecture.Archit
return nil
}

// AddNodesToMachineConfigPool adds nodes to an existing MCP by labeling them and waiting for the MCP to be updated
// This function is similar to CreateCustomMCPWithStreamByNodes but for adding nodes to existing pools
func AddNodesToMachineConfigPool(oc *exutil.CLI, mcpName string, nodes []*Node) error {
mcp := NewMachineConfigPool(oc, mcpName)

currentNodes, err := mcp.GetNodes()
if err != nil {
return fmt.Errorf("error getting current nodes from %s: %w", mcpName, err)
}
expectedCount := len(currentNodes) + len(nodes)

for _, n := range nodes {
err := n.AddLabel(fmt.Sprintf("node-role.kubernetes.io/%s", mcpName), "")
if err != nil {
return fmt.Errorf("error labeling node %s to add it to pool %s: %w", n.GetName(), mcpName, err)
}
logger.Infof("Node %s added to pool %s", n.GetName(), mcpName)
}

err = mcp.WaitForMachineCount(expectedCount, 5*time.Minute)
if err != nil {
return fmt.Errorf("the %s MCP is not reporting the expected machine count: %w", mcpName, err)
}

err = mcp.WaitImmediateForUpdatedStatus()
if err != nil {
return fmt.Errorf("the %s MCP is not updated: %w", mcpName, err)
}

return nil
}

// GetSortedUpdatedNodes returns a list of nodes in the order that they are being updated by the MCO
// If maxUnavailable>0, then the function will fail if more that maxUpdatingNodes are being updated at the same time
func (mcp *MachineConfigPool) GetSortedUpdatedNodes(maxUnavailable int) []*Node {
timeToWait := mcp.estimateWaitDuration()
logger.Infof("Waiting %s in pool %s for all nodes to start updating.", timeToWait, mcp.name)

poolNodes, errget := mcp.GetNodes()
o.Expect(errget).NotTo(o.HaveOccurred(), fmt.Sprintf("Cannot get nodes in pool %s", mcp.GetName()))

pendingNodes := poolNodes
updatedNodes := []*Node{}
immediate := false
err := wait.PollUntilContextTimeout(context.TODO(), 20*time.Second, timeToWait, immediate, func(_ context.Context) (bool, error) {
// If there are degraded machines, stop polling, directly fail
degradedstdout, degradederr := mcp.getDegradedMachineCount()
if degradederr != nil {
logger.Errorf("the err:%v, and try next round", degradederr)
return false, nil
}

if degradedstdout != 0 {
logger.Errorf("Degraded MC:\n%s", mcp.PrettyString())
exutil.AssertWaitPollNoErr(fmt.Errorf("degraded machines"), fmt.Sprintf("mcp %s has degraded %d machines", mcp.name, degradedstdout))
}

// Check that there aren't more thatn maxUpdatingNodes updating at the same time
if maxUnavailable > 0 {
totalUpdating := 0
for _, node := range poolNodes {
isUpdating, err := node.IsUpdating()
if err != nil {
logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err)
return false, err
}
if isUpdating {
totalUpdating++
}
}
if totalUpdating > maxUnavailable {
// print nodes for debug
mcp.oc.Run("get").Args("nodes").Execute()
exutil.AssertWaitPollNoErr(fmt.Errorf("maxUnavailable Not Honored. Pool %s, error: %d nodes were updating at the same time. Only %d nodes should be updating at the same time", mcp.GetName(), totalUpdating, maxUnavailable), "")
}
}

remainingNodes := []*Node{}
for _, node := range pendingNodes {
isUpdating, err := node.IsUpdating()
if err != nil {
logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err)
return false, err
}
if isUpdating {
logger.Infof("Node %s is UPDATING", node.GetName())
updatedNodes = append(updatedNodes, node)
} else {
remainingNodes = append(remainingNodes, node)
}
}

if len(remainingNodes) == 0 {
logger.Infof("All nodes have started to be updated on mcp %s", mcp.name)
return true, nil

}
logger.Infof(" %d remaining nodes", len(remainingNodes))
pendingNodes = remainingNodes
return false, nil
})

exutil.AssertWaitPollNoErr(err, fmt.Sprintf("Could not get the list of updated nodes on mcp %s", mcp.name))
return updatedNodes
}

// IsOCL returns true if the pool is using On Cluster Layering functionality
func (mcp MachineConfigPool) IsOCL() (bool, error) {
isOCLEnabled, err := IsFeaturegateEnabled(mcp.GetOC(), "OnClusterBuild")
Expand Down
23 changes: 23 additions & 0 deletions test/extended-priv/machineosconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,20 @@ func (mosc MachineOSConfig) GetCurrentMachineOSBuild() (*MachineOSBuild, error)
return NewMachineOSBuild(mosc.GetOC(), mosbName), nil
}

// SetRenderedImagePushspec patches the MOSC resource in order to configure a new renderedImagePushspec
func (mosc MachineOSConfig) SetRenderedImagePushspec(rips string) error {
escaped, err := json.Marshal(rips)
if err != nil {
return err
}
return mosc.Patch("json", `[{"op":"replace","path":"/spec/renderedImagePushSpec","value":`+string(escaped)+`}]`)
}

// GetRenderedImagePushspec returns the current valude of renderedImagePushspec
func (mosc MachineOSConfig) GetRenderedImagePushspec() (string, error) {
return mosc.Get(`{.spec.renderedImagePushSpec}`)
}

// SetContainerfiles sets the container files used by this MOSC
func (mosc MachineOSConfig) SetContainerfiles(containerFiles []ContainerFile) error {
containerFilesBytes, err := json.Marshal(containerFiles)
Expand Down Expand Up @@ -472,3 +486,12 @@ func SkipTestIfCannotUseInternalRegistry(oc *exutil.CLI) {
g.Skip("The internal registry cannot be used to store the osImage in this cluster. Skipping test case")
}
}

// IsUsingInternalRegistry checks if the current image is from the internal registry
func (mosc MachineOSConfig) IsUsingInternalRegistry() (bool, error) {
currentImage, err := mosc.GetStatusCurrentImagePullSpec()
if err != nil {
return false, err
}
return strings.Contains(currentImage, InternalRegistrySvcURL), nil
}
10 changes: 6 additions & 4 deletions test/extended-priv/mco_ocb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ func ValidateSuccessfulMOSC(mosc *MachineOSConfig, checkers []Checker) {
logger.Infof("OK!\n")

exutil.By("Check that a new build is successfully executed")
o.Eventually(mosb, "20m", "20s").Should(HaveConditionField("Building", "status", FalseString), "Build was not finished")
// When building several builds at the same time the pods can take more than 20 minutes to build a simple image
o.Eventually(mosb, "35m", "20s").Should(HaveConditionField("Building", "status", FalseString), "Build was not finished")
o.Eventually(mosb, "10m", "20s").Should(HaveConditionField("Succeeded", "status", TrueString), "Build didn't succeed")
o.Eventually(mosb, "2m", "20s").Should(HaveConditionField("Interrupted", "status", FalseString), "Build was interrupted")
o.Eventually(mosb, "2m", "20s").Should(HaveConditionField("Failed", "status", FalseString), "Build was failed")
Expand Down Expand Up @@ -699,8 +700,9 @@ func checkUsbguradExtension(node *Node) {
rpmName = AllExtenstions[extName]
activeString = "Active: active (running)"
inactiveString = "Active: inactive (dead)"
expectedError = "missing /usr/lib/tmpfiles.d/usbguard.conf\nerror: non-zero exit code from debug container"
expectedErrorOCL = ".M....... /var/log/usbguard\nerror: non-zero exit code from debug container"
expectedError = "missing /usr/lib/tmpfiles.d/usbguard.conf\nerror: non-zero exit code from debug container"
expectedErrorOCL = ".M....... /var/log/usbguard\nerror: non-zero exit code from debug container"
expectedErrorCombined = "missing /usr/lib/tmpfiles.d/usbguard.conf\n.M....... /var/log/usbguard\nerror: non-zero exit code from debug container"
)
exutil.By("Verify node includes Usbguard extension")
o.Expect(
Expand Down Expand Up @@ -729,7 +731,7 @@ func checkUsbguradExtension(node *Node) {

exutil.By("Check that all the files in the rpm were correctly deployed ")
rpmOut, _ := node.checkRpmFiles(rpmName...)
o.Expect(strings.TrimSpace(rpmOut)).Should(o.Or(o.BeEmpty(), o.Equal(expectedErrorOCL), o.Equal(expectedError)), "Error in permissions of rpm files")
o.Expect(strings.TrimSpace(rpmOut)).Should(o.Or(o.BeEmpty(), o.Equal(expectedErrorOCL), o.Equal(expectedError), o.Equal(expectedErrorCombined)), "Error in permissions of rpm files")
logger.Infof("OK!\n")
}

Expand Down
Loading