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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/k3s: extend k3s module #275180

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rorosen
Copy link
Contributor

@rorosen rorosen commented Dec 18, 2023

Description of changes

Extend the k3s module to enable the usage of Helm charts and container images in air-gapped environments. Additionally, the manifests option allows to specify arbitrary manifests that are deployed by k3s automatically. It is now possible to deploy Kubernetes workloads using the k3s module.

The following k3s features are enabled by this contribution:

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

@rorosen rorosen changed the title Extend k3s module nixos/k3s: extend k3s module Dec 19, 2023
@rorosen
Copy link
Contributor Author

rorosen commented Feb 6, 2024

ping @euank @superherointj @asonix @yajo @nikstur

@superherointj
Copy link
Contributor

superherointj commented Feb 8, 2024

@rorosen Your contribution here is valid. Eventually I might get to reviewing this. (I plan testing it.) But I cannot promise when.
In advance, I suggest you to clean-up commits.

(I'm working torward this PR. I just want to solve some building blocks first.)

@rorosen
Copy link
Contributor Author

rorosen commented Feb 9, 2024

Done

@yajo
Copy link
Contributor

yajo commented Feb 29, 2024

IMHO this is a great addition. However, the feature needs nixos tests. Would you be able to add some?

@rorosen rorosen force-pushed the extend-k3s-module branch 3 times, most recently from a9bfbc1 to 441724f Compare March 3, 2024 17:17
@rorosen
Copy link
Contributor Author

rorosen commented Mar 3, 2024

I added a test for the new features. Let me know if I can do anything else.

Copy link
Contributor

@yajo yajo left a comment

Choose a reason for hiding this comment

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

It looks great. Just some non-blocking suggestions in case you want to improve it further.

@rorosen rorosen force-pushed the extend-k3s-module branch 4 times, most recently from e2ee02a to cc1e7ee Compare March 9, 2024 17:18
Copy link
Contributor

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Awesome 馃帀

@rorosen
Copy link
Contributor Author

rorosen commented Mar 19, 2024

@superherointj I'm not sure how this works, do I need to do anything else to merge this?

@superherointj
Copy link
Contributor

@superherointj I'm not sure how this works, do I need to do anything else to merge this?

yajo has reviewed, so in theory, any commiter can commit this.

I was meaning to review this PR, but as time is limited I wasn't able to. Sorry.
So, I'll remove myself from this assignment, and let anyone else proceed.

@superherointj superherointj removed their assignment Mar 19, 2024
@rorosen
Copy link
Contributor Author

rorosen commented Mar 25, 2024

@yajo can you merge this or is there anything missing?

@superherointj
Copy link
Contributor

@yajo can you merge this or is there anything missing?

To clarify. From the gray check in his approval, it's possible to infer yajo is not a commiter yet, for a commiter it would be green. So, he cannot merge this. In practice, reviewers do triage work for commiters. Commiters look for reviewed PRs to commit.

I'm currently a commiter. And I could indeed merge your PR. But I have not reviewed it myself. And I can't do it right now.
I'm doing other things. I just did some upkeep for K3s and Etcd, for example. That is time sensitive. A release is happening soon. And other things. And I don't commit what I haven't analyzed. But any other commiter can over take this. Mic92 for example, is a commiter too. And also a K3s maintainer. But I suppose, as he is not answering your calls, he might be busy too.
The delay is a consequence of our limited resources (as always in everything).

In case you are wondering, there's a thread on becoming a commiter here: https://github.com/NixOS/nixpkgs/issues/50105
It's basically necessary to have a significant amount of contributions, understand the release process, be decent at Nix. It's not hard. Many people apply but it is never enough people in the sense that the needs are endless.

The roles of reviewers, maintainers and committers are open to anyone to apply for. If no one is answering your call, it's just a matter of people not being available. Which can be frustrating. I know. I'm sorry for that.

I cannot promise you that I will review/merge your PR soon. Because I do have other priority things that I need to do first. As I said before. Anyway, eventually someone will merge, or you'll get more feedback. I suggest you don't worry about it.

Also, you can use your PR in your host either by cherry-picking it or through an overlay or just swapping the nixos module. So you can benefit from your improvement while people here at nixpkgs are still busy. (I do that myself. In fact, everyone here does that.)

If you feel the pace is slow, consider joining in the upkeep of K3s or anything you are interested in, it might speed things a bit on the topics you are interested. In the sense that it is less work for others, so they become more available. But it's hard to predict what happens, right?

Best regards.

@rorosen
Copy link
Contributor Author

rorosen commented Mar 26, 2024

Ok, understood. Thanks for the clarification and sorry for reaching out all the time. I wasn't aware of what was missing. No pressure from my side.

This contribution extends the k3s module to
enable the usage of Helm charts and container
images in air-gapped environments. Additionally,
the manifests option allows to specify arbitrary
manifests that are deployed by k3s automatically.
It is now possible to deploy Kubernetes workloads
using the k3s module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants