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: fail fast when CLI and Constellation versions don't match #1972

Merged
merged 7 commits into from
Jun 27, 2023

Conversation

elchead
Copy link
Contributor

@elchead elchead commented Jun 27, 2023

Context

We don't support creating e.g. a 2.7.1 Constellation with e.g. the 2.8.0. CLI.
More precisely: The to-be-created Constellation version must always match the CLI's version, down to the patch version.

We don't want to forbid using/loading an old config since this is needed for e.g. the upgrade process.
But we want to fail fast if the user tries to constellation create or constellation init a Constellation.

Mismatches can be found by comparing the CLI's version against the microserviceVersion and image version of the config. Then we want to fail hard on constellation create and constellation init with a corresponding error which states that there is a mismatch and that the corresponding version to the config must be downloaded or the version in the config must be updated.

Creating a Constellation v2.7.1 (nodes and microservices) with a v2.8.0 CLI (Darwin, ARM64) gives the following errror:

➜  demo constellation config fetch-measurements 
➜  demo constellation create -c 1 -w 2 -y   
Creating  
Your Constellation cluster was created successfully.
➜  demo constellation init               
Using community license.
Unable to contact license server.
Please keep your vCPU quota in mind.
Your Constellation master secret was successfully written to ./constellation-mastersecret.json
Note: If you just created the cluster, it can take a few minutes to connect.
Connecting   
Initializing cluster   
Cluster initialization failed. This error is not recoverable.
Terminate your cluster and try again.
The cluster logs were saved to "constellation-cluster.log"
Error: rpc error: code = Internal desc = grpc: failed to unmarshal the received message: proto: cannot parse invalid wire-format data

Proposed change(s)

  • add a validation check for create and init to fail early when the versions don't match.
./constellation create --control-plane-nodes 1 --worker-nodes 1 -y
Error: cli version "v2.9.0-pre.0.20230627092219-e741e8cb5c8e" does not match microservice version "v2.8.0" or image "v2.8.0"

Related issue

Checklist

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

@netlify
Copy link

netlify bot commented Jun 27, 2023

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 3f748c4
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/649b0809bd3a6f0008478d92

@elchead elchead added the bug fix Fixing a bug label Jun 27, 2023
@elchead
Copy link
Contributor Author

elchead commented Jun 27, 2023

Should this be a bug fix or no changelog?

@elchead elchead added this to the v2.9.0 milestone Jun 27, 2023
@elchead elchead marked this pull request as ready for review June 27, 2023 09:29
@elchead elchead marked this pull request as draft June 27, 2023 09:33
@elchead elchead marked this pull request as ready for review June 27, 2023 10:10
@daniel-weisse
Copy link
Member

bug fix seems fine to me

@elchead
Copy link
Contributor Author

elchead commented Jun 27, 2023

is it correct that the microservice version should be exactly equal to the CLI and the image version only major minor equal (ignoring the commit hash suffix) @malt3

@malt3
Copy link
Contributor

malt3 commented Jun 27, 2023

is it correct that the microservice version should be exactly equal to the CLI and the image version only major minor equal (ignoring the commit hash suffix)

Here is the true canonical answer to that, which we could document somewhere. Not sure what the best place would be:

Target microservice version (the version that is referenced in the config during constellation init or referenced as target version during upgrade apply) must match the CLI version exactly (down to the version suffix). When planning an upgrade, the source version (current version before the upgrade is applied) may be up to one minor version older (allow upgrades from v2.8.x to v2.9.x).
The reason is this: The CLI embeds exactly one version of the helm charts (the version of the CLI). So whenever we deploy the helm charts, we can only use the embedded chart version.
The source version can only be older by one minor version to ensure we can perform safe upgrades where we can reasonably test what microservice versions will interact with other versions of CLI, OS images, other microservices.

Target kubernetes version (the version that is referenced in the config during constellation init or referenced as target version during upgrade apply) must match one of the embedded k8s versions. We have a hardcoded list that is embedded in the CLI.
When planning an upgrade, the source k8s version may be up to one minor version older (allow upgrades from k8s 1.25.x to 1.26.x).
The reason is this: The CLI embeds components for kubernetes version for the hardcoded list of supported k8s versions. So whenever we deploy a k8s version (during constellation init or constellation upgrade apply), the k8s components for the target version need to be available.
The source version drift is a general k8s recommendation: k8s was designed to withstand a version drift of one minor version in each component during a rolling upgrade.

Target image version (the version that is referenced in the config during constellation init or referenced as target version during upgrade apply) must have the same major and minor version (but can have a different patch or suffix) when upgrading compared to the cli. Examples: CLI v2.8.2 could correctly upgrade to OS image v2.8.1 or v2.8.2-nightly-foo-bar.
When planning an upgrade, the source image version may be up to one minor version older (allow upgrades from v2.8.x to 2.9.x).
The reason is this: The CLI uses an API endpoint to query configuration and measurements for a configured target image. This generally allows for arbitrary image versions to be referenced/deployed by the CLI. However, to ensure safe upgrades, we always upgrade by at most one minor version and ensure that the CLI is at the same minor version that we target.
This allows us to test upgrades with only a reasonable amount of moving parts.
Patch version upgrades of OS images are not allowed to have any breaking changes. They are designed to deliver bug and security fixes only and should otherwise be 1:1 compatible.

cli/internal/cmd/create.go Outdated Show resolved Hide resolved
Copy link
Contributor

@malt3 malt3 left a comment

Choose a reason for hiding this comment

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

nit. Otherwise lgtm

@elchead
Copy link
Contributor Author

elchead commented Jun 27, 2023

is it correct that the microservice version should be exactly equal to the CLI and the image version only major minor equal (ignoring the commit hash suffix)

Here is the true canonical answer to that, which we could document somewhere. Not sure what the best place would be:

Target microservice version (the version that is referenced in the config during constellation init or referenced as target version during upgrade apply) must match the CLI version exactly (down to the version suffix). When planning an upgrade, the source version (current version before the upgrade is applied) may be up to one minor version older (allow upgrades from v2.8.x to v2.9.x). The reason is this: The CLI embeds exactly one version of the helm charts (the version of the CLI). So whenever we deploy the helm charts, we can only use the embedded chart version. The source version can only be older by one minor version to ensure we can perform safe upgrades where we can reasonably test what microservice versions will interact with other versions of CLI, OS images, other microservices.

Target kubernetes version (the version that is referenced in the config during constellation init or referenced as target version during upgrade apply) must match one of the embedded k8s versions. We have a hardcoded list that is embedded in the CLI. When planning an upgrade, the source k8s version may be up to one minor version older (allow upgrades from k8s 1.25.x to 1.26.x). The reason is this: The CLI embeds components for kubernetes version for the hardcoded list of supported k8s versions. So whenever we deploy a k8s version (during constellation init or constellation upgrade apply), the k8s components for the target version need to be available. The source version drift is a general k8s recommendation: k8s was designed to withstand a version drift of one minor version in each component during a rolling upgrade.

Target image version (the version that is referenced in the config during constellation init or referenced as target version during upgrade apply) must have the same major and minor version (but can have a different patch or suffix) when upgrading compared to the cli. Examples: CLI v2.8.2 could correctly upgrade to OS image v2.8.1 or v2.8.2-nightly-foo-bar. When planning an upgrade, the source image version may be up to one minor version older (allow upgrades from v2.8.x to 2.9.x). The reason is this: The CLI uses an API endpoint to query configuration and measurements for a configured target image. This generally allows for arbitrary image versions to be referenced/deployed by the CLI. However, to ensure safe upgrades, we always upgrade by at most one minor version and ensure that the CLI is at the same minor version that we target. This allows us to test upgrades with only a reasonable amount of moving parts. Patch version upgrades of OS images are not allowed to have any breaking changes. They are designed to deliver bug and security fixes only and should otherwise be 1:1 compatible.

Thanks for the extensive answer which I definitely want to document.
Actually think that the version matrix story should document this in short form without explanations in AB#3198

@elchead
Copy link
Contributor Author

elchead commented Jun 27, 2023

Putting this into the dev-docs for now in a follow up commit

@elchead
Copy link
Contributor Author

elchead commented Jun 27, 2023

@malt3 is the new doc place fine?

@malt3
Copy link
Contributor

malt3 commented Jun 27, 2023

Yes! thank you.

@elchead elchead merged commit 1edbe96 into main Jun 27, 2023
5 of 6 checks passed
@elchead elchead deleted the fix/cli/mismatched-versions branch June 27, 2023 16:24
@derpsteb
Copy link
Member

relabeling to put this into the "Other changes" category. Feels more like a UX improvement than a bug fix.

@derpsteb derpsteb removed the bug fix Fixing a bug label Jul 11, 2023
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

4 participants