From 313eef9209ad67ec4ebe26a6113780541f61da39 Mon Sep 17 00:00:00 2001 From: jp39 Date: Mon, 19 Aug 2024 14:51:44 +0200 Subject: [PATCH 1/2] Add ability to run the provisioner directly on the ZFS host. The default is to run kubernetes-zfs-provisioner in a container and create datasets via SSH on a remote host. To do that, the docker image is created with the zfs and update-permissions stubs that will both call commands on the remote host using SSH. Allows running kubernetes-zfs-provisioner directly on the ZFS host by making the update-permissions script presence optional. The zfs stub is already optional because it merely replaces the command of the same name on the remote host. The provisionner now uses the update-permissions executable if it's present in the current PATH, otherwise it falls back to executing chmod g+w directly. Fixes #130. --- docker/update-permissions.sh | 6 ++++-- pkg/zfs/zfs.go | 14 +++++++++++++- test/update-permissions | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docker/update-permissions.sh b/docker/update-permissions.sh index d07c4be..5cd6472 100755 --- a/docker/update-permissions.sh +++ b/docker/update-permissions.sh @@ -5,7 +5,9 @@ set -eo pipefail zfs_mod="${ZFS_MOD:-g+w}" chmod_bin=${ZFS_CHOWN_BIN:-sudo -H chmod} -zfs_host="${1}" -zfs_mountpoint="${2}" +zfs_mountpoint="${1}" + +# Do not try to manually modify these Env vars, they will be updated by the provisioner just before invoking the script. +zfs_host="${ZFS_HOST}" ssh "${zfs_host}" "${chmod_bin} ${zfs_mod} ${zfs_mountpoint}" diff --git a/pkg/zfs/zfs.go b/pkg/zfs/zfs.go index 015fa16..2c6bee6 100644 --- a/pkg/zfs/zfs.go +++ b/pkg/zfs/zfs.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "sync" gozfs "github.com/mistifyio/go-zfs/v3" @@ -114,7 +115,18 @@ func (z *zfsImpl) SetPermissions(dataset *Dataset) error { if dataset.Mountpoint == "" { return fmt.Errorf("undefined mountpoint for dataset: %s", dataset.Name) } - cmd := exec.Command("update-permissions", dataset.Hostname, dataset.Mountpoint) + + globalLock.Lock() + defer globalLock.Unlock() + if err := setEnvironmentVars(dataset.Hostname); err != nil { + return err + } + cmd := exec.Command("update-permissions", dataset.Mountpoint) + if !filepath.IsAbs(cmd.Path) { + // update-permissions not found in PATH + cmd = exec.Command("chmod", "g+w", dataset.Mountpoint) + } + out, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("could not update permissions on '%s': %w: %s", dataset.Hostname, err, out) diff --git a/test/update-permissions b/test/update-permissions index 543acde..47c3a7c 100755 --- a/test/update-permissions +++ b/test/update-permissions @@ -5,6 +5,6 @@ set -eo pipefail zfs_mod="${ZFS_MOD:-g+w}" chmod_bin=${ZFS_CHOWN_BIN:-chmod} -zfs_mountpoint="${2}" +zfs_mountpoint="${1}" ${chmod_bin} ${zfs_mod} ${zfs_mountpoint} From f5f9732952b6a7e84a658ce7962c9aa84a2571ed Mon Sep 17 00:00:00 2001 From: jp39 Date: Wed, 11 Sep 2024 16:49:15 +0200 Subject: [PATCH 2/2] Do not unnecessarily fork to chmod mountpoint --- pkg/zfs/zfs.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/zfs/zfs.go b/pkg/zfs/zfs.go index 2c6bee6..d529612 100644 --- a/pkg/zfs/zfs.go +++ b/pkg/zfs/zfs.go @@ -122,15 +122,25 @@ func (z *zfsImpl) SetPermissions(dataset *Dataset) error { return err } cmd := exec.Command("update-permissions", dataset.Mountpoint) - if !filepath.IsAbs(cmd.Path) { - // update-permissions not found in PATH - cmd = exec.Command("chmod", "g+w", dataset.Mountpoint) + if filepath.IsAbs(cmd.Path) { + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("could not update permissions on '%s': %w: %s", dataset.Hostname, err, out) + } + return nil } - out, err := cmd.CombinedOutput() + // update-permissions executable not found in PATH + st, err := os.Lstat(dataset.Mountpoint) if err != nil { - return fmt.Errorf("could not update permissions on '%s': %w: %s", dataset.Hostname, err, out) + return err } + + // Add group write bit + if err := os.Chmod(dataset.Mountpoint, st.Mode()|0o020); err != nil { + return err + } + return nil }