From 62417fd1348a4bd1e7aa237bcced35e6cf0825dd Mon Sep 17 00:00:00 2001 From: Savio Menezes Date: Fri, 24 Jul 2026 13:11:58 +0530 Subject: [PATCH 1/4] OCPBUGS-95069: agent/waitfor: verify expected nodes before declaring install complete --- cmd/openshift-install/agent/waitfor.go | 14 ++++- cmd/openshift-install/command/waitfor.go | 73 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/cmd/openshift-install/agent/waitfor.go b/cmd/openshift-install/agent/waitfor.go index 05c128d2886..1c102f6ae4e 100644 --- a/cmd/openshift-install/agent/waitfor.go +++ b/cmd/openshift-install/agent/waitfor.go @@ -124,15 +124,27 @@ func newWaitForInstallCompleteCmd() *cobra.Command { // Load install-config to check if FIPS verification is needed var fipsEnabled bool + + var expectedMasters, expectedWorkers int if installConfigAsset, err := assetStore.Load(&agentasset.OptionalInstallConfig{}); err == nil && installConfigAsset != nil { ic := installConfigAsset.(*agentasset.OptionalInstallConfig) if ic.Config != nil { fipsEnabled = ic.Config.FIPS + if ic.Config.ControlPlane != nil && ic.Config.ControlPlane.Replicas != nil { + expectedMasters = int(*ic.Config.ControlPlane.Replicas) + } + for _, pool := range ic.Config.Compute { + if pool.Replicas != nil { + expectedWorkers += int(*pool.Replicas) + } + } } } options := command.WaitOptions{ - VerifyFIPS: fipsEnabled, + VerifyFIPS: fipsEnabled, + ExpectedMasterNodes: expectedMasters, + ExpectedWorkerNodes: expectedWorkers, } if err = command.WaitForInstallComplete(ctx, cluster.API.Kube.Config, options); err != nil { diff --git a/cmd/openshift-install/command/waitfor.go b/cmd/openshift-install/command/waitfor.go index d096ab60ef7..660d3925601 100644 --- a/cmd/openshift-install/command/waitfor.go +++ b/cmd/openshift-install/command/waitfor.go @@ -11,6 +11,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -67,6 +68,10 @@ type WaitOptions struct { UserProvisionedDNSEnabled bool // VerifyFIPS verifies that FIPS mode is enabled on the cluster before completing. VerifyFIPS bool + // ExpectedMasterNodes is the number of control plane nodes expected from install-config. + ExpectedMasterNodes int + // ExpectedWorkerNodes is the number of compute nodes expected from install-config. + ExpectedWorkerNodes int } // verifyFIPSEnabled checks that the cluster has FIPS enabled by querying @@ -113,6 +118,70 @@ func verifyFIPSEnabled(ctx context.Context, config *rest.Config) error { return nil } +// verifyExpectedNodes checks that the expected number of master and worker nodes +// are present and in Ready state. Returns an error if verification fails. +func verifyExpectedNodes(ctx context.Context, config *rest.Config, expectedMasters, expectedWorkers int) error { + if expectedMasters == 0 && expectedWorkers == 0 { + return nil + } + + client, err := kubernetes.NewForConfig(config) + if err != nil { + return errors.Wrap(err, "failed to create Kubernetes client for node verification") + } + + if expectedMasters > 0 { + masterNodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{ + LabelSelector: "node-role.kubernetes.io/master", + }) + if err != nil { + return errors.Wrap(err, "failed to list master nodes") + } + + readyMasters := 0 + for i := range masterNodes.Items { + for _, condition := range masterNodes.Items[i].Status.Conditions { + if condition.Type == corev1.NodeReady && condition.Status == corev1.ConditionTrue { + readyMasters++ + break + } + } + } + + if readyMasters < expectedMasters { + return fmt.Errorf("expected %d master node(s) to be Ready but only %d found", expectedMasters, readyMasters) + } + logrus.Debugf("Verified %d/%d master node(s) are Ready", readyMasters, expectedMasters) + } + + if expectedWorkers > 0 { + workerNodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{ + LabelSelector: "node-role.kubernetes.io/worker", + }) + if err != nil { + return errors.Wrap(err, "failed to list worker nodes") + } + + readyWorkers := 0 + for i := range workerNodes.Items { + for _, condition := range workerNodes.Items[i].Status.Conditions { + if condition.Type == corev1.NodeReady && condition.Status == corev1.ConditionTrue { + readyWorkers++ + break + } + } + } + + if readyWorkers < expectedWorkers { + return fmt.Errorf("expected %d worker node(s) to be Ready but only %d found", expectedWorkers, readyWorkers) + } + logrus.Debugf("Verified %d/%d worker node(s) are Ready", readyWorkers, expectedWorkers) + } + + logrus.Info("Verified expected nodes are present and Ready") + return nil +} + // WaitForInstallComplete waits for cluster to complete installation, checks for operator stability // and logs cluster information when successful. func WaitForInstallComplete(ctx context.Context, config *rest.Config, options WaitOptions) error { @@ -134,6 +203,10 @@ func WaitForInstallComplete(ctx context.Context, config *rest.Config, options Wa } } + if err := verifyExpectedNodes(ctx, config, options.ExpectedMasterNodes, options.ExpectedWorkerNodes); err != nil { + return err + } + consoleURL, err := getConsole(ctx, config) if err != nil { logrus.Warnf("Cluster does not have a console available: %v", err) From 9cfa60219d0545d693a7a1178b8e1ed949b10ba5 Mon Sep 17 00:00:00 2001 From: Savio Menezes Date: Fri, 24 Jul 2026 18:03:23 +0530 Subject: [PATCH 2/4] OCPBUGS-95069: add unit tests for node verification and extract helper --- cmd/openshift-install/agent/waitfor.go | 25 ++- cmd/openshift-install/agent/waitfor_test.go | 145 ++++++++++++++++++ cmd/openshift-install/command/waitfor.go | 4 + .../command/waitfor_node_test.go | 145 ++++++++++++++++++ 4 files changed, 311 insertions(+), 8 deletions(-) create mode 100644 cmd/openshift-install/agent/waitfor_test.go create mode 100644 cmd/openshift-install/command/waitfor_node_test.go diff --git a/cmd/openshift-install/agent/waitfor.go b/cmd/openshift-install/agent/waitfor.go index 1c102f6ae4e..b8bb2b42cbe 100644 --- a/cmd/openshift-install/agent/waitfor.go +++ b/cmd/openshift-install/agent/waitfor.go @@ -14,6 +14,7 @@ import ( agentasset "github.com/openshift/installer/pkg/asset/agent" "github.com/openshift/installer/pkg/asset/agent/workflow" assetstore "github.com/openshift/installer/pkg/asset/store" + "github.com/openshift/installer/pkg/types" ) // NewWaitForCmd create the commands for waiting the completion of the agent based cluster installation. @@ -130,14 +131,7 @@ func newWaitForInstallCompleteCmd() *cobra.Command { ic := installConfigAsset.(*agentasset.OptionalInstallConfig) if ic.Config != nil { fipsEnabled = ic.Config.FIPS - if ic.Config.ControlPlane != nil && ic.Config.ControlPlane.Replicas != nil { - expectedMasters = int(*ic.Config.ControlPlane.Replicas) - } - for _, pool := range ic.Config.Compute { - if pool.Replicas != nil { - expectedWorkers += int(*pool.Replicas) - } - } + expectedMasters, expectedWorkers = extractExpectedNodeCounts(ic.Config) } } @@ -160,3 +154,18 @@ func newWaitForInstallCompleteCmd() *cobra.Command { }, } } + +func extractExpectedNodeCounts(ic *types.InstallConfig) (masters int, workers int) { + if ic == nil { + return 0, 0 + } + if ic.ControlPlane != nil && ic.ControlPlane.Replicas != nil { + masters = int(*ic.ControlPlane.Replicas) + } + for _, pool := range ic.Compute { + if pool.Replicas != nil { + workers += int(*pool.Replicas) + } + } + return masters, workers +} diff --git a/cmd/openshift-install/agent/waitfor_test.go b/cmd/openshift-install/agent/waitfor_test.go new file mode 100644 index 00000000000..d25aee33870 --- /dev/null +++ b/cmd/openshift-install/agent/waitfor_test.go @@ -0,0 +1,145 @@ +package agent + +import ( + "testing" + + "github.com/openshift/installer/pkg/types" +) + +func int64Ptr(i int64) *int64 { + return &i +} + +func TestExtractExpectedNodeCounts(t *testing.T) { + tests := []struct { + name string + config *types.InstallConfig + expectedMasters int + expectedWorkers int + }{ + { + name: "nil config", + config: nil, + expectedMasters: 0, + expectedWorkers: 0, + }, + { + name: "3 masters and 2 workers", + config: &types.InstallConfig{ + ControlPlane: &types.MachinePool{ + Name: "master", + Replicas: int64Ptr(3), + }, + Compute: []types.MachinePool{ + { + Name: "worker", + Replicas: int64Ptr(2), + }, + }, + }, + expectedMasters: 3, + expectedWorkers: 2, + }, + { + name: "compact cluster with 3 masters and 0 workers", + config: &types.InstallConfig{ + ControlPlane: &types.MachinePool{ + Name: "master", + Replicas: int64Ptr(3), + }, + Compute: []types.MachinePool{ + { + Name: "worker", + Replicas: int64Ptr(0), + }, + }, + }, + expectedMasters: 3, + expectedWorkers: 0, + }, + { + name: "single node openshift", + config: &types.InstallConfig{ + ControlPlane: &types.MachinePool{ + Name: "master", + Replicas: int64Ptr(1), + }, + Compute: []types.MachinePool{ + { + Name: "worker", + Replicas: int64Ptr(0), + }, + }, + }, + expectedMasters: 1, + expectedWorkers: 0, + }, + { + name: "multiple compute pools", + config: &types.InstallConfig{ + ControlPlane: &types.MachinePool{ + Name: "master", + Replicas: int64Ptr(3), + }, + Compute: []types.MachinePool{ + { + Name: "worker", + Replicas: int64Ptr(2), + }, + { + Name: "worker", + Replicas: int64Ptr(3), + }, + }, + }, + expectedMasters: 3, + expectedWorkers: 5, + }, + { + name: "nil replicas fields", + config: &types.InstallConfig{ + ControlPlane: &types.MachinePool{ + Name: "master", + }, + Compute: []types.MachinePool{ + { + Name: "worker", + }, + }, + }, + expectedMasters: 0, + expectedWorkers: 0, + }, + { + name: "nil control plane with workers", + config: &types.InstallConfig{ + Compute: []types.MachinePool{ + { + Name: "worker", + Replicas: int64Ptr(2), + }, + }, + }, + expectedMasters: 0, + expectedWorkers: 2, + }, + { + name: "empty config", + config: &types.InstallConfig{}, + expectedMasters: 0, + expectedWorkers: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + masters, workers := extractExpectedNodeCounts(tt.config) + if masters != tt.expectedMasters { + t.Errorf("expected %d masters but got %d", tt.expectedMasters, masters) + } + if workers != tt.expectedWorkers { + t.Errorf("expected %d workers but got %d", tt.expectedWorkers, workers) + } + }) + } +} diff --git a/cmd/openshift-install/command/waitfor.go b/cmd/openshift-install/command/waitfor.go index 660d3925601..606bd4df575 100644 --- a/cmd/openshift-install/command/waitfor.go +++ b/cmd/openshift-install/command/waitfor.go @@ -130,6 +130,10 @@ func verifyExpectedNodes(ctx context.Context, config *rest.Config, expectedMaste return errors.Wrap(err, "failed to create Kubernetes client for node verification") } + return verifyExpectedNodesWithClient(ctx, client, expectedMasters, expectedWorkers) +} + +func verifyExpectedNodesWithClient(ctx context.Context, client kubernetes.Interface, expectedMasters, expectedWorkers int) error { if expectedMasters > 0 { masterNodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: "node-role.kubernetes.io/master", diff --git a/cmd/openshift-install/command/waitfor_node_test.go b/cmd/openshift-install/command/waitfor_node_test.go new file mode 100644 index 00000000000..92d50a3a305 --- /dev/null +++ b/cmd/openshift-install/command/waitfor_node_test.go @@ -0,0 +1,145 @@ +package command + +import ( + "context" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" +) + +func newNode(name, role string, ready bool) *corev1.Node { + status := corev1.ConditionFalse + if ready { + status = corev1.ConditionTrue + } + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + "node-role.kubernetes.io/" + role: "", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: status, + }, + }, + }, + } +} + +func TestVerifyExpectedNodesWithClient(t *testing.T) { + tests := []struct { + name string + nodes []*corev1.Node + expectedMasters int + expectedWorkers int + expectErr bool + errContains string + }{ + { + name: "all masters ready", + nodes: []*corev1.Node{newNode("master-0", "master", true), newNode("master-1", "master", true), newNode("master-2", "master", true)}, + expectedMasters: 3, + expectedWorkers: 0, + expectErr: false, + }, + { + name: "missing master node", + nodes: []*corev1.Node{newNode("master-0", "master", true), newNode("master-1", "master", true)}, + expectedMasters: 3, + expectedWorkers: 0, + expectErr: true, + errContains: "expected 3 master node(s) to be Ready but only 2 found", + }, + { + name: "master node not ready", + nodes: []*corev1.Node{newNode("master-0", "master", true), newNode("master-1", "master", true), newNode("master-2", "master", false)}, + expectedMasters: 3, + expectedWorkers: 0, + expectErr: true, + errContains: "expected 3 master node(s) to be Ready but only 2 found", + }, + { + name: "all workers ready", + nodes: []*corev1.Node{newNode("worker-0", "worker", true), newNode("worker-1", "worker", true)}, + expectedMasters: 0, + expectedWorkers: 2, + expectErr: false, + }, + { + name: "missing worker node", + nodes: []*corev1.Node{newNode("worker-0", "worker", true)}, + expectedMasters: 0, + expectedWorkers: 2, + expectErr: true, + errContains: "expected 2 worker node(s) to be Ready but only 1 found", + }, + { + name: "masters and workers all ready", + nodes: []*corev1.Node{ + newNode("master-0", "master", true), newNode("master-1", "master", true), newNode("master-2", "master", true), + newNode("worker-0", "worker", true), newNode("worker-1", "worker", true), + }, + expectedMasters: 3, + expectedWorkers: 2, + expectErr: false, + }, + { + name: "masters ready but workers missing", + nodes: []*corev1.Node{ + newNode("master-0", "master", true), newNode("master-1", "master", true), newNode("master-2", "master", true), + newNode("worker-0", "worker", true), + }, + expectedMasters: 3, + expectedWorkers: 2, + expectErr: true, + errContains: "expected 2 worker node(s) to be Ready but only 1 found", + }, + { + name: "no nodes exist at all", + nodes: []*corev1.Node{}, + expectedMasters: 3, + expectedWorkers: 0, + expectErr: true, + errContains: "expected 3 master node(s) to be Ready but only 0 found", + }, + { + name: "zero expectations succeeds with no nodes", + nodes: []*corev1.Node{}, + expectedMasters: 0, + expectedWorkers: 0, + expectErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runtimeObjs := make([]runtime.Object, len(tt.nodes)) + for i, n := range tt.nodes { + runtimeObjs[i] = n + } + client := fake.NewSimpleClientset(runtimeObjs...) + + err := verifyExpectedNodesWithClient(context.Background(), client, tt.expectedMasters, tt.expectedWorkers) + + if tt.expectErr && err == nil { + t.Errorf("expected error but got nil") + } + if !tt.expectErr && err != nil { + t.Errorf("expected no error but got: %v", err) + } + if tt.expectErr && err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("expected error containing %q but got: %v", tt.errContains, err) + } + } + }) + } +} From f03ff97e632494d1fc64bd1313feb67ff4c888bf Mon Sep 17 00:00:00 2001 From: Savio Menezes Date: Fri, 24 Jul 2026 21:40:47 +0530 Subject: [PATCH 3/4] OCPBUGS-95069: warn when install-config fails to load Log a warning instead of silently skipping FIPS and node verification when the install-config cannot be loaded. Co-authored-by: Cursor --- cmd/openshift-install/agent/waitfor.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/openshift-install/agent/waitfor.go b/cmd/openshift-install/agent/waitfor.go index b8bb2b42cbe..94098779376 100644 --- a/cmd/openshift-install/agent/waitfor.go +++ b/cmd/openshift-install/agent/waitfor.go @@ -127,7 +127,9 @@ func newWaitForInstallCompleteCmd() *cobra.Command { var fipsEnabled bool var expectedMasters, expectedWorkers int - if installConfigAsset, err := assetStore.Load(&agentasset.OptionalInstallConfig{}); err == nil && installConfigAsset != nil { + if installConfigAsset, err := assetStore.Load(&agentasset.OptionalInstallConfig{}); err != nil { + logrus.Warnf("Failed to load install-config, skipping FIPS and node verification: %v", err) + } else if installConfigAsset != nil { ic := installConfigAsset.(*agentasset.OptionalInstallConfig) if ic.Config != nil { fipsEnabled = ic.Config.FIPS From 2341128fc2899fa5a254fc85a472b8366db79891 Mon Sep 17 00:00:00 2001 From: Savio Menezes Date: Fri, 24 Jul 2026 23:01:50 +0530 Subject: [PATCH 4/4] OCPBUGS-95069: add worker-not-ready test case Cover the branch where a worker node exists but has NodeReady=False, mirroring the existing master-not-ready test. Co-authored-by: Cursor --- cmd/openshift-install/command/waitfor_node_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/openshift-install/command/waitfor_node_test.go b/cmd/openshift-install/command/waitfor_node_test.go index 92d50a3a305..72325e1be2a 100644 --- a/cmd/openshift-install/command/waitfor_node_test.go +++ b/cmd/openshift-install/command/waitfor_node_test.go @@ -81,6 +81,14 @@ func TestVerifyExpectedNodesWithClient(t *testing.T) { expectErr: true, errContains: "expected 2 worker node(s) to be Ready but only 1 found", }, + { + name: "worker node not ready", + nodes: []*corev1.Node{newNode("worker-0", "worker", true), newNode("worker-1", "worker", false)}, + expectedMasters: 0, + expectedWorkers: 2, + expectErr: true, + errContains: "expected 2 worker node(s) to be Ready but only 1 found", + }, { name: "masters and workers all ready", nodes: []*corev1.Node{