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

fix(e2e): Upgrade helm procedure test #4927

Merged
merged 3 commits into from Nov 30, 2023

Conversation

gansheer
Copy link
Contributor

@gansheer gansheer commented Nov 23, 2023

Ref #4923

Add helm upgrade test using manual upgrade of CRDs and update the documentation.

Release Note

fix: Helm upgrade procedure with CRD management

@gansheer gansheer force-pushed the fix/4923_helm_upgrade_procedure branch from ff40e1e to 20213e4 Compare November 24, 2023 08:45
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

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

I wonder if we have run any test with the --force option. Even in the install instead of the upgrade, ie, to reinstall the operator from scratch with the new set of CRDs.

If you are upgrading the `camel-k` Deployment, you should always use a specific version of the chart and pre-install the CRDS:

```
$ kubectl replace -k "https://github.com/apache/camel-k/config/crd?ref=vx.y.z"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use the CRDs contained into the chart for consistency.

Copy link
Contributor Author

@gansheer gansheer Nov 24, 2023

Choose a reason for hiding this comment

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

I am not sure about this. The charts also contains the edit cluster role, so it would replacing them be one by one instead of the kustomize replace.
AFAIK the CRDs in the charts should always be identical to the ones in config for one version. Also I wonder if we should improve the sync between both versions if we want consistency, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The procedure (and the test) are now using the chart CRDs files.

@gansheer gansheer marked this pull request as ready for review November 24, 2023 12:23
@gansheer gansheer force-pushed the fix/4923_helm_upgrade_procedure branch from aa21346 to 03bea6b Compare November 28, 2023 09:38
@gansheer gansheer force-pushed the fix/4923_helm_upgrade_procedure branch from 03bea6b to 46fcbf5 Compare November 28, 2023 16:11
@squakez squakez merged commit e73adbe into apache:main Nov 30, 2023
11 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants