From c730fbc791c81522fc569b17182175a81b938659 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Fri, 29 May 2026 19:31:42 +0200 Subject: [PATCH] (bug) WithValidateHealths and WithPreDeployChecks in prepareSetters. Problem: Both options were conditioned on clusterSummary being deleted, which is the wrong signal. A ClusterSummary can have a zero deletion timestamp while still removing a specific helm chart (e.g. chart removed from a ClusterProfile that still manages other charts). In that scenario, WithValidateHealths would be included during a pure undeploy, and WithPreDeployChecks was unconditionally included regardless of whether anything was being deployed. --- controllers/handlers_helm.go | 10 +++++----- controllers/handlers_kustomize.go | 2 +- controllers/handlers_resources.go | 4 ++-- controllers/handlers_utils.go | 9 +++++---- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/controllers/handlers_helm.go b/controllers/handlers_helm.go index 0f472dbf..37804fc5 100644 --- a/controllers/handlers_helm.go +++ b/controllers/handlers_helm.go @@ -414,7 +414,7 @@ func undeployHelmChartsInPullMode(ctx context.Context, c client.Client, clusterS if err != nil { return err } - setters := prepareSetters(clusterSummary, libsveltosv1beta1.FeatureHelm, profileRef, nil, true) + setters := prepareSetters(clusterSummary, libsveltosv1beta1.FeatureHelm, profileRef, nil, false, true) // If charts have pre/post delete hooks, those need to be deployed. A ConfigurationGroup to deploy those // is created. If this does not exist yet assume we still have to deploy those. @@ -570,7 +570,7 @@ func walkAndUndeployHelmChartsInPullMode(ctx context.Context, c client.Client, c return &configv1beta1.HandOverError{Message: msg} } - err = commitStagedResourcesForDeployment(ctx, clusterSummary, nil, mgmtResources, logger) + err = commitStagedResourcesForDeployment(ctx, clusterSummary, nil, mgmtResources, false, logger) if err != nil { return err } @@ -1164,7 +1164,7 @@ func handleCharts(ctx context.Context, clusterSummary *configv1beta1.ClusterSumm // sveltos-applier agent processes them and populates ClusterReport.HelmResourceReports. // No action will be performed by the applier in DryRun except generating the reports. if deployError == nil || clusterSummary.Spec.ClusterProfileSpec.SyncMode == configv1beta1.SyncModeDryRun { - if err := commitStagedResourcesForDeployment(ctx, clusterSummary, configurationHash, mgmtResources, logger); err != nil { + if err := commitStagedResourcesForDeployment(ctx, clusterSummary, configurationHash, mgmtResources, true, logger); err != nil { return err } } else { @@ -5300,7 +5300,7 @@ func prepareBundleSettersWithHelmInfo(currentChart *configv1beta1.HelmChart, isU } func commitStagedResourcesForDeployment(ctx context.Context, clusterSummary *configv1beta1.ClusterSummary, - configurationHash []byte, mgmtResources map[string]*unstructured.Unstructured, logger logr.Logger) error { + configurationHash []byte, mgmtResources map[string]*unstructured.Unstructured, includeDeployChecks bool, logger logr.Logger) error { profileRef, err := configv1beta1.GetProfileRef(clusterSummary) if err != nil { @@ -5314,7 +5314,7 @@ func commitStagedResourcesForDeployment(ctx context.Context, clusterSummary *con } // if a stale helm release is being deleted, run the pre/post delete checks - setters := prepareSetters(clusterSummary, libsveltosv1beta1.FeatureHelm, profileRef, configurationHash, len(staleReleases) != 0) + setters := prepareSetters(clusterSummary, libsveltosv1beta1.FeatureHelm, profileRef, configurationHash, includeDeployChecks, len(staleReleases) != 0) // Commit deployment return pullmode.CommitStagedResourcesForDeployment(ctx, getManagementClusterClient(), clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, configv1beta1.ClusterSummaryKind, diff --git a/controllers/handlers_kustomize.go b/controllers/handlers_kustomize.go index 432ccd16..34bafd75 100644 --- a/controllers/handlers_kustomize.go +++ b/controllers/handlers_kustomize.go @@ -229,7 +229,7 @@ func processKustomizeDeployment(ctx context.Context, remoteRestConfig *rest.Conf if isPullMode { setters := prepareSetters(clusterSummary, libsveltosv1beta1.FeatureKustomize, profileRef, - configurationHash, false) + configurationHash, true, false) err = pullmode.CommitStagedResourcesForDeployment(ctx, c, clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, configv1beta1.ClusterSummaryKind, clusterSummary.Name, string(libsveltosv1beta1.FeatureKustomize), diff --git a/controllers/handlers_resources.go b/controllers/handlers_resources.go index 3ee5a329..fa43c0ee 100644 --- a/controllers/handlers_resources.go +++ b/controllers/handlers_resources.go @@ -185,7 +185,7 @@ func postProcessDeployedResources(ctx context.Context, remoteRestConfig *rest.Co if isPullMode { setters := prepareSetters(clusterSummary, libsveltosv1beta1.FeatureResources, profileRef, - configurationHash, false) + configurationHash, true, false) err = pullmode.CommitStagedResourcesForDeployment(ctx, c, clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, configv1beta1.ClusterSummaryKind, clusterSummary.Name, string(libsveltosv1beta1.FeatureResources), @@ -500,7 +500,7 @@ func pullModeUndeployResources(ctx context.Context, c client.Client, clusterSumm return err } - setters := prepareSetters(clusterSummary, fID, profileRef, nil, true) + setters := prepareSetters(clusterSummary, fID, profileRef, nil, false, true) // discard all previous staged resources. This will instruct agent to undeploy err = pullmode.RemoveDeployedResources(ctx, c, clusterSummary.Spec.ClusterNamespace, diff --git a/controllers/handlers_utils.go b/controllers/handlers_utils.go index 5d00d0e5..d22e7d9a 100644 --- a/controllers/handlers_utils.go +++ b/controllers/handlers_utils.go @@ -1566,7 +1566,7 @@ func getPatchesHash(ctx context.Context, clusterSummary *configv1beta1.ClusterSu } func prepareSetters(clusterSummary *configv1beta1.ClusterSummary, featureID libsveltosv1beta1.FeatureID, - profileRef *corev1.ObjectReference, configurationHash []byte, includeDeleteChecks bool) []pullmode.Option { + profileRef *corev1.ObjectReference, configurationHash []byte, includeDeployChecks, includeDeleteChecks bool) []pullmode.Option { setters := make([]pullmode.Option, 0) if clusterSummary.Spec.ClusterProfileSpec.SyncMode == configv1beta1.SyncModeContinuousWithDriftDetection { @@ -1604,11 +1604,12 @@ func prepareSetters(clusterSummary *configv1beta1.ClusterSummary, featureID libs setters = append(setters, pullmode.WithTier(clusterSummary.Spec.ClusterProfileSpec.Tier), pullmode.WithContinueOnConflict(clusterSummary.Spec.ClusterProfileSpec.ContinueOnConflict), pullmode.WithContinueOnError(clusterSummary.Spec.ClusterProfileSpec.ContinueOnError), - pullmode.WithPreDeployChecks(clusterSummary.Spec.ClusterProfileSpec.PreDeployChecks), pullmode.WithDeployedGVKs(gvks)) - if clusterSummary.DeletionTimestamp.IsZero() { - setters = append(setters, pullmode.WithValidateHealths(clusterSummary.Spec.ClusterProfileSpec.ValidateHealths)) + if includeDeployChecks { + setters = append(setters, + pullmode.WithPreDeployChecks(clusterSummary.Spec.ClusterProfileSpec.PreDeployChecks), + pullmode.WithValidateHealths(clusterSummary.Spec.ClusterProfileSpec.ValidateHealths)) } if includeDeleteChecks {