[oadp-1.3] OADP-8484: Added backfill to startup probe

controllers/cli_download_controller.go

@@ -167,11 +167,18 @@ func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDe
 	} else {
 		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 {
+				currentContainer.ReadinessProbe = desiredContainer.ReadinessProbe
+				currentContainer.LivenessProbe = desiredContainer.LivenessProbe
+				needsUpdate = true
+			}
+			if currentContainer.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
@@ -187,7 +194,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")
 		}
 	}
 
@@ -517,3 +524,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
+}

controllers/cli_download_controller_test.go

@@ -197,3 +197,138 @@ func TestReconcileCLIResources_ReconcilesExistingServiceAccount(t *testing.T) {
 		t.Error("expected an owner reference to be added")
 	}
 }
+
+func TestReconcileCLIResources_BackfillsMissingStartupProbe(t *testing.T) {
+	namespace := "openshift-adp"
+	testScheme := getCLIDownloadTestScheme(t)
+
+	operatorDeployment := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{Name: "oadp-operator", Namespace: namespace},
+	}
+
+	// Simulate a Deployment created after readiness/liveness probes existed but
+	// before StartupProbe was added: ReadinessProbe/LivenessProbe are already
+	// set, so the old backfill gate (keyed only on ReadinessProbe == nil) would
+	// never fire and StartupProbe would be stuck missing forever.
+	existingDeployment := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{Name: cliServerDeploymentName, Namespace: namespace},
+		Spec: appsv1.DeploymentSpec{
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					ServiceAccountName: cliServerServiceAccountName,
+					Containers: []corev1.Container{
+						{
+							Name:  "oadp-cli-server",
+							Image: "old-image",
+							ReadinessProbe: &corev1.Probe{
+								ProbeHandler: corev1.ProbeHandler{
+									HTTPGet: &corev1.HTTPGetAction{Path: "/", Port: intstr.FromString("http")},
+								},
+							},
+							LivenessProbe: &corev1.Probe{
+								ProbeHandler: corev1.ProbeHandler{
+									HTTPGet: &corev1.HTTPGetAction{Path: "/", Port: intstr.FromString("http")},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	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.StartupProbe == nil {
+		t.Error("expected StartupProbe to be backfilled even though ReadinessProbe/LivenessProbe already existed")
+	}
+	// 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_DoesNotOverwriteExistingStartupProbe(t *testing.T) {
+	namespace := "openshift-adp"
+	testScheme := getCLIDownloadTestScheme(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,
+	}
+
+	existingDeployment := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{Name: cliServerDeploymentName, Namespace: namespace},
+		Spec: appsv1.DeploymentSpec{
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					ServiceAccountName: cliServerServiceAccountName,
+					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.StartupProbe.InitialDelaySeconds != 99 {
+		t.Errorf("expected existing StartupProbe to be preserved, got InitialDelaySeconds=%d", container.StartupProbe.InitialDelaySeconds)
+	}
+}