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

cli: fix version check for CSI chart #2209

Merged
merged 2 commits into from
Aug 11, 2023
Merged

Conversation

daniel-weisse
Copy link
Member

Context

Helm upgrades never checked if the CSI chart we want to upgrade to was equal to the version of the CLI.
In practice, this should have only occurred under very rare circumstances, if ever: constellationServices and the constellationOperators charts would have to be previously successfully upgrade to the version set in a users config file. The upgrade then had to be aborted before the CSI charts could be upgrade. If a user now changes their CLI version, constellation upgrade apply will upgrade the CSI chart to a different version than specified in the config.

Proposed change(s)

  • Add a new function isCLIVersionedRelease, to check if a given helm release is one of the released directly versioned by the CLI.
  • Fix the ordering of versions in error messages returned by the semver package

Checklist

  • Update docs
  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

Signed-off-by: Daniel Weiße <dw@edgeless.systems>
Signed-off-by: Daniel Weiße <dw@edgeless.systems>
@daniel-weisse daniel-weisse added the bug fix Fixing a bug label Aug 11, 2023
@daniel-weisse daniel-weisse added this to the v2.10.0 milestone Aug 11, 2023
@netlify

This comment was marked as spam.

@elchead elchead self-requested a review August 11, 2023 09:40
@elchead
Copy link
Contributor

elchead commented Aug 11, 2023

Should we version all charts (also the AWS load balancer) with the CLI version? If microservices means all charts managed by us, I think so.

@daniel-weisse
Copy link
Member Author

Should we version all charts (also the AWS load balancer) with the CLI version?

No, since not all charts follow our release cycle.
Cilium, cert-manager, and the AWS load balancer are pulled in from an upstream not controlled by us.
Overriding the version number for these charts makes no sense for two reasons:

  1. The user loses the ability to accurately determine what real version of Cilium, cert-manager etc. is running in their cluster
  2. We have to run upgrades for these charts even though nothing has actually changed
    • For example: We use Cilium at version v1.5.0, we now start overriding that version with our own Constellation release version, lets say v2.12.0. When upgrading from v.2.12.0 to v2.12.1, we have to upgrade Cilium, because the versions say so, when in reality Cilium is still running at version v1.5.0, and nothing should have changed

@daniel-weisse daniel-weisse merged commit 715cc1f into main Aug 11, 2023
5 checks passed
@daniel-weisse daniel-weisse deleted the fix/cli/csi-version-check branch August 11, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants