fix(pull-mode): do not commit helm stage when a chart failed this cycle

handleCharts in controllers/handlers_helm.go was committing the staged
ConfigurationBundles even when walkChartsAndDeploy returned an error
for one of the charts in the profile. Because staging had aborted
before the failing chart was added to the in-memory staging manager,
the commit produced a ConfigurationGroup missing the bundles for that
chart. The applier-manager on the managed cluster then treats the
missing bundles as "this release was removed from the profile" and
uninstalls the chart, removing the previously deployed resources from
the managed cluster.

The user-visible effect is that any pull-mode reconcile which fails
inside helm (helm values schema validation, template instantiation
error, unreachable chart repo, invalid semantic version from a
poisoned cache, etc.) does not just fail loudly: it silently tears
down the deployment that was working moments earlier. Issue
#1724 reported this for cert-manager
when the values ConfigMap referenced by valuesFrom was updated with a
values document that did not match the chart schema.

The two sibling handlers already handle this correctly:

  * handlers_resources.go:151-155 returns on deployError before
    CommitStagedResourcesForDeployment.
  * handlers_kustomize.go:196-197 does the same.

This change aligns handlers_helm.go with that pattern: if
walkChartsAndDeploy returned a deployError, return it immediately
instead of committing a partial stage. The previously committed
ConfigurationGroup is left in place so the agent continues to run the
last known good state until a clean reconcile can complete.

updateClusterReportWithHelmReports is a no-op outside of DryRun sync
mode, so moving the deployError check ahead of it does not affect
non-DryRun reconciles. Staged-but-not-committed bundles are cleared
by DiscardStagedResourcesForDeployment at the top of the next
deployHelmCharts invocation (handlers_helm.go:159), so there is no
leak of in-memory staged state.

Refs: #1724

Author: thecodeassassin

controllers/handlers_helm.go

@@ -928,6 +928,21 @@ func handleCharts(ctx context.Context, clusterSummary *configv1beta1.ClusterSumm
 	}
 
 	if isPullMode {
+		// If any chart in this reconcile failed to stage (helm values schema
+		// error, template instantiation failure, unreachable repo, etc.),
+		// do not commit. Committing a partial staged set produces a
+		// ConfigurationGroup with missing bundles for the failed charts,
+		// which the agent in the managed cluster interprets as "these
+		// releases were removed from the profile" and uninstalls them.
+		// Returning early leaves the previously-committed ConfigurationGroup
+		// in place so the agent continues to run the last known good state
+		// until the reconcile can complete cleanly. This mirrors the
+		// deployError-before-commit pattern already used in
+		// handlers_resources.go and handlers_kustomize.go.
+		if deployError != nil {
+			return deployError
+		}
+
 		err = commitStagedResourcesForDeployment(ctx, clusterSummary, configurationHash, mgmtResources, logger)
 		if err != nil {
 			return err
@@ -937,10 +952,6 @@ func handleCharts(ctx context.Context, clusterSummary *configv1beta1.ClusterSumm
 		if err != nil {
 			return err
 		}
-
-		if deployError != nil {
-			return deployError
-		}
 	} else {
 		// First get the helm releases currently managed and uninstall all the ones
 		// not referenced anymore. Only if this operation succeeds, removes all stale