diff options
-rw-r--r-- | libpod/container_exec.go | 67 | ||||
-rw-r--r-- | libpod/events.go | 39 | ||||
-rw-r--r-- | libpod/events/config.go | 2 | ||||
-rw-r--r-- | libpod/events/events.go | 2 | ||||
-rw-r--r-- | libpod/networking_linux.go | 4 | ||||
-rw-r--r-- | libpod/options.go | 13 | ||||
-rw-r--r-- | libpod/volume_internal.go | 19 | ||||
-rw-r--r-- | pkg/domain/infra/abi/containers.go | 36 | ||||
-rw-r--r-- | pkg/domain/infra/abi/parse/parse.go | 4 | ||||
-rwxr-xr-x | test/compose/test-compose | 2 | ||||
-rw-r--r-- | test/e2e/run_volume_test.go | 32 |
11 files changed, 174 insertions, 46 deletions
diff --git a/libpod/container_exec.go b/libpod/container_exec.go index c359f1e5d..737bf74ad 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -1,6 +1,7 @@ package libpod import ( + "context" "io/ioutil" "net/http" "os" @@ -539,18 +540,7 @@ func (c *Container) ExecStop(sessionID string, timeout *uint) error { var cleanupErr error // Retrieve exit code and update status - exitCode, err := c.readExecExitCode(session.ID()) - if err != nil { - cleanupErr = err - } - session.ExitCode = exitCode - session.PID = 0 - session.State = define.ExecStateStopped - - if err := c.save(); err != nil { - if cleanupErr != nil { - logrus.Errorf("Error stopping container %s exec session %s: %v", c.ID(), session.ID(), cleanupErr) - } + if err := retrieveAndWriteExecExitCode(c, session.ID()); err != nil { cleanupErr = err } @@ -592,15 +582,7 @@ func (c *Container) ExecCleanup(sessionID string) error { return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot clean up container %s exec session %s as it is running", c.ID(), session.ID()) } - exitCode, err := c.readExecExitCode(session.ID()) - if err != nil { - return err - } - session.ExitCode = exitCode - session.PID = 0 - session.State = define.ExecStateStopped - - if err := c.save(); err != nil { + if err := retrieveAndWriteExecExitCode(c, session.ID()); err != nil { return err } } @@ -637,9 +619,9 @@ func (c *Container) ExecRemove(sessionID string, force bool) error { return err } if !running { - session.State = define.ExecStateStopped - // TODO: should we retrieve exit code here? - // TODO: Might be worth saving state here. + if err := retrieveAndWriteExecExitCode(c, session.ID()); err != nil { + return err + } } } @@ -653,6 +635,10 @@ func (c *Container) ExecRemove(sessionID string, force bool) error { return err } + if err := retrieveAndWriteExecExitCode(c, session.ID()); err != nil { + return err + } + if err := c.cleanupExecBundle(session.ID()); err != nil { return err } @@ -757,10 +743,25 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi session, err := c.ExecSession(sessionID) if err != nil { + if errors.Cause(err) == define.ErrNoSuchExecSession { + // TODO: If a proper Context is ever plumbed in here, we + // should use it. + // As things stand, though, it's not worth it - this + // should always terminate quickly since it's not + // streaming. + diedEvent, err := c.runtime.GetExecDiedEvent(context.Background(), c.ID(), sessionID) + if err != nil { + return -1, errors.Wrapf(err, "error retrieving exec session %s exit code", sessionID) + } + return diedEvent.ContainerExitCode, nil + } return -1, err } exitCode := session.ExitCode if err := c.ExecRemove(sessionID, false); err != nil { + if errors.Cause(err) == define.ErrNoSuchExecSession { + return exitCode, nil + } return -1, err } @@ -927,6 +928,8 @@ func (c *Container) getActiveExecSessions() ([]string, error) { session.PID = 0 session.State = define.ExecStateStopped + c.newExecDiedEvent(session.ID(), exitCode) + needSave = true } if err := c.cleanupExecBundle(id); err != nil { @@ -1036,6 +1039,22 @@ func writeExecExitCode(c *Container, sessionID string, exitCode int) error { return errors.Wrapf(err, "error syncing container %s state to remove exec session %s", c.ID(), sessionID) } + return justWriteExecExitCode(c, sessionID, exitCode) +} + +func retrieveAndWriteExecExitCode(c *Container, sessionID string) error { + exitCode, err := c.readExecExitCode(sessionID) + if err != nil { + return err + } + + return justWriteExecExitCode(c, sessionID, exitCode) +} + +func justWriteExecExitCode(c *Container, sessionID string, exitCode int) error { + // Write an event first + c.newExecDiedEvent(sessionID, exitCode) + session, ok := c.state.ExecSessions[sessionID] if !ok { // Exec session already removed. diff --git a/libpod/events.go b/libpod/events.go index 839229674..22c51aeec 100644 --- a/libpod/events.go +++ b/libpod/events.go @@ -46,7 +46,22 @@ func (c *Container) newContainerExitedEvent(exitCode int32) { e.Type = events.Container e.ContainerExitCode = int(exitCode) if err := c.runtime.eventer.Write(e); err != nil { - logrus.Errorf("unable to write pod event: %q", err) + logrus.Errorf("unable to write container exited event: %q", err) + } +} + +// newExecDiedEvent creates a new event for an exec session's death +func (c *Container) newExecDiedEvent(sessionID string, exitCode int) { + e := events.NewEvent(events.ExecDied) + e.ID = c.ID() + e.Name = c.Name() + e.Image = c.config.RootfsImageName + e.Type = events.Container + e.ContainerExitCode = exitCode + e.Attributes = make(map[string]string) + e.Attributes["execID"] = sessionID + if err := c.runtime.eventer.Write(e); err != nil { + logrus.Errorf("unable to write exec died event: %q", err) } } @@ -154,3 +169,25 @@ func (r *Runtime) GetLastContainerEvent(ctx context.Context, nameOrID string, co // return the last element in the slice return containerEvents[len(containerEvents)-1], nil } + +// GetExecDiedEvent takes a container name or ID, exec session ID, and returns +// that exec session's Died event (if it has already occurred). +func (r *Runtime) GetExecDiedEvent(ctx context.Context, nameOrID, execSessionID string) (*events.Event, error) { + filters := []string{ + fmt.Sprintf("container=%s", nameOrID), + "event=exec_died", + "type=container", + fmt.Sprintf("label=execID=%s", execSessionID), + } + + containerEvents, err := r.GetEvents(ctx, filters) + if err != nil { + return nil, err + } + // There *should* only be one event maximum. + // But... just in case... let's not blow up if there's more than one. + if len(containerEvents) < 1 { + return nil, errors.Wrapf(events.ErrEventNotFound, "exec died event for session %s (container %s) not found", execSessionID, nameOrID) + } + return containerEvents[len(containerEvents)-1], nil +} diff --git a/libpod/events/config.go b/libpod/events/config.go index 085fa9d52..d88d7b6e3 100644 --- a/libpod/events/config.go +++ b/libpod/events/config.go @@ -127,6 +127,8 @@ const ( Create Status = "create" // Exec ... Exec Status = "exec" + // ExecDied indicates that an exec session in a container died. + ExecDied Status = "exec_died" // Exited indicates that a container's process died Exited Status = "died" // Export ... diff --git a/libpod/events/events.go b/libpod/events/events.go index 01ea6a386..e03215eff 100644 --- a/libpod/events/events.go +++ b/libpod/events/events.go @@ -149,6 +149,8 @@ func StringToStatus(name string) (Status, error) { return Create, nil case Exec.String(): return Exec, nil + case ExecDied.String(): + return ExecDied, nil case Exited.String(): return Exited, nil case Export.String(): diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index c928e02a6..5446841f6 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -1090,7 +1090,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro } c.newNetworkEvent(events.NetworkDisconnect, netName) - if c.state.State != define.ContainerStateRunning { + if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) { return nil } @@ -1145,7 +1145,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e return err } c.newNetworkEvent(events.NetworkConnect, netName) - if c.state.State != define.ContainerStateRunning { + if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) { return nil } if c.state.NetNS == nil { diff --git a/libpod/options.go b/libpod/options.go index f942d264b..d3be46ad8 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1641,6 +1641,19 @@ func WithVolumeGID(gid int) VolumeCreateOption { } } +// WithVolumeNoChown prevents the volume from being chowned to the process uid at first use. +func WithVolumeNoChown() VolumeCreateOption { + return func(volume *Volume) error { + if volume.valid { + return define.ErrVolumeFinalized + } + + volume.state.NeedsChown = false + + return nil + } +} + // withSetAnon sets a bool notifying libpod that this volume is anonymous and // should be removed when containers using it are removed and volumes are // specified for removal. diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go index 694cdd149..19008a253 100644 --- a/libpod/volume_internal.go +++ b/libpod/volume_internal.go @@ -39,8 +39,23 @@ func (v *Volume) needsMount() bool { return true } - // Local driver with options needs mount - return len(v.config.Options) > 0 + // Commit 28138dafcc added the UID and GID options to this map + // However we should only mount when options other than uid and gid are set. + // see https://github.com/containers/podman/issues/10620 + index := 0 + if _, ok := v.config.Options["UID"]; ok { + index++ + } + if _, ok := v.config.Options["GID"]; ok { + index++ + } + // when uid or gid is set there is also the "o" option + // set so we have to ignore this one as well + if index > 0 { + index++ + } + // Local driver with options other than uid,gid needs mount + return len(v.config.Options) > index } // update() updates the volume state from the DB. diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 4908e72f6..5a953c047 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -595,7 +595,7 @@ func (ic *ContainerEngine) ContainerAttach(ctx context.Context, nameOrID string, return nil } -func makeExecConfig(options entities.ExecOptions) *libpod.ExecConfig { +func makeExecConfig(options entities.ExecOptions, rt *libpod.Runtime) (*libpod.ExecConfig, error) { execConfig := new(libpod.ExecConfig) execConfig.Command = options.Cmd execConfig.Terminal = options.Tty @@ -607,7 +607,20 @@ func makeExecConfig(options entities.ExecOptions) *libpod.ExecConfig { execConfig.PreserveFDs = options.PreserveFDs execConfig.AttachStdin = options.Interactive - return execConfig + // Make an exit command + storageConfig := rt.StorageConfig() + runtimeConfig, err := rt.GetConfig() + if err != nil { + return nil, errors.Wrapf(err, "error retrieving Libpod configuration to build exec exit command") + } + // TODO: Add some ability to toggle syslog + exitCommandArgs, err := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, false, true, true) + if err != nil { + return nil, errors.Wrapf(err, "error constructing exit command for exec session") + } + execConfig.ExitCommand = exitCommandArgs + + return execConfig, nil } func checkExecPreserveFDs(options entities.ExecOptions) error { @@ -647,7 +660,10 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrID string, o } ctr := ctrs[0] - execConfig := makeExecConfig(options) + execConfig, err := makeExecConfig(options, ic.Libpod) + if err != nil { + return ec, err + } ec, err = terminal.ExecAttachCtr(ctx, ctr, execConfig, &streams) return define.TranslateExecErrorToExitCode(ec, err), err @@ -664,20 +680,10 @@ func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrID s } ctr := ctrs[0] - execConfig := makeExecConfig(options) - - // Make an exit command - storageConfig := ic.Libpod.StorageConfig() - runtimeConfig, err := ic.Libpod.GetConfig() - if err != nil { - return "", errors.Wrapf(err, "error retrieving Libpod configuration to build exec exit command") - } - // TODO: Add some ability to toggle syslog - exitCommandArgs, err := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, false, true, true) + execConfig, err := makeExecConfig(options, ic.Libpod) if err != nil { - return "", errors.Wrapf(err, "error constructing exit command for exec session") + return "", err } - execConfig.ExitCommand = exitCommandArgs // Create and start the exec session id, err := ctr.ExecCreate(execConfig) diff --git a/pkg/domain/infra/abi/parse/parse.go b/pkg/domain/infra/abi/parse/parse.go index 1c590d2d6..56c747711 100644 --- a/pkg/domain/infra/abi/parse/parse.go +++ b/pkg/domain/infra/abi/parse/parse.go @@ -37,7 +37,7 @@ func VolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error) return nil, errors.Wrapf(err, "cannot convert UID %s to integer", splitO[1]) } logrus.Debugf("Removing uid= from options and adding WithVolumeUID for UID %d", intUID) - libpodOptions = append(libpodOptions, libpod.WithVolumeUID(intUID)) + libpodOptions = append(libpodOptions, libpod.WithVolumeUID(intUID), libpod.WithVolumeNoChown()) finalVal = append(finalVal, o) // set option "UID": "$uid" volumeOptions["UID"] = splitO[1] @@ -50,7 +50,7 @@ func VolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error) return nil, errors.Wrapf(err, "cannot convert GID %s to integer", splitO[1]) } logrus.Debugf("Removing gid= from options and adding WithVolumeGID for GID %d", intGID) - libpodOptions = append(libpodOptions, libpod.WithVolumeGID(intGID)) + libpodOptions = append(libpodOptions, libpod.WithVolumeGID(intGID), libpod.WithVolumeNoChown()) finalVal = append(finalVal, o) // set option "GID": "$gid" volumeOptions["GID"] = splitO[1] diff --git a/test/compose/test-compose b/test/compose/test-compose index 981f78a79..70db6dd55 100755 --- a/test/compose/test-compose +++ b/test/compose/test-compose @@ -183,6 +183,8 @@ function test_port() { fi echo "# cat $WORKDIR/server.log:" cat $WORKDIR/server.log + echo "# cat $logfile:" + cat $logfile return fi diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index 9b77aaef8..4be1b2009 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -668,4 +668,36 @@ USER testuser`, fedoraMinimal) Expect(strings.Contains(test2.OutputToString(), testString)).To(BeTrue()) }) + + It("podman volume with uid and gid works", func() { + volName := "testVol" + volCreate := podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=1000", volName}) + volCreate.WaitWithDefaultTimeout() + Expect(volCreate.ExitCode()).To(Equal(0)) + + volMount := podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%u", "/test"}) + volMount.WaitWithDefaultTimeout() + Expect(volMount.ExitCode()).To(Equal(0)) + Expect(volMount.OutputToString()).To(Equal("1000")) + + volName = "testVol2" + volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=gid=1000", volName}) + volCreate.WaitWithDefaultTimeout() + Expect(volCreate.ExitCode()).To(Equal(0)) + + volMount = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%g", "/test"}) + volMount.WaitWithDefaultTimeout() + Expect(volMount.ExitCode()).To(Equal(0)) + Expect(volMount.OutputToString()).To(Equal("1000")) + + volName = "testVol3" + volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=1000,gid=1000", volName}) + volCreate.WaitWithDefaultTimeout() + Expect(volCreate.ExitCode()).To(Equal(0)) + + volMount = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%u:%g", "/test"}) + volMount.WaitWithDefaultTimeout() + Expect(volMount.ExitCode()).To(Equal(0)) + Expect(volMount.OutputToString()).To(Equal("1000:1000")) + }) }) |