From f60edd07190508e2fab7a7b13d9c68ad7e13d718 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Thu, 15 Oct 2026 09:25:35 +0200 Subject: [PATCH] chore: Convert the outdated Helm chart and semver tests to Ginkgo The only tests of the controllers package still using plain testing.T now use Ginkgo and Gomega like the rest of the package. They move to package controllers_test, and what they need from unexported code is exposed through export_test.go. The test of a chart which could not be checked uses an httptest server which is closed, instead of relying on nothing listening on port 1. Co-Authored-By: Claude Sonnet 5.5 --- controllers/export_test.go | 148 +++++ controllers/helmchart_outdated_check_test.go | 533 +++++++------------ controllers/helmchart_semver_test.go | 131 ++--- 3 files changed, 379 insertions(+), 433 deletions(-) diff --git a/controllers/export_test.go b/controllers/export_test.go index 0b79821e..b61d77e0 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -17,12 +17,16 @@ limitations under the License. package controllers import ( + "context" "strings" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/rest" "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/metrics" configv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" ) @@ -398,3 +402,147 @@ func RenderHealthMonitoringLua(ownerKind, ownerName string, releases []helmRelea ownerKind: ownerKind, ownerName: ownerName, helmReleases: releases, }) } + +// CollectedHelmChartKey identifies a chart the outdated Helm chart check fetches the versions of +type CollectedHelmChartKey struct { + ChartName string + RepositoryName string + RepositoryURL string +} + +// CollectHelmChartRefs returns, for each chart the ClusterSummaries reference, the names of the +// ClusterSummaries referencing it +func CollectHelmChartRefs(clusterSummaries ...*configv1beta1.ClusterSummary) map[CollectedHelmChartKey][]string { + refs := map[helmChartKey][]helmChartRef{} + for i := range clusterSummaries { + collectHelmChartRefs(clusterSummaries[i], refs, logr.Discard()) + } + + result := map[CollectedHelmChartKey][]string{} + for key, keyRefs := range refs { + collectedKey := CollectedHelmChartKey{ + ChartName: key.chartName, RepositoryName: key.repositoryName, RepositoryURL: key.repositoryURL, + } + for i := range keyRefs { + result[collectedKey] = append(result[collectedKey], keyRefs[i].clusterSummaryName) + } + } + + return result +} + +// HelmChartVersionDiff returns the latest version and the latest patch version of the same minor, as +// strings, empty when there is none +func HelmChartVersionDiff(current string, available []string) (latest, latestPatch string, err error) { + latestVersion, latestPatchVersion, err := helmChartVersionDiff(current, available) + if err != nil { + return "", "", err + } + + if latestVersion != nil { + latest = latestVersion.String() + } + if latestPatchVersion != nil { + latestPatch = latestPatchVersion.String() + } + + return latest, latestPatch, nil +} + +// SetResolvedHelmChartIdentity updates the HelmChartSummary of a release after a deployment of +// deployedChartVersion +func SetResolvedHelmChartIdentity(clusterSummary *configv1beta1.ClusterSummary, helmChartSummary *configv1beta1.HelmChartSummary, + requestedChart *configv1beta1.HelmChart, deployedChartVersion string) { + + currentRelease := &releaseInfo{ChartVersion: deployedChartVersion} + setResolvedHelmChartIdentity(context.TODO(), nil, clusterSummary, helmChartSummary, requestedChart, currentRelease, + logr.Discard()) +} + +// OutdatedHelmChart identifies the outdated_helm_chart series of one Helm release +type OutdatedHelmChart struct { + ProfileKind string + ProfileNamespace string + ProfileName string + ClusterType string + ClusterNamespace string + ClusterName string + ChartName string + ReleaseNamespace string + ReleaseName string +} + +func (o *OutdatedHelmChart) metricKey() outdatedHelmChartMetricKey { + return outdatedHelmChartMetricKey{ + profileKind: o.ProfileKind, profileNamespace: o.ProfileNamespace, profileName: o.ProfileName, + clusterType: o.ClusterType, clusterNamespace: o.ClusterNamespace, clusterName: o.ClusterName, + chartName: o.ChartName, releaseNamespace: o.ReleaseNamespace, releaseName: o.ReleaseName, + } +} + +func (o *OutdatedHelmChart) clear() { + clearOutdatedHelmChart(o.ProfileKind, o.ProfileNamespace, o.ProfileName, o.ClusterType, o.ClusterNamespace, + o.ClusterName, o.ChartName, o.ReleaseNamespace, o.ReleaseName) +} + +// SetOutdatedByPreviousPass leaves the outdated Helm chart check as a previous pass does when it found +// the chart outdated. It returns the function restoring the check. +func SetOutdatedByPreviousPass(chart *OutdatedHelmChart) (restore func()) { + saved := previouslyOutdatedHelmCharts + + previouslyOutdatedHelmCharts = map[outdatedHelmChartMetricKey]struct{}{chart.metricKey(): {}} + trackOutdatedHelmChart(chart.ProfileKind, chart.ProfileNamespace, chart.ProfileName, chart.ClusterType, + chart.ClusterNamespace, chart.ClusterName, chart.ChartName, chart.ReleaseNamespace, chart.ReleaseName, + logr.Discard()) + + return func() { + previouslyOutdatedHelmCharts = saved + chart.clear() + } +} + +// CheckOutdatedHelmCharts runs one pass of the outdated Helm chart check +func CheckOutdatedHelmCharts(ctx context.Context, c client.Client) { + checkOutdatedHelmCharts(ctx, c, logr.Discard()) +} + +// IsOutdatedHelmChartTracked returns true if the outdated Helm chart check still tracks the chart as outdated +func IsOutdatedHelmChartTracked(chart *OutdatedHelmChart) bool { + _, tracked := previouslyOutdatedHelmCharts[chart.metricKey()] + return tracked +} + +// IsOutdatedHelmChartExposed returns true if the outdated_helm_chart series of the chart is still exposed +func IsOutdatedHelmChartExposed(chart *OutdatedHelmChart) (bool, error) { + want := map[string]string{ + metricProfileKindLabel: chart.ProfileKind, metricProfileNamespaceLabel: chart.ProfileNamespace, + metricProfileNameLabel: chart.ProfileName, metricClusterTypeLabel: chart.ClusterType, + metricClusterNamespaceLabel: chart.ClusterNamespace, metricClusterNameLabel: chart.ClusterName, + metricChartNameLabel: chart.ChartName, metricReleaseNamespaceLabel: chart.ReleaseNamespace, + metricReleaseNameLabel: chart.ReleaseName, + } + + families, err := metrics.Registry.Gather() + if err != nil { + return false, err + } + + for _, family := range families { + if family.GetName() != metricNamespace+"_outdated_helm_chart" { + continue + } + for _, metric := range family.GetMetric() { + matched := 0 + for _, label := range metric.GetLabel() { + if value, ok := want[label.GetName()]; ok && value == label.GetValue() { + matched++ + } + } + if matched == len(want) { + return true, nil + } + } + } + + return false, nil +} diff --git a/controllers/helmchart_outdated_check_test.go b/controllers/helmchart_outdated_check_test.go index 5fb66d2e..1d4528c0 100644 --- a/controllers/helmchart_outdated_check_test.go +++ b/controllers/helmchart_outdated_check_test.go @@ -14,385 +14,244 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package-internal (not controllers_test): collectHelmChartRefs is pure and dependency-free -// (no client, no envtest), and asserting on its output requires reading the unexported -// helmChartKey/helmChartRef fields it produces, so this is tested directly rather than through -// the Ginkgo/envtest suite used by the rest of this package's tests. -package controllers +package controllers_test import ( "context" "net/http" "net/http/httptest" - "testing" - "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/metrics" configv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" + "github.com/projectsveltos/addon-controller/controllers" ) const ( - testReleaseName = "r1" - testReleaseNamespace = "ns1" - testChartName = "nginx" - testChartVersion = "1.0.0" - testRepoURL = "https://example.com/charts" - testRepositoryName = "bitnami" - testBitnamiRepoURL = "https://charts.bitnami.com/bitnami" - testNamespace = "ns" - testClusterNamespace = "cluster-ns" - testClusterName = "cluster" - testClusterType = "Capi" - testProfileName = "profile" + outdatedCheckReleaseName = "r1" + outdatedCheckReleaseNamespace = "ns1" + outdatedCheckChartName = "nginx" + outdatedCheckChartVersion = "1.0.0" + outdatedCheckRepoURL = "https://example.com/charts" + outdatedCheckRepositoryName = "bitnami" + outdatedCheckBitnamiRepoURL = "https://charts.bitnami.com/bitnami" + outdatedCheckNamespace = "ns" + outdatedCheckClusterNamespace = "cluster-ns" + outdatedCheckClusterName = "cluster" + outdatedCheckClusterType = "Capi" + outdatedCheckProfileName = "profile" ) -func TestCollectHelmChartRefs_SkipsConflictEntries(t *testing.T) { - cs := &configv1beta1.ClusterSummary{ - Status: configv1beta1.ClusterSummaryStatus{ - HelmReleaseSummaries: []configv1beta1.HelmChartSummary{ - { - ReleaseName: testReleaseName, - ReleaseNamespace: testReleaseNamespace, - Status: configv1beta1.HelmChartStatusConflict, - ChartName: testChartName, - RepoURL: testRepoURL, - ChartVersion: testChartVersion, - }, - }, - }, - } - - refs := map[helmChartKey][]helmChartRef{} - collectHelmChartRefs(cs, refs, logr.Discard()) - - if len(refs) != 0 { - t.Fatalf("expected no refs for a Conflict-status entry, got %d keys", len(refs)) - } -} - -func TestCollectHelmChartRefs_SkipsUnresolvedIdentity(t *testing.T) { - tests := []struct { - name string - rs configv1beta1.HelmChartSummary - }{ - { - name: "never deployed (no ChartName/RepoURL/ChartVersion)", - rs: configv1beta1.HelmChartSummary{ - ReleaseName: testReleaseName, ReleaseNamespace: testReleaseNamespace, Status: configv1beta1.HelmChartStatusManaging, - }, - }, - { - name: "flux-sourced (ChartName/RepoURL never populated)", - rs: configv1beta1.HelmChartSummary{ - ReleaseName: testReleaseName, ReleaseNamespace: testReleaseNamespace, Status: configv1beta1.HelmChartStatusManaging, - ChartVersion: testChartVersion, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - cs := &configv1beta1.ClusterSummary{ - Status: configv1beta1.ClusterSummaryStatus{HelmReleaseSummaries: []configv1beta1.HelmChartSummary{tt.rs}}, - } - - refs := map[helmChartKey][]helmChartRef{} - collectHelmChartRefs(cs, refs, logr.Discard()) - - if len(refs) != 0 { - t.Fatalf("expected no refs, got %d keys", len(refs)) - } - }) - } -} - -func TestCollectHelmChartRefs_DedupsAcrossClusterSummaries(t *testing.T) { - makeClusterSummary := func(name string) *configv1beta1.ClusterSummary { - return &configv1beta1.ClusterSummary{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: testNamespace}, - Spec: configv1beta1.ClusterSummarySpec{ - ClusterNamespace: testClusterNamespace, ClusterName: name, ClusterType: testClusterType, - }, - Status: configv1beta1.ClusterSummaryStatus{ - HelmReleaseSummaries: []configv1beta1.HelmChartSummary{ - { - ReleaseName: testChartName, - ReleaseNamespace: "web", - Status: configv1beta1.HelmChartStatusManaging, - ChartName: testChartName, - RepositoryName: testRepositoryName, - RepoURL: testBitnamiRepoURL, - ChartVersion: testChartVersion, - }, - }, - }, - } - } - - refs := map[helmChartKey][]helmChartRef{} - collectHelmChartRefs(makeClusterSummary("cluster-a"), refs, logr.Discard()) - collectHelmChartRefs(makeClusterSummary("cluster-b"), refs, logr.Discard()) - - if len(refs) != 1 { - t.Fatalf("expected exactly one deduped chart key, got %d", len(refs)) - } - - for key, keyRefs := range refs { - if key.chartName != testChartName || key.repositoryName != testRepositoryName || key.repositoryURL != testBitnamiRepoURL { - t.Errorf("unexpected key: %+v", key) - } - if len(keyRefs) != 2 { - t.Fatalf("expected 2 refs for the deduped key, got %d", len(keyRefs)) - } - if keyRefs[0].clusterSummaryName == keyRefs[1].clusterSummaryName { - t.Errorf("expected refs from two distinct ClusterSummaries, got the same name twice: %q", - keyRefs[0].clusterSummaryName) - } - } -} - -func TestSetResolvedHelmChartIdentity_ClearsStaleOutdatedVersionInfoOnVersionChange(t *testing.T) { - newLatest := v1_1_0 - lastChecked := metav1.Now() - - clusterSummary := &configv1beta1.ClusterSummary{ +// newClusterSummaryWithHelmRelease returns a ClusterSummary of the ClusterProfile outdatedCheckProfileName, +// with a single Helm release +func newClusterSummaryWithHelmRelease(release *configv1beta1.HelmChartSummary) *configv1beta1.ClusterSummary { + return &configv1beta1.ClusterSummary{ ObjectMeta: metav1.ObjectMeta{ - Name: controllerNameClusterSummary, Namespace: testNamespace, + Name: outdatedCheckProfileName + "-" + outdatedCheckClusterName, + Namespace: outdatedCheckClusterNamespace, OwnerReferences: []metav1.OwnerReference{ - {Kind: configv1beta1.ClusterProfileKind, APIVersion: configv1beta1.GroupVersion.String(), Name: testProfileName}, + { + APIVersion: configv1beta1.GroupVersion.String(), Kind: configv1beta1.ClusterProfileKind, + Name: outdatedCheckProfileName, + }, }, }, Spec: configv1beta1.ClusterSummarySpec{ - ClusterNamespace: testClusterNamespace, ClusterName: testClusterName, ClusterType: testClusterType, - }, - } - - requestedChart := &configv1beta1.HelmChart{ - ChartName: testChartName, RepositoryName: testRepositoryName, RepositoryURL: testBitnamiRepoURL, - ReleaseName: testReleaseName, ReleaseNamespace: testReleaseNamespace, - } - - rs := &configv1beta1.HelmChartSummary{ - ReleaseName: testReleaseName, ReleaseNamespace: testReleaseNamespace, Status: configv1beta1.HelmChartStatusManaging, - ChartName: testChartName, RepositoryName: testRepositoryName, RepoURL: testBitnamiRepoURL, ChartVersion: testChartVersion, - LatestVersion: &newLatest, LastCheckedTime: &lastChecked, - } - - currentRelease := &releaseInfo{ChartVersion: newLatest} // upgrading to the version previously flagged as latest - - setResolvedHelmChartIdentity(context.TODO(), nil, clusterSummary, rs, requestedChart, currentRelease, logr.Discard()) - - if rs.ChartVersion != newLatest { - t.Errorf("expected ChartVersion to be updated to %q, got %q", newLatest, rs.ChartVersion) - } - if rs.LatestVersion != nil { - t.Errorf("expected LatestVersion to be cleared, got %v", *rs.LatestVersion) - } - if rs.LatestPatchVersion != nil { - t.Errorf("expected LatestPatchVersion to be cleared, got %v", *rs.LatestPatchVersion) - } - if rs.LastCheckedTime != nil { - t.Errorf("expected LastCheckedTime to be cleared, got %v", *rs.LastCheckedTime) - } -} - -func TestSetResolvedHelmChartIdentity_KeepsOutdatedVersionInfoWhenVersionUnchanged(t *testing.T) { - newLatest := v1_1_0 - lastChecked := metav1.Now() - - clusterSummary := &configv1beta1.ClusterSummary{ - ObjectMeta: metav1.ObjectMeta{ - Name: controllerNameClusterSummary, Namespace: testNamespace, - OwnerReferences: []metav1.OwnerReference{ - {Kind: configv1beta1.ClusterProfileKind, APIVersion: configv1beta1.GroupVersion.String(), Name: testProfileName}, - }, + ClusterNamespace: outdatedCheckClusterNamespace, ClusterName: outdatedCheckClusterName, + ClusterType: outdatedCheckClusterType, }, - Spec: configv1beta1.ClusterSummarySpec{ - ClusterNamespace: testClusterNamespace, ClusterName: testClusterName, ClusterType: testClusterType, + Status: configv1beta1.ClusterSummaryStatus{ + HelmReleaseSummaries: []configv1beta1.HelmChartSummary{*release}, }, } - - requestedChart := &configv1beta1.HelmChart{ - ChartName: testChartName, RepositoryName: testRepositoryName, RepositoryURL: testBitnamiRepoURL, - ReleaseName: testReleaseName, ReleaseNamespace: testReleaseNamespace, - } - - rs := &configv1beta1.HelmChartSummary{ - ReleaseName: testReleaseName, ReleaseNamespace: testReleaseNamespace, Status: configv1beta1.HelmChartStatusManaging, - ChartName: testChartName, RepositoryName: testRepositoryName, RepoURL: testBitnamiRepoURL, ChartVersion: testChartVersion, - LatestVersion: &newLatest, LastCheckedTime: &lastChecked, - } - - // Same deployed version as before (e.g. a values-only reconcile) -- nothing to invalidate. - currentRelease := &releaseInfo{ChartVersion: testChartVersion} - - setResolvedHelmChartIdentity(context.TODO(), nil, clusterSummary, rs, requestedChart, currentRelease, logr.Discard()) - - if rs.LatestVersion == nil || *rs.LatestVersion != newLatest { - t.Errorf("expected LatestVersion to be left untouched at %q, got %v", newLatest, rs.LatestVersion) - } - if rs.LastCheckedTime == nil { - t.Errorf("expected LastCheckedTime to be left untouched, got nil") - } } -// outdatedChartKey is the outdated_helm_chart series of the release clusterSummaryManagingChart manages. -func outdatedChartKey() outdatedHelmChartMetricKey { - return outdatedHelmChartMetricKey{ - profileKind: configv1beta1.ClusterProfileKind, profileName: testProfileName, - clusterType: testClusterType, clusterNamespace: testClusterNamespace, clusterName: testClusterName, - chartName: testChartName, releaseNamespace: testReleaseNamespace, releaseName: testReleaseName, - } -} +var _ = Describe("Outdated Helm chart check", func() { + Context("collecting the charts referenced by the ClusterSummaries", func() { + It("skips the releases the ClusterSummary does not manage", func() { + clusterSummary := newClusterSummaryWithHelmRelease(&configv1beta1.HelmChartSummary{ + ReleaseName: outdatedCheckReleaseName, + ReleaseNamespace: outdatedCheckReleaseNamespace, + Status: configv1beta1.HelmChartStatusConflict, + ChartName: outdatedCheckChartName, + RepoURL: outdatedCheckRepoURL, + ChartVersion: outdatedCheckChartVersion, + }) + + Expect(controllers.CollectHelmChartRefs(clusterSummary)).To(BeEmpty()) + }) -// clusterSummaryManagingChart returns the ClusterSummary of the ClusterProfile testProfileName that -// manages version testChartVersion of testChartName, from repoURL. -func clusterSummaryManagingChart(repoURL string) *configv1beta1.ClusterSummary { - return &configv1beta1.ClusterSummary{ - ObjectMeta: metav1.ObjectMeta{ - Name: testProfileName + "-" + testClusterName, - Namespace: testClusterNamespace, - OwnerReferences: []metav1.OwnerReference{ - {APIVersion: configv1beta1.GroupVersion.String(), Kind: configv1beta1.ClusterProfileKind, Name: testProfileName}, + DescribeTable("skips the releases whose chart identity is not resolved", + func(release *configv1beta1.HelmChartSummary) { + clusterSummary := newClusterSummaryWithHelmRelease(release) + + Expect(controllers.CollectHelmChartRefs(clusterSummary)).To(BeEmpty()) }, - }, - Spec: configv1beta1.ClusterSummarySpec{ - ClusterNamespace: testClusterNamespace, ClusterName: testClusterName, ClusterType: testClusterType, - }, - Status: configv1beta1.ClusterSummaryStatus{ - HelmReleaseSummaries: []configv1beta1.HelmChartSummary{ - { - ReleaseName: testReleaseName, - ReleaseNamespace: testReleaseNamespace, + Entry("never deployed (no ChartName, RepoURL and ChartVersion)", &configv1beta1.HelmChartSummary{ + ReleaseName: outdatedCheckReleaseName, ReleaseNamespace: outdatedCheckReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, + }), + Entry("sourced from flux (ChartName and RepoURL never populated)", &configv1beta1.HelmChartSummary{ + ReleaseName: outdatedCheckReleaseName, ReleaseNamespace: outdatedCheckReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, ChartVersion: outdatedCheckChartVersion, + }), + ) + + It("groups the ClusterSummaries referencing the same chart", func() { + newClusterSummary := func(clusterName string) *configv1beta1.ClusterSummary { + clusterSummary := newClusterSummaryWithHelmRelease(&configv1beta1.HelmChartSummary{ + ReleaseName: outdatedCheckChartName, + ReleaseNamespace: "web", Status: configv1beta1.HelmChartStatusManaging, - ChartName: testChartName, - RepoURL: repoURL, - ChartVersion: testChartVersion, - }, - }, - }, - } -} + ChartName: outdatedCheckChartName, + RepositoryName: outdatedCheckRepositoryName, + RepoURL: outdatedCheckBitnamiRepoURL, + ChartVersion: outdatedCheckChartVersion, + }) + clusterSummary.Name = clusterName + clusterSummary.Namespace = outdatedCheckNamespace + clusterSummary.Spec.ClusterName = clusterName + return clusterSummary + } -// markOutdatedByPreviousPass leaves the checker as a previous pass does when it finds key outdated, -// and restores the checker when t ends. -func markOutdatedByPreviousPass(t *testing.T, key *outdatedHelmChartMetricKey) { - t.Helper() + refs := controllers.CollectHelmChartRefs(newClusterSummary("cluster-a"), newClusterSummary("cluster-b")) - saved := previouslyOutdatedHelmCharts - t.Cleanup(func() { - previouslyOutdatedHelmCharts = saved - clearOutdatedHelmChart(key.profileKind, key.profileNamespace, key.profileName, key.clusterType, - key.clusterNamespace, key.clusterName, key.chartName, key.releaseNamespace, key.releaseName) + chartKey := controllers.CollectedHelmChartKey{ + ChartName: outdatedCheckChartName, + RepositoryName: outdatedCheckRepositoryName, + RepositoryURL: outdatedCheckBitnamiRepoURL, + } + Expect(refs).To(HaveLen(1)) + Expect(refs).To(HaveKey(chartKey)) + Expect(refs[chartKey]).To(ConsistOf("cluster-a", "cluster-b")) + }) }) - previouslyOutdatedHelmCharts = map[outdatedHelmChartMetricKey]struct{}{*key: {}} - trackOutdatedHelmChart(key.profileKind, key.profileNamespace, key.profileName, key.clusterType, - key.clusterNamespace, key.clusterName, key.chartName, key.releaseNamespace, key.releaseName, logr.Discard()) -} + Context("resolving the identity of a deployed chart", func() { + var ( + clusterSummary *configv1beta1.ClusterSummary + requestedChart *configv1beta1.HelmChart + release *configv1beta1.HelmChartSummary + latestVersion string + ) + + BeforeEach(func() { + latestVersion = v1_1_0 + lastChecked := metav1.Now() + + clusterSummary = newClusterSummaryWithHelmRelease(&configv1beta1.HelmChartSummary{}) + clusterSummary.Namespace = outdatedCheckNamespace + + requestedChart = &configv1beta1.HelmChart{ + ChartName: outdatedCheckChartName, RepositoryName: outdatedCheckRepositoryName, + RepositoryURL: outdatedCheckBitnamiRepoURL, ReleaseName: outdatedCheckReleaseName, + ReleaseNamespace: outdatedCheckReleaseNamespace, + } -// runCheckerPass runs one checkOutdatedHelmCharts pass against a fake client holding objs. -func runCheckerPass(t *testing.T, objs ...client.Object) { - t.Helper() + // The release is outdated: a newer version was found + release = &configv1beta1.HelmChartSummary{ + ReleaseName: outdatedCheckReleaseName, ReleaseNamespace: outdatedCheckReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, + ChartName: outdatedCheckChartName, RepositoryName: outdatedCheckRepositoryName, + RepoURL: outdatedCheckBitnamiRepoURL, ChartVersion: outdatedCheckChartVersion, + LatestVersion: &latestVersion, LastCheckedTime: &lastChecked, + } + }) - scheme := runtime.NewScheme() - if err := configv1beta1.AddToScheme(scheme); err != nil { - t.Fatalf("AddToScheme() = %v", err) - } - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).WithStatusSubresource(objs...).Build() + It("clears the stale outdated version information when the deployed version changes", func() { + // Upgrading to the version previously flagged as the latest one + controllers.SetResolvedHelmChartIdentity(clusterSummary, release, requestedChart, v1_1_0) - checkOutdatedHelmCharts(context.Background(), c, logr.Discard()) -} + Expect(release.ChartVersion).To(Equal(v1_1_0)) + Expect(release.LatestVersion).To(BeNil()) + Expect(release.LatestPatchVersion).To(BeNil()) + Expect(release.LastCheckedTime).To(BeNil()) + }) -// outdatedHelmChartState returns whether the checker still tracks key as outdated, and whether its -// outdated_helm_chart series is still exposed. -func outdatedHelmChartState(t *testing.T, key *outdatedHelmChartMetricKey) (tracked, exposed bool) { - t.Helper() + It("keeps the outdated version information when the deployed version is unchanged", func() { + // For instance a reconciliation changing the values only: nothing to invalidate + controllers.SetResolvedHelmChartIdentity(clusterSummary, release, requestedChart, outdatedCheckChartVersion) - _, tracked = previouslyOutdatedHelmCharts[*key] + Expect(release.LatestVersion).ToNot(BeNil()) + Expect(*release.LatestVersion).To(Equal(v1_1_0)) + Expect(release.LastCheckedTime).ToNot(BeNil()) + }) + }) - want := map[string]string{ - metricProfileKindLabel: key.profileKind, metricProfileNamespaceLabel: key.profileNamespace, - metricProfileNameLabel: key.profileName, metricClusterTypeLabel: key.clusterType, - metricClusterNamespaceLabel: key.clusterNamespace, metricClusterNameLabel: key.clusterName, - metricChartNameLabel: key.chartName, metricReleaseNamespaceLabel: key.releaseNamespace, - metricReleaseNameLabel: key.releaseName, - } + // The series of a chart found outdated by a previous pass, after a whole pass against a fake client + Context("checking the charts after one found outdated", func() { + var chart controllers.OutdatedHelmChart - families, err := metrics.Registry.Gather() - if err != nil { - t.Fatalf("Gather() = %v", err) - } - for _, family := range families { - if family.GetName() != metricNamespace+"_outdated_helm_chart" { - continue - } - for _, metric := range family.GetMetric() { - matched := 0 - for _, label := range metric.GetLabel() { - if value, ok := want[label.GetName()]; ok && value == label.GetValue() { - matched++ - } - } - if matched == len(want) { - return tracked, true + BeforeEach(func() { + chart = controllers.OutdatedHelmChart{ + ProfileKind: configv1beta1.ClusterProfileKind, ProfileName: outdatedCheckProfileName, + ClusterType: outdatedCheckClusterType, ClusterNamespace: outdatedCheckClusterNamespace, + ClusterName: outdatedCheckClusterName, ChartName: outdatedCheckChartName, + ReleaseNamespace: outdatedCheckReleaseNamespace, ReleaseName: outdatedCheckReleaseName, } + DeferCleanup(controllers.SetOutdatedByPreviousPass(&chart)) + }) + + // runPass runs one pass of the check against a fake client holding objs + runPass := func(objs ...client.Object) { + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).WithStatusSubresource(objs...).Build() + controllers.CheckOutdatedHelmCharts(context.TODO(), c) } - } - return tracked, false -} + // newClusterSummaryManagingChart returns the ClusterSummary managing the chart, deployed from repoURL + newClusterSummaryManagingChart := func(repoURL string) *configv1beta1.ClusterSummary { + return newClusterSummaryWithHelmRelease(&configv1beta1.HelmChartSummary{ + ReleaseName: outdatedCheckReleaseName, + ReleaseNamespace: outdatedCheckReleaseNamespace, + Status: configv1beta1.HelmChartStatusManaging, + ChartName: outdatedCheckChartName, + RepoURL: repoURL, + ChartVersion: outdatedCheckChartVersion, + }) + } -// A deleted ClusterSummary is never checked again, so its series must not be carried forward the -// way the series of a chart that could not be fetched is. -func TestCheckOutdatedHelmCharts_ClearsChartOfDeletedClusterSummary(t *testing.T) { - key := outdatedChartKey() - markOutdatedByPreviousPass(t, &key) + expectTrackedAndExposed := func(want bool) { + Expect(controllers.IsOutdatedHelmChartTracked(&chart)).To(Equal(want)) - runCheckerPass(t) + exposed, err := controllers.IsOutdatedHelmChartExposed(&chart) + Expect(err).To(BeNil()) + Expect(exposed).To(Equal(want)) + } - if tracked, exposed := outdatedHelmChartState(t, &key); tracked || exposed { - t.Errorf("chart of a deleted ClusterSummary: tracked = %v, exposed = %v, want false, false", tracked, exposed) - } -} + It("clears the series of a deleted ClusterSummary: it is never checked again", func() { + expectTrackedAndExposed(true) -func TestCheckOutdatedHelmCharts_ClearsChartRemovedFromProfile(t *testing.T) { - key := outdatedChartKey() - markOutdatedByPreviousPass(t, &key) + runPass() - clusterSummary := clusterSummaryManagingChart(testRepoURL) - clusterSummary.Status.HelmReleaseSummaries = nil + expectTrackedAndExposed(false) + }) - runCheckerPass(t, clusterSummary) + It("clears the series of a chart removed from the profile", func() { + clusterSummary := newClusterSummaryManagingChart(outdatedCheckRepoURL) + clusterSummary.Status.HelmReleaseSummaries = nil - if tracked, exposed := outdatedHelmChartState(t, &key); tracked || exposed { - t.Errorf("chart removed from its profile: tracked = %v, exposed = %v, want false, false", tracked, exposed) - } -} + runPass(clusterSummary) -// A failed fetch says nothing about the chart, so its series stays until a pass can check it. -func TestCheckOutdatedHelmCharts_KeepsChartItCouldNotCheck(t *testing.T) { - key := outdatedChartKey() - markOutdatedByPreviousPass(t, &key) + expectTrackedAndExposed(false) + }) - // Nothing listens on port 1, so fetching the index fails right away. - runCheckerPass(t, clusterSummaryManagingChart("http://127.0.0.1:1/charts")) + It("keeps the series of a chart which could not be checked: nothing was learned about it", func() { + // The server is closed: fetching the index fails right away + server := httptest.NewServer(http.NotFoundHandler()) + server.Close() - if tracked, exposed := outdatedHelmChartState(t, &key); !tracked || !exposed { - t.Errorf("chart that could not be checked: tracked = %v, exposed = %v, want true, true", tracked, exposed) - } -} + runPass(newClusterSummaryManagingChart(server.URL)) -func TestCheckOutdatedHelmCharts_ClearsChartThatIsUpToDate(t *testing.T) { - key := outdatedChartKey() - markOutdatedByPreviousPass(t, &key) + expectTrackedAndExposed(true) + }) - index := []byte(`apiVersion: v1 + It("clears the series of a chart which is up to date", func() { + index := []byte(`apiVersion: v1 entries: nginx: - apiVersion: v2 @@ -401,18 +260,18 @@ entries: urls: - nginx-1.0.0.tgz `) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/index.yaml" { - http.NotFound(w, r) - return - } - _, _ = w.Write(index) - })) - defer server.Close() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/index.yaml" { + http.NotFound(w, r) + return + } + _, _ = w.Write(index) + })) + defer server.Close() - runCheckerPass(t, clusterSummaryManagingChart(server.URL)) + runPass(newClusterSummaryManagingChart(server.URL)) - if tracked, exposed := outdatedHelmChartState(t, &key); tracked || exposed { - t.Errorf("chart that is up to date: tracked = %v, exposed = %v, want false, false", tracked, exposed) - } -} + expectTrackedAndExposed(false) + }) + }) +}) diff --git a/controllers/helmchart_semver_test.go b/controllers/helmchart_semver_test.go index 1776695c..872a5e8a 100644 --- a/controllers/helmchart_semver_test.go +++ b/controllers/helmchart_semver_test.go @@ -14,12 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package-internal (not controllers_test): helmChartVersionDiff is a pure, dependency-free -// function, so it is table-tested directly with the standard library rather than through the -// Ginkgo/envtest suite used by the rest of this package's tests. -package controllers +package controllers_test -import "testing" +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/projectsveltos/addon-controller/controllers" +) const ( v1_0_0 = "1.0.0" @@ -32,96 +34,33 @@ const ( notAVersion = "not-a-version" ) -func TestHelmChartVersionDiff(t *testing.T) { - tests := []struct { - name string - current string - available []string - wantLatest string // "" means expect nil - wantLatestPatch string // "" means expect nil - wantErr bool - }{ - { - name: "newer minor and same-minor patch both found", - current: v1_2_3, - available: []string{v1_2_3, v1_2_4, "1.3.0", v2_0_0}, - wantLatest: v2_0_0, - wantLatestPatch: v1_2_4, - }, - { - name: "only a same-minor patch found", - current: v1_2_3, - available: []string{v1_2_3, v1_2_4, v1_2_5}, - wantLatest: v1_2_5, - wantLatestPatch: v1_2_5, - }, - { - name: "already the latest", - current: v2_0_0, - available: []string{v1_2_3, v1_9_9, v2_0_0}, - }, - { - name: "current not in available list but nothing newer", - current: v2_0_0, - available: []string{v1_9_9}, - }, - { - name: "unparseable current version", - current: notAVersion, - available: []string{v1_0_0}, - wantErr: true, +var _ = Describe("Helm chart version diff", func() { + // An empty wantLatest or wantLatestPatch means no version is expected + DescribeTable("helmChartVersionDiff", + func(current string, available []string, wantLatest, wantLatestPatch string) { + latest, latestPatch, err := controllers.HelmChartVersionDiff(current, available) + Expect(err).To(BeNil()) + Expect(latest).To(Equal(wantLatest)) + Expect(latestPatch).To(Equal(wantLatestPatch)) }, - { - name: "unparseable entries in available list are skipped", - current: v1_0_0, - available: []string{notAVersion, v1_1_0, "also-bad"}, - wantLatest: v1_1_0, - wantLatestPatch: "", - }, - { - name: "stable current excludes prereleases", - current: v1_0_0, - available: []string{"1.1.0-rc.1"}, - wantLatest: "", - }, - { - name: "prerelease current includes prereleases", - current: "1.0.0-rc.1", - available: []string{"1.0.0-rc.2", v1_0_0}, - wantLatest: v1_0_0, - wantLatestPatch: v1_0_0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - latest, latestPatch, err := helmChartVersionDiff(tt.current, tt.available) - - if tt.wantErr { - if err == nil { - t.Fatalf("expected an error, got none") - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if tt.wantLatest == "" { - if latest != nil { - t.Errorf("expected nil latest, got %v", latest) - } - } else if latest == nil || latest.String() != tt.wantLatest { - t.Errorf("expected latest %q, got %v", tt.wantLatest, latest) - } + Entry("newer minor and same-minor patch both found", + v1_2_3, []string{v1_2_3, v1_2_4, "1.3.0", v2_0_0}, v2_0_0, v1_2_4), + Entry("only a same-minor patch found", + v1_2_3, []string{v1_2_3, v1_2_4, v1_2_5}, v1_2_5, v1_2_5), + Entry("already the latest", + v2_0_0, []string{v1_2_3, v1_9_9, v2_0_0}, "", ""), + Entry("current not in available list but nothing newer", + v2_0_0, []string{v1_9_9}, "", ""), + Entry("unparseable entries in available list are skipped", + v1_0_0, []string{notAVersion, v1_1_0, "also-bad"}, v1_1_0, ""), + Entry("stable current excludes prereleases", + v1_0_0, []string{"1.1.0-rc.1"}, "", ""), + Entry("prerelease current includes prereleases", + "1.0.0-rc.1", []string{"1.0.0-rc.2", v1_0_0}, v1_0_0, v1_0_0), + ) - if tt.wantLatestPatch == "" { - if latestPatch != nil { - t.Errorf("expected nil latestPatch, got %v", latestPatch) - } - } else if latestPatch == nil || latestPatch.String() != tt.wantLatestPatch { - t.Errorf("expected latestPatch %q, got %v", tt.wantLatestPatch, latestPatch) - } - }) - } -} + It("fails when the current version cannot be parsed", func() { + _, _, err := controllers.HelmChartVersionDiff(notAVersion, []string{v1_0_0}) + Expect(err).ToNot(BeNil()) + }) +})