pdtpartners / nix-snapshotter

Brings native understanding of Nix packages to containerd
MIT License
566 stars 15 forks source link

Upgrade k3s version to v1.27.9+k3s1 #108

Closed RobbieBuxton closed 9 months ago

RobbieBuxton commented 9 months ago

Added an overlay to upgrade k3s to the version on unstable so that we get the bug fix mentioned in #102 . Also removed the containerd overlay as the 23.11 contains the fix we required.

closes #102

RobbieBuxton commented 9 months ago

@antoinerg do you mind testing this locally? For some reason I can't seem to replicate the bug. Thanks!

antoinerg commented 9 months ago

@antoinerg do you mind testing this locally? For some reason I can't seem to replicate the bug. Thanks!

Please see the extra flag needed at https://github.com/pdtpartners/nix-snapshotter/pull/109/files#diff-d6431f0c929f50d0ee7cc009dafb4a0f0b45b124e8da1a2643622bfee77204a2. It won't work without this!

antoinerg commented 9 months ago

For some reason I can't seem to replicate the bug.

That's surprising to me. Are you testing via the VM? So doing nix run ".#vm", logging in, and then run kubectl apply -Rf /etc/kubernetes/redis?

RobbieBuxton commented 9 months ago

For some reason I can't seem to replicate the bug.

That's surprising to me. Are you testing via the VM? So doing nix run ".#vm", logging in, and then run kubectl apply -Rf /etc/kubernetes/redis?

Yeah, not entirely sure why, might be something to do with cache like I think you mentioned a few days ago explaining why the CI isn't picking it up either. Haven't had time to thoroughly test though.

RobbieBuxton commented 9 months ago

Added @antoinerg's fix from #109

RobbieBuxton commented 9 months ago

Would appreciate the review as well @elpdt852 when you have a moment, thanks!

antoinerg commented 9 months ago

@RobbieBuxton the way I test is described in https://github.com/pdtpartners/nix-snapshotter/issues/102#issue-1945626033. When following the steps from that comment and starting from your branch, I end up with a VM with:

[root@nixos:~]# k3s --version
k3s version v1.27.6+k3s1 (bd04941a)
go version go1.20.11

Somehow the newer k3s version does not find its way into the VM!

RobbieBuxton commented 9 months ago

@RobbieBuxton the way I test is described in #102 (comment). When following the steps from that comment and starting from your branch, I end up with a VM with:

[root@nixos:~]# k3s --version
k3s version v1.27.6+k3s1 (bd04941a)
go version go1.20.11

Somehow the newer k3s version does not find its way into the VM!

@RobbieBuxton the way I test is described in #102 (comment). When following the steps from that comment and starting from your branch, I end up with a VM with:

[root@nixos:~]# k3s --version
k3s version v1.27.6+k3s1 (bd04941a)
go version go1.20.11

Somehow the newer k3s version does not find its way into the VM!

So yeah, turns out overriding k3s is a lot more of a pain than I previously thought and it wasn't being overridden properly at all. I think your original approach of just taking unstable is might better after all. Sorry about all that! We should probably just use your PR. EDIT: I am also able to reproduce the bug now as well which is nice. EDIT2: maybe a patch of the new commit would work, I'll have a look into it tonight

elpdt852 commented 9 months ago

turns out overriding k3s is a lot more of a pain than I previously thought

Yeah I recall looking into this and didn't add my patch in an overlay due to how the upstream derivation was structured. If nixpkgs-unstable already has k3s with my PR merged, then we should just switch back to nixpkgs-unstable. I will take a look at this PR tonight.

Excited to have rootless k3s + nix-snapshotter working as a home-manager module.

antoinerg commented 9 months ago

I think your original approach of just taking unstable is might better after all. Sorry about all that! We should probably just use your PR.

No worries! Feel free to review https://github.com/pdtpartners/nix-snapshotter/pull/109 then!

cc @elpdt852

antoinerg commented 9 months ago

Please consider https://github.com/pdtpartners/nix-snapshotter/pull/110 which is even simpler!

RobbieBuxton commented 9 months ago

110 is a better PR so closing this one