From 670d9b9a8c452ceb6341fa60963ae2abd1696014 Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Tue, 26 Aug 2025 10:32:16 -0700 Subject: [PATCH] feat(v3): Sort charts by the weight field in the HelmChart custom resource --- .../managers/app/install/install_test.go | 27 ++++-- api/internal/managers/app/release/template.go | 6 ++ .../managers/app/release/template_test.go | 97 +++++++++++++++++++ 3 files changed, 120 insertions(+), 10 deletions(-) diff --git a/api/internal/managers/app/install/install_test.go b/api/internal/managers/app/install/install_test.go index ae66ad078f..3bc473b88c 100644 --- a/api/internal/managers/app/install/install_test.go +++ b/api/internal/managers/app/install/install_test.go @@ -63,16 +63,16 @@ func TestAppInstallManager_Install(t *testing.T) { }, } - // Create InstallableHelmCharts with pre-processed values + // Create InstallableHelmCharts with weights - should already be sorted at this stage installableCharts := []types.InstallableHelmChart{ - createTestInstallableHelmChart(t, "nginx-chart", "1.0.0", "web-server", "web", map[string]any{ + createTestInstallableHelmChart(t, "nginx-chart", "1.0.0", "web-server", "web", 10, map[string]any{ "image": map[string]any{ "repository": "nginx", "tag": "latest", }, "replicas": 3, }), - createTestInstallableHelmChart(t, "postgres-chart", "2.0.0", "database", "data", map[string]any{ + createTestInstallableHelmChart(t, "postgres-chart", "2.0.0", "database", "data", 20, map[string]any{ "database": map[string]any{ "host": "postgres.example.com", "password": "secret", @@ -84,7 +84,7 @@ func TestAppInstallManager_Install(t *testing.T) { mockHelmClient := &helm.MockClient{} // Chart 1 installation (nginx chart) - mockHelmClient.On("Install", mock.Anything, mock.MatchedBy(func(opts helm.InstallOptions) bool { + nginxCall := mockHelmClient.On("Install", mock.Anything, mock.MatchedBy(func(opts helm.InstallOptions) bool { if opts.ReleaseName != "web-server" { return false } @@ -99,7 +99,7 @@ func TestAppInstallManager_Install(t *testing.T) { })).Return(&helmrelease.Release{Name: "web-server"}, nil) // Chart 2 installation (database chart) - mockHelmClient.On("Install", mock.Anything, mock.MatchedBy(func(opts helm.InstallOptions) bool { + databaseCall := mockHelmClient.On("Install", mock.Anything, mock.MatchedBy(func(opts helm.InstallOptions) bool { if opts.ReleaseName != "database" { return false } @@ -113,6 +113,12 @@ func TestAppInstallManager_Install(t *testing.T) { return false })).Return(&helmrelease.Release{Name: "database"}, nil) + // Verify installation order + mock.InOrder( + nginxCall, + databaseCall, + ) + // Create mock installer with detailed verification of config values mockInstaller := &MockKotsCLIInstaller{} mockInstaller.On("Install", mock.MatchedBy(func(opts kotscli.InstallOptions) bool { @@ -190,7 +196,7 @@ func TestAppInstallManager_Install(t *testing.T) { t.Run("Install updates status correctly", func(t *testing.T) { installableCharts := []types.InstallableHelmChart{ - createTestInstallableHelmChart(t, "monitoring-chart", "1.0.0", "prometheus", "monitoring", map[string]any{"key": "value"}), + createTestInstallableHelmChart(t, "monitoring-chart", "1.0.0", "prometheus", "monitoring", 0, map[string]any{"key": "value"}), } // Create mock helm client @@ -239,7 +245,7 @@ func TestAppInstallManager_Install(t *testing.T) { t.Run("Install handles errors correctly", func(t *testing.T) { installableCharts := []types.InstallableHelmChart{ - createTestInstallableHelmChart(t, "logging-chart", "1.0.0", "fluentd", "logging", map[string]any{"key": "value"}), + createTestInstallableHelmChart(t, "logging-chart", "1.0.0", "fluentd", "logging", 0, map[string]any{"key": "value"}), } // Create mock helm client that fails @@ -372,7 +378,7 @@ type: application } // Helper functions to create test data that can be reused across test cases -func createTestHelmChartCR(name, releaseName, namespace string) *kotsv1beta2.HelmChart { +func createTestHelmChartCR(name, releaseName, namespace string, weight int64) *kotsv1beta2.HelmChart { return &kotsv1beta2.HelmChart{ TypeMeta: metav1.TypeMeta{ APIVersion: "kots.io/v1beta2", @@ -384,14 +390,15 @@ func createTestHelmChartCR(name, releaseName, namespace string) *kotsv1beta2.Hel Spec: kotsv1beta2.HelmChartSpec{ ReleaseName: releaseName, Namespace: namespace, + Weight: weight, }, } } -func createTestInstallableHelmChart(t *testing.T, chartName, chartVersion, releaseName, namespace string, values map[string]any) types.InstallableHelmChart { +func createTestInstallableHelmChart(t *testing.T, chartName, chartVersion, releaseName, namespace string, weight int64, values map[string]any) types.InstallableHelmChart { return types.InstallableHelmChart{ Archive: createTestChartArchive(t, chartName, chartVersion), Values: values, - CR: createTestHelmChartCR(chartName, releaseName, namespace), + CR: createTestHelmChartCR(chartName, releaseName, namespace, weight), } } diff --git a/api/internal/managers/app/release/template.go b/api/internal/managers/app/release/template.go index 5206f0d9f1..3ed24aedef 100644 --- a/api/internal/managers/app/release/template.go +++ b/api/internal/managers/app/release/template.go @@ -5,6 +5,7 @@ import ( "fmt" "maps" "os" + "sort" "strconv" "github.com/replicatedhq/embedded-cluster/api/pkg/template" @@ -101,6 +102,11 @@ func (m *appReleaseManager) ExtractInstallableHelmCharts(ctx context.Context, co installableCharts = append(installableCharts, installableChart) } + // Sort charts by weight field before returning + sort.Slice(installableCharts, func(i, j int) bool { + return installableCharts[i].CR.GetWeight() < installableCharts[j].CR.GetWeight() + }) + return installableCharts, nil } diff --git a/api/internal/managers/app/release/template_test.go b/api/internal/managers/app/release/template_test.go index dd9ab73a3f..710d84e3f6 100644 --- a/api/internal/managers/app/release/template_test.go +++ b/api/internal/managers/app/release/template_test.go @@ -2396,6 +2396,103 @@ spec: }, }, }, + { + name: "charts sorted by weight - negative, zero, positive", + helmChartCRs: [][]byte{ + []byte(`apiVersion: kots.io/v1beta2 +kind: HelmChart +metadata: + name: positive-weight-chart +spec: + chart: + name: nginx + chartVersion: "1.0.0" + weight: 100 + values: + name: "positive"`), + []byte(`apiVersion: kots.io/v1beta2 +kind: HelmChart +metadata: + name: no-weight-chart +spec: + chart: + name: redis + chartVersion: "2.0.0" + values: + name: "zero"`), // No weight specified, defaults to 0 + []byte(`apiVersion: kots.io/v1beta2 +kind: HelmChart +metadata: + name: negative-weight-chart +spec: + chart: + name: postgresql + chartVersion: "1.0.0" + weight: -10 + values: + name: "negative"`), + }, + chartArchives: [][]byte{ + createTestChartArchive(t, "nginx", "1.0.0"), + createTestChartArchive(t, "redis", "2.0.0"), + createTestChartArchive(t, "postgresql", "1.0.0"), + }, + configValues: types.AppConfigValues{}, + expectError: false, + expected: []types.InstallableHelmChart{ + // Should be sorted by weight: postgresql (-10), redis (0), nginx (100) + { + Archive: createTestChartArchive(t, "postgresql", "1.0.0"), + Values: map[string]any{ + "name": "negative", + }, + CR: createHelmChartCRFromYAML(`apiVersion: kots.io/v1beta2 +kind: HelmChart +metadata: + name: negative-weight-chart +spec: + chart: + name: postgresql + chartVersion: "1.0.0" + weight: -10 + values: + name: "negative"`), + }, + { + Archive: createTestChartArchive(t, "redis", "2.0.0"), + Values: map[string]any{ + "name": "zero", + }, + CR: createHelmChartCRFromYAML(`apiVersion: kots.io/v1beta2 +kind: HelmChart +metadata: + name: no-weight-chart +spec: + chart: + name: redis + chartVersion: "2.0.0" + values: + name: "zero"`), + }, + { + Archive: createTestChartArchive(t, "nginx", "1.0.0"), + Values: map[string]any{ + "name": "positive", + }, + CR: createHelmChartCRFromYAML(`apiVersion: kots.io/v1beta2 +kind: HelmChart +metadata: + name: positive-weight-chart +spec: + chart: + name: nginx + chartVersion: "1.0.0" + weight: 100 + values: + name: "positive"`), + }, + }, + }, } for _, tt := range tests {