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

bug: retry saving helm chart object status #4064

Conversation

ricardomaraschini
Copy link
Contributor

Description

if we fail to save the status, specially after installing the chart, we will try to install the chart again during the next reconcile (its Status.Status.ReleaseName will be empty). this pr attempts to mitigate this in two ways:

  • before updating fetch the newest version of the chart.
  • retry if it is still on conflict.

this may be overkill as it most likely won't fail after we fetch an updated version of the chart but given the importance of this operation we better be safe than sorry.

when this bug this solves happens this is what is reported in the chart status:

can't install loadedChart ```chart-name```: cannot re-use a name that is
still in use

Mitigates #4047

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

if we fail to save the status, specially after installing the chart, we
will try to install the chart again during the next reconcile (its
Status.Status.ReleaseName will be empty). this pr attempts to mitigate
this in two ways:

- before updating fetch the newest version of the chart.
- retry if it is still on conflict.

this may be overkill as it most likely won't fail after we fetch an
updated version of the chart but given the importance of this operation
we better be safe than sorry.

when this bug this solves happens this is what is reported in the chart
status:

```
can't install loadedChart ```chart-name```: cannot re-use a name that is
still in use
```

Signed-off-by: Ricardo Maraschini <ricardo.maraschini@gmail.com>
@juanluisvaladas
Copy link
Contributor

Thanks for the contribution @ricardomaraschini and sorry for taking this long to review this.

@juanluisvaladas juanluisvaladas merged commit e9e7e98 into k0sproject:main Mar 4, 2024
73 checks passed
@juanluisvaladas juanluisvaladas added backport/release-1.26 PR that needs to be backported/cherrypicked to release-1.26 branch 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 labels Mar 4, 2024
@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.26 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Validation Failed: {"resource":"PullRequest","code":"custom","message":"A pull request already exists for k0sproject:backport-4064-to-release-1.26."}

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Successfully created backport PR for release-1.26:

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.27 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Successfully created backport PR for release-1.27:

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Successfully created backport PR for release-1.27:

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.28 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Successfully created backport PR for release-1.28:

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.28 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.29 with exitcode 1

1 similar comment
@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.29 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Successfully created backport PR for release-1.29:

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.26 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.27 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.28 with exitcode 1

@k0s-bot
Copy link

k0s-bot commented Mar 4, 2024

Git push to origin failed for release-1.29 with exitcode 1

@twz123 twz123 added bug Something isn't working area/helm and removed backport/release-1.26 PR that needs to be backported/cherrypicked to release-1.26 branch 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 labels Mar 11, 2024
@ricardomaraschini ricardomaraschini deleted the retry-to-save-helm-chart-object branch May 15, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm backport/release-1.29 PR that needs to be backported/cherrypicked to the release-1.29 branch bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants