From dc263425e2f3b6afb1fb524d934d9acb39322632 Mon Sep 17 00:00:00 2001 From: Christian Goll Date: Tue, 20 Aug 2024 17:41:29 +0200 Subject: [PATCH 1/5] Added cow option to bind The option cow can now be set for files which are then mounted during exec, but copied into the image and removed if not modified. Signed-off-by: Christian Goll --- CHANGELOG.md | 1 + .../app/wwctl/container/exec/child/main.go | 9 ++- internal/app/wwctl/container/exec/main.go | 72 ++++++++++++++++++- internal/app/wwctl/container/exec/root.go | 6 +- internal/app/wwctl/container/shell/root.go | 6 +- internal/pkg/config/mounts.go | 1 + internal/pkg/container/mountpoints.go | 10 ++- userdocs/contents/containers.rst | 15 ++++ 8 files changed, 113 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ff8ec4e..1cdb509c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Added option for wwclient port number. #1349 - Additional helper directions during syncuser conflict. #1359 +- Add `:cow` suffix to `wwctl container exec --bind` to temporarily copy files into the node image. #1365 ### Changed diff --git a/internal/app/wwctl/container/exec/child/main.go b/internal/app/wwctl/container/exec/child/main.go index 253a6ad2..ba9ad9f7 100644 --- a/internal/app/wwctl/container/exec/child/main.go +++ b/internal/app/wwctl/container/exec/child/main.go @@ -92,7 +92,7 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { wwlog.Debug("overlay options: %s", options) err = syscall.Mount("overlay", containerPath, "overlay", 0, options) if err != nil { - wwlog.Warn(fmt.Sprintf("Couldn't create overlay for ephermal mount points: %s", err)) + wwlog.Warn("Couldn't create overlay for ephermal mount points: %s", err) } } else if nodename != "" { nodeDB, err := node.New() @@ -142,6 +142,10 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { } for _, mntPnt := range mountPts { + if mntPnt.Cow { + continue + } + wwlog.Debug("bind mounting: %s -> %s", mntPnt.Source, path.Join(containerPath, mntPnt.Dest)) err = syscall.Mount(mntPnt.Source, path.Join(containerPath, mntPnt.Dest), "", syscall.MS_BIND, "") if err != nil { wwlog.Warn("Couldn't mount %s to %s: %s", mntPnt.Source, mntPnt.Dest, err) @@ -195,6 +199,9 @@ the invalid mount points. Directories always have '/' as suffix func checkMountPoints(containerName string, binds []*warewulfconf.MountEntry) (overlayObjects []string) { overlayObjects = []string{} for _, b := range binds { + if b.Cow { + continue + } _, err := os.Stat(b.Source) if err != nil { wwlog.Debug("Couldn't stat %s create no mount point in container", b.Source) diff --git a/internal/app/wwctl/container/exec/main.go b/internal/app/wwctl/container/exec/main.go index 60cf9d48..0512119c 100644 --- a/internal/app/wwctl/container/exec/main.go +++ b/internal/app/wwctl/container/exec/main.go @@ -54,7 +54,6 @@ func runContainedCmd(cmd *cobra.Command, containerName string, args []string) (e }() logStr := fmt.Sprint(wwlog.GetLogLevel()) - childArgs := []string{"--warewulfconf", conf.GetWarewulfConf(), "--loglevel", logStr, "container", "exec", "__child"} childArgs = append(childArgs, containerName) for _, b := range binds { @@ -64,8 +63,40 @@ func runContainedCmd(cmd *cobra.Command, containerName string, args []string) (e childArgs = append(childArgs, "--node", nodeName) } childArgs = append(childArgs, args...) + // copy the files into the container at this stage, es in __child the + // command syscall.Exec which replaces the __child process with the + // exec command in the container. All the mounts, have to be done in + // __child so that the used mounts don't propagate outside on the host + // (see the CLONE attributes), but as for the cow copy option we need + // to see if a file was modified after it was copied into the container + // so do this here. + // At first read out conf, the parse commandline, as copy files has the + // same synatx as mount points + mountPts := conf.MountsContainer + mountPts = append(container.InitMountPnts(binds), mountPts...) + filesToCpy := getCopyFiles(containerName, mountPts) + for i, cpyFile := range filesToCpy { + if err = util.CopyFile(cpyFile.Src, path.Join(container.RootFsDir(containerName), cpyFile.FileName)); err != nil { + return fmt.Errorf("couldn't copy files into container: %w", err) + } + // we can ignore error as the file was copied + stat, _ := os.Stat(path.Join(container.RootFsDir(containerName), cpyFile.FileName)) + filesToCpy[i].ModTime = stat.ModTime() + } wwlog.Verbose("Running contained command: %s", childArgs) - return childCommandFunc(cmd, childArgs) + retVal := childCommandFunc(cmd, childArgs) + for _, cpyFile := range filesToCpy { + if modStat, err := os.Stat(path.Join(container.RootFsDir(containerName), cpyFile.FileName)); err != nil { + wwlog.Info("copied file was removed: %s", err) + } else { + if modStat.ModTime() == cpyFile.ModTime { + if err := os.Remove(path.Join(container.RootFsDir(containerName), cpyFile.FileName)); err != nil { + wwlog.Warn("couldn't remove copied file: %s", err) + } + } + } + } + return retVal } func CobraRunE(cmd *cobra.Command, args []string) error { @@ -154,3 +185,40 @@ func SetBinds(myBinds []string) { func SetNode(myNode string) { nodeName = myNode } + +// file name and last modification time so we can remove the file if it wasn't modified +type cowFile struct { + FileName string + Src string + ModTime time.Time + Cow bool +} + +/* +Check the objects we want to copy in, instead of mounting +*/ +func getCopyFiles(containerNamer string, binds []*warewulfconf.MountEntry) (copyObjects []cowFile) { + for _, bind := range binds { + if !bind.Cow || bind.ReadOnly { + continue + } + if _, err := os.Stat(path.Join(container.RootFsDir(containerNamer), path.Dir(bind.Dest))); err != nil { + wwlog.Warn("destination directory doesn't exist: %s", err) + continue + } + if _, err := os.Stat(path.Join(container.RootFsDir(containerNamer), bind.Dest)); err == nil { + wwlog.Verbose("file exists in container: %s", bind.Dest) + continue + } + if _, err := os.Stat(bind.Source); err != nil { + wwlog.Warn("source doesn't exist: %s", err) + continue + } + copyObjects = append(copyObjects, cowFile{ + FileName: bind.Dest, + Src: bind.Source, + Cow: bind.Cow, + }) + } + return +} diff --git a/internal/app/wwctl/container/exec/root.go b/internal/app/wwctl/container/exec/root.go index 4b74c941..0b6aaf63 100644 --- a/internal/app/wwctl/container/exec/root.go +++ b/internal/app/wwctl/container/exec/root.go @@ -32,7 +32,11 @@ var ( func init() { baseCmd.AddCommand(child.GetCommand()) - baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, "Bind a local path into the container (must exist)") + baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `Bind a local path which must exist into the container. Syntax is +source[:destination[:{ro|cow}]] If destination is not set, +it will the same path as source.The additional parameter is +ro for read only and cow, which means the file is copied into +the container and removed if not modified.`) baseCmd.PersistentFlags().BoolVar(&SyncUser, "syncuser", false, "Synchronize UIDs/GIDs from host to container") baseCmd.PersistentFlags().StringVarP(&nodeName, "node", "n", "", "Create a read only view of the container for the given node") } diff --git a/internal/app/wwctl/container/shell/root.go b/internal/app/wwctl/container/shell/root.go index cceb7512..52d9825d 100644 --- a/internal/app/wwctl/container/shell/root.go +++ b/internal/app/wwctl/container/shell/root.go @@ -28,7 +28,11 @@ var ( ) func init() { - baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, "Bind a local path into the container (must exist)") + baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `Bind a local path which must exist into the container. Syntax is +source[:destination[:{ro|cow}]] If destination is not set, +it will the same path as source.The additional parameter is +ro for read only and cow, which means the file is copied into +the container and removed if not modified.`) baseCmd.PersistentFlags().StringVarP(&nodeName, "node", "n", "", "Create a read only view of the container for the given node") } diff --git a/internal/pkg/config/mounts.go b/internal/pkg/config/mounts.go index 2eb5060b..35805202 100644 --- a/internal/pkg/config/mounts.go +++ b/internal/pkg/config/mounts.go @@ -7,4 +7,5 @@ type MountEntry struct { Dest string `yaml:"dest,omitempty"` ReadOnly bool `yaml:"readonly,omitempty"` Options string `yaml:"options,omitempty"` // ignored at the moment + Cow bool `yaml:"cow,omitempty"` // copy the file into the container and don't remove if modified } diff --git a/internal/pkg/container/mountpoints.go b/internal/pkg/container/mountpoints.go index 5de00c75..7c16a0a9 100644 --- a/internal/pkg/container/mountpoints.go +++ b/internal/pkg/container/mountpoints.go @@ -21,13 +21,19 @@ func InitMountPnts(binds []string) (mounts []*warewulfconf.MountEntry) { dest = bind[1] } readonly := false - if len(bind) >= 3 && bind[2] == "ro" { - readonly = true + cow := false + if len(bind) >= 3 { + if bind[2] == "ro" { + readonly = true + } else if bind[2] == "cow" { + cow = true + } } mntPnt := warewulfconf.MountEntry{ Source: bind[0], Dest: dest, ReadOnly: readonly, + Cow: cow, } mounts = append(mounts, &mntPnt) } diff --git a/userdocs/contents/containers.rst b/userdocs/contents/containers.rst index 2e84921f..f30451a2 100644 --- a/userdocs/contents/containers.rst +++ b/userdocs/contents/containers.rst @@ -213,6 +213,21 @@ when using the exec command. This works as follows: location, as it is almost always present and empty in every Linux distribution (as prescribed by the LSB file hierarchy standard). +Files which should always be present in a container image like ``resolv.conf`` +can be specified in ``warewulf.conf`` with following container_exit + +.. code-block:: yaml + container mounts: + - source: /etc/resolv.conf + dest: /etc/resolv.conf + readonly: true + +.. note:: + Instead of the ``readonly`` setting you can set ``cow``, which + has the effect, that the source file is copied to the container + and removed if it was modified. This useful for file used for + registrations. + When the command completes, if anything within the container changed, the container will be rebuilt into a bootable static object automatically. From 173546348fb796731e48088271f6e4845e5d5c9d Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Thu, 5 Sep 2024 13:38:54 -0600 Subject: [PATCH 2/5] Edited documentation for new copy-on-write feature Signed-off-by: Jonathon Anderson --- internal/app/wwctl/container/exec/root.go | 9 ++++----- internal/app/wwctl/container/shell/root.go | 9 ++++----- userdocs/contents/containers.rst | 9 ++++----- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/internal/app/wwctl/container/exec/root.go b/internal/app/wwctl/container/exec/root.go index 0b6aaf63..bcb8cf5a 100644 --- a/internal/app/wwctl/container/exec/root.go +++ b/internal/app/wwctl/container/exec/root.go @@ -32,11 +32,10 @@ var ( func init() { baseCmd.AddCommand(child.GetCommand()) - baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `Bind a local path which must exist into the container. Syntax is -source[:destination[:{ro|cow}]] If destination is not set, -it will the same path as source.The additional parameter is -ro for read only and cow, which means the file is copied into -the container and removed if not modified.`) + baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `source[:destination[:{ro|cow}]] +Bind a local path which must exist into the container. If destination is not +set, uses the same path as source. "ro" binds read-only. "cow" temporarily +copies the file into the container.`) baseCmd.PersistentFlags().BoolVar(&SyncUser, "syncuser", false, "Synchronize UIDs/GIDs from host to container") baseCmd.PersistentFlags().StringVarP(&nodeName, "node", "n", "", "Create a read only view of the container for the given node") } diff --git a/internal/app/wwctl/container/shell/root.go b/internal/app/wwctl/container/shell/root.go index 52d9825d..df9b2c6e 100644 --- a/internal/app/wwctl/container/shell/root.go +++ b/internal/app/wwctl/container/shell/root.go @@ -28,11 +28,10 @@ var ( ) func init() { - baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `Bind a local path which must exist into the container. Syntax is -source[:destination[:{ro|cow}]] If destination is not set, -it will the same path as source.The additional parameter is -ro for read only and cow, which means the file is copied into -the container and removed if not modified.`) + baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `source[:destination[:{ro|cow}]] +Bind a local path which must exist into the container. If destination is not +set, uses the same path as source. "ro" binds read-only. "cow" temporarily +copies the file into the container.`) baseCmd.PersistentFlags().StringVarP(&nodeName, "node", "n", "", "Create a read only view of the container for the given node") } diff --git a/userdocs/contents/containers.rst b/userdocs/contents/containers.rst index f30451a2..b46f34eb 100644 --- a/userdocs/contents/containers.rst +++ b/userdocs/contents/containers.rst @@ -214,7 +214,7 @@ when using the exec command. This works as follows: distribution (as prescribed by the LSB file hierarchy standard). Files which should always be present in a container image like ``resolv.conf`` -can be specified in ``warewulf.conf`` with following container_exit +can be specified in ``warewulf.conf``: .. code-block:: yaml container mounts: @@ -223,10 +223,9 @@ can be specified in ``warewulf.conf`` with following container_exit readonly: true .. note:: - Instead of the ``readonly`` setting you can set ``cow``, which - has the effect, that the source file is copied to the container - and removed if it was modified. This useful for file used for - registrations. + Instead of ``readonly: true`` you can set ``cow: true``. This causes the + source file to be copied to the container and removed if it was not + modified. This can be useful for files used for registrations. When the command completes, if anything within the container changed, the container will be rebuilt into a bootable static object From 3a36016707cc51f13449f6a37d25bc1488f0ec14 Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Thu, 5 Sep 2024 13:39:30 -0600 Subject: [PATCH 3/5] Refactor of new copy-on-write feature Signed-off-by: Jonathon Anderson --- internal/app/wwctl/container/exec/main.go | 88 ++++++++++++----------- 1 file changed, 47 insertions(+), 41 deletions(-) diff --git a/internal/app/wwctl/container/exec/main.go b/internal/app/wwctl/container/exec/main.go index 0512119c..bffe0993 100644 --- a/internal/app/wwctl/container/exec/main.go +++ b/internal/app/wwctl/container/exec/main.go @@ -72,28 +72,18 @@ func runContainedCmd(cmd *cobra.Command, containerName string, args []string) (e // so do this here. // At first read out conf, the parse commandline, as copy files has the // same synatx as mount points - mountPts := conf.MountsContainer - mountPts = append(container.InitMountPnts(binds), mountPts...) - filesToCpy := getCopyFiles(containerName, mountPts) - for i, cpyFile := range filesToCpy { - if err = util.CopyFile(cpyFile.Src, path.Join(container.RootFsDir(containerName), cpyFile.FileName)); err != nil { - return fmt.Errorf("couldn't copy files into container: %w", err) + mountPts := append(container.InitMountPnts(binds), conf.MountsContainer...) + filesToCpy := getCopyFiles(mountPts) + for _, cpyFile := range filesToCpy { + if err := (cpyFile).copyToContainer(containerName); err != nil { + wwlog.Warn("couldn't copy file: %s", err) } - // we can ignore error as the file was copied - stat, _ := os.Stat(path.Join(container.RootFsDir(containerName), cpyFile.FileName)) - filesToCpy[i].ModTime = stat.ModTime() } wwlog.Verbose("Running contained command: %s", childArgs) retVal := childCommandFunc(cmd, childArgs) for _, cpyFile := range filesToCpy { - if modStat, err := os.Stat(path.Join(container.RootFsDir(containerName), cpyFile.FileName)); err != nil { - wwlog.Info("copied file was removed: %s", err) - } else { - if modStat.ModTime() == cpyFile.ModTime { - if err := os.Remove(path.Join(container.RootFsDir(containerName), cpyFile.FileName)); err != nil { - wwlog.Warn("couldn't remove copied file: %s", err) - } - } + if err := cpyFile.removeFromContainer(containerName); err != nil { + wwlog.Warn("couldn't remove file: %s", err) } } return retVal @@ -188,37 +178,53 @@ func SetNode(myNode string) { // file name and last modification time so we can remove the file if it wasn't modified type cowFile struct { - FileName string - Src string - ModTime time.Time - Cow bool + fileName string + src string + modTime time.Time +} + +func (this *cowFile) copyToContainer(containerName string) error { + containerDest := path.Join(container.RootFsDir(containerName), this.fileName) + if _, err := os.Stat(path.Dir(containerDest)); err != nil { + return fmt.Errorf("destination directory doesn't exist: %s", err) + } else if _, err := os.Stat(containerDest); err == nil { + return fmt.Errorf("file already exists in container: %s", this.fileName) + } else if _, err := os.Stat(this.src); err != nil { + return fmt.Errorf("source doesn't exist: %s", err) + } else if err := util.CopyFile(this.src, containerDest); err != nil { + return fmt.Errorf("couldn't copy files into container: %w", err) + } else { + // we can ignore error as the file was copied + stat, _ := os.Stat(containerDest) + this.modTime = stat.ModTime() + return nil + } +} + +func (this *cowFile) removeFromContainer(containerName string) error { + containerDest := path.Join(container.RootFsDir(containerName), this.fileName) + if this.modTime.IsZero() { + return fmt.Errorf("not previously copied: %s", this.fileName) + } else if destStat, err := os.Stat(containerDest); err != nil { + return fmt.Errorf("no longer present: %s", err) + } else if destStat.ModTime() == this.modTime { + return os.Remove(containerDest) + } else { + return fmt.Errorf("modified since copy: %s", this.fileName) + } } /* Check the objects we want to copy in, instead of mounting */ -func getCopyFiles(containerNamer string, binds []*warewulfconf.MountEntry) (copyObjects []cowFile) { +func getCopyFiles(binds []*warewulfconf.MountEntry) (copyObjects []*cowFile) { for _, bind := range binds { - if !bind.Cow || bind.ReadOnly { - continue + if bind.Cow { + copyObjects = append(copyObjects, &cowFile{ + fileName: bind.Dest, + src: bind.Source, + }) } - if _, err := os.Stat(path.Join(container.RootFsDir(containerNamer), path.Dir(bind.Dest))); err != nil { - wwlog.Warn("destination directory doesn't exist: %s", err) - continue - } - if _, err := os.Stat(path.Join(container.RootFsDir(containerNamer), bind.Dest)); err == nil { - wwlog.Verbose("file exists in container: %s", bind.Dest) - continue - } - if _, err := os.Stat(bind.Source); err != nil { - wwlog.Warn("source doesn't exist: %s", err) - continue - } - copyObjects = append(copyObjects, cowFile{ - FileName: bind.Dest, - Src: bind.Source, - Cow: bind.Cow, - }) } return } From 9e91b1c19a45e1370be7e47d3e2f270e8c17a32a Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Thu, 5 Sep 2024 13:53:33 -0600 Subject: [PATCH 4/5] Rename :cow to :copy Signed-off-by: Jonathon Anderson --- CHANGELOG.md | 1 + internal/app/wwctl/container/exec/child/main.go | 4 ++-- internal/app/wwctl/container/exec/main.go | 14 +++++++------- internal/app/wwctl/container/exec/root.go | 4 ++-- internal/app/wwctl/container/shell/root.go | 4 ++-- internal/pkg/config/mounts.go | 2 +- internal/pkg/container/mountpoints.go | 8 ++++---- userdocs/contents/containers.rst | 2 +- 8 files changed, 20 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cdb509c..1835925e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Added option for wwclient port number. #1349 - Additional helper directions during syncuser conflict. #1359 - Add `:cow` suffix to `wwctl container exec --bind` to temporarily copy files into the node image. #1365 +- Add `:copy` suffix to `wwctl container exec --bind` to temporarily copy files into the node image. #1365 ### Changed diff --git a/internal/app/wwctl/container/exec/child/main.go b/internal/app/wwctl/container/exec/child/main.go index ba9ad9f7..0785ba74 100644 --- a/internal/app/wwctl/container/exec/child/main.go +++ b/internal/app/wwctl/container/exec/child/main.go @@ -142,7 +142,7 @@ func CobraRunE(cmd *cobra.Command, args []string) (err error) { } for _, mntPnt := range mountPts { - if mntPnt.Cow { + if mntPnt.Copy { continue } wwlog.Debug("bind mounting: %s -> %s", mntPnt.Source, path.Join(containerPath, mntPnt.Dest)) @@ -199,7 +199,7 @@ the invalid mount points. Directories always have '/' as suffix func checkMountPoints(containerName string, binds []*warewulfconf.MountEntry) (overlayObjects []string) { overlayObjects = []string{} for _, b := range binds { - if b.Cow { + if b.Copy { continue } _, err := os.Stat(b.Source) diff --git a/internal/app/wwctl/container/exec/main.go b/internal/app/wwctl/container/exec/main.go index bffe0993..5aed57fa 100644 --- a/internal/app/wwctl/container/exec/main.go +++ b/internal/app/wwctl/container/exec/main.go @@ -67,7 +67,7 @@ func runContainedCmd(cmd *cobra.Command, containerName string, args []string) (e // command syscall.Exec which replaces the __child process with the // exec command in the container. All the mounts, have to be done in // __child so that the used mounts don't propagate outside on the host - // (see the CLONE attributes), but as for the cow copy option we need + // (see the CLONE attributes), but as for the copy option we need // to see if a file was modified after it was copied into the container // so do this here. // At first read out conf, the parse commandline, as copy files has the @@ -177,13 +177,13 @@ func SetNode(myNode string) { } // file name and last modification time so we can remove the file if it wasn't modified -type cowFile struct { +type copyFile struct { fileName string src string modTime time.Time } -func (this *cowFile) copyToContainer(containerName string) error { +func (this *copyFile) copyToContainer(containerName string) error { containerDest := path.Join(container.RootFsDir(containerName), this.fileName) if _, err := os.Stat(path.Dir(containerDest)); err != nil { return fmt.Errorf("destination directory doesn't exist: %s", err) @@ -201,7 +201,7 @@ func (this *cowFile) copyToContainer(containerName string) error { } } -func (this *cowFile) removeFromContainer(containerName string) error { +func (this *copyFile) removeFromContainer(containerName string) error { containerDest := path.Join(container.RootFsDir(containerName), this.fileName) if this.modTime.IsZero() { return fmt.Errorf("not previously copied: %s", this.fileName) @@ -217,10 +217,10 @@ func (this *cowFile) removeFromContainer(containerName string) error { /* Check the objects we want to copy in, instead of mounting */ -func getCopyFiles(binds []*warewulfconf.MountEntry) (copyObjects []*cowFile) { +func getCopyFiles(binds []*warewulfconf.MountEntry) (copyObjects []*copyFile) { for _, bind := range binds { - if bind.Cow { - copyObjects = append(copyObjects, &cowFile{ + if bind.Copy { + copyObjects = append(copyObjects, ©File{ fileName: bind.Dest, src: bind.Source, }) diff --git a/internal/app/wwctl/container/exec/root.go b/internal/app/wwctl/container/exec/root.go index bcb8cf5a..1152fd8e 100644 --- a/internal/app/wwctl/container/exec/root.go +++ b/internal/app/wwctl/container/exec/root.go @@ -32,9 +32,9 @@ var ( func init() { baseCmd.AddCommand(child.GetCommand()) - baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `source[:destination[:{ro|cow}]] + baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `source[:destination[:{ro|copy}]] Bind a local path which must exist into the container. If destination is not -set, uses the same path as source. "ro" binds read-only. "cow" temporarily +set, uses the same path as source. "ro" binds read-only. "copy" temporarily copies the file into the container.`) baseCmd.PersistentFlags().BoolVar(&SyncUser, "syncuser", false, "Synchronize UIDs/GIDs from host to container") baseCmd.PersistentFlags().StringVarP(&nodeName, "node", "n", "", "Create a read only view of the container for the given node") diff --git a/internal/app/wwctl/container/shell/root.go b/internal/app/wwctl/container/shell/root.go index df9b2c6e..e1ae01d3 100644 --- a/internal/app/wwctl/container/shell/root.go +++ b/internal/app/wwctl/container/shell/root.go @@ -28,9 +28,9 @@ var ( ) func init() { - baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `source[:destination[:{ro|cow}]] + baseCmd.PersistentFlags().StringArrayVarP(&binds, "bind", "b", []string{}, `source[:destination[:{ro|copy}]] Bind a local path which must exist into the container. If destination is not -set, uses the same path as source. "ro" binds read-only. "cow" temporarily +set, uses the same path as source. "ro" binds read-only. "copy" temporarily copies the file into the container.`) baseCmd.PersistentFlags().StringVarP(&nodeName, "node", "n", "", "Create a read only view of the container for the given node") } diff --git a/internal/pkg/config/mounts.go b/internal/pkg/config/mounts.go index 35805202..305fbad4 100644 --- a/internal/pkg/config/mounts.go +++ b/internal/pkg/config/mounts.go @@ -7,5 +7,5 @@ type MountEntry struct { Dest string `yaml:"dest,omitempty"` ReadOnly bool `yaml:"readonly,omitempty"` Options string `yaml:"options,omitempty"` // ignored at the moment - Cow bool `yaml:"cow,omitempty"` // copy the file into the container and don't remove if modified + Copy bool `yaml:"copy,omitempty"` // temporarily copy the file into the container } diff --git a/internal/pkg/container/mountpoints.go b/internal/pkg/container/mountpoints.go index 7c16a0a9..961513fb 100644 --- a/internal/pkg/container/mountpoints.go +++ b/internal/pkg/container/mountpoints.go @@ -21,19 +21,19 @@ func InitMountPnts(binds []string) (mounts []*warewulfconf.MountEntry) { dest = bind[1] } readonly := false - cow := false + copy_ := false if len(bind) >= 3 { if bind[2] == "ro" { readonly = true - } else if bind[2] == "cow" { - cow = true + } else if bind[2] == "copy" { + copy_ = true } } mntPnt := warewulfconf.MountEntry{ Source: bind[0], Dest: dest, ReadOnly: readonly, - Cow: cow, + Copy: copy_, } mounts = append(mounts, &mntPnt) } diff --git a/userdocs/contents/containers.rst b/userdocs/contents/containers.rst index b46f34eb..2ee7a236 100644 --- a/userdocs/contents/containers.rst +++ b/userdocs/contents/containers.rst @@ -223,7 +223,7 @@ can be specified in ``warewulf.conf``: readonly: true .. note:: - Instead of ``readonly: true`` you can set ``cow: true``. This causes the + Instead of ``readonly: true`` you can set ``copy: true``. This causes the source file to be copied to the container and removed if it was not modified. This can be useful for files used for registrations. From 5860334a8969bf2b22dcb7c043ef87d4326619de Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Fri, 6 Sep 2024 12:23:58 -0600 Subject: [PATCH 5/5] Improve error handling during container file copy removal Based on feedback at #1365 Signed-off-by: Jonathon Anderson --- internal/app/wwctl/container/exec/main.go | 44 +++++++++++++++-------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/internal/app/wwctl/container/exec/main.go b/internal/app/wwctl/container/exec/main.go index 5aed57fa..336218a9 100644 --- a/internal/app/wwctl/container/exec/main.go +++ b/internal/app/wwctl/container/exec/main.go @@ -82,8 +82,10 @@ func runContainedCmd(cmd *cobra.Command, containerName string, args []string) (e wwlog.Verbose("Running contained command: %s", childArgs) retVal := childCommandFunc(cmd, childArgs) for _, cpyFile := range filesToCpy { - if err := cpyFile.removeFromContainer(containerName); err != nil { - wwlog.Warn("couldn't remove file: %s", err) + if cpyFile.shouldRemoveFromContainer(containerName) { + if err := cpyFile.removeFromContainer(containerName); err != nil { + wwlog.Warn("couldn't remove file: %s", err) + } } } return retVal @@ -183,37 +185,49 @@ type copyFile struct { modTime time.Time } +func (this *copyFile) containerDest(containerName string) string { + return path.Join(container.RootFsDir(containerName), this.fileName) +} + func (this *copyFile) copyToContainer(containerName string) error { - containerDest := path.Join(container.RootFsDir(containerName), this.fileName) + containerDest := this.containerDest(containerName) if _, err := os.Stat(path.Dir(containerDest)); err != nil { - return fmt.Errorf("destination directory doesn't exist: %s", err) + return err } else if _, err := os.Stat(containerDest); err == nil { - return fmt.Errorf("file already exists in container: %s", this.fileName) + return err } else if _, err := os.Stat(this.src); err != nil { - return fmt.Errorf("source doesn't exist: %s", err) + return err } else if err := util.CopyFile(this.src, containerDest); err != nil { - return fmt.Errorf("couldn't copy files into container: %w", err) + return err + } else if stat, err := os.Stat(containerDest); err != nil { + return err } else { - // we can ignore error as the file was copied - stat, _ := os.Stat(containerDest) this.modTime = stat.ModTime() return nil } } -func (this *copyFile) removeFromContainer(containerName string) error { - containerDest := path.Join(container.RootFsDir(containerName), this.fileName) +func (this *copyFile) shouldRemoveFromContainer(containerName string) bool { + containerDest := this.containerDest(containerName) if this.modTime.IsZero() { - return fmt.Errorf("not previously copied: %s", this.fileName) + wwlog.Debug("file was not previously copied: %s", this.fileName) + return false } else if destStat, err := os.Stat(containerDest); err != nil { - return fmt.Errorf("no longer present: %s", err) + wwlog.Verbose("file is no longer present: %s (%s)", this.fileName, err) + return false } else if destStat.ModTime() == this.modTime { - return os.Remove(containerDest) + wwlog.Verbose("don't remove modified file:", this.fileName) + return false } else { - return fmt.Errorf("modified since copy: %s", this.fileName) + return true } } +func (this *copyFile) removeFromContainer(containerName string) error { + containerDest := this.containerDest(containerName) + return os.Remove(containerDest) +} + /* Check the objects we want to copy in, instead of mounting */