Skip to content

Commit 2a60640

Browse files
committed
(bug) Rebuild chart versions
When addon-controller starts, it builds an in-memory map keeping track of which helm release and version is already deployed on a managed cluster. Before this PR, addon-controller used to walk all clusterSummary instances. The problem with that approach is that ClusterSummary can be expressed as a template (release namespace, release name and chart version). Instantiated values are needed. Instead of instantiating the template, this PR switches to use ClusterConfiguration data.
1 parent f54a8de commit 2a60640

File tree

2 files changed

+124
-26
lines changed

2 files changed

+124
-26
lines changed

controllers/chartmanager/chartmanager.go

Lines changed: 67 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -627,15 +627,15 @@ func (m *instance) rebuildChartVersions(ctx context.Context, c client.Client) er
627627
m.chartMux.Lock()
628628
defer m.chartMux.Unlock()
629629

630-
clusterSummaryList := &configv1beta1.ClusterSummaryList{}
631-
err := c.List(ctx, clusterSummaryList)
630+
clusterConfigurations := &configv1beta1.ClusterConfigurationList{}
631+
err := c.List(ctx, clusterConfigurations)
632632
if err != nil {
633633
return err
634634
}
635635

636-
for i := range clusterSummaryList.Items {
637-
cs := &clusterSummaryList.Items[i]
638-
m.addHelmVersions(cs)
636+
for i := range clusterConfigurations.Items {
637+
cc := &clusterConfigurations.Items[i]
638+
m.addHelmVersions(cc)
639639
}
640640

641641
return nil
@@ -676,32 +676,73 @@ func (m *instance) addNonManagers(clusterSummary *configv1beta1.ClusterSummary)
676676
}
677677
}
678678

