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

Unpack cni_plugins, crictl & etcd to folders in /opt and use symlinks #96

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Unpack cni_plugins, crictl & etcd to folders in /opt and use symlinks #96

wants to merge 2 commits into from

Conversation

olavst-spk
Copy link
Contributor

Pull Request (PR) description

Makes the installation of cni_plugins, crictl & etcd similar to the way the main k8s binaries are installed. The archives are unpacked to folders in /opt and symlinked to /usr/local/bin . The target folder is unique per version, making it possible to update these components.

This Pull Request (PR) fixes the following issues

Fixes #95.

@olavst-spk olavst-spk force-pushed the fix_updating_cni_plugins_crictl_etcd branch from 6770129 to f7dd164 Compare May 31, 2024 11:47
ananace
ananace requested changes May 31, 2024
View reviewed changes
Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

As a thought, wouldn't it be simpler to store all these things under /opt/k8s?
With a /opt/k8s/archives for archives in general, and then - for example - /opt/k8s/1.28.6/ for Kubernetes itself, /opt/k8s/cni-1.5.0/ for CNI, /opt/k8s/etcd-3.3.25/ for etcd, /opt/k8s/crictl-1.26.0/ for crictl, etc.

manifests/install/cni_plugins.pp Outdated Show resolved Hide resolved
@olavst-spk olavst-spk force-pushed the fix_updating_cni_plugins_crictl_etcd branch from f7dd164 to 0cab643 Compare May 31, 2024 12:04
@olavst-spk
Copy link
Contributor Author

As a thought, wouldn't it be simpler to store all these things under /opt/k8s? With a /opt/k8s/archives for archives in general, and then - for example - /opt/k8s/1.28.6/ for Kubernetes itself, /opt/k8s/cni-1.5.0/ for CNI, /opt/k8s/etcd-3.3.25/ for etcd, /opt/k8s/crictl-1.26.0/ for crictl, etc.

I didn't want to use /opt/k8s for everything, in case we want to separate these components out in their own modules in the future. At least crictl and etcd can be installed independently of Kubernetes. (I'm not sure about the cni-plugins)

@ananace
Copy link
Member

ananace commented May 31, 2024 •
edited
Loading

I didn't want to use /opt/k8s for everything, in case we want to separate these components out in their own modules in the future. At least crictl and etcd can be installed independently of Kubernetes. (I'm not sure about the cni-plugins)

Technically every single component of Kubernetes can be installed independently, and as a note the CNI plugins are also used by podman and other CRI-compliant container runtimes.
But for the loose components linked together - in this case - as part of a Kubernetes cluster managed by the k8s module, it makes some some sense both from an organizational aspect to keep them bundled together, and also to avoid collisions with components managed by other modules - like a potentially separate etcd module.

Now, I don't want to make it sound like a dealbreaker to keep them separate, but they really need to be under some kind of path that will keep them from colliding with their default names - since Kubernetes has historically involved lots of loose components being stored under paths with default names. (/opt/cni, /opt/etcd, and /opt/cri-tools are ones I've personally seen used by in-cluster components and tooling that's been extracted from hosted solutions)

@olavst-spk
Copy link
Contributor Author

Not a dealbreaker for me either 🙂 I will update the PR today with a new commit where all the components are stored under /opt/k8s. We can then easily keep/drop that commit depending on which solution is preferred.

@olavst-spk
Unpack cni_plugins, crictl & etcd to folders in /opt and use symlinks
306dbaf 
Makes the installation of cni_plugins, crictl & etcd similar to the way
the main k8s binaries are installed. The archives are unpacked to folders
in /opt and symlinked to /usr/local/bin . The target folder is unique
per version, making it possible to update these components.

Fixes voxpupuli#95.
@olavst-spk olavst-spk force-pushed the fix_updating_cni_plugins_crictl_etcd branch from 0cab643 to 306dbaf Compare June 3, 2024 13:01
@olavst-spk olavst-spk force-pushed the fix_updating_cni_plugins_crictl_etcd branch from aa22c30 to c9e4f2e Compare June 3, 2024 13:03
@olavst-spk
Copy link
Contributor Author

I have updated the PR, so all the files are under /opt/k8s now

@olavst-spk
Copy link
Contributor Author

Can this be merged now?

ananace
ananace requested changes Jun 14, 2024
View reviewed changes
Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

Sorry, been kept busy so I haven't had time to test the change, I still have some concerns on the CNI plugins though - not entirely sure how to best solve them either.

manifests/install/cni_plugins.pp Show resolved Hide resolved
@ananace ananace requested a review from rwaffen September 24, 2024 10:09
@ananace
Copy link
Member

ananace commented Sep 24, 2024

Going to see about getting this merged before the next release, since it's going to be marked as backwards-incompatible anyway (because of #104)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ananace ananace ananace approved these changes

@rwaffen rwaffen Awaiting requested review from rwaffen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cni-plugins, crictl & etcd cannot be updated
2 participants
@olavst-spk @ananace