fix(api): use correct helm client k8s version #2751
Conversation
| return nil | ||
| } | ||
|
|
||
| k8sVersion, err := m.getK8sVersion() |
There was a problem hiding this comment.
edge case: this logic can break later on upgrades if we start to deploy the app first before upgrading infra. for now it's fine
There was a problem hiding this comment.
why would we install the app before upgrading the infra? is this how it works today? i could see running the app preflights before installing the infra.
There was a problem hiding this comment.
Ah preflights is a good point. No that's not how it works today, we deploy infra first today
| return manifests, nil | ||
| } | ||
|
|
||
| func (m *appReleaseManager) setupHelmClient() error { |
There was a problem hiding this comment.
i think we should break this template.go file down later on, it feels overloaded now
There was a problem hiding this comment.
moved this to the util.go file
| appreleasemanager.WithReleaseData(controller.releaseData), | ||
| appreleasemanager.WithLicense(license), | ||
| appreleasemanager.WithPrivateCACertConfigMapName(controller.privateCACertConfigMapName), | ||
| appreleasemanager.WithK8sVersion(controller.k8sVersion), |
There was a problem hiding this comment.
this is only passing the version to the release manager, not the install manager. is that correct?
There was a problem hiding this comment.
Hmm does the install manager assume a linux target and uses the k0s version? If so then it should be fixed to follow the pattern in this PR.
There was a problem hiding this comment.
updated so they both use it
|
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer: Airgap Installer (may take a few minutes before the airgap bundle is built): Happy debugging! |
| } | ||
|
|
||
| k8sVersion := m.k8sVersion | ||
| if k8sVersion == "" { |
There was a problem hiding this comment.
i feel this is going to be hard to debug if the caller forgets to pass the k8s version. what if we make it a required option?
emosbaugh
deleted the
emosbaugh/sc-128387/set-k0s-version-when-rendering-helm-chart
branch
* fix(api): use correct helm client k8s version * f * use embedded k0s version for linux installs * use embedded k0s version for linux installs * pass in k8s version to app release manager when target is kubernetes * f
* fix(api): use correct helm client k8s version * f * use embedded k0s version for linux installs * use embedded k0s version for linux installs * pass in k8s version to app release manager when target is kubernetes * f
What this PR does / why we need it:
App release manager needs to use a helm client with the k8s version so that it can properly render templates using KubeVersion helm built-in.
Additionally, app install manager is using the incorrect k8s version when instantiating the helm client for target=kubernetes. It is using the version from the build when it should be using the version discovered from the cluster. This makes it so the version is always retrieved from the cluster.
Which issue(s) this PR fixes:
Does this PR require a test?
Does this PR require a release note?
Does this PR require documentation?