From 0d73ee40b2e95ec7d50c7ee72fcb24cae0e190a7 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 1 Apr 2019 15:22:32 -0400 Subject: Add container restart policy to Libpod & Podman This initial version does not support restart count, but it works as advertised otherwise. Signed-off-by: Matthew Heon --- libpod/container_api.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'libpod/container_api.go') diff --git a/libpod/container_api.go b/libpod/container_api.go index 5bfd869b3..46c913e99 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -583,6 +583,7 @@ func (c *Container) Cleanup(ctx context.Context) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() + if err := c.syncContainer(); err != nil { return err } @@ -593,6 +594,24 @@ func (c *Container) Cleanup(ctx context.Context) error { return errors.Wrapf(ErrCtrStateInvalid, "container %s is running or paused, refusing to clean up", c.ID()) } + // If we have a restart policy match when updating the state, we need to + // restart the container. + // However, perform a full validation of restart policy first. + if c.state.RestartPolicyMatch { + // if restart policy is on-error and exit code is 0, we do + // nothing. + // TODO: if restart retries is set, handle that here. + if c.config.RestartRetries != 0 { + return errors.Wrapf(ErrNotImplemented, "restart retries not yet implemented") + } + if (c.config.RestartPolicy == "on-error" && c.state.ExitCode == 0) || c.config.RestartPolicy == "always" { + // The container stopped. We need to restart it. + return c.handleRestartPolicy(ctx) + } + } + + // If we aren't hitting restart policy, we perform a normal cleanup + // Check if we have active exec sessions if len(c.state.ExecSessions) != 0 { return errors.Wrapf(ErrCtrStateInvalid, "container %s has active exec sessions, refusing to clean up", c.ID()) -- cgit v1.2.3-54-g00ecf From dc42304f3804632d01345478ab9b1f122b48d516 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 1 Apr 2019 18:17:16 -0400 Subject: Sending signals to containers prevents restart policy Noticed this when testing some behavior with Docker. Signed-off-by: Matthew Heon --- libpod/container_api.go | 9 ++++++++- libpod/pod_api.go | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'libpod/container_api.go') diff --git a/libpod/container_api.go b/libpod/container_api.go index 46c913e99..8b3dd4186 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -199,8 +199,15 @@ func (c *Container) Kill(signal uint) error { if c.state.State != ContainerStateRunning { return errors.Wrapf(ErrCtrStateInvalid, "can only kill running containers") } + defer c.newContainerEvent(events.Kill) - return c.runtime.ociRuntime.killContainer(c, signal) + if err := c.runtime.ociRuntime.killContainer(c, signal); err != nil { + return err + } + + c.state.StoppedByUser = true + + return c.save() } // Exec starts a new process inside the container diff --git a/libpod/pod_api.go b/libpod/pod_api.go index 9a6baf23e..9ed5c88eb 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -364,6 +364,13 @@ func (p *Pod) Kill(signal uint) (map[string]error, error) { } logrus.Debugf("Killed container %s with signal %d", ctr.ID(), signal) + + ctr.state.StoppedByUser = true + if err := ctr.save(); err != nil { + ctrErrors[ctr.ID()] = err + } + + ctr.lock.Unlock() } if len(ctrErrors) > 0 { -- cgit v1.2.3-54-g00ecf From f4db6d5cf61741f9b0de163b158ecdc2bcfa6669 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 1 Apr 2019 19:20:03 -0400 Subject: Add support for retry count with --restart flag The on-failure restart option supports restarting only a given number of times. To do this, we need one additional field in the DB to track restart count (which conveniently fills a field in Inspect we weren't populating), plus some plumbing logic. Signed-off-by: Matthew Heon --- libpod/container.go | 4 ++++ libpod/container_api.go | 18 +++++++++--------- libpod/container_inspect.go | 1 + libpod/container_internal.go | 34 +++++++++++++++++++++++----------- pkg/inspect/inspect.go | 2 +- pkg/spec/createconfig.go | 3 +-- 6 files changed, 39 insertions(+), 23 deletions(-) (limited to 'libpod/container_api.go') diff --git a/libpod/container.go b/libpod/container.go index 4236f2456..769c4f69c 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -185,6 +185,10 @@ type ContainerState struct { // RestartPolicyMatch indicates whether the conditions for restart // policy have been met. RestartPolicyMatch bool `json:"restartPolicyMatch,omitempty"` + // RestartCount is how many times the container was restarted by its + // restart policy. This is NOT incremented by normal container restarts + // (only by restart policy). + RestartCount uint `json:"restartCount,omitempty"` // ExtensionStageHooks holds hooks which will be executed by libpod // and not delegated to the OCI runtime. diff --git a/libpod/container_api.go b/libpod/container_api.go index 8b3dd4186..7132a82c2 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -57,11 +57,11 @@ func (c *Container) Init(ctx context.Context) (err error) { if c.state.State == ContainerStateStopped { // Reinitialize the container - return c.reinit(ctx) + return c.reinit(ctx, false) } // Initialize the container for the first time - return c.init(ctx) + return c.init(ctx, false) } // Start starts a container. @@ -605,13 +605,13 @@ func (c *Container) Cleanup(ctx context.Context) error { // restart the container. // However, perform a full validation of restart policy first. if c.state.RestartPolicyMatch { - // if restart policy is on-error and exit code is 0, we do - // nothing. - // TODO: if restart retries is set, handle that here. - if c.config.RestartRetries != 0 { - return errors.Wrapf(ErrNotImplemented, "restart retries not yet implemented") + if c.config.RestartPolicy == "on-failure" && c.state.ExitCode != 0 { + logrus.Debugf("Container %s restart policy trigger: on retry %d (of %d)", + c.ID(), c.state.RestartCount, c.config.RestartRetries) } - if (c.config.RestartPolicy == "on-error" && c.state.ExitCode == 0) || c.config.RestartPolicy == "always" { + if (c.config.RestartPolicy == "on-failure" && c.state.ExitCode != 0 && + (c.config.RestartRetries > 0 && c.state.RestartCount < c.config.RestartRetries)) || + c.config.RestartPolicy == "always" { // The container stopped. We need to restart it. return c.handleRestartPolicy(ctx) } @@ -780,7 +780,7 @@ func (c *Container) Refresh(ctx context.Context) error { if err := c.prepare(); err != nil { return err } - if err := c.init(ctx); err != nil { + if err := c.init(ctx, false); err != nil { return err } } diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index aa3a07888..a7369bfdd 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -95,6 +95,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *inspect.Data) LogPath: config.LogPath, ConmonPidFile: config.ConmonPidFile, Name: config.Name, + RestartCount: int32(runtimeInfo.RestartCount), Driver: driverData.Name, MountLabel: config.MountLabel, ProcessLabel: config.ProcessLabel, diff --git a/libpod/container_internal.go b/libpod/container_internal.go index f72d86c84..b1b7a090f 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -221,6 +221,13 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (err error) { return err } + // Increment restart count + c.state.RestartCount = c.state.RestartCount + 1 + logrus.Debugf("Container %s now on retry %d", c.ID(), c.state.RestartCount) + if err := c.save(); err != nil { + return err + } + defer func() { if err != nil { if err2 := c.cleanup(ctx); err2 != nil { @@ -234,13 +241,13 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (err error) { if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to - if err := c.reinit(ctx); err != nil { + if err := c.reinit(ctx, true); err != nil { return err } } else if c.state.State == ContainerStateConfigured || c.state.State == ContainerStateExited { // Initialize the container - if err := c.init(ctx); err != nil { + if err := c.init(ctx, true); err != nil { return err } } @@ -423,6 +430,7 @@ func resetState(state *ContainerState) error { state.BindMounts = make(map[string]string) state.StoppedByUser = false state.RestartPolicyMatch = false + state.RestartCount = 0 return nil } @@ -616,13 +624,13 @@ func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err err if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to - if err := c.reinit(ctx); err != nil { + if err := c.reinit(ctx, false); err != nil { return err } } else if c.state.State == ContainerStateConfigured || c.state.State == ContainerStateExited { // Or initialize it if necessary - if err := c.init(ctx); err != nil { + if err := c.init(ctx, false); err != nil { return err } } @@ -810,7 +818,7 @@ func (c *Container) completeNetworkSetup() error { } // Initialize a container, creating it in the runtime -func (c *Container) init(ctx context.Context) error { +func (c *Container) init(ctx context.Context, retainRetries bool) error { span, _ := opentracing.StartSpanFromContext(ctx, "init") span.SetTag("struct", "container") defer span.Finish() @@ -839,6 +847,10 @@ func (c *Container) init(ctx context.Context) error { c.state.StoppedByUser = false c.state.RestartPolicyMatch = false + if !retainRetries { + c.state.RestartCount = 0 + } + if err := c.save(); err != nil { return err } @@ -900,7 +912,7 @@ func (c *Container) cleanupRuntime(ctx context.Context) error { // Should only be done on ContainerStateStopped containers. // Not necessary for ContainerStateExited - the container has already been // removed from the runtime, so init() can proceed freely. -func (c *Container) reinit(ctx context.Context) error { +func (c *Container) reinit(ctx context.Context, retainRetries bool) error { span, _ := opentracing.StartSpanFromContext(ctx, "reinit") span.SetTag("struct", "container") defer span.Finish() @@ -912,7 +924,7 @@ func (c *Container) reinit(ctx context.Context) error { } // Initialize the container again - return c.init(ctx) + return c.init(ctx, retainRetries) } // Initialize (if necessary) and start a container @@ -950,12 +962,12 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { if c.state.State == ContainerStateStopped { logrus.Debugf("Recreating container %s in OCI runtime", c.ID()) - if err := c.reinit(ctx); err != nil { + if err := c.reinit(ctx, false); err != nil { return err } } else if c.state.State == ContainerStateConfigured || c.state.State == ContainerStateExited { - if err := c.init(ctx); err != nil { + if err := c.init(ctx, false); err != nil { return err } } @@ -1058,13 +1070,13 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to - if err := c.reinit(ctx); err != nil { + if err := c.reinit(ctx, false); err != nil { return err } } else if c.state.State == ContainerStateConfigured || c.state.State == ContainerStateExited { // Initialize the container - if err := c.init(ctx); err != nil { + if err := c.init(ctx, false); err != nil { return err } } diff --git a/pkg/inspect/inspect.go b/pkg/inspect/inspect.go index 6978370ef..693755aa8 100644 --- a/pkg/inspect/inspect.go +++ b/pkg/inspect/inspect.go @@ -161,7 +161,7 @@ type ContainerInspectData struct { LogPath string `json:"LogPath"` ConmonPidFile string `json:"ConmonPidFile"` Name string `json:"Name"` - RestartCount int32 `json:"RestartCount"` //TODO + RestartCount int32 `json:"RestartCount"` Driver string `json:"Driver"` MountLabel string `json:"MountLabel"` ProcessLabel string `json:"ProcessLabel"` diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index f1861dd19..9979e773c 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -376,8 +376,7 @@ func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *l } options = append(options, libpod.WithRestartRetries(uint(numTries))) } - - options = append(options, libpod.WithRestartPolicy(c.RestartPolicy)) + options = append(options, libpod.WithRestartPolicy(split[0])) } // Always use a cleanup process to clean up Podman after termination -- cgit v1.2.3-54-g00ecf From 7ba1b609aad678f458951c978455ea44c5b4d3ec Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 1 Apr 2019 19:45:23 -0400 Subject: Move to using constants for valid restart policy types Signed-off-by: Matthew Heon --- libpod/container.go | 14 ++++++++++++++ libpod/container_api.go | 6 +++--- libpod/container_internal.go | 2 +- libpod/options.go | 2 +- 4 files changed, 19 insertions(+), 5 deletions(-) (limited to 'libpod/container_api.go') diff --git a/libpod/container.go b/libpod/container.go index 769c4f69c..c07f4c78d 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -102,6 +102,20 @@ func (ns LinuxNS) String() string { } } +// Valid restart policy types. +const ( + // RestartPolicyNone indicates that no restart policy has been requested + // by a container. + RestartPolicyNone = "" + // RestartPolicyNo is identical in function to RestartPolicyNone. + RestartPolicyNo = "no" + // RestartPolicyAlways unconditionally restarts the container. + RestartPolicyAlways = "always" + // RestartPolicyOnFailure restarts the container on non-0 exit code, + // with an optional maximum number of retries. + RestartPolicyOnFailure = "on-failure" +) + // Container is a single OCI container. // All operations on a Container that access state must begin with a call to // syncContainer(). diff --git a/libpod/container_api.go b/libpod/container_api.go index 7132a82c2..faf01eccb 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -605,13 +605,13 @@ func (c *Container) Cleanup(ctx context.Context) error { // restart the container. // However, perform a full validation of restart policy first. if c.state.RestartPolicyMatch { - if c.config.RestartPolicy == "on-failure" && c.state.ExitCode != 0 { + if c.config.RestartPolicy == RestartPolicyOnFailure && c.state.ExitCode != 0 { logrus.Debugf("Container %s restart policy trigger: on retry %d (of %d)", c.ID(), c.state.RestartCount, c.config.RestartRetries) } - if (c.config.RestartPolicy == "on-failure" && c.state.ExitCode != 0 && + if (c.config.RestartPolicy == RestartPolicyOnFailure && c.state.ExitCode != 0 && (c.config.RestartRetries > 0 && c.state.RestartCount < c.config.RestartRetries)) || - c.config.RestartPolicy == "always" { + c.config.RestartPolicy == RestartPolicyAlways { // The container stopped. We need to restart it. return c.handleRestartPolicy(ctx) } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index b1b7a090f..76de37a09 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -275,7 +275,7 @@ func (c *Container) syncContainer() error { // Only save back to DB if state changed if c.state.State != oldState { // Check for a restart policy match - if c.config.RestartPolicy != "" && c.config.RestartPolicy != "no" && + if c.config.RestartPolicy != RestartPolicyNone && c.config.RestartPolicy != RestartPolicyNo && (oldState == ContainerStateRunning || oldState == ContainerStatePaused) && (c.state.State == ContainerStateStopped || c.state.State == ContainerStateExited) && !c.state.StoppedByUser { diff --git a/libpod/options.go b/libpod/options.go index e83515822..7ec7dfe63 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1249,7 +1249,7 @@ func WithRestartPolicy(policy string) CtrCreateOption { } switch policy { - case "", "no", "on-failure", "always": + case RestartPolicyNone, RestartPolicyNo, RestartPolicyOnFailure, RestartPolicyAlways: ctr.config.RestartPolicy = policy default: return errors.Wrapf(ErrInvalidArg, "%q is not a valid restart policy", policy) -- cgit v1.2.3-54-g00ecf From d7c367aa61850936249fc99461a92185cb6ba8aa Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 3 May 2019 10:35:48 -0400 Subject: Address review comments on restart policy Signed-off-by: Matthew Heon --- cmd/podman/shared/create.go | 2 +- docs/podman-create.1.md | 19 ++---------------- docs/podman-run.1.md | 4 ++-- libpod/container_api.go | 25 +++++++++-------------- libpod/container_internal.go | 48 +++++++++++++++++++++++++++++++++++--------- 5 files changed, 54 insertions(+), 44 deletions(-) (limited to 'libpod/container_api.go') diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index d2bd23e3e..d1f704374 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -42,7 +42,7 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod. defer span.Finish() } if c.Bool("rm") && c.String("restart") != "" && c.String("restart") != "no" { - return nil, nil, errors.Errorf("the --rm flag conflicts with --restart") + return nil, nil, errors.Errorf("the --rm option conflicts with --restart") } rtc, err := runtime.GetConfig() diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md index abdfe9f8b..3d9b9ca7d 100644 --- a/docs/podman-create.1.md +++ b/docs/podman-create.1.md @@ -576,8 +576,8 @@ Valid values are: - `always` : Restart containers when they exit, regardless of status, retrying indefinitely Please note that restart will not restart containers after a system reboot. -If you require this functionality, please add your Podman commands to a systemd unit file, or create an init script for your init system of choice. -There is an example of restarting a container with systemd below. +This this functionality is required in your environment, you can invoke Podman from a systemd unit file, or create an init script for whichever init system is in use. +To generate systemd unit files, please see *podman generate systemd* **--rm**=*true*|*false* @@ -865,21 +865,6 @@ the uids and gids from the host. $ podman create --uidmap 0:30000:7000 --gidmap 0:30000:7000 fedora echo hello ``` -### Running a podman container to restart inside of a systemd unit file - - -``` -[Unit] -Description=My App -[Service] -Restart=always -ExecStart=/usr/bin/podman start -a my_app -ExecStop=/usr/bin/podman stop -t 10 my_app -KillMode=process -[Install] -WantedBy=multi-user.target -``` - ### Rootless Containers Podman runs as a non root user on most systems. This feature requires that a new enough version of shadow-utils diff --git a/docs/podman-run.1.md b/docs/podman-run.1.md index 078e388ea..f9dee4713 100644 --- a/docs/podman-run.1.md +++ b/docs/podman-run.1.md @@ -598,8 +598,8 @@ Valid values are: - `always` : Restart containers when they exit, regardless of status, retrying indefinitely Please note that restart will not restart containers after a system reboot. -If you require this functionality, please add your Podman commands to a systemd unit file, or create an init script for your init system of choice. -To manage container restart via systemd, see *podman generate systemd*. +This this functionality is required in your environment, you can invoke Podman from a systemd unit file, or create an init script for whichever init system is in use. +To generate systemd unit files, please see *podman generate systemd* **--rm**=*true*|*false* diff --git a/libpod/container_api.go b/libpod/container_api.go index faf01eccb..5bb610aab 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -601,23 +601,18 @@ func (c *Container) Cleanup(ctx context.Context) error { return errors.Wrapf(ErrCtrStateInvalid, "container %s is running or paused, refusing to clean up", c.ID()) } - // If we have a restart policy match when updating the state, we need to - // restart the container. - // However, perform a full validation of restart policy first. - if c.state.RestartPolicyMatch { - if c.config.RestartPolicy == RestartPolicyOnFailure && c.state.ExitCode != 0 { - logrus.Debugf("Container %s restart policy trigger: on retry %d (of %d)", - c.ID(), c.state.RestartCount, c.config.RestartRetries) - } - if (c.config.RestartPolicy == RestartPolicyOnFailure && c.state.ExitCode != 0 && - (c.config.RestartRetries > 0 && c.state.RestartCount < c.config.RestartRetries)) || - c.config.RestartPolicy == RestartPolicyAlways { - // The container stopped. We need to restart it. - return c.handleRestartPolicy(ctx) - } + // Handle restart policy. + // Returns a bool indicating whether we actually restarted. + // If we did, don't proceed to cleanup - just exit. + didRestart, err := c.handleRestartPolicy(ctx) + if err != nil { + return err + } + if didRestart { + return nil } - // If we aren't hitting restart policy, we perform a normal cleanup + // If we didn't restart, we perform a normal cleanup // Check if we have active exec sessions if len(c.state.ExecSessions) != 0 { diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 00f916d0f..0dadcd1ca 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -213,20 +213,47 @@ func (c *Container) handleExitFile(exitFile string, fi os.FileInfo) error { // Handle container restart policy. // This is called when a container has exited, and was not explicitly stopped by // an API call to stop the container or pod it is in. -func (c *Container) handleRestartPolicy(ctx context.Context) (err error) { +func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, err error) { + // If we did not get a restart policy match, exit immediately. + // Do the same if we're not a policy that restarts. + if !c.state.RestartPolicyMatch || + c.config.RestartPolicy == RestartPolicyNo || + c.config.RestartPolicy == RestartPolicyNone { + return false, nil + } + + // If we're RestartPolicyOnFailure, we need to check retries and exit + // code. + if c.config.RestartPolicy == RestartPolicyOnFailure { + if c.state.ExitCode == 0 { + return false, nil + } + + // If we don't have a max retries set, continue + if c.config.RestartRetries > 0 { + if c.state.RestartCount < c.config.RestartRetries { + logrus.Debugf("Container %s restart policy trigger: on retry %d (of %d)", + c.ID(), c.state.RestartCount, c.config.RestartRetries) + } else { + logrus.Debugf("Container %s restart policy trigger: retries exhausted") + return false, nil + } + } + } + logrus.Debugf("Restarting container %s due to restart policy %s", c.ID(), c.config.RestartPolicy) // Need to check if dependencies are alive. if err = c.checkDependenciesAndHandleError(ctx); err != nil { - return err + return false, err } // Is the container running again? // If so, we don't have to do anything if c.state.State == ContainerStateRunning || c.state.State == ContainerStatePaused { - return nil + return false, nil } else if c.state.State == ContainerStateUnknown { - return errors.Wrapf(ErrInternal, "invalid container state encountered in restart attempt!") + return false, errors.Wrapf(ErrInternal, "invalid container state encountered in restart attempt!") } c.newContainerEvent(events.Restart) @@ -235,7 +262,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (err error) { c.state.RestartCount = c.state.RestartCount + 1 logrus.Debugf("Container %s now on retry %d", c.ID(), c.state.RestartCount) if err := c.save(); err != nil { - return err + return false, err } defer func() { @@ -246,22 +273,25 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (err error) { } }() if err := c.prepare(); err != nil { - return err + return false, err } if c.state.State == ContainerStateStopped { // Reinitialize the container if we need to if err := c.reinit(ctx, true); err != nil { - return err + return false, err } } else if c.state.State == ContainerStateConfigured || c.state.State == ContainerStateExited { // Initialize the container if err := c.init(ctx, true); err != nil { - return err + return false, err } } - return c.start() + if err := c.start(); err != nil { + return false, err + } + return true, nil } // Sync this container with on-disk state and runtime status -- cgit v1.2.3-54-g00ecf