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

Implement Distro detection #84

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

phyrog
Copy link
Collaborator

@phyrog phyrog commented Apr 3, 2024

Describe your changes

Implements some basic distro detection similar to what the shell script is doing (checking for existence of containerd configs in various places). This will also allow handling of special cases (e.g. RKE2 and K3s need their containerd config copied to a different file before modifying). This is needed for the manager to remain unaware of what exact distro is running on each node.

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • I tested the changes with the following distributions:
    • Kind
    • MiniKube
    • MicroK8s
    • Rancher RKE2
    • Azure AKS
    • GCP GKE (Ubuntu nodes)
    • AWS EKS (AmazonLinux2 nodes)
    • AWS EKS (Ubuntu nodes)
    • Digital Ocean Kubernetes


config.Runtime.ConfigPath = distro.ConfigPath

if err := RunInstall(config, rootFs, hostFs, distro.Restarter(preset.Env{ConfigPath: distro.ConfigPath, HostFs: hostFs})); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not happy yet with how parts of the returned distro struct ends up in config and the restarter is passed as separate argument.


if err := RunInstall(config, rootFs, hostFs, restarter); err != nil {
distro, err := DetectDistro(config, hostFs)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not happy with this being duplicated in uninstall.go

@vdice
Copy link
Collaborator

vdice commented Sep 16, 2024

I recently created an updated helm chart PR and noticed k3s support in runtime-class-manager's node-installer was still pending, which led me to this PR. I tested it and got things working on k3s with the following updates: https://github.com/spinkube/runtime-class-manager/compare/feat-autodetect-containerd-conf...vdice:autodetect-pr-84-updates?expand=1

Checking in to see if you'll have a chance to revisit this PR.

I'd love to help, as I'm keen to sub runtime-class-manager in for kwasm-operator for SpinKube -- and for parity with the latter's node-installer, we'd need to make sure microk8s, rke2, k3s continue to be supported. (I haven't yet tested microk8s or rke2 from this PR, but can if that would be helpful.)

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

Successfully merging this pull request may close these issues.

2 participants