From acde3827f60bc90738429fa490a36a1856a93283 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 19:02:40 -0500 Subject: Add per-app flock locking around deploy, config change, rollback, new-app Nothing previously serialized these against each other, so concurrent invocations for the same app (a manual config change racing an in-flight deploy, a double git push, a rollback mid-deploy) could interleave snapshot/apply/restart/rollback sequences against the same dataset and jail. The lock file lives under a new LocksDir, outside any per-app ZFS dataset, since those datasets get rolled back on a failed health check or a manual rollback. --- cmd/skiff/main.go | 28 +++++++++++++++++++++++++++- internal/layout/layout.go | 11 +++++++++++ internal/lock/lock.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 internal/lock/lock.go diff --git a/cmd/skiff/main.go b/cmd/skiff/main.go index 1c48681..7c9d505 100644 --- a/cmd/skiff/main.go +++ b/cmd/skiff/main.go @@ -16,6 +16,7 @@ import ( "skiff/internal/envfile" "skiff/internal/jailmgr" "skiff/internal/layout" + "skiff/internal/lock" "skiff/internal/sh" "skiff/internal/tmpl" "skiff/internal/zfsutil" @@ -65,7 +66,14 @@ func cmdDeploy(args []string) error { return nil } - return deploy.Run(context.Background(), os.Stdout, layout.Default(), *app, *newSHA) + cfg := layout.Default() + unlock, err := lock.Acquire(cfg.LockPath(*app)) + if err != nil { + return fmt.Errorf("acquire app lock: %w", err) + } + defer unlock.Release() + + return deploy.Run(context.Background(), os.Stdout, cfg, *app, *newSHA) } func cmdNewApp(args []string) error { @@ -89,6 +97,12 @@ func cmdNewApp(args []string) error { out := os.Stdout cfg := layout.Default() + unlock, err := lock.Acquire(cfg.LockPath(*name)) + if err != nil { + return fmt.Errorf("acquire app lock: %w", err) + } + defer unlock.Release() + fmt.Fprintf(out, "==> cloning %s@%s -> %s\n", cfg.BaseDataset, *baseSnapshot, cfg.AppDataset(*name)) if err := zfsutil.Clone(ctx, out, cfg.BaseDataset, *baseSnapshot, cfg.AppDataset(*name), cfg.JailRoot(*name)); err != nil { return fmt.Errorf("clone base dataset: %w", err) @@ -161,6 +175,12 @@ func cmdRollback(args []string) error { return fmt.Errorf("--to is required (use --list to see snapshots)") } + unlock, err := lock.Acquire(cfg.LockPath(*app)) + if err != nil { + return fmt.Errorf("acquire app lock: %w", err) + } + defer unlock.Release() + if err := zfsutil.Rollback(ctx, os.Stdout, dataset, *to); err != nil { return fmt.Errorf("rollback: %w", err) } @@ -275,6 +295,12 @@ func applyConfigChange(app string, mutate func(map[string]string) error) error { return fmt.Errorf("app %q not found: %w", app, err) } + unlock, err := lock.Acquire(cfg.LockPath(app)) + if err != nil { + return fmt.Errorf("acquire app lock: %w", err) + } + defer unlock.Release() + kv, err := envfile.Load(cfg.EnvPath(app)) if err != nil { return fmt.Errorf("load config: %w", err) diff --git a/internal/layout/layout.go b/internal/layout/layout.go index d5888dc..5c2cf30 100644 --- a/internal/layout/layout.go +++ b/internal/layout/layout.go @@ -22,6 +22,7 @@ type Config struct { JailsMount string // parent mountpoint for per-app datasets JailConfDir string // /etc/jail.conf.d NginxConfDir string // nginx include dir regenerated on every deploy + LocksDir string // per-app flock files live here, outside any per-app dataset } func Default() Config { @@ -32,6 +33,7 @@ func Default() Config { JailsMount: "/skiff/apps", JailConfDir: "/etc/jail.conf.d", NginxConfDir: "/usr/local/etc/nginx/skiff.d", + LocksDir: "/skiff/locks", } } @@ -47,6 +49,15 @@ func (c Config) RepoPath(app string) string { return filepath.Join(c.ReposDir, app+".git") } +// LockPath is the per-app flock file that serializes deploy, config change, +// and rollback against each other. It deliberately lives outside +// AppDataset(app): that dataset gets `zfs rollback`ed on a failed health +// check or a manual rollback, which would discard a lock file living inside +// it along with everything else written since the snapshot. +func (c Config) LockPath(app string) string { + return filepath.Join(c.LocksDir, app+".lock") +} + func (c Config) JailConfPath(app string) string { return filepath.Join(c.JailConfDir, app+".conf") } diff --git a/internal/lock/lock.go b/internal/lock/lock.go new file mode 100644 index 0000000..af24882 --- /dev/null +++ b/internal/lock/lock.go @@ -0,0 +1,43 @@ +// Package lock provides per-app exclusive locking, so that concurrent skiff +// invocations against the same app - a deploy racing a config change, either +// racing a manual rollback - can't interleave their snapshot/apply/restart +// sequences against the same dataset and jail. +package lock + +import ( + "fmt" + "os" + "path/filepath" + "syscall" +) + +// Handle is a held lock; call Release to give it up. +type Handle struct { + f *os.File +} + +// Acquire blocks until it holds an exclusive lock on path, creating the file +// (and its parent directory) if needed. The lock is held via flock(2) on the +// open file descriptor, so it's released automatically if the process dies - +// a crash never leaves a stale lock behind. +func Acquire(path string) (*Handle, error) { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return nil, fmt.Errorf("create lock dir: %w", err) + } + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDONLY, 0644) + if err != nil { + return nil, fmt.Errorf("open lock file: %w", err) + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { + f.Close() + return nil, fmt.Errorf("flock: %w", err) + } + return &Handle{f: f}, nil +} + +// Release gives up the lock. Safe to call via defer immediately after a +// successful Acquire. +func (h *Handle) Release() error { + defer h.f.Close() + return syscall.Flock(int(h.f.Fd()), syscall.LOCK_UN) +} -- cgit v1.3