-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
k3s: add multiple versions #214026
k3s: add multiple versions #214026
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely awesome! ❤️
Do you think this could be backported to 22.11? To ease migration from 22.05... Otherwise I can just overlay them, and the problem will be fixed for at least 23.05 onward. |
Back-porting is always possible, with fixes - if necessary.
Up to you. -- Personally, I'd rather spend time moving things forward than backwards. |
Makes sense indeed. I'll just overlay what I need. Thanks for the help! 😊 |
Do we really need all four versions? Especially, 1.23 with less than a month of support looks pretty pointless to me. How about reducing the set to the actively maintained ones?
That does resonate with me quite a bit. |
I have no strong opinion on this.
Release 22.05 is pinned to 1.23.
I wouldn't mind reducing the set. Will remove 1.23 then. Can follow this. |
I think that yes, we need 1.23. Quoting from #213943:
However, upstream's latest for that minor version is v1.23.16+k3s1. I can add myself as maintainer to help you after this is merged, and propose the upgrades. |
I'm okay keeping 1.23 since it's in "maintenance mode" for a couple weeks yet. If, hypothetically, there were a CVE that were patched for 1.23, and someone were using an older nixpkgs with an insecure 1.23, it would be safest for them to update to the latest patch version of 1.23 to be secure. I think that's a reasonable enough argument for keeping it around, and then dropping it once it won't see security releases (admittedly, in 3 weeks). I, personally, expect that going forward it won't be much of a burden to maintain a few extra older versions. Typically, maintenance work comes with upstream packaging changes, and those shouldn't happen for release branches, except in unusual cases. I'll leave a separate review comment regarding the actual PR, but figured this discussion about what to keep would be easier to respond to separately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this @superherointj! This definitely seems helpful to have, and like a good first step. As you said, this is very much not fancy (no DRY, no automatic-update-script-ing for old releases, etc), but this is still an improvement to the current state of things I think.
I think there are a few things that we can weigh here as for this approach. If you don't mind, I'll just kinda dump my thoughts here.
Basically, there's two motivating reasons we should keep around supported versions:
- To allow people to correctly update their clusters (i.e. step forward the api server, then the kubelet, without having to use different nixpkgs versions across the cluster)
- To allow people to consume patches for bugs and security issues without having to do more difficult upgrades (i.e. to consume a CVE fix, without also having to deal with k8s deprecations, by letting them stick on the same minor version)
This PR, if you follow k8s's recommendations exactly, actually does neither of those things. 2 obviously requires also spending time updating the patch version for old releases.
1 would work in practice, but K8s docs tell you to update to the latest patch version before introducing minor version number skew... again, in reality, it rarely matters.
The argument against this, I think, is that introducing old patch versions is kinda providing an "attractive nuisance", in that it lets people think they might be getting 1 & 2, but in reality they get sorta-1 and not 2 at all.
That all said, the other argument for this is that it doesn't do 1 or 2, but it's an incremental step towards 1 & 2. The first step of having up-to-date old versions is having old versions in the first place.
This last bit is why I'm in favor of this, even without the DRY etc, because I think this lays the foundation to make those next steps (patch backporting, automating old patch updates, etc) happen more quickly.
... Lotta words, I know, sorry! The tl;dr is just thanks, the PR is appreciated, let's try to followup with patch updates and update script automation before too long!
I'm hoping to find some time soon to tackle a new update script, if you're not already planning to!
Hi @euank, First, sorry for the delay in answering you. My approach was simplistic indeed. I wouldn't mind if you have a better idea for implementing this. Either I could merge this as it is now (without 1.23 even?) and then you could change anything you would like (you have my green light for sure). Or another option would be to just start a new PR closing this. Regarding the update script, if I were to do it I'd likely do it much later, so please, feel free to do it. Just let me know which path forward you prefer. |
I was trying to say in that long-winded review comment that I'm in favor of merging this, and then iterating on it! I think this is an improvement over the current state of things, and I'm not confident I (or someone else) will do a better iteration super soon! |
I would appreciate it if you could restore the 1.23 derivation because that's the one I'd need right now and, after all, the job was already done, so why not? I noticed that most packages are a bit outdated, but I think it would be better to just merge this foundational PR, so we can do further updates later in smaller PRs, one by one. If we wait too much, 1.23 will disappear before I can fix it! 😢
I also think that the latest patch upgrade probably is not necessary... but TBH k8s is so complex and important that I prefer to be extra cautious here. 😅 |
Don't cry, just learn Nix. No package really disappears. You can always use any past versions already, you just need to add a new input to your flake referencing the commit, and inherit package from that. Nix magic.
I deleted it. -.-
Yes...
I've been meaning to get to this PR for a week already. K3s is a priority for me because I use it in my Pi cluster. But time is limited. I or euank may bump it on and off, but it is not guaranteed.
Questions really is to whom...
No doubt. |
Anyone cares to review/approve again? So I can merge this... |
Fixes #213943
This PR is a start offering some structure to having multiple versions.
Listing all versions we have available right now. Newer versions require further work.
I'm trying to keep things simple here. (so no fancy abstraction, DRY, etc)
NixOS tests and update script are disabled in legacy versions.
k3s_1_26 is now rolling. Update script and tests are working just as were before. (No changes for latest!)
Patches are duplicated on purpose for when time is due directory can be deleted and nothing will be affected.
Due to priorities, I do not plan doing upgrades, back-ports, tests, update script for previous releases. But PRs are welcomed. And would be better someone with genuine interest in this to step up. Hopefully, as we move forward, we can get these things to a better state.
I might improve latest update script to automatically add new major versions in a new folder and pin previous version.
So basically what we have here, is exactly what we had before (latest rolling versions), plus previous releases we had. No changes. (I did copy and paste from history. Changed just a few minor issues that were patched, like cross compilation patch.)
This PR is awaiting review from maintainers: @euank @Mic92