From 312abd82109b5812a35846729476cf6ae36e5ab3 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Mon, 14 Sep 2026 11:07:49 +0200 Subject: [PATCH] bug: surface reason a profile is not being deployed When a Sveltos profile depends on another, non existing, profile ClusterSummary was being created and it was staying in Provisioning without reporting any error. This PR fixes that by updating the ClusterSummary status on why the deployment is not happening. --- controllers/clustersummary_controller.go | 4 + controllers/clustersummary_controller_test.go | 47 ++++++ test/fv/dependencies_test.go | 146 +++++++----------- 3 files changed, 105 insertions(+), 92 deletions(-) diff --git a/controllers/clustersummary_controller.go b/controllers/clustersummary_controller.go index fa0713bc..67313ee2 100644 --- a/controllers/clustersummary_controller.go +++ b/controllers/clustersummary_controller.go @@ -558,6 +558,10 @@ func (r *ClusterSummaryReconciler) prepareForDeployment(ctx context.Context, } clusterSummaryScope.SetDependenciesMessage(&msg) if !allDeployed { + // Deploy is deferred, not in progress: reflect that in featureSummaries instead of + // leaving it empty (a fresh ClusterSummary) or frozen at whatever it last had, the + // same way the mirroring dependsOn/transitionFrom gates on the delete side already do. + r.resetFeatureStatus(clusterSummaryScope, libsveltosv1beta1.FeatureStatusBlocked) r.setNextReconcileTime(clusterSummaryScope, normalRequeueAfter) return reconcile.Result{RequeueAfter: normalRequeueAfter} } diff --git a/controllers/clustersummary_controller_test.go b/controllers/clustersummary_controller_test.go index 6c1a6ad6..70418c5b 100644 --- a/controllers/clustersummary_controller_test.go +++ b/controllers/clustersummary_controller_test.go @@ -598,6 +598,53 @@ var _ = Describe("ClustersummaryController", func() { Expect(featureHelmVerified).To(BeTrue()) }) + It("prepareForDeployment marks featureSummaries Blocked when a DependsOn prerequisite is missing", func() { + // Mirrors "cleanupBeforeFinalizerRemoval marks featureSummaries Blocked while a + // dependent ClusterSummary still exists" on the delete side: before this fix, a + // ClusterSummary blocked on a not-yet-existing DependsOn prerequisite got a + // status.dependencies message but no featureSummaries entry at all, so + // `kubectl get clustersummary` (no featureSummaries to populate the printer columns + // from) showed nothing indicating anything was wrong. + missingProfileName := randomString() + clusterSummary.Spec.ClusterProfileSpec.DependsOn = []string{missingProfileName} + clusterSummary.Spec.ClusterProfileSpec.PolicyRefs = []configv1beta1.PolicyRef{ + {Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), Namespace: randomString(), Name: 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())) + + Expect(clusterSummary.Status.Dependencies).ToNot(BeNil()) + Expect(*clusterSummary.Status.Dependencies).To(ContainSubstring(missingProfileName)) + + 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.FeatureStatusBlocked)) + 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/test/fv/dependencies_test.go b/test/fv/dependencies_test.go index c88647ac..697a8bab 100644 --- a/test/fv/dependencies_test.go +++ b/test/fv/dependencies_test.go @@ -24,7 +24,6 @@ import ( . "github.com/onsi/gomega" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" configv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" "github.com/projectsveltos/addon-controller/lib/clusterops" @@ -52,121 +51,84 @@ var _ = Describe("Dependencies", func() { It("ClusterProfile with dependencies is deployed after dependencies are provisioned", Label("FV", "PULLMODE", "EXTENDED"), func() { - Byf("Create a ClusterProfile matching Cluster %s/%s", - kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName()) + // clusterProfileDependency is only built in memory here, not yet created: the + // point of this test is to cover the prerequisite not existing yet at all when + // the dependent ClusterProfile is created, not just not being provisioned yet. + Byf("Compute name for a ClusterProfile that will act as a dependency, without creating it yet") clusterProfileDependency := getClusterProfile(namePrefix, map[string]string{key: value}) clusterProfileDependency.Spec.SyncMode = configv1beta1.SyncModeContinuous - Expect(k8sClient.Create(context.TODO(), clusterProfileDependency)).To(Succeed()) - verifyClusterProfileMatches(clusterProfileDependency) - verifyClusterSummary(clusterops.ClusterProfileLabelName, clusterProfileDependency.Name, - &clusterProfileDependency.Spec, kindWorkloadCluster.GetNamespace(), - kindWorkloadCluster.GetName(), getClusterType()) - Byf("Create a ClusterProfile matching Cluster %s/%s", - kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName()) + Byf("Create a ClusterProfile matching Cluster %s/%s, depending on a ClusterProfile (%s) that does not exist yet", + kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), clusterProfileDependency.Name) clusterProfile := getClusterProfile(namePrefix, map[string]string{key: value}) clusterProfile.Spec.SyncMode = configv1beta1.SyncModeContinuous - Byf("Set ClusterProfile %s as dependency for ClusterProfile %s", - clusterProfileDependency.Name, clusterProfile.Name) clusterProfile.Spec.DependsOn = []string{clusterProfileDependency.Name} + clusterProfile.Spec.HelmCharts = []configv1beta1.HelmChart{ + { + RepositoryURL: bitnamiURL, + RepositoryName: bitnamiName, + ChartName: "bitnami/flink", + ChartVersion: "1.4.0", + ReleaseName: flinkRelease, + ReleaseNamespace: flinkRelease, + HelmChartAction: configv1beta1.HelmChartActionInstall, + }, + } Expect(k8sClient.Create(context.TODO(), clusterProfile)).To(Succeed()) verifyClusterProfileMatches(clusterProfile) - verifyClusterSummary(clusterops.ClusterProfileLabelName, clusterProfile.Name, + clusterSummary := verifyClusterSummary(clusterops.ClusterProfileLabelName, clusterProfile.Name, &clusterProfile.Spec, kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), getClusterType()) - Byf("Update ClusterProfile %s to deploy helm charts", clusterProfileDependency.Name) - currentClusterProfile := &configv1beta1.ClusterProfile{} - - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - Expect(k8sClient.Get(context.TODO(), - types.NamespacedName{Name: clusterProfileDependency.Name}, currentClusterProfile)).To(Succeed()) - currentClusterProfile.Spec.HelmCharts = []configv1beta1.HelmChart{ - { - RepositoryURL: "https://airflow.apache.org", - RepositoryName: "apache-airflow", - ChartName: "apache-airflow/airflow", - ChartVersion: "1.15.0", - ReleaseName: airflowRelease, - ReleaseNamespace: airflowRelease, - HelmChartAction: configv1beta1.HelmChartActionInstall, - Values: `createUserJob: - useHelmHooks: false - applyCustomEnv: false -migrateDatabaseJob: - useHelmHooks: false - applyCustomEnv: false`, - }, - } - return k8sClient.Update(context.TODO(), currentClusterProfile) - }) - Expect(err).To(BeNil()) - - Expect(k8sClient.Get(context.TODO(), - types.NamespacedName{Name: clusterProfileDependency.Name}, currentClusterProfile)).To(Succeed()) - - clusterSummaryDependency := verifyClusterSummary(clusterops.ClusterProfileLabelName, - currentClusterProfile.Name, ¤tClusterProfile.Spec, - kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), getClusterType()) - - Byf("Update ClusterProfile %s to deploy helm charts", clusterProfile.Name) - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - Expect(k8sClient.Get(context.TODO(), - types.NamespacedName{Name: clusterProfile.Name}, currentClusterProfile)).To(Succeed()) - currentClusterProfile.Spec.HelmCharts = []configv1beta1.HelmChart{ - { - RepositoryURL: bitnamiURL, - RepositoryName: bitnamiName, - ChartName: "bitnami/flink", - ChartVersion: "1.4.0", - ReleaseName: flinkRelease, - ReleaseNamespace: flinkRelease, - HelmChartAction: configv1beta1.HelmChartActionInstall, - }, - } - - return k8sClient.Update(context.TODO(), currentClusterProfile) - }) - Expect(err).To(BeNil()) - - Expect(k8sClient.Get(context.TODO(), - types.NamespacedName{Name: clusterProfile.Name}, currentClusterProfile)).To(Succeed()) - - clusterSummary := verifyClusterSummary(clusterops.ClusterProfileLabelName, - currentClusterProfile.Name, ¤tClusterProfile.Spec, - kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), getClusterType()) - - By("Verifying clusterSummary is not deployed till the dependencies are provisioned") + Byf("Verifying ClusterSummary %s status reports the missing dependency and Blocked, not empty, featureSummaries", + clusterSummary.Name) Eventually(func() bool { - currentClusterSummaryDependecy := &configv1beta1.ClusterSummary{} - err := k8sClient.Get(context.TODO(), - types.NamespacedName{Namespace: clusterSummaryDependency.Namespace, Name: clusterSummaryDependency.Name}, - currentClusterSummaryDependecy) - if err != nil { - return false - } currentClusterSummary := &configv1beta1.ClusterSummary{} - err = k8sClient.Get(context.TODO(), + err := k8sClient.Get(context.TODO(), types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, currentClusterSummary) if err != nil { return false } - // currentClusterSummary depends on currentClusterSummaryDependecy so expects - // currentClusterSummary.Status.FeatureSummaries to be nil till all helm charts - // in currentClusterSummaryDependecy not provisioned - if currentClusterSummaryDependecy.Status.FeatureSummaries == nil { - return currentClusterSummary.Status.FeatureSummaries == nil + if currentClusterSummary.Status.Dependencies == nil || + !strings.Contains(*currentClusterSummary.Status.Dependencies, clusterProfileDependency.Name) { + + return false } - for i := range currentClusterSummaryDependecy.Status.FeatureSummaries { - if currentClusterSummaryDependecy.Status.FeatureSummaries[i].Status != libsveltosv1beta1.FeatureStatusProvisioned { - return currentClusterSummary.Status.FeatureSummaries == nil + for i := range currentClusterSummary.Status.FeatureSummaries { + if currentClusterSummary.Status.FeatureSummaries[i].FeatureID == libsveltosv1beta1.FeatureHelm { + return currentClusterSummary.Status.FeatureSummaries[i].Status == libsveltosv1beta1.FeatureStatusBlocked } } - return true + return false }, timeout, pollingInterval).Should(BeTrue()) + Byf("Create the dependency ClusterProfile %s matching Cluster %s/%s", + clusterProfileDependency.Name, kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName()) + clusterProfileDependency.Spec.HelmCharts = []configv1beta1.HelmChart{ + { + RepositoryURL: "https://airflow.apache.org", + RepositoryName: "apache-airflow", + ChartName: "apache-airflow/airflow", + ChartVersion: "1.15.0", + ReleaseName: airflowRelease, + ReleaseNamespace: airflowRelease, + HelmChartAction: configv1beta1.HelmChartActionInstall, + Values: `createUserJob: + useHelmHooks: false + applyCustomEnv: false +migrateDatabaseJob: + useHelmHooks: false + applyCustomEnv: false`, + }, + } + Expect(k8sClient.Create(context.TODO(), clusterProfileDependency)).To(Succeed()) + verifyClusterProfileMatches(clusterProfileDependency) + clusterSummaryDependency := verifyClusterSummary(clusterops.ClusterProfileLabelName, + clusterProfileDependency.Name, &clusterProfileDependency.Spec, + kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), getClusterType()) + Byf("Verifying ClusterSummary %s status is set to Deployed for Helm feature", clusterSummaryDependency.Name) verifyFeatureStatusIsProvisioned(kindWorkloadCluster.GetNamespace(), clusterSummaryDependency.Name, libsveltosv1beta1.FeatureHelm)