679-
// addHelmVersions walks clusterSummary's status and register helm versions for each chart managed
680-
func (m *instance) addHelmVersions(clusterSummary *configv1beta1.ClusterSummary) {
681-
clusterKey := m.getClusterKey(clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName,
682-
clusterSummary.Spec.ClusterType)
679+
// addHelmVersions walks clusterConfigurations's status and register helm versions for each chart managed
680+
func (m *instance) addHelmVersions(clusterConfiguration *configv1beta1.ClusterConfiguration) {
681+
lbls := clusterConfiguration.Labels
682+
if lbls == nil {
683+
m.logger.V(logs.LogInfo).Info(fmt.Sprintf("ClusterConfiguration %s/%s has no labels",
684+
clusterConfiguration.Namespace, clusterConfiguration.Name))
685+
return
686+
}
683687

684-
for i := range clusterSummary.Status.HelmReleaseSummaries {
685-
summary := &clusterSummary.Status.HelmReleaseSummaries[i]
686-
if summary.Status == configv1beta1.HelmChartStatusManaging {
687-
chartVersion := m.getVersion(clusterSummary, summary.ReleaseNamespace, summary.ReleaseName)
688-
releaseKey := m.GetReleaseKey(summary.ReleaseNamespace, summary.ReleaseName)
689-
m.setChartVersion(clusterKey, releaseKey, chartVersion)
690-
}
688+
clusterName, ok := clusterConfiguration.Labels[configv1beta1.ClusterNameLabel]
689+
if !ok {
690+
m.logger.V(logs.LogInfo).Info(fmt.Sprintf("ClusterConfiguration %s/%s has no %s label",
691+
clusterConfiguration.Namespace, clusterConfiguration.Name, configv1beta1.ClusterNameLabel))
692+
return
691693
}
692-
}
693694

694-
func (m *instance) getVersion(clusterSummary *configv1beta1.ClusterSummary,
695-
releaseNamespace, releaseName string) string {
695+
var clusterKey string
696696

697-
for i := range clusterSummary.Spec.ClusterProfileSpec.HelmCharts {
698-
hc := &clusterSummary.Spec.ClusterProfileSpec.HelmCharts[i]
699-
if hc.ReleaseNamespace == releaseNamespace &&
700-
hc.ReleaseName == releaseName {
697+
rawValueClusterType, ok := clusterConfiguration.Labels[configv1beta1.ClusterTypeLabel]
698+
if !ok {
699+
m.logger.V(logs.LogInfo).Info(fmt.Sprintf("ClusterConfiguration %s/%s has no %s label",
700+
clusterConfiguration.Namespace, clusterConfiguration.Name, configv1beta1.ClusterTypeLabel))
701+
return
702+
}
701703

702-
return hc.ChartVersion
703-
}
704+
var clusterType libsveltosv1beta1.ClusterType
705+
706+
// Normalize the input to match your defined constants
707+
switch {
708+
case strings.EqualFold(rawValueClusterType, string(libsveltosv1beta1.ClusterTypeCapi)):
709+
clusterType = libsveltosv1beta1.ClusterTypeCapi
710+
case strings.EqualFold(rawValueClusterType, string(libsveltosv1beta1.ClusterTypeSveltos)):
711+
clusterType = libsveltosv1beta1.ClusterTypeSveltos
712+
default:
713+
m.logger.V(logs.LogInfo).Info(fmt.Sprintf("ClusterConfiguration %s/%s label %s has unknown value %s",
714+
clusterConfiguration.Namespace, clusterConfiguration.Name,
715+
configv1beta1.ClusterTypeLabel, rawValueClusterType))
716+
return
717+
}
718+
719+
clusterKey = m.getClusterKey(clusterConfiguration.Namespace, clusterName, clusterType)
720+
721+
for i := range clusterConfiguration.Status.ClusterProfileResources {
722+
m.walkClusterConfigurationFeature(clusterKey,
723+
clusterConfiguration.Status.ClusterProfileResources[i].Features)
704724
}
705725

706-
return ""
726+
for i := range clusterConfiguration.Status.ProfileResources {
727+
m.walkClusterConfigurationFeature(clusterKey,
728+
clusterConfiguration.Status.ProfileResources[i].Features)
729+
}
730+
}
731+
732+
func (m *instance) walkClusterConfigurationFeature(clusterKey string, features []configv1beta1.Feature) {
733+
for i := range features {
734+
m.walkClusterConfigurationCharts(clusterKey, features[i])
735+
}
736+
}
737+
738+
func (m *instance) walkClusterConfigurationCharts(clusterKey string, feature configv1beta1.Feature) {
739+
for i := range feature.Charts {
740+
chartVersion := feature.Charts[i].ChartVersion
741+
releaseKey := m.GetReleaseKey(feature.Charts[i].Namespace,
742+
feature.Charts[i].ReleaseName)
743+
744+
m.logger.V(logs.LogDebug).Info(fmt.Sprintf("cluster %s %s %s %s",
745+
clusterKey, feature.Charts[i].Namespace, feature.Charts[i].ReleaseName, chartVersion))
746+
m.setChartVersion(clusterKey, releaseKey, chartVersion)
747+
}
707748
}

controllers/handlers_helm_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,63 @@ var _ = Describe("HandlersHelm", func() {
453453
Equal(chartDeployed[0].ChartVersion))
454454
})
455455

456+
It("updateChartsInClusterConfiguration stores the live release ChartVersion, not the spec version", func() {
457+
// chartDeployed is built from currentRelease.ChartVersion (the actually-deployed version),
458+
// not from the spec. This test verifies that what lands in ClusterConfiguration reflects
459+
// what the cluster actually has, including multi-chart profiles.
460+
chartsDeployed := []configv1beta1.Chart{
461+
{
462+
RepoURL: "https://prometheus-community.github.io/helm-charts",
463+
ReleaseName: "prometheus",
464+
Namespace: "prometheus",
465+
ChartVersion: "26.0.0",
466+
AppVersion: "v3.0.0",
467+
},
468+
{
469+
RepoURL: "https://grafana-community.github.io/helm-charts",
470+
ReleaseName: "grafana",
471+
Namespace: "grafana",
472+
ChartVersion: "11.3.6",
473+
AppVersion: "12.4.2",
474+
},
475+
}
476+
477+
clusterConfiguration := &configv1beta1.ClusterConfiguration{
478+
ObjectMeta: metav1.ObjectMeta{
479+
Name: controllers.GetClusterConfigurationName(clusterSummary.Spec.ClusterName, libsveltosv1beta1.ClusterTypeCapi),
480+
Namespace: clusterSummary.Spec.ClusterNamespace,
481+
},
482+
Status: configv1beta1.ClusterConfigurationStatus{
483+
ClusterProfileResources: []configv1beta1.ClusterProfileResource{
484+
{ClusterProfileName: clusterProfile.Name},
485+
},
486+
},
487+
}
488+
489+
initObjects := []client.Object{clusterConfiguration}
490+
c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(initObjects...).WithObjects(initObjects...).Build()
491+
492+
Expect(controllers.UpdateChartsInClusterConfiguration(context.TODO(), c, clusterSummary,
493+
chartsDeployed, textlogger.NewLogger(textlogger.NewConfig()))).To(Succeed())
494+
495+
current := &configv1beta1.ClusterConfiguration{}
496+
Expect(c.Get(context.TODO(),
497+
types.NamespacedName{Namespace: clusterConfiguration.Namespace, Name: clusterConfiguration.Name},
498+
current)).To(Succeed())
499+
500+
Expect(current.Status.ClusterProfileResources).To(HaveLen(1))
501+
charts := current.Status.ClusterProfileResources[0].Features[0].Charts
502+
Expect(charts).To(HaveLen(2))
503+
504+
for i, expected := range chartsDeployed {
505+
Expect(charts[i].ChartVersion).To(Equal(expected.ChartVersion),
506+
"chart %s: stored version must match the live release version", expected.ReleaseName)
507+
Expect(charts[i].AppVersion).To(Equal(expected.AppVersion))
508+
Expect(charts[i].ReleaseName).To(Equal(expected.ReleaseName))
509+
Expect(charts[i].RepoURL).To(Equal(expected.RepoURL))
510+
}
511+
})
512+
456513
It("createReportForUnmanagedHelmRelease ", func() {
457514
helmChart := &configv1beta1.HelmChart{
458515
ReleaseName: randomString(), ReleaseNamespace: randomString(),

0 commit comments

Comments
 (0)