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

More granular releases in YAML, for example "v1.5.3" instead of "v1.5" #679

Closed
max-rocket-internet opened this issue Oct 25, 2019 · 9 comments

Comments

@max-rocket-internet
Copy link
Contributor

If you are going to update the release YAML files in confg for minor releases, for example updating config/v1.5/aws-k8s-cni.yaml to v1.5.4, then it needs to be bulletproof.

But it clearly is NOT.

The result of this is not just things like that bug, but also we end up with different versions of the CNI running depending on when we updated it. Even though we are under the impression it's the same as it's version v1.5. I would say this is bad, kind of like using the latest docker image tag.

To support my point, look at the notes under minor releases, e.g. v1.52, those are major changes: https://github.com/aws/amazon-vpc-cni-k8s/releases/tag/v1.5.2

So, I am asking for release YAML in confg for the minor releases 🙂

@jaypipes
Copy link
Contributor

@max-rocket-internet Given the recent kerfuffle with 1.5.4, I understand your frustration. How do you propose we handle the minor release YAMLs, though? Within the config/v1.5 directory, we have a v1.5.2-aws-k8s-cni.yaml file, a v1.5.3-aws-k8s-cni.yaml file, a v1.5.4-aws-k8s-cni.yaml file, etc, each mostly a copy of the other files with the image version changed? That seems like it could quickly get cumbersome and introduce another vector for copy/pasta mistakes. That said, we could rename aws-k8s-cni.yaml to aws-k8s-cni-latest.yaml and switch the image tag to 'latest' to make it clearer that the version of the image used in that manifest changes.

We could add some documentation that explains how to install the CNI plugin pinned to a specific minor version (essentially replacing "latest" with a specific pinned image version). Would that be a reasonable solution?

@mogren
Copy link
Contributor

mogren commented Oct 25, 2019

@max-rocket-internet Good point. One option would be to have config/v1.5/aws-k8s-cni.yaml in the master branch to always point to the same minor version as the default for all new EKS clusters. That way, new patch releases would stay in the release-branches when they are published, and only be updated when we actually switch over the default.

In regards to v1.5.4, we have worked a lot to improve our test cases to make sure it doesn't happen again. I'll do a full write up of how these changes caused random ip rules to be deleted.

@max-rocket-internet
Copy link
Contributor Author

Within the config/v1.5 directory, we have a v1.5.2-aws-k8s-cni.yaml file, a v1.5.3-aws-k8s-cni.yaml file, a v1.5.4-aws-k8s-cni.yaml...

No there isn't 😅 That's kind of my point, look: https://github.com/aws/amazon-vpc-cni-k8s/tree/3bc194dba294421fa2cc469f061c044f807928e5/config/v1.5

There's only 3 files and it's not clear what minor release is used there.

That seems like it could quickly get cumbersome and introduce another vector for copy/pasta mistakes.

There will always be a lot of duplicated text, especially with k8s YAML, but a solid release process with automation should prevent "copy/pasta" mistakes.

and switch the image tag to 'latest' to make it clearer that the version of the image used

You should never use latest tag, otherwise you will get different versions of the CNI running in the same cluster ⚠

We could add some documentation that explains how to install the CNI plugin pinned to a specific minor version

100%. I think this is the main problem now. I go to the AWS docs and run the command to install version v1.5 today. But next week if I do that same process, it could be a different version. And this isn't obvious.

In regards to v1.5.4, we have worked a lot to improve our test cases to make sure it doesn't happen again

Nice 😃

One option would be to have config/v1.5/aws-k8s-cni.yaml in the master branch to always point to the same minor version as the default for all new EKS clusters
That way, new patch releases would stay in the release-branches when they are published, and only be updated when we actually switch over the default.

I think this would be better, yes for sure.

But for me, I need an immutable release to point to when doing an update. We roll out changes like CNI to each of our clusters, one by one, going from least to most important. So it's essential to have version that we can point to that is immutable as the time between doing first and last clusters could be 1-2 months. So now I would ignore the AWS documentation is use the release tag, e.g. https://raw.githubusercontent.com/aws/amazon-vpc-cni-k8s/v1.5.3/config/v1.5/aws-k8s-cni.yaml

I still think AWS should move to Helm for managing their components. As I've suggested again and again 🙂

@jaypipes
Copy link
Contributor

Within the config/v1.5 directory, we have a v1.5.2-aws-k8s-cni.yaml file, a v1.5.3-aws-k8s-cni.yaml file, a v1.5.4-aws-k8s-cni.yaml...

No there isn't sweat_smile That's kind of my point, look: https://github.com/aws/amazon-vpc-cni-k8s/tree/3bc194dba294421fa2cc469f061c044f807928e5/config/v1.5

I was asking you whether adding a separate file for each minor version is what you wanted. I wasn't saying that is what exists now.

@max-rocket-internet
Copy link
Contributor Author

I was asking you whether adding a separate file for each minor version is what you wanted.

Ahh sorry, I misunderstood! Yes, that would also solve the problem. But the files need to be immutable, i.e. only updated if there was a typo or something. Or we use file links that belong to a specific tag.

@jaypipes
Copy link
Contributor

and switch the image tag to 'latest' to make it clearer that the version of the image used

You should never use latest tag, otherwise you will get different versions of the CNI running in the same cluster ⚠

Yes, I understand that. I was asking you whether renaming the YAML file to aws-k8s-cni-latest.yaml would make it more clear to users that wish to have an immutable deployment artifact to avoid that YAML file.

We could add some documentation that explains how to install the CNI plugin pinned to a specific minor version

100%. I think this is the main problem now. I go to the AWS docs and run the command to install version v1.5 today. But next week if I do that same process, it could be a different version. And this isn't obvious.

Right, thus my question about renaming to aws-k8s-cni-latest.yaml to make that much more obvious :)

I still think AWS should move to Helm for managing their components. As I've suggested again and again slightly_smiling_face

Check this out: https://github.com/aws/eks-charts/

I think it would be good to put a Helm chart for the AWS VPC CNI plugin in there. Thoughts?

@max-rocket-internet
Copy link
Contributor Author

Right, thus my question about renaming to aws-k8s-cni-latest.yaml to make that much more obvious :)

OK I get you, yes, that would be better.

Check this out: https://github.com/aws/eks-charts/
I think it would be good to put a Helm chart for the AWS VPC CNI plugin in there. Thoughts?

Awesome! Can I make a PR?

@jaypipes
Copy link
Contributor

Of course you can! :) We'd be more than happy to review.

@max-rocket-internet
Copy link
Contributor Author

Done: aws/eks-charts#27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants