fix(bundler): scope ArgoCD ApplyOutOfSyncOnly to readiness-gate Application

Force=true alone deletes and recreates the readiness-gate Job on every
ArgoCD sync, not just genuine spec diffs, needlessly rerunning readiness
checks (CodeRabbit finding on PR #2408).

ApplyOutOfSyncOnly only works as an Application-level
spec.syncPolicy.syncOptions setting, not a per-resource annotation
(silently ignored there). Add ApplicationData.ApplyOutOfSyncOnly, scoped
to -readiness folders via a shared isReadinessFolder helper, rendered
conditionally in application.yaml.tmpl.

Verified on kind + real ArgoCD v3.5.1: no-op syncs now leave the Job's
UID unchanged; a genuine diff still correctly replaces it.

Signed-off-by: Kevin Hawkins <khawkins@nvidia.com>

Author: Kevin-Hawkins

pkg/bundler/deployer/argocd/argocd.go

@@ -108,6 +108,15 @@ type ApplicationData struct {
 	// CascadeDelete adds ResourcesFinalizer to the rendered Application.
 	// See #1628.
 	CascadeDelete bool
+
+	// ApplyOutOfSyncOnly adds ApplyOutOfSyncOnly=true to spec.syncPolicy.syncOptions.
+	// Set only for the -readiness folder's Application: paired with the Job-level
+	// Replace=true,Force=true annotation (see gatemanifest.jobMetadataAnnotations),
+	// this prevents ArgoCD from deleting/recreating the readiness-gate Job on every
+	// sync when nothing actually changed — Replace+Force alone forces a
+	// delete-and-recreate unconditionally. See #2367 and the CodeRabbit finding on
+	// PR #2408.
+	ApplyOutOfSyncOnly bool
 }
 
 // AppOfAppsData contains data for rendering the App of Apps manifest.
@@ -790,13 +799,22 @@ func waveForFolder(f localformat.Folder, level int) int {
 		return base
 	case f.Parent + "-post":
 		return base + 2
-	case f.Parent + "-readiness":
-		return base + 3
-	default: // primary: Name == Parent
-		return base + 1
+	default:
+		if isReadinessFolder(f) {
+			return base + 3
+		}
+		return base + 1 // primary: Name == Parent
 	}
 }
 
+// isReadinessFolder reports whether f is the injected -readiness folder for
+// its parent component. Shared by waveForFolder (sync-wave banding) and
+// buildApplicationData (ApplyOutOfSyncOnly scoping) so the two can't
+// silently diverge on what counts as a readiness folder. See #2367.
+func isReadinessFolder(f localformat.Folder) bool {
+	return f.Name == f.Parent+"-readiness"
+}
+
 // buildApplicationData constructs ApplicationData for a single folder. The
 // FolderKind drives the Application shape — KindLocalHelm sets IsLocalChart
 // (path-based single-source); KindUpstreamHelm leaves it empty (multi-source
@@ -811,12 +829,13 @@ func waveForFolder(f localformat.Folder, level int) int {
 func buildApplicationData(comp recipe.ComponentRef, f localformat.Folder, syncWave int, repoURL, targetRevision string, values map[string]any, inline bool) (ApplicationData, error) {
 	chart := comp.EffectiveChart()
 	data := ApplicationData{
-		Name:           f.Name,
-		Namespace:      comp.Namespace,
-		SyncWave:       syncWave,
-		RepoURL:        repoURL,
-		TargetRevision: targetRevision,
-		BundleDir:      f.Dir,
+		Name:               f.Name,
+		Namespace:          comp.Namespace,
+		SyncWave:           syncWave,
+		RepoURL:            repoURL,
+		TargetRevision:     targetRevision,
+		BundleDir:          f.Dir,
+		ApplyOutOfSyncOnly: isReadinessFolder(f),
 	}
 	switch f.Kind {
 	case localformat.KindLocalHelm:

pkg/bundler/deployer/argocd/argocd_test.go

@@ -2343,3 +2343,128 @@ func TestGenerate_ChildNameLimits(t *testing.T) {
 		})
 	}
 }
+
+// TestBuildApplicationData_ApplyOutOfSyncOnly is the regression-proofing
+// test for the SCOPING of ApplicationData.ApplyOutOfSyncOnly, not just its
+// presence: it exercises all four folder kinds for the same parent
+// component so a test that only checked the readiness case could not pass
+// if the predicate degenerated to unconditionally true. Paired with the
+// Job-level Replace=true,Force=true annotation in
+// pkg/bundler/gatemanifest/manifest.go, ApplyOutOfSyncOnly (set only for
+// the readiness Application) stops ArgoCD from delete-and-recreating the
+// readiness-gate Job on every no-op resync. See #2367.
+func TestBuildApplicationData_ApplyOutOfSyncOnly(t *testing.T) {
+	comp := recipe.ComponentRef{
+		Name:    "gpu-operator",
+		Source:  "https://helm.ngc.nvidia.com/nvidia",
+		Chart:   "gpu-operator",
+		Version: "v25.3.3",
+	}
+
+	tests := []struct {
+		name   string
+		folder localformat.Folder
+		want   bool
+	}{
+		{
+			name:   "primary folder",
+			folder: localformat.Folder{Name: "gpu-operator", Dir: "001-gpu-operator", Kind: localformat.KindUpstreamHelm, Parent: "gpu-operator"},
+			want:   false,
+		},
+		{
+			name:   "-pre folder",
+			folder: localformat.Folder{Name: "gpu-operator-pre", Dir: "001-gpu-operator-pre", Kind: localformat.KindLocalHelm, Parent: "gpu-operator"},
+			want:   false,
+		},
+		{
+			name:   "-post folder",
+			folder: localformat.Folder{Name: "gpu-operator-post", Dir: "003-gpu-operator-post", Kind: localformat.KindLocalHelm, Parent: "gpu-operator"},
+			want:   false,
+		},
+		{
+			name:   "-readiness folder",
+			folder: localformat.Folder{Name: "gpu-operator-readiness", Dir: "004-gpu-operator-readiness", Kind: localformat.KindLocalHelm, Parent: "gpu-operator"},
+			want:   true,
+		},
+		{
+			// Adversarial: a primary folder whose NAME merely contains
+			// "-readiness" (not the synthetic suffix pattern relative to
+			// its own Parent) must not be scoped in. f.Parent+"-readiness"
+			// = "foo-readiness-readiness" != "foo-readiness", so the
+			// predicate correctly evaluates false.
+			name:   "primary folder whose name happens to contain -readiness",
+			folder: localformat.Folder{Name: "foo-readiness", Dir: "005-foo-readiness", Kind: localformat.KindUpstreamHelm, Parent: "foo-readiness"},
+			want:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			data, err := buildApplicationData(comp, tt.folder, 0, "https://github.com/example/repo.git", "main", nil, false)
+			if err != nil {
+				t.Fatalf("buildApplicationData() error = %v", err)
+			}
+			if data.ApplyOutOfSyncOnly != tt.want {
+				t.Errorf("ApplyOutOfSyncOnly = %v, want %v (folder=%+v)", data.ApplyOutOfSyncOnly, tt.want, tt.folder)
+			}
+		})
+	}
+}
+
+// TestGenerate_ApplyOutOfSyncOnlySyncOptions asserts the rendered
+// application.yaml for a -readiness folder carries the exact
+// "- ApplyOutOfSyncOnly=true" syncOptions entry, and that a non-readiness
+// folder's rendered application.yaml does not mention ApplyOutOfSyncOnly
+// anywhere. See #2367.
+func TestGenerate_ApplyOutOfSyncOnlySyncOptions(t *testing.T) {
+	ctx := context.Background()
+	outputDir := t.TempDir()
+
+	recipeResult := &recipe.RecipeResult{}
+	recipeResult.Metadata.Version = testVersion
+	recipeResult.ComponentRefs = []recipe.ComponentRef{
+		{
+			Name:      "gpu-operator",
+			Namespace: "gpu-operator",
+			Chart:     "gpu-operator",
+			Version:   "v25.3.3",
+			Type:      recipe.ComponentTypeHelm,
+			Source:    "https://helm.ngc.nvidia.com/nvidia",
+		},
+	}
+	recipeResult.DeploymentOrder = []string{"gpu-operator"}
+
+	g := &Generator{
+		RecipeResult:    recipeResult,
+		ComponentValues: map[string]map[string]any{"gpu-operator": {}},
+		Version:         "v0.0.0-test",
+		RepoURL:         "https://github.com/example/aicr-bundles.git",
+		TargetRevision:  "main",
+		ComponentReadiness: map[string]map[string][]byte{
+			"gpu-operator": {
+				"readiness.yaml": []byte("apiVersion: batch/v1\nkind: Job\nmetadata:\n" +
+					"  name: gpu-operator-readiness-gate\n  namespace: {{ .Release.Namespace }}\n"),
+			},
+		},
+	}
+
+	if _, err := g.Generate(ctx, outputDir); err != nil {
+		t.Fatalf("Generate() error = %v", err)
+	}
+
+	primary, err := os.ReadFile(filepath.Join(outputDir, "001-gpu-operator", "application.yaml"))
+	if err != nil {
+		t.Fatalf("read primary application.yaml: %v", err)
+	}
+	if strings.Contains(string(primary), "ApplyOutOfSyncOnly") {
+		t.Errorf("primary Application must not mention ApplyOutOfSyncOnly:\n%s", primary)
+	}
+
+	readiness, err := os.ReadFile(filepath.Join(outputDir, "002-gpu-operator-readiness", "application.yaml"))
+	if err != nil {
+		t.Fatalf("read readiness application.yaml: %v", err)
+	}
+	if !strings.Contains(string(readiness), "- ApplyOutOfSyncOnly=true") {
+		t.Errorf("readiness Application must contain \"- ApplyOutOfSyncOnly=true\":\n%s", readiness)
+	}
+}

pkg/bundler/deployer/argocd/templates/application.yaml.tmpl

@@ -56,3 +56,12 @@ spec:
       # per-Application template branching and a confusing mix of sync
       # strategies in a single bundle.
       - ServerSideApply=true
+{{- if .ApplyOutOfSyncOnly }}
+      # ApplyOutOfSyncOnly: scoped to the readiness-gate Application only.
+      # Paired with the Job's Replace=true,Force=true annotation
+      # (pkg/bundler/gatemanifest/manifest.go), this skips resources ArgoCD's
+      # diff already considers in-sync so a no-op resync does not
+      # delete-and-recreate the readiness-gate Job (and rerun its checks)
+      # when nothing changed. See #2367.
+      - ApplyOutOfSyncOnly=true
+{{- end }}

pkg/bundler/deployer/argocd/testdata/readiness_gate/002-gpu-operator-readiness/application.yaml

@@ -31,3 +31,10 @@ spec:
       # per-Application template branching and a confusing mix of sync
       # strategies in a single bundle.
       - ServerSideApply=true
+      # ApplyOutOfSyncOnly: scoped to the readiness-gate Application only.
+      # Paired with the Job's Replace=true,Force=true annotation
+      # (pkg/bundler/gatemanifest/manifest.go), this skips resources ArgoCD's
+      # diff already considers in-sync so a no-op resync does not
+      # delete-and-recreate the readiness-gate Job (and rerun its checks)
+      # when nothing changed. See #2367.
+      - ApplyOutOfSyncOnly=true

pkg/bundler/deployer/argocdhelm/argocdhelm_test.go

@@ -1949,6 +1949,63 @@ func TestBundleGolden_ReadinessGate(t *testing.T) {
 	}
 }
 
+// TestGenerate_ApplyOutOfSyncOnlySyncOptions is the empirical proof that
+// transformApplication's generic map[string]any YAML round-trip preserves
+// the ApplyOutOfSyncOnly=true syncOptions entry the delegated argocd.Generator
+// adds to the readiness folder's Application, and that it stays scoped to
+// only that folder's transformed Helm-chart-template output. See #2367.
+func TestGenerate_ApplyOutOfSyncOnlySyncOptions(t *testing.T) {
+	ctx := context.Background()
+	outputDir := t.TempDir()
+
+	rr := newRecipeResult("v1.0.0", []recipe.ComponentRef{
+		{
+			Name:      "gpu-operator",
+			Namespace: "gpu-operator",
+			Chart:     "gpu-operator",
+			Version:   "v25.3.3",
+			Type:      recipe.ComponentTypeHelm,
+			Source:    "https://helm.ngc.nvidia.com/nvidia",
+		},
+	})
+	rr.DeploymentOrder = []string{"gpu-operator"}
+
+	g := &Generator{
+		RecipeResult: rr,
+		ComponentValues: map[string]map[string]any{
+			"gpu-operator": {"driver": map[string]any{"version": "580"}},
+		},
+		Version:        "v0.0.0-test",
+		RepoURL:        "https://github.com/example/aicr-bundles.git",
+		TargetRevision: "main",
+		ComponentReadiness: map[string]map[string][]byte{
+			"gpu-operator": {
+				"readiness.yaml": readinessGateManifest(t, config.DeployerArgoCDHelm),
+			},
+		},
+	}
+
+	if _, err := g.Generate(ctx, outputDir); err != nil {
+		t.Fatalf("Generate() error = %v", err)
+	}
+
+	primary, err := os.ReadFile(filepath.Join(outputDir, "templates", "gpu-operator.yaml"))
+	if err != nil {
+		t.Fatalf("read primary template: %v", err)
+	}
+	if strings.Contains(string(primary), "ApplyOutOfSyncOnly") {
+		t.Errorf("primary child template must not mention ApplyOutOfSyncOnly:\n%s", primary)
+	}
+
+	readiness, err := os.ReadFile(filepath.Join(outputDir, "templates", "gpu-operator-readiness.yaml"))
+	if err != nil {
+		t.Fatalf("read readiness template: %v", err)
+	}
+	if !strings.Contains(string(readiness), "ApplyOutOfSyncOnly=true") {
+		t.Errorf("readiness child template must contain \"ApplyOutOfSyncOnly=true\":\n%s", readiness)
+	}
+}
+
 // TestHelmTemplate_RendersWithSetRepoURL is the live-render counterpart to
 // the golden tests: goldens freeze the pre-render template bytes, this
 // test verifies that running `helm template` against the generated bundle

pkg/bundler/deployer/argocdhelm/testdata/readiness_gate/templates/gpu-operator-readiness.yaml

@@ -28,3 +28,4 @@ spec:
         syncOptions:
             - CreateNamespace=true
             - ServerSideApply=true
+            - ApplyOutOfSyncOnly=true

pkg/bundler/gatemanifest/manifest.go

@@ -179,8 +179,19 @@ func jobMetadataAnnotations(deployer config.DeployerType) string {
 		// API server rejects on any upgrade that changes the Job spec (e.g.
 		// an image tag bump), leaving the Application permanently
 		// OutOfSync. Force=true is ArgoCD's documented option to
-		// delete-and-recreate when a replace fails. Deliberately NOT using a
-		// Helm-style sync hook (helm.sh/hook) here: per
+		// delete-and-recreate when a replace fails. This alone would
+		// delete-and-recreate the Job on EVERY sync, including no-op
+		// resyncs where nothing changed — see the ApplyOutOfSyncOnly=true
+		// entry this deployer adds to the readiness folder's
+		// Application-level spec.syncPolicy.syncOptions
+		// (pkg/bundler/deployer/argocd/argocd.go's buildApplicationData),
+		// which excludes already-in-sync resources from a sync operation
+		// and stops the needless rerun. The two mechanisms are
+		// complementary: Job-level Replace+Force handles genuine spec
+		// diffs (e.g. an image tag bump); Application-level
+		// ApplyOutOfSyncOnly prevents unnecessary reruns when there is no
+		// diff at all. Deliberately NOT using a Helm-style sync hook
+		// (helm.sh/hook) here: per
 		// pkg/bundler/deployer/localformat/hooks.go's stripHelmHooks doc,
 		// hook-annotated resources are excluded from ArgoCD's normal drift
 		// detection, so an image-tag-only bump could silently go undetected.

pkg/bundler/gatemanifest/manifest_test.go

@@ -118,6 +118,12 @@ func TestRender_ArgoCDSyncOptions(t *testing.T) {
 	// Application permanently OutOfSync (#2367). Force=true makes ArgoCD
 	// delete-and-recreate on replace failure instead. Both ArgoCD deployer
 	// branches (native and Helm-rendered) must emit the same annotation.
+	// This Job-level annotation is only half the fix: see
+	// TestBuildApplicationData_ApplyOutOfSyncOnly and
+	// TestGenerate_ApplyOutOfSyncOnlySyncOptions in
+	// pkg/bundler/deployer/argocd for the Application-level
+	// ApplyOutOfSyncOnly=true entry that stops Force=true from
+	// delete-and-recreating the Job on every no-op resync.
 	tests := []struct {
 		name     string
 		deployer config.DeployerType