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_api.go | 8 +++--- libpod/container_internal.go | 56 ++++++++++++++++++++++---------------- libpod/container_internal_linux.go | 44 ++++++------------------------ 3 files changed, 45 insertions(+), 63 deletions(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index 41a131ea2..83f93cf9e 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -10,8 +10,8 @@ import ( "time" "github.com/containers/libpod/libpod/driver" - "github.com/containers/libpod/pkg/chrootuser" "github.com/containers/libpod/pkg/inspect" + "github.com/containers/libpod/pkg/lookup" "github.com/containers/storage/pkg/stringid" "github.com/docker/docker/daemon/caps" "github.com/pkg/errors" @@ -292,13 +292,13 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e // the host hostUser := "" if user != "" { - uid, gid, err := chrootuser.GetUser(c.state.Mountpoint, user) + execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, nil) if err != nil { - return errors.Wrapf(err, "error getting user to launch exec session as") + return err } // runc expects user formatted as uid:gid - hostUser = fmt.Sprintf("%d:%d", uid, gid) + hostUser = fmt.Sprintf("%d:%d", execUser.Uid, execUser.Gid) } // Generate exec session ID 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()) } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 0a1784ba7..7bf2c71ca 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -19,12 +19,10 @@ import ( cnitypes "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" crioAnnotations "github.com/containers/libpod/pkg/annotations" - "github.com/containers/libpod/pkg/chrootuser" "github.com/containers/libpod/pkg/criu" + "github.com/containers/libpod/pkg/lookup" "github.com/containers/libpod/pkg/rootless" "github.com/containers/storage/pkg/idtools" - "github.com/cyphar/filepath-securejoin" - "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" @@ -135,6 +133,10 @@ func (c *Container) cleanupNetwork() error { // Generate spec for a container // Accepts a map of the container's dependencies func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { + execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, c.config.User, nil) + if err != nil { + return nil, err + } g := generate.NewFromSpec(c.config.Spec) // If network namespace was requested, add it now @@ -188,7 +190,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } } - var err 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") @@ -206,13 +207,9 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { if !c.state.Mounted { return nil, 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 nil, err - } // User and Group must go together - g.SetProcessUID(uid) - g.SetProcessGID(gid) + g.SetProcessUID(uint32(execUser.Uid)) + g.SetProcessGID(uint32(execUser.Gid)) } // Add addition groups if c.config.GroupAdd is not empty @@ -220,11 +217,8 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { if !c.state.Mounted { return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to add additional groups", c.ID()) } - for _, group := range c.config.Groups { - gid, err := chrootuser.GetGroup(c.state.Mountpoint, group) - if err != nil { - return nil, err - } + gids, _ := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, nil) + for _, gid := range gids { g.AddProcessAdditionalGid(gid) } } @@ -237,26 +231,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { // Look up and add groups the user belongs to, if a group wasn't directly specified if !rootless.IsRootless() && !strings.Contains(c.config.User, ":") { - var groupDest, passwdDest string - defaultExecUser := user.ExecUser{ - Uid: 0, - Gid: 0, - Home: "/", - } - - // Make sure the /etc/group and /etc/passwd destinations are not a symlink to something naughty - if groupDest, err = securejoin.SecureJoin(c.state.Mountpoint, "/etc/group"); err != nil { - logrus.Debug(err) - return nil, err - } - if passwdDest, err = securejoin.SecureJoin(c.state.Mountpoint, "/etc/passwd"); err != nil { - logrus.Debug(err) - return nil, err - } - execUser, err := user.GetExecUserPath(c.config.User, &defaultExecUser, passwdDest, groupDest) - if err != nil { - return nil, err - } for _, gid := range execUser.Sgids { g.AddProcessAdditionalGid(uint32(gid)) } -- cgit v1.2.3-54-g00ecf From f77d846536718950575fa5206d9e3654ba140590 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 23 Oct 2018 11:20:01 +0200 Subject: attach: fix attach when cuid is too long conmon creates a symlink to avoid using a too long UNIX path. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1641800 There is still one issue when the path length of the symlink has the same length of the attach socket parent directory since conmon fails to create the symlink, but that must be addressed in conmon first. Signed-off-by: Giuseppe Scrivano --- libpod/container_attach.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'libpod') diff --git a/libpod/container_attach.go b/libpod/container_attach.go index 3c4e0775d..f925c3897 100644 --- a/libpod/container_attach.go +++ b/libpod/container_attach.go @@ -16,6 +16,10 @@ import ( "k8s.io/client-go/tools/remotecommand" ) +//#include +// extern int unix_path_length(){struct sockaddr_un addr; return sizeof(addr.sun_path) - 1;} +import "C" + /* Sync with stdpipe_t in conmon.c */ const ( AttachPipeStdin = 1 @@ -81,11 +85,19 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi logrus.Warnf("Failed to write to control file to resize terminal: %v", err) } }) - logrus.Debug("connecting to socket ", c.AttachSocketPath()) - conn, err := net.DialUnix("unixpacket", nil, &net.UnixAddr{Name: c.AttachSocketPath(), Net: "unixpacket"}) + socketPath := c.AttachSocketPath() + + maxUnixLength := int(C.unix_path_length()) + if maxUnixLength < len(socketPath) { + socketPath = socketPath[0:maxUnixLength] + } + + logrus.Debug("connecting to socket ", socketPath) + + conn, err := net.DialUnix("unixpacket", nil, &net.UnixAddr{Name: socketPath, Net: "unixpacket"}) if err != nil { - return errors.Wrapf(err, "failed to connect to container's attach socket: %v", c.AttachSocketPath()) + return errors.Wrapf(err, "failed to connect to container's attach socket: %v", socketPath) } defer conn.Close() -- cgit v1.2.3-54-g00ecf From b2fef1a8badb875339d6329c095842baee4b3a17 Mon Sep 17 00:00:00 2001 From: Anders F Björklund Date: Tue, 30 Oct 2018 23:55:48 +0100 Subject: Fix setting of version information MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was setting the wrong variable (CamelCase) in the wrong module ("main", not "libpod")... Signed-off-by: Anders F Björklund --- Makefile | 3 ++- cmd/podman/info.go | 3 ++- cmd/podman/version.go | 2 +- libpod/version.go | 10 +++++----- pkg/varlinkapi/system.go | 13 ++++++++----- 5 files changed, 18 insertions(+), 13 deletions(-) (limited to 'libpod') diff --git a/Makefile b/Makefile index b3e2fcc26..d8a870fa2 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,8 @@ PACKAGES ?= $(shell $(GO) list -tags "${BUILDTAGS}" ./... | grep -v github.com/c COMMIT_NO ?= $(shell git rev-parse HEAD 2> /dev/null || true) GIT_COMMIT ?= $(if $(shell git status --porcelain --untracked-files=no),"${COMMIT_NO}-dirty","${COMMIT_NO}") BUILD_INFO ?= $(shell date +%s) -LDFLAGS_PODMAN ?= $(LDFLAGS) -X main.gitCommit=$(GIT_COMMIT) -X main.buildInfo=$(BUILD_INFO) +LIBPOD := ${PROJECT}/libpod +LDFLAGS_PODMAN ?= $(LDFLAGS) -X $(LIBPOD).gitCommit=$(GIT_COMMIT) -X $(LIBPOD).buildInfo=$(BUILD_INFO) ISODATE ?= $(shell date --iso-8601) LIBSECCOMP_COMMIT := release-2.3 diff --git a/cmd/podman/info.go b/cmd/podman/info.go index 563e63ba3..c0639725e 100644 --- a/cmd/podman/info.go +++ b/cmd/podman/info.go @@ -81,6 +81,7 @@ func debugInfo(c *cli.Context) map[string]interface{} { info["compiler"] = runtime.Compiler info["go version"] = runtime.Version() info["podman version"] = c.App.Version - info["git commit"] = libpod.GitCommit + version, _ := libpod.GetVersion() + info["git commit"] = version.GitCommit return info } diff --git a/cmd/podman/version.go b/cmd/podman/version.go index f896229c4..d80f24a14 100644 --- a/cmd/podman/version.go +++ b/cmd/podman/version.go @@ -21,7 +21,7 @@ func versionCmd(c *cli.Context) error { fmt.Println("Git Commit: ", output.GitCommit) } // Prints out the build time in readable format - if libpod.BuildInfo != "" { + if output.Built != 0 { fmt.Println("Built: ", time.Unix(output.Built, 0).Format(time.ANSIC)) } diff --git a/libpod/version.go b/libpod/version.go index 5e7cd83c9..966588ae9 100644 --- a/libpod/version.go +++ b/libpod/version.go @@ -11,10 +11,10 @@ import ( var ( // GitCommit is the commit that the binary is being built from. // It will be populated by the Makefile. - GitCommit string + gitCommit string // BuildInfo is the time at which the binary was built // It will be populated by the Makefile. - BuildInfo string + buildInfo string ) //Version is an output struct for varlink @@ -30,9 +30,9 @@ type Version struct { func GetVersion() (Version, error) { var err error var buildTime int64 - if BuildInfo != "" { + if buildInfo != "" { // Converts unix time from string to int64 - buildTime, err = strconv.ParseInt(BuildInfo, 10, 64) + buildTime, err = strconv.ParseInt(buildInfo, 10, 64) if err != nil { return Version{}, err @@ -41,7 +41,7 @@ func GetVersion() (Version, error) { return Version{ Version: podmanVersion.Version, GoVersion: runtime.Version(), - GitCommit: GitCommit, + GitCommit: gitCommit, Built: buildTime, OsArch: runtime.GOOS + "/" + runtime.GOARCH, }, nil diff --git a/pkg/varlinkapi/system.go b/pkg/varlinkapi/system.go index 287f42209..a29d22e7d 100644 --- a/pkg/varlinkapi/system.go +++ b/pkg/varlinkapi/system.go @@ -34,6 +34,10 @@ func (i *LibpodAPI) Ping(call iopodman.VarlinkCall) error { // GetInfo returns details about the podman host and its stores func (i *LibpodAPI) GetInfo(call iopodman.VarlinkCall) error { + versionInfo, err := libpod.GetVersion() + if err != nil { + return err + } var ( registries, insecureRegistries []string ) @@ -64,11 +68,10 @@ func (i *LibpodAPI) GetInfo(call iopodman.VarlinkCall) error { podmanInfo.Host = infoHost store := info[1].Data pmaninfo := iopodman.InfoPodmanBinary{ - Compiler: goruntime.Compiler, - Go_version: goruntime.Version(), - // TODO : How are we going to get this here? - //Podman_version: - Git_commit: libpod.GitCommit, + Compiler: goruntime.Compiler, + Go_version: goruntime.Version(), + Podman_version: versionInfo.Version, + Git_commit: versionInfo.GitCommit, } graphStatus := iopodman.InfoGraphStatus{ -- cgit v1.2.3-54-g00ecf From ea50ce6a5915f71e0cf355d2006ad035adf37034 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 31 Oct 2018 11:43:33 +0100 Subject: rootless: avoid hang on failed slirp4netns If for any reason slirp4netns fails at startup, podman waits indefinitely. Check every second if the process is still running so that we avoid to hang. Signed-off-by: Giuseppe Scrivano --- libpod/networking_linux.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'libpod') diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 0d9ec2809..863a764e2 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" "syscall" + "time" cnitypes "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" @@ -134,12 +135,33 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessSlirpSyncR, syncW) if err := cmd.Start(); err != nil { - return errors.Wrapf(err, "failed to start process") + return errors.Wrapf(err, "failed to start slirp4netns process") } + defer cmd.Process.Release() b := make([]byte, 16) - if _, err := syncR.Read(b); err != nil { - return errors.Wrapf(err, "failed to read from sync pipe") + for { + if err := syncR.SetDeadline(time.Now().Add(1 * time.Second)); err != nil { + return errors.Wrapf(err, "error setting slirp4netns pipe timeout") + } + if _, err := syncR.Read(b); err == nil { + break + } else { + if os.IsTimeout(err) { + // Check if the process is still running. + var status syscall.WaitStatus + _, err := syscall.Wait4(cmd.Process.Pid, &status, syscall.WNOHANG, nil) + if err != nil { + return errors.Wrapf(err, "failed to read slirp4netns process status") + } + if status.Exited() || status.Signaled() { + return errors.New("slirp4netns failed") + } + + continue + } + return errors.Wrapf(err, "failed to read from slirp4netns sync pipe") + } } return nil } -- cgit v1.2.3-54-g00ecf From 3bacacce94089c4104cea0fff0f2945c22fc4813 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 6 Nov 2018 16:28:10 -0500 Subject: Remove conmon cgroup before pod cgroup for cgroupfs For pods using cgroupfs, we were seeing some error messages in CI from an inability to remove the pod CGroup, which was traced down to the conmon cgroup still being present as a child. Try to remove these error messages and ensure successful CGroup deletion by removing the conmon CGroup first, then the pod cgroup. Signed-off-by: Matthew Heon --- libpod/runtime_pod_linux.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index eb3d471dd..3d6fad52f 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -265,15 +265,26 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool) } case CgroupfsCgroupsManager: // Delete the cgroupfs cgroup + // Make sure the conmon cgroup is deleted first + // Since the pod is almost gone, don't bother failing + // hard - instead, just log errors. v1CGroups := GetV1CGroups(getExcludedCGroups()) + conmonCgroupPath := filepath.Join(p.state.CgroupPath, "conmon") + conmonCgroup, err := cgroups.Load(v1CGroups, cgroups.StaticPath(conmonCgroupPath)) + if err != nil && err != cgroups.ErrCgroupDeleted { + return err + } + if err == nil { + if err := conmonCgroup.Delete(); err != nil { + logrus.Errorf("Error deleting pod %s conmon cgroup %s: %v", p.ID(), conmonCgroupPath, err) + } + } cgroup, err := cgroups.Load(v1CGroups, cgroups.StaticPath(p.state.CgroupPath)) if err != nil && err != cgroups.ErrCgroupDeleted { return err - } else if err == nil { + } + if err == nil { if err := cgroup.Delete(); err != nil { - // The pod is already almost gone. - // No point in hard-failing if we fail - // this bit of cleanup. logrus.Errorf("Error deleting pod %s cgroup %s: %v", p.ID(), p.state.CgroupPath, err) } } -- 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') 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 11c5b0237b799ad61b53d67817af673a3d286f25 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 5 Nov 2018 20:02:50 +0100 Subject: rootless: don't bind mount /sys/fs/cgroup/systemd in systemd mode it is not writeable by non-root users so there is no point in having access to it from a container. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 7bf2c71ca..dc829ca55 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -360,19 +360,23 @@ func (c *Container) setupSystemd(mounts []spec.Mount, g generate.Generator) erro g.AddMount(tmpfsMnt) } - cgroupPath, err := c.CGroupPath() - if err != nil { - return err - } - sourcePath := filepath.Join("/sys/fs/cgroup/systemd", cgroupPath) + // rootless containers have no write access to /sys/fs/cgroup, so don't + // add any mount into the container. + if !rootless.IsRootless() { + cgroupPath, err := c.CGroupPath() + if err != nil { + return err + } + sourcePath := filepath.Join("/sys/fs/cgroup/systemd", cgroupPath) - systemdMnt := spec.Mount{ - Destination: "/sys/fs/cgroup/systemd", - Type: "bind", - Source: sourcePath, - Options: []string{"bind", "private"}, + systemdMnt := spec.Mount{ + Destination: "/sys/fs/cgroup/systemd", + Type: "bind", + Source: sourcePath, + Options: []string{"bind", "private"}, + } + g.AddMount(systemdMnt) } - g.AddMount(systemdMnt) return nil } -- cgit v1.2.3-54-g00ecf From f813881b81ca669119bd7ac0c313c4f3198228c2 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 5 Nov 2018 22:48:29 +0100 Subject: rootless: mount /sys/fs/cgroup/systemd from the host systemd requires /sys/fs/cgroup/systemd to be writeable. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index dc829ca55..d89eefd3b 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -376,6 +376,14 @@ func (c *Container) setupSystemd(mounts []spec.Mount, g generate.Generator) erro Options: []string{"bind", "private"}, } g.AddMount(systemdMnt) + } else { + systemdMnt := spec.Mount{ + Destination: "/sys/fs/cgroup/systemd", + Type: "bind", + Source: "/sys/fs/cgroup/systemd", + Options: []string{"bind", "nodev", "noexec", "nosuid"}, + } + g.AddMount(systemdMnt) } return nil -- 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') 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') 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 94763a47a6e86ccf13d8a3ded3365f9c4440a668 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 25 Oct 2018 13:28:19 -0400 Subject: If a container ceases to exist in runc, set exit status When we scan a container in runc and see that it no longer exists, we already set ContainerStatusExited to indicate that it no longer exists in runc. Now, also set an exit code and exit time, so PS output will make some sense. Signed-off-by: Matthew Heon --- libpod/oci.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index b390b1967..e8a97226a 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -508,6 +508,8 @@ func (r *OCIRuntime) updateContainerStatus(ctr *Container, useRunc bool) error { } if strings.Contains(string(out), "does not exist") { ctr.removeConmonFiles() + ctr.state.ExitCode = -1 + ctr.state.FinishedTime = time.Now() ctr.state.State = ContainerStateExited return nil } -- 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') 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 0f45403c9ba5eace4b48bad2b86ad540bae3435d Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 25 Oct 2018 13:49:39 -0400 Subject: Fix misspelling Signed-off-by: Matthew Heon --- libpod/oci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index d3ce70133..233bacfbb 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -451,7 +451,7 @@ 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. // 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 +// status, but will instead only check for the existence 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() -- 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') 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') 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') 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') 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 46e672bad6d7d616961ae0fc7fb0b484951ef07d Mon Sep 17 00:00:00 2001 From: baude Date: Thu, 8 Nov 2018 13:45:11 -0600 Subject: correct assignment of networkStatus once we changed configureNetNS to return a result beyond an error, we need to make sure that we used locals instead of ctr attributes when determining networks. Resolves #1752 Signed-off-by: baude --- libpod/networking_linux.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'libpod') diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 863a764e2..212485d8a 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -64,20 +64,20 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re } }() - networkStatus := make([]*cnitypes.Result, 1) + networkStatus := make([]*cnitypes.Result, 0) for idx, r := range results { logrus.Debugf("[%d] CNI result: %v", idx, r.String()) resultCurrent, err := cnitypes.GetResult(r) if err != nil { return nil, errors.Wrapf(err, "error parsing CNI plugin result %q: %v", r.String(), err) } - networkStatus = append(ctr.state.NetworkStatus, resultCurrent) + networkStatus = append(networkStatus, resultCurrent) } // Add firewall rules to ensure the container has network access. // Will not be necessary once CNI firewall plugin merges upstream. // https://github.com/containernetworking/plugins/pull/75 - for _, netStatus := range ctr.state.NetworkStatus { + for _, netStatus := range networkStatus { firewallConf := &firewall.FirewallNetConf{ PrevResult: netStatus, } -- cgit v1.2.3-54-g00ecf From 2dd9cae37cb076418393ba61c0fb7b8cf97148f3 Mon Sep 17 00:00:00 2001 From: baude Date: Wed, 7 Nov 2018 13:20:43 -0600 Subject: rm -f now removes a paused container We now can remove a paused container by sending it a kill signal while it is paused. We then unpause the container and it is immediately killed. Also, reworked how the parallelWorker results are handled to provide a more consistent approach to how each subcommand implements it. It also fixes a bug where if one container errors, the error message is duplicated when printed out. Signed-off-by: baude --- cmd/podman/kill.go | 23 +++++++---------------- cmd/podman/pause.go | 17 ++--------------- cmd/podman/restart.go | 16 ++-------------- cmd/podman/rm.go | 26 +++++++++++--------------- cmd/podman/shared/parallel.go | 10 +++++++--- cmd/podman/stop.go | 23 +++++++++-------------- cmd/podman/unpause.go | 18 ++---------------- cmd/podman/utils.go | 17 +++++++++++++++++ docs/podman-rm.1.md | 19 ++++++++++++++++--- libpod/runtime_ctr.go | 14 +++++++++++++- test/e2e/pause_test.go | 19 ++++++++++--------- 11 files changed, 96 insertions(+), 106 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/kill.go b/cmd/podman/kill.go index 27882aeee..cfe4b4218 100644 --- a/cmd/podman/kill.go +++ b/cmd/podman/kill.go @@ -43,7 +43,6 @@ var ( // killCmd kills one or more containers with a signal func killCmd(c *cli.Context) error { var ( - lastError error killFuncs []shared.ParallelWorkerInput killSignal uint = uint(syscall.SIGTERM) ) @@ -75,8 +74,12 @@ func killCmd(c *cli.Context) error { containers, err := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") if err != nil { - return err + if len(containers) == 0 { + return err + } + fmt.Println(err.Error()) } + for _, ctr := range containers { con := ctr f := func() error { @@ -95,18 +98,6 @@ func killCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - killErrors := shared.ParallelExecuteWorkerPool(maxWorkers, killFuncs) - - for cid, result := range killErrors { - if result != nil { - if len(killErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - - return lastError + killErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, killFuncs) + return printParallelOutput(killErrors, errCount) } diff --git a/cmd/podman/pause.go b/cmd/podman/pause.go index 1e1585216..fcb2f3cb8 100644 --- a/cmd/podman/pause.go +++ b/cmd/podman/pause.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "github.com/containers/libpod/cmd/podman/libpodruntime" @@ -37,7 +36,6 @@ var ( func pauseCmd(c *cli.Context) error { var ( - lastError error pauseContainers []*libpod.Container pauseFuncs []shared.ParallelWorkerInput ) @@ -90,17 +88,6 @@ func pauseCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - pauseErrors := shared.ParallelExecuteWorkerPool(maxWorkers, pauseFuncs) - - for cid, result := range pauseErrors { - if result != nil { - if len(pauseErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + pauseErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, pauseFuncs) + return printParallelOutput(pauseErrors, errCount) } diff --git a/cmd/podman/restart.go b/cmd/podman/restart.go index 2e264db79..630493ef4 100644 --- a/cmd/podman/restart.go +++ b/cmd/podman/restart.go @@ -1,8 +1,6 @@ package main import ( - "fmt" - "github.com/containers/libpod/cmd/podman/libpodruntime" "github.com/containers/libpod/cmd/podman/shared" "github.com/containers/libpod/libpod" @@ -46,7 +44,6 @@ func restartCmd(c *cli.Context) error { var ( restartFuncs []shared.ParallelWorkerInput containers []*libpod.Container - lastError error restartContainers []*libpod.Container ) @@ -124,15 +121,6 @@ func restartCmd(c *cli.Context) error { logrus.Debugf("Setting maximum workers to %d", maxWorkers) - restartErrors := shared.ParallelExecuteWorkerPool(maxWorkers, restartFuncs) - - for cid, result := range restartErrors { - if result != nil { - fmt.Println(result.Error()) - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + restartErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, restartFuncs) + return printParallelOutput(restartErrors, errCount) } diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go index 0fb5345ee..7c0569b78 100644 --- a/cmd/podman/rm.go +++ b/cmd/podman/rm.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/containers/libpod/cmd/podman/libpodruntime" "github.com/containers/libpod/cmd/podman/shared" - "github.com/containers/libpod/libpod" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/urfave/cli" @@ -46,9 +45,7 @@ Running containers will not be removed without the -f option. // saveCmd saves the image to either docker-archive or oci func rmCmd(c *cli.Context) error { var ( - delContainers []*libpod.Container - lastError error - deleteFuncs []shared.ParallelWorkerInput + deleteFuncs []shared.ParallelWorkerInput ) ctx := getContext() @@ -65,7 +62,13 @@ func rmCmd(c *cli.Context) error { return err } - delContainers, lastError = getAllOrLatestContainers(c, runtime, -1, "all") + delContainers, err := getAllOrLatestContainers(c, runtime, -1, "all") + if err != nil { + if len(delContainers) == 0 { + return err + } + fmt.Println(err.Error()) + } for _, container := range delContainers { con := container @@ -84,14 +87,7 @@ func rmCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - deleteErrors := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs) - for cid, result := range deleteErrors { - if result != nil { - fmt.Println(result.Error()) - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + // Run the parallel funcs + deleteErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs) + return printParallelOutput(deleteErrors, errCount) } diff --git a/cmd/podman/shared/parallel.go b/cmd/podman/shared/parallel.go index 633781a45..e6ce50f95 100644 --- a/cmd/podman/shared/parallel.go +++ b/cmd/podman/shared/parallel.go @@ -30,9 +30,10 @@ func ParallelWorker(wg *sync.WaitGroup, jobs <-chan ParallelWorkerInput, results // ParallelExecuteWorkerPool takes container jobs and performs them in parallel. The worker // int determines how many workers/threads should be premade. -func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) map[string]error { +func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) (map[string]error, int) { var ( - wg sync.WaitGroup + wg sync.WaitGroup + errorCount int ) resultChan := make(chan containerError, len(functions)) @@ -62,9 +63,12 @@ func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) map close(resultChan) for ctrError := range resultChan { results[ctrError.ContainerID] = ctrError.Err + if ctrError.Err != nil { + errorCount += 1 + } } - return results + return results, errorCount } // Parallelize provides the maximum number of parallel workers (int) as calculated by a basic diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go index cb36fd5cd..04022839a 100644 --- a/cmd/podman/stop.go +++ b/cmd/podman/stop.go @@ -59,7 +59,13 @@ func stopCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") + containers, err := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") + if err != nil { + if len(containers) == 0 { + return err + } + fmt.Println(err.Error()) + } var stopFuncs []shared.ParallelWorkerInput for _, ctr := range containers { @@ -85,17 +91,6 @@ func stopCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - stopErrors := shared.ParallelExecuteWorkerPool(maxWorkers, stopFuncs) - - for cid, result := range stopErrors { - if result != nil && result != libpod.ErrCtrStopped { - if len(stopErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + stopErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, stopFuncs) + return printParallelOutput(stopErrors, errCount) } diff --git a/cmd/podman/unpause.go b/cmd/podman/unpause.go index 648fc9d3d..d77e056f8 100644 --- a/cmd/podman/unpause.go +++ b/cmd/podman/unpause.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "github.com/containers/libpod/cmd/podman/libpodruntime" @@ -37,7 +36,6 @@ var ( func unpauseCmd(c *cli.Context) error { var ( - lastError error unpauseContainers []*libpod.Container unpauseFuncs []shared.ParallelWorkerInput ) @@ -90,18 +88,6 @@ func unpauseCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - unpauseErrors := shared.ParallelExecuteWorkerPool(maxWorkers, unpauseFuncs) - - for cid, result := range unpauseErrors { - if result != nil && result != libpod.ErrCtrStopped { - if len(unpauseErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - - return lastError + unpauseErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, unpauseFuncs) + return printParallelOutput(unpauseErrors, errCount) } diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go index afeccb668..5735156c2 100644 --- a/cmd/podman/utils.go +++ b/cmd/podman/utils.go @@ -207,3 +207,20 @@ func getPodsFromContext(c *cli.Context, r *libpod.Runtime) ([]*libpod.Pod, error } return pods, lastError } + +//printParallelOutput takes the map of parallel worker results and outputs them +// to stdout +func printParallelOutput(m map[string]error, errCount int) error { + var lastError error + for cid, result := range m { + if result != nil { + if errCount > 1 { + fmt.Println(result.Error()) + } + lastError = result + continue + } + fmt.Println(cid) + } + return lastError +} diff --git a/docs/podman-rm.1.md b/docs/podman-rm.1.md index 7474a0d1f..56664a8c1 100644 --- a/docs/podman-rm.1.md +++ b/docs/podman-rm.1.md @@ -13,7 +13,7 @@ podman\-rm - Remove one or more containers **--force, f** -Force the removal of a running container +Force the removal of a running and paused containers **--all, a** @@ -29,16 +29,29 @@ to run containers such as CRI-O, the last started container could be from either Remove the volumes associated with the container. (Not yet implemented) ## EXAMPLE - +Remove a container by its name *mywebserver* +``` podman rm mywebserver - +``` +Remove several containers by name and container id. +``` podman rm mywebserver myflaskserver 860a4b23 +``` +Forcibly remove a container by container ID. +``` podman rm -f 860a4b23 +``` +Remove all containers regardless of its run state. +``` podman rm -f -a +``` +Forcibly remove the latest container created. +``` podman rm -f --latest +``` ## SEE ALSO podman(1), podman-rmi(1) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 09dc7c48b..09d0ec042 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -246,7 +246,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) } if c.state.State == ContainerStatePaused { - return errors.Wrapf(ErrCtrStateInvalid, "container %s is paused, cannot remove until unpaused", c.ID()) + if !force { + return errors.Wrapf(ErrCtrStateInvalid, "container %s is paused, cannot remove until unpaused", c.ID()) + } + if err := c.runtime.ociRuntime.killContainer(c, 9); err != nil { + return err + } + if err := c.unpause(); err != nil { + return err + } + // Need to update container state to make sure we know it's stopped + if err := c.waitForExitFileAndSync(); err != nil { + return err + } } // Check that the container's in a good state to be removed diff --git a/test/e2e/pause_test.go b/test/e2e/pause_test.go index 1a2eb1a09..e80915670 100644 --- a/test/e2e/pause_test.go +++ b/test/e2e/pause_test.go @@ -91,7 +91,7 @@ var _ = Describe("Podman pause", func() { }) - It("podman remove a paused container by id", func() { + It("podman remove a paused container by id without force", func() { session := podmanTest.RunTopContainer("") session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -111,25 +111,26 @@ var _ = Describe("Podman pause", func() { Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) - result = podmanTest.Podman([]string{"rm", "--force", cid}) - result.WaitWithDefaultTimeout() + }) - Expect(result.ExitCode()).To(Equal(125)) - Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) - Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) + It("podman remove a paused container by id with force", func() { + session := podmanTest.RunTopContainer("") + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + cid := session.OutputToString() - result = podmanTest.Podman([]string{"unpause", cid}) + result := podmanTest.Podman([]string{"pause", cid}) result.WaitWithDefaultTimeout() Expect(result.ExitCode()).To(Equal(0)) - Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) result = podmanTest.Podman([]string{"rm", "--force", cid}) result.WaitWithDefaultTimeout() Expect(result.ExitCode()).To(Equal(0)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) - }) It("podman stop a paused container by id", func() { -- cgit v1.2.3-54-g00ecf From 2fb6ef9f407d7933c4dd6de7908bf53104e99e74 Mon Sep 17 00:00:00 2001 From: Šimon Lukašík Date: Thu, 8 Nov 2018 22:27:51 +0100 Subject: Do not hide errors when creating container with UserNSRoot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This one is tricky. By using `:=` operator we have made err variable to be local in the gorutine and different from `err` variable in the surrounding function. And thus `createContainer` function returned always nil, even in cases when some error occurred in the gorutine. Signed-off-by: Šimon Lukašík --- libpod/oci_linux.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go index 0447670b3..e6b7cbe4f 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -74,7 +74,8 @@ func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restor defer wg.Done() runtime.LockOSThread() - fd, err := os.Open(fmt.Sprintf("/proc/%d/task/%d/ns/mnt", os.Getpid(), unix.Gettid())) + var fd *os.File + fd, err = os.Open(fmt.Sprintf("/proc/%d/task/%d/ns/mnt", os.Getpid(), unix.Gettid())) if err != nil { return } -- cgit v1.2.3-54-g00ecf From 7e15084d1987425c70850353462b0b80269300f9 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 8 Nov 2018 16:06:59 -0500 Subject: Accurately update state if prepare() partially fails We are seeing some issues where, when part of prepare() fails (originally noticed due to a bad static IP), the other half does not successfully clean up, and the state can be left in a bad place (not knowing about an active SHM mount for example). Signed-off-by: Matthew Heon --- libpod/container_internal_linux.go | 58 ++++++++++++++++++++++++++------------ libpod/networking_linux.go | 5 +++- 2 files changed, 44 insertions(+), 19 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 163cd75e7..66c7e8a04 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -57,7 +57,7 @@ func (c *Container) prepare() (err error) { networkStatus []*cnitypes.Result createNetNSErr, mountStorageErr error mountPoint string - saveNetworkStatus bool + tmpStateLock sync.Mutex ) wg.Add(2) @@ -66,17 +66,55 @@ func (c *Container) prepare() (err error) { defer wg.Done() // Set up network namespace if not already set up if c.config.CreateNetNS && c.state.NetNS == nil && !c.config.PostConfigureNetNS { - saveNetworkStatus = true netNS, networkStatus, createNetNSErr = c.runtime.createNetNS(c) + + tmpStateLock.Lock() + defer tmpStateLock.Unlock() + + // Assign NetNS attributes to container + if createNetNSErr == nil { + c.state.NetNS = netNS + c.state.NetworkStatus = networkStatus + } } }() // Mount storage if not mounted go func() { defer wg.Done() mountPoint, mountStorageErr = c.mountStorage() + + if mountStorageErr != nil { + return + } + + tmpStateLock.Lock() + defer tmpStateLock.Unlock() + + // Finish up mountStorage + c.state.Mounted = true + c.state.Mountpoint = mountPoint + if c.state.UserNSRoot == "" { + c.state.RealMountpoint = c.state.Mountpoint + } else { + c.state.RealMountpoint = filepath.Join(c.state.UserNSRoot, "mountpoint") + } + + logrus.Debugf("Created root filesystem for container %s at %s", c.ID(), c.state.Mountpoint) + }() + + defer func() { + if err != nil { + if err2 := c.cleanupNetwork(); err2 != nil { + logrus.Errorf("Error cleaning up container %s network: %v", c.ID(), err2) + } + if err2 := c.cleanupStorage(); err2 != nil { + logrus.Errorf("Error cleaning up container %s storage: %v", c.ID(), err2) + } + } }() wg.Wait() + if createNetNSErr != nil { if mountStorageErr != nil { logrus.Error(createNetNSErr) @@ -88,22 +126,6 @@ func (c *Container) prepare() (err error) { return mountStorageErr } - // Assign NetNS attributes to container - if saveNetworkStatus { - c.state.NetNS = netNS - c.state.NetworkStatus = networkStatus - } - - // Finish up mountStorage - c.state.Mounted = true - c.state.Mountpoint = mountPoint - if c.state.UserNSRoot == "" { - c.state.RealMountpoint = c.state.Mountpoint - } else { - c.state.RealMountpoint = filepath.Join(c.state.UserNSRoot, "mountpoint") - } - - logrus.Debugf("Created root filesystem for container %s at %s", c.ID(), c.state.Mountpoint) // Save the container return c.save() } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 863a764e2..952ad83a5 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -90,13 +90,16 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re } // Create and configure a new network namespace for a container -func (r *Runtime) createNetNS(ctr *Container) (ns.NetNS, []*cnitypes.Result, error) { +func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q []*cnitypes.Result, err error) { ctrNS, err := netns.NewNS() if err != nil { return nil, nil, errors.Wrapf(err, "error creating network namespace for container %s", ctr.ID()) } defer func() { if err != nil { + if err2 := netns.UnmountNS(ctrNS); err2 != nil { + logrus.Errorf("Error unmounting partially created network namespace for container %s: %v", ctr.ID(), err2) + } if err2 := ctrNS.Close(); err2 != nil { logrus.Errorf("Error closing partially created network namespace for container %s: %v", ctr.ID(), err2) } -- cgit v1.2.3-54-g00ecf From 615df2ecd7ecc4a211d6293c753eea21cf0baab2 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 9 Nov 2018 09:35:54 +0100 Subject: info: add rootless field Add a rootless field to the info data (e.g., `podman info`) to indicate if the executing user is root or not. In most cases, this can be guessed but now it is clear and may aid in debugging, reporting and understanding certain issues. Signed-off-by: Valentin Rothberg --- libpod/info.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'libpod') diff --git a/libpod/info.go b/libpod/info.go index 4cbf3f734..5d8d160c8 100644 --- a/libpod/info.go +++ b/libpod/info.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/utils" "github.com/containers/storage/pkg/system" "github.com/pkg/errors" @@ -30,6 +31,7 @@ func (r *Runtime) hostInfo() (map[string]interface{}, error) { info["os"] = runtime.GOOS info["arch"] = runtime.GOARCH info["cpus"] = runtime.NumCPU() + info["rootless"] = rootless.IsRootless() mi, err := system.ReadMemInfo() if err != nil { return nil, errors.Wrapf(err, "error reading memory info") -- cgit v1.2.3-54-g00ecf From 223d102ec79496a3a7e92d75b32eb9938f461c28 Mon Sep 17 00:00:00 2001 From: Šimon Lukašík Date: Fri, 9 Nov 2018 10:23:24 +0100 Subject: Lint: Do not ignore errors from docker run command when selinux enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Redefining err by := operator within block makes this err variable block local. Addressing lint: libpod/oci.go:368:3:warning: ineffectual assignment to err (ineffassign) Signed-off-by: Šimon Lukašík --- libpod/oci.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index 233bacfbb..71da830b5 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -350,7 +350,8 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res // Set the label of the conmon process to be level :s0 // This will allow the container processes to talk to fifo-files // passed into the container by conmon - plabel, err := selinux.CurrentLabel() + var plabel string + plabel, err = selinux.CurrentLabel() if err != nil { childPipe.Close() return errors.Wrapf(err, "Failed to get current SELinux label") @@ -360,7 +361,7 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res runtime.LockOSThread() if c["level"] != "s0" && c["level"] != "" { c["level"] = "s0" - if err := label.SetProcessLabel(c.Get()); err != nil { + if err = label.SetProcessLabel(c.Get()); err != nil { runtime.UnlockOSThread() return err } -- cgit v1.2.3-54-g00ecf From 4e755515300d450139d85354082efdedf6fb5357 Mon Sep 17 00:00:00 2001 From: Šimon Lukašík Date: Fri, 9 Nov 2018 10:40:03 +0100 Subject: Lint: Tests: add missing assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addressing: ineffectual assignment to err (ineffassign) Signed-off-by: Šimon Lukašík --- libpod/container_graph_test.go | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'libpod') diff --git a/libpod/container_graph_test.go b/libpod/container_graph_test.go index bba3d7aad..25461f1f4 100644 --- a/libpod/container_graph_test.go +++ b/libpod/container_graph_test.go @@ -205,6 +205,7 @@ func TestBuildContainerGraphFourContainersNoEdges(t *testing.T) { ctr3, err := getTestCtrN("3", tmpDir) assert.NoError(t, err) ctr4, err := getTestCtrN("4", tmpDir) + assert.NoError(t, err) graph, err := buildContainerGraph([]*Container{ctr1, ctr2, ctr3, ctr4}) assert.NoError(t, err) @@ -241,6 +242,7 @@ func TestBuildContainerGraphFourContainersTwoInCycle(t *testing.T) { ctr3, err := getTestCtrN("3", tmpDir) assert.NoError(t, err) ctr4, err := getTestCtrN("4", tmpDir) + assert.NoError(t, err) ctr1.config.IPCNsCtr = ctr2.config.ID ctr2.config.UserNsCtr = ctr1.config.ID @@ -260,6 +262,7 @@ func TestBuildContainerGraphFourContainersAllInCycle(t *testing.T) { ctr3, err := getTestCtrN("3", tmpDir) assert.NoError(t, err) ctr4, err := getTestCtrN("4", tmpDir) + assert.NoError(t, err) ctr1.config.IPCNsCtr = ctr2.config.ID ctr2.config.UserNsCtr = ctr3.config.ID ctr3.config.NetNsCtr = ctr4.config.ID @@ -281,6 +284,7 @@ func TestBuildContainerGraphFourContainersNoneInCycle(t *testing.T) { ctr3, err := getTestCtrN("3", tmpDir) assert.NoError(t, err) ctr4, err := getTestCtrN("4", tmpDir) + assert.NoError(t, err) ctr1.config.IPCNsCtr = ctr2.config.ID ctr1.config.NetNsCtr = ctr3.config.ID ctr2.config.UserNsCtr = ctr3.config.ID -- cgit v1.2.3-54-g00ecf From 7457815ba930fdabff6ee57f4b7ec29d9fbcfcf8 Mon Sep 17 00:00:00 2001 From: Šimon Lukašík Date: Sat, 10 Nov 2018 10:26:57 +0100 Subject: Lint: Extract constant unknownPackage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addressing goconst warning: 3 other occurrence(s) of "Unknown" found Signed-off-by: Šimon Lukašík --- libpod/oci_linux.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go index e6b7cbe4f..b159eae78 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -19,6 +19,8 @@ import ( "golang.org/x/sys/unix" ) +const unknownPackage = "Unknown" + func (r *OCIRuntime) moveConmonToCgroup(ctr *Container, cgroupParent string, cmd *exec.Cmd) error { if os.Geteuid() == 0 { if r.cgroupManager == SystemdCgroupsManager { @@ -112,7 +114,7 @@ func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restor } func rpmVersion(path string) string { - output := "Unknown" + output := unknownPackage cmd := exec.Command("/usr/bin/rpm", "-q", "-f", path) if outp, err := cmd.Output(); err == nil { output = string(outp) @@ -121,7 +123,7 @@ func rpmVersion(path string) string { } func dpkgVersion(path string) string { - output := "Unknown" + output := unknownPackage cmd := exec.Command("/usr/bin/dpkg", "-S", path) if outp, err := cmd.Output(); err == nil { output = string(outp) @@ -130,14 +132,14 @@ func dpkgVersion(path string) string { } func (r *OCIRuntime) pathPackage() string { - if out := rpmVersion(r.path); out != "Unknown" { + if out := rpmVersion(r.path); out != unknownPackage { return out } return dpkgVersion(r.path) } func (r *OCIRuntime) conmonPackage() string { - if out := rpmVersion(r.conmonPath); out != "Unknown" { + if out := rpmVersion(r.conmonPath); out != unknownPackage { return out } return dpkgVersion(r.conmonPath) -- 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') 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 c3d8328150d12bd0981a43aeea2924bcc4d5c1cc Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Thu, 15 Nov 2018 09:32:51 -0500 Subject: Increase pidWaitTimeout to 60s At scale, it appears that we sometimes hit the 1000ms timeout to create the PID file when a container is created or executed. Increasing the value to 60s should help when running a lot of containers in heavy-loaded environment. Related #1495 Fixes #1816 Signed-off-by: Emilien Macchi --- libpod/container_api.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index d99aec5b4..390987394 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -330,9 +330,10 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e } pidFile := c.execPidPath(sessionID) - // 1 second seems a reasonable time to wait - // See https://github.com/containers/libpod/issues/1495 - const pidWaitTimeout = 1000 + // 60 second seems a reasonable time to wait + // https://github.com/containers/libpod/issues/1495 + // https://github.com/containers/libpod/issues/1816 + const pidWaitTimeout = 60000 // Wait until the runtime makes the pidfile // TODO: If runtime errors before the PID file is created, we have to -- cgit v1.2.3-54-g00ecf From a6aca6d106192a577ea1f2b1c0d82ddb5c73c2bc Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 13 Nov 2018 09:49:22 +0100 Subject: rootless: create libpod.conf when it doesn't exist Signed-off-by: Giuseppe Scrivano --- libpod/runtime.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/runtime.go b/libpod/runtime.go index 318cd0369..9feae03fc 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -264,6 +264,7 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { configPath := ConfigPath foundConfig := true + rootlessConfigPath := "" if rootless.IsRootless() { home := os.Getenv("HOME") if runtime.config.SignaturePolicyPath == "" { @@ -272,7 +273,10 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { runtime.config.SignaturePolicyPath = newPath } } - configPath = filepath.Join(home, ".config/containers/libpod.conf") + + rootlessConfigPath = filepath.Join(home, ".config/containers/libpod.conf") + + configPath = rootlessConfigPath if _, err := os.Stat(configPath); err != nil { foundConfig = false } @@ -317,6 +321,22 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { if err := makeRuntime(runtime); err != nil { return nil, err } + + if !foundConfig && rootlessConfigPath != "" { + os.MkdirAll(filepath.Dir(rootlessConfigPath), 0755) + file, err := os.OpenFile(rootlessConfigPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) + if err != nil && !os.IsExist(err) { + return nil, errors.Wrapf(err, "cannot open file %s", rootlessConfigPath) + } + if err == nil { + defer file.Close() + enc := toml.NewEncoder(file) + if err := enc.Encode(runtime.config); err != nil { + os.Remove(rootlessConfigPath) + } + } + } + return runtime, nil } -- cgit v1.2.3-54-g00ecf From f11a74e7150c2929a0ed18732ee0c4895f7e75be Mon Sep 17 00:00:00 2001 From: baude Date: Tue, 13 Nov 2018 10:53:18 -0600 Subject: output libpod container to kubernetes yaml scope out new kube subcommand where we can add generate. you can now generate kubernetes YAML that will allow you to run the container in a kubernetes environment. When The YAML description will always "wrap" a container in a simple v1.Pod description. Tests and further documentation will be added in additional PRs. This function should be considered very much "under heavy development" at this point. Signed-off-by: baude --- cmd/podman/kube.go | 22 ++++ cmd/podman/kube_generate.go | 93 +++++++++++++++ cmd/podman/main.go | 1 + libpod/kube.go | 270 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 386 insertions(+) create mode 100644 cmd/podman/kube.go create mode 100644 cmd/podman/kube_generate.go create mode 100644 libpod/kube.go (limited to 'libpod') diff --git a/cmd/podman/kube.go b/cmd/podman/kube.go new file mode 100644 index 000000000..ced87e2bd --- /dev/null +++ b/cmd/podman/kube.go @@ -0,0 +1,22 @@ +package main + +import ( + "github.com/urfave/cli" +) + +var ( + kubeSubCommands = []cli.Command{ + containerKubeCommand, + } + + kubeDescription = "Work with Kubernetes objects" + kubeCommand = cli.Command{ + Name: "kube", + Usage: "Import and export Kubernetes objections from and to Podman", + Description: containerDescription, + ArgsUsage: "", + Subcommands: kubeSubCommands, + UseShortOptionHandling: true, + OnUsageError: usageErrorHandler, + } +) diff --git a/cmd/podman/kube_generate.go b/cmd/podman/kube_generate.go new file mode 100644 index 000000000..a18912668 --- /dev/null +++ b/cmd/podman/kube_generate.go @@ -0,0 +1,93 @@ +package main + +import ( + "fmt" + + "github.com/containers/libpod/cmd/podman/libpodruntime" + "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/rootless" + "github.com/ghodss/yaml" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "github.com/urfave/cli" +) + +var ( + containerKubeFlags = []cli.Flag{ + cli.BoolFlag{ + Name: "service, s", + Usage: "only generate YAML for kubernetes service object", + }, + LatestFlag, + } + containerKubeDescription = "Generate Kubernetes Pod YAML" + containerKubeCommand = cli.Command{ + Name: "generate", + Usage: "Generate Kubernetes pod YAML for a container", + Description: containerKubeDescription, + Flags: sortFlags(containerKubeFlags), + Action: generateKubeYAMLCmd, + ArgsUsage: "CONTAINER-NAME", + UseShortOptionHandling: true, + OnUsageError: usageErrorHandler, + } +) + +// generateKubeYAMLCmdgenerates or replays kube +func generateKubeYAMLCmd(c *cli.Context) error { + var ( + container *libpod.Container + err error + output []byte + ) + + if rootless.IsRootless() { + return errors.Wrapf(libpod.ErrNotImplemented, "rootless users") + } + args := c.Args() + if len(args) > 1 || (len(args) < 1 && !c.Bool("latest")) { + return errors.Errorf("you must provide one container ID or name or --latest") + } + if c.Bool("service") { + return errors.Wrapf(libpod.ErrNotImplemented, "service generation") + } + + runtime, err := libpodruntime.GetRuntime(c) + if err != nil { + return errors.Wrapf(err, "could not get runtime") + } + defer runtime.Shutdown(false) + + // Get the container in question + if c.Bool("latest") { + container, err = runtime.GetLatestContainer() + } else { + container, err = runtime.LookupContainer(args[0]) + } + if err != nil { + return err + } + + if len(container.Dependencies()) > 0 { + return errors.Wrapf(libpod.ErrNotImplemented, "containers with dependencies") + } + + podYAML, err := container.InspectForKube() + if err != nil { + return err + } + + developmentComment := []byte("# Generation of Kubenetes YAML is still under development!\n") + logrus.Warn("This function is still under heavy development.") + // Marshall the results + b, err := yaml.Marshal(podYAML) + if err != nil { + return err + } + output = append(output, developmentComment...) + output = append(output, b...) + // Output the v1.Pod with the v1.Container + fmt.Println(string(output)) + + return nil +} diff --git a/cmd/podman/main.go b/cmd/podman/main.go index 38eac4504..6be192593 100644 --- a/cmd/podman/main.go +++ b/cmd/podman/main.go @@ -77,6 +77,7 @@ func main() { infoCommand, inspectCommand, killCommand, + kubeCommand, loadCommand, loginCommand, logoutCommand, diff --git a/libpod/kube.go b/libpod/kube.go new file mode 100644 index 000000000..00db0033b --- /dev/null +++ b/libpod/kube.go @@ -0,0 +1,270 @@ +package libpod + +import ( + "fmt" + "strings" + + "github.com/containers/libpod/pkg/lookup" + "github.com/containers/libpod/pkg/util" + "github.com/cri-o/ocicni/pkg/ocicni" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + v12 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// InspectForKube takes a slice of libpod containers and generates +// one v1.Pod description that includes just a single container. +func (c *Container) InspectForKube() (*v1.Pod, error) { + // Generate the v1.Pod yaml description + return simplePodWithV1Container(c) +} + +// simplePodWithV1Container is a function used by inspect when kube yaml needs to be generated +// for a single container. we "insert" that container description in a pod. +func simplePodWithV1Container(ctr *Container) (*v1.Pod, error) { + var containers []v1.Container + result, err := containerToV1Container(ctr) + if err != nil { + return nil, err + } + containers = append(containers, result) + + tm := v12.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + } + + // Add a label called "app" with the containers name as a value + labels := make(map[string]string) + labels["app"] = removeUnderscores(ctr.Name()) + om := v12.ObjectMeta{ + // The name of the pod is container_name-libpod + Name: fmt.Sprintf("%s-libpod", removeUnderscores(ctr.Name())), + Labels: labels, + // CreationTimestamp seems to be required, so adding it; in doing so, the timestamp + // will reflect time this is run (not container create time) because the conversion + // of the container create time to v1 Time is probably not warranted nor worthwhile. + CreationTimestamp: v12.Now(), + } + ps := v1.PodSpec{ + Containers: containers, + } + p := v1.Pod{ + TypeMeta: tm, + ObjectMeta: om, + Spec: ps, + } + return &p, nil +} + +// containerToV1Container converts information we know about a libpod container +// to a V1.Container specification. +func containerToV1Container(c *Container) (v1.Container, error) { + kubeContainer := v1.Container{} + kubeSec, err := generateKubeSecurityContext(c) + if err != nil { + return kubeContainer, err + } + + if len(c.config.Spec.Linux.Devices) > 0 { + // TODO Enable when we can support devices and their names + devices, err := generateKubeVolumeDeviceFromLinuxDevice(c.Spec().Linux.Devices) + if err != nil { + return kubeContainer, err + } + kubeContainer.VolumeDevices = devices + return kubeContainer, errors.Wrapf(ErrNotImplemented, "linux devices") + } + + if len(c.config.UserVolumes) > 0 { + // TODO When we until we can resolve what the volume name should be, this is disabled + // Volume names need to be coordinated "globally" in the kube files. + volumes, err := libpodMountsToKubeVolumeMounts(c) + if err != nil { + return kubeContainer, err + } + kubeContainer.VolumeMounts = volumes + return kubeContainer, errors.Wrapf(ErrNotImplemented, "volume names") + } + + envVariables, err := libpodEnvVarsToKubeEnvVars(c.config.Spec.Process.Env) + if err != nil { + return kubeContainer, nil + } + + ports, err := ocicniPortMappingToContainerPort(c.PortMappings()) + if err != nil { + return kubeContainer, nil + } + + containerCommands := c.Command() + kubeContainer.Name = removeUnderscores(c.Name()) + + _, image := c.Image() + kubeContainer.Image = image + kubeContainer.Stdin = c.Stdin() + kubeContainer.Command = containerCommands + // TODO need to figure out how we handle command vs entry point. Kube appears to prefer entrypoint. + // right now we just take the container's command + //container.Args = args + kubeContainer.WorkingDir = c.WorkingDir() + kubeContainer.Ports = ports + // This should not be applicable + //container.EnvFromSource = + kubeContainer.Env = envVariables + // TODO enable resources when we can support naming conventions + //container.Resources + kubeContainer.SecurityContext = kubeSec + kubeContainer.StdinOnce = false + kubeContainer.TTY = c.config.Spec.Process.Terminal + + return kubeContainer, nil +} + +// ocicniPortMappingToContainerPort takes an ocicni portmapping and converts +// it to a v1.ContainerPort format for kube output +func ocicniPortMappingToContainerPort(portMappings []ocicni.PortMapping) ([]v1.ContainerPort, error) { + var containerPorts []v1.ContainerPort + for _, p := range portMappings { + var protocol v1.Protocol + switch strings.ToUpper(p.Protocol) { + case "TCP": + protocol = v1.ProtocolTCP + case "UDP": + protocol = v1.ProtocolUDP + default: + return containerPorts, errors.Errorf("unknown network protocol %s", p.Protocol) + } + cp := v1.ContainerPort{ + // Name will not be supported + HostPort: p.HostPort, + HostIP: p.HostIP, + ContainerPort: p.ContainerPort, + Protocol: protocol, + } + containerPorts = append(containerPorts, cp) + } + return containerPorts, nil +} + +// libpodEnvVarsToKubeEnvVars converts a key=value string slice to []v1.EnvVar +func libpodEnvVarsToKubeEnvVars(envs []string) ([]v1.EnvVar, error) { + var envVars []v1.EnvVar + for _, e := range envs { + splitE := strings.SplitN(e, "=", 2) + if len(splitE) != 2 { + return envVars, errors.Errorf("environment variable %s is malformed; should be key=value", e) + } + ev := v1.EnvVar{ + Name: splitE[0], + Value: splitE[1], + } + envVars = append(envVars, ev) + } + return envVars, nil +} + +// Is this worth it? +func libpodMaxAndMinToResourceList(c *Container) (v1.ResourceList, v1.ResourceList) { //nolint + // It does not appear we can properly calculate CPU resources from the information + // we know in libpod. Libpod knows CPUs by time, shares, etc. + + // We also only know about a memory limit; no memory minimum + maxResources := make(map[v1.ResourceName]resource.Quantity) + minResources := make(map[v1.ResourceName]resource.Quantity) + config := c.Config() + maxMem := config.Spec.Linux.Resources.Memory.Limit + + _ = maxMem + + return maxResources, minResources +} + +func generateKubeVolumeMount(hostSourcePath string, mounts []specs.Mount) (v1.VolumeMount, error) { + vm := v1.VolumeMount{} + for _, m := range mounts { + if m.Source == hostSourcePath { + // TODO Name is not provided and is required by Kube; therefore, this is disabled earlier + //vm.Name = + vm.MountPath = m.Source + vm.SubPath = m.Destination + if util.StringInSlice("ro", m.Options) { + vm.ReadOnly = true + } + return vm, nil + } + } + return vm, errors.New("unable to find mount source") +} + +// libpodMountsToKubeVolumeMounts converts the containers mounts to a struct kube understands +func libpodMountsToKubeVolumeMounts(c *Container) ([]v1.VolumeMount, error) { + // At this point, I dont think we can distinguish between the default + // volume mounts and user added ones. For now, we pass them all. + var vms []v1.VolumeMount + for _, hostSourcePath := range c.config.UserVolumes { + vm, err := generateKubeVolumeMount(hostSourcePath, c.config.Spec.Mounts) + if err != nil { + return vms, err + } + vms = append(vms, vm) + } + return vms, nil +} + +// generateKubeSecurityContext generates a securityContext based on the existing container +func generateKubeSecurityContext(c *Container) (*v1.SecurityContext, error) { + priv := c.Privileged() + ro := c.IsReadOnly() + allowPrivEscalation := !c.Spec().Process.NoNewPrivileges + + // TODO enable use of capabilities when we can figure out how to extract cap-add|remove + //caps := v1.Capabilities{ + // //Add: c.config.Spec.Process.Capabilities + //} + sc := v1.SecurityContext{ + // TODO enable use of capabilities when we can figure out how to extract cap-add|remove + //Capabilities: &caps, + Privileged: &priv, + // TODO How do we know if selinux were passed into podman + //SELinuxOptions: + // RunAsNonRoot is an optional parameter; our first implementations should be root only; however + // I'm leaving this as a bread-crumb for later + //RunAsNonRoot: &nonRoot, + ReadOnlyRootFilesystem: &ro, + AllowPrivilegeEscalation: &allowPrivEscalation, + } + + if c.User() != "" { + // It is *possible* that + logrus.Debug("Looking in container for user: %s", c.User()) + u, err := lookup.GetUser(c.state.Mountpoint, c.User()) + if err != nil { + return nil, err + } + user := int64(u.Uid) + sc.RunAsUser = &user + } + return &sc, nil +} + +// generateKubeVolumeDeviceFromLinuxDevice takes a list of devices and makes a VolumeDevice struct for kube +func generateKubeVolumeDeviceFromLinuxDevice(devices []specs.LinuxDevice) ([]v1.VolumeDevice, error) { + var volumeDevices []v1.VolumeDevice + for _, d := range devices { + vd := v1.VolumeDevice{ + // TBD How are we going to sync up these names + //Name: + DevicePath: d.Path, + } + volumeDevices = append(volumeDevices, vd) + } + return volumeDevices, nil +} + +func removeUnderscores(s string) string { + return strings.Replace(s, "_", "", -1) +} -- cgit v1.2.3-54-g00ecf From 690c52a113124efcedccb84e44198e7602f064ec Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 19 Nov 2018 13:20:56 -0600 Subject: Allow users to expose ports from the pod to the host we need to allow users to expose ports to the host for the purposes of networking, like a webserver. the port exposure must be done at the time the pod is created. strictly speaking, the port exposure occurs on the infra container. Signed-off-by: baude --- cmd/podman/pod_create.go | 59 ++++++++++++++++++ completions/bash/podman | 6 +- docs/podman-pod-create.1.md | 9 +++ libpod/options.go | 11 ++++ libpod/pod.go | 4 +- libpod/pod_easyjson.go | 128 ++++++++++++++++++++++++++++++++++++++ libpod/runtime_pod_infra_linux.go | 4 +- pkg/spec/createconfig.go | 1 - test/e2e/pod_create_test.go | 39 ++++++++++++ 9 files changed, 254 insertions(+), 7 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/pod_create.go b/cmd/podman/pod_create.go index 63fa6b294..a3364ac4b 100644 --- a/cmd/podman/pod_create.go +++ b/cmd/podman/pod_create.go @@ -3,11 +3,15 @@ package main import ( "fmt" "os" + "strconv" "strings" "github.com/containers/libpod/cmd/podman/libpodruntime" "github.com/containers/libpod/cmd/podman/shared" "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/rootless" + "github.com/cri-o/ocicni/pkg/ocicni" + "github.com/docker/go-connections/nat" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/urfave/cli" @@ -58,6 +62,10 @@ var podCreateFlags = []cli.Flag{ Name: "pod-id-file", Usage: "Write the pod ID to the file", }, + cli.StringSliceFlag{ + Name: "publish, p", + Usage: "Publish a container's port, or a range of ports, to the host (default [])", + }, cli.StringFlag{ Name: "share", Usage: "A comma delimited list of kernel namespaces the pod will share", @@ -102,6 +110,16 @@ func podCreateCmd(c *cli.Context) error { defer podIdFile.Close() defer podIdFile.Sync() } + + if len(c.StringSlice("publish")) > 0 { + if !c.BoolT("infra") { + return errors.Errorf("you must have an infra container to publish port bindings to the host") + } + if rootless.IsRootless() { + return errors.Errorf("rootless networking does not allow port binding to the host") + } + } + if !c.BoolT("infra") && c.IsSet("share") && c.String("share") != "none" && c.String("share") != "" { return errors.Errorf("You cannot share kernel namespaces on the pod level without an infra container") } @@ -131,6 +149,14 @@ func podCreateCmd(c *cli.Context) error { options = append(options, nsOptions...) } + if len(c.StringSlice("publish")) > 0 { + portBindings, err := CreatePortBindings(c.StringSlice("publish")) + if err != nil { + return err + } + options = append(options, libpod.WithInfraContainerPorts(portBindings)) + + } // always have containers use pod cgroups // User Opt out is not yet supported options = append(options, libpod.WithPodCgroups()) @@ -152,3 +178,36 @@ func podCreateCmd(c *cli.Context) error { return nil } + +// CreatePortBindings iterates ports mappings and exposed ports into a format CNI understands +func CreatePortBindings(ports []string) ([]ocicni.PortMapping, error) { + var portBindings []ocicni.PortMapping + // The conversion from []string to natBindings is temporary while mheon reworks the port + // deduplication code. Eventually that step will not be required. + _, natBindings, err := nat.ParsePortSpecs(ports) + if err != nil { + return nil, err + } + for containerPb, hostPb := range natBindings { + var pm ocicni.PortMapping + pm.ContainerPort = int32(containerPb.Int()) + for _, i := range hostPb { + var hostPort int + var err error + pm.HostIP = i.HostIP + if i.HostPort == "" { + hostPort = containerPb.Int() + } else { + hostPort, err = strconv.Atoi(i.HostPort) + if err != nil { + return nil, errors.Wrapf(err, "unable to convert host port to integer") + } + } + + pm.HostPort = int32(hostPort) + pm.Protocol = containerPb.Proto() + portBindings = append(portBindings, pm) + } + } + return portBindings, nil +} diff --git a/completions/bash/podman b/completions/bash/podman index c029f893a..222511a3c 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -2178,12 +2178,14 @@ _podman_pod_create() { --cgroup-parent --infra-command --infra-image - --share - --podidfile --label-file --label -l --name + --podidfile + --publish + -p + --share " local boolean_options=" diff --git a/docs/podman-pod-create.1.md b/docs/podman-pod-create.1.md index 673ad9a8c..a63b12d73 100644 --- a/docs/podman-pod-create.1.md +++ b/docs/podman-pod-create.1.md @@ -51,6 +51,15 @@ Assign a name to the pod Write the pod ID to the file +**-p**, **--publish**=[] + +Publish a port or range of ports from the pod to the host + +Format: `ip:hostPort:containerPort | ip::containerPort | hostPort:containerPort | containerPort` +Both hostPort and containerPort can be specified as a range of ports. +When specifying ranges for both, the number of container ports in the range must match the number of host ports in the range. +Use `podman port` to see the actual mapping: `podman port CONTAINER $CONTAINERPORT` + **--share**="" A comma deliminated list of kernel namespaces to share. If none or "" is specified, no namespaces will be shared. The namespaces to choose from are ipc, net, pid, user, uts. diff --git a/libpod/options.go b/libpod/options.go index 8d044313b..507847d65 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1295,3 +1295,14 @@ func WithInfraContainer() PodCreateOption { return nil } } + +// WithInfraContainerPorts tells the pod to add port bindings to the pause container +func WithInfraContainerPorts(bindings []ocicni.PortMapping) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return ErrPodFinalized + } + pod.config.InfraContainer.PortBindings = bindings + return nil + } +} diff --git a/libpod/pod.go b/libpod/pod.go index 8ac976f6a..07f41f5c6 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -4,6 +4,7 @@ import ( "time" "github.com/containers/storage" + "github.com/cri-o/ocicni/pkg/ocicni" "github.com/pkg/errors" ) @@ -96,7 +97,8 @@ type PodContainerInfo struct { // InfraContainerConfig is the configuration for the pod's infra container type InfraContainerConfig struct { - HasInfraContainer bool `json:"makeInfraContainer"` + HasInfraContainer bool `json:"makeInfraContainer"` + PortBindings []ocicni.PortMapping `json:"infraPortBindings"` } // ID retrieves the pod's ID diff --git a/libpod/pod_easyjson.go b/libpod/pod_easyjson.go index 6c1c939f3..8ea9a5e72 100644 --- a/libpod/pod_easyjson.go +++ b/libpod/pod_easyjson.go @@ -6,6 +6,7 @@ package libpod import ( json "encoding/json" + ocicni "github.com/cri-o/ocicni/pkg/ocicni" easyjson "github.com/mailru/easyjson" jlexer "github.com/mailru/easyjson/jlexer" jwriter "github.com/mailru/easyjson/jwriter" @@ -721,6 +722,29 @@ func easyjsonBe091417DecodeGithubComContainersLibpodLibpod5(in *jlexer.Lexer, ou switch key { case "makeInfraContainer": out.HasInfraContainer = bool(in.Bool()) + case "infraPortBindings": + if in.IsNull() { + in.Skip() + out.PortBindings = nil + } else { + in.Delim('[') + if out.PortBindings == nil { + if !in.IsDelim(']') { + out.PortBindings = make([]ocicni.PortMapping, 0, 1) + } else { + out.PortBindings = []ocicni.PortMapping{} + } + } else { + out.PortBindings = (out.PortBindings)[:0] + } + for !in.IsDelim(']') { + var v6 ocicni.PortMapping + easyjsonBe091417DecodeGithubComContainersLibpodVendorGithubComCriOOcicniPkgOcicni(in, &v6) + out.PortBindings = append(out.PortBindings, v6) + in.WantComma() + } + in.Delim(']') + } default: in.SkipRecursive() } @@ -745,5 +769,109 @@ func easyjsonBe091417EncodeGithubComContainersLibpodLibpod5(out *jwriter.Writer, } out.Bool(bool(in.HasInfraContainer)) } + { + const prefix string = ",\"infraPortBindings\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + if in.PortBindings == nil && (out.Flags&jwriter.NilSliceAsEmpty) == 0 { + out.RawString("null") + } else { + out.RawByte('[') + for v7, v8 := range in.PortBindings { + if v7 > 0 { + out.RawByte(',') + } + easyjsonBe091417EncodeGithubComContainersLibpodVendorGithubComCriOOcicniPkgOcicni(out, v8) + } + out.RawByte(']') + } + } + out.RawByte('}') +} +func easyjsonBe091417DecodeGithubComContainersLibpodVendorGithubComCriOOcicniPkgOcicni(in *jlexer.Lexer, out *ocicni.PortMapping) { + isTopLevel := in.IsStart() + if in.IsNull() { + if isTopLevel { + in.Consumed() + } + in.Skip() + return + } + in.Delim('{') + for !in.IsDelim('}') { + key := in.UnsafeString() + in.WantColon() + if in.IsNull() { + in.Skip() + in.WantComma() + continue + } + switch key { + case "hostPort": + out.HostPort = int32(in.Int32()) + case "containerPort": + out.ContainerPort = int32(in.Int32()) + case "protocol": + out.Protocol = string(in.String()) + case "hostIP": + out.HostIP = string(in.String()) + default: + in.SkipRecursive() + } + in.WantComma() + } + in.Delim('}') + if isTopLevel { + in.Consumed() + } +} +func easyjsonBe091417EncodeGithubComContainersLibpodVendorGithubComCriOOcicniPkgOcicni(out *jwriter.Writer, in ocicni.PortMapping) { + out.RawByte('{') + first := true + _ = first + { + const prefix string = ",\"hostPort\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + out.Int32(int32(in.HostPort)) + } + { + const prefix string = ",\"containerPort\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + out.Int32(int32(in.ContainerPort)) + } + { + const prefix string = ",\"protocol\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + out.String(string(in.Protocol)) + } + { + const prefix string = ",\"hostIP\":" + if first { + first = false + out.RawString(prefix[1:]) + } else { + out.RawString(prefix) + } + out.String(string(in.HostIP)) + } out.RawByte('}') } diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index fea79e994..450a2fb32 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -7,7 +7,6 @@ import ( "github.com/containers/libpod/libpod/image" "github.com/containers/libpod/pkg/rootless" - "github.com/cri-o/ocicni/pkg/ocicni" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" ) @@ -50,9 +49,8 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, imgID options = append(options, withIsInfra()) // Since user namespace sharing is not implemented, we only need to check if it's rootless - portMappings := make([]ocicni.PortMapping, 0) networks := make([]string, 0) - options = append(options, WithNetNS(portMappings, isRootless, networks)) + options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, isRootless, networks)) return r.newContainer(ctx, g.Config, options...) } diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index 6ac9d82da..6a0642ee7 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -335,7 +335,6 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime) ([]lib } options = append(options, runtime.WithPod(pod)) } - if len(c.PortBindings) > 0 { portBindings, err = c.CreatePortBindings() if err != nil { diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index 51522ffd1..5abf9613b 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -80,4 +80,43 @@ var _ = Describe("Podman pod create", func() { check.WaitWithDefaultTimeout() Expect(len(check.OutputToStringArray())).To(Equal(0)) }) + + It("podman create pod without network portbindings", func() { + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--name", name}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + pod := session.OutputToString() + + webserver := podmanTest.Podman([]string{"run", "--pod", pod, "-dt", nginx}) + webserver.WaitWithDefaultTimeout() + Expect(webserver.ExitCode()).To(Equal(0)) + + check := SystemExec("nc", []string{"-z", "localhost", "80"}) + check.WaitWithDefaultTimeout() + Expect(check.ExitCode()).To(Equal(1)) + }) + + It("podman create pod with network portbindings", func() { + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--name", name, "-p", "80:80"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + pod := session.OutputToString() + + webserver := podmanTest.Podman([]string{"run", "--pod", pod, "-dt", nginx}) + webserver.WaitWithDefaultTimeout() + Expect(webserver.ExitCode()).To(Equal(0)) + + check := SystemExec("nc", []string{"-z", "localhost", "80"}) + check.WaitWithDefaultTimeout() + Expect(check.ExitCode()).To(Equal(0)) + }) + + It("podman create pod with no infra but portbindings should fail", func() { + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--infra=false", "--name", name, "-p", "80:80"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(125)) + }) }) -- cgit v1.2.3-54-g00ecf From fd014029300328c27ddb535d4868858ec47530f8 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 20 Nov 2018 10:00:20 +0100 Subject: exec: always make explicit the tty value otherwise runc will take by default the value used for creating the container. Setting it explicit overrides its default value and we won't end up trying to use a terminal when not available. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1625876 Signed-off-by: Giuseppe Scrivano --- libpod/oci.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index 71da830b5..190a4dc7f 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -741,6 +741,8 @@ func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty if tty { args = append(args, "--tty") + } else { + args = append(args, "--tty=false") } if user != "" { -- cgit v1.2.3-54-g00ecf From ff47a4c2d5485fc49f937f3ce0c4e2fd6bdb1956 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 20 Nov 2018 14:08:08 +0000 Subject: Use a struct to pass options to Checkpoint() For upcoming changes to the Checkpoint() functions this commit switches checkpoint options from a boolean to a struct, so that additional options can be passed easily to Checkpoint() without changing the function parameters all the time. Signed-off-by: Adrian Reber --- cmd/podman/checkpoint.go | 6 ++++-- libpod/container_api.go | 10 ++++++++-- libpod/container_internal_linux.go | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/checkpoint.go b/cmd/podman/checkpoint.go index bf280920d..49e2aec63 100644 --- a/cmd/podman/checkpoint.go +++ b/cmd/podman/checkpoint.go @@ -50,7 +50,9 @@ func checkpointCmd(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 @@ -59,7 +61,7 @@ func checkpointCmd(c *cli.Context) error { containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") for _, ctr := range containers { - if err = ctr.Checkpoint(context.TODO(), keep); err != nil { + if err = ctr.Checkpoint(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 390987394..9f5436b14 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -830,8 +830,14 @@ func (c *Container) Refresh(ctx context.Context) error { return nil } +// ContainerCheckpointOptions is a struct used to pass the parameters +// for checkpointing to corresponding functions +type ContainerCheckpointOptions struct { + Keep bool +} + // Checkpoint checkpoints a container -func (c *Container) Checkpoint(ctx context.Context, keep bool) error { +func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointOptions) error { logrus.Debugf("Trying to checkpoint container %s", c) if !c.batched { c.lock.Lock() @@ -842,7 +848,7 @@ func (c *Container) Checkpoint(ctx context.Context, keep bool) error { } } - return c.checkpoint(ctx, keep) + return c.checkpoint(ctx, options) } // Restore restores a container diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 66c7e8a04..003e8284a 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -431,7 +431,7 @@ func (c *Container) addNamespaceContainer(g *generate.Generator, ns LinuxNS, ctr return nil } -func (c *Container) checkpoint(ctx context.Context, keep bool) (err error) { +func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointOptions) (err error) { if !criu.CheckForCriu() { return errors.Errorf("checkpointing a container requires at least CRIU %d", criu.MinCriuVersion) @@ -464,7 +464,7 @@ func (c *Container) checkpoint(ctx context.Context, keep bool) (err error) { return err } - if !keep { + if !options.Keep { // Remove log file os.Remove(filepath.Join(c.bundlePath(), "dump.log")) // Remove statistic file -- cgit v1.2.3-54-g00ecf From b0572d622974837c2221ed1f01a2ab982f078370 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 20 Nov 2018 15:34:15 +0000 Subject: Added option to keep containers running after checkpointing CRIU supports to leave processes running after checkpointing: -R|--leave-running leave tasks in running state after checkpoint runc also support to leave containers running after checkpointing: --leave-running leave the process running after checkpointing With this commit the support to leave a container running after checkpointing is brought to Podman: --leave-running, -R leave the container running after writing checkpoint to disk Now it is possible to checkpoint a container at some point in time without stopping the container. This can be used to rollback the container to an early state: $ podman run --tmpfs /tmp --name podman-criu-test -d docker://docker.io/yovfiatbeb/podman-criu-test $ curl 10.88.64.253:8080/examples/servlets/servlet/HelloWorldExample 3 $ podman container checkpoint -R -l $ curl 10.88.64.253:8080/examples/servlets/servlet/HelloWorldExample 4 $ curl 10.88.64.253:8080/examples/servlets/servlet/HelloWorldExample 5 $ podman stop -l $ podman container restore -l $ curl 10.88.64.253:8080/examples/servlets/servlet/HelloWorldExample 4 So after checkpointing the container kept running and was stopped after some time. Restoring this container will restore the state right at the checkpoint. Signed-off-by: Adrian Reber --- cmd/podman/checkpoint.go | 7 ++++++- libpod/container_api.go | 3 ++- libpod/container_internal_linux.go | 12 +++++++----- libpod/oci.go | 15 ++++++++++++--- 4 files changed, 27 insertions(+), 10 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/checkpoint.go b/cmd/podman/checkpoint.go index 49e2aec63..ddfd12bc3 100644 --- a/cmd/podman/checkpoint.go +++ b/cmd/podman/checkpoint.go @@ -23,6 +23,10 @@ var ( Name: "keep, k", Usage: "keep all temporary checkpoint files", }, + cli.BoolFlag{ + Name: "leave-running, R", + Usage: "leave the container running after writing checkpoint to disk", + }, cli.BoolFlag{ Name: "all, a", Usage: "checkpoint all running containers", @@ -51,7 +55,8 @@ func checkpointCmd(c *cli.Context) error { defer runtime.Shutdown(false) options := libpod.ContainerCheckpointOptions{ - Keep: c.Bool("keep"), + Keep: c.Bool("keep"), + KeepRunning: c.Bool("leave-running"), } if err := checkAllAndLatest(c); err != nil { diff --git a/libpod/container_api.go b/libpod/container_api.go index 9f5436b14..df6b6e962 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -833,7 +833,8 @@ func (c *Container) Refresh(ctx context.Context) error { // ContainerCheckpointOptions is a struct used to pass the parameters // for checkpointing to corresponding functions type ContainerCheckpointOptions struct { - Keep bool + Keep bool + KeepRunning bool } // Checkpoint checkpoints a container diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 003e8284a..e6071945d 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -440,7 +440,7 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO if c.state.State != ContainerStateRunning { return errors.Wrapf(ErrCtrStateInvalid, "%q is not running, cannot checkpoint", c.state.State) } - if err := c.runtime.ociRuntime.checkpointContainer(c); err != nil { + if err := c.runtime.ociRuntime.checkpointContainer(c, options); err != nil { return err } @@ -457,11 +457,13 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO logrus.Debugf("Checkpointed container %s", c.ID()) - c.state.State = ContainerStateStopped + if !options.KeepRunning { + c.state.State = ContainerStateStopped - // Cleanup Storage and Network - if err := c.cleanup(ctx); err != nil { - return err + // Cleanup Storage and Network + if err := c.cleanup(ctx); err != nil { + return err + } } if !options.Keep { diff --git a/libpod/oci.go b/libpod/oci.go index 71da830b5..8ee2c948f 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -844,13 +844,22 @@ func (r *OCIRuntime) execStopContainer(ctr *Container, timeout uint) error { } // checkpointContainer checkpoints the given container -func (r *OCIRuntime) checkpointContainer(ctr *Container) error { +func (r *OCIRuntime) checkpointContainer(ctr *Container, options ContainerCheckpointOptions) error { // imagePath is used by CRIU to store the actual checkpoint files imagePath := ctr.CheckpointPath() // workPath will be used to store dump.log and stats-dump workPath := ctr.bundlePath() logrus.Debugf("Writing checkpoint to %s", imagePath) logrus.Debugf("Writing checkpoint logs to %s", workPath) - return utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, nil, r.path, "checkpoint", - "--image-path", imagePath, "--work-path", workPath, ctr.ID()) + args := []string{} + args = append(args, "checkpoint") + args = append(args, "--image-path") + args = append(args, imagePath) + args = append(args, "--work-path") + args = append(args, workPath) + if options.KeepRunning { + args = append(args, "--leave-running") + } + args = append(args, ctr.ID()) + return utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, nil, r.path, args...) } -- cgit v1.2.3-54-g00ecf From 0e2042ebd72c0053513ea4979926e071e1eefddc Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 21 Nov 2018 15:56:31 +0100 Subject: set root propagation based on volume properties Set the root propagation based on the properties of volumes and default mounts. To remain compatibility, follow the semantics of Docker. If a volume is shared, keep the root propagation shared which works for slave and private volumes too. For slave volumes, it can either be shared or rshared. Do not change the root propagation for private volumes and stick with the default. Fixes: #1834 Signed-off-by: Valentin Rothberg --- libpod/container_internal_linux.go | 26 ++++++++++++++++++++++++++ libpod/mounts_linux.go | 18 ++++++++++++++++++ test/e2e/run_test.go | 5 ++++- 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 libpod/mounts_linux.go (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index e6071945d..93bd23b55 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -347,8 +347,34 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { // Mounts need to be sorted so paths will not cover other paths mounts := sortMounts(g.Mounts()) g.ClearMounts() + + // Determine property of RootPropagation based on volume properties. If + // a volume is shared, then keep root propagation shared. This should + // work for slave and private volumes too. + // + // For slave volumes, it can be either [r]shared/[r]slave. + // + // For private volumes any root propagation value should work. + rootPropagation := "" for _, m := range mounts { g.AddMount(m) + for _, opt := range m.Options { + switch opt { + case MountShared, MountRShared: + if rootPropagation != MountShared && rootPropagation != MountRShared { + rootPropagation = MountShared + } + case MountSlave, MountRSlave: + if rootPropagation != MountShared && rootPropagation != MountRShared && rootPropagation != MountSlave && rootPropagation != MountRSlave { + rootPropagation = MountRSlave + } + } + } + } + + if rootPropagation != "" { + logrus.Debugf("set root propagation to %q", rootPropagation) + g.SetLinuxRootPropagation(rootPropagation) } return g.Config, nil } diff --git a/libpod/mounts_linux.go b/libpod/mounts_linux.go new file mode 100644 index 000000000..e6aa09eac --- /dev/null +++ b/libpod/mounts_linux.go @@ -0,0 +1,18 @@ +// +build linux + +package libpod + +const ( + // MountPrivate represents the private mount option. + MountPrivate = "private" + // MountRPrivate represents the rprivate mount option. + MountRPrivate = "rprivate" + // MountShared represents the shared mount option. + MountShared = "shared" + // MountRShared represents the rshared mount option. + MountRShared = "rshared" + // MountSlave represents the slave mount option. + MountSlave = "slave" + // MountRSlave represents the rslave mount option. + MountRSlave = "rslave" +) diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index beb408fd4..ff166f466 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -609,7 +609,10 @@ USER mail` session := podmanTest.Podman([]string{"run", "--volume", vol1 + ":/myvol1:z", "--volume", vol2 + ":/myvol2:shared,z", fedoraMinimal, "findmnt", "-o", "TARGET,PROPAGATION"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - match, _ := session.GrepString("shared") + match, shared := session.GrepString("shared") Expect(match).Should(BeTrue()) + // make sure it's only shared (and not 'shared,slave') + isSharedOnly := !strings.Contains(shared[0], "shared,") + Expect(isSharedOnly).Should(BeTrue()) }) }) -- cgit v1.2.3-54-g00ecf From 9d883d2032b112d5c65040629313cfba0de6c479 Mon Sep 17 00:00:00 2001 From: baude Date: Sun, 25 Nov 2018 16:26:39 -0600 Subject: add podman container|image exists Add an exists subcommand to podman container and podman image that allows users to verify the existence of a container or image by ID or name. The return code can be 0 (success), 1 (failed to find), or 125 (failed to work with runtime). Issue #1845 Signed-off-by: baude --- cmd/podman/container.go | 1 + cmd/podman/exists.go | 83 ++++++++++++++++++++++++++++++++++++++ cmd/podman/image.go | 1 + completions/bash/podman | 16 ++++++++ docs/podman-container-exists.1.md | 40 ++++++++++++++++++ docs/podman-container.1.md | 1 + docs/podman-image-exists.1.md | 40 ++++++++++++++++++ docs/podman-image.1.md | 1 + libpod/image/errors.go | 15 +++++++ libpod/image/image.go | 4 +- test/e2e/exists_test.go | 85 +++++++++++++++++++++++++++++++++++++++ 11 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 cmd/podman/exists.go create mode 100644 docs/podman-container-exists.1.md create mode 100644 docs/podman-image-exists.1.md create mode 100644 libpod/image/errors.go create mode 100644 test/e2e/exists_test.go (limited to 'libpod') diff --git a/cmd/podman/container.go b/cmd/podman/container.go index ff634278f..b6262f890 100644 --- a/cmd/podman/container.go +++ b/cmd/podman/container.go @@ -9,6 +9,7 @@ var ( attachCommand, checkpointCommand, cleanupCommand, + containerExistsCommand, commitCommand, createCommand, diffCommand, diff --git a/cmd/podman/exists.go b/cmd/podman/exists.go new file mode 100644 index 000000000..2f7b7c185 --- /dev/null +++ b/cmd/podman/exists.go @@ -0,0 +1,83 @@ +package main + +import ( + "os" + + "github.com/containers/libpod/cmd/podman/libpodruntime" + "github.com/containers/libpod/libpod" + "github.com/containers/libpod/libpod/image" + "github.com/pkg/errors" + "github.com/urfave/cli" +) + +var ( + imageExistsDescription = ` + podman image exists + + Check if an image exists in local storage +` + + imageExistsCommand = cli.Command{ + Name: "exists", + Usage: "Check if an image exists in local storage", + Description: imageExistsDescription, + Action: imageExistsCmd, + ArgsUsage: "IMAGE-NAME", + OnUsageError: usageErrorHandler, + } +) + +var ( + containerExistsDescription = ` + podman container exists + + Check if a container exists in local storage +` + + containerExistsCommand = cli.Command{ + Name: "exists", + Usage: "Check if a container exists in local storage", + Description: containerExistsDescription, + Action: containerExistsCmd, + ArgsUsage: "CONTAINER-NAME", + OnUsageError: usageErrorHandler, + } +) + +func imageExistsCmd(c *cli.Context) error { + args := c.Args() + if len(args) > 1 || len(args) < 1 { + return errors.New("you may only check for the existence of one image at a time") + } + runtime, err := libpodruntime.GetRuntime(c) + if err != nil { + return errors.Wrapf(err, "could not get runtime") + } + defer runtime.Shutdown(false) + if _, err := runtime.ImageRuntime().NewFromLocal(args[0]); err != nil { + if errors.Cause(err) == image.ErrNoSuchImage { + os.Exit(1) + } + return err + } + return nil +} + +func containerExistsCmd(c *cli.Context) error { + args := c.Args() + if len(args) > 1 || len(args) < 1 { + return errors.New("you may only check for the existence of one container at a time") + } + runtime, err := libpodruntime.GetRuntime(c) + if err != nil { + return errors.Wrapf(err, "could not get runtime") + } + defer runtime.Shutdown(false) + if _, err := runtime.LookupContainer(args[0]); err != nil { + if errors.Cause(err) == libpod.ErrNoSuchCtr { + os.Exit(1) + } + return err + } + return nil +} diff --git a/cmd/podman/image.go b/cmd/podman/image.go index e67f61799..418b442e3 100644 --- a/cmd/podman/image.go +++ b/cmd/podman/image.go @@ -9,6 +9,7 @@ var ( buildCommand, historyCommand, importCommand, + imageExistsCommand, inspectCommand, loadCommand, lsImagesCommand, diff --git a/completions/bash/podman b/completions/bash/podman index 8eaa1e6a9..3c6b6ec50 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -2178,6 +2178,22 @@ _podman_container_runlabel() { esac } +_podman_container_exists() { + local options_with_args=" + " + + local boolean_options=" + " +} + +_podman_image_exists() { + local options_with_args=" + " + + local boolean_options=" + " +} + _podman_pod_create() { local options_with_args=" --cgroup-parent diff --git a/docs/podman-container-exists.1.md b/docs/podman-container-exists.1.md new file mode 100644 index 000000000..76701e2c2 --- /dev/null +++ b/docs/podman-container-exists.1.md @@ -0,0 +1,40 @@ +% PODMAN(1) Podman Man Pages +% Brent Baude +% November 2018 +# NAME +podman-container-exists- Check if a container exists in local storage + +# SYNOPSIS +**podman container exists** +[**-h**|**--help**] +CONTAINER + +# DESCRIPTION +**podman container exists** checks if a container exists in local storage. The **ID** or **Name** +of the container may be used as input. Podman will return an exit code +of `0` when the container is found. A `1` will be returned otherwise. An exit code of `125` indicates there +was an issue accessing the local storage. + +## Examples ## + +Check if an container called `webclient` exists in local storage (the container does actually exist). +``` +$ sudo podman container exists webclient +$ echo $? +0 +$ +``` + +Check if an container called `webbackend` exists in local storage (the container does not actually exist). +``` +$ sudo podman container exists webbackend +$ echo $? +1 +$ +``` + +## SEE ALSO +podman(1) + +# HISTORY +November 2018, Originally compiled by Brent Baude (bbaude at redhat dot com) diff --git a/docs/podman-container.1.md b/docs/podman-container.1.md index 67d42bfef..aa5dfa82c 100644 --- a/docs/podman-container.1.md +++ b/docs/podman-container.1.md @@ -20,6 +20,7 @@ The container command allows you to manage containers | create | [podman-create(1)](podman-create.1.md) | Create a new container. | | diff | [podman-diff(1)](podman-diff.1.md) | Inspect changes on a container or image's filesystem. | | exec | [podman-exec(1)](podman-exec.1.md) | Execute a command in a running container. | +| exists | [podman-exists(1)](podman-container-exists.1.md) | Check if a container exists in local storage | | export | [podman-export(1)](podman-export.1.md) | Export a container's filesystem contents as a tar archive. | | inspect | [podman-inspect(1)](podman-inspect.1.md) | Display a container or image's configuration. | | kill | [podman-kill(1)](podman-kill.1.md) | Kill the main process in one or more containers. | diff --git a/docs/podman-image-exists.1.md b/docs/podman-image-exists.1.md new file mode 100644 index 000000000..e04c23721 --- /dev/null +++ b/docs/podman-image-exists.1.md @@ -0,0 +1,40 @@ +% PODMAN(1) Podman Man Pages +% Brent Baude +% November 2018 +# NAME +podman-image-exists- Check if an image exists in local storage + +# SYNOPSIS +**podman image exists** +[**-h**|**--help**] +IMAGE + +# DESCRIPTION +**podman image exists** checks if an image exists in local storage. The **ID** or **Name** +of the image may be used as input. Podman will return an exit code +of `0` when the image is found. A `1` will be returned otherwise. An exit code of `125` indicates there +was an issue accessing the local storage. + +## Examples ## + +Check if an image called `webclient` exists in local storage (the image does actually exist). +``` +$ sudo podman image exists webclient +$ echo $? +0 +$ +``` + +Check if an image called `webbackend` exists in local storage (the image does not actually exist). +``` +$ sudo podman image exists webbackend +$ echo $? +1 +$ +``` + +## SEE ALSO +podman(1) + +# HISTORY +November 2018, Originally compiled by Brent Baude (bbaude at redhat dot com) diff --git a/docs/podman-image.1.md b/docs/podman-image.1.md index 33de0456f..446f8667d 100644 --- a/docs/podman-image.1.md +++ b/docs/podman-image.1.md @@ -14,6 +14,7 @@ The image command allows you to manage images | Command | Man Page | Description | | -------- | ----------------------------------------- | ------------------------------------------------------------------------------ | | build | [podman-build(1)](podman-build.1.md) | Build a container using a Dockerfile. | +| exists | [podman-exists(1)](podman-image-exists.1.md) | Check if a image exists in local storage | | history | [podman-history(1)](podman-history.1.md) | Show the history of an image. | | import | [podman-import(1)](podman-import.1.md) | Import a tarball and save it as a filesystem image. | | inspect | [podman-inspect(1)](podman-inspect.1.md) | Display a image or image's configuration. | diff --git a/libpod/image/errors.go b/libpod/image/errors.go new file mode 100644 index 000000000..4088946cb --- /dev/null +++ b/libpod/image/errors.go @@ -0,0 +1,15 @@ +package image + +import ( + "errors" +) + +// Copied directly from libpod errors to avoid circular imports +var ( + // ErrNoSuchCtr indicates the requested container does not exist + ErrNoSuchCtr = errors.New("no such container") + // ErrNoSuchPod indicates the requested pod does not exist + ErrNoSuchPod = errors.New("no such pod") + // ErrNoSuchImage indicates the requested image does not exist + ErrNoSuchImage = errors.New("no such image") +) diff --git a/libpod/image/image.go b/libpod/image/image.go index 7e520d97e..a05c15160 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -252,7 +252,7 @@ func (i *Image) getLocalImage() (*storage.Image, error) { // The image has a registry name in it and we made sure we looked for it locally // with a tag. It cannot be local. if decomposedImage.hasRegistry { - return nil, errors.Errorf("%s", imageError) + return nil, errors.Wrapf(ErrNoSuchImage, imageError) } @@ -275,7 +275,7 @@ func (i *Image) getLocalImage() (*storage.Image, error) { return repoImage, nil } - return nil, errors.Wrapf(err, imageError) + return nil, errors.Wrapf(ErrNoSuchImage, err.Error()) } // ID returns the image ID as a string diff --git a/test/e2e/exists_test.go b/test/e2e/exists_test.go new file mode 100644 index 000000000..9165e8902 --- /dev/null +++ b/test/e2e/exists_test.go @@ -0,0 +1,85 @@ +package integration + +import ( + "fmt" + "os" + + . "github.com/containers/libpod/test/utils" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Podman image|container exists", func() { + var ( + tempdir string + err error + podmanTest *PodmanTestIntegration + ) + + BeforeEach(func() { + tempdir, err = CreateTempDirInTempDir() + if err != nil { + os.Exit(1) + } + podmanTest = PodmanTestCreate(tempdir) + podmanTest.RestoreAllArtifacts() + }) + + AfterEach(func() { + podmanTest.Cleanup() + f := CurrentGinkgoTestDescription() + timedResult := fmt.Sprintf("Test: %s completed in %f seconds", f.TestText, f.Duration.Seconds()) + GinkgoWriter.Write([]byte(timedResult)) + + }) + It("podman image exists in local storage by fq name", func() { + session := podmanTest.Podman([]string{"image", "exists", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) + It("podman image exists in local storage by short name", func() { + session := podmanTest.Podman([]string{"image", "exists", "alpine"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) + It("podman image does not exist in local storage", func() { + session := podmanTest.Podman([]string{"image", "exists", "alpine9999"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(1)) + }) + It("podman container exists in local storage by name", func() { + setup := podmanTest.RunTopContainer("foobar") + setup.WaitWithDefaultTimeout() + Expect(setup.ExitCode()).To(Equal(0)) + + session := podmanTest.Podman([]string{"container", "exists", "foobar"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) + It("podman container exists in local storage by container ID", func() { + setup := podmanTest.RunTopContainer("") + setup.WaitWithDefaultTimeout() + Expect(setup.ExitCode()).To(Equal(0)) + cid := setup.OutputToString() + + session := podmanTest.Podman([]string{"container", "exists", cid}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) + It("podman container exists in local storage by short container ID", func() { + setup := podmanTest.RunTopContainer("") + setup.WaitWithDefaultTimeout() + Expect(setup.ExitCode()).To(Equal(0)) + cid := setup.OutputToString()[0:12] + + session := podmanTest.Podman([]string{"container", "exists", cid}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) + It("podman container does not exist in local storage", func() { + session := podmanTest.Podman([]string{"container", "exists", "foobar"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(1)) + }) + +}) -- cgit v1.2.3-54-g00ecf From 5438ec19181cc5855e6b54e152e0fe3161a75913 Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Thu, 22 Nov 2018 21:49:30 -0500 Subject: Add history and namespaceoptions to image inspect Signed-off-by: Qi Wang --- libpod/image/image.go | 1 + pkg/inspect/inspect.go | 1 + 2 files changed, 2 insertions(+) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 7e520d97e..2d03786a9 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -869,6 +869,7 @@ func (i *Image) Inspect(ctx context.Context) (*inspect.ImageData, error) { GraphDriver: driver, ManifestType: manifestType, User: ociv1Img.Config.User, + History: ociv1Img.History, } return data, nil } diff --git a/pkg/inspect/inspect.go b/pkg/inspect/inspect.go index 62ba53147..5bdcf677f 100644 --- a/pkg/inspect/inspect.go +++ b/pkg/inspect/inspect.go @@ -126,6 +126,7 @@ type ImageData struct { Annotations map[string]string `json:"Annotations"` ManifestType string `json:"ManifestType"` User string `json:"User"` + History []v1.History `json:"History"` } // RootFS holds the root fs information of an image -- cgit v1.2.3-54-g00ecf From 070ce0c85570b79a3612ad1fdf15b5c1287b00b6 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 20 Nov 2018 11:06:20 +0100 Subject: exec: don't wait for pidfile when the runtime exited don't wait for the timeout to expire if the runtime process exited. I've noticed podman to hang on exit and keeping the container lock taken when the OCI runtime already exited. Additionally, it reduces the waiting time as we won't hit the 25 milliseconds waiting time in the worst case. Signed-off-by: Giuseppe Scrivano --- libpod/container_api.go | 24 +++++++++++++----------- libpod/util.go | 8 +++++--- 2 files changed, 18 insertions(+), 14 deletions(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index df6b6e962..e1d5e15c4 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -328,6 +328,11 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e if err != nil { return errors.Wrapf(err, "error exec %s", c.ID()) } + chWait := make(chan error) + go func() { + chWait <- execCmd.Wait() + }() + defer close(chWait) pidFile := c.execPidPath(sessionID) // 60 second seems a reasonable time to wait @@ -336,18 +341,12 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e const pidWaitTimeout = 60000 // Wait until the runtime makes the pidfile - // TODO: If runtime errors before the PID file is created, we have to - // wait for timeout here - if err := WaitForFile(pidFile, pidWaitTimeout*time.Millisecond); err != nil { - logrus.Debugf("Timed out waiting for pidfile from runtime for container %s exec", c.ID()) - - // Check if an error occurred in the process before we made a pidfile - // TODO: Wait() here is a poor choice - is there a way to see if - // a process has finished, instead of waiting for it to finish? - if err := execCmd.Wait(); err != nil { + exited, err := WaitForFile(pidFile, chWait, pidWaitTimeout*time.Millisecond) + if err != nil { + if exited { + // If the runtime exited, propagate the error we got from the process. return err } - return errors.Wrapf(err, "timed out waiting for runtime to create pidfile for exec session in container %s", c.ID()) } @@ -389,7 +388,10 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e locked = false } - waitErr := execCmd.Wait() + var waitErr error + if !exited { + waitErr = <-chWait + } // Lock again if !c.batched { diff --git a/libpod/util.go b/libpod/util.go index 7007b29cd..25e78bd01 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -90,7 +90,7 @@ func MountExists(specMounts []spec.Mount, dest string) bool { } // WaitForFile waits until a file has been created or the given timeout has occurred -func WaitForFile(path string, timeout time.Duration) error { +func WaitForFile(path string, chWait chan error, timeout time.Duration) (bool, error) { done := make(chan struct{}) chControl := make(chan struct{}) go func() { @@ -110,11 +110,13 @@ func WaitForFile(path string, timeout time.Duration) error { }() select { + case e := <-chWait: + return true, e case <-done: - return nil + return false, nil case <-time.After(timeout): close(chControl) - return errors.Wrapf(ErrInternal, "timed out waiting for file %s", path) + return false, errors.Wrapf(ErrInternal, "timed out waiting for file %s", path) } } -- cgit v1.2.3-54-g00ecf From 870eed9378c025f3684aa8baf3db6de969da3c5d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 21 Nov 2018 09:30:03 -0500 Subject: Use host's resolv.conf if no network namespace enabled My host system runs Fedora Silverblue 29 and I have NetworkManager's `dns=dnsmasq` setting enabled, so my `/etc/resolv.conf` only has `127.0.0.1`. I also run my development podman containers with `--net=host` for various reasons. If we have a host network namespace, there's no reason not to just use the host's nameserver configuration either. This fixes e.g. accessing content on a VPN, and is also faster since the container is using cached DNS. I know this doesn't solve the bigger picture issue of localhost-DNS conflicting with bridged networking, but that's far more involved, probably requiring a DNS proxy in the container. This patch makes my workflow a lot nicer and was easy to write. Signed-off-by: Colin Walters --- libpod/container_internal_linux.go | 5 +++-- pkg/resolvconf/resolvconf.go | 12 ++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index ffb82cc94..677645e7d 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -729,9 +729,10 @@ func (c *Container) generateResolvConf() (string, error) { return "", errors.Wrapf(err, "unable to read %s", resolvPath) } - // Process the file to remove localhost nameservers + // Ensure that the container's /etc/resolv.conf is compatible with its + // network configuration. // TODO: set ipv6 enable bool more sanely - resolv, err := resolvconf.FilterResolvDNS(contents, true) + resolv, err := resolvconf.FilterResolvDNS(contents, true, c.config.CreateNetNS) if err != nil { return "", errors.Wrapf(err, "error parsing host resolv.conf") } diff --git a/pkg/resolvconf/resolvconf.go b/pkg/resolvconf/resolvconf.go index fccd60093..e85bcb377 100644 --- a/pkg/resolvconf/resolvconf.go +++ b/pkg/resolvconf/resolvconf.go @@ -103,13 +103,21 @@ func GetLastModified() *File { } // FilterResolvDNS cleans up the config in resolvConf. It has two main jobs: -// 1. It looks for localhost (127.*|::1) entries in the provided +// 1. If a netns is enabled, it looks for localhost (127.*|::1) entries in the provided // resolv.conf, removing local nameserver entries, and, if the resulting // cleaned config has no defined nameservers left, adds default DNS entries // 2. Given the caller provides the enable/disable state of IPv6, the filter // code will remove all IPv6 nameservers if it is not enabled for containers // -func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) (*File, error) { +func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool, netnsEnabled bool) (*File, error) { + // If we're using the host netns, we have nothing to do besides hash the file. + if !netnsEnabled { + hash, err := ioutils.HashData(bytes.NewReader(resolvConf)) + if err != nil { + return nil, err + } + return &File{Content: resolvConf, Hash: hash}, nil + } cleanedResolvConf := localhostNSRegexp.ReplaceAll(resolvConf, []byte{}) // if IPv6 is not enabled, also clean out any IPv6 address nameserver if !ipv6Enabled { -- 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') 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 03c88a3debf77780bdad2382d4c01ccedc6d27a5 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Wed, 21 Nov 2018 14:40:43 +0000 Subject: Added tcp-established to checkpoint/restore CRIU can checkpoint and restore processes/containers with established TCP connections if the correct option is specified. To implement checkpoint and restore with support for established TCP connections with Podman this commit adds the necessary options to runc during checkpoint and also tells conmon during restore to use 'runc restore' with '--tcp-established'. For this Podman feature to work a corresponding conmon change is required. Example: $ podman run --tmpfs /tmp --name podman-criu-test -d docker://docker.io/yovfiatbeb/podman-criu-test $ nc `podman inspect -l | jq -r '.[0].NetworkSettings.IPAddress'` 8080 GET /examples/servlets/servlet/HelloWorldExample Connection: keep-alive 1 GET /examples/servlets/servlet/HelloWorldExample Connection: keep-alive 2 $ # Using HTTP keep-alive multiple requests are send to the server in the container $ # Different terminal: $ podman container checkpoint -l criu failed: type NOTIFY errno 0 $ # Looking at the log file would show errors because of established TCP connections $ podman container checkpoint -l --tcp-established $ # This works now and after the restore the same connection as above can be used for requests $ podman container restore -l --tcp-established The restore would fail without '--tcp-established' as the checkpoint image contains established TCP connections. Signed-off-by: Adrian Reber --- cmd/podman/checkpoint.go | 9 +++++++-- cmd/podman/restore.go | 7 ++++++- libpod/container_api.go | 8 +++++++- libpod/oci.go | 6 ++++++ 4 files changed, 26 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/checkpoint.go b/cmd/podman/checkpoint.go index ddfd12bc3..824c97662 100644 --- a/cmd/podman/checkpoint.go +++ b/cmd/podman/checkpoint.go @@ -27,6 +27,10 @@ var ( Name: "leave-running, R", Usage: "leave the container running after writing checkpoint to disk", }, + cli.BoolFlag{ + Name: "tcp-established", + Usage: "checkpoint a container with established TCP connections", + }, cli.BoolFlag{ Name: "all, a", Usage: "checkpoint all running containers", @@ -55,8 +59,9 @@ func checkpointCmd(c *cli.Context) error { defer runtime.Shutdown(false) options := libpod.ContainerCheckpointOptions{ - Keep: c.Bool("keep"), - KeepRunning: c.Bool("leave-running"), + Keep: c.Bool("keep"), + KeepRunning: c.Bool("leave-running"), + TCPEstablished: c.Bool("tcp-established"), } if err := checkAllAndLatest(c); err != nil { diff --git a/cmd/podman/restore.go b/cmd/podman/restore.go index 6383ebf0b..afdbc36e0 100644 --- a/cmd/podman/restore.go +++ b/cmd/podman/restore.go @@ -26,6 +26,10 @@ var ( // restore --all would make more sense if there would be // dedicated state for container which are checkpointed. // TODO: add ContainerStateCheckpointed + cli.BoolFlag{ + Name: "tcp-established", + Usage: "checkpoint a container with established TCP connections", + }, cli.BoolFlag{ Name: "all, a", Usage: "restore all checkpointed containers", @@ -54,7 +58,8 @@ func restoreCmd(c *cli.Context) error { defer runtime.Shutdown(false) options := libpod.ContainerCheckpointOptions{ - Keep: c.Bool("keep"), + Keep: c.Bool("keep"), + TCPEstablished: c.Bool("tcp-established"), } if err := checkAllAndLatest(c); err != nil { diff --git a/libpod/container_api.go b/libpod/container_api.go index 396f06c20..aee87cc04 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -833,8 +833,14 @@ func (c *Container) Refresh(ctx context.Context) error { // ContainerCheckpointOptions is a struct used to pass the parameters // for checkpointing (and restoring) to the corresponding functions type ContainerCheckpointOptions struct { - Keep bool + // Keep tells the API to not delete checkpoint artifacts + Keep bool + // KeepRunning tells the API to keep the container running + // after writing the checkpoint to disk KeepRunning bool + // TCPEstablished tells the API to checkpoint a container + // even if it contains established TCP connections + TCPEstablished bool } // Checkpoint checkpoints a container diff --git a/libpod/oci.go b/libpod/oci.go index 6aedc5662..5a041b7d6 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -291,6 +291,9 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res if restoreOptions != nil { args = append(args, "--restore", ctr.CheckpointPath()) + if restoreOptions.TCPEstablished { + args = append(args, "--restore-arg", "--tcp-established") + } } logrus.WithFields(logrus.Fields{ @@ -862,6 +865,9 @@ func (r *OCIRuntime) checkpointContainer(ctr *Container, options ContainerCheckp if options.KeepRunning { args = append(args, "--leave-running") } + if options.TCPEstablished { + args = append(args, "--tcp-established") + } args = append(args, ctr.ID()) return utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, nil, r.path, args...) } -- 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') 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 0365f573710dfc8ee7f9e13082a238deea675dec Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Fri, 23 Nov 2018 23:39:25 +0100 Subject: rootless: fix cleanup The conmon exit command is running inside of a namespace where the process is running with uid=0. When it launches again podman for the cleanup, podman is not running in rootless mode as the uid=0. Export some more env variables to tell podman we are in rootless mode. Closes: https://github.com/containers/libpod/issues/1859 Signed-off-by: Giuseppe Scrivano --- libpod/oci.go | 4 ++++ test/e2e/rootless_test.go | 4 ++++ 2 files changed, 8 insertions(+) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index a7aec06e5..ee1677b67 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -316,6 +316,10 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res cmd.Env = append(r.conmonEnv, fmt.Sprintf("_OCI_SYNCPIPE=%d", 3)) cmd.Env = append(cmd.Env, fmt.Sprintf("_OCI_STARTPIPE=%d", 4)) cmd.Env = append(cmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)) + cmd.Env = append(cmd.Env, fmt.Sprintf("_LIBPOD_USERNS_CONFIGURED=%s", os.Getenv("_LIBPOD_USERNS_CONFIGURED"))) + cmd.Env = append(cmd.Env, fmt.Sprintf("_LIBPOD_ROOTLESS_UID=%s", os.Getenv("_LIBPOD_ROOTLESS_UID"))) + cmd.Env = append(cmd.Env, fmt.Sprintf("HOME=%s", os.Getenv("HOME"))) + cmd.Env = append(cmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)) if r.reservePorts { ports, err := bindPorts(ctr.config.PortMappings) diff --git a/test/e2e/rootless_test.go b/test/e2e/rootless_test.go index 995744ae5..676459416 100644 --- a/test/e2e/rootless_test.go +++ b/test/e2e/rootless_test.go @@ -205,6 +205,10 @@ var _ = Describe("Podman rootless", func() { cmd.WaitWithDefaultTimeout() Expect(cmd.ExitCode()).To(Equal(0)) + cmd = rootlessTest.PodmanAsUser([]string{"inspect", "-l", "--type", "container", "--format", "{{ .State.Status }}"}, 1000, 1000, env) + cmd.WaitWithDefaultTimeout() + Expect(cmd.LineInOutputContains("exited")).To(BeTrue()) + cmd = rootlessTest.PodmanAsUser([]string{"start", "-l"}, 1000, 1000, env) cmd.WaitWithDefaultTimeout() Expect(cmd.ExitCode()).To(Equal(0)) -- cgit v1.2.3-54-g00ecf From fc3047322a527347072ce98ba183cbc8cb49231d Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 20 Nov 2018 11:19:40 +0100 Subject: util: use fsnotify to wait for file prefer a fsnotify watcher to polling the file, we take advantage of inotify on Linux and react more promptly to the PID file being created. If the watcher cannot be created, then fallback to the old polling mechanism. Signed-off-by: Giuseppe Scrivano --- libpod/util.go | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/util.go b/libpod/util.go index 25e78bd01..aa3494529 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -13,6 +13,7 @@ import ( "github.com/containers/image/signature" "github.com/containers/image/types" "github.com/containers/libpod/pkg/util" + "github.com/fsnotify/fsnotify" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" ) @@ -93,18 +94,49 @@ func MountExists(specMounts []spec.Mount, dest string) bool { func WaitForFile(path string, chWait chan error, timeout time.Duration) (bool, error) { done := make(chan struct{}) chControl := make(chan struct{}) + + var inotifyEvents chan fsnotify.Event + var timer chan struct{} + watcher, err := fsnotify.NewWatcher() + if err == nil { + if err := watcher.Add(filepath.Dir(path)); err == nil { + inotifyEvents = watcher.Events + } + defer watcher.Close() + } + if inotifyEvents == nil { + // If for any reason we fail to create the inotify + // watcher, fallback to polling the file + timer = make(chan struct{}) + go func() { + select { + case <-chControl: + close(timer) + return + default: + time.Sleep(25 * time.Millisecond) + timer <- struct{}{} + } + }() + } + go func() { for { select { case <-chControl: return - default: + case <-timer: + _, err := os.Stat(path) + if err == nil { + close(done) + return + } + case <-inotifyEvents: _, err := os.Stat(path) if err == nil { close(done) return } - time.Sleep(25 * time.Millisecond) } } }() -- 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') 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 fe919e4914657d197adfb1be9e6885dbac82d310 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Sat, 10 Nov 2018 21:43:57 +0100 Subject: oci: propagate NOTIFY_SOCKET on runtime start with https://github.com/opencontainers/runc/pull/1807 we moved the systemd notify initialization from "create" to "start", so that the OCI runtime doesn't hang while waiting on reading from the notify socket. This means we also need to set the correct NOTIFY_SOCKET when start'ing the container. Closes: https://github.com/containers/libpod/issues/746 Signed-off-by: Giuseppe Scrivano --- libpod/oci.go | 3 +++ 1 file changed, 3 insertions(+) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index a8013aa47..6ca3ef2e6 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -591,6 +591,9 @@ func (r *OCIRuntime) startContainer(ctr *Container) error { return err } env := []string{fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)} + if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { + env = append(env, fmt.Sprintf("NOTIFY_SOCKET=%s", notify)) + } if err := utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, env, r.path, "start", ctr.ID()); err != nil { return err } -- 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') 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') 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 1d2e1eece5ccb62c731e4d84d451a6b544c3389a Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 27 Nov 2018 21:18:44 +0100 Subject: rootless: propagate XDG_RUNTIME_DIR to the OCI runtime Signed-off-by: Giuseppe Scrivano --- libpod/oci.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/oci.go b/libpod/oci.go index 6ca3ef2e6..3222f9403 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -696,8 +696,12 @@ func (r *OCIRuntime) stopContainer(ctr *Container, timeout uint) error { // deleteContainer deletes a container from the OCI runtime func (r *OCIRuntime) deleteContainer(ctr *Container) error { - _, err := utils.ExecCmd(r.path, "delete", "--force", ctr.ID()) - return err + runtimeDir, err := util.GetRootlessRuntimeDir() + if err != nil { + return err + } + env := []string{fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)} + return utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, env, r.path, "delete", "--force", ctr.ID()) } // pauseContainer pauses the given container -- cgit v1.2.3-54-g00ecf From 6e167029478e29d24ff75d259123e7f7e093b6ff Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 28 Nov 2018 15:27:09 -0500 Subject: Add ability to retrieve runtime configuration from DB When we create a Libpod database, we store a number of runtime configuration fields in it. If we can retrieve those, we can use them to configure the runtime to match the DB instead of inbuilt defaults, helping to ensure that we don't error in cases where our compiled-in defaults changed. Signed-off-by: Matthew Heon --- libpod/boltdb_state.go | 42 ++++++++++++++++++++++++++++++++++++++++ libpod/boltdb_state_internal.go | 43 +++++++++++++++++++++++------------------ libpod/in_memory_state.go | 5 +++++ libpod/state.go | 17 ++++++++++++++++ 4 files changed, 88 insertions(+), 19 deletions(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 42f029379..7191b184a 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -240,6 +240,48 @@ func (s *BoltState) Refresh() error { return err } +// GetDBConfig retrieves runtime configuration fields that were created when +// the database was first initialized +func (s *BoltState) GetDBConfig() (*DBConfig, error) { + cfg := new(DBConfig) + + db, err := s.getDBCon() + if err != nil { + return nil, err + } + defer s.closeDBCon(db) + + err = db.View(func(tx *bolt.Tx) error { + configBucket, err := getRuntimeConfigBucket(tx) + if err != nil { + return nil + } + + // Some of these may be nil + // When we convert to string, Go will coerce them to "" + // That's probably fine - we could raise an error if the key is + // missing, but just not including it is also OK. + libpodRoot := configBucket.Get(staticDirKey) + libpodTmp := configBucket.Get(tmpDirKey) + storageRoot := configBucket.Get(graphRootKey) + storageTmp := configBucket.Get(runRootKey) + graphDriver := configBucket.Get(graphDriverKey) + + cfg.LibpodRoot = string(libpodRoot) + cfg.LibpodTmp = string(libpodTmp) + cfg.StorageRoot = string(storageRoot) + cfg.StorageTmp = string(storageTmp) + cfg.GraphDriver = string(graphDriver) + + return nil + }) + if err != nil { + return nil, err + } + + return cfg, nil +} + // SetNamespace sets the namespace that will be used for container and pod // retrieval func (s *BoltState) SetNamespace(ns string) error { diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index cc7d106cc..8b7c3ae60 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -30,6 +30,13 @@ const ( containersName = "containers" podIDName = "pod-id" namespaceName = "namespace" + + staticDirName = "static-dir" + tmpDirName = "tmp-dir" + runRootName = "run-root" + graphRootName = "graph-root" + graphDriverName = "graph-driver-name" + osName = "os" ) var ( @@ -49,21 +56,19 @@ var ( containersBkt = []byte(containersName) podIDKey = []byte(podIDName) namespaceKey = []byte(namespaceName) + + staticDirKey = []byte(staticDirName) + tmpDirKey = []byte(tmpDirName) + runRootKey = []byte(runRootName) + graphRootKey = []byte(graphRootName) + graphDriverKey = []byte(graphDriverName) + osKey = []byte(osName) ) // Check if the configuration of the database is compatible with the // configuration of the runtime opening it // If there is no runtime configuration loaded, load our own func checkRuntimeConfig(db *bolt.DB, rt *Runtime) error { - var ( - staticDir = []byte("static-dir") - tmpDir = []byte("tmp-dir") - runRoot = []byte("run-root") - graphRoot = []byte("graph-root") - graphDriverName = []byte("graph-driver-name") - osKey = []byte("os") - ) - err := db.Update(func(tx *bolt.Tx) error { configBkt, err := getRuntimeConfigBucket(tx) if err != nil { @@ -74,31 +79,31 @@ func checkRuntimeConfig(db *bolt.DB, rt *Runtime) error { return err } - if err := validateDBAgainstConfig(configBkt, "static dir", - rt.config.StaticDir, staticDir, ""); err != nil { + if err := validateDBAgainstConfig(configBkt, "libpod root directory", + rt.config.StaticDir, staticDirKey, ""); err != nil { return err } - if err := validateDBAgainstConfig(configBkt, "tmp dir", - rt.config.TmpDir, tmpDir, ""); err != nil { + if err := validateDBAgainstConfig(configBkt, "libpod temporary files directory", + rt.config.TmpDir, tmpDirKey, ""); err != nil { return err } - if err := validateDBAgainstConfig(configBkt, "run root", - rt.config.StorageConfig.RunRoot, runRoot, + if err := validateDBAgainstConfig(configBkt, "storage temporary directory", + rt.config.StorageConfig.RunRoot, runRootKey, storage.DefaultStoreOptions.RunRoot); err != nil { return err } - if err := validateDBAgainstConfig(configBkt, "graph root", - rt.config.StorageConfig.GraphRoot, graphRoot, + if err := validateDBAgainstConfig(configBkt, "storage graph root directory", + rt.config.StorageConfig.GraphRoot, graphRootKey, storage.DefaultStoreOptions.GraphRoot); err != nil { return err } - return validateDBAgainstConfig(configBkt, "graph driver name", + return validateDBAgainstConfig(configBkt, "storage graph driver", rt.config.StorageConfig.GraphDriverName, - graphDriverName, + graphDriverKey, storage.DefaultStoreOptions.GraphDriverName) }) diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 78e765ccd..3a775eb43 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -73,6 +73,11 @@ func (s *InMemoryState) Refresh() error { return nil } +// GetDBConfig is not implemented for the in-memory state +func (s *InMemoryState) GetDBConfig() (*DBConfig, error) { + return nil, ErrNotImplemented +} + // SetNamespace sets the namespace for container and pod retrieval. func (s *InMemoryState) SetNamespace(ns string) error { s.namespace = ns diff --git a/libpod/state.go b/libpod/state.go index 273e81318..7f4efa21b 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -1,5 +1,15 @@ package libpod +// DBConfig is a set of Libpod runtime configuration settings that are saved +// in a State when it is first created, and can subsequently be retrieved. +type DBConfig struct { + LibpodRoot string + LibpodTmp string + StorageRoot string + StorageTmp string + GraphDriver string +} + // State is a storage backend for libpod's current state. // A State is only initialized once per instance of libpod. // As such, initialization methods for State implementations may safely assume @@ -21,6 +31,13 @@ type State interface { // Refresh clears container and pod states after a reboot Refresh() error + // GetDBConfig retrieves several paths configured within the database + // when it was created - namely, Libpod root and tmp dirs, c/storage + // root and tmp dirs, and c/storage graph driver. + // This is not implemented by the in-memory state, as it has no need to + // validate runtime configuration. + GetDBConfig() (*DBConfig, error) + // SetNamespace() sets the namespace for the store, and will determine // what containers are retrieved with container and pod retrieval calls. // A namespace of "", the empty string, acts as no namespace, and -- cgit v1.2.3-54-g00ecf From b0f79ff4df58c12fdfaff5c4a7c6e029cb566459 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 29 Nov 2018 11:36:16 -0500 Subject: Move DB configuration up in runtime setup When we configure a runtime, we now will need to hit the DB early on, so we can verify the paths we're going to use for c/storage are correct. Signed-off-by: Matthew Heon --- libpod/runtime.go | 54 +++++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime.go b/libpod/runtime.go index 9feae03fc..8615e5e12 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -426,6 +426,33 @@ func makeRuntime(runtime *Runtime) (err error) { runtime.config.ConmonPath) } + // Set up the state + switch runtime.config.StateType { + case InMemoryStateStore: + state, err := NewInMemoryState() + if err != nil { + return err + } + runtime.state = state + case SQLiteStateStore: + return errors.Wrapf(ErrInvalidArg, "SQLite state is currently disabled") + case BoltDBStateStore: + dbPath := filepath.Join(runtime.config.StaticDir, "bolt_state.db") + + state, err := NewBoltState(dbPath, runtime.lockDir, runtime) + if err != nil { + return err + } + runtime.state = state + default: + return errors.Wrapf(ErrInvalidArg, "unrecognized state type passed") + } + + if err := runtime.state.SetNamespace(runtime.config.Namespace); err != nil { + return errors.Wrapf(err, "error setting libpod namespace in state") + } + logrus.Debugf("Set libpod namespace to %q", runtime.config.Namespace) + // Set up containers/storage var store storage.Store if rootless.SkipStorageSetup() { @@ -540,33 +567,6 @@ func makeRuntime(runtime *Runtime) (err error) { } runtime.firewallBackend = fwBackend - // Set up the state - switch runtime.config.StateType { - case InMemoryStateStore: - state, err := NewInMemoryState() - if err != nil { - return err - } - runtime.state = state - case SQLiteStateStore: - return errors.Wrapf(ErrInvalidArg, "SQLite state is currently disabled") - case BoltDBStateStore: - dbPath := filepath.Join(runtime.config.StaticDir, "bolt_state.db") - - state, err := NewBoltState(dbPath, runtime.lockDir, runtime) - if err != nil { - return err - } - runtime.state = state - default: - return errors.Wrapf(ErrInvalidArg, "unrecognized state type passed") - } - - if err := runtime.state.SetNamespace(runtime.config.Namespace); err != nil { - return errors.Wrapf(err, "error setting libpod namespace in state") - } - logrus.Debugf("Set libpod namespace to %q", runtime.config.Namespace) - // We now need to see if the system has restarted // We check for the presence of a file in our tmp directory to verify this // This check must be locked to prevent races -- cgit v1.2.3-54-g00ecf From 137e0948aed96c3fe6412512e0d138eedf71d499 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 13:36:55 -0500 Subject: Make DB config validation an explicit step Previously, we implicitly validated runtime configuration against what was stored in the database as part of database init. Make this an explicit step, so we can call it after the database has been initialized. This will allow us to retrieve paths from the database and use them to overwrite our defaults if they differ. Signed-off-by: Matthew Heon --- libpod/boltdb_state.go | 29 ++++++++++++++++++++++++----- libpod/in_memory_state.go | 6 ++++++ libpod/runtime.go | 5 +++++ libpod/state.go | 9 +++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 7191b184a..37b309c0d 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -115,11 +115,6 @@ func NewBoltState(path, lockDir string, runtime *Runtime) (State, error) { return nil, errors.Wrapf(err, "error creating initial database layout") } - // Check runtime configuration - if err := checkRuntimeConfig(db, runtime); err != nil { - return nil, err - } - state.valid = true return state, nil @@ -243,6 +238,10 @@ func (s *BoltState) Refresh() error { // GetDBConfig retrieves runtime configuration fields that were created when // the database was first initialized func (s *BoltState) GetDBConfig() (*DBConfig, error) { + if !s.valid { + return nil, ErrDBClosed + } + cfg := new(DBConfig) db, err := s.getDBCon() @@ -282,6 +281,26 @@ func (s *BoltState) GetDBConfig() (*DBConfig, error) { return cfg, nil } +// ValidateDBConfig validates paths in the given runtime against the database +func (s *BoltState) ValidateDBConfig(runtime *Runtime) error { + if !s.valid { + return ErrDBClosed + } + + db, err := s.getDBCon() + if err != nil { + return err + } + defer s.closeDBCon(db) + + // Check runtime configuration + if err := checkRuntimeConfig(db, runtime); err != nil { + return err + } + + return nil +} + // SetNamespace sets the namespace that will be used for container and pod // retrieval func (s *BoltState) SetNamespace(ns string) error { diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 3a775eb43..8cd2f47b9 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -78,6 +78,12 @@ func (s *InMemoryState) GetDBConfig() (*DBConfig, error) { return nil, ErrNotImplemented } +// ValidateDBConfig is not implemented for the in-memory state. +// Since we do nothing just return no error. +func (s *InMemoryState) ValidateDBConfig(runtime *Runtime) error { + return nil +} + // SetNamespace sets the namespace for container and pod retrieval. func (s *InMemoryState) SetNamespace(ns string) error { s.namespace = ns diff --git a/libpod/runtime.go b/libpod/runtime.go index 8615e5e12..2e76f159b 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -448,6 +448,11 @@ func makeRuntime(runtime *Runtime) (err error) { return errors.Wrapf(ErrInvalidArg, "unrecognized state type passed") } + // Validate our config against the database + if err := runtime.state.ValidateDBConfig(runtime); err != nil { + return err + } + if err := runtime.state.SetNamespace(runtime.config.Namespace); err != nil { return errors.Wrapf(err, "error setting libpod namespace in state") } diff --git a/libpod/state.go b/libpod/state.go index 7f4efa21b..99e2435a2 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -38,6 +38,15 @@ type State interface { // validate runtime configuration. GetDBConfig() (*DBConfig, error) + // ValidateDBConfig ralidates the config in the given Runtime struct + // against paths stored in the configured database. + // Libpod root and tmp dirs and c/storage root and tmp dirs and graph + // driver are validated. + // This is not implemented by the in-memory state, as it has no need to + // validate runtime configuration that may change over multiple runs of + // the program. + ValidateDBConfig(runtime *Runtime) error + // SetNamespace() sets the namespace for the store, and will determine // what containers are retrieved with container and pod retrieval calls. // A namespace of "", the empty string, acts as no namespace, and -- cgit v1.2.3-54-g00ecf From aa7ce33b7a7698d220f258bf9b29068be6fdb531 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 14:06:33 -0500 Subject: Add a struct indicating if some Runtime fields were set To configure runtime fields from the database, we need to know whether they were explicitly overwritten by the user (we don't want to overwrite anything that was explicitly set). Store a struct containing whether the variables we'll grab from the DB were explicitly set by the user so we know what we can and can't overwrite. This determines whether libpod runtime and static dirs were set via config file in a horribly hackish way (double TOML decode), but I can't think of a better way, and it shouldn't be that expensive as the libpod config is tiny. Signed-off-by: Matthew Heon --- libpod/options.go | 21 ++++++++++++++++++++- libpod/runtime.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/options.go b/libpod/options.go index 7f4e3ac6b..6783e2a39 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -30,9 +30,26 @@ func WithStorageConfig(config storage.StoreOptions) RuntimeOption { } rt.config.StorageConfig.RunRoot = config.RunRoot + if config.RunRoot != "" { + rt.configuredFrom.storageRunRootSet = true + } + rt.config.StorageConfig.GraphRoot = config.GraphRoot + if config.GraphRoot != "" { + rt.configuredFrom.storageGraphRootSet = true + } + rt.config.StorageConfig.GraphDriverName = config.GraphDriverName - rt.config.StaticDir = filepath.Join(config.GraphRoot, "libpod") + if config.GraphDriverName != "" { + rt.configuredFrom.storageGraphDriverSet = true + } + + // Only set our static dir if it was not already explicitly + // overridden + if config.GraphRoot != "" && !rt.configuredFrom.libpodStaticDirSet { + rt.config.StaticDir = filepath.Join(config.GraphRoot, "libpod") + rt.configuredFrom.libpodStaticDirSet = true + } rt.config.StorageConfig.GraphDriverOptions = make([]string, len(config.GraphDriverOptions)) copy(rt.config.StorageConfig.GraphDriverOptions, config.GraphDriverOptions) @@ -174,6 +191,7 @@ func WithStaticDir(dir string) RuntimeOption { } rt.config.StaticDir = dir + rt.configuredFrom.libpodStaticDirSet = true return nil } @@ -226,6 +244,7 @@ func WithTmpDir(dir string) RuntimeOption { } rt.config.TmpDir = dir + rt.configuredFrom.libpodTmpDirSet = true return nil } diff --git a/libpod/runtime.go b/libpod/runtime.go index 2e76f159b..1e05810fb 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -84,6 +84,7 @@ type Runtime struct { lock sync.RWMutex imageRuntime *image.Runtime firewallBackend firewall.FirewallBackend + configuredFrom *runtimeConfiguredFrom } // RuntimeConfig contains configuration options used to set up the runtime @@ -175,6 +176,20 @@ type RuntimeConfig struct { EnableLabeling bool `toml:"label"` } +// runtimeConfiguredFrom is a struct used during early runtime init to help +// assemble the full RuntimeConfig struct from defaults. +// It indicated whether several fields in the runtime configuration were set +// explicitly. +// If they were not, we may override them with information from the database, +// if it exists and differs from what is present in the system already. +type runtimeConfiguredFrom struct { + storageGraphDriverSet bool + storageGraphRootSet bool + storageRunRootSet bool + libpodStaticDirSet bool + libpodTmpDirSet bool +} + var ( defaultRuntimeConfig = RuntimeConfig{ // Leave this empty so containers/storage will use its defaults @@ -253,6 +268,7 @@ func SetXdgRuntimeDir(val string) error { func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { runtime = new(Runtime) runtime.config = new(RuntimeConfig) + runtime.configuredFrom = new(runtimeConfiguredFrom) // Copy the default configuration tmpDir, err := getDefaultTmpDir() @@ -307,6 +323,25 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { if err != nil { return nil, errors.Wrapf(err, "error reading configuration file %s", configPath) } + + // This is ugly, but we need to decode twice. + // Once to check if libpod static and tmp dirs were explicitly + // set (not enough to check if they're not the default value, + // might have been explicitly configured to the default). + // A second time to actually get a usable config. + tmpConfig := new(RuntimeConfig) + if _, err := toml.Decode(string(contents), tmpConfig); err != nil { + return nil, errors.Wrapf(err, "error decoding configuration file %s", + configPath) + } + + if tmpConfig.StaticDir != "" { + runtime.configuredFrom.libpodStaticDirSet = true + } + if tmpConfig.TmpDir != "" { + runtime.configuredFrom.libpodTmpDirSet = true + } + if _, err := toml.Decode(string(contents), runtime.config); err != nil { return nil, errors.Wrapf(err, "error decoding configuration file %s", configPath) } @@ -348,6 +383,7 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { func NewRuntimeFromConfig(configPath string, options ...RuntimeOption) (runtime *Runtime, err error) { runtime = new(Runtime) runtime.config = new(RuntimeConfig) + runtime.configuredFrom = new(runtimeConfiguredFrom) // Set two fields not in the TOML config runtime.config.StateType = defaultRuntimeConfig.StateType -- cgit v1.2.3-54-g00ecf From 92ff83f5b9dd00ff91dc8c3d6b48f1e0b5c5f3e2 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 14:21:22 -0500 Subject: Set default paths from DB if not explicitly overridden If the DB contains default paths, and the user has not explicitly overridden them, use the paths in the DB over our own defaults. The DB validates these paths, so it would error and prevent operation if they did not match. As such, instead of erroring, we can use the DB's paths instead of our own. Signed-off-by: Matthew Heon --- libpod/in_memory_state.go | 5 +++-- libpod/runtime.go | 26 +++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) (limited to 'libpod') diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 8cd2f47b9..77eba0cc6 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -73,9 +73,10 @@ func (s *InMemoryState) Refresh() error { return nil } -// GetDBConfig is not implemented for the in-memory state +// GetDBConfig is not implemented for in-memory state. +// As we do not store a config, return an empty one. func (s *InMemoryState) GetDBConfig() (*DBConfig, error) { - return nil, ErrNotImplemented + return &DBConfig{}, nil } // ValidateDBConfig is not implemented for the in-memory state. diff --git a/libpod/runtime.go b/libpod/runtime.go index 1e05810fb..e01fa781b 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -484,7 +484,31 @@ func makeRuntime(runtime *Runtime) (err error) { return errors.Wrapf(ErrInvalidArg, "unrecognized state type passed") } - // Validate our config against the database + // Grab config from the database so we can reset some defaults + dbConfig, err := runtime.state.GetDBConfig() + if err != nil { + return errors.Wrapf(err, "error retrieving runtime configuration from database") + } + + // Reset defaults if they were not explicitly set + if !runtime.configuredFrom.storageGraphDriverSet && dbConfig.GraphDriver != "" { + runtime.config.StorageConfig.GraphDriverName = dbConfig.GraphDriver + } + if !runtime.configuredFrom.storageGraphRootSet && dbConfig.StorageRoot != "" { + runtime.config.StorageConfig.GraphRoot = dbConfig.StorageRoot + } + if !runtime.configuredFrom.storageRunRootSet && dbConfig.StorageTmp != "" { + runtime.config.StorageConfig.RunRoot = dbConfig.StorageTmp + } + if !runtime.configuredFrom.libpodStaticDirSet && dbConfig.LibpodRoot != "" { + runtime.config.StaticDir = dbConfig.LibpodRoot + } + if !runtime.configuredFrom.libpodTmpDirSet && dbConfig.LibpodTmp != "" { + runtime.config.TmpDir = dbConfig.LibpodTmp + } + + // Validate our config against the database, now that we've set our + // final storage configuration if err := runtime.state.ValidateDBConfig(runtime); err != nil { return err } -- cgit v1.2.3-54-g00ecf From 562fa57dc9f497db772baa03bfa052082db68646 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 15:21:35 -0500 Subject: Move rootless storage config into libpod Previous commits ensured that we would use database-configured paths if not explicitly overridden. However, our runtime generation did unconditionally override storage config, which made this useless. Move rootless storage configuration setup to libpod, and change storage setup so we only override if a setting is explicitly set, so we can still override what we want. Signed-off-by: Matthew Heon --- cmd/podman/create.go | 2 +- cmd/podman/libpodruntime/runtime.go | 34 +++++++++------------------- cmd/podman/run.go | 2 +- libpod/options.go | 24 ++++++++++++-------- libpod/runtime.go | 9 ++++++++ pkg/util/utils.go | 44 ++++++++++++++++++------------------- 6 files changed, 57 insertions(+), 58 deletions(-) (limited to 'libpod') diff --git a/cmd/podman/create.go b/cmd/podman/create.go index bcf830c7c..1ef9fa47a 100644 --- a/cmd/podman/create.go +++ b/cmd/podman/create.go @@ -66,7 +66,7 @@ func createCmd(c *cli.Context) error { rootless.SetSkipStorageSetup(true) } - runtime, err := libpodruntime.GetContainerRuntime(c) + runtime, err := libpodruntime.GetRuntime(c) if err != nil { return errors.Wrapf(err, "error creating libpod runtime") } diff --git a/cmd/podman/libpodruntime/runtime.go b/cmd/podman/libpodruntime/runtime.go index a4b3581be..13a821b9f 100644 --- a/cmd/podman/libpodruntime/runtime.go +++ b/cmd/podman/libpodruntime/runtime.go @@ -11,32 +11,18 @@ import ( // GetRuntime generates a new libpod runtime configured by command line options func GetRuntime(c *cli.Context) (*libpod.Runtime, error) { - storageOpts, err := util.GetDefaultStoreOptions() - if err != nil { - return nil, err - } - return GetRuntimeWithStorageOpts(c, &storageOpts) -} - -// GetContainerRuntime generates a new libpod runtime configured by command line options for containers -func GetContainerRuntime(c *cli.Context) (*libpod.Runtime, error) { - mappings, err := util.ParseIDMapping(c.StringSlice("uidmap"), c.StringSlice("gidmap"), c.String("subuidmap"), c.String("subgidmap")) - if err != nil { - return nil, err - } - storageOpts, err := util.GetDefaultStoreOptions() - if err != nil { - return nil, err - } - storageOpts.UIDMap = mappings.UIDMap - storageOpts.GIDMap = mappings.GIDMap - return GetRuntimeWithStorageOpts(c, &storageOpts) -} - -// GetRuntime generates a new libpod runtime configured by command line options -func GetRuntimeWithStorageOpts(c *cli.Context, storageOpts *storage.StoreOptions) (*libpod.Runtime, error) { + storageOpts := new(storage.StoreOptions) options := []libpod.RuntimeOption{} + if c.IsSet("uidmap") || c.IsSet("gidmap") || c.IsSet("subuidmap") || c.IsSet("subgidmap") { + mappings, err := util.ParseIDMapping(c.StringSlice("uidmap"), c.StringSlice("gidmap"), c.String("subuidmap"), c.String("subgidmap")) + if err != nil { + return nil, err + } + storageOpts.UIDMap = mappings.UIDMap + storageOpts.GIDMap = mappings.GIDMap + } + if c.GlobalIsSet("root") { storageOpts.GraphRoot = c.GlobalString("root") } diff --git a/cmd/podman/run.go b/cmd/podman/run.go index af6ced45d..a4b5c918e 100644 --- a/cmd/podman/run.go +++ b/cmd/podman/run.go @@ -44,7 +44,7 @@ func runCmd(c *cli.Context) error { rootless.SetSkipStorageSetup(true) } - runtime, err := libpodruntime.GetContainerRuntime(c) + runtime, err := libpodruntime.GetRuntime(c) if err != nil { return errors.Wrapf(err, "error creating libpod runtime") } diff --git a/libpod/options.go b/libpod/options.go index 6783e2a39..661bd8d91 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -29,18 +29,18 @@ func WithStorageConfig(config storage.StoreOptions) RuntimeOption { return ErrRuntimeFinalized } - rt.config.StorageConfig.RunRoot = config.RunRoot if config.RunRoot != "" { + rt.config.StorageConfig.RunRoot = config.RunRoot rt.configuredFrom.storageRunRootSet = true } - rt.config.StorageConfig.GraphRoot = config.GraphRoot if config.GraphRoot != "" { + rt.config.StorageConfig.GraphRoot = config.GraphRoot rt.configuredFrom.storageGraphRootSet = true } - rt.config.StorageConfig.GraphDriverName = config.GraphDriverName if config.GraphDriverName != "" { + rt.config.StorageConfig.GraphDriverName = config.GraphDriverName rt.configuredFrom.storageGraphDriverSet = true } @@ -51,14 +51,20 @@ func WithStorageConfig(config storage.StoreOptions) RuntimeOption { rt.configuredFrom.libpodStaticDirSet = true } - rt.config.StorageConfig.GraphDriverOptions = make([]string, len(config.GraphDriverOptions)) - copy(rt.config.StorageConfig.GraphDriverOptions, config.GraphDriverOptions) + if config.GraphDriverOptions != nil { + rt.config.StorageConfig.GraphDriverOptions = make([]string, len(config.GraphDriverOptions)) + copy(rt.config.StorageConfig.GraphDriverOptions, config.GraphDriverOptions) + } - rt.config.StorageConfig.UIDMap = make([]idtools.IDMap, len(config.UIDMap)) - copy(rt.config.StorageConfig.UIDMap, config.UIDMap) + if config.UIDMap != nil { + rt.config.StorageConfig.UIDMap = make([]idtools.IDMap, len(config.UIDMap)) + copy(rt.config.StorageConfig.UIDMap, config.UIDMap) + } - rt.config.StorageConfig.GIDMap = make([]idtools.IDMap, len(config.GIDMap)) - copy(rt.config.StorageConfig.GIDMap, config.GIDMap) + if config.GIDMap != nil { + rt.config.StorageConfig.GIDMap = make([]idtools.IDMap, len(config.GIDMap)) + copy(rt.config.StorageConfig.GIDMap, config.GIDMap) + } return nil } diff --git a/libpod/runtime.go b/libpod/runtime.go index e01fa781b..6a5d2ad39 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -278,6 +278,15 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { deepcopier.Copy(defaultRuntimeConfig).To(runtime.config) runtime.config.TmpDir = tmpDir + if rootless.IsRootless() { + // If we're rootless, override the default storage config + storageConf, err := util.GetDefaultRootlessStoreOptions() + if err != nil { + return nil, errors.Wrapf(err, "error retrieving rootless storage config") + } + runtime.config.StorageConfig = storageConf + } + configPath := ConfigPath foundConfig := true rootlessConfigPath := "" diff --git a/pkg/util/utils.go b/pkg/util/utils.go index de29bc5d8..78484eb78 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -313,33 +313,31 @@ func getTomlStorage(storeOptions *storage.StoreOptions) *tomlConfig { return config } -// GetDefaultStoreOptions returns the storage ops for containers -func GetDefaultStoreOptions() (storage.StoreOptions, error) { - storageOpts := storage.DefaultStoreOptions - if rootless.IsRootless() { - var err error - storageOpts, err = GetRootlessStorageOpts() +// GetDefaultStoreOptions returns the storage ops for containers. +func GetDefaultRootlessStoreOptions() (storage.StoreOptions, error) { + var err error + storageOpts, err := GetRootlessStorageOpts() + if err != nil { + return storageOpts, err + } + + storageConf := filepath.Join(os.Getenv("HOME"), ".config/containers/storage.conf") + if _, err := os.Stat(storageConf); err == nil { + storage.ReloadConfigurationFile(storageConf, &storageOpts) + } else if os.IsNotExist(err) { + os.MkdirAll(filepath.Dir(storageConf), 0755) + file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) if err != nil { - return storageOpts, err + return storageOpts, errors.Wrapf(err, "cannot open %s", storageConf) } - storageConf := filepath.Join(os.Getenv("HOME"), ".config/containers/storage.conf") - if _, err := os.Stat(storageConf); err == nil { - storage.ReloadConfigurationFile(storageConf, &storageOpts) - } else if os.IsNotExist(err) { - os.MkdirAll(filepath.Dir(storageConf), 0755) - file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) - if err != nil { - return storageOpts, errors.Wrapf(err, "cannot open %s", storageConf) - } - - tomlConfiguration := getTomlStorage(&storageOpts) - defer file.Close() - enc := toml.NewEncoder(file) - if err := enc.Encode(tomlConfiguration); err != nil { - os.Remove(storageConf) - } + tomlConfiguration := getTomlStorage(&storageOpts) + defer file.Close() + enc := toml.NewEncoder(file) + if err := enc.Encode(tomlConfiguration); err != nil { + os.Remove(storageConf) } } + return storageOpts, nil } -- cgit v1.2.3-54-g00ecf From 03229239b018b611b5f0307dc0d11bb0fb15c1ae Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 15:32:06 -0500 Subject: Do not initialize locks dir in BoltDB We already create the locks directory as part of the libpod runtime's init - no need to do it again as part of BoltDB's init. Signed-off-by: Matthew Heon --- libpod/boltdb_state.go | 10 ---------- libpod/runtime.go | 7 +++++++ 2 files changed, 7 insertions(+), 10 deletions(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 37b309c0d..8b9b77a54 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -3,7 +3,6 @@ package libpod import ( "bytes" "encoding/json" - "os" "strings" "sync" @@ -62,15 +61,6 @@ func NewBoltState(path, lockDir string, runtime *Runtime) (State, error) { logrus.Debugf("Initializing boltdb state at %s", path) - // Make the directory that will hold container lockfiles - if err := os.MkdirAll(lockDir, 0750); err != nil { - // The directory is allowed to exist - if !os.IsExist(err) { - return nil, errors.Wrapf(err, "error creating lockfiles dir %s", lockDir) - } - } - state.lockDir = lockDir - db, err := bolt.Open(path, 0600, nil) if err != nil { return nil, errors.Wrapf(err, "error opening database %s", path) diff --git a/libpod/runtime.go b/libpod/runtime.go index 6a5d2ad39..9afa1bc10 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -285,6 +285,7 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { return nil, errors.Wrapf(err, "error retrieving rootless storage config") } runtime.config.StorageConfig = storageConf + runtime.config.StaticDir = filepath.Join(storageConf.GraphRoot, "libpod") } configPath := ConfigPath @@ -516,6 +517,12 @@ func makeRuntime(runtime *Runtime) (err error) { runtime.config.TmpDir = dbConfig.LibpodTmp } + logrus.Debugf("Using graph driver %s", runtime.config.StorageConfig.GraphDriverName) + logrus.Debugf("Using graph root %s", runtime.config.StorageConfig.GraphRoot) + logrus.Debugf("Using run root %s", runtime.config.StorageConfig.RunRoot) + logrus.Debugf("Using static dir %s", runtime.config.StaticDir) + logrus.Debugf("Using tmp dir %s", runtime.config.TmpDir) + // Validate our config against the database, now that we've set our // final storage configuration if err := runtime.state.ValidateDBConfig(runtime); err != nil { -- cgit v1.2.3-54-g00ecf From 69ed2ccc54f48b8fff30e28644ad96d56d093dd1 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 16:10:26 -0500 Subject: Make locks dir in unit tests Ensure we don't break the unit tests by creating a locks directory (which, prior to the last commit, would be created by BoltDB state init). Signed-off-by: Matthew Heon --- libpod/state_test.go | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'libpod') diff --git a/libpod/state_test.go b/libpod/state_test.go index 04572fb29..0f5da62e4 100644 --- a/libpod/state_test.go +++ b/libpod/state_test.go @@ -45,6 +45,10 @@ func getEmptyBoltState() (s State, p string, p2 string, err error) { dbPath := filepath.Join(tmpDir, "db.sql") lockDir := filepath.Join(tmpDir, "locks") + if err := os.Mkdir(lockDir, 0755); err != nil { + return nil, "", "", err + } + runtime := new(Runtime) runtime.config = new(RuntimeConfig) runtime.config.StorageConfig = storage.StoreOptions{} -- cgit v1.2.3-54-g00ecf From b104a45f35a437593774f851b0a3b45fd692b263 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 2 Dec 2018 16:40:38 -0500 Subject: Fix gofmt and lint Signed-off-by: Matthew Heon --- libpod/state.go | 6 +++--- pkg/util/utils.go | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/state.go b/libpod/state.go index 99e2435a2..53b66cdb3 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -3,10 +3,10 @@ package libpod // DBConfig is a set of Libpod runtime configuration settings that are saved // in a State when it is first created, and can subsequently be retrieved. type DBConfig struct { - LibpodRoot string - LibpodTmp string + LibpodRoot string + LibpodTmp string StorageRoot string - StorageTmp string + StorageTmp string GraphDriver string } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 78484eb78..ed79c4b46 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -313,7 +313,8 @@ func getTomlStorage(storeOptions *storage.StoreOptions) *tomlConfig { return config } -// GetDefaultStoreOptions returns the storage ops for containers. +// GetDefaultRootlessStoreOptions returns the storage opts for rootless +// containers. func GetDefaultRootlessStoreOptions() (storage.StoreOptions, error) { var err error storageOpts, err := GetRootlessStorageOpts() -- cgit v1.2.3-54-g00ecf From 32fc865b6d0b530b74b1429775d3f1f5f24f288a Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 3 Dec 2018 10:38:32 -0500 Subject: Add better descriptions for validation errors in DB When validating fields against the DB, report more verbosely the name of the field being validated if it fails. Specifically, add the name used in config files, so people will actually know what to change it errors happen. Signed-off-by: Matthew Heon --- libpod/boltdb_state_internal.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 8b7c3ae60..05536e069 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -79,23 +79,23 @@ func checkRuntimeConfig(db *bolt.DB, rt *Runtime) error { return err } - if err := validateDBAgainstConfig(configBkt, "libpod root directory", + if err := validateDBAgainstConfig(configBkt, "libpod root directory (staticdir)", rt.config.StaticDir, staticDirKey, ""); err != nil { return err } - if err := validateDBAgainstConfig(configBkt, "libpod temporary files directory", + if err := validateDBAgainstConfig(configBkt, "libpod temporary files directory (tmpdir)", rt.config.TmpDir, tmpDirKey, ""); err != nil { return err } - if err := validateDBAgainstConfig(configBkt, "storage temporary directory", + if err := validateDBAgainstConfig(configBkt, "storage temporary directory (runroot)", rt.config.StorageConfig.RunRoot, runRootKey, storage.DefaultStoreOptions.RunRoot); err != nil { return err } - if err := validateDBAgainstConfig(configBkt, "storage graph root directory", + if err := validateDBAgainstConfig(configBkt, "storage graph root directory (graphroot)", rt.config.StorageConfig.GraphRoot, graphRootKey, storage.DefaultStoreOptions.GraphRoot); err != nil { return err -- cgit v1.2.3-54-g00ecf From ea13264958f3382fe8fe6a9709a7eae00f753acc Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 3 Dec 2018 10:48:33 -0500 Subject: Fix typo Signed-off-by: Matthew Heon --- libpod/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/state.go b/libpod/state.go index 53b66cdb3..06c2003d8 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -38,7 +38,7 @@ type State interface { // validate runtime configuration. GetDBConfig() (*DBConfig, error) - // ValidateDBConfig ralidates the config in the given Runtime struct + // ValidateDBConfig validates the config in the given Runtime struct // against paths stored in the configured database. // Libpod root and tmp dirs and c/storage root and tmp dirs and graph // driver are validated. -- cgit v1.2.3-54-g00ecf From 677c44446375680c5a69a9612f7df42b25de783f Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 3 Dec 2018 11:07:01 -0500 Subject: Ensure directory where we will make database exists Ensure that the directory where we will create the Podman db exists prior to creating the database - otherwise creating the DB will fail. Signed-off-by: Matthew Heon --- libpod/runtime.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime.go b/libpod/runtime.go index 9afa1bc10..8b5bc32b4 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -472,6 +472,15 @@ func makeRuntime(runtime *Runtime) (err error) { runtime.config.ConmonPath) } + // Make the static files directory if it does not exist + if err := os.MkdirAll(runtime.config.StaticDir, 0700); err != nil { + // The directory is allowed to exist + if !os.IsExist(err) { + return errors.Wrapf(err, "error creating runtime static files directory %s", + runtime.config.StaticDir) + } + } + // Set up the state switch runtime.config.StateType { case InMemoryStateStore: @@ -601,15 +610,6 @@ func makeRuntime(runtime *Runtime) (err error) { } runtime.ociRuntime = ociRuntime - // Make the static files directory if it does not exist - if err := os.MkdirAll(runtime.config.StaticDir, 0755); err != nil { - // The directory is allowed to exist - if !os.IsExist(err) { - return errors.Wrapf(err, "error creating runtime static files directory %s", - runtime.config.StaticDir) - } - } - // Make a directory to hold container lockfiles lockDir := filepath.Join(runtime.config.TmpDir, "lock") if err := os.MkdirAll(lockDir, 0755); err != nil { -- cgit v1.2.3-54-g00ecf From 7c575bdce26b0cc5560bb5a8fe5ac680c2843903 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 3 Dec 2018 15:13:07 -0500 Subject: Fix libpod static dir selection when graphroot changed When graphroot is set by the user, we should set libpod's static directory to a subdirectory of that by default, to duplicate previous behavior. Signed-off-by: Matthew Heon --- libpod/options.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'libpod') diff --git a/libpod/options.go b/libpod/options.go index 661bd8d91..4a3d30cfe 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -37,6 +37,11 @@ func WithStorageConfig(config storage.StoreOptions) RuntimeOption { if config.GraphRoot != "" { rt.config.StorageConfig.GraphRoot = config.GraphRoot rt.configuredFrom.storageGraphRootSet = true + + // Also set libpod static dir, so we are a subdirectory + // of the c/storage store by default + rt.config.StaticDir = filepath.Join(config.GraphRoot, "libpod") + rt.configuredFrom.libpodStaticDirSet = true } if config.GraphDriverName != "" { @@ -44,13 +49,6 @@ func WithStorageConfig(config storage.StoreOptions) RuntimeOption { rt.configuredFrom.storageGraphDriverSet = true } - // Only set our static dir if it was not already explicitly - // overridden - if config.GraphRoot != "" && !rt.configuredFrom.libpodStaticDirSet { - rt.config.StaticDir = filepath.Join(config.GraphRoot, "libpod") - rt.configuredFrom.libpodStaticDirSet = true - } - if config.GraphDriverOptions != nil { rt.config.StorageConfig.GraphDriverOptions = make([]string, len(config.GraphDriverOptions)) copy(rt.config.StorageConfig.GraphDriverOptions, config.GraphDriverOptions) -- cgit v1.2.3-54-g00ecf From 795fbba7695b03736acaf9abe75922404f5eea44 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 3 Dec 2018 15:38:35 -0500 Subject: Revert changes to GetDefaultStoreOptions We don't need this for anything more than rootless work in Libpod now, but Buildah still uses it as it was originally written, so leave it intact as part of our API. Signed-off-by: Matthew Heon --- libpod/runtime.go | 2 +- pkg/util/utils.go | 45 +++++++++++++++++++++++---------------------- 2 files changed, 24 insertions(+), 23 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime.go b/libpod/runtime.go index 8b5bc32b4..e69b63a24 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -280,7 +280,7 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { if rootless.IsRootless() { // If we're rootless, override the default storage config - storageConf, err := util.GetDefaultRootlessStoreOptions() + storageConf, err := util.GetDefaultStoreOptions() if err != nil { return nil, errors.Wrapf(err, "error retrieving rootless storage config") } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index ed79c4b46..e483253a4 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -313,32 +313,33 @@ func getTomlStorage(storeOptions *storage.StoreOptions) *tomlConfig { return config } -// GetDefaultRootlessStoreOptions returns the storage opts for rootless -// containers. -func GetDefaultRootlessStoreOptions() (storage.StoreOptions, error) { - var err error - storageOpts, err := GetRootlessStorageOpts() - if err != nil { - return storageOpts, err - } - - storageConf := filepath.Join(os.Getenv("HOME"), ".config/containers/storage.conf") - if _, err := os.Stat(storageConf); err == nil { - storage.ReloadConfigurationFile(storageConf, &storageOpts) - } else if os.IsNotExist(err) { - os.MkdirAll(filepath.Dir(storageConf), 0755) - file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) +// GetDefaultStoreOptions returns the default storage options for containers. +func GetDefaultStoreOptions() (storage.StoreOptions, error) { + storageOpts := storage.DefaultStoreOptions + if rootless.IsRootless() { + var err error + storageOpts, err = GetRootlessStorageOpts() if err != nil { - return storageOpts, errors.Wrapf(err, "cannot open %s", storageConf) + return storageOpts, err } - tomlConfiguration := getTomlStorage(&storageOpts) - defer file.Close() - enc := toml.NewEncoder(file) - if err := enc.Encode(tomlConfiguration); err != nil { - os.Remove(storageConf) + storageConf := filepath.Join(os.Getenv("HOME"), ".config/containers/storage.conf") + if _, err := os.Stat(storageConf); err == nil { + storage.ReloadConfigurationFile(storageConf, &storageOpts) + } else if os.IsNotExist(err) { + os.MkdirAll(filepath.Dir(storageConf), 0755) + file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) + if err != nil { + return storageOpts, errors.Wrapf(err, "cannot open %s", storageConf) + } + + tomlConfiguration := getTomlStorage(&storageOpts) + defer file.Close() + enc := toml.NewEncoder(file) + if err := enc.Encode(tomlConfiguration); err != nil { + os.Remove(storageConf) + } } } - return storageOpts, nil } -- 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') 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 From e3882cfa2d1329d44c8a580418ea1d56804b331d Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 4 Dec 2018 13:50:38 -0500 Subject: Use runtime lockDir in BoltDB state Instead of storing the runtime's file lock dir in the BoltDB state, refer to the runtime inside the Bolt state instead, and use the path stored in the runtime. This is necessary since we moved DB initialization very far up in runtime init, before the locks dir is properly initialized (and it must happen before the locks dir can be created, as we use the DB to retrieve the proper path for the locks dir now). Signed-off-by: Matthew Heon --- libpod/boltdb_state.go | 4 +--- libpod/boltdb_state_internal.go | 4 ++-- libpod/runtime.go | 2 +- libpod/state_test.go | 3 ++- 4 files changed, 6 insertions(+), 7 deletions(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 8b9b77a54..cb661d4e9 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -18,7 +18,6 @@ type BoltState struct { dbLock sync.Mutex namespace string namespaceBytes []byte - lockDir string runtime *Runtime } @@ -51,10 +50,9 @@ type BoltState struct { // containers/storage do not occur. // NewBoltState creates a new bolt-backed state database -func NewBoltState(path, lockDir string, runtime *Runtime) (State, error) { +func NewBoltState(path string, runtime *Runtime) (State, error) { state := new(BoltState) state.dbPath = path - state.lockDir = lockDir state.runtime = runtime state.namespace = "" state.namespaceBytes = nil diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 05536e069..3f00657ea 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -266,7 +266,7 @@ func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt. } // Get the lock - lockPath := filepath.Join(s.lockDir, string(id)) + lockPath := filepath.Join(s.runtime.lockDir, string(id)) lock, err := storage.GetLockfile(lockPath) if err != nil { return errors.Wrapf(err, "error retrieving lockfile for container %s", string(id)) @@ -302,7 +302,7 @@ func (s *BoltState) getPodFromDB(id []byte, pod *Pod, podBkt *bolt.Bucket) error } // Get the lock - lockPath := filepath.Join(s.lockDir, string(id)) + lockPath := filepath.Join(s.runtime.lockDir, string(id)) lock, err := storage.GetLockfile(lockPath) if err != nil { return errors.Wrapf(err, "error retrieving lockfile for pod %s", string(id)) diff --git a/libpod/runtime.go b/libpod/runtime.go index e69b63a24..083ab1ec3 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -494,7 +494,7 @@ func makeRuntime(runtime *Runtime) (err error) { case BoltDBStateStore: dbPath := filepath.Join(runtime.config.StaticDir, "bolt_state.db") - state, err := NewBoltState(dbPath, runtime.lockDir, runtime) + state, err := NewBoltState(dbPath, runtime) if err != nil { return err } diff --git a/libpod/state_test.go b/libpod/state_test.go index 0f5da62e4..d93a371f3 100644 --- a/libpod/state_test.go +++ b/libpod/state_test.go @@ -52,8 +52,9 @@ func getEmptyBoltState() (s State, p string, p2 string, err error) { runtime := new(Runtime) runtime.config = new(RuntimeConfig) runtime.config.StorageConfig = storage.StoreOptions{} + runtime.lockDir = lockDir - state, err := NewBoltState(dbPath, lockDir, runtime) + state, err := NewBoltState(dbPath, runtime) if err != nil { return nil, "", "", err } -- cgit v1.2.3-54-g00ecf From 32aa45e344abc3e9501b0fddbac099285869e224 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 4 Dec 2018 14:57:06 -0500 Subject: Don't initialize CNI when running as rootless We don't use CNI to configure networks for rootless containers, so no need to set it up. It may also cause issues with inotify, so disabling it resolves some potential problems. Signed-off-by: Matthew Heon --- libpod/runtime.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime.go b/libpod/runtime.go index 9feae03fc..78092536d 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -523,11 +523,13 @@ func makeRuntime(runtime *Runtime) (err error) { } // Set up the CNI net plugin - netPlugin, err := ocicni.InitCNI(runtime.config.CNIDefaultNetwork, runtime.config.CNIConfigDir, runtime.config.CNIPluginDir...) - if err != nil { - return errors.Wrapf(err, "error configuring CNI network plugin") + if !rootless.IsRootless() { + netPlugin, err := ocicni.InitCNI(runtime.config.CNIDefaultNetwork, runtime.config.CNIConfigDir, runtime.config.CNIPluginDir...) + if err != nil { + return errors.Wrapf(err, "error configuring CNI network plugin") + } + runtime.netPlugin = netPlugin } - runtime.netPlugin = netPlugin // Set up a firewall backend backendType := "" -- cgit v1.2.3-54-g00ecf From 650f95cb06cb5a4e979fbe9f9fcd125a229e4e09 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 4 Dec 2018 11:46:17 -0800 Subject: libpod/container_internal_linux: Allow gids that aren't in the group file When an image config sets config.User [1] to a numeric group (like 1000:1000), but those values do not exist in the container's /etc/group, libpod is currently breaking: $ podman run --rm registry.svc.ci.openshift.org/ci-op-zvml7cd6/pipeline:installer --help error creating temporary passwd file for container 228f6e9943d6f18b93c19644e9b619ec4d459a3e0eb31680e064eeedf6473678: unable to get gid 1000 from group file: no matching entries in group file However, the OCI spec requires converters to copy numeric uid and gid to the runtime config verbatim [2]. With this commit, I'm frontloading the "is groupspec an integer?" check and only bothering with lookup.GetGroup when it was not. I've also removed a few .Mounted checks, which are originally from 00d38cb3 (podman create/run need to load information from the image, 2017-12-18, #110). We don't need a mounted container filesystem to translate integers. And when the lookup code needs to fall back to the mounted root to translate names, it can handle erroring out internally (and looking it over, it seems to do that already). [1]: https://github.com/opencontainers/image-spec/blame/v1.0.1/config.md#L118-L123 [2]: https://github.com/opencontainers/image-spec/blame/v1.0.1/conversion.md#L70 Signed-off-by: W. Trevor King --- libpod/container_internal_linux.go | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 8861d7728..b72eefdf1 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -238,9 +238,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } if c.config.User != "" { - if !c.state.Mounted { - return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to translate User field", c.ID()) - } // User and Group must go together g.SetProcessUID(uint32(execUser.Uid)) g.SetProcessGID(uint32(execUser.Gid)) @@ -248,9 +245,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { // Add addition groups if c.config.GroupAdd is not empty if len(c.config.Groups) > 0 { - if !c.state.Mounted { - return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to add additional groups", c.ID()) - } gids, _ := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, nil) for _, gid := range gids { g.AddProcessAdditionalGid(gid) @@ -802,7 +796,6 @@ func (c *Container) generateHosts() (string, error) { func (c *Container) generatePasswd() (string, error) { var ( groupspec string - group *user.Group gid int ) if c.config.User == "" { @@ -827,17 +820,16 @@ func (c *Container) generatePasswd() (string, error) { 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 { + ugid, err := strconv.ParseUint(groupspec, 10, 32) + if err == nil { + gid = int(ugid) + } else { + group, err := lookup.GetGroup(c.state.Mountpoint, groupspec) + if err != nil { return "", errors.Wrapf(err, "unable to get gid %s from group file", groupspec) } - return "", err + gid = group.Gid } - gid = group.Gid } originPasswdFile := filepath.Join(c.state.Mountpoint, "/etc/passwd") orig, err := ioutil.ReadFile(originPasswdFile) -- cgit v1.2.3-54-g00ecf