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

Generate-by: Cursor/Claude

Author: camilamacedo86

test/e2e/features/rollout-restart.feature

@@ -0,0 +1,75 @@
+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}
+
+  # KNOWN BUG: Boxcutter currently reverts user-initiated deployment changes
+  #
+  # ROOT CAUSE:
+  # When Boxcutter (pkg.package-operator.run/boxcutter v0.10.0) reconciles a
+  # ClusterExtensionRevision, it applies the manifest stored in the revision spec.
+  # This manifest snapshot was taken when the bundle was unpacked and does NOT
+  # include user-added annotations like the restart timestamp from `kubectl rollout restart`.
+  #
+  # Because Boxcutter applies this full manifest (including the annotations field),
+  # it takes ownership of the entire field via Server-Side Apply, overwriting any
+  # annotations added by other field managers (like kubectl).
+  #
+  # EXPECTED BEHAVIOR (not yet implemented):
+  # The controller should only manage the fields it explicitly cares about, allowing
+  # user-managed fields (like restart annotations) to persist across reconciliation.
+  #
+  # This is the same issue that existed in OLMv0 (see upstream issue below) and needs
+  # to be fixed in the OLMv1 Boxcutter implementation.
+  #
+  # UPSTREAM ISSUE: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
+  #
+  # TODO: Fix the Boxcutter implementation to properly use Server-Side Apply field
+  # ownership so user changes persist, then remove @skip tag
+  @skip
+  @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
+    # In OLMv0, the controller would revert this annotation
+    # In OLMv1 with SSA, the annotation should persist because kubectl owns this field
+    When user adds restart annotation to "deployment/test-operator"
+    # The restart annotation should still be present - OLMv1 should not revert user changes
+    # The controller reconciles periodically, so if it was going to revert the change, it would happen
+    Then resource "deployment/test-operator" has restart annotation

test/e2e/steps/steps.go

@@ -87,6 +87,8 @@ 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)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 +1170,64 @@ 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)
+	resourceName = substituteScenarioVars(resourceName, sc)
+
+	kind, _, ok := strings.Cut(resourceName, "/")
+	if !ok {
+		return fmt.Errorf("invalid resource name format: %s (expected kind/name)", resourceName)
+	}
+
+	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)
+	resourceName = substituteScenarioVars(resourceName, sc)
+
+	kind, deploymentName, ok := strings.Cut(resourceName, "/")
+	if !ok {
+		return fmt.Errorf("invalid resource name format: %s (expected kind/name)", resourceName)
+	}
+
+	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
+}