diff --git a/api/v1beta1/clustersummary_types.go b/api/v1beta1/clustersummary_types.go index d318e6d2..fdd472f4 100644 --- a/api/v1beta1/clustersummary_types.go +++ b/api/v1beta1/clustersummary_types.go @@ -166,6 +166,13 @@ type HelmChartSummary struct { // LastCheckedTime is when LatestVersion/LatestPatchVersion were last evaluated. // +optional LastCheckedTime *metav1.Time `json:"lastCheckedTime,omitempty"` + + // NeedsRedeploy is set to true when drift-detection reports that a resource deployed by + // this chart changed out of band. Only meaningful when SyncMode is ContinuousWithDriftDetection. + // When true, this chart is upgraded on the next reconciliation regardless of whether its + // desired values/version changed; cleared once the upgrade succeeds. + // +optional + NeedsRedeploy bool `json:"needsRedeploy,omitempty"` } // ClusterSummarySpec defines the desired state of ClusterSummary diff --git a/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml b/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml index db072f5c..bb96095a 100644 --- a/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml +++ b/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml @@ -2532,6 +2532,13 @@ spec: greater than ChartVersion. Populated by a periodic background check, independent of the reconcile loop. Detection only: Sveltos never mutates ChartVersion based on this. type: string + needsRedeploy: + description: |- + NeedsRedeploy is set to true when drift-detection reports that a resource deployed by + this chart changed out of band. Only meaningful when SyncMode is ContinuousWithDriftDetection. + When true, this chart is upgraded on the next reconciliation regardless of whether its + desired values/version changed; cleared once the upgrade succeeds. + type: boolean patchesHash: description: PatchesHash represents of a unique value for the patches section diff --git a/controllers/export_test.go b/controllers/export_test.go index 2c0f9d3b..26b82f4c 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -136,6 +136,7 @@ var ( ShouldInstall = shouldInstall ShouldUninstall = shouldUninstall ShouldUpgrade = shouldUpgrade + NeedsRedeployForDrift = needsRedeployForDrift UpdateChartsInClusterConfiguration = updateChartsInClusterConfiguration UpdateStatusForReferencedHelmReleases = updateStatusForReferencedHelmReleases UpdateStatusForNonReferencedHelmReleases = updateStatusForNonReferencedHelmReleases @@ -143,6 +144,9 @@ var ( UpdateClusterReportWithHelmReports = updateClusterReportWithHelmReports HandleCharts = handleCharts GetHelmChartValuesHash = getHelmChartValuesHash + GetHelmChartPatchesHash = getHelmChartPatchesHash + GetPatchesHash = getPatchesHash + UpdateValueHashOnHelmChartSummary = updateValueHashOnHelmChartSummary DesiredValuesAreSubset = desiredValuesAreSubset GetCredentialsAndCAFiles = getCredentialsAndCAFiles GetInstantiatedChart = getInstantiatedChart @@ -211,6 +215,8 @@ var ( resourceSummaryInstalledCache = make(map[corev1.ObjectReference]bool) resourceSummaryInstalledCacheMu.Unlock() } + ProcessResourceSummary = processResourceSummary + MarkDriftedHelmCharts = markDriftedHelmCharts ) var ( @@ -261,6 +267,11 @@ type ( RegistryClientOptions = registryClientOptions ) +var ( + UpdateClusterSummaryHelmHashes = updateClusterSummaryHelmHashes + SetHelmChartsPatchesHash = setHelmChartsPatchesHash +) + // NewDeploymentContext constructs a deploymentContext for use in tests. func NewDeploymentContext( clusterSummary *configv1beta1.ClusterSummary, diff --git a/controllers/handlers_helm.go b/controllers/handlers_helm.go index 4384ae29..f4d4eb9b 100644 --- a/controllers/handlers_helm.go +++ b/controllers/handlers_helm.go @@ -1455,6 +1455,27 @@ func getValuesHashFromHelmChartSummary(requestedChart *configv1beta1.HelmChart, return nil } +// getNeedsRedeployFromHelmChartSummary returns the NeedsRedeploy flag stored for this chart in +// clusterSummary. buildReferencedHelmReleaseSummaries rebuilds Status.HelmReleaseSummaries from +// scratch on every reconcile, before shouldUpgrade ever runs, so NeedsRedeploy (set by +// markDriftedHelmCharts when drift-detection reports this chart changed out of band) must be +// carried forward explicitly here, the same way ValuesHash already is, or it gets silently reset +// to false before shouldUpgrade gets a chance to read it. +func getNeedsRedeployFromHelmChartSummary(requestedChart *configv1beta1.HelmChart, + clusterSummary *configv1beta1.ClusterSummary) bool { + + for i := range clusterSummary.Status.HelmReleaseSummaries { + rs := &clusterSummary.Status.HelmReleaseSummaries[i] + if rs.ReleaseName == requestedChart.ReleaseName && + rs.ReleaseNamespace == requestedChart.ReleaseNamespace { + + return rs.NeedsRedeploy + } + } + + return false +} + // getPatchesHashFromHelmChartSummary returns the patchesHash stored for this chart // in the ClusterSummary func getPatchesHashFromHelmChartSummary(requestedChart *configv1beta1.HelmChart, @@ -2842,14 +2863,30 @@ func shouldUpgrade(ctx context.Context, currentRelease *releaseInfo, instantiate return shouldUpgradeForContinuousMode(ctx, currentRelease, instantiatedChart, dCtx, currentPatchesHash, logger) } - // ContinuousWithDriftDetection: always upgrade except on the very first reconciliation - // when the live release already matches desired state (no stored ValuesHash yet). - // Subsequent reconciliations always upgrade so the drift-detection agent can repair drift. + // ContinuousWithDriftDetection: upgrade if drift-detection reported this specific chart's + // deployed resources changed out of band (NeedsRedeploy, set by markDriftedHelmCharts in + // resourcesummary_collection.go), or if the desired chart spec itself changed, same check + // Continuous mode uses. A chart that neither drifted nor changed is left alone, instead of + // being upgraded on every single reconcile regardless of whether anything needs it. // ResourceSummary is deployed by postProcessDeployedHelmCharts regardless of this decision. - if driftDetectionFirstReconciliationCanSkip(ctx, currentRelease, instantiatedChart, dCtx, currentPatchesHash, logger) { - return false + if needsRedeployForDrift(instantiatedChart, dCtx.clusterSummary) { + return true } - return true + return shouldUpgradeForContinuousMode(ctx, currentRelease, instantiatedChart, dCtx, currentPatchesHash, logger) +} + +// needsRedeployForDrift returns true if drift-detection has flagged this chart's deployed +// resources as changed out of band since the last successful deploy of this chart. +func needsRedeployForDrift(instantiatedChart *configv1beta1.HelmChart, clusterSummary *configv1beta1.ClusterSummary) bool { + for i := range clusterSummary.Status.HelmReleaseSummaries { + summary := &clusterSummary.Status.HelmReleaseSummaries[i] + if summary.ReleaseName == instantiatedChart.ReleaseName && + summary.ReleaseNamespace == instantiatedChart.ReleaseNamespace { + + return summary.NeedsRedeploy + } + } + return false } // shouldUpgradeForContinuousMode handles the upgrade decision for Continuous and DryRun modes. @@ -2885,8 +2922,9 @@ func shouldUpgradeForContinuousMode(ctx context.Context, currentRelease *release } } if oldPatchesHash == nil { - emptyHash := sha256.Sum256([]byte("")) - if reflect.DeepEqual(currentPatchesHash, emptyHash[:]) { + // getHelmChartPatchesHash returns an empty []byte when no patches are configured + // (getPatchesHash short-circuits to "" before hashing anything). + if len(currentPatchesHash) == 0 { oldPatchesHash = currentPatchesHash } } @@ -2916,35 +2954,6 @@ func shouldUpgradeForContinuousMode(ctx context.Context, currentRelease *release return false } -// driftDetectionFirstReconciliationCanSkip returns true when the live release already -// reflects the desired state and this is the first reconciliation (no stored ValuesHash). -// Conditions: no stored state, release deployed, same version, desired values are a subset -// of the coalesced release values, and no patches configured. -// Patches are not skipped: we cannot infer from the release whether they were applied before. -func driftDetectionFirstReconciliationCanSkip(ctx context.Context, currentRelease *releaseInfo, - instantiatedChart *configv1beta1.HelmChart, dCtx *deploymentContext, - currentPatchesHash []byte, logger logr.Logger) bool { - - if getValuesHashFromHelmChartSummary(instantiatedChart, dCtx.clusterSummary) != nil { - return false - } - if currentRelease == nil || currentRelease.Status != releasecommon.StatusDeployed.String() { - return false - } - emptyPatchesHash := sha256.Sum256([]byte("")) - if !reflect.DeepEqual(currentPatchesHash, emptyPatchesHash[:]) { - return false - } - current, err1 := semver.NewVersion(currentRelease.ChartVersion) - expected, err2 := semver.NewVersion(instantiatedChart.ChartVersion) - if err1 != nil || err2 != nil || !current.Equal(expected) { - return false - } - desiredValues, err := getHelmChartInstantiatedValues(ctx, dCtx.clusterSummary, - dCtx.mgmtResources, instantiatedChart, logger) - return err == nil && desiredValuesAreSubset(desiredValues, currentRelease.FullValues) -} - // shouldUninstall returns true if action is uninstall there is a release installed currently func shouldUninstall(currentRelease *releaseInfo, requestedChart *configv1beta1.HelmChart) bool { if currentRelease == nil { @@ -3152,7 +3161,7 @@ type getManagerForChart func(clusterNamespace, clusterName string, func buildReferencedHelmReleaseSummaries(ctx context.Context, c client.Client, getManager getManagerForChart, clusterSummary *configv1beta1.ClusterSummary, currentClusterSummary *configv1beta1.ClusterSummary, dCtx *deploymentContext, - patchesHash string, logger logr.Logger, + logger logr.Logger, ) (summaries []configv1beta1.HelmChartSummary, conflict bool, err error) { helmInfo := func(releaseNamespace, releaseName string) string { @@ -3180,8 +3189,16 @@ func buildReferencedHelmReleaseSummaries(ctx context.Context, c client.Client, ReleaseNamespace: instantiatedChart.ReleaseNamespace, Status: configv1beta1.HelmChartStatusManaging, FailureMessage: getFailureMessageFromHelmChartSummary(instantiatedChart, clusterSummary), - PatchesHash: []byte(patchesHash), - ValuesHash: getValuesHashFromHelmChartSummary(instantiatedChart, clusterSummary), + // Carried forward, not recomputed: shouldUpgradeForContinuousMode compares this + // against a freshly computed value to detect a real patches change. Overwriting + // it with the fresh value here, before that comparison ever runs, would make the + // comparison structurally unable to see a difference (old and current would + // always match, since both come from the same current state). PatchesHash is + // only advanced to the current value on a successful deploy, in + // updateValueHashOnHelmChartSummary, the same point ValuesHash is. + PatchesHash: getPatchesHashFromHelmChartSummary(instantiatedChart, clusterSummary), + ValuesHash: getValuesHashFromHelmChartSummary(instantiatedChart, clusterSummary), + NeedsRedeploy: getNeedsRedeployFromHelmChartSummary(instantiatedChart, clusterSummary), } currentlyReferenced[helmInfo(instantiatedChart.ReleaseNamespace, instantiatedChart.ReleaseName)] = true } else { @@ -3235,11 +3252,6 @@ func updateStatusForReferencedHelmReleases(ctx context.Context, c client.Client, conflict := false - patchesHash, err := getPatchesHash(ctx, clusterSummary, logger) - if err != nil { - return clusterSummary, false, err - } - currentClusterSummary := &configv1beta1.ClusterSummary{} err = retry.RetryOnConflict(retry.DefaultRetry, func() error { err = c.Get(ctx, @@ -3262,7 +3274,7 @@ func updateStatusForReferencedHelmReleases(ctx context.Context, c client.Client, var summaries []configv1beta1.HelmChartSummary summaries, conflict, err = buildReferencedHelmReleaseSummaries(ctx, c, chartManager.GetManagerForChart, - clusterSummary, currentClusterSummary, innerDCtx, patchesHash, logger) + clusterSummary, currentClusterSummary, innerDCtx, logger) if err != nil { return err } @@ -4669,6 +4681,14 @@ func updateValueHashOnHelmChartSummary(ctx context.Context, requestedChart *conf return nil, err } + // Advanced here, on a successful deploy, same as ValuesHash. buildReferencedHelmReleaseSummaries + // only ever carries the stored PatchesHash forward, it never recomputes it, so this is the only + // place PatchesHash catches up to the patches that were actually just applied. + helmChartPatchesHash, err := getHelmChartPatchesHash(ctx, dCtx.clusterSummary, logger) + if err != nil { + return nil, err + } + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { currentClusterSummary := &configv1beta1.ClusterSummary{} err = c.Get(ctx, @@ -4683,6 +4703,8 @@ func updateValueHashOnHelmChartSummary(ctx context.Context, requestedChart *conf rs.ReleaseNamespace == requestedChart.ReleaseNamespace { rs.ValuesHash = helmChartValuesHash + rs.PatchesHash = helmChartPatchesHash + rs.NeedsRedeploy = false setResolvedHelmChartIdentity(ctx, c, dCtx.clusterSummary, rs, requestedChart, currentRelease, logger) } } @@ -4774,6 +4796,10 @@ func clearStaleOutdatedVersionInfo(clusterSummary *configv1beta1.ClusterSummary, rs.LastCheckedTime = nil } +// getHelmChartPatchesHash returns the same hex-string-as-bytes encoding buildReferencedHelmReleaseSummaries +// stores in HelmChartSummary.PatchesHash (getPatchesHash already returns a hex-encoded sha256 digest), so +// the two are directly comparable. Do not hash patchesHash again here, that would compare a hex(sha256(...)) +// value against a sha256(hex(sha256(...))) value, which can never be equal regardless of whether patches changed. func getHelmChartPatchesHash(ctx context.Context, clusterSummary *configv1beta1.ClusterSummary, logger logr.Logger) ([]byte, error) { @@ -4782,9 +4808,7 @@ func getHelmChartPatchesHash(ctx context.Context, clusterSummary *configv1beta1. return nil, err } - h := sha256.New() - h.Write([]byte(patchesHash)) - return h.Sum(nil), nil + return []byte(patchesHash), nil } func getCredentialsAndCAFiles(ctx context.Context, c client.Client, clusterSummary *configv1beta1.ClusterSummary, diff --git a/controllers/handlers_helm_test.go b/controllers/handlers_helm_test.go index 8db5f942..2b926f42 100644 --- a/controllers/handlers_helm_test.go +++ b/controllers/handlers_helm_test.go @@ -17,6 +17,7 @@ limitations under the License. package controllers_test import ( + "bytes" "context" "crypto/sha256" "encoding/json" @@ -397,9 +398,10 @@ var _ = Describe("HandlersHelm", func() { textlogger.NewLogger(textlogger.NewConfig()))).To(BeFalse()) }) - It("shouldUpgrade returns true on second reconciliation with ContinuousWithDriftDetection (stored ValuesHash present)", func() { - // Once a ValuesHash is stored in HelmReleaseSummaries, every reconciliation must - // trigger an upgrade so the drift-detection agent can repair configuration drift. + // shouldUpgradeWithDriftDetection sets up a chart whose stored ValuesHash already matches + // what's actually deployed (nothing about the desired spec changed), with NeedsRedeploy set + // as requested, and returns what shouldUpgrade decides for it under ContinuousWithDriftDetection. + shouldUpgradeWithDriftDetection := func(needsRedeploy bool) bool { ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterSummary.Spec.ClusterNamespace}} Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) @@ -414,12 +416,25 @@ var _ = Describe("HandlersHelm", func() { Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeContinuousWithDriftDetection + + requestChart := &configv1beta1.HelmChart{ + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + ChartVersion: testChartVersion100, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + + storedValuesHash, err := controllers.GetHelmChartValuesHash(context.TODO(), testEnv, requestChart, + clusterSummary, nil, textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + clusterSummary.Status.HelmReleaseSummaries = []configv1beta1.HelmChartSummary{ { ReleaseName: testReleaseNameNginxLatest, ReleaseNamespace: testNginxRepo, - ValuesHash: []byte("previously-stored-hash"), + ValuesHash: storedValuesHash, Status: configv1beta1.HelmChartStatusManaging, + NeedsRedeploy: needsRedeploy, }, } @@ -428,6 +443,48 @@ var _ = Describe("HandlersHelm", func() { ChartVersion: testChartVersion100, FullValues: map[string]interface{}{testReplicaCountKey: 1}, } + + return controllers.ShouldUpgrade(context.TODO(), currentRelease, requestChart, + controllers.NewDeploymentContext(clusterSummary, nil, nil), + textlogger.NewLogger(textlogger.NewConfig())) + } + + It("shouldUpgrade returns false with ContinuousWithDriftDetection when stored state matches and NeedsRedeploy is false", func() { + // Nothing changed and drift-detection hasn't flagged this chart, so there's nothing to + // do: shouldUpgrade must not blindly re-upgrade every chart on every reconcile + // regardless of state. + Expect(shouldUpgradeWithDriftDetection(false)).To(BeFalse()) + }) + + It("shouldUpgrade returns true with ContinuousWithDriftDetection when NeedsRedeploy is set, even if stored state matches", func() { + // Same as above, nothing about the desired spec changed, but drift-detection flagged + // this chart's deployed resources as changed out of band. That alone must trigger the + // upgrade, scoped to this chart. + Expect(shouldUpgradeWithDriftDetection(true)).To(BeTrue()) + }) + + It("shouldUpgrade returns true when patches are added to a chart that previously had none", func() { + // Regression test: buildReferencedHelmReleaseSummaries used to overwrite PatchesHash + // with the freshly computed value on every reconcile, before shouldUpgrade ever compared + // it, so a real patches change was undetectable, old and current were always equal by + // construction, since both came from the same current state. PatchesHash now behaves + // like ValuesHash: carried forward until a successful deploy, so a genuine change here + // (patches added where there were none) must be visible to the comparison. + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterSummary.Spec.ClusterNamespace}} + Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterSummary.Spec.ClusterName, + Namespace: clusterSummary.Spec.ClusterNamespace, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeContinuous + requestChart := &configv1beta1.HelmChart{ ReleaseName: testReleaseNameNginxLatest, ReleaseNamespace: testNginxRepo, @@ -435,6 +492,41 @@ var _ = Describe("HandlersHelm", func() { HelmChartAction: configv1beta1.HelmChartActionInstall, } + // Deployed previously with no patches configured. + noPatchesHash, err := controllers.GetHelmChartPatchesHash(context.TODO(), clusterSummary, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + + storedValuesHash, err := controllers.GetHelmChartValuesHash(context.TODO(), testEnv, requestChart, + clusterSummary, nil, textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + + clusterSummary.Status.HelmReleaseSummaries = []configv1beta1.HelmChartSummary{ + { + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + ValuesHash: storedValuesHash, + PatchesHash: noPatchesHash, + Status: configv1beta1.HelmChartStatusManaging, + }, + } + + // Patches just got added to the ClusterProfile. + clusterSummary.Spec.ClusterProfileSpec.Patches = []libsveltosv1beta1.Patch{ + { + Patch: testManagedAnnotationPatch, + Target: &libsveltosv1beta1.PatchSelector{ + Kind: testKindDeployment, + }, + }, + } + + currentRelease := &controllers.ReleaseInfo{ + Status: releasecommon.StatusDeployed.String(), + ChartVersion: testChartVersion100, + FullValues: map[string]interface{}{testReplicaCountKey: 1}, + } + Expect(controllers.ShouldUpgrade(context.TODO(), currentRelease, requestChart, controllers.NewDeploymentContext(clusterSummary, nil, nil), textlogger.NewLogger(textlogger.NewConfig()))).To(BeTrue()) @@ -460,9 +552,7 @@ var _ = Describe("HandlersHelm", func() { clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeContinuousWithDriftDetection clusterSummary.Spec.ClusterProfileSpec.Patches = []libsveltosv1beta1.Patch{ { - Patch: `- op: add - path: /metadata/annotations/projectsveltos.io~1managed - value: "true"`, + Patch: testManagedAnnotationPatch, Target: &libsveltosv1beta1.PatchSelector{ Kind: testKindDeployment, }, @@ -486,6 +576,110 @@ var _ = Describe("HandlersHelm", func() { textlogger.NewLogger(textlogger.NewConfig()))).To(BeTrue()) }) + It("getHelmChartPatchesHash returns the same encoding stored by buildReferencedHelmReleaseSummaries", func() { + // buildReferencedHelmReleaseSummaries stores getPatchesHash's return value directly as + // []byte(patchesHash), a hex-encoded digest kept as text. getHelmChartPatchesHash must + // return that same encoding, not hash it again, otherwise the two are never equal and + // shouldUpgradeForContinuousMode reports "patches changed" on every single reconcile + // regardless of whether patches actually changed. + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterSummary.Spec.ClusterNamespace}} + Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterSummary.Spec.ClusterName, + Namespace: clusterSummary.Spec.ClusterNamespace, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + clusterSummary.Spec.ClusterProfileSpec.Patches = []libsveltosv1beta1.Patch{ + { + Patch: testManagedAnnotationPatch, + Target: &libsveltosv1beta1.PatchSelector{ + Kind: testKindDeployment, + }, + }, + } + + storedFormat, err := controllers.GetPatchesHash(context.TODO(), clusterSummary, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + Expect(storedFormat).ToNot(BeEmpty()) + + comparisonFormat, err := controllers.GetHelmChartPatchesHash(context.TODO(), clusterSummary, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + + Expect(comparisonFormat).To(Equal([]byte(storedFormat))) + }) + + It("shouldUpgrade returns false when patches are configured but unchanged", func() { + // Regression test: previously getHelmChartPatchesHash hashed the already-hex-encoded + // stored value a second time, so this always evaluated to true, forcing a redeploy of + // every chart with patches configured on every reconcile, in any sync mode, regardless + // of whether anything actually changed. + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterSummary.Spec.ClusterNamespace}} + Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterSummary.Spec.ClusterName, + Namespace: clusterSummary.Spec.ClusterNamespace, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + clusterSummary.Spec.ClusterProfileSpec.SyncMode = configv1beta1.SyncModeContinuous + clusterSummary.Spec.ClusterProfileSpec.Patches = []libsveltosv1beta1.Patch{ + { + Patch: testManagedAnnotationPatch, + Target: &libsveltosv1beta1.PatchSelector{ + Kind: testKindDeployment, + }, + }, + } + + requestChart := &configv1beta1.HelmChart{ + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + ChartVersion: testChartVersion100, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + + storedValuesHash, err := controllers.GetHelmChartValuesHash(context.TODO(), testEnv, requestChart, + clusterSummary, nil, textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + + storedPatchesHash, err := controllers.GetHelmChartPatchesHash(context.TODO(), clusterSummary, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + + clusterSummary.Status.HelmReleaseSummaries = []configv1beta1.HelmChartSummary{ + { + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + ValuesHash: storedValuesHash, + PatchesHash: storedPatchesHash, + Status: configv1beta1.HelmChartStatusManaging, + }, + } + + currentRelease := &controllers.ReleaseInfo{ + Status: releasecommon.StatusDeployed.String(), + ChartVersion: testChartVersion100, + FullValues: map[string]interface{}{testReplicaCountKey: 1}, + } + + Expect(controllers.ShouldUpgrade(context.TODO(), currentRelease, requestChart, + controllers.NewDeploymentContext(clusterSummary, nil, nil), + textlogger.NewLogger(textlogger.NewConfig()))).To(BeFalse()) + }) + It("UpdateStatusForeferencedHelmReleases updates ClusterSummary.Status.HelmReleaseSummaries", func() { calicoChart := &configv1beta1.HelmChart{ RepositoryURL: "https://projectcalico.docs.tigera.io/charts", @@ -570,6 +764,152 @@ var _ = Describe("HandlersHelm", func() { }) + It("UpdateStatusForReferencedHelmReleases preserves NeedsRedeploy across a rebuild", func() { + // Regression test: UpdateStatusForReferencedHelmReleases (via buildReferencedHelmReleaseSummaries) + // rebuilds Status.HelmReleaseSummaries from scratch on every reconcile, before shouldUpgrade + // ever runs. It already carries ValuesHash forward explicitly; it must do the same for + // NeedsRedeploy, set by markDriftedHelmCharts when drift-detection reports this chart + // changed out of band, otherwise the flag is silently wiped before shouldUpgrade can act on + // it, and a chart with a real, detected drift is never actually redeployed. + nginxChart := &configv1beta1.HelmChart{ + RepositoryURL: testRepoURLNginxStable, + RepositoryName: testRepoNameNginxStable, + ChartName: testChartNameNginxIngress, + ChartVersion: testChartVersion100, + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + + clusterSummary.Spec.ClusterProfileSpec = configv1beta1.Spec{ + HelmCharts: []configv1beta1.HelmChart{*nginxChart}, + } + clusterSummary.Namespace = defaultNamespace + clusterSummary.Spec.ClusterNamespace = defaultNamespace + + Expect(testEnv.Create(context.TODO(), clusterSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, clusterSummary)).To(Succeed()) + + clusterSummary.Status = configv1beta1.ClusterSummaryStatus{ + HelmReleaseSummaries: []configv1beta1.HelmChartSummary{ + { + ReleaseName: nginxChart.ReleaseName, + ReleaseNamespace: nginxChart.ReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, + NeedsRedeploy: true, + }, + }, + } + Expect(testEnv.Status().Update(context.TODO(), clusterSummary)).To(Succeed()) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterSummary.Spec.ClusterName, + Namespace: clusterSummary.Spec.ClusterNamespace, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + manager, err := chartmanager.GetChartManagerInstance(context.TODO(), testEnv.Client) + Expect(err).To(BeNil()) + manager.RegisterClusterSummaryForCharts(clusterSummary) + + _, conflict, err := controllers.UpdateStatusForReferencedHelmReleases(context.TODO(), + testEnv.Client, clusterSummary, nil, textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + Expect(conflict).To(BeFalse()) + + Eventually(func() bool { + currentClusterSummary := &configv1beta1.ClusterSummary{} + err = testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary) + if err != nil || len(currentClusterSummary.Status.HelmReleaseSummaries) != 1 { + return false + } + return currentClusterSummary.Status.HelmReleaseSummaries[0].NeedsRedeploy + }, timeout, pollingInterval).Should(BeTrue()) + }) + + It("UpdateStatusForReferencedHelmReleases carries PatchesHash forward instead of recomputing it", func() { + // Regression test: buildReferencedHelmReleaseSummaries used to store the freshly computed + // PatchesHash directly, on every reconcile, before shouldUpgrade ever ran. That made a + // real patches change undetectable: by the time shouldUpgrade compared old vs current, + // both had already been set to the same fresh value moments earlier in the same + // reconcile. PatchesHash must instead be carried forward here, the same way ValuesHash + // already is, and only advanced to the current value on a successful deploy + // (updateValueHashOnHelmChartSummary), so shouldUpgrade's comparison has something real + // to compare against. + nginxChart := &configv1beta1.HelmChart{ + RepositoryURL: testRepoURLNginxStable, + RepositoryName: testRepoNameNginxStable, + ChartName: testChartNameNginxIngress, + ChartVersion: testChartVersion100, + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + + clusterSummary.Spec.ClusterProfileSpec = configv1beta1.Spec{ + HelmCharts: []configv1beta1.HelmChart{*nginxChart}, + // Patches configured now. If PatchesHash were recomputed instead of carried + // forward, this reconcile would store the hash of *this* patch list, matching + // whatever shouldUpgrade computes as "current" right after, and the change below + // would never be visible. + Patches: []libsveltosv1beta1.Patch{ + { + Patch: testManagedAnnotationPatch, + Target: &libsveltosv1beta1.PatchSelector{ + Kind: testKindDeployment, + }, + }, + }, + } + clusterSummary.Namespace = defaultNamespace + clusterSummary.Spec.ClusterNamespace = defaultNamespace + + Expect(testEnv.Create(context.TODO(), clusterSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, clusterSummary)).To(Succeed()) + + stalePatchesHash := []byte("stale-patches-hash-from-before-patches-were-added") + clusterSummary.Status = configv1beta1.ClusterSummaryStatus{ + HelmReleaseSummaries: []configv1beta1.HelmChartSummary{ + { + ReleaseName: nginxChart.ReleaseName, + ReleaseNamespace: nginxChart.ReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, + PatchesHash: stalePatchesHash, + }, + }, + } + Expect(testEnv.Status().Update(context.TODO(), clusterSummary)).To(Succeed()) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterSummary.Spec.ClusterName, + Namespace: clusterSummary.Spec.ClusterNamespace, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + manager, err := chartmanager.GetChartManagerInstance(context.TODO(), testEnv.Client) + Expect(err).To(BeNil()) + manager.RegisterClusterSummaryForCharts(clusterSummary) + + // Checked on the function's direct return value, not via a Get against testEnv's cache: + // the cache can still be showing the pre-call state for a beat after this returns, which + // would let a broken (fresh-value-writing) implementation slip past an Eventually/Get + // check by accident, since the stale cache read looks like the pre-existing value too. + updatedClusterSummary, conflict, err := controllers.UpdateStatusForReferencedHelmReleases(context.TODO(), + testEnv.Client, clusterSummary, nil, textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + Expect(conflict).To(BeFalse()) + Expect(updatedClusterSummary.Status.HelmReleaseSummaries).To(HaveLen(1)) + Expect(bytes.Equal(updatedClusterSummary.Status.HelmReleaseSummaries[0].PatchesHash, stalePatchesHash)).To(BeTrue()) + }) + It("updateStatusForeferencedHelmReleases is no-op in DryRun mode", func() { clusterSummary.Spec.ClusterProfileSpec = configv1beta1.Spec{ HelmCharts: []configv1beta1.HelmChart{ @@ -1140,9 +1480,9 @@ var _ = Describe("Hash methods", func() { } nginxChart := configv1beta1.HelmChart{ - RepositoryURL: "https://helm.nginx.com/stable/", - RepositoryName: "nginx-stable", - ChartName: "nginx-stable/nginx-ingress", + RepositoryURL: testRepoURLNginxStable, + RepositoryName: testRepoNameNginxStable, + ChartName: testChartNameNginxIngress, ChartVersion: "0.17.1", ReleaseName: testReleaseNameNginxLatest, ReleaseNamespace: testNginxRepo, diff --git a/controllers/init_container_work.go b/controllers/init_container_work.go index 7e59fbb1..9cc0553d 100644 --- a/controllers/init_container_work.go +++ b/controllers/init_container_work.go @@ -124,6 +124,16 @@ func updateClusterSummaryHelmHashes(ctx context.Context, directClient client.Cli } setHelmHash(currentClusterSummary, helmHash) + // PatchesHash is one value for the whole ClusterProfileSpec.Patches list, shared across + // every chart (same as buildReferencedHelmReleaseSummaries), so it's computed once here, + // not per chart. + patchesHash, err := getHelmChartPatchesHash(ctx, currentClusterSummary, logger) + if err != nil { + logger.V(logs.LogInfo).Error(err, "failed to get helm patches hash") + return err + } + setHelmChartsPatchesHash(currentClusterSummary, patchesHash) + for i := range currentClusterSummary.Spec.ClusterProfileSpec.HelmCharts { helmChart := ¤tClusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] innerDCtx := &deploymentContext{ @@ -171,3 +181,12 @@ func setHelmChartValueHash(clusterSummary *configv1beta1.ClusterSummary, helmCha } } } + +// setHelmChartsPatchesHash stores patchesHash on every HelmReleaseSummaries entry, mirroring +// buildReferencedHelmReleaseSummaries: PatchesHash is one value for the whole ClusterProfileSpec.Patches +// list, not per chart, so every chart gets the same value. +func setHelmChartsPatchesHash(clusterSummary *configv1beta1.ClusterSummary, patchesHash []byte) { + for i := range clusterSummary.Status.HelmReleaseSummaries { + clusterSummary.Status.HelmReleaseSummaries[i].PatchesHash = patchesHash + } +} diff --git a/controllers/init_container_work_test.go b/controllers/init_container_work_test.go new file mode 100644 index 00000000..66460216 --- /dev/null +++ b/controllers/init_container_work_test.go @@ -0,0 +1,155 @@ +/* +Copyright 2026. projectsveltos.io. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers_test + +import ( + "bytes" + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/v2/textlogger" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + + configv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" + "github.com/projectsveltos/addon-controller/controllers" + libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" +) + +var _ = Describe("Init container work", func() { + It("updateClusterSummaryHelmHashes refreshes ValuesHash and PatchesHash without triggering a redeploy", func() { + clusterNamespace := randomString() + + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterNamespace}} + Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: randomString(), + Namespace: clusterNamespace, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + requestChart := configv1beta1.HelmChart{ + RepositoryURL: testRepoURLNginxStable, + RepositoryName: testRepoNameNginxStable, + ChartName: testChartNameNginxIngress, + ReleaseName: testReleaseNameNginxLatest, + ReleaseNamespace: testNginxRepo, + ChartVersion: testChartVersion100, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + + clusterSummary := &configv1beta1.ClusterSummary{ + ObjectMeta: metav1.ObjectMeta{ + Name: randomString(), + Namespace: clusterNamespace, + }, + Spec: configv1beta1.ClusterSummarySpec{ + ClusterNamespace: clusterNamespace, + ClusterName: cluster.Name, + ClusterType: libsveltosv1beta1.ClusterTypeCapi, + ClusterProfileSpec: configv1beta1.Spec{ + SyncMode: configv1beta1.SyncModeContinuousWithDriftDetection, + HelmCharts: []configv1beta1.HelmChart{requestChart}, + Patches: []libsveltosv1beta1.Patch{ + { + Patch: testManagedAnnotationPatch, + Target: &libsveltosv1beta1.PatchSelector{ + Kind: testKindDeployment, + }, + }, + }, + }, + }, + } + Expect(testEnv.Create(context.TODO(), clusterSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, clusterSummary)).To(Succeed()) + + currentClusterSummary := &configv1beta1.ClusterSummary{} + Expect(testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary)).To(Succeed()) + + // Simulate the state right before an addon-controller upgrade: the feature is + // already Provisioned, and the stored hashes are stale/wrong (as if computed by an + // older, different hash implementation). + currentClusterSummary.Status.FeatureSummaries = []configv1beta1.FeatureSummary{ + { + FeatureID: libsveltosv1beta1.FeatureHelm, + Status: libsveltosv1beta1.FeatureStatusProvisioned, + Hash: []byte(randomString()), + }, + } + currentClusterSummary.Status.HelmReleaseSummaries = []configv1beta1.HelmChartSummary{ + { + ReleaseName: requestChart.ReleaseName, + ReleaseNamespace: requestChart.ReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, + ValuesHash: []byte("stale-values-hash"), + PatchesHash: []byte("stale-patches-hash"), + }, + } + Expect(testEnv.Status().Update(context.TODO(), currentClusterSummary)).To(Succeed()) + Eventually(func() error { + return testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary) + }, timeout, pollingInterval).Should(Succeed()) + + logger := textlogger.NewLogger(textlogger.NewConfig()) + + expectedPatchesHash, err := controllers.GetHelmChartPatchesHash(context.TODO(), currentClusterSummary, logger) + Expect(err).To(BeNil()) + Expect(expectedPatchesHash).ToNot(Equal([]byte("stale-patches-hash"))) + + Expect(controllers.UpdateClusterSummaryHelmHashes(context.TODO(), testEnv.Client, "", + currentClusterSummary, logger)).To(Succeed()) + + Eventually(func() bool { + err := testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary) + if err != nil { + return false + } + if len(currentClusterSummary.Status.HelmReleaseSummaries) != 1 { + return false + } + rs := currentClusterSummary.Status.HelmReleaseSummaries[0] + return bytes.Equal(rs.PatchesHash, expectedPatchesHash) && + string(rs.ValuesHash) != "stale-values-hash" + }, timeout, pollingInterval).Should(BeTrue()) + + // Refreshing the stored hashes must not, by itself, mark the feature for redeploy: + // no reset of Status/Hash, that decision belongs to the main reconcile loop, not to + // this startup pre-warm step. + for i := range currentClusterSummary.Status.FeatureSummaries { + if currentClusterSummary.Status.FeatureSummaries[i].FeatureID == libsveltosv1beta1.FeatureHelm { + Expect(currentClusterSummary.Status.FeatureSummaries[i].Status).To( + Equal(libsveltosv1beta1.FeatureStatusProvisioned)) + } + } + }) +}) diff --git a/controllers/resourcesummary_collection.go b/controllers/resourcesummary_collection.go index afa2a58e..9346443e 100644 --- a/controllers/resourcesummary_collection.go +++ b/controllers/resourcesummary_collection.go @@ -532,6 +532,7 @@ func processResourceSummary(ctx context.Context, clusterClient client.Client, l.V(logs.LogDebug).Info("redeploy helm") clusterSummary.Status.FeatureSummaries[i].Hash = nil clusterSummary.Status.FeatureSummaries[i].Status = libsveltosv1beta1.FeatureStatusProvisioning + markDriftedHelmCharts(clusterSummary, rs.Status.DriftedHelmCharts, l) trackDrifts(clusterSummaryNamespace, clusterSummary.Spec.ClusterName, string(clusterSummary.Status.FeatureSummaries[i].FeatureID), string(clusterSummary.Spec.ClusterType), profileKind, profileNamespace, profileName, logger) } @@ -586,9 +587,39 @@ func resetResourceSummaryStatus(ctx context.Context, remoteClient client.Client, resourceSummary.Status.ResourcesChanged = false resourceSummary.Status.KustomizeResourcesChanged = false resourceSummary.Status.HelmResourcesChanged = false + resourceSummary.Status.DriftedHelmCharts = nil return remoteClient.Status().Update(ctx, resourceSummary) } +// markDriftedHelmCharts sets NeedsRedeploy on the HelmReleaseSummaries entries matching +// driftedCharts (by release name/namespace), so shouldUpgrade can later scope the redeploy +// to only those charts instead of the whole Helm feature. If driftedCharts is empty (e.g. an +// older drift-detection-manager that doesn't report chart ownership yet), every chart is +// marked, same as today's behavior, so drift is never silently missed. +func markDriftedHelmCharts(clusterSummary *configv1beta1.ClusterSummary, + driftedCharts []libsveltosv1beta1.HelmChartRef, logger logr.Logger) { + + if len(driftedCharts) == 0 { + logger.V(logs.LogDebug).Info("no chart-scoped drift info available, marking all charts for redeploy") + for i := range clusterSummary.Status.HelmReleaseSummaries { + clusterSummary.Status.HelmReleaseSummaries[i].NeedsRedeploy = true + } + return + } + + for i := range clusterSummary.Status.HelmReleaseSummaries { + summary := &clusterSummary.Status.HelmReleaseSummaries[i] + for j := range driftedCharts { + if summary.ReleaseName == driftedCharts[j].ReleaseName && + summary.ReleaseNamespace == driftedCharts[j].ReleaseNamespace { + + summary.NeedsRedeploy = true + break + } + } + } +} + func getListOfClusterWithDriftDetectionDeployed(ctx context.Context, c client.Client, ) (map[corev1.ObjectReference]bool, error) { diff --git a/controllers/resourcesummary_collection_test.go b/controllers/resourcesummary_collection_test.go index bb7589c9..7e9300bb 100644 --- a/controllers/resourcesummary_collection_test.go +++ b/controllers/resourcesummary_collection_test.go @@ -91,7 +91,7 @@ var _ = Describe("ResourceSummary Collection", func() { } Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) - resourceSummary := getResourceSummary(nil, nil) + resourceSummary := getResourceSummary() resourceSummary.Annotations = map[string]string{ libsveltosv1beta1.ClusterSummaryNameAnnotation: clusterSummary.Name, libsveltosv1beta1.ClusterSummaryNamespaceAnnotation: clusterSummary.Namespace, @@ -151,6 +151,118 @@ var _ = Describe("ResourceSummary Collection", func() { return err == nil && !currentResourceSummary.Status.HelmResourcesChanged }, timeout, pollingInterval).Should(BeTrue()) }) + It("processResourceSummary scopes NeedsRedeploy to only the drifted chart", func() { + cluster := prepareCluster() + + chartA := configv1beta1.HelmChartSummary{ + ReleaseName: randomString(), + ReleaseNamespace: randomString(), + Status: configv1beta1.HelmChartStatusManaging, + } + chartB := configv1beta1.HelmChartSummary{ + ReleaseName: randomString(), + ReleaseNamespace: randomString(), + Status: configv1beta1.HelmChartStatusManaging, + } + + clusterSummary := &configv1beta1.ClusterSummary{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: clusterProfileNamePrefix + randomString(), + Labels: map[string]string{clusterops.ClusterProfileLabelName: randomString()}, + }, + Spec: configv1beta1.ClusterSummarySpec{ + ClusterType: libsveltosv1beta1.ClusterTypeCapi, + }, + } + Expect(testEnv.Create(context.TODO(), clusterSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, clusterSummary)).To(Succeed()) + + currentClusterSummary := &configv1beta1.ClusterSummary{} + Expect(testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary)).To(Succeed()) + currentClusterSummary.Status.FeatureSummaries = []configv1beta1.FeatureSummary{ + { + FeatureID: libsveltosv1beta1.FeatureHelm, + Status: libsveltosv1beta1.FeatureStatusProvisioned, + Hash: []byte(randomString()), + }, + } + currentClusterSummary.Status.HelmReleaseSummaries = []configv1beta1.HelmChartSummary{chartA, chartB} + Expect(testEnv.Status().Update(context.TODO(), currentClusterSummary)).To(Succeed()) + + resourceSummary := getResourceSummary() + resourceSummary.Annotations = map[string]string{ + libsveltosv1beta1.ClusterSummaryNameAnnotation: clusterSummary.Name, + libsveltosv1beta1.ClusterSummaryNamespaceAnnotation: clusterSummary.Namespace, + } + Expect(testEnv.Create(context.TODO(), resourceSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, resourceSummary)).To(Succeed()) + + currentResourceSummary := &libsveltosv1beta1.ResourceSummary{} + Expect(testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: resourceSummary.Namespace, Name: resourceSummary.Name}, + currentResourceSummary)).To(Succeed()) + currentResourceSummary.Status.HelmResourcesChanged = true + // Only chartA drifted. chartB must be left alone. + currentResourceSummary.Status.DriftedHelmCharts = []libsveltosv1beta1.HelmChartRef{ + { + ChartName: randomString(), + ReleaseName: chartA.ReleaseName, + ReleaseNamespace: chartA.ReleaseNamespace, + }, + } + Expect(testEnv.Status().Update(context.TODO(), currentResourceSummary)).To(Succeed()) + + Eventually(func() bool { + err := testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: resourceSummary.Namespace, Name: resourceSummary.Name}, + currentResourceSummary) + return err == nil && currentResourceSummary.Status.HelmResourcesChanged + }, timeout, pollingInterval).Should(BeTrue()) + + Expect(controllers.ProcessResourceSummary(context.TODO(), testEnv.Client, currentResourceSummary, + textlogger.NewLogger(textlogger.NewConfig()))).To(Succeed()) + + Eventually(func() bool { + err := testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary) + if err != nil { + return false + } + for i := range currentClusterSummary.Status.HelmReleaseSummaries { + rs := ¤tClusterSummary.Status.HelmReleaseSummaries[i] + if rs.ReleaseName == chartA.ReleaseName { + if !rs.NeedsRedeploy { + return false + } + } else if rs.NeedsRedeploy { + return false + } + } + return true + }, timeout, pollingInterval).Should(BeTrue()) + }) + + It("markDriftedHelmCharts marks every chart when no chart-scoped drift info is available", func() { + clusterSummary := &configv1beta1.ClusterSummary{ + Status: configv1beta1.ClusterSummaryStatus{ + HelmReleaseSummaries: []configv1beta1.HelmChartSummary{ + {ReleaseName: randomString(), ReleaseNamespace: randomString()}, + {ReleaseName: randomString(), ReleaseNamespace: randomString()}, + }, + }, + } + + controllers.MarkDriftedHelmCharts(clusterSummary, nil, textlogger.NewLogger(textlogger.NewConfig())) + + for i := range clusterSummary.Status.HelmReleaseSummaries { + Expect(clusterSummary.Status.HelmReleaseSummaries[i].NeedsRedeploy).To(BeTrue()) + } + }) + It("isResourceSummaryInstalledCached caches positive result", func() { controllers.ResetResourceSummaryInstalledCache() @@ -233,7 +345,7 @@ var _ = Describe("ResourceSummary Collection", func() { Expect(testEnv.Status().Update(context.TODO(), currentClusterSummary)).To(Succeed()) // RS labeled as CAPI cluster — drift detected. - capiRS := getResourceSummary(nil, nil) + capiRS := getResourceSummary() capiRS.Namespace = capiCluster.Namespace capiRS.Annotations = map[string]string{ libsveltosv1beta1.ClusterSummaryNameAnnotation: clusterSummary.Name, @@ -253,7 +365,7 @@ var _ = Describe("ResourceSummary Collection", func() { // RS labeled as Sveltos cluster — same namespace/name as the CAPI cluster, also with drift. // This must NOT be processed since its cluster type does not match the CAPI entry in clustersWithDD. - sveltosRS := getResourceSummary(nil, nil) + sveltosRS := getResourceSummary() sveltosRS.Namespace = capiCluster.Namespace sveltosRS.Annotations = map[string]string{ libsveltosv1beta1.ClusterSummaryNameAnnotation: clusterSummary.Name, @@ -311,44 +423,13 @@ var _ = Describe("ResourceSummary Collection", func() { }) }) -func getResourceSummary(resource, helmResource *corev1.ObjectReference) *libsveltosv1beta1.ResourceSummary { - rs := &libsveltosv1beta1.ResourceSummary{ +// getResourceSummary returns a bare ResourceSummary. None of the tests in this file need it +// pre-populated with Spec.Resources/ChartResources, they set Status directly instead. +func getResourceSummary() *libsveltosv1beta1.ResourceSummary { + return &libsveltosv1beta1.ResourceSummary{ ObjectMeta: metav1.ObjectMeta{ Name: randomString(), Namespace: sveltosNamespace, }, } - - if resource != nil { - rs.Spec.Resources = []libsveltosv1beta1.Resource{ - { - Name: resource.Name, - Namespace: resource.Namespace, - Kind: resource.Kind, - Group: resource.GroupVersionKind().Group, - Version: resource.GroupVersionKind().Version, - }, - } - } - - if helmResource != nil { - rs.Spec.ChartResources = []libsveltosv1beta1.HelmResources{ - { - ChartName: randomString(), - ReleaseName: randomString(), - ReleaseNamespace: randomString(), - Resources: []libsveltosv1beta1.ResourceSummaryResource{ - { - Name: helmResource.Name, - Namespace: helmResource.Namespace, - Kind: helmResource.Kind, - Group: helmResource.GroupVersionKind().Group, - Version: helmResource.GroupVersionKind().Version, - }, - }, - }, - } - } - - return rs } diff --git a/controllers/test_constants_test.go b/controllers/test_constants_test.go index e565ba56..508bb7c3 100644 --- a/controllers/test_constants_test.go +++ b/controllers/test_constants_test.go @@ -43,6 +43,9 @@ const ( testReleaseNameContour = "contour-latest" testRepoURLBitnami = "https://charts.bitnami.com/bitnami" testChartNameBitnamiContour = "bitnami/contour" + testRepoURLNginxStable = "https://helm.nginx.com/stable/" + testRepoNameNginxStable = "nginx-stable" + testChartNameNginxIngress = "nginx-stable/nginx-ingress" // Label keys and values testEnvLabelKey = "env" @@ -114,6 +117,9 @@ const ( testEnvLabelPatch = `- op: add path: /metadata/labels/environment value: production` + testManagedAnnotationPatch = `- op: add + path: /metadata/annotations/projectsveltos.io~1managed + value: "true"` // Verbs and resources testCreateVerb = "create" diff --git a/go.mod b/go.mod index c1058dfa..ed447ea9 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/onsi/gomega v1.42.1 github.com/opencontainers/image-spec v1.1.1 github.com/pkg/errors v0.9.1 - github.com/projectsveltos/libsveltos v1.13.1-0.20260812123753-3148ff0e45f2 + github.com/projectsveltos/libsveltos v1.13.1-0.20260813141903-0bf44bc3c246 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 511a1ed4..ea686e52 100644 --- a/go.sum +++ b/go.sum @@ -639,8 +639,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.13.1-0.20260812123753-3148ff0e45f2 h1:kPqbjM5zOj2IazbaCdjgO/IIyqMdWV8N60L5DIl/HjE= -github.com/projectsveltos/libsveltos v1.13.1-0.20260812123753-3148ff0e45f2/go.mod h1:dgQKoyCm3xr5TV0vJxbq+dkuBLmwm4mqt6tMAgJiP+s= +github.com/projectsveltos/libsveltos v1.13.1-0.20260813141903-0bf44bc3c246 h1:ByiXtT8ebkKCnsFl3iwHH8ZbK3hvgc8TwmT7U4O0UuM= +github.com/projectsveltos/libsveltos v1.13.1-0.20260813141903-0bf44bc3c246/go.mod h1:dgQKoyCm3xr5TV0vJxbq+dkuBLmwm4mqt6tMAgJiP+s= 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= diff --git a/lib/crd/clustersummaries.go b/lib/crd/clustersummaries.go index c1668b99..fe5d8d25 100644 --- a/lib/crd/clustersummaries.go +++ b/lib/crd/clustersummaries.go @@ -2551,6 +2551,13 @@ spec: greater than ChartVersion. Populated by a periodic background check, independent of the reconcile loop. Detection only: Sveltos never mutates ChartVersion based on this. type: string + needsRedeploy: + description: |- + NeedsRedeploy is set to true when drift-detection reports that a resource deployed by + this chart changed out of band. Only meaningful when SyncMode is ContinuousWithDriftDetection. + When true, this chart is upgraded on the next reconciliation regardless of whether its + desired values/version changed; cleared once the upgrade succeeds. + type: boolean patchesHash: description: PatchesHash represents of a unique value for the patches section diff --git a/manifest/manifest.yaml b/manifest/manifest.yaml index fdc5c81a..a602d56f 100644 --- a/manifest/manifest.yaml +++ b/manifest/manifest.yaml @@ -8929,6 +8929,13 @@ spec: greater than ChartVersion. Populated by a periodic background check, independent of the reconcile loop. Detection only: Sveltos never mutates ChartVersion based on this. type: string + needsRedeploy: + description: |- + NeedsRedeploy is set to true when drift-detection reports that a resource deployed by + this chart changed out of band. Only meaningful when SyncMode is ContinuousWithDriftDetection. + When true, this chart is upgraded on the next reconciliation regardless of whether its + desired values/version changed; cleared once the upgrade succeeds. + type: boolean patchesHash: description: PatchesHash represents of a unique value for the patches section diff --git a/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.go b/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.go index f68fbab6..4ec7734c 100644 --- a/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.go +++ b/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.go @@ -60,7 +60,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/drift-detection-manager@sha256:651ef898a5116d07f74fec85f533927f74af56f36422efed926d69ab481ff36d + image: docker.io/projectsveltos/drift-detection-manager@sha256:b69e6497fdb4bf2309b34d39412d10cd9eba158abe0cfff53a786509ebc38ddc livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.yaml b/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.yaml index 7a932eb7..238c18bf 100644 --- a/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.yaml +++ b/pkg/drift-detection/drift-detection-manager-in-mgmt-cluster.yaml @@ -42,7 +42,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/drift-detection-manager@sha256:651ef898a5116d07f74fec85f533927f74af56f36422efed926d69ab481ff36d + image: docker.io/projectsveltos/drift-detection-manager@sha256:b69e6497fdb4bf2309b34d39412d10cd9eba158abe0cfff53a786509ebc38ddc livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/drift-detection/drift-detection-manager.go b/pkg/drift-detection/drift-detection-manager.go index 41b78d20..e3f0a62b 100644 --- a/pkg/drift-detection/drift-detection-manager.go +++ b/pkg/drift-detection/drift-detection-manager.go @@ -162,7 +162,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/drift-detection-manager@sha256:651ef898a5116d07f74fec85f533927f74af56f36422efed926d69ab481ff36d + image: docker.io/projectsveltos/drift-detection-manager@sha256:b69e6497fdb4bf2309b34d39412d10cd9eba158abe0cfff53a786509ebc38ddc livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/drift-detection/drift-detection-manager.yaml b/pkg/drift-detection/drift-detection-manager.yaml index 27070b94..8fe16854 100644 --- a/pkg/drift-detection/drift-detection-manager.yaml +++ b/pkg/drift-detection/drift-detection-manager.yaml @@ -144,7 +144,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/drift-detection-manager@sha256:651ef898a5116d07f74fec85f533927f74af56f36422efed926d69ab481ff36d + image: docker.io/projectsveltos/drift-detection-manager@sha256:b69e6497fdb4bf2309b34d39412d10cd9eba158abe0cfff53a786509ebc38ddc livenessProbe: failureThreshold: 3 httpGet: