From b44e22d41cbf5e5ab1b555980ea851f73883c921 Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Fri, 24 Jul 2026 16:02:36 -0400 Subject: [PATCH 1/2] Add startup probe to CLI and VMDP download servers --- .../controller/cli_download_controller.go | 46 +++- .../cli_download_controller_test.go | 198 ++++++++++++++++++ .../controller/vmdp_download_controller.go | 33 ++- .../vmdp_download_controller_test.go | 184 ++++++++++++++++ 4 files changed, 449 insertions(+), 12 deletions(-) diff --git a/internal/controller/cli_download_controller.go b/internal/controller/cli_download_controller.go index 537804347b..032b66a5e3 100644 --- a/internal/controller/cli_download_controller.go +++ b/internal/controller/cli_download_controller.go @@ -171,11 +171,21 @@ func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDe // Deployment exists from a version before probes/service account were added; backfill them. desired := buildCLIServerDeployment(c.Namespace, cliServerImage) needsUpdate := false - if len(deployment.Spec.Template.Spec.Containers) > 0 && - deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil { - deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = desired.Spec.Template.Spec.Containers[0].ReadinessProbe - deployment.Spec.Template.Spec.Containers[0].LivenessProbe = desired.Spec.Template.Spec.Containers[0].LivenessProbe - needsUpdate = true + if idx := findContainerIndexByName(deployment.Spec.Template.Spec.Containers, "oadp-cli-server"); idx != -1 { + desiredContainer := desired.Spec.Template.Spec.Containers[0] + currentContainer := &deployment.Spec.Template.Spec.Containers[idx] + if currentContainer.ReadinessProbe == nil && desiredContainer.ReadinessProbe != nil { + currentContainer.ReadinessProbe = desiredContainer.ReadinessProbe + needsUpdate = true + } + if currentContainer.LivenessProbe == nil && desiredContainer.LivenessProbe != nil { + currentContainer.LivenessProbe = desiredContainer.LivenessProbe + needsUpdate = true + } + if currentContainer.StartupProbe == nil && desiredContainer.StartupProbe != nil { + currentContainer.StartupProbe = desiredContainer.StartupProbe + needsUpdate = true + } } if deployment.Spec.Template.Spec.ServiceAccountName != cliServerServiceAccountName { deployment.Spec.Template.Spec.ServiceAccountName = desired.Spec.Template.Spec.ServiceAccountName @@ -191,7 +201,7 @@ func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDe if err := c.Client.Update(ctx, deployment); err != nil { return fmt.Errorf("failed to update CLI server deployment: %w", err) } - c.Log.Info("Updated CLI server deployment with readiness/liveness probes and/or service account") + c.Log.Info("Updated CLI server deployment with probes and/or service account") } } @@ -444,6 +454,17 @@ func buildCLIServerDeployment(namespace, image string) *appsv1.Deployment { InitialDelaySeconds: 15, PeriodSeconds: 20, }, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromString("http"), + }, + }, + InitialDelaySeconds: 5, + PeriodSeconds: 5, + FailureThreshold: 12, + }, }, }, TerminationGracePeriodSeconds: int64Ptr(10), @@ -524,3 +545,16 @@ func buildCLIServerRoute(namespace string) *routev1.Route { func int64Ptr(i int64) *int64 { return &i } + +// findContainerIndexByName returns the index of the container with the given +// name, or -1 if no such container exists. Used to avoid assuming the target +// server container is always at index 0, which may not hold if a sidecar is +// injected or the container order changes. +func findContainerIndexByName(containers []corev1.Container, name string) int { + for i := range containers { + if containers[i].Name == name { + return i + } + } + return -1 +} diff --git a/internal/controller/cli_download_controller_test.go b/internal/controller/cli_download_controller_test.go index 60bd7b095b..7e4fc3a943 100644 --- a/internal/controller/cli_download_controller_test.go +++ b/internal/controller/cli_download_controller_test.go @@ -5,14 +5,41 @@ import ( "testing" "github.com/go-logr/logr" + consolev1 "github.com/openshift/api/console/v1" + routev1 "github.com/openshift/api/route/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +// getDownloadTestScheme returns a scheme with the console/route API groups +// registered, suitable for use with a fake client in CLI/VMDP reconciliation tests. +func getDownloadTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + if err := consolev1.AddToScheme(scheme.Scheme); err != nil { + t.Fatalf("failed to add consolev1 to scheme: %v", err) + } + if err := routev1.AddToScheme(scheme.Scheme); err != nil { + t.Fatalf("failed to add routev1 to scheme: %v", err) + } + return scheme.Scheme +} + +// newTestCLIRoute returns a CLI server route with a hostname already +// assigned, so reconcileCLIResources doesn't hit its hostname-assignment +// retry/backoff path. +func newTestCLIRoute(namespace string) *routev1.Route { + route := buildCLIServerRoute(namespace) + route.Spec.Host = "cli.example.com" + return route +} + func TestBuildCLIServerDeployment_ServiceAccount(t *testing.T) { deployment := buildCLIServerDeployment("openshift-adp", "test-image") podSpec := deployment.Spec.Template.Spec @@ -234,3 +261,174 @@ func TestReconcileCLIResources_NoopWhenServiceAccountAlreadyCorrect(t *testing.T t.Errorf("expected no update (ResourceVersion unchanged), got before=%s after=%s", before.ResourceVersion, after.ResourceVersion) } } + +func TestBuildCLIServerDeployment_StartupProbe(t *testing.T) { + deployment := buildCLIServerDeployment("openshift-adp", "test-image") + + if len(deployment.Spec.Template.Spec.Containers) == 0 { + t.Fatal("expected at least one container") + } + container := deployment.Spec.Template.Spec.Containers[0] + + if container.StartupProbe == nil { + t.Fatal("expected StartupProbe to be set") + } + if container.StartupProbe.HTTPGet == nil { + t.Fatal("expected StartupProbe to use HTTPGet") + } + if container.StartupProbe.HTTPGet.Path != "/" { + t.Errorf("expected StartupProbe path \"/\", got %q", container.StartupProbe.HTTPGet.Path) + } + if container.StartupProbe.HTTPGet.Port != intstr.FromString("http") { + t.Errorf("expected StartupProbe port \"http\", got %v", container.StartupProbe.HTTPGet.Port) + } + if container.StartupProbe.FailureThreshold != 12 { + t.Errorf("expected StartupProbe failureThreshold 12, got %d", container.StartupProbe.FailureThreshold) + } + if container.StartupProbe.InitialDelaySeconds != 5 { + t.Errorf("expected StartupProbe initialDelaySeconds 5, got %d", container.StartupProbe.InitialDelaySeconds) + } + if container.StartupProbe.PeriodSeconds != 5 { + t.Errorf("expected StartupProbe periodSeconds 5, got %d", container.StartupProbe.PeriodSeconds) + } + + if container.ReadinessProbe == nil { + t.Fatal("expected ReadinessProbe to remain set") + } + if container.LivenessProbe == nil { + t.Fatal("expected LivenessProbe to remain set") + } +} + +func TestReconcileCLIResources_BackfillsMissingProbes(t *testing.T) { + namespace := "openshift-adp" + testScheme := getDownloadTestScheme(t) + + operatorDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "oadp-operator", Namespace: namespace}, + } + + // Simulate a Deployment created by a pre-fix version of the operator: it has + // the server container, but no probes configured at all. + existingDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: cliServerDeploymentName, Namespace: namespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "oadp-cli-server", Image: "old-image"}, + }, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(existingDeployment, newTestCLIRoute(namespace)). + Build() + + setup := &CLIDownloadSetup{ + Client: fakeClient, + Namespace: namespace, + OperatorName: "oadp-operator", + OperatorNamespace: namespace, + Log: logr.Discard(), + } + + if err := setup.reconcileCLIResources(context.Background(), operatorDeployment, "test-image"); err != nil { + t.Fatalf("reconcileCLIResources returned error: %v", err) + } + + updated := &appsv1.Deployment{} + if err := fakeClient.Get(context.Background(), client.ObjectKey{Name: cliServerDeploymentName, Namespace: namespace}, updated); err != nil { + t.Fatalf("failed to get updated deployment: %v", err) + } + container := updated.Spec.Template.Spec.Containers[0] + + if container.ReadinessProbe == nil { + t.Error("expected ReadinessProbe to be backfilled") + } + if container.LivenessProbe == nil { + t.Error("expected LivenessProbe to be backfilled") + } + if container.StartupProbe == nil { + t.Error("expected StartupProbe to be backfilled") + } + // The image on an existing deployment should not be clobbered by the backfill. + if container.Image != "old-image" { + t.Errorf("expected existing image to be preserved, got %q", container.Image) + } +} + +func TestReconcileCLIResources_DoesNotOverwriteExistingProbes(t *testing.T) { + namespace := "openshift-adp" + testScheme := getDownloadTestScheme(t) + + operatorDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "oadp-operator", Namespace: namespace}, + } + + customProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/custom", Port: intstr.FromString("http")}, + }, + InitialDelaySeconds: 99, + PeriodSeconds: 99, + } + + // Simulate a Deployment that already has probes configured (e.g. from a + // prior reconcile, or customized by a user) to ensure the backfill logic + // doesn't clobber them. + existingDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: cliServerDeploymentName, Namespace: namespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "oadp-cli-server", + Image: "old-image", + ReadinessProbe: customProbe.DeepCopy(), + LivenessProbe: customProbe.DeepCopy(), + StartupProbe: customProbe.DeepCopy(), + }, + }, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(existingDeployment, newTestCLIRoute(namespace)). + Build() + + setup := &CLIDownloadSetup{ + Client: fakeClient, + Namespace: namespace, + OperatorName: "oadp-operator", + OperatorNamespace: namespace, + Log: logr.Discard(), + } + + if err := setup.reconcileCLIResources(context.Background(), operatorDeployment, "test-image"); err != nil { + t.Fatalf("reconcileCLIResources returned error: %v", err) + } + + updated := &appsv1.Deployment{} + if err := fakeClient.Get(context.Background(), client.ObjectKey{Name: cliServerDeploymentName, Namespace: namespace}, updated); err != nil { + t.Fatalf("failed to get updated deployment: %v", err) + } + container := updated.Spec.Template.Spec.Containers[0] + + if container.ReadinessProbe.InitialDelaySeconds != 99 { + t.Errorf("expected existing ReadinessProbe to be preserved, got InitialDelaySeconds=%d", container.ReadinessProbe.InitialDelaySeconds) + } + if container.LivenessProbe.InitialDelaySeconds != 99 { + t.Errorf("expected existing LivenessProbe to be preserved, got InitialDelaySeconds=%d", container.LivenessProbe.InitialDelaySeconds) + } + if container.StartupProbe.InitialDelaySeconds != 99 { + t.Errorf("expected existing StartupProbe to be preserved, got InitialDelaySeconds=%d", container.StartupProbe.InitialDelaySeconds) + } +} diff --git a/internal/controller/vmdp_download_controller.go b/internal/controller/vmdp_download_controller.go index 45b9619611..efd1bf3403 100644 --- a/internal/controller/vmdp_download_controller.go +++ b/internal/controller/vmdp_download_controller.go @@ -166,11 +166,21 @@ func (v *VMDPDownloadSetup) reconcileVMDPResources(ctx context.Context, operator // Deployment exists from a version before probes/service account were added; backfill them. desired := buildVMDPServerDeployment(v.Namespace, vmdpServerImage) needsUpdate := false - if len(deployment.Spec.Template.Spec.Containers) > 0 && - deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil { - deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = desired.Spec.Template.Spec.Containers[0].ReadinessProbe - deployment.Spec.Template.Spec.Containers[0].LivenessProbe = desired.Spec.Template.Spec.Containers[0].LivenessProbe - needsUpdate = true + if idx := findContainerIndexByName(deployment.Spec.Template.Spec.Containers, "oadp-vmdp-server"); idx != -1 { + desiredContainer := desired.Spec.Template.Spec.Containers[0] + currentContainer := &deployment.Spec.Template.Spec.Containers[idx] + if currentContainer.ReadinessProbe == nil && desiredContainer.ReadinessProbe != nil { + currentContainer.ReadinessProbe = desiredContainer.ReadinessProbe + needsUpdate = true + } + if currentContainer.LivenessProbe == nil && desiredContainer.LivenessProbe != nil { + currentContainer.LivenessProbe = desiredContainer.LivenessProbe + needsUpdate = true + } + if currentContainer.StartupProbe == nil && desiredContainer.StartupProbe != nil { + currentContainer.StartupProbe = desiredContainer.StartupProbe + needsUpdate = true + } } if deployment.Spec.Template.Spec.ServiceAccountName != vmdpServerServiceAccountName { deployment.Spec.Template.Spec.ServiceAccountName = desired.Spec.Template.Spec.ServiceAccountName @@ -186,7 +196,7 @@ func (v *VMDPDownloadSetup) reconcileVMDPResources(ctx context.Context, operator if err := v.Client.Update(ctx, deployment); err != nil { return fmt.Errorf("failed to update VMDP server deployment: %w", err) } - v.Log.Info("Updated VMDP server deployment with readiness/liveness probes and/or service account") + v.Log.Info("Updated VMDP server deployment with probes and/or service account") } } @@ -426,6 +436,17 @@ func buildVMDPServerDeployment(namespace, image string) *appsv1.Deployment { InitialDelaySeconds: 15, PeriodSeconds: 20, }, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromString("http"), + }, + }, + InitialDelaySeconds: 5, + PeriodSeconds: 5, + FailureThreshold: 12, + }, }, }, TerminationGracePeriodSeconds: int64Ptr(10), diff --git a/internal/controller/vmdp_download_controller_test.go b/internal/controller/vmdp_download_controller_test.go index 3212079365..882b00961c 100644 --- a/internal/controller/vmdp_download_controller_test.go +++ b/internal/controller/vmdp_download_controller_test.go @@ -5,12 +5,25 @@ import ( "testing" "github.com/go-logr/logr" + routev1 "github.com/openshift/api/route/v1" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +// newTestVMDPRoute returns a VMDP server route with a hostname already +// assigned, so reconcileVMDPResources doesn't hit its hostname-assignment +// retry/backoff path. +func newTestVMDPRoute(namespace string) *routev1.Route { + route := buildVMDPServerRoute(namespace) + route.Spec.Host = "vmdp.example.com" + return route +} + func TestBuildVMDPServerDeployment_ServiceAccount(t *testing.T) { deployment := buildVMDPServerDeployment("openshift-adp", "test-image") podSpec := deployment.Spec.Template.Spec @@ -208,3 +221,174 @@ func TestReconcileVMDPResources_NoopWhenServiceAccountAlreadyCorrect(t *testing. t.Errorf("expected no update (ResourceVersion unchanged), got before=%s after=%s", before.ResourceVersion, after.ResourceVersion) } } + +func TestBuildVMDPServerDeployment_StartupProbe(t *testing.T) { + deployment := buildVMDPServerDeployment("openshift-adp", "test-image") + + if len(deployment.Spec.Template.Spec.Containers) == 0 { + t.Fatal("expected at least one container") + } + container := deployment.Spec.Template.Spec.Containers[0] + + if container.StartupProbe == nil { + t.Fatal("expected StartupProbe to be set") + } + if container.StartupProbe.HTTPGet == nil { + t.Fatal("expected StartupProbe to use HTTPGet") + } + if container.StartupProbe.HTTPGet.Path != "/" { + t.Errorf("expected StartupProbe path \"/\", got %q", container.StartupProbe.HTTPGet.Path) + } + if container.StartupProbe.HTTPGet.Port != intstr.FromString("http") { + t.Errorf("expected StartupProbe port \"http\", got %v", container.StartupProbe.HTTPGet.Port) + } + if container.StartupProbe.FailureThreshold != 12 { + t.Errorf("expected StartupProbe failureThreshold 12, got %d", container.StartupProbe.FailureThreshold) + } + if container.StartupProbe.InitialDelaySeconds != 5 { + t.Errorf("expected StartupProbe initialDelaySeconds 5, got %d", container.StartupProbe.InitialDelaySeconds) + } + if container.StartupProbe.PeriodSeconds != 5 { + t.Errorf("expected StartupProbe periodSeconds 5, got %d", container.StartupProbe.PeriodSeconds) + } + + if container.ReadinessProbe == nil { + t.Fatal("expected ReadinessProbe to remain set") + } + if container.LivenessProbe == nil { + t.Fatal("expected LivenessProbe to remain set") + } +} + +func TestReconcileVMDPResources_BackfillsMissingProbes(t *testing.T) { + namespace := "openshift-adp" + testScheme := getDownloadTestScheme(t) + + operatorDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "oadp-operator", Namespace: namespace}, + } + + // Simulate a Deployment created by a pre-fix version of the operator: it has + // the server container, but no probes configured at all. + existingDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: vmdpServerDeploymentName, Namespace: namespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "oadp-vmdp-server", Image: "old-image"}, + }, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(existingDeployment, newTestVMDPRoute(namespace)). + Build() + + setup := &VMDPDownloadSetup{ + Client: fakeClient, + Namespace: namespace, + OperatorName: "oadp-operator", + OperatorNamespace: namespace, + Log: logr.Discard(), + } + + if err := setup.reconcileVMDPResources(context.Background(), operatorDeployment, "test-image"); err != nil { + t.Fatalf("reconcileVMDPResources returned error: %v", err) + } + + updated := &appsv1.Deployment{} + if err := fakeClient.Get(context.Background(), client.ObjectKey{Name: vmdpServerDeploymentName, Namespace: namespace}, updated); err != nil { + t.Fatalf("failed to get updated deployment: %v", err) + } + container := updated.Spec.Template.Spec.Containers[0] + + if container.ReadinessProbe == nil { + t.Error("expected ReadinessProbe to be backfilled") + } + if container.LivenessProbe == nil { + t.Error("expected LivenessProbe to be backfilled") + } + if container.StartupProbe == nil { + t.Error("expected StartupProbe to be backfilled") + } + // The image on an existing deployment should not be clobbered by the backfill. + if container.Image != "old-image" { + t.Errorf("expected existing image to be preserved, got %q", container.Image) + } +} + +func TestReconcileVMDPResources_DoesNotOverwriteExistingProbes(t *testing.T) { + namespace := "openshift-adp" + testScheme := getDownloadTestScheme(t) + + operatorDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "oadp-operator", Namespace: namespace}, + } + + customProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/custom", Port: intstr.FromString("http")}, + }, + InitialDelaySeconds: 99, + PeriodSeconds: 99, + } + + // Simulate a Deployment that already has probes configured (e.g. from a + // prior reconcile, or customized by a user) to ensure the backfill logic + // doesn't clobber them. + existingDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: vmdpServerDeploymentName, Namespace: namespace}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "oadp-vmdp-server", + Image: "old-image", + ReadinessProbe: customProbe.DeepCopy(), + LivenessProbe: customProbe.DeepCopy(), + StartupProbe: customProbe.DeepCopy(), + }, + }, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(existingDeployment, newTestVMDPRoute(namespace)). + Build() + + setup := &VMDPDownloadSetup{ + Client: fakeClient, + Namespace: namespace, + OperatorName: "oadp-operator", + OperatorNamespace: namespace, + Log: logr.Discard(), + } + + if err := setup.reconcileVMDPResources(context.Background(), operatorDeployment, "test-image"); err != nil { + t.Fatalf("reconcileVMDPResources returned error: %v", err) + } + + updated := &appsv1.Deployment{} + if err := fakeClient.Get(context.Background(), client.ObjectKey{Name: vmdpServerDeploymentName, Namespace: namespace}, updated); err != nil { + t.Fatalf("failed to get updated deployment: %v", err) + } + container := updated.Spec.Template.Spec.Containers[0] + + if container.ReadinessProbe.InitialDelaySeconds != 99 { + t.Errorf("expected existing ReadinessProbe to be preserved, got InitialDelaySeconds=%d", container.ReadinessProbe.InitialDelaySeconds) + } + if container.LivenessProbe.InitialDelaySeconds != 99 { + t.Errorf("expected existing LivenessProbe to be preserved, got InitialDelaySeconds=%d", container.LivenessProbe.InitialDelaySeconds) + } + if container.StartupProbe.InitialDelaySeconds != 99 { + t.Errorf("expected existing StartupProbe to be preserved, got InitialDelaySeconds=%d", container.StartupProbe.InitialDelaySeconds) + } +} From da882c3fb37c21cd89de4ecf9e5ee08bd9c3477c Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Mon, 27 Jul 2026 09:42:43 -0400 Subject: [PATCH 2/2] Fix import shadowing lint error in CLI download controller tests --- .../cli_download_controller_test.go | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/controller/cli_download_controller_test.go b/internal/controller/cli_download_controller_test.go index 7e4fc3a943..541a6d8b28 100644 --- a/internal/controller/cli_download_controller_test.go +++ b/internal/controller/cli_download_controller_test.go @@ -78,14 +78,14 @@ func TestBuildCLIServerServiceAccount(t *testing.T) { // ignored: by that point the ServiceAccount step (step 1) has already run // and persisted its result, which is what these tests verify. func newCLITestScheme(t *testing.T) *runtime.Scheme { - scheme := runtime.NewScheme() - if err := corev1.AddToScheme(scheme); err != nil { + testScheme := runtime.NewScheme() + if err := corev1.AddToScheme(testScheme); err != nil { t.Fatal(err) } - if err := appsv1.AddToScheme(scheme); err != nil { + if err := appsv1.AddToScheme(testScheme); err != nil { t.Fatal(err) } - return scheme + return testScheme } func newCLITestOperatorDeployment() *appsv1.Deployment { @@ -102,9 +102,9 @@ func newCLITestOperatorDeployment() *appsv1.Deployment { // ServiceAccount is created with the desired state when it doesn't exist. func TestReconcileCLIResources_CreatesServiceAccountWhenMissing(t *testing.T) { const ns = "openshift-adp" - scheme := newCLITestScheme(t) + testScheme := newCLITestScheme(t) operatorDeploy := newCLITestOperatorDeployment() - fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorDeploy).Build() + fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(operatorDeploy).Build() setup := &CLIDownloadSetup{Client: fakeClient, Namespace: ns, Log: logr.Discard()} // Expect an error once reconcileCLIResources reaches the unregistered @@ -135,7 +135,7 @@ func TestReconcileCLIResources_CreatesServiceAccountWhenMissing(t *testing.T) { func TestReconcileCLIResources_FixesExistingServiceAccountDrift(t *testing.T) { const ns = "openshift-adp" trueVal := true - scheme := newCLITestScheme(t) + testScheme := newCLITestScheme(t) operatorDeploy := newCLITestOperatorDeployment() existing := &corev1.ServiceAccount{ @@ -147,7 +147,7 @@ func TestReconcileCLIResources_FixesExistingServiceAccountDrift(t *testing.T) { AutomountServiceAccountToken: &trueVal, // wrong: should be false } - fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorDeploy, existing).Build() + fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(operatorDeploy, existing).Build() setup := &CLIDownloadSetup{Client: fakeClient, Namespace: ns, Log: logr.Discard()} if err := setup.reconcileCLIResources(context.Background(), operatorDeploy, "test-image"); err != nil { @@ -186,7 +186,7 @@ func TestReconcileCLIResources_FixesExistingServiceAccountDrift(t *testing.T) { func TestReconcileCLIResources_FixesMissingOwnerReferenceOnly(t *testing.T) { const ns = "openshift-adp" falseVal := false - scheme := newCLITestScheme(t) + testScheme := newCLITestScheme(t) operatorDeploy := newCLITestOperatorDeployment() existing := &corev1.ServiceAccount{ @@ -203,7 +203,7 @@ func TestReconcileCLIResources_FixesMissingOwnerReferenceOnly(t *testing.T) { AutomountServiceAccountToken: &falseVal, } - fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorDeploy, existing).Build() + fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(operatorDeploy, existing).Build() setup := &CLIDownloadSetup{Client: fakeClient, Namespace: ns, Log: logr.Discard()} if err := setup.reconcileCLIResources(context.Background(), operatorDeploy, "test-image"); err != nil { @@ -232,7 +232,7 @@ func TestReconcileCLIResources_FixesMissingOwnerReferenceOnly(t *testing.T) { // updated (no ResourceVersion bump). func TestReconcileCLIResources_NoopWhenServiceAccountAlreadyCorrect(t *testing.T) { const ns = "openshift-adp" - scheme := newCLITestScheme(t) + testScheme := newCLITestScheme(t) operatorDeploy := newCLITestOperatorDeployment() desired := buildCLIServerServiceAccount(ns) @@ -240,7 +240,7 @@ func TestReconcileCLIResources_NoopWhenServiceAccountAlreadyCorrect(t *testing.T {UID: operatorDeploy.UID, Name: operatorDeploy.Name, Kind: "Deployment", APIVersion: "apps/v1"}, } - fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorDeploy, desired.DeepCopy()).Build() + fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(operatorDeploy, desired.DeepCopy()).Build() before := &corev1.ServiceAccount{} if err := fakeClient.Get(context.Background(), types.NamespacedName{Name: cliServerServiceAccountName, Namespace: ns}, before); err != nil {