diff --git a/CHANGELOG.md b/CHANGELOG.md index edc125cf..9a27e2b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## Unreleased + +### Added + +- `wwclient --once` prompts wwclient to run once. #1226 + +### Changed + +- `wwclient` places files from the runtime overlay atomically. #1307, #1975 + +### Fixed + +- Fixed a bug in `wwclient` which prevented proper cleanup of ephemeral files. +- Fixed `wwclient --debug` to properly enable debug logging. + ## v4.6.3, 2025-08-01 ### Added diff --git a/cmd/wwclient/main.go b/cmd/wwclient/main.go index d1460ec2..6d1f3539 100644 --- a/cmd/wwclient/main.go +++ b/cmd/wwclient/main.go @@ -13,7 +13,6 @@ func main() { err := root.Execute() if err != nil { - fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) if wwclient.DebugFlag { fmt.Printf("\nSTACK TRACE: %+v\n", err) } diff --git a/internal/app/wwclient/root.go b/internal/app/wwclient/root.go index e2e077f7..00f0465d 100644 --- a/internal/app/wwclient/root.go +++ b/internal/app/wwclient/root.go @@ -1,8 +1,8 @@ package wwclient import ( - "errors" "fmt" + "io" "net" "net/http" "net/url" @@ -10,6 +10,7 @@ import ( "os/exec" "os/signal" "path" + "path/filepath" "strings" "syscall" "time" @@ -32,6 +33,7 @@ var ( SilenceUsage: true, Args: cobra.NoArgs, } + Once bool DebugFlag bool PIDFile string Webclient *http.Client @@ -39,6 +41,7 @@ var ( ) func init() { + rootCmd.PersistentFlags().BoolVar(&Once, "once", false, "Run once and exit") rootCmd.PersistentFlags().BoolVarP(&DebugFlag, "debug", "d", false, "Run with debugging messages enabled.") rootCmd.PersistentFlags().StringVarP(&PIDFile, "pidfile", "p", "/var/run/wwclient.pid", "PIDFile to use") rootCmd.PersistentFlags().StringVar(&WarewulfConfArg, "warewulfconf", "", "Set the warewulf configuration file") @@ -52,6 +55,12 @@ func GetRootCommand() *cobra.Command { } func CobraRunE(cmd *cobra.Command, args []string) (err error) { + if DebugFlag { + wwlog.SetLogLevel(wwlog.DEBUG) + } else { + wwlog.SetLogLevel(wwlog.INFO) + } + conf := warewulfconf.Get() if WarewulfConfArg != "" { err = conf.Read(WarewulfConfArg, false) @@ -64,33 +73,29 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { return } pid, err := pidfile.Write(PIDFile) - defer cleanUp() - if err != nil && pid == -1 { - wwlog.Warn("%v. starting new wwclient", err) - } else if err != nil && pid > 0 { - return errors.New("found pidfile " + PIDFile + " not starting") - } - - if os.Args[0] == path.Join(conf.Paths.WWClientdir, "wwclient") { - err := os.Chdir("/") - if err != nil { - return fmt.Errorf("failed to change dir: %w", err) + if err != nil { + if pid > 0 { // wwclient is already running + return fmt.Errorf("%v: not starting", err) + } else { // the pidfile is stale + wwlog.Warn("%s: starting new wwclient", err) } + } + defer cleanUp() + + target := "/" + if os.Args[0] == path.Join(conf.Paths.WWClientdir, "wwclient") { wwlog.Warn("updating live file system: cancel now if this is in error") time.Sleep(5000 * time.Millisecond) } else { + target = "/warewulf/wwclient-test" + fmt.Printf("Called via: %s\n", os.Args[0]) - fmt.Printf("Runtime overlay is being put in '/warewulf/wwclient-test' rather than '/'\n") + fmt.Printf("Runtime overlay is being put in '%s' rather than '/'\n", target) fmt.Printf("For full functionality call with: %s\n", path.Join(conf.Paths.WWClientdir, "wwclient")) - err := os.MkdirAll("/warewulf/wwclient-test", 0755) + err := os.MkdirAll(target, 0755) if err != nil { return fmt.Errorf("failed to create dir: %w", err) } - - err = os.Chdir("/warewulf/wwclient-test") - if err != nil { - return fmt.Errorf("failed to change dir: %w", err) - } } localTCPAddr := net.TCPAddr{} @@ -146,19 +151,18 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { tag = "Unknown" } + wwlog.Debug("uuid: %s", localUUID.String()) + wwlog.Debug("assetkey: %s", tag) + cmdline, err := os.ReadFile("/proc/cmdline") if err != nil { return fmt.Errorf("could not read from /proc/cmdline: %w", err) } - - wwid_tmp := strings.Split(string(cmdline), "wwid=") - if len(wwid_tmp) < 2 { - return fmt.Errorf("'wwid' is not defined in /proc/cmdline") + wwid, err := parseWWIDFromCmdline(string(cmdline)) + if err != nil { + return fmt.Errorf("failed to parse wwid: %w", err) } - wwid := strings.Split(wwid_tmp[1], " ")[0] - wwid = strings.TrimSuffix(wwid, "\n") - // Dereference wwid from [interface] for cases that cannot have /proc/cmdline set by bootloader if string(wwid[0]) == "[" { iface := wwid[1 : len(wwid)-1] @@ -170,6 +174,8 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { wwlog.Info("Dereferencing wwid from [%s] to %s", iface, wwid) } + wwlog.Debug("wwid: %s", wwid) + duration := 300 if conf.Warewulf.UpdateInterval > 0 { duration = conf.Warewulf.UpdateInterval @@ -178,6 +184,10 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { // listen on SIGHUP sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) + + // Add a channel to signal main loop to exit gracefully + exitChan := make(chan bool, 1) + go func() { for { sig := <-sigs @@ -188,7 +198,9 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { stopTimer.Reset(0) case syscall.SIGTERM, syscall.SIGINT: wwlog.Info("terminating wwclient, %v", sig) - os.Exit(0) + // Signal main loop to exit instead of calling os.Exit(0) + exitChan <- true + return } } }() @@ -197,20 +209,50 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { if ipaddr == "" { ipaddr = conf.Ipaddr } + for { - updateSystem(ipaddr, conf.Warewulf.Port, wwid, tag, localUUID) + updateSystem(target, ipaddr, conf.Warewulf.Port, wwid, tag, localUUID) if !finishedInitialSync { - // ignore error and status here, as this wouldn't change anything + // Notify systemd that the service has started successfully. + // + // Ignoring error and status, as this wouldn't change anything. _, _ = daemon.SdNotify(false, daemon.SdNotifyReady) finishedInitialSync = true } - <-stopTimer.C - stopTimer.Reset(time.Duration(duration) * time.Second) + if Once { + return nil + } + + // Check for exit signal or timer + select { + case <-exitChan: + wwlog.Info("gracefully shutting down") + return nil + case <-stopTimer.C: + stopTimer.Reset(time.Duration(duration) * time.Second) + } } } -func updateSystem(ipaddr string, port int, wwid string, tag string, localUUID uuid.UUID) { +// parseWWIDFromCmdline extracts the wwid parameter from kernel command line +func parseWWIDFromCmdline(cmdline string) (string, error) { + params := strings.Fields(cmdline) + + for _, param := range params { + if strings.HasPrefix(param, "wwid=") { + wwid := strings.TrimPrefix(param, "wwid=") + if wwid == "" { + return "", fmt.Errorf("wwid parameter is empty") + } + return wwid, nil + } + } + + return "", fmt.Errorf("wwid parameter not found in kernel command line") +} + +func updateSystem(target string, ipaddr string, port int, wwid string, tag string, localUUID uuid.UUID) { var resp *http.Response counter := 0 for { @@ -246,13 +288,200 @@ func updateSystem(ipaddr string, port int, wwid string, tag string, localUUID uu time.Sleep(60000 * time.Millisecond) return } + wwlog.Info("applying runtime overlay") - command := exec.Command("/bin/sh", "-c", "gzip -dc | cpio -iu") + + // unpack overlay into a temporary directory + tempDir, err := os.MkdirTemp("", "wwclient-") + if err != nil { + wwlog.Error("failed to create temp directory: %s", err) + return + } + defer os.RemoveAll(tempDir) + wwlog.Debug("unpacking runtime overlay to %s", tempDir) + command := exec.Command("/bin/sh", "-c", fmt.Sprintf("gzip -dc | cpio -iu --directory=%s", tempDir)) command.Stdin = resp.Body - err := command.Run() + err = command.Run() if err != nil { wwlog.Error("failed running cpio: %s", err) + return } + + // Atomically move files from temp directory to current working directory + err = atomicApplyOverlay(tempDir, target) + if err != nil { + wwlog.Error("failed to apply overlay: %s", err) + } +} + +func atomicApplyOverlay(srcDir, destDir string) error { + return filepath.Walk(srcDir, func(srcPath string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Calculate relative path from srcDir + relPath, err := filepath.Rel(srcDir, srcPath) + if err != nil { + return err + } + + // Skip the root directory itself + if relPath == "." { + return nil + } + + destPath := filepath.Join(destDir, relPath) + + if info.IsDir() { + // Create directory if it doesn't exist + wwlog.Debug("Ensuring directory exists: %s", destPath) + err := os.MkdirAll(destPath, info.Mode()) + if err != nil { + return err + } + // Ensure permissions are updated even if directory already existed + wwlog.Debug("Updating permissions for directory: %s", destPath) + err = os.Chmod(destPath, info.Mode()) + if err != nil { + return fmt.Errorf("failed to update directory permissions for %s: %w", destPath, err) + } + // Update ownership and timestamps + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + wwlog.Debug("Updating ownership for directory: %s", destPath) + err := os.Chown(destPath, int(stat.Uid), int(stat.Gid)) + if err != nil { + wwlog.Warn("failed to update ownership for directory %s: %s", destPath, err) + } + } + err = os.Chtimes(destPath, info.ModTime(), info.ModTime()) + if err != nil { + wwlog.Warn("failed to update timestamps for directory %s: %s", destPath, err) + } + return nil + + } else if info.Mode()&os.ModeSymlink != 0 { + // Handle symbolic links + linkTarget, err := os.Readlink(srcPath) + if err != nil { + return fmt.Errorf("failed to read symlink %s: %w", srcPath, err) + } + + // Create a temporary symlink in same directory as the destination. + // This ensures the temp file is on the same filesystem as the final + // destination, so the rename placement is atomic. + destParent := filepath.Dir(destPath) + + // Ensure destination directory exists + if err := os.MkdirAll(destParent, 0755); err != nil { + return fmt.Errorf("failed to create destination directory %s: %w", destParent, err) + } + + tempFile, err := os.CreateTemp(destParent, ".wwclient-tmp-") + if err != nil { + return fmt.Errorf("failed to create temp file for symlink %s: %w", destPath, err) + } + tempPath := tempFile.Name() + _ = tempFile.Close() + os.Remove(tempPath) // Remove the regular file so we can create a symlink + + wwlog.Debug("Creating temporary symlink: %s -> %s", tempPath, linkTarget) + err = os.Symlink(linkTarget, tempPath) + if err != nil { + return fmt.Errorf("failed to create temporary symlink %s: %w", tempPath, err) + } + + // Update ownership if possible (note: lchown for symlinks) + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + wwlog.Debug("Updating ownership for temporary symlink: %s", tempPath) + err := syscall.Lchown(tempPath, int(stat.Uid), int(stat.Gid)) + if err != nil { + wwlog.Warn("failed to update ownership for temporary symlink %s: %s", tempPath, err) + } + } + + // Atomic rename - this will be atomic since both files are in the same directory + wwlog.Debug("Moving symlink %s to %s", tempPath, destPath) + err = os.Rename(tempPath, destPath) + if err != nil { + os.Remove(tempPath) + return fmt.Errorf("failed to atomically move symlink %s to %s: %w", tempPath, destPath, err) + } + + return nil + + } else { + // Create a temporary file in same directory as the destination. + // This ensures the temp file is on the same filesystem as the final + // destination, so the rename placement is atomic. + destParent := filepath.Dir(destPath) + + // Ensure destination directory exists + if err := os.MkdirAll(destParent, 0755); err != nil { + return fmt.Errorf("failed to create destination directory %s: %w", destParent, err) + } + + tempFile, err := os.CreateTemp(destParent, ".wwclient-tmp-") + if err != nil { + return fmt.Errorf("failed to create temp file for %s: %w", destPath, err) + } + tempPath := tempFile.Name() + _ = tempFile.Close() + + // Copy file content and metadata + wwlog.Debug("Copying file from %s to temp location %s", srcPath, tempPath) + err = copyFile(srcPath, tempPath, info) + if err != nil { + os.Remove(tempPath) + return fmt.Errorf("failed to copy %s to temp location: %w", srcPath, err) + } + + // Atomic rename - this will be atomic since both files are in the same directory + // (and thus on the same filesystem) + wwlog.Debug("Moving %s to %s", tempPath, destPath) + err = os.Rename(tempPath, destPath) + if err != nil { + os.Remove(tempPath) + return fmt.Errorf("failed to atomically move %s to %s: %w", tempPath, destPath, err) + } + } + + return nil + }) +} + +func copyFile(src, dst string, srcInfo os.FileInfo) error { + srcFile, err := os.Open(src) + if err != nil { + return err + } + defer srcFile.Close() + + dstFile, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, srcInfo.Mode()) + if err != nil { + return err + } + defer dstFile.Close() + + _, err = io.Copy(dstFile, srcFile) + if err != nil { + return err + } + + // Preserve timestamps and ownership if possible + if stat, ok := srcInfo.Sys().(*syscall.Stat_t); ok { + wwlog.Debug("Updating ownership for file: %s", dst) + err = os.Chown(dst, int(stat.Uid), int(stat.Gid)) + if err != nil { + wwlog.Warn("failed to update ownership for file %s: %s", dst, err) + } + } + err = os.Chtimes(dst, srcInfo.ModTime(), srcInfo.ModTime()) + if err != nil { + wwlog.Warn("failed to update timestamps for file %s: %s", dst, err) + } + + return nil } func cleanUp() { diff --git a/internal/pkg/pidfile/pidfile.go b/internal/pkg/pidfile/pidfile.go index 25565340..986f947b 100644 --- a/internal/pkg/pidfile/pidfile.go +++ b/internal/pkg/pidfile/pidfile.go @@ -1,7 +1,7 @@ +// Package pidfile provides utilities for managing process ID files. +// Based on original work found here, github.com/soellman/pidfile package pidfile -// based on original work found here, github.com/soellman/pidfile - import ( "errors" "fmt" @@ -12,22 +12,30 @@ import ( ) var ( + // ErrProcessRunning indicates that a process with the PID from the pidfile is currently running. ErrProcessRunning = errors.New("process is running") - ErrFileStale = errors.New("pidfile exists but process is not running") - ErrFileInvalid = errors.New("pidfile has invalid contents") + // ErrFileStale indicates that the pidfile exists but the process is not running. + ErrFileStale = errors.New("pidfile exists but process is not running") + // ErrFileInvalid indicates that the pidfile contains invalid or unparseable contents. + ErrFileInvalid = errors.New("pidfile has invalid contents") ) -// Remove a pidfile +// Remove removes a pidfile from the filesystem. +// It returns an error if the file cannot be removed. func Remove(filename string) error { return os.RemoveAll(filename) } -// Write writes a pidfile, returning an error -// if the process is already running or pidfile is orphaned +// Write writes a pidfile containing the current process ID. +// It returns the PID and an error if the process is already running or pidfile is orphaned. func Write(filename string) (int, error) { return WriteControl(filename, os.Getpid(), false) } +// WriteControl writes a pidfile with the specified PID and control options. +// If overwrite is false and a stale pidfile exists, it returns ErrFileStale. +// If overwrite is true, it will overwrite stale pidfiles. +// It returns the PID and an error if the process is already running. func WriteControl(filename string, pid int, overwrite bool) (int, error) { // Check for existing pid oldpid, err := pidfileContents(filename) @@ -49,6 +57,8 @@ func WriteControl(filename string, pid int, overwrite bool) (int, error) { return pid, os.WriteFile(filename, []byte(fmt.Sprintf("%d\n", pid)), 0644) } +// pidfileContents reads and parses the PID from a pidfile. +// It returns the PID and an error if the file cannot be read or contains invalid data. func pidfileContents(filename string) (int, error) { contents, err := os.ReadFile(filename) if err != nil { @@ -63,6 +73,8 @@ func pidfileContents(filename string) (int, error) { return pid, nil } +// pidIsRunning checks if a process with the given PID is currently running. +// It returns true if the process is running, false otherwise. func pidIsRunning(pid int) bool { process, err := os.FindProcess(pid) if err != nil { diff --git a/userdocs/nodes/disks.rst b/userdocs/nodes/disks.rst index 43a81bdd..0acc9f78 100644 --- a/userdocs/nodes/disks.rst +++ b/userdocs/nodes/disks.rst @@ -130,7 +130,7 @@ mounted. If partitions or file systems already exist on the disk, ``ignition`` tries to reuse existing file systems by default. To ignore existing file systems and provision fresh file systems on each boot, -specify the ``--fswipe``` flag for that filesystem, and ``--diskwipe`` for the +specify the ``--fswipe`` flag for that filesystem, and ``--diskwipe`` for the disk, as necessary. If you would like to re-use existing partitions but want to replace existing