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

Predefined tool paths #85

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

Predefined tool paths #85

wants to merge 2 commits into from

Conversation

ressu
Copy link
Contributor

@ressu ressu commented Jun 1, 2024

When using minimalistic Kubernetes deployments, like Talos, the /usr/bin/env command isn't available. Instead of always relying on env, replace the chroot shell wrapper with a tool that can either invoke chroot with the env command or directly using a full path of the tool.

The change adds a new flag to define the location of chroot and the relevant tools. The node service now also check for existence of the chroot directory at the beginning of startup to warn about misconfiguration.

ressu added 2 commits May 26, 2024 11:37
The tool executor is capable of running given tools in chroot either by automatically locating the tools in path using the `env` command or directly if the path is known.

This removes the need for the shell script to wrap commands and reduces number of needed external binaries on the node.
Instead of checking the existence of chroot target directory runtime, only enable chroot on the node daemon. This way the service will fail early if the chroot doesn't exist, instead of printing an error during runtime.
@MartinLoeper
Copy link

MartinLoeper commented Oct 13, 2024

Fantastic! Thanks @ressu! I tested this on my Synology DS923+ and it works!

I want to add that you need the Talos iscsi-tools extension on talos to run the fork: https://github.com/siderolabs/extensions/pkgs/container/iscsi-tools#storage

Additionally I had to modify synology-csi-node daemon set to add the following args to the csi-plugin container:

- '--chroot-dir=/host'
- '--iscsiadm-path=/usr/local/sbin/iscsiadm'

@avanier
Copy link

avanier commented Nov 16, 2024

I would love to see this merged. As such, I've opened ressu#1 to bring @ressu 's changes up to date.

🙏 🙏

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.

3 participants