Skip to content

Commit a182c11

Browse files
mgianlucgianlucam76
authored andcommitted
Instantiate ValuesFrom before evaluating hash
Ensures valuesFrom resources are fully instantiated/templated before calculating the hash used for change detection. When a Helm chart utilizes valuesFrom (referencing a ConfigMap or Secret) and those resources contain Sveltos templates, Sveltos was previously calculating the hash based on the raw resource content rather than the instantiated content. **Example Scenario**: A ConfigMap is used in valuesFrom with template annotations: ```yaml apiVersion: v1 kind: ConfigMap metadata: name: cluster-values annotations: projectsveltos.io/template: ok data: values.yaml: |- role: "{{ index .Cluster.metadata.annotations.role }}" ``` If the Cluster annotation `role` changed, the ConfigMap content itself remained the same (the template string didn't change), causing Sveltos to erroneously conclude that the Helm values were unchanged. Consequently, the Helm release was not updated to reflect the new metadata. This was visible in the ClusterSummary Status.HelmReleaseSummaries valuesHash not changing: ```yaml status: dependencies: no dependencies featureSummaries: - featureID: Helm hash: kXmMfsd2tNKNIFA6xxnL+aq7XvIgSa4p36IuMpIRGcs= lastAppliedTime: "2026-01-21T07:33:42Z" status: Provisioned helmReleaseSummaries: - releaseName: vso releaseNamespace: hashicorp status: Managing valuesHash: jyi3q9pXIPMP4u7VlrPlq2ByKttMEPsuVW8tZQbl0KI= ``` This PR modifies the evaluation logic to: 1. Identify all resources referenced in the valuesFrom section. 2. Instantiate the content of those resources (resolving any templates) if necessary. 3. Evaluate the hash based on the final, instantiated values. This ensures that any change in the underlying data (like Cluster metadata or referenced Secrets) changing the helm values, correctly triggers a Helm reconciliation.
1 parent 1ea10dc commit a182c11

29 files changed

+941
-536
lines changed

api/v1beta1/spec.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,13 @@ type TemplateResourceRef struct {
579579
// +kubebuilder:default:=false
580580
// +optional
581581
Optional bool `json:"optional,omitempty"`
582+
583+
// IgnoreStatusChanges indicates whether changes to the status of the referenced
584+
// resource should be ignored. If set to true, only changes to the spec or
585+
// metadata (generation change) will trigger a reconciliation.
586+
// +kubebuilder:default:=false
587+
// +optional
588+
IgnoreStatusChanges bool `json:"ignoreStatusChanges,omitempty"`
582589
}
583590

584591
type PolicyRef struct {

config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,13 @@ spec:
10691069
Identifier is how the resource will be referred to in the
10701070
template
10711071
type: string
1072+
ignoreStatusChanges:
1073+
default: false
1074+
description: |-
1075+
IgnoreStatusChanges indicates whether changes to the status of the referenced
1076+
resource should be ignored. If set to true, only changes to the spec or
1077+
metadata (generation change) will trigger a reconciliation.
1078+
type: boolean
10721079
optional:
10731080
default: false
10741081
description: |-

config/crd/bases/config.projectsveltos.io_clusterpromotions.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,13 @@ spec:
962962
Identifier is how the resource will be referred to in the
963963
template
964964
type: string
965+
ignoreStatusChanges:
966+
default: false
967+
description: |-
968+
IgnoreStatusChanges indicates whether changes to the status of the referenced
969+
resource should be ignored. If set to true, only changes to the spec or
970+
metadata (generation change) will trigger a reconciliation.
971+
type: boolean
965972
optional:
966973
default: false
967974
description: |-

config/crd/bases/config.projectsveltos.io_clustersummaries.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,13 @@ spec:
11071107
Identifier is how the resource will be referred to in the
11081108
template
11091109
type: string
1110+
ignoreStatusChanges:
1111+
default: false
1112+
description: |-
1113+
IgnoreStatusChanges indicates whether changes to the status of the referenced
1114+
resource should be ignored. If set to true, only changes to the spec or
1115+
metadata (generation change) will trigger a reconciliation.
1116+
type: boolean
11101117
optional:
11111118
default: false
11121119
description: |-

config/crd/bases/config.projectsveltos.io_profiles.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,13 @@ spec:
10691069
Identifier is how the resource will be referred to in the
10701070
template
10711071
type: string
1072+
ignoreStatusChanges:
1073+
default: false
1074+
description: |-
1075+
IgnoreStatusChanges indicates whether changes to the status of the referenced
1076+
resource should be ignored. If set to true, only changes to the spec or
1077+
metadata (generation change) will trigger a reconciliation.
1078+
type: boolean
10721079
optional:
10731080
default: false
10741081
description: |-

controllers/clustersummary_controller.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,10 @@ func (r *ClusterSummaryReconciler) proceedDeployingClusterSummary(ctx context.Co
517517
// SetupWithManager sets up the controller with the Manager.
518518
func (r *ClusterSummaryReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
519519
c, err := ctrl.NewControllerManagedBy(mgr).
520-
For(&configv1beta1.ClusterSummary{}, builder.WithPredicates(ClusterSummaryPredicate{Logger: r.Logger.WithName("clusterSummaryPredicate")})).
520+
For(&configv1beta1.ClusterSummary{},
521+
builder.WithPredicates(
522+
ClusterSummaryPredicate{Logger: r.Logger.WithName("clusterSummaryPredicate")}),
523+
).
521524
WithOptions(controller.Options{
522525
MaxConcurrentReconciles: r.ConcurrentReconciles,
523526
}).

controllers/clustersummary_predicates.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ import (
3434
logs "github.com/projectsveltos/libsveltos/lib/logsettings"
3535
)
3636

37+
const (
38+
// retriggerAnnotation is the annotation used to manually or automatically
39+
// force a reconciliation of a ClusterSummary.
40+
retriggerAnnotation = "projectsveltos.io/retrigger"
41+
)
42+
3743
// ConfigMapPredicates predicates for ConfigMaps. ClusterSummaryReconciler watches ConfigMap events
3844
// and react to those by reconciling itself based on following predicates
3945
func ConfigMapPredicates(logger logr.Logger) predicate.Funcs {
@@ -350,6 +356,22 @@ func (p ClusterSummaryPredicate) Update(e event.UpdateEvent) bool {
350356
return true
351357
}
352358

359+
newAnnot := newClusterSummary.GetAnnotations()
360+
oldAnnot := oldClusterSummary.GetAnnotations()
361+
362+
var oldVal, newVal string
363+
if oldAnnot != nil {
364+
oldVal = oldAnnot[retriggerAnnotation]
365+
}
366+
if newAnnot != nil {
367+
newVal = newAnnot[retriggerAnnotation]
368+
}
369+
370+
if oldVal != newVal {
371+
log.V(logs.LogVerbose).Info("Retrigger annotation changed. Reconciling ClusterSummary.")
372+
return true
373+
}
374+
353375
if !reflect.DeepEqual(oldClusterSummary.Spec, newClusterSummary.Spec) {
354376
log.V(logs.LogVerbose).Info(
355377
"ClusterSummary Spec changed. Will attempt to reconcile ClusterSummary.",

controllers/clustersummary_watchers.go

Lines changed: 71 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"sync"
23+
"time"
2324

2425
"github.com/go-logr/logr"
2526
corev1 "k8s.io/api/core/v1"
@@ -33,7 +34,6 @@ import (
3334
"k8s.io/client-go/restmapper"
3435
"k8s.io/client-go/tools/cache"
3536
"k8s.io/client-go/util/retry"
36-
"sigs.k8s.io/cluster-api/util"
3737
"sigs.k8s.io/controller-runtime/pkg/client"
3838

3939
configv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
@@ -75,8 +75,11 @@ type manager struct {
7575
// Those resources' values will be used to instantiate templates contained in referenced
7676
requestorForTemplateResourceRefs map[schema.GroupVersionKind]*libsveltosset.Set
7777

78-
// key: resource being watched, value: list of ClusterSummary interested in this resource
78+
// key: resource being watched, value: list of ClusterSummary interested in ALL changes (including status)
7979
templateResourceRefsWatched map[corev1.ObjectReference]*libsveltosset.Set
80+
81+
// key: resource being watched, value: list of ClusterSummary interested ONLY in spec/metadata changes
82+
templateResourceRefsWatchedIgnoreStatus map[corev1.ObjectReference]*libsveltosset.Set
8083
}
8184

8285
// initializeManager initializes a manager implementing the Watchers
@@ -95,6 +98,7 @@ func initializeManager(l logr.Logger, config *rest.Config, c client.Client) {
9598
managerInstance.mgmtResourcesWatchedKustomizeRef = make(map[corev1.ObjectReference]*libsveltosset.Set)
9699
managerInstance.mgmtResourcesWatchedPolicyRef = make(map[corev1.ObjectReference]*libsveltosset.Set)
97100
managerInstance.templateResourceRefsWatched = make(map[corev1.ObjectReference]*libsveltosset.Set)
101+
managerInstance.templateResourceRefsWatchedIgnoreStatus = make(map[corev1.ObjectReference]*libsveltosset.Set)
98102
}
99103
}
100104
}
@@ -140,7 +144,8 @@ func (m *manager) startWatcherForMgmtResource(ctx context.Context, gvk schema.Gr
140144

141145
// startWatcherForTemplateResourceRef starts a watcher if one does not exist already
142146
// ClusterSummary is listed as one of the consumer for this watcher.
143-
// ClusterSummary will only receive updates when the specific resource itself changes, not when other resources of the same type change.
147+
// ClusterSummary will only receive updates when the specific resource itself changes,
148+
// not when other resources of the same type change.
144149
func (m *manager) startWatcherForTemplateResourceRef(ctx context.Context, gvk schema.GroupVersionKind,
145150
ref *configv1beta1.TemplateResourceRef, clusterSummary *configv1beta1.ClusterSummary) error {
146151

@@ -184,12 +189,29 @@ func (m *manager) startWatcherForTemplateResourceRef(ctx context.Context, gvk sc
184189

185190
m.requestorForTemplateResourceRefs[gvk].Insert(consumer)
186191

187-
if _, ok := m.templateResourceRefsWatched[resource]; !ok {
188-
s := &libsveltosset.Set{}
189-
m.templateResourceRefsWatched[resource] = s
190-
}
192+
if ref.IgnoreStatusChanges {
193+
// Ensure it's NOT in the "All Changes" bucket
194+
if m.templateResourceRefsWatched[resource] != nil {
195+
m.templateResourceRefsWatched[resource].Erase(consumer)
196+
}
197+
198+
// Add to "Ignore Status" bucket
199+
if _, ok := m.templateResourceRefsWatchedIgnoreStatus[resource]; !ok {
200+
m.templateResourceRefsWatchedIgnoreStatus[resource] = &libsveltosset.Set{}
201+
}
202+
m.templateResourceRefsWatchedIgnoreStatus[resource].Insert(consumer)
203+
} else {
204+
// Ensure it's NOT in the "Ignore Status" bucket
205+
if m.templateResourceRefsWatchedIgnoreStatus[resource] != nil {
206+
m.templateResourceRefsWatchedIgnoreStatus[resource].Erase(consumer)
207+
}
191208

192-
m.templateResourceRefsWatched[resource].Insert(consumer)
209+
// Add to "All Changes" bucket
210+
if _, ok := m.templateResourceRefsWatched[resource]; !ok {
211+
m.templateResourceRefsWatched[resource] = &libsveltosset.Set{}
212+
}
213+
m.templateResourceRefsWatched[resource].Insert(consumer)
214+
}
193215

194216
return nil
195217
}
@@ -314,14 +336,25 @@ func (m *manager) stopStaleWatchForTemplateResourceRef(ctx context.Context,
314336
}
315337
}
316338

317-
for resource := range m.templateResourceRefsWatched {
339+
// Clean up the "All Changes" bucket
340+
for resource, set := range m.templateResourceRefsWatched {
318341
if _, ok := currentResources[resource]; !ok {
319-
m.templateResourceRefsWatched[resource].Erase(consumer)
320-
if m.templateResourceRefsWatched[resource].Len() == 0 {
342+
set.Erase(consumer)
343+
if set.Len() == 0 {
321344
delete(m.templateResourceRefsWatched, resource)
322345
}
323346
}
324347
}
348+
349+
// Clean up the "Ignore Status" bucket
350+
for resource, set := range m.templateResourceRefsWatchedIgnoreStatus {
351+
if _, ok := currentResources[resource]; !ok {
352+
set.Erase(consumer)
353+
if set.Len() == 0 {
354+
delete(m.templateResourceRefsWatchedIgnoreStatus, resource)
355+
}
356+
}
357+
}
325358
}
326359

327360
func (m *manager) startWatcher(ctx context.Context, gvk *schema.GroupVersionKind) error {
@@ -389,13 +422,13 @@ func (m *manager) runInformer(stopCh <-chan struct{}, s cache.SharedIndexInforme
389422

390423
handlers := cache.ResourceEventHandlerFuncs{
391424
AddFunc: func(obj interface{}) {
392-
m.react(obj.(client.Object), logger)
425+
m.react(nil, obj.(client.Object), logger)
393426
},
394427
DeleteFunc: func(obj interface{}) {
395-
m.react(obj.(client.Object), logger)
428+
m.react(nil, obj.(client.Object), logger)
396429
},
397430
UpdateFunc: func(oldObj, newObj interface{}) {
398-
m.react(newObj.(client.Object), logger)
431+
m.react(oldObj.(client.Object), newObj.(client.Object), logger)
399432
},
400433
}
401434
_, err := s.AddEventHandler(handlers)
@@ -406,15 +439,15 @@ func (m *manager) runInformer(stopCh <-chan struct{}, s cache.SharedIndexInforme
406439
}
407440

408441
// react gets called when an instance of passed in gvk has been modified.
409-
func (m *manager) react(obj client.Object, logger logr.Logger) {
442+
func (m *manager) react(oldObj, newObj client.Object, logger logr.Logger) {
410443
m.watchMu.RLock()
411444
defer m.watchMu.RUnlock()
412445

413446
ref := corev1.ObjectReference{
414-
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
415-
APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
416-
Namespace: obj.GetNamespace(),
417-
Name: obj.GetName(),
447+
Kind: newObj.GetObjectKind().GroupVersionKind().Kind,
448+
APIVersion: newObj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
449+
Namespace: newObj.GetNamespace(),
450+
Name: newObj.GetName(),
418451
}
419452

420453
logger = logger.WithValues("resource", fmt.Sprintf("%s/%s", ref.Namespace, ref.Name))
@@ -432,10 +465,23 @@ func (m *manager) react(obj client.Object, logger logr.Logger) {
432465
}
433466

434467
// finds all ClusterSummary objects that want to be informed about updates to this specific resource.
435-
// It takes into account registrations made through TemplateResourceRefs
468+
// It takes into account registrations made through TemplateResourceRefs.
469+
// Consumers interested in ALL changes
436470
if v, ok := m.templateResourceRefsWatched[ref]; ok {
437471
m.notify(v, logger)
438472
}
473+
474+
// finds all ClusterSummary objects that want to be informed about updates to this specific resource.
475+
// It takes into account registrations made through TemplateResourceRefs.
476+
// Consumers ignoring Status changes
477+
if v, ok := m.templateResourceRefsWatchedIgnoreStatus[ref]; ok {
478+
// - Or the Generation has increased (Spec/Metadata change)
479+
if oldObj == nil || oldObj.GetGeneration() != newObj.GetGeneration() {
480+
m.notify(v, logger)
481+
} else {
482+
logger.V(logs.LogDebug).Info("skipping notification: only status changed")
483+
}
484+
}
439485
}
440486

441487
func (m *manager) notify(consumers *libsveltosset.Set, logger logr.Logger) {
@@ -460,12 +506,12 @@ func (m *manager) notifyConsumer(consumer *corev1.ObjectReference) {
460506

461507
m.log.V(logs.LogInfo).Info(fmt.Sprintf("requeuing ClusterSummary %s/%s",
462508
currentRequestor.Namespace, currentRequestor.Name))
463-
// reset hash
464-
for i := range currentRequestor.Status.FeatureSummaries {
465-
const length = 20
466-
currentRequestor.Status.FeatureSummaries[i].Hash = []byte(util.RandomString(length))
509+
// reset annotation. This will cause a new reconciliation to happen
510+
if currentRequestor.Annotations == nil {
511+
currentRequestor.Annotations = map[string]string{}
467512
}
468-
return m.Status().Update(context.TODO(), currentRequestor)
513+
currentRequestor.Annotations[retriggerAnnotation] = time.Now().UTC().Format(time.RFC3339Nano)
514+
return m.Update(context.TODO(), currentRequestor)
469515
})
470516
if err != nil {
471517
// TODO: if this fails, there is no way to reconcile the ClusterSummary

controllers/export_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ var (
118118
KustomizationHash = kustomizationHash
119119
GetKustomizeReferenceResourceHash = getKustomizeReferenceResourceHash
120120
ExtractTarGz = extractTarGz
121+
GetHelmChartHash = getHelmChartHash
121122
//nolint: gocritic // getDataSectionHash is generic and needs instantiation
122123
GetStringDataSectionHash = func(aMap map[string]string) string { return getDataSectionHash(aMap) }
123124
//nolint: gocritic // getDataSectionHash is generic and needs instantiation
@@ -137,11 +138,11 @@ var (
137138
CreateReportForUnmanagedHelmRelease = createReportForUnmanagedHelmRelease
138139
UpdateClusterReportWithHelmReports = updateClusterReportWithHelmReports
139140
HandleCharts = handleCharts
140-
GetHelmReferenceResourceHash = getHelmReferenceResourceHash
141141
GetHelmChartValuesHash = getHelmChartValuesHash
142142
GetCredentialsAndCAFiles = getCredentialsAndCAFiles
143143
GetInstantiatedChart = getInstantiatedChart
144144
GetHelmChartValuesFrom = getHelmChartValuesFrom
145+
GetHelmChartInstantiatedValues = getHelmChartInstantiatedValues
145146

146147
InstantiateTemplateValues = instantiateTemplateValues
147148
FetchClusterObjects = fetchClusterObjects
@@ -202,3 +203,7 @@ var (
202203
var (
203204
SkipUpgrading = skipUpgrading
204205
)
206+
207+
var (
208+
GetSortedKeys = getSortedKeys
209+
)

0 commit comments

Comments
 (0)