Merge pull request #9054 from dokku/9053-scheduler-k3s-ensure-charts-ignores-the-pinned-chart-version-when-upgrading-an-existing-release

Honor the pinned chart version when upgrading

docs/deployment/schedulers/k3s.md

@@ -16,8 +16,8 @@ scheduler-k3s:charts:set <chart-name.property> (<value>) # Set or clear a chart-
 scheduler-k3s:cluster:add [--profile PROFILE] [--role ROLE] [--insecure-allow-unknown-hosts] [--server-ip SERVER_IP] [--taint-scheduling] [--kubelet-args KUBELET_ARGS] <ssh://user@host:port> # Adds a server node to a Dokku-managed cluster
 scheduler-k3s:cluster:list [--format json|stdout] # Lists all nodes in a Dokku-managed cluster
 scheduler-k3s:cluster:remove [node-id] # Removes client node to a Dokku-managed cluster
-scheduler-k3s:ensure-charts # Ensures the k3s charts are installed
+scheduler-k3s:ensure-charts [--force] [--charts CHART_NAMES] # Ensures the k3s charts are installed
-scheduler-k3s:initialize [--server-ip SERVER_IP] [--taint-scheduling] [--kubelet-args KUBELET_ARGS] # Initializes a cluster
+scheduler-k3s:initialize [--server-ip SERVER_IP] [--ingress-class INGRESS_CLASS] [--taint-scheduling] [--kubelet-args KUBELET_ARGS] # Initializes a cluster
 scheduler-k3s:labels:clear <app|--global> [--process-type PROCESS_TYPE] [--resource-type RESOURCE_TYPE] # Clear all labels for an app or a single process-type/resource-type scope
 scheduler-k3s:labels:set <app|--global> <property> (<value>) [--process-type PROCESS_TYPE] <--resource-type RESOURCE_TYPE> # Set or clear a label for a given app/process-type/resource-type combination
 scheduler-k3s:labels:set --replace <app|--global> <key=value> [<key=value> ...] [--process-type PROCESS_TYPE] <--resource-type RESOURCE_TYPE> # Replace the entire label map for a given app/process-type/resource-type combination
@@ -1100,18 +1100,20 @@ The default value for the `kube-context` is an empty string, and will result in
 Dokku includes a number of helm charts by default with settings that are optimized for Dokku. That said, it may be useful to further customize the charts for a given environment. Chart overrides are managed via the `scheduler-k3s:charts:set` command, which takes a `<chart-name>.<property>` argument and a value. Chart overrides are always global because helm charts are not managed on a per-app basis.
 
 ```shell
-dokku scheduler-k3s:charts:set cert-manager.version 1.13.3
+dokku scheduler-k3s:charts:set cert-manager.installCRDs false
 ```
 
 > [!NOTE]
 > Properties follow dot-notation, and are expanded according to Helm's internal logic. See the [Helm documentation](https://helm.sh/docs/helm/helm_install/#helm-install) for `helm install` for further details.
 
+Chart overrides are values passed to a chart, not the version of the chart itself. Dokku pins the version of every chart it manages, and those versions are not user-configurable. The `scheduler-k3s:ensure-charts` command reconciles each installed release back to the pinned version, upgrading or downgrading a release that has drifted away from it.
+
 Property names may contain `/` (e.g. for Kubernetes-style annotation keys such as `service.annotations.prometheus.io/scrape`) and values may span multiple lines; both are preserved verbatim.
 
 To unset a chart property, omit the value from the `scheduler-k3s:charts:set` call:
 
 ```shell
-dokku scheduler-k3s:charts:set cert-manager.version
+dokku scheduler-k3s:charts:set cert-manager.installCRDs
 ```
 
 Configured chart overrides can be inspected with the `scheduler-k3s:charts:report` command. Without an argument, it lists every chart known to Dokku along with any configured overrides; passing a chart name scopes the report to that chart:

plugins/scheduler-k3s/functions.go

@@ -12,6 +12,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -2142,19 +2143,68 @@ func hasKustomizeDirectory(appName string) bool {
 	return common.DirectoryExists(directory)
 }
 
