From 02abf29c73d4922c51215a297624833a3da61ff4 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Mon, 7 Sep 2026 09:05:44 +0200 Subject: [PATCH] fix: Surface removeResourceSummary failures in ClusterSummary status removeResourceSummary failing (in both prepareForDeployment and cleanupBeforeFinalizerRemoval) was only logged, never written to ClusterSummary.status. An ongoing failure was invisible to `kubectl get clustersummary`; status just sat wherever it last was (often Provisioning), silently retrying every reconcile with no visible sign anything was wrong. Both call sites now call setFailureMessage/resetFeatureStatus on that error, the same helpers updateChartMap's NotFound handling already used three lines above one of them. FeatureStatusFailed (retriable) rather than FeatureStatusFailedNonRetriable, since this class of failure is generally transient and already retried via requeue. --- controllers/clustersummary_controller.go | 10 ++++ controllers/clustersummary_controller_test.go | 59 +++++++++++++++++++ controllers/export_test.go | 1 + go.mod | 2 +- go.sum | 4 +- 5 files changed, 73 insertions(+), 3 deletions(-) diff --git a/controllers/clustersummary_controller.go b/controllers/clustersummary_controller.go index bb230bc2..01f775af 100644 --- a/controllers/clustersummary_controller.go +++ b/controllers/clustersummary_controller.go @@ -408,6 +408,10 @@ func (r *ClusterSummaryReconciler) cleanupBeforeFinalizerRemoval(ctx context.Con err = r.removeResourceSummary(ctx, clusterSummaryScope, logger) if err != nil { logger.V(logs.LogInfo).Error(err, "failed to remove ResourceSummary.") + // See the matching comment in prepareForDeployment: surface it as a failure + // rather than leaving status wherever it last was. + r.setFailureMessage(clusterSummaryScope, err.Error()) + r.resetFeatureStatus(clusterSummaryScope, libsveltosv1beta1.FeatureStatusFailed) return reconcile.Result{Requeue: true, RequeueAfter: deleteRequeueAfter}, nil, true } r.markResourceSummaryRemovedForAllFeatures(clusterSummaryScope) @@ -557,6 +561,12 @@ func (r *ClusterSummaryReconciler) prepareForDeployment(ctx context.Context, err = r.removeResourceSummary(ctx, clusterSummaryScope, logger) if err != nil { logger.V(logs.LogInfo).Error(err, "failed to remove ResourceSummary.") + // Surface it as a failure so an ongoing connectivity or auth problem is + // visible in status instead of ClusterSummary silently sitting wherever it + // last was (often Provisioning) while this keeps failing every reconcile. + // Retriable: this is generally a transient condition that clears on its own. + r.setFailureMessage(clusterSummaryScope, err.Error()) + r.resetFeatureStatus(clusterSummaryScope, libsveltosv1beta1.FeatureStatusFailed) r.setNextReconcileTime(clusterSummaryScope, normalRequeueAfter) return reconcile.Result{RequeueAfter: normalRequeueAfter} } diff --git a/controllers/clustersummary_controller_test.go b/controllers/clustersummary_controller_test.go index 64b9aecb..796c0bb7 100644 --- a/controllers/clustersummary_controller_test.go +++ b/controllers/clustersummary_controller_test.go @@ -494,6 +494,65 @@ var _ = Describe("ClustersummaryController", func() { Expect(featureKustomizeVerified).To(BeTrue()) }) + It("prepareForDeployment surfaces a removeResourceSummary failure in ClusterSummary status", func() { + clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeContinuous + clusterSummary.Spec.ClusterProfileSpec.PolicyRefs = []configv1beta1.PolicyRef{ + { + Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), + Namespace: randomString(), + Name: randomString(), + }, + } + // No ResourceSummaryDeployed recorded yet: ShouldRemoveResourceSummary defaults to + // true, so prepareForDeployment attempts removeResourceSummary. + clusterSummary.Status.FeatureSummaries = []configv1beta1.FeatureSummary{ + {FeatureID: libsveltosv1beta1.FeatureResources, Status: libsveltosv1beta1.FeatureStatusProvisioned}, + } + + // The outer BeforeEach (prepareForDeployment, the test helper) already created a + // working kubeconfig Secret for cluster, pointing at testEnv itself. Corrupt it: this + // makes removeResourceSummary fail with a real error, neither apierrors.IsNotFound nor + // meta.IsNoMatchError, matching an unreachable or misconfigured managed cluster rather + // than one that is simply absent. + kubeconfigSecret := &corev1.Secret{} + Expect(testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name + kubeconfigPostfix}, + kubeconfigSecret)).To(Succeed()) + kubeconfigSecret.Data[testValueKey] = []byte("not a valid kubeconfig") + Expect(testEnv.Update(context.TODO(), kubeconfigSecret)).To(Succeed()) + + 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())) + + featureResourcesVerified := false + for i := range clusterSummary.Status.FeatureSummaries { + if clusterSummary.Status.FeatureSummaries[i].FeatureID == libsveltosv1beta1.FeatureResources { + Expect(clusterSummary.Status.FeatureSummaries[i].Status).To(Equal(libsveltosv1beta1.FeatureStatusFailed)) + Expect(clusterSummary.Status.FeatureSummaries[i].FailureMessage).ToNot(BeNil()) + featureResourcesVerified = true + } + } + Expect(featureResourcesVerified).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{ diff --git a/controllers/export_test.go b/controllers/export_test.go index d7e82d39..f7102485 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -74,6 +74,7 @@ var ( AreDependentsRemoved = (*ClusterSummaryReconciler).areDependentsRemoved SetFailureMessage = (*ClusterSummaryReconciler).setFailureMessage ResetFeatureStatus = (*ClusterSummaryReconciler).resetFeatureStatus + PrepareForDeployment = (*ClusterSummaryReconciler).prepareForDeployment ConvertResultStatus = (*ClusterSummaryReconciler).convertResultStatus RequeueClusterSummaryForReference = (*ClusterSummaryReconciler).requeueClusterSummaryForReference diff --git a/go.mod b/go.mod index 4af6eab7..416fac36 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/onsi/gomega v1.43.0 github.com/opencontainers/image-spec v1.1.1 github.com/pkg/errors v0.9.1 - github.com/projectsveltos/libsveltos v1.14.1-0.20260906152235-8d04320142da + github.com/projectsveltos/libsveltos v1.14.1-0.20260907061605-a1285524bdc0 github.com/prometheus/client_golang v1.24.1 github.com/sigstore/cosign/v3 v3.1.3 github.com/sigstore/sigstore v1.10.9 diff --git a/go.sum b/go.sum index 5e9f84fe..883da7a0 100644 --- a/go.sum +++ b/go.sum @@ -641,8 +641,8 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/poy/onpar v1.1.2 h1:QaNrNiZx0+Nar5dLgTVp5mXkyoVFIbepjyEoGSnhbAY= github.com/poy/onpar v1.1.2/go.mod h1:6X8FLNoxyr9kkmnlqpK6LSoiOtrO6MICtWwEuWkLjzg= -github.com/projectsveltos/libsveltos v1.14.1-0.20260906152235-8d04320142da h1:UZpRfT1NHsnpojbGdhP2fuca+iqZYbHIqAw7qbOmneY= -github.com/projectsveltos/libsveltos v1.14.1-0.20260906152235-8d04320142da/go.mod h1:U6iGj5KoC/PcTD2vh3XU6gy7g11suThT6sZSEpmLEkU= +github.com/projectsveltos/libsveltos v1.14.1-0.20260907061605-a1285524bdc0 h1:P6V3CLijEPQfiCqM684wnzMvnCHBE8Wy8r4ciWmDzwE= +github.com/projectsveltos/libsveltos v1.14.1-0.20260907061605-a1285524bdc0/go.mod h1:U6iGj5KoC/PcTD2vh3XU6gy7g11suThT6sZSEpmLEkU= github.com/projectsveltos/lua-utils/glua-json v0.0.0-20251212200258-2b3cdcb7c0f5 h1:khnc+994UszxZYu69J+R5FKiLA/Nk1JQj0EYAkwTWz0= github.com/projectsveltos/lua-utils/glua-json v0.0.0-20251212200258-2b3cdcb7c0f5/go.mod h1:yVL8KQFa9tmcxgwl9nwIMtKgtmIVC1zaFRSCfOwYvPY= github.com/projectsveltos/lua-utils/glua-runes v0.0.0-20251212200258-2b3cdcb7c0f5 h1:YbsebwRwTRhV8QacvEAdFqxcxHdeu7JTVtsBovbkgos=