sets a sensible default of **2 replicas** and update upgrade e2e test cases

jianzhangbjz · jianzhangbjz · commit 54032275fbec · 2025-12-04T17:13:28.000+08:00
diff --git a/Makefile b/Makefile
@@ -484,8 +484,8 @@ run-experimental: run-internal #HELP Build the operator-controller then deploy i
 CATD_NAMESPACE := olmv1-system
 .PHONY: wait
 wait:
-	kubectl wait --for=condition=Available --namespace=$(CATD_NAMESPACE) deployment/catalogd-controller-manager --timeout=60s
-	kubectl wait --for=condition=Ready --namespace=$(CATD_NAMESPACE) certificate/catalogd-service-cert # Avoid upgrade test flakes when reissuing cert
+	kubectl wait --for=condition=Available --namespace=$(CATD_NAMESPACE) deployment/catalogd-controller-manager --timeout=3m
+	kubectl wait --for=condition=Ready --namespace=$(CATD_NAMESPACE) certificate/catalogd-service-cert --timeout=3m # Avoid upgrade test flakes when reissuing cert
 
 .PHONY: docker-build
 docker-build: build-linux #EXHELP Build docker image for operator-controller and catalog with GOOS=linux and local GOARCH.
diff --git a/hack/test/pre-upgrade-setup.sh b/hack/test/pre-upgrade-setup.sh
@@ -155,5 +155,5 @@ spec:
       version: 1.0.0
 EOF
 
-kubectl wait --for=condition=Serving --timeout=60s ClusterCatalog $TEST_CLUSTER_CATALOG_NAME
-kubectl wait --for=condition=Installed --timeout=60s ClusterExtension $TEST_CLUSTER_EXTENSION_NAME
+kubectl wait --for=condition=Serving --timeout=3m ClusterCatalog $TEST_CLUSTER_CATALOG_NAME
+kubectl wait --for=condition=Installed --timeout=3m ClusterExtension $TEST_CLUSTER_EXTENSION_NAME
diff --git a/helm/olmv1/templates/deployment-olmv1-system-catalogd-controller-manager.yml b/helm/olmv1/templates/deployment-olmv1-system-catalogd-controller-manager.yml
@@ -12,11 +12,11 @@ metadata:
   namespace: {{ .Values.namespaces.olmv1.name }}
 spec:
   minReadySeconds: 5
-  replicas: 1
+  replicas: {{ .Values.options.catalogd.deployment.replicas }}
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
diff --git a/helm/olmv1/templates/deployment-olmv1-system-operator-controller-controller-manager.yml b/helm/olmv1/templates/deployment-olmv1-system-operator-controller-controller-manager.yml
@@ -11,11 +11,11 @@ metadata:
   name: operator-controller-controller-manager
   namespace: {{ .Values.namespaces.olmv1.name }}
 spec:
-  replicas: 1
+  replicas: {{ .Values.options.operatorController.deployment.replicas }}
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
diff --git a/helm/olmv1/values.yaml b/helm/olmv1/values.yaml
@@ -8,6 +8,7 @@ options:
     enabled: true
     deployment:
       image: quay.io/operator-framework/operator-controller:devel
+      replicas: 2
       extraArguments: []
     features:
       enabled: []
@@ -19,6 +20,7 @@ options:
     enabled: true
     deployment:
       image: quay.io/operator-framework/catalogd:devel
+      replicas: 2
       extraArguments: []
     features:
       enabled: []
diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml
@@ -2107,11 +2107,11 @@ metadata:
   namespace: olmv1-system
 spec:
   minReadySeconds: 5
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
@@ -2258,11 +2258,11 @@ metadata:
   name: operator-controller-controller-manager
   namespace: olmv1-system
 spec:
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml
@@ -2032,11 +2032,11 @@ metadata:
   namespace: olmv1-system
 spec:
   minReadySeconds: 5
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
@@ -2170,11 +2170,11 @@ metadata:
   name: operator-controller-controller-manager
   namespace: olmv1-system
 spec:
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml
@@ -1795,11 +1795,11 @@ metadata:
   namespace: olmv1-system
 spec:
   minReadySeconds: 5
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
@@ -1945,11 +1945,11 @@ metadata:
   name: operator-controller-controller-manager
   namespace: olmv1-system
 spec:
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
diff --git a/manifests/standard.yaml b/manifests/standard.yaml
@@ -1720,11 +1720,11 @@ metadata:
   namespace: olmv1-system
 spec:
   minReadySeconds: 5
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
@@ -1857,11 +1857,11 @@ metadata:
   name: operator-controller-controller-manager
   namespace: olmv1-system
 spec:
-  replicas: 1
+  replicas: 2
   strategy:
     type: RollingUpdate
     rollingUpdate:
-      maxSurge: 1          # Allow temporary 2 pods (1 + 1) for zero-downtime updates
+      maxSurge: 1          # Allow temporary extra pod (e.g., 3 pods with 2 replicas) for zero-downtime updates
       maxUnavailable: 0    # Never allow pods to be unavailable during updates
   selector:
     matchLabels:
diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go
@@ -28,7 +28,10 @@ import (
 
 const (
 	artifactName         = "operator-controller-e2e"
-	pollDuration         = time.Minute
+	// pollDuration is set to 3 minutes to account for leader election time in multi-replica deployments.
+	// In the worst case (previous leader crashed), leader election can take up to 163 seconds
+	// (LeaseDuration: 137s + RetryPeriod: 26s). Adding buffer for reconciliation time.
+	pollDuration         = 3 * time.Minute
 	pollInterval         = time.Second
 	testCatalogRefEnvVar = "CATALOG_IMG"
 	testCatalogName      = "test-catalog"
diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go
@@ -34,7 +34,10 @@ var (
 )
 
 const (
-	pollDuration         = time.Minute
+	// pollDuration is set to 3 minutes to account for leader election time in multi-replica deployments.
+	// In the worst case (previous leader crashed), leader election can take up to 163 seconds
+	// (LeaseDuration: 137s + RetryPeriod: 26s). Adding buffer for reconciliation time.
+	pollDuration         = 3 * time.Minute
 	pollInterval         = time.Second
 	testCatalogName      = "test-catalog"
 	testCatalogRefEnvVar = "CATALOG_IMG"
diff --git a/test/upgrade-e2e/post_upgrade_test.go b/test/upgrade-e2e/post_upgrade_test.go
@@ -45,23 +45,22 @@ func TestClusterCatalogUnpacking(t *testing.T) {
 		require.Equal(ct, *managerDeployment.Spec.Replicas, managerDeployment.Status.ReadyReplicas)
 	}, time.Minute, time.Second)
 
-	var managerPod corev1.Pod
-	t.Log("Waiting for only one controller-manager pod to remain")
+	t.Log("Waiting for controller-manager pods to match the desired replica count")
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
 		var managerPods corev1.PodList
 		err := c.List(ctx, &managerPods, client.MatchingLabels(managerLabelSelector))
 		require.NoError(ct, err)
-		require.Len(ct, managerPods.Items, 1)
-		managerPod = managerPods.Items[0]
+		require.Len(ct, managerPods.Items, int(*managerDeployment.Spec.Replicas))
 	}, time.Minute, time.Second)
 
 	t.Log("Waiting for acquired leader election")
 	leaderCtx, leaderCancel := context.WithTimeout(ctx, 3*time.Minute)
 	defer leaderCancel()
-	leaderSubstrings := []string{"successfully acquired lease"}
-	leaderElected, err := watchPodLogsForSubstring(leaderCtx, &managerPod, leaderSubstrings...)
+
+	// When there are multiple replicas, find the leader pod
+	managerPod, err := findLeaderPod(leaderCtx, "catalogd")
 	require.NoError(t, err)
-	require.True(t, leaderElected)
+	require.NotNil(t, managerPod)
 
 	t.Log("Reading logs to make sure that ClusterCatalog was reconciled by catalogdv1")
 	logCtx, cancel := context.WithTimeout(ctx, time.Minute)
@@ -70,7 +69,7 @@ func TestClusterCatalogUnpacking(t *testing.T) {
 		"reconcile ending",
 		fmt.Sprintf(`ClusterCatalog=%q`, testClusterCatalogName),
 	}
-	found, err := watchPodLogsForSubstring(logCtx, &managerPod, substrings...)
+	found, err := watchPodLogsForSubstring(logCtx, managerPod, substrings...)
 	require.NoError(t, err)
 	require.True(t, found)
 
@@ -103,22 +102,30 @@ func TestClusterExtensionAfterOLMUpgrade(t *testing.T) {
 
 	// wait for catalogd deployment to finish
 	t.Log("Wait for catalogd deployment to be ready")
-	catalogdManagerPod := waitForDeployment(t, ctx, "catalogd")
+	waitForDeployment(t, ctx, "catalogd")
+
+	// Find the catalogd leader pod
+	catalogdLeaderCtx, catalogdLeaderCancel := context.WithTimeout(ctx, 3*time.Minute)
+	defer catalogdLeaderCancel()
+	catalogdManagerPod, err := findLeaderPod(catalogdLeaderCtx, "catalogd")
+	require.NoError(t, err)
+	require.NotNil(t, catalogdManagerPod)
 
 	// wait for operator-controller deployment to finish
 	t.Log("Wait for operator-controller deployment to be ready")
-	managerPod := waitForDeployment(t, ctx, "operator-controller")
+	waitForDeployment(t, ctx, "operator-controller")
 
 	t.Log("Wait for acquired leader election")
 	// Average case is under 1 minute but in the worst case: (previous leader crashed)
 	// we could have LeaseDuration (137s) + RetryPeriod (26s) +/- 163s
 	leaderCtx, leaderCancel := context.WithTimeout(ctx, 3*time.Minute)
 	defer leaderCancel()
 
-	leaderSubstrings := []string{"successfully acquired lease"}
-	leaderElected, err := watchPodLogsForSubstring(leaderCtx, managerPod, leaderSubstrings...)
+	// When there are multiple replicas, find the leader pod
+	var managerPod *corev1.Pod
+	managerPod, err = findLeaderPod(leaderCtx, "operator-controller")
 	require.NoError(t, err)
-	require.True(t, leaderElected)
+	require.NotNil(t, managerPod)
 
 	t.Log("Reading logs to make sure that ClusterExtension was reconciled by operator-controller before we update it")
 	// Make sure that after we upgrade OLM itself we can still reconcile old objects without any changes
@@ -195,9 +202,8 @@ func TestClusterExtensionAfterOLMUpgrade(t *testing.T) {
 
 // waitForDeployment checks that the updated deployment with the given app.kubernetes.io/name label
 // has reached the desired number of replicas and that the number pods matches that number
-// i.e. no old pods remain. It will return a pointer to the first pod. This is only necessary
-// to facilitate the mitigation put in place for https://github.com/operator-framework/operator-controller/issues/1626
-func waitForDeployment(t *testing.T, ctx context.Context, controlPlaneLabel string) *corev1.Pod {
+// i.e. no old pods remain.
+func waitForDeployment(t *testing.T, ctx context.Context, controlPlaneLabel string) {
 	deploymentLabelSelector := labels.Set{"app.kubernetes.io/name": controlPlaneLabel}.AsSelector()
 
 	t.Log("Checking that the deployment is updated")
@@ -217,13 +223,101 @@ func waitForDeployment(t *testing.T, ctx context.Context, controlPlaneLabel stri
 		desiredNumReplicas = *managerDeployment.Spec.Replicas
 	}, time.Minute, time.Second)
 
-	var managerPods corev1.PodList
 	t.Logf("Ensure the number of remaining pods equal the desired number of replicas (%d)", desiredNumReplicas)
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
+		var managerPods corev1.PodList
 		require.NoError(ct, c.List(ctx, &managerPods, client.MatchingLabelsSelector{Selector: deploymentLabelSelector}))
-		require.Len(ct, managerPods.Items, 1)
+		require.Len(ct, managerPods.Items, int(desiredNumReplicas))
 	}, time.Minute, time.Second)
-	return &managerPods.Items[0]
+}
+
+// findLeaderPod finds the pod that has acquired the leader lease by checking logs of all pods
+func findLeaderPod(ctx context.Context, controlPlaneLabel string) (*corev1.Pod, error) {
+	deploymentLabelSelector := labels.Set{"app.kubernetes.io/name": controlPlaneLabel}.AsSelector()
+
+	// If there's only one pod, it must be the leader
+	// First, do a quick check to see if we can find the leader in existing logs
+	var leaderPod *corev1.Pod
+	leaderSubstrings := []string{"successfully acquired lease"}
+
+	// Retry logic: keep checking until we find a leader or context times out
+	ticker := time.NewTicker(2 * time.Second)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-ctx.Done():
+			return nil, fmt.Errorf("timeout waiting for leader election: %w", ctx.Err())
+		case <-ticker.C:
+			var managerPods corev1.PodList
+			if err := c.List(ctx, &managerPods, client.MatchingLabelsSelector{Selector: deploymentLabelSelector}); err != nil {
+				continue
+			}
+
+			if len(managerPods.Items) == 0 {
+				continue
+			}
+
+			// If there's only one pod, it must be the leader
+			if len(managerPods.Items) == 1 {
+				return &managerPods.Items[0], nil
+			}
+
+			// Check each pod's existing logs (without following) for leader election message
+			for i := range managerPods.Items {
+				pod := &managerPods.Items[i]
+
+				// Check existing logs only (don't follow)
+				isLeader, err := checkPodLogsForSubstring(ctx, pod, leaderSubstrings...)
+				if err != nil {
+					// If we can't read logs from this pod, try the next one
+					continue
+				}
+				if isLeader {
+					leaderPod = pod
+					break
+				}
+			}
+
+			if leaderPod != nil {
+				return leaderPod, nil
+			}
+			// No leader found yet, retry after ticker interval
+		}
+	}
+}
+
+// checkPodLogsForSubstring checks existing pod logs (without following) for the given substrings
+func checkPodLogsForSubstring(ctx context.Context, pod *corev1.Pod, substrings ...string) (bool, error) {
+	podLogOpts := corev1.PodLogOptions{
+		Follow:    false, // Don't follow, just read existing logs
+		Container: container,
+	}
+
+	req := kclientset.CoreV1().Pods(pod.Namespace).GetLogs(pod.Name, &podLogOpts)
+	podLogs, err := req.Stream(ctx)
+	if err != nil {
+		return false, err
+	}
+	defer podLogs.Close()
+
+	scanner := bufio.NewScanner(podLogs)
+	for scanner.Scan() {
+		line := scanner.Text()
+
+		foundCount := 0
+		for _, substring := range substrings {
+			if strings.Contains(line, substring) {
+				foundCount++
+			}
+		}
+		if foundCount == len(substrings) {
+			return true, nil
+		}
+	}
+
+	// If we didn't find the substrings, return false (not an error)
+	return false, nil
 }
 
 func watchPodLogsForSubstring(ctx context.Context, pod *corev1.Pod, substrings ...string) (bool, error) {
