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

ci(test): add upgrade strategy matrix for helm tests #201

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Sep 23, 2024

Related to #192

Description of changes

Added a matrix to specify upgrade strategy. I think this would help us test all supported upgrade paths.

Notes

This does not fix the test fail, as we must resolve #200. See #201 (comment)

It would have been nicer to specifies helm-extra-args to include/overwrite arguments for helm upgrade. But:

  • The upgrade strategy precedence: reset-then-reuse-values < reuse-values < reset-values. So, specifying --reset-then-reuse-values --reuse-values will just pick up reuse-values (incorrect).

Thus, it is a bit inconvenient to have a wrapper script :D

@tthvo tthvo added chore Refactor, rename, cleanup, etc. ci safe-to-test labels Sep 23, 2024
@tthvo tthvo changed the title ci(test): add upgrade strategy matrix fort helm tests ci(test): add upgrade strategy matrix for helm tests Sep 23, 2024
@tthvo tthvo requested a review from a team September 23, 2024 22:43
@tthvo tthvo force-pushed the test-ci branch 3 times, most recently from 85f5a95 to 1404e69 Compare September 24, 2024 00:51
@tthvo tthvo marked this pull request as draft September 24, 2024 01:30
@tthvo tthvo marked this pull request as ready for review September 24, 2024 02:37
@tthvo
Copy link
Member Author

tthvo commented Sep 24, 2024

Sorry for the noises from CI fails :D I took the chance to validate the behaviour of helm upgrade --force here, but unfortunately I misunderstood it for the helm v2 behaviour.

In helm v3, helm upgrade --force will work similarly to kubectl replace, which will still fail on immutable field changes. I think we do need #200 for the upgrade to work at all.

@tthvo
Copy link
Member Author

tthvo commented Sep 24, 2024

In helm v3, helm upgrade --force will work similarly to kubectl replace, which will still fail on immutable field changes. I think we do need #200 for the upgrade to work at all.

I guess #200 would help allow a nice and seamless upgrade for users. Though, there are other ways to upgrade that requires a bit of work. Here are some ways I tested that worked.

1. Delete the release but keep the release history & install with replace

$ helm uninstall --keep-history <release-name>
$ helm install --replace <release-name> <chart-source>

All resources of previous release is removed but the release history is kept intact. However, rollback would fail (i.e. immutable field).

2. Delete the main deployment & upgrade

kubectl -n <release-namespace> delete deployment/<main-deployment>
helm upgrade <release-name> <chart-source>

Since the deployment is deleted, helm will recreate it when upgrading. Again, rollback would fail unless the current deployment is deleted.

I came across this in the Prometheus chart: https://github.com/prometheus-community/helm-charts/blob/main/charts/prometheus/README.md#to-220

3. Rename the deployment

This is the idea of #200 but I guess it is simpler since we can append -main to the main deployment. Rollback should work as the deployment is recreated.

All above ways are, like you said, one time temporary workaround. #200 is still nicest way around :D

@andrewazores andrewazores merged commit 12879aa into cryostatio:separate-db-storage Sep 24, 2024
5 checks passed
@tthvo tthvo deleted the test-ci branch September 24, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. ci safe-to-test
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants