[oadp-1.4] OADP-8486: Added 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")
 		}
 	}
 
@@ -389,6 +396,17 @@ func buildCLIServerDeployment(namespace, image string) *appsv1.Deployment {
 									Protocol:      corev1.ProtocolTCP,
 								},
 							},
+							StartupProbe: &corev1.Probe{
+								ProbeHandler: corev1.ProbeHandler{
+									HTTPGet: &corev1.HTTPGetAction{
+										Path: "/",
+										Port: intstr.FromString("http"),
+									},
+								},
+								InitialDelaySeconds: 5,
+								PeriodSeconds:       5,
+								FailureThreshold:    12,
+							},
 							ReadinessProbe: &corev1.Probe{
 								ProbeHandler: corev1.ProbeHandler{
 									HTTPGet: &corev1.HTTPGetAction{
@@ -506,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

@@ -11,6 +11,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"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"
@@ -90,3 +91,176 @@ func TestReconcileCLIResources_ReconcilesExistingServiceAccount(t *testing.T) {
 		t.Error("expected an owner reference to be added")
 	}
 }
+
+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 := getCLIDownloadTestScheme(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{
+					ServiceAccountName: cliServerServiceAccountName,
+					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 := 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,
+	}
+
+	// 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{
+					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.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)
+	}
+}