Use Helm binary instead of the Go SDK to manage charts #2770

sgalsaleh · 2025-08-28T22:14:42Z

What this PR does / why we need it:

Uses the Helm binary instead of the Go SDK to manage Helm charts

Which issue(s) this PR fixes:

Does this PR require a test?

Yes

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

…instead-of-the-go-sdk-to

github-actions · 2025-09-02T18:32:35Z

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-d458620" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-d458620?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

emosbaugh · 2025-09-02T20:58:58Z

versions.mk

@@ -25,6 +25,9 @@ K0S_VERSION = $(K0S_VERSION_1_$(K0S_MINOR_VERSION))
 # Format: K0S_BINARY_SOURCE_OVERRIDE_<MINOR_VERSION>
 # Example: K0S_BINARY_SOURCE_OVERRIDE_32 = https://github.com/k0sproject/k0s/releases/download/v1.32.7+k0s.0/k0s-v1.32.7+k0s.0-amd64

+# Helm Version
+HELM_VERSION = v3.18.6


should you add automation to update this to the dependencies.yaml github actions workflow?

emosbaugh · 2025-09-02T21:03:21Z

api/controllers/kubernetes/install/controller_test.go

@@ -90,6 +90,8 @@ func TestGetInstallationConfig(t *testing.T) {
 		},
 	}

+	t.Setenv("HELM_BINARY_PATH", "helm") // use the helm binary in PATH


Does this mean i need helm install locally to run unit tests?

yes, any other ideas? helm binary is currently a requirement for the dev env anyways.

if you were to instantiate only one client as suggested you would not have to set the env var

emosbaugh · 2025-09-02T21:07:20Z

pkg/helm/client.go

+		// used in tests so that we don't pass the helm binary path all over the place
+		return hp, nil
+	}
+	hp, err := goods.Binary("helm")


would it be possible to pass around a HelmClient rather than an EnvSettings so you dont have to use the goods package outside of the cli? Looks like you would have to factor out logging into the call to the client functions.

makes sense. i will update so there's a single helm client used by the cli / api.

hmm this will make it hard to override the log function for the helm client and other operation specific fields.

what if i pass the helm binary path explicitly from callers instead of using the goods package?

If you didnt want to add the log as an argument to all methods, could you add the log to the context? Similar to this https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/log

emosbaugh · 2025-09-02T21:28:32Z

pkg/helm/client.go

-	chartRequested, err := loader.Load(localPath)
-	if err != nil {
-		return nil, fmt.Errorf("load: %w", err)
+	// Add all helm CLI flags from kubernetesEnvSettings


could you move addKubernetesCLIFlags to this package and export it or make this dynamic?

can you elaborate? what do you mean by making it dynamic?

i would prefer that the add flags and add env args were just moved closer in the code, perhaps both in this package. if not, it may be possible to use flagset.Visit to iterate over the flags and construct the cli args

emosbaugh · 2025-09-02T21:29:43Z

pkg/helm/client.go

+	if h.airgapPath != "" {
+		// Use chart from airgap path
+		return filepath.Join(h.airgapPath, fmt.Sprintf("%s-%s.tgz", releaseName, chartVersion)), nil
+	}


this never felt like it belonged in this package. perhaps something to address later

emosbaugh · 2025-09-02T21:45:20Z

cmd/installer/cli/install.go

-	restConfig, err := flags.kubernetesEnvSettings.RESTClientGetter().ToRESTConfig()
+	restConfig, err := clientcmd.BuildConfigFromFlags("", flags.kubernetesEnvSettings.KubeConfig)


why change this? is it equivalent? what if i dont explicitly set the kubeconfig as a flag?

emosbaugh · 2025-09-02T21:49:16Z

pkg/helm/client.go

-	LogFn            action.DebugLog
+	HelmPath              string
+	KubernetesEnvSettings *helmcli.EnvSettings
+	K8sVersion            string


is K8sVersion no longer used? Is this going to be an issue for things like buildtools or rendering before install k0s? If not can you remove it?

I see this is only used for render. should it be an argument to that function? or is that difficult?

Use Helm binary instead of the Go SDK to manage charts

308f730

sgalsaleh force-pushed the salah/sc-128058/use-helm-binary-instead-of-the-go-sdk-to branch from a2a1610 to 308f730 Compare August 29, 2025 23:20

sgalsaleh added 2 commits August 29, 2025 20:17

Merge branch 'milestone-7-redo' into salah/sc-128058/use-helm-binary-…

c468582

…instead-of-the-go-sdk-to

pass version to helm show command

1f5e68f

sgalsaleh added 2 commits September 2, 2025 13:24

add helm to operator image

660e317

fix tests

75554c5

sgalsaleh marked this pull request as ready for review September 2, 2025 20:56

sgalsaleh requested a review from emosbaugh September 2, 2025 20:56

This comment was marked as outdated.

Sign in to view

emosbaugh reviewed Sep 2, 2025

View reviewed changes

sgalsaleh added 7 commits September 3, 2025 09:37

feedback

2a20b7c

feedback

b62ca4d

fix web lint

fd1e5bb

remove todo

274df3f

fix tests and logs

cdbf5ce

move env flags

659ab3a

add helm log prefix

c0296f9

emosbaugh approved these changes Sep 3, 2025

View reviewed changes

sgalsaleh added 5 commits September 3, 2025 20:35

try fixing integration test

e6bdbe1

return context error

23fa025

one more try

1da7636

revert

79ba2bc

debug logs

f8c9b38

This comment was marked as outdated.

Sign in to view

debug logs

5ee5bcf

sgalsaleh force-pushed the salah/sc-128058/use-helm-binary-instead-of-the-go-sdk-to branch from 999881f to 5ee5bcf Compare September 4, 2025 16:10

sgalsaleh added 2 commits September 4, 2025 09:15

no-op

accb891

one more try

b782156

This comment was marked as outdated.

Sign in to view

sgalsaleh added 9 commits September 4, 2025 09:42

use helm history

e5b7a8b

override cmd cancel function

bc7f393

auto rollback

d2f4a72

remove modify cmd

1bf3835

tests

091260d

put log back

6845b48

remove debug log

aa3b01c

remove debug log

4eae495

err msg

d07ecfa

This comment was marked as outdated.

Sign in to view

sgalsaleh added 3 commits September 4, 2025 14:28

err msgs

69c69b2

refactor

ae78ea9

rename

65bfda3

emosbaugh approved these changes Sep 4, 2025

View reviewed changes

sgalsaleh added 3 commits September 5, 2025 08:20

set helm HOME env vars

ffefb6c

rename tmp dir

dba5fec

update comment

d458620

emosbaugh approved these changes Sep 5, 2025

View reviewed changes

sgalsaleh merged commit 08bd1f9 into milestone-7-redo Sep 5, 2025
163 of 172 checks passed

sgalsaleh deleted the salah/sc-128058/use-helm-binary-instead-of-the-go-sdk-to branch September 5, 2025 18:28

		restConfig, err := flags.kubernetesEnvSettings.RESTClientGetter().ToRESTConfig()
		restConfig, err := clientcmd.BuildConfigFromFlags("", flags.kubernetesEnvSettings.KubeConfig)

Use Helm binary instead of the Go SDK to manage charts #2770

Use Helm binary instead of the Go SDK to manage charts #2770

Uh oh!

Conversation

sgalsaleh commented Aug 28, 2025

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Does this PR require documentation?

Uh oh!

github-actions bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emosbaugh Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 2, 2025 •

edited

Loading

emosbaugh Sep 3, 2025 •

edited

Loading