diff --git a/cmd/openshift-install/agent/waitfor.go b/cmd/openshift-install/agent/waitfor.go index 05c128d2886..94098779376 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. @@ -124,15 +125,22 @@ func newWaitForInstallCompleteCmd() *cobra.Command { // Load install-config to check if FIPS verification is needed var fipsEnabled bool - if installConfigAsset, err := assetStore.Load(&agentasset.OptionalInstallConfig{}); err == nil && installConfigAsset != nil { + + var expectedMasters, expectedWorkers int + 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 + expectedMasters, expectedWorkers = extractExpectedNodeCounts(ic.Config) } } options := command.WaitOptions{ - VerifyFIPS: fipsEnabled, + VerifyFIPS: fipsEnabled, + ExpectedMasterNodes: expectedMasters, + ExpectedWorkerNodes: expectedWorkers, } if err = command.WaitForInstallComplete(ctx, cluster.API.Kube.Config, options); err != nil { @@ -148,3 +156,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 d096ab60ef7..606bd4df575 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,74 @@ 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") + } + + 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", + }) + 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 +207,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) 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..72325e1be2a --- /dev/null +++ b/cmd/openshift-install/command/waitfor_node_test.go @@ -0,0 +1,153 @@ +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: "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{ + 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) + } + } + }) + } +}