From 1dd7f13dfbc1dd377eabace0239b1c05cd60b144 Mon Sep 17 00:00:00 2001 From: baude Date: Thu, 25 Oct 2018 13:39:25 -0500 Subject: get user and group information using securejoin and runc's user library for the purposes of performance and security, we use securejoin to contstruct the root fs's path so that symlinks are what they appear to be and no pointing to something naughty. then instead of chrooting to parse /etc/passwd|/etc/group, we now use the runc user/group methods which saves us quite a bit of performance. Signed-off-by: baude --- libpod/container_internal.go | 56 +++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 24 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 2af216358..d928c4aed 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/opencontainers/runc/libcontainer/user" "io" "io/ioutil" "os" @@ -14,9 +15,9 @@ import ( "syscall" "github.com/containers/buildah/imagebuildah" - "github.com/containers/libpod/pkg/chrootuser" "github.com/containers/libpod/pkg/hooks" "github.com/containers/libpod/pkg/hooks/exec" + "github.com/containers/libpod/pkg/lookup" "github.com/containers/libpod/pkg/resolvconf" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/pkg/secrets" @@ -1029,7 +1030,8 @@ func (c *Container) writeStringToRundir(destFile, output string) (string, error) func (c *Container) generatePasswd() (string, error) { var ( groupspec string - gid uint32 + group *user.Group + gid int ) if c.config.User == "" { return "", nil @@ -1044,21 +1046,27 @@ func (c *Container) generatePasswd() (string, error) { if err != nil { return "", nil } - // if UID exists inside of container rootfs /etc/passwd then - // don't generate passwd - if _, _, err := chrootuser.LookupUIDInContainer(c.state.Mountpoint, uid); err == nil { + // Lookup the user to see if it exists in the container image + _, err = lookup.GetUser(c.state.Mountpoint, userspec) + if err != nil && err != user.ErrNoPasswdEntries { + return "", err + } + if err == nil { return "", nil } - if err == nil && groupspec != "" { + if groupspec != "" { if !c.state.Mounted { return "", errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate group field for passwd record", c.ID()) } - gid, err = chrootuser.GetGroup(c.state.Mountpoint, groupspec) + group, err = lookup.GetGroup(c.state.Mountpoint, groupspec) if err != nil { - return "", errors.Wrapf(err, "unable to get gid from %s formporary passwd file") + if err == user.ErrNoGroupEntries { + return "", errors.Wrapf(err, "unable to get gid %s from group file", groupspec) + } + return "", err } + gid = group.Gid } - originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") orig, err := ioutil.ReadFile(originPasswdFile) if err != nil { @@ -1153,6 +1161,7 @@ func (c *Container) generateHosts() (string, error) { } func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) error { + var uid, gid int mountPoint := c.state.Mountpoint if !c.state.Mounted { return errors.Wrapf(ErrInternal, "container is not mounted") @@ -1176,6 +1185,18 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) } } + if c.config.User != "" { + if !c.state.Mounted { + return errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID()) + } + execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, c.config.User, nil) + if err != nil { + return err + } + uid = execUser.Uid + gid = execUser.Gid + } + for k := range imageData.ContainerConfig.Volumes { mount := spec.Mount{ Destination: k, @@ -1186,19 +1207,6 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) continue } volumePath := filepath.Join(c.config.StaticDir, "volumes", k) - var ( - uid uint32 - gid uint32 - ) - if c.config.User != "" { - if !c.state.Mounted { - return errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID()) - } - uid, gid, err = chrootuser.GetUser(c.state.Mountpoint, c.config.User) - if err != nil { - return err - } - } // Ensure the symlinks are resolved resolvedSymlink, err := imagebuildah.ResolveSymLink(mountPoint, k) @@ -1218,7 +1226,7 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) return errors.Wrapf(err, "error creating directory %q for volume %q in container %q", volumePath, k, c.ID()) } - if err = os.Chown(srcPath, int(uid), int(gid)); err != nil { + if err = os.Chown(srcPath, uid, gid); err != nil { return errors.Wrapf(err, "error chowning directory %q for volume %q in container %q", srcPath, k, c.ID()) } } @@ -1228,7 +1236,7 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) return errors.Wrapf(err, "error creating directory %q for volume %q in container %q", volumePath, k, c.ID()) } - if err = os.Chown(volumePath, int(uid), int(gid)); err != nil { + if err = os.Chown(volumePath, uid, gid); err != nil { return errors.Wrapf(err, "error chowning directory %q for volume %q in container %q", volumePath, k, c.ID()) } -- cgit v1.2.3-54-g00ecf From 879f9116de8a186558fb4e7b6a259b1b8ad1aaca Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Fri, 2 Nov 2018 13:19:15 -0400 Subject: Add hostname to /etc/hosts Signed-off-by: Qi Wang --- libpod/container_internal.go | 4 ++++ test/e2e/run_dns_test.go | 21 ++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d928c4aed..9bdf4b8fc 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1157,6 +1157,10 @@ func (c *Container) generateHosts() (string, error) { hosts += fmt.Sprintf("%s %s\n", fields[1], fields[0]) } } + if len(c.state.NetworkStatus) > 0 && len(c.state.NetworkStatus[0].IPs) > 0 { + ipAddress := strings.Split(c.state.NetworkStatus[0].IPs[0].Address.String(), "/")[0] + hosts += fmt.Sprintf("%s\t%s\n", ipAddress, c.Hostname()) + } return c.writeStringToRundir("hosts", hosts) } diff --git a/test/e2e/run_dns_test.go b/test/e2e/run_dns_test.go index c5a02c776..674a57aeb 100644 --- a/test/e2e/run_dns_test.go +++ b/test/e2e/run_dns_test.go @@ -1,9 +1,10 @@ package integration import ( + "fmt" "os" + "strings" - "fmt" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -82,5 +83,23 @@ var _ = Describe("Podman run dns", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).To(Equal("foobar")) + + session = podmanTest.Podman([]string{"run", "-d", "--hostname=foobar", ALPINE, "cat", "/etc/hosts"}) + session.WaitWithDefaultTimeout() + cid := session.OutputToString() + session = podmanTest.Podman([]string{"start", "-ia", cid}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + session.LineInOutputContains("foobar") + line := strings.Split(session.OutputToStringArray()[len(session.OutputToStringArray())-1], "\t") + ip1 := line[0] + + session = podmanTest.Podman([]string{"start", "-ia", cid}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + session.LineInOutputContains("foobar") + line = strings.Split(session.OutputToStringArray()[len(session.OutputToStringArray())-1], "\t") + ip2 := line[0] + Expect(ip2).To(Not(Equal(ip1))) }) }) -- cgit v1.2.3-54-g00ecf From f714ee4fb14085c326ee5d56432781a01ffabbb0 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 19 Oct 2018 13:53:15 -0400 Subject: Actually save changes from post-stop sync After stopping containers, we run updateContainerStatus to sync our state with runc (pick up exit code, for example). Then we proceed to not save this to the database, requiring us to grab it again on the next sync. This should remove the need to read the exit file more than once. Signed-off-by: Matthew Heon --- libpod/container_internal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d928c4aed..867d539be 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -679,7 +679,7 @@ func (c *Container) stop(timeout uint) error { } // Container should clean itself up - return nil + return c.save() } // Internal, non-locking function to pause a container -- cgit v1.2.3-54-g00ecf From 140f87c474f5a075d5833335593895491b83dab6 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 19 Oct 2018 16:02:14 -0400 Subject: EXPERIMENTAL: Do not call out to runc for sync When syncing container state, we normally call out to runc to see the container's status. This does have significant performance implications, though, and we've seen issues with large amounts of runc processes being spawned. This patch attempts to use stat calls on the container exit file created by Conmon instead to sync state. This massively decreases the cost of calling updateContainer (it has gone from an almost-unconditional fork/exec of runc to a single stat call that can be avoided in most states). Signed-off-by: Matthew Heon --- libpod/container_api.go | 2 +- libpod/container_internal.go | 90 +++++++++++++++++++++++++++++++++++++++----- libpod/oci.go | 58 +++++++++++++++++----------- libpod/runtime_ctr.go | 2 +- 4 files changed, 118 insertions(+), 34 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_api.go b/libpod/container_api.go index 30c67eb2a..25c1fb5f9 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -685,7 +685,7 @@ func (c *Container) Sync() error { (c.state.State != ContainerStateConfigured) { oldState := c.state.State // TODO: optionally replace this with a stat for the exit file - if err := c.runtime.ociRuntime.updateContainerStatus(c); err != nil { + if err := c.runtime.ociRuntime.updateContainerStatus(c, true); err != nil { return err } // Only save back to DB if state changed diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 867d539be..db1cf6c99 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -13,8 +13,10 @@ import ( "strconv" "strings" "syscall" + "time" "github.com/containers/buildah/imagebuildah" + "github.com/containers/libpod/pkg/ctime" "github.com/containers/libpod/pkg/hooks" "github.com/containers/libpod/pkg/hooks/exec" "github.com/containers/libpod/pkg/lookup" @@ -31,6 +33,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/text/language" + kwait "k8s.io/apimachinery/pkg/util/wait" ) const ( @@ -147,6 +150,77 @@ func (c *Container) execPidPath(sessionID string) string { return filepath.Join(c.state.RunDir, "exec_pid_"+sessionID) } +// exitFilePath gets the path to the container's exit file +func (c *Container) exitFilePath() string { + return filepath.Join(c.runtime.ociRuntime.exitsDir, c.ID()) +} + +// Wait for the container's exit file to appear. +// When it does, update our state based on it. +func (c *Container) waitForExitFileAndSync() error { + exitFile := c.exitFilePath() + + err := kwait.ExponentialBackoff( + kwait.Backoff{ + Duration: 500 * time.Millisecond, + Factor: 1.2, + Steps: 6, + }, + func() (bool, error) { + _, err := os.Stat(exitFile) + if err != nil { + // wait longer + return false, nil + } + return true, nil + }) + if err != nil { + // Exit file did not appear + // Reset our state + c.state.ExitCode = -1 + c.state.FinishedTime = time.Now() + c.state.State = ContainerStateStopped + + if err2 := c.save(); err2 != nil { + logrus.Errorf("Error saving container %s state: %v", c.ID(), err2) + } + + return err + } + + if err := c.runtime.ociRuntime.updateContainerStatus(c, false); err != nil { + return err + } + + return c.save() +} + +// Handle the container exit file. +// The exit file is used to supply container exit time and exit code. +// This assumes the exit file already exists. +func (c *Container) handleExitFile(exitFile string, fi os.FileInfo) error { + c.state.FinishedTime = ctime.Created(fi) + statusCodeStr, err := ioutil.ReadFile(exitFile) + if err != nil { + return errors.Wrapf(err, "failed to read exit file for container %s", c.ID()) + } + statusCode, err := strconv.Atoi(string(statusCodeStr)) + if err != nil { + return errors.Wrapf(err, "error converting exit status code for container %s to int", + c.ID()) + } + c.state.ExitCode = int32(statusCode) + + oomFilePath := filepath.Join(c.bundlePath(), "oom") + if _, err = os.Stat(oomFilePath); err == nil { + c.state.OOMKilled = true + } + + c.state.Exited = true + + return nil +} + // Sync this container with on-disk state and runtime status // Should only be called with container lock held // This function should suffice to ensure a container's state is accurate and @@ -162,7 +236,7 @@ func (c *Container) syncContainer() error { (c.state.State != ContainerStateExited) { oldState := c.state.State // TODO: optionally replace this with a stat for the exit file - if err := c.runtime.ociRuntime.updateContainerStatus(c); err != nil { + if err := c.runtime.ociRuntime.updateContainerStatus(c, false); err != nil { return err } // Only save back to DB if state changed @@ -660,7 +734,10 @@ func (c *Container) start() error { } logrus.Debugf("Started container %s", c.ID()) - c.state.State = ContainerStateRunning + // We need to pick up full container state, including PID. + if err := c.runtime.ociRuntime.updateContainerStatus(c, true); err != nil { + return err + } return c.save() } @@ -673,13 +750,8 @@ func (c *Container) stop(timeout uint) error { return err } - // Sync the container's state to pick up return code - if err := c.runtime.ociRuntime.updateContainerStatus(c); err != nil { - return err - } - - // Container should clean itself up - return c.save() + // Wait until we have an exit file, and sync once we do + return c.waitForExitFileAndSync() } // Internal, non-locking function to pause a container diff --git a/libpod/oci.go b/libpod/oci.go index ca8f967c4..b390b1967 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -11,12 +11,10 @@ import ( "os/exec" "path/filepath" "runtime" - "strconv" "strings" "syscall" "time" - "github.com/containers/libpod/pkg/ctime" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/pkg/util" "github.com/coreos/go-systemd/activation" @@ -451,17 +449,47 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res // updateContainerStatus retrieves the current status of the container from the // runtime. It updates the container's state but does not save it. -func (r *OCIRuntime) updateContainerStatus(ctr *Container) error { - state := new(spec.State) +// If useRunc is false, we will not directly hit runc to see the container's +// status, but will instead only check for the existance of the conmon exit file +// and update state to stopped if it exists. +func (r *OCIRuntime) updateContainerStatus(ctr *Container, useRunc bool) error { + exitFile := ctr.exitFilePath() runtimeDir, err := util.GetRootlessRuntimeDir() if err != nil { return err } + // If not using runc, we don't need to do most of this. + if !useRunc { + // If the container's not running, nothing to do. + if ctr.state.State != ContainerStateRunning { + return nil + } + + // Check for the exit file conmon makes + info, err := os.Stat(exitFile) + if err != nil { + if os.IsNotExist(err) { + // Container is still running, no error + return nil + } + + return errors.Wrapf(err, "error running stat on container %s exit file", ctr.ID()) + } + + // Alright, it exists. Transition to Stopped state. + ctr.state.State = ContainerStateStopped + + // Read the exit file to get our stopped time and exit code. + return ctr.handleExitFile(exitFile, info) + } + // Store old state so we know if we were already stopped oldState := ctr.state.State + state := new(spec.State) + cmd := exec.Command(r.path, "state", ctr.ID()) cmd.Env = append(cmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)) outPipe, err := cmd.StdoutPipe() @@ -514,7 +542,6 @@ func (r *OCIRuntime) updateContainerStatus(ctr *Container) error { // Only grab exit status if we were not already stopped // If we were, it should already be in the database if ctr.state.State == ContainerStateStopped && oldState != ContainerStateStopped { - exitFile := filepath.Join(r.exitsDir, ctr.ID()) var fi os.FileInfo err = kwait.ExponentialBackoff( kwait.Backoff{ @@ -538,24 +565,7 @@ func (r *OCIRuntime) updateContainerStatus(ctr *Container) error { return nil } - ctr.state.FinishedTime = ctime.Created(fi) - statusCodeStr, err := ioutil.ReadFile(exitFile) - if err != nil { - return errors.Wrapf(err, "failed to read exit file for container %s", ctr.ID()) - } - statusCode, err := strconv.Atoi(string(statusCodeStr)) - if err != nil { - return errors.Wrapf(err, "error converting exit status code for container %s to int", - ctr.ID()) - } - ctr.state.ExitCode = int32(statusCode) - - oomFilePath := filepath.Join(ctr.bundlePath(), "oom") - if _, err = os.Stat(oomFilePath); err == nil { - ctr.state.OOMKilled = true - } - - ctr.state.Exited = true + return ctr.handleExitFile(exitFile, fi) } return nil @@ -601,6 +611,8 @@ func (r *OCIRuntime) killContainer(ctr *Container, signal uint) error { // Does not set finished time for container, assumes you will run updateStatus // after to pull the exit code func (r *OCIRuntime) stopContainer(ctr *Container, timeout uint) error { + logrus.Debugf("Stopping container %s (PID %d)", ctr.ID(), ctr.state.PID) + // Ping the container to see if it's alive // If it's not, it's already stopped, return err := unix.Kill(ctr.state.PID, 0) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index b63726f29..09dc7c48b 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -256,7 +256,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) } // Need to update container state to make sure we know it's stopped - if err := c.syncContainer(); err != nil { + if err := c.waitForExitFileAndSync(); err != nil { return err } } else if !(c.state.State == ContainerStateConfigured || -- cgit v1.2.3-54-g00ecf From 3286b0185debc809664d854dd2ee75bf75efadc4 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 25 Oct 2018 13:35:39 -0400 Subject: Retrieve container PID from conmon Instead of running a full sync after starting a container to pick up its PID, grab it from Conmon instead. Signed-off-by: Matthew Heon --- libpod/container_internal.go | 5 ----- libpod/oci.go | 1 + 2 files changed, 1 insertion(+), 5 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index db1cf6c99..9bb45a08a 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -734,11 +734,6 @@ func (c *Container) start() error { } logrus.Debugf("Started container %s", c.ID()) - // We need to pick up full container state, including PID. - if err := c.runtime.ociRuntime.updateContainerStatus(c, true); err != nil { - return err - } - return c.save() } diff --git a/libpod/oci.go b/libpod/oci.go index e8a97226a..d3ce70133 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -441,6 +441,7 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res } return errors.Wrapf(ErrInternal, "container create failed") } + ctr.state.PID = ss.si.Pid case <-time.After(ContainerCreateTimeout): return errors.Wrapf(ErrInternal, "container creation timeout") } -- cgit v1.2.3-54-g00ecf From c9e9ca5671775652b2e097641713084ea367a7fa Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 25 Oct 2018 14:11:44 -0400 Subject: Properly set Running state when starting containers Signed-off-by: Matthew Heon --- libpod/container_internal.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 9bb45a08a..5221ac5b6 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -734,6 +734,8 @@ func (c *Container) start() error { } logrus.Debugf("Started container %s", c.ID()) + c.state.State = ContainerStateRunning + return c.save() } -- cgit v1.2.3-54-g00ecf From 536af1f68944c68956e705d589e11cd52875273a Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 29 Oct 2018 13:20:26 -0400 Subject: Print error status code if we fail to parse it When we read the conmon error status file, if Atoi fails to parse the string we read from the file as an int, print the string as part of the error message so we know what might have gone wrong. Signed-off-by: Matthew Heon --- libpod/container_internal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 5221ac5b6..54558f3fc 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -206,8 +206,8 @@ func (c *Container) handleExitFile(exitFile string, fi os.FileInfo) error { } statusCode, err := strconv.Atoi(string(statusCodeStr)) if err != nil { - return errors.Wrapf(err, "error converting exit status code for container %s to int", - c.ID()) + return errors.Wrapf(err, "error converting exit status code (%q) for container %s to int", + c.ID(), statusCodeStr) } c.state.ExitCode = int32(statusCode) -- cgit v1.2.3-54-g00ecf From ae68bec75cf59e8a530dbc55f320f7bb0be9a62b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 6 Nov 2018 06:26:35 -0500 Subject: Don't fail if /etc/passwd or /etc/group does not exists Container images can be created without passwd or group file, currently if one of these containers gets run with a --user flag the container blows up complaining about t a missing /etc/passwd file. We just need to check if the error on read is ENOEXIST then allow the read to return, not fail. Signed-off-by: Daniel J Walsh --- libpod/container_internal.go | 4 ++-- pkg/lookup/lookup.go | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d928c4aed..558099e82 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/opencontainers/runc/libcontainer/user" "io" "io/ioutil" "os" @@ -25,6 +24,7 @@ import ( "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/chrootarchive" "github.com/containers/storage/pkg/mount" + "github.com/opencontainers/runc/libcontainer/user" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/selinux/go-selinux/label" @@ -1069,7 +1069,7 @@ func (c *Container) generatePasswd() (string, error) { } originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") orig, err := ioutil.ReadFile(originPasswdFile) - if err != nil { + if err != nil && !os.IsNotExist(err) { return "", errors.Wrapf(err, "unable to read passwd file %s", originPasswdFile) } diff --git a/pkg/lookup/lookup.go b/pkg/lookup/lookup.go index b27e2a724..a9d975b4b 100644 --- a/pkg/lookup/lookup.go +++ b/pkg/lookup/lookup.go @@ -1,10 +1,12 @@ package lookup import ( + "os" + "strconv" + "github.com/cyphar/filepath-securejoin" "github.com/opencontainers/runc/libcontainer/user" "github.com/sirupsen/logrus" - "strconv" ) const ( @@ -116,7 +118,7 @@ func GetUser(containerMount, userIDorName string) (*user.User, error) { } return u.Uid == uid }) - if err != nil { + if err != nil && !os.IsNotExist(err) { return nil, err } if len(users) > 0 { @@ -146,7 +148,7 @@ func GetGroup(containerMount, groupIDorName string) (*user.Group, error) { } return g.Gid == gid }) - if err != nil { + if err != nil && !os.IsNotExist(err) { return nil, err } if len(groups) > 0 { -- cgit v1.2.3-54-g00ecf From e022efa0f87b8c0d59ce8efeb370b3eeed145a28 Mon Sep 17 00:00:00 2001 From: baude Date: Wed, 7 Nov 2018 10:44:33 -0600 Subject: move defer'd function declaration ahead of prepare error return Signed-off-by: baude --- libpod/container_api.go | 21 ++++++++++++--------- libpod/container_internal.go | 13 +++++++------ libpod/container_internal_linux.go | 7 ++++--- 3 files changed, 23 insertions(+), 18 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_api.go b/libpod/container_api.go index 30c67eb2a..c706c5b30 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -46,9 +46,6 @@ func (c *Container) Init(ctx context.Context) (err error) { return errors.Wrapf(ErrCtrStateInvalid, "some dependencies of container %s are not started: %s", c.ID(), depString) } - if err := c.prepare(); err != nil { - return err - } defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -57,6 +54,10 @@ func (c *Container) Init(ctx context.Context) (err error) { } }() + if err := c.prepare(); err != nil { + return err + } + if c.state.State == ContainerStateStopped { // Reinitialize the container return c.reinit(ctx) @@ -99,9 +100,6 @@ func (c *Container) Start(ctx context.Context) (err error) { return errors.Wrapf(ErrCtrStateInvalid, "some dependencies of container %s are not started: %s", c.ID(), depString) } - if err := c.prepare(); err != nil { - return err - } defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -110,6 +108,10 @@ func (c *Container) Start(ctx context.Context) (err error) { } }() + if err := c.prepare(); err != nil { + return err + } + if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to if err := c.reinit(ctx); err != nil { @@ -164,9 +166,6 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, return nil, errors.Wrapf(ErrCtrStateInvalid, "some dependencies of container %s are not started: %s", c.ID(), depString) } - if err := c.prepare(); err != nil { - return nil, err - } defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -175,6 +174,10 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, } }() + if err := c.prepare(); err != nil { + return nil, err + } + if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to if err := c.reinit(ctx); err != nil { diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d928c4aed..de15736f4 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -623,9 +623,6 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { return errors.Wrapf(ErrCtrStateInvalid, "cannot start paused container %s", c.ID()) } - if err := c.prepare(); err != nil { - return err - } defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -634,6 +631,10 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { } }() + if err := c.prepare(); err != nil { + return err + } + // If we are ContainerStateStopped we need to remove from runtime // And reset to ContainerStateConfigured if c.state.State == ContainerStateStopped { @@ -719,9 +720,6 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e return err } } - if err := c.prepare(); err != nil { - return err - } defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -729,6 +727,9 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e } } }() + if err := c.prepare(); err != nil { + return err + } if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 7bf2c71ca..df94a0571 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -484,9 +484,6 @@ func (c *Container) restore(ctx context.Context, keep bool) (err error) { } } - if err := c.prepare(); err != nil { - return err - } defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -495,6 +492,10 @@ func (c *Container) restore(ctx context.Context, keep bool) (err error) { } }() + if err := c.prepare(); err != nil { + return err + } + // TODO: use existing way to request static IPs, once it is merged in ocicni // https://github.com/cri-o/ocicni/pull/23/ -- cgit v1.2.3-54-g00ecf From bb6c1cf8d1667c7c8e4d539ea2250a18fa89a58a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 8 Nov 2018 06:12:14 -0500 Subject: libpod should know if the network is disabled /etc/resolv.conf and /etc/hosts should not be created and mounted when the network is disabled. We should not be calling the network setup and cleanup functions when it is disabled either. In doing this patch, I found that all of the bind mounts were particular to Linux along with the generate functions, so I moved them to container_internal_linux.go Since we are checking if we are using a network namespace, we need to check after the network namespaces has been created in the spec. Signed-off-by: Daniel J Walsh --- cmd/podman/create.go | 5 + libpod/container.go | 12 ++ libpod/container_internal.go | 228 +---------------------------------- libpod/container_internal_linux.go | 235 +++++++++++++++++++++++++++++++++++++ 4 files changed, 253 insertions(+), 227 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/cmd/podman/create.go b/cmd/podman/create.go index 9f6825c95..bcf830c7c 100644 --- a/cmd/podman/create.go +++ b/cmd/podman/create.go @@ -670,6 +670,11 @@ func parseCreateOpts(ctx context.Context, c *cli.Context, runtime *libpod.Runtim if util.StringInSlice(".", c.StringSlice("dns-search")) && len(c.StringSlice("dns-search")) > 1 { return nil, errors.Errorf("cannot pass additional search domains when also specifying '.'") } + if !netMode.IsPrivate() { + if c.IsSet("dns-search") || c.IsSet("dns") || c.IsSet("dns-opt") { + return nil, errors.Errorf("specifying DNS flags when network mode is shared with the host or another container is not allowed") + } + } // Validate domains are good for _, dom := range c.StringSlice("dns-search") { diff --git a/libpod/container.go b/libpod/container.go index 7bb5b2687..16f61d021 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -996,3 +996,15 @@ func (c *Container) IsInfra() bool { func (c *Container) IsReadOnly() bool { return c.config.Spec.Root.Readonly } + +// NetworkDisabled returns whether the container is running with a disabled network +func (c *Container) NetworkDisabled() bool { + if !c.config.PostConfigureNetNS { + for _, ns := range c.config.Spec.Linux.Namespaces { + if ns.Type == spec.NetworkNamespace { + return ns.Path == "" + } + } + } + return false +} diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d2f48d661..051e0aeb7 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -19,14 +19,11 @@ import ( "github.com/containers/libpod/pkg/hooks" "github.com/containers/libpod/pkg/hooks/exec" "github.com/containers/libpod/pkg/lookup" - "github.com/containers/libpod/pkg/resolvconf" "github.com/containers/libpod/pkg/rootless" - "github.com/containers/libpod/pkg/secrets" "github.com/containers/storage" "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/chrootarchive" "github.com/containers/storage/pkg/mount" - "github.com/opencontainers/runc/libcontainer/user" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/selinux/go-selinux/label" @@ -583,7 +580,7 @@ func (c *Container) checkDependenciesRunningLocked(depCtrs map[string]*Container } func (c *Container) completeNetworkSetup() error { - if !c.config.PostConfigureNetNS { + if !c.config.PostConfigureNetNS || c.NetworkDisabled() { return nil } if err := c.syncContainer(); err != nil { @@ -597,10 +594,6 @@ func (c *Container) completeNetworkSetup() error { // Initialize a container, creating it in the runtime func (c *Container) init(ctx context.Context) error { - if err := c.makeBindMounts(); err != nil { - return err - } - // Generate the OCI spec spec, err := c.generateSpec(ctx) if err != nil { @@ -987,86 +980,6 @@ func (c *Container) postDeleteHooks(ctx context.Context) (err error) { return nil } -// Make standard bind mounts to include in the container -func (c *Container) makeBindMounts() error { - if err := os.Chown(c.state.RunDir, c.RootUID(), c.RootGID()); err != nil { - return errors.Wrapf(err, "cannot chown run directory %s", c.state.RunDir) - } - - if c.state.BindMounts == nil { - c.state.BindMounts = make(map[string]string) - } - - // SHM is always added when we mount the container - c.state.BindMounts["/dev/shm"] = c.config.ShmDir - - // Make /etc/resolv.conf - if _, ok := c.state.BindMounts["/etc/resolv.conf"]; ok { - // If it already exists, delete so we can recreate - delete(c.state.BindMounts, "/etc/resolv.conf") - } - newResolv, err := c.generateResolvConf() - if err != nil { - return errors.Wrapf(err, "error creating resolv.conf for container %s", c.ID()) - } - c.state.BindMounts["/etc/resolv.conf"] = newResolv - - newPasswd, err := c.generatePasswd() - if err != nil { - return errors.Wrapf(err, "error creating temporary passwd file for container %s", c.ID()) - } - if newPasswd != "" { - // Make /etc/passwd - if _, ok := c.state.BindMounts["/etc/passwd"]; ok { - // If it already exists, delete so we can recreate - delete(c.state.BindMounts, "/etc/passwd") - } - logrus.Debugf("adding entry to /etc/passwd for non existent default user") - c.state.BindMounts["/etc/passwd"] = newPasswd - } - // Make /etc/hosts - if _, ok := c.state.BindMounts["/etc/hosts"]; ok { - // If it already exists, delete so we can recreate - delete(c.state.BindMounts, "/etc/hosts") - } - newHosts, err := c.generateHosts() - if err != nil { - return errors.Wrapf(err, "error creating hosts file for container %s", c.ID()) - } - c.state.BindMounts["/etc/hosts"] = newHosts - - // Make /etc/hostname - // This should never change, so no need to recreate if it exists - if _, ok := c.state.BindMounts["/etc/hostname"]; !ok { - hostnamePath, err := c.writeStringToRundir("hostname", c.Hostname()) - if err != nil { - return errors.Wrapf(err, "error creating hostname file for container %s", c.ID()) - } - c.state.BindMounts["/etc/hostname"] = hostnamePath - } - - // Make .containerenv - // Empty file, so no need to recreate if it exists - if _, ok := c.state.BindMounts["/run/.containerenv"]; !ok { - // Empty string for now, but we may consider populating this later - containerenvPath, err := c.writeStringToRundir(".containerenv", "") - if err != nil { - return errors.Wrapf(err, "error creating containerenv file for container %s", c.ID()) - } - c.state.BindMounts["/run/.containerenv"] = containerenvPath - } - - // Add Secret Mounts - secretMounts := secrets.SecretMountsWithUIDGID(c.config.MountLabel, c.state.RunDir, c.runtime.config.DefaultMountsFile, c.state.DestinationRunDir, c.RootUID(), c.RootGID()) - for _, mount := range secretMounts { - if _, ok := c.state.BindMounts[mount.Destination]; !ok { - c.state.BindMounts[mount.Destination] = mount.Source - } - } - - return nil -} - // writeStringToRundir copies the provided file to the runtimedir func (c *Container) writeStringToRundir(destFile, output string) (string, error) { destFileName := filepath.Join(c.state.RunDir, destFile) @@ -1095,145 +1008,6 @@ func (c *Container) writeStringToRundir(destFile, output string) (string, error) return filepath.Join(c.state.DestinationRunDir, destFile), nil } -// generatePasswd generates a container specific passwd file, -// iff g.config.User is a number -func (c *Container) generatePasswd() (string, error) { - var ( - groupspec string - group *user.Group - gid int - ) - if c.config.User == "" { - return "", nil - } - spec := strings.SplitN(c.config.User, ":", 2) - userspec := spec[0] - if len(spec) > 1 { - groupspec = spec[1] - } - // If a non numeric User, then don't generate passwd - uid, err := strconv.ParseUint(userspec, 10, 32) - if err != nil { - return "", nil - } - // Lookup the user to see if it exists in the container image - _, err = lookup.GetUser(c.state.Mountpoint, userspec) - if err != nil && err != user.ErrNoPasswdEntries { - return "", err - } - if err == nil { - return "", nil - } - if groupspec != "" { - if !c.state.Mounted { - return "", errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate group field for passwd record", c.ID()) - } - group, err = lookup.GetGroup(c.state.Mountpoint, groupspec) - if err != nil { - if err == user.ErrNoGroupEntries { - return "", errors.Wrapf(err, "unable to get gid %s from group file", groupspec) - } - return "", err - } - gid = group.Gid - } - originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") - orig, err := ioutil.ReadFile(originPasswdFile) - if err != nil && !os.IsNotExist(err) { - return "", errors.Wrapf(err, "unable to read passwd file %s", originPasswdFile) - } - - pwd := fmt.Sprintf("%s%d:x:%d:%d:container user:%s:/bin/sh\n", orig, uid, uid, gid, c.WorkingDir()) - passwdFile, err := c.writeStringToRundir("passwd", pwd) - if err != nil { - return "", errors.Wrapf(err, "failed to create temporary passwd file") - } - if os.Chmod(passwdFile, 0644); err != nil { - return "", err - } - return passwdFile, nil -} - -// generateResolvConf generates a containers resolv.conf -func (c *Container) generateResolvConf() (string, error) { - // Determine the endpoint for resolv.conf in case it is a symlink - resolvPath, err := filepath.EvalSymlinks("/etc/resolv.conf") - if err != nil { - return "", err - } - - contents, err := ioutil.ReadFile(resolvPath) - if err != nil { - return "", errors.Wrapf(err, "unable to read %s", resolvPath) - } - - // Process the file to remove localhost nameservers - // TODO: set ipv6 enable bool more sanely - resolv, err := resolvconf.FilterResolvDNS(contents, true) - if err != nil { - return "", errors.Wrapf(err, "error parsing host resolv.conf") - } - - // Make a new resolv.conf - nameservers := resolvconf.GetNameservers(resolv.Content) - if len(c.config.DNSServer) > 0 { - // We store DNS servers as net.IP, so need to convert to string - nameservers = []string{} - for _, server := range c.config.DNSServer { - nameservers = append(nameservers, server.String()) - } - } - - search := resolvconf.GetSearchDomains(resolv.Content) - if len(c.config.DNSSearch) > 0 { - search = c.config.DNSSearch - } - - options := resolvconf.GetOptions(resolv.Content) - if len(c.config.DNSOption) > 0 { - options = c.config.DNSOption - } - - destPath := filepath.Join(c.state.RunDir, "resolv.conf") - - if err := os.Remove(destPath); err != nil && !os.IsNotExist(err) { - return "", errors.Wrapf(err, "error removing resolv.conf for container %s", c.ID()) - } - - // Build resolv.conf - if _, err = resolvconf.Build(destPath, nameservers, search, options); err != nil { - return "", errors.Wrapf(err, "error building resolv.conf for container %s") - } - - // Relabel resolv.conf for the container - if err := label.Relabel(destPath, c.config.MountLabel, false); err != nil { - return "", err - } - - return filepath.Join(c.state.DestinationRunDir, "resolv.conf"), nil -} - -// generateHosts creates a containers hosts file -func (c *Container) generateHosts() (string, error) { - orig, err := ioutil.ReadFile("/etc/hosts") - if err != nil { - return "", errors.Wrapf(err, "unable to read /etc/hosts") - } - hosts := string(orig) - if len(c.config.HostAdd) > 0 { - for _, host := range c.config.HostAdd { - // the host format has already been verified at this point - fields := strings.SplitN(host, ":", 2) - hosts += fmt.Sprintf("%s %s\n", fields[1], fields[0]) - } - } - if len(c.state.NetworkStatus) > 0 && len(c.state.NetworkStatus[0].IPs) > 0 { - ipAddress := strings.Split(c.state.NetworkStatus[0].IPs[0].Address.String(), "/")[0] - hosts += fmt.Sprintf("%s\t%s\n", ipAddress, c.Hostname()) - } - return c.writeStringToRundir("hosts", hosts) -} - func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator) error { var uid, gid int mountPoint := c.state.Mountpoint diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 66c7e8a04..86e3db4e3 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -11,6 +11,7 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "sync" "syscall" @@ -21,8 +22,11 @@ import ( crioAnnotations "github.com/containers/libpod/pkg/annotations" "github.com/containers/libpod/pkg/criu" "github.com/containers/libpod/pkg/lookup" + "github.com/containers/libpod/pkg/resolvconf" "github.com/containers/libpod/pkg/rootless" + "github.com/containers/libpod/pkg/secrets" "github.com/containers/storage/pkg/idtools" + "github.com/opencontainers/runc/libcontainer/user" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/selinux/go-selinux/label" @@ -132,6 +136,9 @@ func (c *Container) prepare() (err error) { // cleanupNetwork unmounts and cleans up the container's network func (c *Container) cleanupNetwork() error { + if c.NetworkDisabled() { + return nil + } if c.state.NetNS == nil { logrus.Debugf("Network is already cleaned up, skipping...") return nil @@ -169,6 +176,11 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { g.AddOrReplaceLinuxNamespace(spec.NetworkNamespace, c.state.NetNS.Path()) } } + + if err := c.makeBindMounts(); err != nil { + return nil, err + } + // Check if the spec file mounts contain the label Relabel flags z or Z. // If they do, relabel the source directory and then remove the option. for _, m := range g.Mounts() { @@ -591,3 +603,226 @@ func (c *Container) restore(ctx context.Context, keep bool) (err error) { return c.save() } + +// Make standard bind mounts to include in the container +func (c *Container) makeBindMounts() error { + if err := os.Chown(c.state.RunDir, c.RootUID(), c.RootGID()); err != nil { + return errors.Wrapf(err, "cannot chown run directory %s", c.state.RunDir) + } + + if c.state.BindMounts == nil { + c.state.BindMounts = make(map[string]string) + } + + if !c.NetworkDisabled() { + // Make /etc/resolv.conf + if _, ok := c.state.BindMounts["/etc/resolv.conf"]; ok { + // If it already exists, delete so we can recreate + delete(c.state.BindMounts, "/etc/resolv.conf") + } + newResolv, err := c.generateResolvConf() + if err != nil { + return errors.Wrapf(err, "error creating resolv.conf for container %s", c.ID()) + } + c.state.BindMounts["/etc/resolv.conf"] = newResolv + + // Make /etc/hosts + if _, ok := c.state.BindMounts["/etc/hosts"]; ok { + // If it already exists, delete so we can recreate + delete(c.state.BindMounts, "/etc/hosts") + } + newHosts, err := c.generateHosts() + if err != nil { + return errors.Wrapf(err, "error creating hosts file for container %s", c.ID()) + } + c.state.BindMounts["/etc/hosts"] = newHosts + + } + + // SHM is always added when we mount the container + c.state.BindMounts["/dev/shm"] = c.config.ShmDir + + newPasswd, err := c.generatePasswd() + if err != nil { + return errors.Wrapf(err, "error creating temporary passwd file for container %s", c.ID()) + } + if newPasswd != "" { + // Make /etc/passwd + if _, ok := c.state.BindMounts["/etc/passwd"]; ok { + // If it already exists, delete so we can recreate + delete(c.state.BindMounts, "/etc/passwd") + } + logrus.Debugf("adding entry to /etc/passwd for non existent default user") + c.state.BindMounts["/etc/passwd"] = newPasswd + } + + // Make /etc/hostname + // This should never change, so no need to recreate if it exists + if _, ok := c.state.BindMounts["/etc/hostname"]; !ok { + hostnamePath, err := c.writeStringToRundir("hostname", c.Hostname()) + if err != nil { + return errors.Wrapf(err, "error creating hostname file for container %s", c.ID()) + } + c.state.BindMounts["/etc/hostname"] = hostnamePath + } + + // Make .containerenv + // Empty file, so no need to recreate if it exists + if _, ok := c.state.BindMounts["/run/.containerenv"]; !ok { + // Empty string for now, but we may consider populating this later + containerenvPath, err := c.writeStringToRundir(".containerenv", "") + if err != nil { + return errors.Wrapf(err, "error creating containerenv file for container %s", c.ID()) + } + c.state.BindMounts["/run/.containerenv"] = containerenvPath + } + + // Add Secret Mounts + secretMounts := secrets.SecretMountsWithUIDGID(c.config.MountLabel, c.state.RunDir, c.runtime.config.DefaultMountsFile, c.state.DestinationRunDir, c.RootUID(), c.RootGID()) + for _, mount := range secretMounts { + if _, ok := c.state.BindMounts[mount.Destination]; !ok { + c.state.BindMounts[mount.Destination] = mount.Source + } + } + + return nil +} + +// generateResolvConf generates a containers resolv.conf +func (c *Container) generateResolvConf() (string, error) { + // Determine the endpoint for resolv.conf in case it is a symlink + resolvPath, err := filepath.EvalSymlinks("/etc/resolv.conf") + if err != nil { + return "", err + } + + contents, err := ioutil.ReadFile(resolvPath) + if err != nil { + return "", errors.Wrapf(err, "unable to read %s", resolvPath) + } + + // Process the file to remove localhost nameservers + // TODO: set ipv6 enable bool more sanely + resolv, err := resolvconf.FilterResolvDNS(contents, true) + if err != nil { + return "", errors.Wrapf(err, "error parsing host resolv.conf") + } + + // Make a new resolv.conf + nameservers := resolvconf.GetNameservers(resolv.Content) + if len(c.config.DNSServer) > 0 { + // We store DNS servers as net.IP, so need to convert to string + nameservers = []string{} + for _, server := range c.config.DNSServer { + nameservers = append(nameservers, server.String()) + } + } + + search := resolvconf.GetSearchDomains(resolv.Content) + if len(c.config.DNSSearch) > 0 { + search = c.config.DNSSearch + } + + options := resolvconf.GetOptions(resolv.Content) + if len(c.config.DNSOption) > 0 { + options = c.config.DNSOption + } + + destPath := filepath.Join(c.state.RunDir, "resolv.conf") + + if err := os.Remove(destPath); err != nil && !os.IsNotExist(err) { + return "", errors.Wrapf(err, "error removing resolv.conf for container %s", c.ID()) + } + + // Build resolv.conf + if _, err = resolvconf.Build(destPath, nameservers, search, options); err != nil { + return "", errors.Wrapf(err, "error building resolv.conf for container %s") + } + + // Relabel resolv.conf for the container + if err := label.Relabel(destPath, c.config.MountLabel, false); err != nil { + return "", err + } + + return filepath.Join(c.state.DestinationRunDir, "resolv.conf"), nil +} + +// generateHosts creates a containers hosts file +func (c *Container) generateHosts() (string, error) { + orig, err := ioutil.ReadFile("/etc/hosts") + if err != nil { + return "", errors.Wrapf(err, "unable to read /etc/hosts") + } + hosts := string(orig) + if len(c.config.HostAdd) > 0 { + for _, host := range c.config.HostAdd { + // the host format has already been verified at this point + fields := strings.SplitN(host, ":", 2) + hosts += fmt.Sprintf("%s %s\n", fields[1], fields[0]) + } + } + if len(c.state.NetworkStatus) > 0 && len(c.state.NetworkStatus[0].IPs) > 0 { + ipAddress := strings.Split(c.state.NetworkStatus[0].IPs[0].Address.String(), "/")[0] + hosts += fmt.Sprintf("%s\t%s\n", ipAddress, c.Hostname()) + } + return c.writeStringToRundir("hosts", hosts) +} + +// generatePasswd generates a container specific passwd file, +// iff g.config.User is a number +func (c *Container) generatePasswd() (string, error) { + var ( + groupspec string + group *user.Group + gid int + ) + if c.config.User == "" { + return "", nil + } + spec := strings.SplitN(c.config.User, ":", 2) + userspec := spec[0] + if len(spec) > 1 { + groupspec = spec[1] + } + // If a non numeric User, then don't generate passwd + uid, err := strconv.ParseUint(userspec, 10, 32) + if err != nil { + return "", nil + } + // Lookup the user to see if it exists in the container image + _, err = lookup.GetUser(c.state.Mountpoint, userspec) + if err != nil && err != user.ErrNoPasswdEntries { + return "", err + } + if err == nil { + return "", nil + } + if groupspec != "" { + if !c.state.Mounted { + return "", errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate group field for passwd record", c.ID()) + } + group, err = lookup.GetGroup(c.state.Mountpoint, groupspec) + if err != nil { + if err == user.ErrNoGroupEntries { + return "", errors.Wrapf(err, "unable to get gid %s from group file", groupspec) + } + return "", err + } + gid = group.Gid + } + originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") + orig, err := ioutil.ReadFile(originPasswdFile) + if err != nil && !os.IsNotExist(err) { + return "", errors.Wrapf(err, "unable to read passwd file %s", originPasswdFile) + } + + pwd := fmt.Sprintf("%s%d:x:%d:%d:container user:%s:/bin/sh\n", orig, uid, uid, gid, c.WorkingDir()) + passwdFile, err := c.writeStringToRundir("passwd", pwd) + if err != nil { + return "", errors.Wrapf(err, "failed to create temporary passwd file") + } + if os.Chmod(passwdFile, 0644); err != nil { + return "", err + } + return passwdFile, nil +} -- cgit v1.2.3-54-g00ecf From 0592558289c354447d28903910f4165ac5acd71a Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Wed, 21 Nov 2018 13:09:17 +0000 Subject: Use also a struct to pass options to Restore() This is basically the same change as ff47a4c2d5485fc49f937f3ce0c4e2fd6bdb1956 (Use a struct to pass options to Checkpoint()) just for the Restore() function. It is used to pass multiple restore options to the API and down to conmon which is used to restore containers. This is for the upcoming changes to support checkpointing and restoring containers with '--tcp-established'. Signed-off-by: Adrian Reber --- cmd/podman/restore.go | 6 ++++-- libpod/container_api.go | 6 +++--- libpod/container_internal.go | 2 +- libpod/container_internal_linux.go | 6 +++--- libpod/oci.go | 4 ++-- libpod/oci_linux.go | 6 +++--- libpod/oci_unsupported.go | 2 +- 7 files changed, 17 insertions(+), 15 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/cmd/podman/restore.go b/cmd/podman/restore.go index 067a2b5d4..6383ebf0b 100644 --- a/cmd/podman/restore.go +++ b/cmd/podman/restore.go @@ -53,7 +53,9 @@ func restoreCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - keep := c.Bool("keep") + options := libpod.ContainerCheckpointOptions{ + Keep: c.Bool("keep"), + } if err := checkAllAndLatest(c); err != nil { return err @@ -62,7 +64,7 @@ func restoreCmd(c *cli.Context) error { containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "checkpointed") for _, ctr := range containers { - if err = ctr.Restore(context.TODO(), keep); err != nil { + if err = ctr.Restore(context.TODO(), options); err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError) } diff --git a/libpod/container_api.go b/libpod/container_api.go index df6b6e962..396f06c20 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -831,7 +831,7 @@ func (c *Container) Refresh(ctx context.Context) error { } // ContainerCheckpointOptions is a struct used to pass the parameters -// for checkpointing to corresponding functions +// for checkpointing (and restoring) to the corresponding functions type ContainerCheckpointOptions struct { Keep bool KeepRunning bool @@ -853,7 +853,7 @@ func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointO } // Restore restores a container -func (c *Container) Restore(ctx context.Context, keep bool) (err error) { +func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOptions) (err error) { logrus.Debugf("Trying to restore container %s", c) if !c.batched { c.lock.Lock() @@ -864,5 +864,5 @@ func (c *Container) Restore(ctx context.Context, keep bool) (err error) { } } - return c.restore(ctx, keep) + return c.restore(ctx, options) } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 051e0aeb7..da0456554 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -606,7 +606,7 @@ func (c *Container) init(ctx context.Context) error { } // With the spec complete, do an OCI create - if err := c.runtime.ociRuntime.createContainer(c, c.config.CgroupParent, false); err != nil { + if err := c.runtime.ociRuntime.createContainer(c, c.config.CgroupParent, nil); err != nil { return err } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index ffb82cc94..99f8652df 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -514,7 +514,7 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO return c.save() } -func (c *Container) restore(ctx context.Context, keep bool) (err error) { +func (c *Container) restore(ctx context.Context, options ContainerCheckpointOptions) (err error) { if !criu.CheckForCriu() { return errors.Errorf("restoring a container requires at least CRIU %d", criu.MinCriuVersion) @@ -602,7 +602,7 @@ func (c *Container) restore(ctx context.Context, keep bool) (err error) { // Cleanup for a working restore. c.removeConmonFiles() - if err := c.runtime.ociRuntime.createContainer(c, c.config.CgroupParent, true); err != nil { + if err := c.runtime.ociRuntime.createContainer(c, c.config.CgroupParent, &options); err != nil { return err } @@ -610,7 +610,7 @@ func (c *Container) restore(ctx context.Context, keep bool) (err error) { c.state.State = ContainerStateRunning - if !keep { + if !options.Keep { // Delete all checkpoint related files. At this point, in theory, all files // should exist. Still ignoring errors for now as the container should be // restored and running. Not erroring out just because some cleanup operation diff --git a/libpod/oci.go b/libpod/oci.go index a7aec06e5..6aedc5662 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -227,7 +227,7 @@ func bindPorts(ports []ocicni.PortMapping) ([]*os.File, error) { return files, nil } -func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, restoreContainer bool) (err error) { +func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, restoreOptions *ContainerCheckpointOptions) (err error) { var stderrBuf bytes.Buffer runtimeDir, err := util.GetRootlessRuntimeDir() @@ -289,7 +289,7 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res args = append(args, "--syslog") } - if restoreContainer { + if restoreOptions != nil { args = append(args, "--restore", ctr.CheckpointPath()) } diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go index b159eae78..2737a641e 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -65,10 +65,10 @@ func newPipe() (parent *os.File, child *os.File, err error) { // CreateContainer creates a container in the OCI runtime // TODO terminal support for container // Presently just ignoring conmon opts related to it -func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restoreContainer bool) (err error) { +func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restoreOptions *ContainerCheckpointOptions) (err error) { if ctr.state.UserNSRoot == "" { // no need of an intermediate mount ns - return r.createOCIContainer(ctr, cgroupParent, restoreContainer) + return r.createOCIContainer(ctr, cgroupParent, restoreOptions) } var wg sync.WaitGroup wg.Add(1) @@ -106,7 +106,7 @@ func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restor if err != nil { return } - err = r.createOCIContainer(ctr, cgroupParent, restoreContainer) + err = r.createOCIContainer(ctr, cgroupParent, restoreOptions) }() wg.Wait() diff --git a/libpod/oci_unsupported.go b/libpod/oci_unsupported.go index b133eb402..8c084d1e2 100644 --- a/libpod/oci_unsupported.go +++ b/libpod/oci_unsupported.go @@ -15,7 +15,7 @@ func newPipe() (parent *os.File, child *os.File, err error) { return nil, nil, ErrNotImplemented } -func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restoreContainer bool) (err error) { +func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restoreOptions *ContainerCheckpointOptions) (err error) { return ErrNotImplemented } -- cgit v1.2.3-54-g00ecf From 95f22a2ca055d6dec0281cee109375dc4fd9b78b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 26 Nov 2018 21:31:06 +0100 Subject: network: allow slirp4netns mode also for root containers Signed-off-by: Giuseppe Scrivano --- docs/podman-create.1.md | 3 ++- docs/podman-run.1.md | 3 ++- libpod/container.go | 3 +++ libpod/container_easyjson.go | 13 +++++++++++++ libpod/container_internal.go | 2 +- libpod/oci.go | 2 +- libpod/options.go | 4 +++- libpod/runtime_pod_infra_linux.go | 6 +++++- pkg/spec/createconfig.go | 4 ++-- 9 files changed, 32 insertions(+), 8 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md index 68c00685b..474796a35 100644 --- a/docs/podman-create.1.md +++ b/docs/podman-create.1.md @@ -426,7 +426,8 @@ Set the Network mode for the container 'container:': reuse another container's network stack 'host': use the podman host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure. '|': connect to a user-defined network - 'ns:' path to a network namespace to join + 'ns:': path to a network namespace to join + 'slirp4netns': use slirp4netns to create a user network stack. This is the default for rootless containers **--network-alias**=[] diff --git a/docs/podman-run.1.md b/docs/podman-run.1.md index 912026a55..202091b07 100644 --- a/docs/podman-run.1.md +++ b/docs/podman-run.1.md @@ -408,7 +408,8 @@ Set the Network mode for the container: - `container:`: reuse another container's network stack - `host`: use the podman host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure. - `|`: connect to a user-defined network -- `ns:` path to a network namespace to join +- `ns:`: path to a network namespace to join +- `slirp4netns`: use slirp4netns to create a user network stack. This is the default for rootless containers **--network-alias**=[] diff --git a/libpod/container.go b/libpod/container.go index 16f61d021..a8a58f4d8 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -9,6 +9,7 @@ import ( "github.com/containernetworking/cni/pkg/types" cnitypes "github.com/containernetworking/cni/pkg/types/current" + "github.com/containers/libpod/pkg/namespaces" "github.com/containers/storage" "github.com/cri-o/ocicni/pkg/ocicni" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -296,6 +297,8 @@ type ContainerConfig struct { HostAdd []string `json:"hostsAdd,omitempty"` // Network names (CNI) to add container to. Empty to use default network. Networks []string `json:"networks,omitempty"` + // Network mode specified for the default network. + NetMode namespaces.NetworkMode `json:"networkMode,omitempty"` // Image Config diff --git a/libpod/container_easyjson.go b/libpod/container_easyjson.go index 041cc08ac..8bf5cb64f 100644 --- a/libpod/container_easyjson.go +++ b/libpod/container_easyjson.go @@ -8,6 +8,7 @@ import ( json "encoding/json" types "github.com/containernetworking/cni/pkg/types" current "github.com/containernetworking/cni/pkg/types/current" + namespaces "github.com/containers/libpod/pkg/namespaces" storage "github.com/containers/storage" idtools "github.com/containers/storage/pkg/idtools" ocicni "github.com/cri-o/ocicni/pkg/ocicni" @@ -1550,6 +1551,8 @@ func easyjson1dbef17bDecodeGithubComContainersLibpodLibpod2(in *jlexer.Lexer, ou } in.Delim(']') } + case "networkMode": + out.NetMode = namespaces.NetworkMode(in.String()) case "userVolumes": if in.IsNull() { in.Skip() @@ -2177,6 +2180,16 @@ func easyjson1dbef17bEncodeGithubComContainersLibpodLibpod2(out *jwriter.Writer, out.RawByte(']') } } + if in.NetMode != "" { + const prefix string = ",\"networkMode\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + out.String(string(in.NetMode)) + } if len(in.UserVolumes) != 0 { const prefix string = ",\"userVolumes\":" if first { diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 051e0aeb7..e44ec76ec 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -586,7 +586,7 @@ func (c *Container) completeNetworkSetup() error { if err := c.syncContainer(); err != nil { return err } - if rootless.IsRootless() { + if c.config.NetMode == "slirp4netns" { return c.runtime.setupRootlessNetNS(c) } return c.runtime.setupNetNS(c) diff --git a/libpod/oci.go b/libpod/oci.go index a7aec06e5..e9cceda82 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -329,7 +329,7 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res cmd.ExtraFiles = append(cmd.ExtraFiles, ports...) } - if rootless.IsRootless() { + if ctr.config.NetMode.IsSlirp4netns() { ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe() if err != nil { return errors.Wrapf(err, "failed to create rootless network sync pipe") diff --git a/libpod/options.go b/libpod/options.go index 507847d65..7f4e3ac6b 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -7,6 +7,7 @@ import ( "regexp" "syscall" + "github.com/containers/libpod/pkg/namespaces" "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" "github.com/cri-o/ocicni/pkg/ocicni" @@ -817,7 +818,7 @@ func WithDependencyCtrs(ctrs []*Container) CtrCreateOption { // namespace with a minimal configuration. // An optional array of port mappings can be provided. // Conflicts with WithNetNSFrom(). -func WithNetNS(portMappings []ocicni.PortMapping, postConfigureNetNS bool, networks []string) CtrCreateOption { +func WithNetNS(portMappings []ocicni.PortMapping, postConfigureNetNS bool, netmode string, networks []string) CtrCreateOption { return func(ctr *Container) error { if ctr.valid { return ErrCtrFinalized @@ -831,6 +832,7 @@ func WithNetNS(portMappings []ocicni.PortMapping, postConfigureNetNS bool, netwo ctr.config.CreateNetNS = true ctr.config.PortMappings = portMappings ctr.config.Networks = networks + ctr.config.NetMode = namespaces.NetworkMode(netmode) return nil } diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index 450a2fb32..8a5dbef56 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -50,7 +50,11 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID // Since user namespace sharing is not implemented, we only need to check if it's rootless networks := make([]string, 0) - options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, isRootless, networks)) + netmode := "bridge" + if isRootless { + netmode = "slirp4netns" + } + options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, isRootless, netmode, networks)) return r.newContainer(ctx, g.Config, options...) } diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index 6a0642ee7..a0fd40318 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -391,11 +391,11 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime) ([]lib options = append(options, libpod.WithNetNSFrom(connectedCtr)) } else if !c.NetMode.IsHost() && !c.NetMode.IsNone() { isRootless := rootless.IsRootless() - postConfigureNetNS := isRootless || (len(c.IDMappings.UIDMap) > 0 || len(c.IDMappings.GIDMap) > 0) && !c.UsernsMode.IsHost() + postConfigureNetNS := c.NetMode.IsSlirp4netns() || (len(c.IDMappings.UIDMap) > 0 || len(c.IDMappings.GIDMap) > 0) && !c.UsernsMode.IsHost() if isRootless && len(portBindings) > 0 { return nil, errors.New("port bindings are not yet supported by rootless containers") } - options = append(options, libpod.WithNetNS(portBindings, postConfigureNetNS, networks)) + options = append(options, libpod.WithNetNS(portBindings, postConfigureNetNS, string(c.NetMode), networks)) } if c.PidMode.IsContainer() { -- cgit v1.2.3-54-g00ecf From 3beacb73bced227b211bf3b8710382b94358614b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 16 Nov 2018 06:51:26 -0500 Subject: Disable mount options when running --privileged We now default to setting storage options to "nodev", when running privileged containers, we need to turn this off so the processes can manipulate the image. Signed-off-by: Daniel J Walsh --- docs/podman-create.1.md | 7 ++++--- docs/podman-run.1.md | 5 +++-- libpod/container_internal.go | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md index 68c00685b..ab2cb8c60 100644 --- a/docs/podman-create.1.md +++ b/docs/podman-create.1.md @@ -465,9 +465,10 @@ By default, podman containers are This is because by default a container is not allowed to access any devices. A “privileged” container is given access to all devices. -When the operator executes **podman run --privileged**, podman enables access -to all devices on the host as well as set turn off most of the security measures -protecting the host from the container. +When the operator executes a privileged container, podman enables access +to all devices on the host, turns off graphdriver mount options, as well as +turning off most of the security measures protecting the host from the +container. **-p**, **--publish**=[] diff --git a/docs/podman-run.1.md b/docs/podman-run.1.md index 912026a55..f99d2f863 100644 --- a/docs/podman-run.1.md +++ b/docs/podman-run.1.md @@ -450,8 +450,9 @@ container is not allowed to access any devices. A “privileged” container is given access to all devices. When the operator executes **podman run --privileged**, podman enables access -to all devices on the host as well as set turn off most of the security measures -protecting the host from the container. +to all devices on the host, turns off graphdriver mount options, as well as +turning off most of the security measures protecting the host from the +container. **-p**, **--publish**=[] diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 051e0aeb7..a426191a4 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -273,6 +273,27 @@ func (c *Container) setupStorage(ctx context.Context) error { }, LabelOpts: c.config.LabelOpts, } + if c.config.Privileged { + privOpt := func(opt string) bool { + for _, privopt := range []string{"nodev", "nosuid", "noexec"} { + if opt == privopt { + return true + } + } + return false + } + defOptions, err := storage.GetDefaultMountOptions() + if err != nil { + return errors.Wrapf(err, "error getting default mount options") + } + var newOptions []string + for _, opt := range defOptions { + if !privOpt(opt) { + newOptions = append(newOptions, opt) + } + } + options.MountOpts = newOptions + } if c.config.Rootfs == "" { options.IDMappingOptions = c.config.IDMappings -- cgit v1.2.3-54-g00ecf From 61d4db480650c57d383990750ed87990e903f4da Mon Sep 17 00:00:00 2001 From: baude Date: Tue, 27 Nov 2018 10:33:19 -0600 Subject: Fix golang formatting issues Whe running unittests on newer golang versions, we observe failures with some formatting types when no declared correctly. Signed-off-by: baude --- cmd/podman/commit.go | 2 +- cmd/podman/images.go | 4 ++-- cmd/podman/import.go | 2 +- cmd/podman/inspect.go | 6 +++--- libpod/container.go | 4 ++-- libpod/container_api.go | 14 +++++++------- libpod/container_internal.go | 2 +- libpod/container_internal_linux.go | 2 +- libpod/kube.go | 2 +- libpod/pod_internal.go | 2 +- 10 files changed, 20 insertions(+), 20 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/cmd/podman/commit.go b/cmd/podman/commit.go index b09c6b0d9..02ede4f73 100644 --- a/cmd/podman/commit.go +++ b/cmd/podman/commit.go @@ -95,7 +95,7 @@ func commitCmd(c *cli.Context) error { for _, change := range c.StringSlice("change") { splitChange := strings.Split(strings.ToUpper(change), "=") if !util.StringInSlice(splitChange[0], libpod.ChangeCmds) { - return errors.Errorf("invalid syntax for --change ", change) + return errors.Errorf("invalid syntax for --change: %s", change) } } } diff --git a/cmd/podman/images.go b/cmd/podman/images.go index c52b26260..3351123ed 100644 --- a/cmd/podman/images.go +++ b/cmd/podman/images.go @@ -376,13 +376,13 @@ func CreateFilterFuncs(ctx context.Context, r *libpod.Runtime, c *cli.Context, i case "before": before, err := r.ImageRuntime().NewFromLocal(splitFilter[1]) if err != nil { - return nil, errors.Wrapf(err, "unable to find image % in local stores", splitFilter[1]) + return nil, errors.Wrapf(err, "unable to find image %s in local stores", splitFilter[1]) } filterFuncs = append(filterFuncs, image.CreatedBeforeFilter(before.Created())) case "after": after, err := r.ImageRuntime().NewFromLocal(splitFilter[1]) if err != nil { - return nil, errors.Wrapf(err, "unable to find image % in local stores", splitFilter[1]) + return nil, errors.Wrapf(err, "unable to find image %s in local stores", splitFilter[1]) } filterFuncs = append(filterFuncs, image.CreatedAfterFilter(after.Created())) case "dangling": diff --git a/cmd/podman/import.go b/cmd/podman/import.go index be516e4fa..144354fa6 100644 --- a/cmd/podman/import.go +++ b/cmd/podman/import.go @@ -139,7 +139,7 @@ func downloadFromURL(source string) (string, error) { _, err = io.Copy(outFile, response.Body) if err != nil { - return "", errors.Wrapf(err, "error saving %q to %q", source, outFile) + return "", errors.Wrapf(err, "error saving %s to %s", source, outFile.Name()) } return outFile.Name(), nil diff --git a/cmd/podman/inspect.go b/cmd/podman/inspect.go index bd9e8c13c..6ffcde55f 100644 --- a/cmd/podman/inspect.go +++ b/cmd/podman/inspect.go @@ -119,7 +119,7 @@ func iterateInput(ctx context.Context, c *cli.Context, args []string, runtime *l } libpodInspectData, err := ctr.Inspect(c.Bool("size")) if err != nil { - inspectError = errors.Wrapf(err, "error getting libpod container inspect data %q", ctr.ID) + inspectError = errors.Wrapf(err, "error getting libpod container inspect data %s", ctr.ID()) break } data, err = shared.GetCtrInspectInfo(ctr, libpodInspectData) @@ -154,12 +154,12 @@ func iterateInput(ctx context.Context, c *cli.Context, args []string, runtime *l } else { libpodInspectData, err := ctr.Inspect(c.Bool("size")) if err != nil { - inspectError = errors.Wrapf(err, "error getting libpod container inspect data %q", ctr.ID) + inspectError = errors.Wrapf(err, "error getting libpod container inspect data %s", ctr.ID()) break } data, err = shared.GetCtrInspectInfo(ctr, libpodInspectData) if err != nil { - inspectError = errors.Wrapf(err, "error parsing container data %q", ctr.ID) + inspectError = errors.Wrapf(err, "error parsing container data %s", ctr.ID()) break } } diff --git a/libpod/container.go b/libpod/container.go index a8a58f4d8..b6155a809 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -829,7 +829,7 @@ func (c *Container) IPs() ([]net.IPNet, error) { } if !c.config.CreateNetNS { - return nil, errors.Wrapf(ErrInvalidArg, "container %s network namespace is not managed by libpod") + return nil, errors.Wrapf(ErrInvalidArg, "container %s network namespace is not managed by libpod", c.ID()) } ips := make([]net.IPNet, 0) @@ -857,7 +857,7 @@ func (c *Container) Routes() ([]types.Route, error) { } if !c.config.CreateNetNS { - return nil, errors.Wrapf(ErrInvalidArg, "container %s network namespace is not managed by libpod") + return nil, errors.Wrapf(ErrInvalidArg, "container %s network namespace is not managed by libpod", c.ID()) } routes := make([]types.Route, 0) diff --git a/libpod/container_api.go b/libpod/container_api.go index 4789c0cd2..bc92cae69 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -39,7 +39,7 @@ func (c *Container) Init(ctx context.Context) (err error) { notRunning, err := c.checkDependenciesRunning() if err != nil { - return errors.Wrapf(err, "error checking dependencies for container %s") + return errors.Wrapf(err, "error checking dependencies for container %s", c.ID()) } if len(notRunning) > 0 { depString := strings.Join(notRunning, ",") @@ -93,7 +93,7 @@ func (c *Container) Start(ctx context.Context) (err error) { notRunning, err := c.checkDependenciesRunning() if err != nil { - return errors.Wrapf(err, "error checking dependencies for container %s") + return errors.Wrapf(err, "error checking dependencies for container %s", c.ID()) } if len(notRunning) > 0 { depString := strings.Join(notRunning, ",") @@ -159,7 +159,7 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams, notRunning, err := c.checkDependenciesRunning() if err != nil { - return nil, errors.Wrapf(err, "error checking dependencies for container %s") + return nil, errors.Wrapf(err, "error checking dependencies for container %s", c.ID()) } if len(notRunning) > 0 { depString := strings.Join(notRunning, ",") @@ -718,7 +718,7 @@ func (c *Container) RestartWithTimeout(ctx context.Context, timeout uint) (err e notRunning, err := c.checkDependenciesRunning() if err != nil { - return errors.Wrapf(err, "error checking dependencies for container %s") + return errors.Wrapf(err, "error checking dependencies for container %s", c.ID()) } if len(notRunning) > 0 { depString := strings.Join(notRunning, ",") @@ -803,7 +803,7 @@ func (c *Container) Refresh(ctx context.Context) error { return err } - logrus.Debugf("Successfully refresh container %s state") + logrus.Debugf("Successfully refresh container %s state", c.ID()) // Initialize the container if it was created in runc if wasCreated || wasRunning || wasPaused { @@ -847,7 +847,7 @@ type ContainerCheckpointOptions struct { // Checkpoint checkpoints a container func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointOptions) error { - logrus.Debugf("Trying to checkpoint container %s", c) + logrus.Debugf("Trying to checkpoint container %s", c.ID()) if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -862,7 +862,7 @@ func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointO // Restore restores a container func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOptions) (err error) { - logrus.Debugf("Trying to restore container %s", c) + logrus.Debugf("Trying to restore container %s", c.ID()) if !c.batched { c.lock.Lock() defer c.lock.Unlock() diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 700773e7f..b616e0a07 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1191,7 +1191,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) { return nil, err } - logrus.Warnf("failed to load hooks: {}", err) + logrus.Warnf("failed to load hooks: %q", err) return nil, nil } hooks, err := manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 99f8652df..5fc94eb8a 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -764,7 +764,7 @@ func (c *Container) generateResolvConf() (string, error) { // Build resolv.conf if _, err = resolvconf.Build(destPath, nameservers, search, options); err != nil { - return "", errors.Wrapf(err, "error building resolv.conf for container %s") + return "", errors.Wrapf(err, "error building resolv.conf for container %s", c.ID()) } // Relabel resolv.conf for the container diff --git a/libpod/kube.go b/libpod/kube.go index 00db0033b..1a5f80878 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -240,7 +240,7 @@ func generateKubeSecurityContext(c *Container) (*v1.SecurityContext, error) { if c.User() != "" { // It is *possible* that - logrus.Debug("Looking in container for user: %s", c.User()) + logrus.Debugf("Looking in container for user: %s", c.User()) u, err := lookup.GetUser(c.state.Mountpoint, c.User()) if err != nil { return nil, err diff --git a/libpod/pod_internal.go b/libpod/pod_internal.go index 46162c7ef..39a25c004 100644 --- a/libpod/pod_internal.go +++ b/libpod/pod_internal.go @@ -48,7 +48,7 @@ func (p *Pod) updatePod() error { // Save pod state to database func (p *Pod) save() error { if err := p.runtime.state.SavePod(p); err != nil { - return errors.Wrapf(err, "error saving pod %s state") + return errors.Wrapf(err, "error saving pod %s state", p.ID()) } return nil -- cgit v1.2.3-54-g00ecf From a5be3ffa4d1502072fd18ee280cc062b71fbe574 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 8 Nov 2018 06:14:46 -0500 Subject: /dev/shm should be mounted even in rootless mode. Currently we are mounting /dev/shm from disk, it should be from a tmpfs. User Namespace supports tmpfs mounts for nonroot users, so this section of code should work fine in bother root and rootless mode. Signed-off-by: Daniel J Walsh --- libpod/container_internal.go | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index b616e0a07..24ddb6655 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -821,28 +821,22 @@ func (c *Container) mountStorage() (string, error) { return c.state.Mountpoint, nil } - if !rootless.IsRootless() { - // TODO: generalize this mount code so it will mount every mount in ctr.config.Mounts - mounted, err := mount.Mounted(c.config.ShmDir) - if err != nil { - return "", errors.Wrapf(err, "unable to determine if %q is mounted", c.config.ShmDir) - } + mounted, err := mount.Mounted(c.config.ShmDir) + if err != nil { + return "", errors.Wrapf(err, "unable to determine if %q is mounted", c.config.ShmDir) + } + if !mounted { + shmOptions := fmt.Sprintf("mode=1777,size=%d", c.config.ShmSize) + if err := c.mountSHM(shmOptions); err != nil { + return "", err + } if err := os.Chown(c.config.ShmDir, c.RootUID(), c.RootGID()); err != nil { return "", errors.Wrapf(err, "failed to chown %s", c.config.ShmDir) } - - if !mounted { - shmOptions := fmt.Sprintf("mode=1777,size=%d", c.config.ShmSize) - if err := c.mountSHM(shmOptions); err != nil { - return "", err - } - if err := os.Chown(c.config.ShmDir, c.RootUID(), c.RootGID()); err != nil { - return "", errors.Wrapf(err, "failed to chown %s", c.config.ShmDir) - } - } } + // TODO: generalize this mount code so it will mount every mount in ctr.config.Mounts mountPoint := c.config.Rootfs if mountPoint == "" { mountPoint, err = c.mount() -- cgit v1.2.3-54-g00ecf From a4b483c8484bb6fb9ae487264bccc663f007e711 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 2 Dec 2018 21:22:08 -0800 Subject: libpod/container_internal: Deprecate implicit hook directories Part of the motivation for 800eb863 (Hooks supports two directories, process default and override, 2018-09-17, #1487) was [1]: > We only use this for override. The reason this was caught is people > are trying to get hooks to work with CoreOS. You are not allowed to > write to /usr/share... on CoreOS, so they wanted podman to also look > at /etc, where users and third parties can write. But we'd also been disabling hooks completely for rootless users. And even for root users, the override logic was tricky when folks actually had content in both directories. For example, if you wanted to disable a hook from the default directory, you'd have to add a no-op hook to the override directory. Also, the previous implementation failed to handle the case where there hooks defined in the override directory but the default directory did not exist: $ podman version Version: 0.11.2-dev Go Version: go1.10.3 Git Commit: "6df7409cb5a41c710164c42ed35e33b28f3f7214" Built: Sun Dec 2 21:30:06 2018 OS/Arch: linux/amd64 $ ls -l /etc/containers/oci/hooks.d/test.json -rw-r--r--. 1 root root 184 Dec 2 16:27 /etc/containers/oci/hooks.d/test.json $ podman --log-level=debug run --rm docker.io/library/alpine echo 'successful container' 2>&1 | grep -i hook time="2018-12-02T21:31:19-08:00" level=debug msg="reading hooks from /usr/share/containers/oci/hooks.d" time="2018-12-02T21:31:19-08:00" level=warning msg="failed to load hooks: {}%!(EXTRA *os.PathError=open /usr/share/containers/oci/hooks.d: no such file or directory)" With this commit: $ podman --log-level=debug run --rm docker.io/library/alpine echo 'successful container' 2>&1 | grep -i hook time="2018-12-02T21:33:07-08:00" level=debug msg="reading hooks from /usr/share/containers/oci/hooks.d" time="2018-12-02T21:33:07-08:00" level=debug msg="reading hooks from /etc/containers/oci/hooks.d" time="2018-12-02T21:33:07-08:00" level=debug msg="added hook /etc/containers/oci/hooks.d/test.json" time="2018-12-02T21:33:07-08:00" level=debug msg="hook test.json matched; adding to stages [prestart]" time="2018-12-02T21:33:07-08:00" level=warning msg="implicit hook directories are deprecated; set --hooks-dir="/etc/containers/oci/hooks.d" explicitly to continue to load hooks from this directory" time="2018-12-02T21:33:07-08:00" level=error msg="container create failed: container_linux.go:336: starting container process caused "process_linux.go:399: container init caused \"process_linux.go:382: running prestart hook 0 caused \\\"error running hook: exit status 1, stdout: , stderr: oh, noes!\\\\n\\\"\"" (I'd setup the hook to error out). You can see that it's silenly ignoring the ENOENT for /usr/share/containers/oci/hooks.d and continuing on to load hooks from /etc/containers/oci/hooks.d. When it loads the hook, it also logs a warning-level message suggesting that callers explicitly configure their hook directories. That will help consumers migrate, so we can drop the implicit hook directories in some future release. When folks *do* explicitly configure hook directories (via the newly-public --hooks-dir and hooks_dir options), we error out if they're missing: $ podman --hooks-dir /does/not/exist run --rm docker.io/library/alpine echo 'successful container' error setting up OCI Hooks: open /does/not/exist: no such file or directory I've dropped the trailing "path" from the old, hidden --hooks-dir-path and hooks_dir_path because I think "dir(ectory)" is already enough context for "we expect a path argument". I consider this name change non-breaking because the old forms were undocumented. Coming back to rootless users, I've enabled hooks now. I expect they were previously disabled because users had no way to avoid /usr/share/containers/oci/hooks.d which might contain hooks that required root permissions. But now rootless users will have to explicitly configure hook directories, and since their default config is from ~/.config/containers/libpod.conf, it's a misconfiguration if it contains hooks_dir entries which point at directories with hooks that require root access. We error out so they can fix their libpod.conf. [1]: https://github.com/containers/libpod/pull/1487#discussion_r218149355 Signed-off-by: W. Trevor King --- cmd/podman/libpodruntime/runtime.go | 4 ++-- cmd/podman/main.go | 9 +++---- docs/libpod.conf.5.md | 12 ++++++++++ docs/podman.1.md | 24 +++++++++---------- libpod/container_internal.go | 48 ++++++++++++++++++++++--------------- libpod/container_internal_linux.go | 6 ++--- libpod/options.go | 15 ++++++------ libpod/runtime.go | 10 ++++---- libpod/testdata/config.toml | 2 +- test/e2e/run_test.go | 2 +- 10 files changed, 73 insertions(+), 59 deletions(-) (limited to 'libpod/container_internal.go') diff --git a/cmd/podman/libpodruntime/runtime.go b/cmd/podman/libpodruntime/runtime.go index a4b3581be..f69eaf3a4 100644 --- a/cmd/podman/libpodruntime/runtime.go +++ b/cmd/podman/libpodruntime/runtime.go @@ -90,8 +90,8 @@ func GetRuntimeWithStorageOpts(c *cli.Context, storageOpts *storage.StoreOptions if c.GlobalIsSet("default-mounts-file") { options = append(options, libpod.WithDefaultMountsFile(c.GlobalString("default-mounts-file"))) } - if c.GlobalIsSet("hooks-dir-path") { - options = append(options, libpod.WithHooksDir(c.GlobalString("hooks-dir-path"))) + if c.GlobalIsSet("hooks-dir") { + options = append(options, libpod.WithHooksDir(c.GlobalStringSlice("hooks-dir")...)) } // TODO flag to set CNI plugins dir? diff --git a/cmd/podman/main.go b/cmd/podman/main.go index 6be192593..bcae04575 100644 --- a/cmd/podman/main.go +++ b/cmd/podman/main.go @@ -8,7 +8,6 @@ import ( "syscall" "github.com/containers/libpod/libpod" - "github.com/containers/libpod/pkg/hooks" _ "github.com/containers/libpod/pkg/hooks/0.1.0" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/version" @@ -206,11 +205,9 @@ func main() { Usage: "path to default mounts file", Hidden: true, }, - cli.StringFlag{ - Name: "hooks-dir-path", - Usage: "set the OCI hooks directory path", - Value: hooks.DefaultDir, - Hidden: true, + cli.StringSliceFlag{ + Name: "hooks-dir", + Usage: "set the OCI hooks directory path (may be set multiple times)", }, cli.IntFlag{ Name: "max-workers", diff --git a/docs/libpod.conf.5.md b/docs/libpod.conf.5.md index 198e927ee..d63baeb88 100644 --- a/docs/libpod.conf.5.md +++ b/docs/libpod.conf.5.md @@ -24,6 +24,18 @@ libpod to manage containers. **cgroup_manager**="" Specify the CGroup Manager to use; valid values are "systemd" and "cgroupfs" +**hooks_dir**=["*path*", ...] + + Each `*.json` file in the path configures a hook for Podman containers. For more details on the syntax of the JSON files and the semantics of hook injection, see `oci-hooks(5)`. Podman and libpod currently support both the 1.0.0 and 0.1.0 hook schemas, although the 0.1.0 schema is deprecated. + + Paths listed later in the array higher precedence (`oci-hooks(5)` discusses directory precedence). + + For the annotation conditions, libpod uses any annotations set in the generated OCI configuration. + + For the bind-mount conditions, only mounts explicitly requested by the caller via `--volume` are considered. Bind mounts that libpod inserts by default (e.g. `/dev/shm`) are not considered. + + If `hooks_dir` is unset for root callers, Podman and libpod will currently default to `/usr/share/containers/oci/hooks.d` and `/etc/containers/oci/hooks.d` in order of increasing precedence. Using these defaults is deprecated, and callers should migrate to explicitly setting `hooks_dir`. + **static_dir**="" Directory for persistent libpod files (database, etc) By default this will be configured relative to where containers/storage diff --git a/docs/podman.1.md b/docs/podman.1.md index b7433d850..bde349e6f 100644 --- a/docs/podman.1.md +++ b/docs/podman.1.md @@ -31,6 +31,18 @@ CGroup manager to use for container cgroups. Supported values are cgroupfs or sy Path to where the cpu performance results should be written +**--hooks-dir**=**path** + +Each `*.json` file in the path configures a hook for Podman containers. For more details on the syntax of the JSON files and the semantics of hook injection, see `oci-hooks(5)`. Podman and libpod currently support both the 1.0.0 and 0.1.0 hook schemas, although the 0.1.0 schema is deprecated. + +This option may be set multiple times; paths from later options have higher precedence (`oci-hooks(5)` discusses directory precedence). + +For the annotation conditions, libpod uses any annotations set in the generated OCI configuration. + +For the bind-mount conditions, only mounts explicitly requested by the caller via `--volume` are considered. Bind mounts that libpod inserts by default (e.g. `/dev/shm`) are not considered. + +If `--hooks-dir` is unset for root callers, Podman and libpod will currently default to `/usr/share/containers/oci/hooks.d` and `/etc/containers/oci/hooks.d` in order of increasing precedence. Using these defaults is deprecated, and callers should migrate to explicitly setting `--hooks-dir`. + **--log-level** Log messages above specified level: debug, info, warn, error (default), fatal or panic @@ -161,18 +173,6 @@ the exit codes follow the `chroot` standard, see below: The mounts.conf file specifies volume mount directories that are automatically mounted inside containers when executing the `podman run` or `podman start` commands. When Podman runs in rootless mode, the file `$HOME/.config/containers/mounts.conf` is also used. Please refer to containers-mounts.conf(5) for further details. -**OCI hooks JSON** (`/etc/containers/oci/hooks.d/*.json`, `/usr/share/containers/oci/hooks.d/*.json`) - - Each `*.json` file in `/etc/containers/oci/hooks.d` and `/usr/share/containers/oci/hooks.d` configures a hook for Podman containers, with `/etc/containers/oci/hooks.d` having higher precedence. For more details on the syntax of the JSON files and the semantics of hook injection, see `oci-hooks(5)`. - - Podman and libpod currently support both the 1.0.0 and 0.1.0 hook schemas, although the 0.1.0 schema is deprecated. - - For the annotation conditions, libpod uses any annotations set in the generated OCI configuration. - - For the bind-mount conditions, only mounts explicitly requested by the caller via `--volume` are considered. Bind mounts that libpod inserts by default (e.g. `/dev/shm`) are not considered. - - Hooks are not used when running in rootless mode. - **policy.json** (`/etc/containers/policy.json`) Signature verification policy files are used to specify policy, e.g. trusted keys, applicable when deciding whether to accept an image, or individual signatures of that image, as valid. diff --git a/libpod/container_internal.go b/libpod/container_internal.go index e31a8099c..934ad7a22 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1168,10 +1168,6 @@ func (c *Container) saveSpec(spec *spec.Spec) error { } func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (extensionStageHooks map[string][]spec.Hook, err error) { - if len(c.runtime.config.HooksDir) == 0 { - return nil, nil - } - var locale string var ok bool for _, envVar := range []string{ @@ -1199,25 +1195,39 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten } } - allHooks := make(map[string][]spec.Hook) - for _, hDir := range c.runtime.config.HooksDir { - manager, err := hooks.New(ctx, []string{hDir}, []string{"poststop"}, lang) - if err != nil { - if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) { - return nil, err - } - logrus.Warnf("failed to load hooks: %q", err) + if c.runtime.config.HooksDir == nil { + if rootless.IsRootless() { return nil, nil } - hooks, err := manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0) - if err != nil { - return nil, err - } - for i, hook := range hooks { - allHooks[i] = hook + allHooks := make(map[string][]spec.Hook) + for _, hDir := range []string{hooks.DefaultDir, hooks.OverrideDir} { + manager, err := hooks.New(ctx, []string{hDir}, []string{"poststop"}, lang) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, err + } + hooks, err := manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0) + if err != nil { + return nil, err + } + if len(hooks) > 0 || config.Hooks != nil { + logrus.Warnf("implicit hook directories are deprecated; set --hooks-dir=%q explicitly to continue to load hooks from this directory", hDir) + } + for i, hook := range hooks { + allHooks[i] = hook + } } + return allHooks, nil } - return allHooks, nil + + manager, err := hooks.New(ctx, c.runtime.config.HooksDir, []string{"poststop"}, lang) + if err != nil { + return nil, err + } + + return manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0) } // mount mounts the container's root filesystem diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 8861d7728..780bf5279 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -224,10 +224,8 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } } - if !rootless.IsRootless() { - if c.state.ExtensionStageHooks, err = c.setupOCIHooks(ctx, g.Config); err != nil { - return nil, errors.Wrapf(err, "error setting up OCI Hooks") - } + if c.state.ExtensionStageHooks, err = c.setupOCIHooks(ctx, g.Config); err != nil { + return nil, errors.Wrapf(err, "error setting up OCI Hooks") } // Bind builtin image volumes diff --git a/libpod/options.go b/libpod/options.go index 7f4e3ac6b..e1d0b5007 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -179,21 +179,20 @@ func WithStaticDir(dir string) RuntimeOption { } } -// WithHooksDir sets the directory to look for OCI runtime hooks config. -// Note we are not saving this in database, since this is really just for used -// for testing. -func WithHooksDir(hooksDir string) RuntimeOption { +// WithHooksDir sets the directories to look for OCI runtime hook configuration. +func WithHooksDir(hooksDirs ...string) RuntimeOption { return func(rt *Runtime) error { if rt.valid { return ErrRuntimeFinalized } - if hooksDir == "" { - return errors.Wrap(ErrInvalidArg, "empty-string hook directories are not supported") + for _, hooksDir := range hooksDirs { + if hooksDir == "" { + return errors.Wrap(ErrInvalidArg, "empty-string hook directories are not supported") + } } - rt.config.HooksDir = []string{hooksDir} - rt.config.HooksDirNotExistFatal = true + rt.config.HooksDir = hooksDirs return nil } } diff --git a/libpod/runtime.go b/libpod/runtime.go index 9feae03fc..e043715b1 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -12,7 +12,6 @@ import ( "github.com/containers/image/types" "github.com/containers/libpod/libpod/image" "github.com/containers/libpod/pkg/firewall" - "github.com/containers/libpod/pkg/hooks" sysreg "github.com/containers/libpod/pkg/registries" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/pkg/util" @@ -141,11 +140,11 @@ type RuntimeConfig struct { // CNIDefaultNetwork is the network name of the default CNI network // to attach pods to CNIDefaultNetwork string `toml:"cni_default_network,omitempty"` - // HooksDir Path to the directory containing hooks configuration files + // HooksDir holds paths to the directories containing hooks + // configuration files. When the same filename is present in in + // multiple directories, the file in the directory listed last in + // this slice takes precedence. HooksDir []string `toml:"hooks_dir"` - // HooksDirNotExistFatal switches between fatal errors and non-fatal - // warnings if the configured HooksDir does not exist. - HooksDirNotExistFatal bool `toml:"hooks_dir_not_exist_fatal"` // DefaultMountsFile is the path to the default mounts file for testing // purposes only DefaultMountsFile string `toml:"-"` @@ -203,7 +202,6 @@ var ( "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", }, CgroupManager: SystemdCgroupsManager, - HooksDir: []string{hooks.DefaultDir, hooks.OverrideDir}, StaticDir: filepath.Join(storage.DefaultStoreOptions.GraphRoot, "libpod"), TmpDir: "", MaxLogSize: -1, diff --git a/libpod/testdata/config.toml b/libpod/testdata/config.toml index e19d36017..1d78f2083 100644 --- a/libpod/testdata/config.toml +++ b/libpod/testdata/config.toml @@ -14,7 +14,7 @@ seccomp_profile = "/etc/crio/seccomp.json" apparmor_profile = "crio-default" cgroup_manager = "cgroupfs" - hooks_dir_path = "/usr/share/containers/oci/hooks.d" + hooks_dir = ["/usr/share/containers/oci/hooks.d"] pids_limit = 2048 container_exits_dir = "/var/run/podman/exits" [crio.image] diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 38504828b..b04116218 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -336,7 +336,7 @@ var _ = Describe("Podman run", func() { hooksDir := tempdir + "/hooks" os.Mkdir(hooksDir, 0755) fileutils.CopyFile("hooks/hooks.json", hooksDir) - os.Setenv("HOOK_OPTION", fmt.Sprintf("--hooks-dir-path=%s", hooksDir)) + os.Setenv("HOOK_OPTION", fmt.Sprintf("--hooks-dir=%s", hooksDir)) os.Remove(hcheck) session := podmanTest.Podman([]string{"run", ALPINE, "ls"}) session.Wait(10) -- cgit v1.2.3-54-g00ecf