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.

Sign up for GitHub

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

Jump to bottom

kube3d: 3.4.0 -> 4.0.0 #106176

Merged
merged 1 commit into from Jan 25, 2021
Merged

kube3d: 3.4.0 -> 4.0.0 #106176

merged 1 commit into from Jan 25, 2021

Conversation

06kellyjac
Copy link
Member

@06kellyjac 06kellyjac commented Dec 7, 2020 •
edited

Motivation for this change

Bump kube3d/k3d to 4.0.0

Since this is a major version bump and still in-progress I've created this as a draft PR to try things out as it's being developed.

Released: https://github.com/rancher/k3d/releases/tag/v4.0.0

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS (x86_64)
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@06kellyjac 06kellyjac force-pushed the kube3d_4.0.0 branch 2 times, most recently from a7a7a63 to 0ae0062 Compare December 10, 2020 14:15
@06kellyjac
Copy link
Member Author

Result of nixpkgs-review pr 106176 run on x86_64-linux 1

1 package built:
  • kube3d

@06kellyjac
Copy link
Member Author

Result of nixpkgs-review pr 106176 run on x86_64-linux 1

1 package built:
  • kube3d

@ofborg ofborg bot requested a review from ngerstle January 6, 2021 11:35
@06kellyjac 06kellyjac force-pushed the kube3d_4.0.0 branch 2 times, most recently from 5c012c1 to c116555 Compare January 14, 2021 13:41
@06kellyjac
Copy link
Member Author

Result of nixpkgs-review pr 106176 run on x86_64-linux 1

1 package built:
  • kube3d

@06kellyjac 06kellyjac force-pushed the kube3d_4.0.0 branch 2 times, most recently from b2b8a8b to 5be643d Compare January 19, 2021 10:19
@06kellyjac
Copy link
Member Author

Result of nixpkgs-review pr 106176 run on x86_64-linux 1

1 package built:
  • kube3d

@jlesquembre
Copy link
Member

As mention here: #109253 (comment)
Maybe we can make the k3sVersion easier to override? something like

{ lib, buildGoModule, fetchFromGitHub, installShellFiles
,  k3sVersion ? "1.19.4-k3s1" }:

@06kellyjac
Copy link
Member Author

06kellyjac commented Jan 20, 2021 •
edited

Yeah I'm open to it. edit: added

https://github.com/rancher/k3d/blob/main/version/version.go#L37-L38

I've been thinking for a while now I don't think it actually does anything 🤔

k3d version grabs it as is:
https://github.com/rancher/k3d/blob/608f24a6e3d43e596416db046cb24df20da65405/cmd/root.go#L176-L179

But the get version function fetches the latest version anyway if the arg is set to true
https://github.com/rancher/k3d/blob/main/version/version.go#L63-L74

These lines set the latest argument to false but they seem to be lines specifically designed to let you set an exact image so you'd think you'd be setting the k3s version yourself 🤔 🤔 🤔 🤔 🤔
https://github.com/rancher/k3d/blob/7b8506b1d90d6cf7c9337a71b1a0c7a1a216834f/cmd/cluster/clusterCreate.go#L185
https://github.com/rancher/k3d/blob/5092a90d56fb2fa1df3c42dc49aa7977d4968fab/cmd/node/nodeCreate.go#L69

The default "simple" config seems to use true
https://github.com/rancher/k3d/blob/683a92792eb30ab174e806baffcf0bc3364c4f80/pkg/config/transform.go#L46-L56

I feel like k3d should just use latest less a flag or envvar is set to a specific version. Or if a full docker image is specified by a flag or envvar use that instead. 🤷‍♂️

@06kellyjac
Copy link
Member Author

Result of nixpkgs-review pr 106176 run on x86_64-linux 1

1 package built:
  • kube3d

@06kellyjac 06kellyjac marked this pull request as ready for review January 20, 2021 14:42
@06kellyjac
Copy link
Member Author

Result of nixpkgs-review pr 106176 run on x86_64-linux 1

1 package built:
  • kube3d

@jlesquembre
Copy link
Member

I feel like k3d should just use latest less a flag or envvar is set to a specific version. Or if a full docker image is specified by a flag or envvar use that instead.

But you can already specify the image version in the CLI, isn't it? With the --image argument. IMO, makes sense to use the latest version, but I think it is better to stick to the same version used upstream. Envvars would be nice, but not supported by k3d. But they added support for a config file, you can use that to always use the latest version

Other than that, 4.0.0 was released! Let's merge this

@06kellyjac
Copy link
Member Author

/marvin opt-in

@jlesquembre can you post / status needs_merger (without the space)? The PR creator can't post it

@marvin-mk2
Copy link

marvin-mk2 bot commented Jan 25, 2021

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

@marvin-mk2 marvin-mk2 bot added the marvin label Jan 25, 2021
@jlesquembre
Copy link
Member

/status needs_merger

@marvin-mk2 marvin-mk2 bot requested a review from kevincox January 25, 2021 12:35
@kevincox kevincox merged commit 8708b6d into NixOS:master Jan 25, 2021
@06kellyjac 06kellyjac deleted the kube3d_4.0.0 branch January 25, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlesquembre jlesquembre jlesquembre approved these changes

@kalbasit kalbasit Awaiting requested review from kalbasit

@kuznero kuznero Awaiting requested review from kuznero

@ngerstle ngerstle Awaiting requested review from ngerstle

@kevincox kevincox Awaiting requested review from kevincox

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer awaiting_merger marvin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@06kellyjac @jlesquembre @kevincox