Add test verifies that OLMv1 does not revert user-initiated changes to deployed resources

Author: camilamacedo86

test/e2e/features/rollout-restart.feature

@@ -0,0 +1,52 @@
+Feature: Rollout Restart User Changes
+
+  # This test verifies that OLMv1 does not revert user-initiated changes to deployed resources,
+  # specifically testing the scenario where a user runs `kubectl rollout restart deployment`.
+  #
+  # Background:
+  # - In OLMv0, running `kubectl rollout restart deployment` would add a restart annotation
+  #   to the deployment, but OLM would revert this change because it actively reconciles
+  #   the deployment based on CSV contents.
+  # - In OLMv1, we use Server-Side Apply which should only manage the fields that the controller
+  #   explicitly owns, allowing user-initiated changes to other fields to persist.
+  #
+  # This test ensures that OLMv1 handles this correctly and does not exhibit the OLMv0 behavior.
+  # See: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
+
+  Background:
+    Given OLM is available
+    And ClusterCatalog "test" serves bundles
+    And ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+
+  @BoxcutterRuntime
+  Scenario: User-initiated deployment changes persist after OLM reconciliation
+    When ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    Then ClusterExtension is rolled out
+    And ClusterExtension is available
+    And resource "deployment/test-operator" is installed
+    # Simulate user running "kubectl rollout restart deployment/test-operator"
+    # This adds a restart annotation to trigger a rolling restart
+    When user adds restart annotation to "deployment/test-operator"
+    # Trigger OLM reconciliation by updating a ClusterExtension annotation
+    # This simulates the controller re-reconciling the deployment
+    And ClusterExtension annotation "test-annotation" is set to "trigger-reconcile"
+    And ClusterExtension has been reconciled the latest generation
+    # The restart annotation should still be present - OLMv1 should not revert user changes
+    Then resource "deployment/test-operator" has restart annotation

test/e2e/steps/steps.go

@@ -87,6 +87,9 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 	sc.Step(`^(?i)resource apply fails with error msg containing "([^"]+)"$`, ResourceApplyFails)
 	sc.Step(`^(?i)resource "([^"]+)" is eventually restored$`, ResourceRestored)
 	sc.Step(`^(?i)resource "([^"]+)" matches$`, ResourceMatches)
+	sc.Step(`^(?i)user adds restart annotation to "([^"]+)"$`, UserAddsRestartAnnotation)
+	sc.Step(`^(?i)resource "([^"]+)" has restart annotation$`, ResourceHasRestartAnnotation)
+	sc.Step(`^(?i)ClusterExtension annotation "([^"]+)" is set to "([^"]+)"$`, ClusterExtensionAnnotationIsSet)
 
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in test namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInNamespace)
 	sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithNeededPermissionsIsAvailableInNamespace)
@@ -1168,3 +1171,95 @@ func latestActiveRevisionForExtension(extName string) (*ocv1.ClusterExtensionRev
 
 	return latest, nil
 }
+
+// UserAddsRestartAnnotation simulates a user running `kubectl rollout restart deployment/<name>`.
+// This adds a restart annotation to the deployment's pod template to trigger a rolling restart.
+// In OLMv0, this annotation would be reverted by the controller. In OLMv1 with Server-Side Apply,
+// it should persist because the user (kubectl) manages this field, not the controller.
+// See: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
+func UserAddsRestartAnnotation(ctx context.Context, resourceName string) error {
+	sc := scenarioCtx(ctx)
+
+	// Parse the resource name (format: "deployment/test-operator")
+	parts := strings.Split(resourceName, "/")
+	if len(parts) != 2 {
+		return fmt.Errorf("invalid resource name format: %s (expected kind/name)", resourceName)
+	}
+	kind := parts[0]
+
+	if kind != "deployment" {
+		return fmt.Errorf("only deployment resources are supported for restart annotation, got: %s", kind)
+	}
+
+	// Use kubectl rollout restart to add the restart annotation
+	// This is the actual command users would run, ensuring we test real-world behavior
+	_, err := k8sClient("rollout", "restart", resourceName, "-n", sc.namespace)
+	if err != nil {
+		return fmt.Errorf("failed to rollout restart %s: %w", resourceName, err)
+	}
+
+	return nil
+}
+
+// ResourceHasRestartAnnotation verifies that a deployment has a restart annotation.
+// This confirms that user-initiated changes persist after OLM reconciliation.
+func ResourceHasRestartAnnotation(ctx context.Context, resourceName string) error {
+	sc := scenarioCtx(ctx)
+
+	// Parse the resource name (format: "deployment/test-operator")
+	parts := strings.Split(resourceName, "/")
+	if len(parts) != 2 {
+		return fmt.Errorf("invalid resource name format: %s (expected kind/name)", resourceName)
+	}
+	kind := parts[0]
+	deploymentName := parts[1]
+
+	if kind != "deployment" {
+		return fmt.Errorf("only deployment resources are supported for restart annotation check, got: %s", kind)
+	}
+
+	// Check for the restart annotation added by kubectl rollout restart
+	restartAnnotationKey := "kubectl.kubernetes.io/restartedAt"
+
+	waitFor(ctx, func() bool {
+		// Get the restart annotation from the deployment's pod template
+		out, err := k8sClient("get", "deployment", deploymentName, "-n", sc.namespace,
+			"-o", fmt.Sprintf("jsonpath={.spec.template.metadata.annotations['%s']}", restartAnnotationKey))
+		if err != nil {
+			return false
+		}
+
+		// If the annotation exists and has a value, it persisted
+		return out != ""
+	})
+
+	return nil
+}
+
+// ClusterExtensionAnnotationIsSet sets an annotation on the ClusterExtension to trigger reconciliation.
+// This simulates changes that would cause the controller to re-apply managed resources.
+func ClusterExtensionAnnotationIsSet(ctx context.Context, annotationKey, annotationValue string) error {
+	sc := scenarioCtx(ctx)
+
+	// Build the patch to add the annotation
+	patch := map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"annotations": map[string]interface{}{
+				annotationKey: annotationValue,
+			},
+		},
+	}
+
+	patchBytes, err := json.Marshal(patch)
+	if err != nil {
+		return fmt.Errorf("failed to marshal annotation patch: %w", err)
+	}
+
+	// Apply the patch to add the annotation
+	_, err = k8sClient("patch", "clusterextension", sc.clusterExtensionName, "--type", "merge", "-p", string(patchBytes))
+	if err != nil {
+		return fmt.Errorf("failed to patch ClusterExtension with annotation: %w", err)
+	}
+
+	return nil
+}