-func installHelmCharts(ctx context.Context, clientset KubernetesClient, shouldInstall func(HelmChart) bool) error {
+// ChartInstallDecisionInput is the input for the chartInstallDecision function
-	for _, repo := range HelmRepositories {
+type ChartInstallDecisionInput struct {
-		helmAgent, err := NewHelmAgent("default", DeployLogPrinter)
+	// Chart is the chart being evaluated
-		if err != nil {
+	Chart HelmChart
-			return fmt.Errorf("Error creating helm agent: %w", err)
-		}
 
-		err = helmAgent.AddRepository(ctx, AddRepositoryInput(repo))
+	// ForceChartNames is the list of release names to install regardless of state
-		if err != nil {
+	ForceChartNames []string
-			return fmt.Errorf("Error adding helm repository %s: %w", repo.Name, err)
+
+	// ForceInstall is whether every chart should be installed regardless of state
+	ForceInstall bool
+
+	// IngressClass is the ingress class configured for the cluster
+	IngressClass string
+
+	// InstalledRevision is the helm release currently installed for the chart
+	InstalledRevision Release
+}
+
+// ChartInstallDecision reports whether a chart should be installed and why
+type ChartInstallDecision struct {
+	// Install is whether the chart should be installed
+	Install bool
+
+	// Reason describes why the chart is being installed or skipped
+	Reason string
+}
+
+// chartInstallDecision reports whether a chart should be installed, and why. A
+// chart whose installed release has drifted from the pinned version is
+// reinstalled so that the pinned version is what ends up on the cluster.
+func chartInstallDecision(input ChartInstallDecisionInput) ChartInstallDecision {
+	if input.Chart.ChartPath == "traefik" && input.IngressClass == "nginx" {
+		return ChartInstallDecision{Install: false, Reason: "Skipping chart due to ingress-class mismatch"}
+	}
+
+	if input.Chart.ChartPath == "ingress-nginx" && input.IngressClass == "traefik" {
+		return ChartInstallDecision{Install: false, Reason: "Skipping chart due to ingress-class mismatch"}
+	}
+
+	if input.ForceInstall {
+		return ChartInstallDecision{Install: true, Reason: "Force installing chart"}
+	}
+
+	if slices.Contains(input.ForceChartNames, input.Chart.ReleaseName) {
+		return ChartInstallDecision{Install: true, Reason: "Force installing chart due to flag"}
+	}
+
+	if input.InstalledRevision.Name == "" {
+		return ChartInstallDecision{Install: true, Reason: "Installing missing chart"}
+	}
+
+	if input.InstalledRevision.Version != input.Chart.Version {
+		return ChartInstallDecision{
+			Install: true,
+			Reason:  fmt.Sprintf("Installing chart due to version mismatch: %s != %s", input.InstalledRevision.Version, input.Chart.Version),
 		}
 	}
 
+	return ChartInstallDecision{Install: false, Reason: "Skipping chart: already installed"}
+}
+
+func installHelmCharts(ctx context.Context, clientset KubernetesClient, shouldInstall func(HelmChart) bool) error {
 	for _, chart := range HelmCharts {
 		if !shouldInstall(chart) {
 			continue

plugins/scheduler-k3s/functions_test.go

@@ -723,3 +723,131 @@ func TestAttachExitError(t *testing.T) {
 		t.Errorf("attachExitError(zero code error) = %v, want the original error", err)
 	}
 }
+
+func TestChartInstallDecision(t *testing.T) {
+	traefik := HelmChart{ChartPath: "traefik", ReleaseName: "traefik", Version: "26.0.0"}
+	ingressNginx := HelmChart{ChartPath: "ingress-nginx", ReleaseName: "ingress-nginx", Version: "4.15.1"}
+	installed := Release{Name: "traefik", Version: "26.0.0"}
+
+	cases := []struct {
+		name        string
+		input       ChartInstallDecisionInput
+		wantInstall bool
+		wantReason  string
+	}{
+		{
+			name: "traefik is skipped when the ingress class is nginx",
+			input: ChartInstallDecisionInput{
+				Chart:        traefik,
+				ForceInstall: true,
+				IngressClass: "nginx",
+			},
+			wantInstall: false,
+			wantReason:  "Skipping chart due to ingress-class mismatch",
+		},
+		{
+			name: "ingress-nginx is skipped when the ingress class is traefik",
+			input: ChartInstallDecisionInput{
+				Chart:        ingressNginx,
+				ForceInstall: true,
+				IngressClass: "traefik",
+			},
+			wantInstall: false,
+			wantReason:  "Skipping chart due to ingress-class mismatch",
+		},
+		{
+			name: "force installs an up to date chart",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				ForceInstall:      true,
+				IngressClass:      "traefik",
+				InstalledRevision: installed,
+			},
+			wantInstall: true,
+			wantReason:  "Force installing chart",
+		},
+		{
+			name: "a named chart installs even when up to date",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				ForceChartNames:   []string{"traefik"},
+				IngressClass:      "traefik",
+				InstalledRevision: installed,
+			},
+			wantInstall: true,
+			wantReason:  "Force installing chart due to flag",
+		},
+		{
+			name: "a chart named for another release is left alone",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				ForceChartNames:   []string{"vector"},
+				IngressClass:      "traefik",
+				InstalledRevision: installed,
+			},
+			wantInstall: false,
+			wantReason:  "Skipping chart: already installed",
+		},
+		{
+			name: "a missing release is installed",
+			input: ChartInstallDecisionInput{
+				Chart:        traefik,
+				IngressClass: "traefik",
+			},
+			wantInstall: true,
+			wantReason:  "Installing missing chart",
+		},
+		{
+			name: "a release behind the pinned version is upgraded",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				IngressClass:      "traefik",
+				InstalledRevision: Release{Name: "traefik", Version: "25.0.0"},
+			},
+			wantInstall: true,
+			wantReason:  "Installing chart due to version mismatch: 25.0.0 != 26.0.0",
+		},
+		{
+			name: "a release ahead of the pinned version is reconciled back",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				IngressClass:      "traefik",
+				InstalledRevision: Release{Name: "traefik", Version: "41.6.0"},
+			},
+			wantInstall: true,
+			wantReason:  "Installing chart due to version mismatch: 41.6.0 != 26.0.0",
+		},
+		{
+			name: "the mismatch reason reports the chart version, not the app version",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				IngressClass:      "traefik",
+				InstalledRevision: Release{Name: "traefik", AppVersion: "v2.11.0", Version: "41.6.0"},
+			},
+			wantInstall: true,
+			wantReason:  "Installing chart due to version mismatch: 41.6.0 != 26.0.0",
+		},
+		{
+			name: "a release at the pinned version is skipped",
+			input: ChartInstallDecisionInput{
+				Chart:             traefik,
+				IngressClass:      "traefik",
+				InstalledRevision: installed,
+			},
+			wantInstall: false,
+			wantReason:  "Skipping chart: already installed",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := chartInstallDecision(tc.input)
+			if got.Install != tc.wantInstall {
+				t.Errorf("Install = %t, want %t", got.Install, tc.wantInstall)
+			}
+			if got.Reason != tc.wantReason {
+				t.Errorf("Reason = %q, want %q", got.Reason, tc.wantReason)
+			}
+		})
+	}
+}

