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()) + }) +})