From d1a2e638ae6138849a159577c5d82d119a4897dc Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Mon, 14 Sep 2026 08:17:39 +0200 Subject: [PATCH] fix: surface every updateChartMap error in ClusterSummary status prepareForDeployment only surfaced a failure to ClusterSummary.Status when updateChartMap returned apierrors.IsNotFound (a missing non-optional templateResourceRef). Every other error like an Helm chart field that fails Sveltos template instantiation only caused the ClusterSummary to be requeued for reconciliation again. The real error only ever showed up in the controller's own logs. This PR fixes that by surfacing any updateChartMap error the same way the IsNotFound case already did, dropping the type check so nothing falls through silently. Fixes #1960 --- controllers/clustersummary_controller.go | 12 ++--- controllers/clustersummary_controller_test.go | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/controllers/clustersummary_controller.go b/controllers/clustersummary_controller.go index 662de454..fa0713bc 100644 --- a/controllers/clustersummary_controller.go +++ b/controllers/clustersummary_controller.go @@ -564,12 +564,12 @@ func (r *ClusterSummaryReconciler) prepareForDeployment(ctx context.Context, err = r.updateChartMap(ctx, clusterSummaryScope, logger) if err != nil { - if apierrors.IsNotFound(err) { - // A required (non-optional) templateResourceRef is missing. Surface it as a - // failure so the operator can see why deployment is blocked. - r.setFailureMessage(clusterSummaryScope, err.Error()) - r.resetFeatureStatus(clusterSummaryScope, libsveltosv1beta1.FeatureStatusFailedNonRetriable) - } + // Whether this is a missing (non-optional) templateResourceRef, a Helm chart field + // that fails Sveltos template instantiation, or anything else updateChartMap can + // return: surface it as a failure so the operator can see why deployment is blocked + // from `kubectl get clustersummary` alone, instead of only in the controller logs. + r.setFailureMessage(clusterSummaryScope, err.Error()) + r.resetFeatureStatus(clusterSummaryScope, libsveltosv1beta1.FeatureStatusFailedNonRetriable) r.setNextReconcileTime(clusterSummaryScope, normalRequeueAfter) return reconcile.Result{RequeueAfter: normalRequeueAfter} } diff --git a/controllers/clustersummary_controller_test.go b/controllers/clustersummary_controller_test.go index afda0e2a..6c1a6ad6 100644 --- a/controllers/clustersummary_controller_test.go +++ b/controllers/clustersummary_controller_test.go @@ -553,6 +553,51 @@ var _ = Describe("ClustersummaryController", func() { Expect(featureResourcesVerified).To(BeTrue()) }) + It("prepareForDeployment surfaces an updateChartMap template failure in ClusterSummary status", func() { + clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeContinuous + clusterSummary.Spec.ClusterProfileSpec.HelmCharts = []configv1beta1.HelmChart{ + { + // ReleaseName is still instantiated as a Sveltos template by + // getInstantiatedChartIdentity (only Values is excluded). An unclosed + // action here fails template parsing with a generic error: neither + // apierrors.IsNotFound nor any other special-cased error type. + RepositoryURL: randomString(), ChartName: randomString(), ChartVersion: randomString(), + ReleaseName: "{{ .Bad", ReleaseNamespace: randomString(), RepositoryName: randomString(), + }, + } + + clusterSummaryScope, err := scope.NewClusterSummaryScope(&scope.ClusterSummaryScopeParams{ + Client: testEnv.Client, + Logger: textlogger.NewLogger(textlogger.NewConfig()), + ClusterSummary: clusterSummary, + ControllerName: testControllerNameSummary, + }) + Expect(err).To(BeNil()) + + reconciler := &controllers.ClusterSummaryReconciler{ + Client: testEnv.Client, + Scheme: scheme, + Deployer: nil, + ClusterMap: make(map[corev1.ObjectReference]*libsveltosset.Set), + ReferenceMap: make(map[corev1.ObjectReference]*libsveltosset.Set), + PolicyMux: sync.Mutex{}, + NextReconcileTimes: make(map[types.NamespacedName]controllers.ReconcileCooldown), + } + + controllers.PrepareForDeployment(reconciler, context.TODO(), clusterSummaryScope, + textlogger.NewLogger(textlogger.NewConfig())) + + featureHelmVerified := false + for i := range clusterSummary.Status.FeatureSummaries { + if clusterSummary.Status.FeatureSummaries[i].FeatureID == libsveltosv1beta1.FeatureHelm { + Expect(clusterSummary.Status.FeatureSummaries[i].Status).To(Equal(libsveltosv1beta1.FeatureStatusFailedNonRetriable)) + Expect(clusterSummary.Status.FeatureSummaries[i].FailureMessage).ToNot(BeNil()) + featureHelmVerified = true + } + } + Expect(featureHelmVerified).To(BeTrue()) + }) + It("shouldReconcile returns true when mode is OneTime but not all helm charts are deployed", func() { clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeOneTime clusterSummary.Spec.ClusterProfileSpec.HelmCharts = []configv1beta1.HelmChart{