plugins/scheduler-k3s/go.mod

@@ -19,7 +19,6 @@ require (
 	github.com/fluxcd/pkg/kustomize v1.40.0
 	github.com/go-openapi/jsonpointer v1.0.1
 	github.com/go-resty/resty/v2 v2.17.2
-	github.com/gofrs/flock v0.13.1
 	github.com/gosimple/slug v1.15.0
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
 	github.com/kedacore/keda/v2 v2.20.2

plugins/scheduler-k3s/go.sum

@@ -167,8 +167,6 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v
 github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8=
 github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
 github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
-github.com/gofrs/flock v0.13.1 h1:jjREztyBeSKBZYAC+mgc1laB+xsgy4kYMf3FbKF2UBo=
-github.com/gofrs/flock v0.13.1/go.mod h1:sf4BFiHwnvgxa25DlQoDqXQnwRMEOwqxRq37P6MzzmE=
 github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg=
 github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4=
 github.com/google/gnostic-models v0.7.1 h1:SisTfuFKJSKM5CPZkffwi6coztzzeYUhc3v4yxLWH8c=

plugins/scheduler-k3s/helm.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log"
 	"os"
 	"path/filepath"
 	"sort"
@@ -15,16 +14,13 @@ import (
 
 	"github.com/dokku/dokku/plugins/common"
 	"github.com/fluxcd/pkg/kustomize/filesys"
-	"github.com/gofrs/flock"
-	"gopkg.in/yaml.v3"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/cli"
-	"helm.sh/helm/v3/pkg/getter"
+	"helm.sh/helm/v3/pkg/helmpath"
 	"helm.sh/helm/v3/pkg/kube"
 	"helm.sh/helm/v3/pkg/postrender"
 	"helm.sh/helm/v3/pkg/release"
-	"helm.sh/helm/v3/pkg/repo"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	"sigs.k8s.io/kustomize/api/konfig"
 	"sigs.k8s.io/kustomize/api/krusty"
@@ -93,6 +89,73 @@ type Release struct {
 	Version    string
 }
 
+// applyChartPathOptions pins the repository and the version from input onto the
+// chart path options used to locate a chart. Both the install and the upgrade
+// path must do this, otherwise helm resolves the newest chart published to the
+// repository instead of the version dokku pins.
+func applyChartPathOptions(options *action.ChartPathOptions, input ChartInput) {
+	if input.RepoURL != "" {
+		options.RepoURL = input.RepoURL
+	}
+	if input.Version != "" {
+		options.Version = input.Version
+	}
+}
+
+// helmHomeEnvVars are the environment variables helm resolves its cache, config
+// and data homes from, mapped to the scratch subdirectory each one gets.
+var helmHomeEnvVars = map[string]string{
+	helmpath.CacheHomeEnvVar:  "cache",
+	helmpath.ConfigHomeEnvVar: "config",
+	helmpath.DataHomeEnvVar:   "data",
+}
+
+// newHelmSettings returns helm settings backed by a private scratch directory,
+// along with a function that removes it and restores the environment.
+//
+// helm otherwise resolves its cache, config and data homes from $HOME. Dokku
+// runs scheduler-k3s:initialize as root and every other subcommand as the dokku
+// user, so $HOME is not always readable by the user a command ends up as, and
+// helm treats a repositories file it cannot read as a fatal error rather than
+// an absent one. Nothing dokku keeps there needs to outlive a command: every
+// chart carries its own repository url, and helm re-fetches both the repository
+// index and the chart archive on every call.
+func newHelmSettings() (*cli.EnvSettings, func(), error) {
+	directory, err := os.MkdirTemp("", "dokku-helm-")
+	if err != nil {
+		return nil, nil, fmt.Errorf("Error creating helm scratch directory: %w", err)
+	}
+
+	restore := map[string]*string{}
+	cleanup := func() {
+		for envVar, previous := range restore {
+			if previous == nil {
+				os.Unsetenv(envVar) // nolint: errcheck
+				continue
+			}
+
+			os.Setenv(envVar, *previous) // nolint: errcheck
+		}
+
+		os.RemoveAll(directory) // nolint: errcheck
+	}
+
+	for envVar, subdirectory := range helmHomeEnvVars {
+		if previous, ok := os.LookupEnv(envVar); ok {
+			restore[envVar] = &previous
+		} else {
+			restore[envVar] = nil
+		}
+
+		if err := os.Setenv(envVar, filepath.Join(directory, subdirectory)); err != nil {
+			cleanup()
+			return nil, nil, fmt.Errorf("Error setting %s: %w", envVar, err)
+		}
+	}
+
+	return cli.New(), cleanup, nil
+}
+
 type HelmAgent struct {
 	Configuration *action.Configuration
 	Namespace     string
@@ -121,69 +184,6 @@ func NewHelmAgent(namespace string, logger action.DebugLog) (*HelmAgent, error)
 	}, nil
 }
 
-type AddRepositoryInput struct {
-	Name string
-	URL  string
-}
-
-func (h *HelmAgent) AddRepository(ctx context.Context, helmRepo AddRepositoryInput) error {
-	settings := cli.New()
-	repoFile := settings.RepositoryConfig
-
-	err := os.MkdirAll(filepath.Dir(repoFile), os.ModePerm)
-	if err != nil && !os.IsExist(err) {
-		return fmt.Errorf("Error creating repository directory: %w", err)
-	}
-
-	fileLock := flock.New(strings.Replace(repoFile, filepath.Ext(repoFile), ".lock", 1))
-	locked, err := fileLock.TryLockContext(ctx, time.Second)
-	if err != nil {
-		return fmt.Errorf("Error acquiring file lock: %w", err)
-	}
-
-	if !locked {
-		return fmt.Errorf("Could not acquire file lock")
-	}
-
-	defer fileLock.Unlock() // nolint: errcheck
-
-	b, err := os.ReadFile(repoFile)
-	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("Error reading repository file: %w", err)
-	}
-
-	var f repo.File
-	if err := yaml.Unmarshal(b, &f); err != nil {
-		return fmt.Errorf("Error unmarshalling yaml: %w", err)
-	}
-
-	if f.Has(helmRepo.Name) {
-		return nil
-	}
-
-	c := repo.Entry{
-		Name: helmRepo.Name,
-		URL:  helmRepo.URL,
-	}
-
-	r, err := repo.NewChartRepository(&c, getter.All(settings))
-	if err != nil {
-		return fmt.Errorf("Error creating chart repository: %w", err)
-	}
-
-	if _, err := r.DownloadIndexFile(); err != nil {
-		return fmt.Errorf("Specified repository '%q' is not a valid chart repository or cannot be reached: %w", helmRepo.URL, err)
-	}
-
-	f.Update(&c)
-
-	if err := f.WriteFile(repoFile, 0644); err != nil {
-		log.Fatal(err)
-	}
-
-	return nil
-}
-
 func (h *HelmAgent) ChartExists(releaseName string) (bool, error) {
 	if releaseName == "" {
 		return false, fmt.Errorf("Release name is required")
@@ -270,7 +270,6 @@ func (h *HelmAgent) InstallChart(ctx context.Context, input ChartInput) error {
 
 	client := action.NewInstall(h.Configuration)
 	client.Atomic = false
-	client.ChartPathOptions = action.ChartPathOptions{}
 	client.CreateNamespace = true
 	client.DryRun = false
 	if os.Getenv("DOKKU_TRACE") == "1" {
@@ -285,13 +284,13 @@ func (h *HelmAgent) InstallChart(ctx context.Context, input ChartInput) error {
 	client.Timeout = input.Timeout
 	client.Wait = input.Wait
 
-	settings := cli.New()
+	applyChartPathOptions(&client.ChartPathOptions, input)
-	if input.RepoURL != "" {
+
-		client.ChartPathOptions.RepoURL = input.RepoURL
+	settings, cleanup, err := newHelmSettings()
-	}
+	if err != nil {
-	if input.Version != "" {
+		return err
-		client.ChartPathOptions.Version = input.Version
 	}
+	defer cleanup()
 
 	chart, err := client.ChartPathOptions.LocateChart(input.ChartPath, settings)
 	if err != nil {
@@ -324,7 +323,7 @@ func (h *HelmAgent) InstalledRevision(releaseName string) (Release, error) {
 		return Release{}, nil
 	}
 
-	return revisions[0], nil
+	return revisions[len(revisions)-1], nil
 }
 
 type ListRevisionsInput struct {
@@ -332,11 +331,24 @@ type ListRevisionsInput struct {
 	Max         int
 }
 
+// selectRevisions sorts releases ascending by revision and keeps at most the
+// most recent max entries. A max of zero keeps every revision. helm's history
+// action ignores its own Max field and hands back the entire ledger unsorted,
+// so both the ordering and the truncation happen here.
+func selectRevisions(releases []Release, max int) []Release {
+	sort.Slice(releases, func(i, j int) bool {
+		return releases[i].Revision < releases[j].Revision
+	})
+
+	if max > 0 && len(releases) > max {
+		return releases[len(releases)-max:]
+	}
+
+	return releases
+}
+
 func (h *HelmAgent) ListRevisions(input ListRevisionsInput) ([]Release, error) {
 	client := action.NewHistory(h.Configuration)
-	if input.Max > 0 {
-		client.Max = input.Max
-	}
 
 	releases := []Release{}
 	response, err := client.Run(input.ReleaseName)
@@ -364,11 +376,7 @@ func (h *HelmAgent) ListRevisions(input ListRevisionsInput) ([]Release, error) {
 		})
 	}
 
-	sort.Slice(releases, func(i, j int) bool {
+	return selectRevisions(releases, input.Max), nil
-		return releases[i].Revision < releases[j].Revision
-	})
-
-	return releases, nil
 }
 
 func (h *HelmAgent) UpgradeChart(ctx context.Context, input ChartInput) error {
@@ -394,7 +402,6 @@ func (h *HelmAgent) UpgradeChart(ctx context.Context, input ChartInput) error {
 
 	client := action.NewUpgrade(h.Configuration)
 	client.Atomic = input.RollbackOnFailure
-	client.ChartPathOptions = action.ChartPathOptions{}
 	client.CleanupOnFail = true
 	client.MaxHistory = 10
 	if os.Getenv("DOKKU_TRACE") == "1" {
@@ -407,11 +414,15 @@ func (h *HelmAgent) UpgradeChart(ctx context.Context, input ChartInput) error {
 	client.Namespace = namespace
 	client.Timeout = input.Timeout
 	client.Wait = input.Wait
-	if input.RepoURL != "" {
-		client.RepoURL = input.RepoURL
-	}
 
-	settings := cli.New()
+	applyChartPathOptions(&client.ChartPathOptions, input)
+
+	settings, cleanup, err := newHelmSettings()
+	if err != nil {
+		return err
+	}
+	defer cleanup()
+
 	chart, err := client.ChartPathOptions.LocateChart(input.ChartPath, settings)
 	if err != nil {
 		return fmt.Errorf("Error locating chart: %w", err)

plugins/scheduler-k3s/helm_test.go

@@ -0,0 +1,226 @@
+package scheduler_k3s
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/helmpath"
+)
+
+func TestApplyChartPathOptions(t *testing.T) {
+	cases := []struct {
+		name        string
+		options     action.ChartPathOptions
+		input       ChartInput
+		wantRepoURL string
+		wantVersion string
+	}{
+		{
+			name:        "pins both the repository and the version",
+			input:       ChartInput{RepoURL: "https://helm.traefik.io/traefik", Version: "26.0.0"},
+			wantRepoURL: "https://helm.traefik.io/traefik",
+			wantVersion: "26.0.0",
+		},
+		{
+			name:        "an empty version leaves the field unset",
+			input:       ChartInput{RepoURL: "https://helm.traefik.io/traefik"},
+			wantRepoURL: "https://helm.traefik.io/traefik",
+			wantVersion: "",
+		},
+		{
+			name:        "an empty repository leaves the field unset",
+			input:       ChartInput{Version: "26.0.0"},
+			wantRepoURL: "",
+			wantVersion: "26.0.0",
+		},
+		{
+			name:        "an empty input leaves both fields unset",
+			input:       ChartInput{},
+			wantRepoURL: "",
+			wantVersion: "",
+		},
+		{
+			name:        "existing values are preserved when the input is empty",
+			options:     action.ChartPathOptions{RepoURL: "https://charts.jetstack.io", Version: "v1.13.3"},
+			input:       ChartInput{},
+			wantRepoURL: "https://charts.jetstack.io",
+			wantVersion: "v1.13.3",
+		},
+		{
+			name:        "existing values are overridden by the input",
+			options:     action.ChartPathOptions{RepoURL: "https://charts.jetstack.io", Version: "v1.13.3"},
+			input:       ChartInput{RepoURL: "https://helm.traefik.io/traefik", Version: "26.0.0"},
+			wantRepoURL: "https://helm.traefik.io/traefik",
+			wantVersion: "26.0.0",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			options := tc.options
+			applyChartPathOptions(&options, tc.input)
+
+			if options.RepoURL != tc.wantRepoURL {
+				t.Errorf("RepoURL = %q, want %q", options.RepoURL, tc.wantRepoURL)
+			}
+			if options.Version != tc.wantVersion {
+				t.Errorf("Version = %q, want %q", options.Version, tc.wantVersion)
+			}
+		})
+	}
+}
+
+func makeRevisions(revisions ...int) []Release {
+	out := make([]Release, 0, len(revisions))
+	for _, revision := range revisions {
+		out = append(out, Release{Name: "traefik", Revision: revision})
+	}
+	return out
+}
+
+func revisionNumbers(releases []Release) []int {
+	out := make([]int, 0, len(releases))
+	for _, release := range releases {
+		out = append(out, release.Revision)
+	}
+	return out
+}
+
+func equalInts(a, b []int) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func TestSelectRevisions(t *testing.T) {
+	cases := []struct {
+		name     string
+		releases []Release
+		max      int
+		want     []int
+	}{
+		{
+			name:     "sorts an unordered ledger ascending",
+			releases: makeRevisions(3, 1, 2),
+			max:      0,
+			want:     []int{1, 2, 3},
+		},
+		{
+			name:     "a max of one keeps the newest revision",
+			releases: makeRevisions(3, 1, 2),
+			max:      1,
+			want:     []int{3},
+		},
+		{
+			name:     "a max below the ledger size keeps the newest revisions",
+			releases: makeRevisions(4, 1, 3, 2),
+			max:      2,
+			want:     []int{3, 4},
+		},
+		{
+			name:     "a max above the ledger size keeps everything",
+			releases: makeRevisions(2, 1),
+			max:      10,
+			want:     []int{1, 2},
+		},
+		{
+			name:     "an empty ledger stays empty",
+			releases: []Release{},
+			max:      1,
+			want:     []int{},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := revisionNumbers(selectRevisions(tc.releases, tc.max))
+			if !equalInts(got, tc.want) {
+				t.Errorf("selectRevisions() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestNewHelmSettings(t *testing.T) {
+	sentinel := map[string]string{
+		helmpath.CacheHomeEnvVar:  "/sentinel/cache",
+		helmpath.ConfigHomeEnvVar: "/sentinel/config",
+		helmpath.DataHomeEnvVar:   "/sentinel/data",
+	}
+	for envVar, value := range sentinel {
+		t.Setenv(envVar, value)
+	}
+
+	settings, cleanup, err := newHelmSettings()
+	if err != nil {
+		t.Fatalf("newHelmSettings() returned an error: %v", err)
+	}
+
+	directory := os.Getenv(helmpath.CacheHomeEnvVar)
+	if directory == sentinel[helmpath.CacheHomeEnvVar] {
+		t.Fatalf("%s was not overridden", helmpath.CacheHomeEnvVar)
+	}
+	scratch := filepath.Dir(directory)
+
+	if _, err := os.Stat(scratch); err != nil {
+		t.Fatalf("scratch directory %q is not usable: %v", scratch, err)
+	}
+
+	for envVar, subdirectory := range helmHomeEnvVars {
+		want := filepath.Join(scratch, subdirectory)
+		if got := os.Getenv(envVar); got != want {
+			t.Errorf("%s = %q, want %q", envVar, got, want)
+		}
+	}
+
+	paths := map[string]string{
+		"RepositoryConfig": settings.RepositoryConfig,
+		"RepositoryCache":  settings.RepositoryCache,
+		"RegistryConfig":   settings.RegistryConfig,
+	}
+	for name, path := range paths {
+		if !strings.HasPrefix(path, scratch+string(os.PathSeparator)) {
+			t.Errorf("%s = %q, want a path under %q", name, path, scratch)
+		}
+	}
+
+	cleanup()
+
+	if _, err := os.Stat(scratch); !os.IsNotExist(err) {
+		t.Errorf("scratch directory %q still exists after cleanup", scratch)
+	}
+
+	for envVar, want := range sentinel {
+		if got := os.Getenv(envVar); got != want {
+			t.Errorf("%s = %q after cleanup, want the prior value %q", envVar, got, want)
+		}
+	}
+}
+
+func TestNewHelmSettingsUnsetsVariablesThatWereNotSet(t *testing.T) {
+	for envVar := range helmHomeEnvVars {
+		t.Setenv(envVar, "")
+		os.Unsetenv(envVar) // nolint: errcheck
+	}
+
+	_, cleanup, err := newHelmSettings()
+	if err != nil {
+		t.Fatalf("newHelmSettings() returned an error: %v", err)
+	}
+	cleanup()
+
+	for envVar := range helmHomeEnvVars {
+		if value, ok := os.LookupEnv(envVar); ok {
+			t.Errorf("%s = %q after cleanup, want it to be unset", envVar, value)
+		}
+	}
+}

plugins/scheduler-k3s/preview.go

@@ -15,7 +15,6 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
-	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
 )
@@ -108,7 +107,12 @@ func CommandPreview(appName string, diffContext int, showSecrets bool, showSecre
 }
 
 func loadChartForPreview(chartPath string) (*chart.Chart, error) {
-	settings := cli.New()
+	settings, cleanup, err := newHelmSettings()
+	if err != nil {
+		return nil, err
+	}
+	defer cleanup()
+
 	opts := action.ChartPathOptions{}
 	located, err := opts.LocateChart(chartPath, settings)
 	if err != nil {

plugins/scheduler-k3s/scheduler_k3s.go

@@ -219,26 +219,6 @@ var HelmCharts = []HelmChart{
 	},
 }
 
-type HelmRepository struct {
-	Name string
-	URL  string
-}
-
-var HelmRepositories = []HelmRepository{
-	{
-		Name: "jetstack",
-		URL:  "https://charts.jetstack.io",
-	},
-	{
-		Name: "longhorn",
-		URL:  "https://charts.longhorn.io",
-	},
-	{
-		Name: "traefik",
-		URL:  "https://helm.traefik.io/traefik",
-	},
-}
-
 // NodeProfile is a profile for a node in the k3s cluster
 type NodeProfile struct {
 	// Name is the name of the node profile

plugins/scheduler-k3s/src/commands/commands.go

@@ -29,8 +29,8 @@ Additional commands:`
     scheduler-k3s:cluster:add [--profile PROFILE] [--role ROLE] [--insecure-allow-unknown-hosts] [--server-ip SERVER_IP] [--taint-scheduling] [--kubelet-args KUBELET_ARGS] <ssh://user@host:port>, Adds a server node to a Dokku-managed cluster
     scheduler-k3s:cluster:list [--format json|stdout], Lists all nodes in a Dokku-managed cluster
     scheduler-k3s:cluster:remove [node-id], Removes client node to a Dokku-managed cluster
-    scheduler-k3s:ensure-charts, Ensures the k3s charts are installed
+    scheduler-k3s:ensure-charts [--force] [--charts CHART_NAMES], Ensures the k3s charts are installed
-    scheduler-k3s:initialize [--server-ip SERVER_IP] [--taint-scheduling] [--kubelet-args KUBELET_ARGS], Initializes a cluster
+    scheduler-k3s:initialize [--server-ip SERVER_IP] [--ingress-class INGRESS_CLASS] [--taint-scheduling] [--kubelet-args KUBELET_ARGS], Initializes a cluster
     scheduler-k3s:labels:clear <app|--global> [--process-type PROCESS_TYPE] [--resource-type RESOURCE_TYPE], Clear all labels for an app or a single process-type/resource-type scope
     scheduler-k3s:labels:set <app|--global> <property> (<value>) [--process-type PROCESS_TYPE] <--resource-type RESOURCE_TYPE>, Set or clear a label for a given app/process-type/resource-type combination
     scheduler-k3s:labels:set --replace <app|--global> <key=value> [<key=value> ...] [--process-type PROCESS_TYPE] <--resource-type RESOURCE_TYPE>, Replace the entire label map for a given app/process-type/resource-type combination

plugins/scheduler-k3s/subcommands.go

@@ -9,7 +9,6 @@ import (
 	"net/url"
 	"os"
 	"os/signal"
-	"slices"
 	"sort"
 	"strings"
 	"syscall"
@@ -572,15 +571,11 @@ func CommandInitialize(ingressClass string, serverIP string, taintScheduling boo
 
 	common.LogInfo2Quiet("Installing helm charts")
 	err = installHelmCharts(ctx, clientset, func(chart HelmChart) bool {
-		if chart.ChartPath == "traefik" && ingressClass == "nginx" {
+		return chartInstallDecision(ChartInstallDecisionInput{
-			return false
+			Chart:        chart,
-		}
+			ForceInstall: true,
-
+			IngressClass: ingressClass,
-		if chart.ChartPath == "ingress-nginx" && ingressClass == "traefik" {
+		}).Install
-			return false
-		}
-
-		return true
 	})
 	if err != nil {
 		return fmt.Errorf("Unable to install helm charts: %w", err)
@@ -1269,25 +1264,6 @@ func CommandEnsureCharts(forceInstall bool, forceChartNames []string) error {
 	common.LogInfo2Quiet("Installing helm charts")
 	err = installHelmCharts(ctx, clientset, func(chart HelmChart) bool {
 		common.LogInfo1(fmt.Sprintf("Processing chart %s@%s", chart.ReleaseName, chart.Version))
-		if chart.ChartPath == "traefik" && ingressClass == "nginx" {
-			common.LogVerbose("Skipping chart due to ingress-class mismatch")
-			return false
-		}
-
-		if chart.ChartPath == "ingress-nginx" && ingressClass == "traefik" {
-			common.LogVerbose("Skipping chart due to ingress-class mismatch")
-			return false
-		}
-
-		if forceInstall {
-			common.LogVerbose("Force installing chart")
-			return true
-		}
-
-		if len(forceChartNames) > 0 && slices.Contains(forceChartNames, chart.ReleaseName) {
-			common.LogVerbose("Force installing chart due to flag")
-			return true
-		}
 
 		helmAgent := namespacedHelmAgents[chart.Namespace]
 		latestRevision, err := helmAgent.InstalledRevision(chart.ReleaseName)
@@ -1296,18 +1272,16 @@ func CommandEnsureCharts(forceInstall bool, forceChartNames []string) error {
 			return false
 		}
 
-		if latestRevision.Name == "" {
+		decision := chartInstallDecision(ChartInstallDecisionInput{
-			common.LogVerbose("Installing missing chart")
+			Chart:             chart,
-			return true
+			ForceChartNames:   forceChartNames,
-		}
+			ForceInstall:      forceInstall,
+			IngressClass:      ingressClass,
+			InstalledRevision: latestRevision,
+		})
+		common.LogVerbose(decision.Reason)
 
-		if latestRevision.Version != chart.Version {
+		return decision.Install
-			common.LogVerbose(fmt.Sprintf("Installing chart due to version mismatch: %s != %s", latestRevision.AppVersion, chart.Version))
-			return false
-		}
-
-		common.LogVerbose("Skipping chart: already installed")
-		return false
 	})
 	if err != nil {
 		return fmt.Errorf("Unable to install helm charts: %w", err)

tests/unit/scheduler-k3s-ensure-charts.bats

@@ -0,0 +1,65 @@
+#!/usr/bin/env bats
+
+load test_helper
+
+setup() {
+  uninstall_k3s || true
+  global_setup
+  dokku nginx:stop
+  export KUBECONFIG="/etc/rancher/k3s/k3s.yaml"
+}
+
+teardown() {
+  global_teardown
+  dokku nginx:start
+  uninstall_k3s || true
+}
+
+@test "(scheduler-k3s:ensure-charts) upgrades an existing release to the pinned chart version" {
+  if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then
+    skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN"
+  fi
+
+  install_k3s
+
+  run /bin/bash -c "helm list -n traefik -o json | jq -r '.[] | select(.name == \"traefik\") | .chart'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "traefik-26.0.0"
+
+  run /bin/bash -c "helm list -n traefik -o json | jq -r '.[] | select(.name == \"traefik\") | .revision'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  local initial_revision="$output"
+
+  run /bin/bash -c "dokku scheduler-k3s:ensure-charts --charts traefik"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "helm list -n traefik -o json | jq -r '.[] | select(.name == \"traefik\") | .chart'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "traefik-26.0.0"
+
+  run /bin/bash -c "helm list -n traefik -o json | jq -r '.[] | select(.name == \"traefik\") | .revision'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  local upgraded_revision="$output"
+  [[ "$upgraded_revision" -gt "$initial_revision" ]]
+
+  run /bin/bash -c "dokku scheduler-k3s:ensure-charts"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "helm list -n traefik -o json | jq -r '.[] | select(.name == \"traefik\") | .revision'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output "$upgraded_revision"
+}