Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Construct Helm actions using Helm env helpers #4515

Merged
merged 1 commit into from
May 29, 2024

Conversation

jnummelin
Copy link
Member

@jnummelin jnummelin commented May 29, 2024

Description

This way we get the retrying round-tripper setup and Helm now retries some known transient errors such as the etcd leader change.

Fixes #3758

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

This way we get the retrying round-tripper setup and Helm now retries some known transient errors
such as the etcd leader change.

Fixes k0sproject#3758

Signed-off-by: Jussi Nummelin <[email protected]>
@@ -76,6 +78,14 @@ func NewCommands(k0sVars *config.CfgVars) *Commands {
}

func (hc *Commands) getActionCfg(namespace string) (*action.Configuration, error) {
// Construct new helm env so we get the retrying roundtripper etc. setup
// See https://github.com/helm/helm/pull/11426/commits/b5378b3a5dd435e5c364ac0cfa717112ad686bd0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also include a link to the request to make the retrying roundtripper public: helm/helm#13052.

If that's public, we could even consider to use this in k0s's kube clients as well 🤔
And we could add this depending whether we use etcd or not.

@jnummelin jnummelin marked this pull request as ready for review May 29, 2024 12:47
@jnummelin jnummelin requested a review from a team as a code owner May 29, 2024 12:47
@jnummelin jnummelin added backport/release-1.27 PR that needs to be backported/cherrypicked to release-1.27 branch backport/release-1.28 PR that needs to be backported/cherrypicked to release-1.28 branch backport/release-1.29 PR that needs to be backported/cherrypicked to the release-1.29 branch backport/release-1.30 PR that needs to be backported/cherrypicked to the release-1.30 branch labels May 29, 2024
@jnummelin jnummelin merged commit f37ce96 into k0sproject:main May 29, 2024
87 checks passed
@jnummelin jnummelin deleted the fix/helm-retry-roundtripper branch May 29, 2024 13:03
@k0s-bot
Copy link

k0s-bot commented May 29, 2024

Successfully created backport PR for release-1.27:

@k0s-bot
Copy link

k0s-bot commented May 29, 2024

Successfully created backport PR for release-1.28:

@k0s-bot
Copy link

k0s-bot commented May 29, 2024

Successfully created backport PR for release-1.29:

@k0s-bot
Copy link

k0s-bot commented May 29, 2024

Successfully created backport PR for release-1.30:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm backport/release-1.27 PR that needs to be backported/cherrypicked to release-1.27 branch backport/release-1.28 PR that needs to be backported/cherrypicked to release-1.28 branch backport/release-1.29 PR that needs to be backported/cherrypicked to the release-1.29 branch backport/release-1.30 PR that needs to be backported/cherrypicked to the release-1.30 branch bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm reconciler aborts and uninstalls chart when installing multi-node
3 participants