aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--cmd/podman/common/createparse.go2
-rw-r--r--libpod/container_api.go14
-rw-r--r--libpod/container_internal.go28
-rw-r--r--pkg/api/handlers/libpod/containers.go24
-rw-r--r--pkg/bindings/containers/containers.go12
-rw-r--r--pkg/domain/infra/abi/containers.go15
-rw-r--r--pkg/domain/infra/tunnel/containers.go25
-rw-r--r--test/e2e/run_test.go4
8 files changed, 99 insertions, 25 deletions
diff --git a/cmd/podman/common/createparse.go b/cmd/podman/common/createparse.go
index 09ee5aa0c..3a69f11b6 100644
--- a/cmd/podman/common/createparse.go
+++ b/cmd/podman/common/createparse.go
@@ -9,7 +9,7 @@ import (
// by validate must not need any state information on the flag (i.e. changed)
func (c *ContainerCLIOpts) validate() error {
var ()
- if c.Rm && c.Restart != "" && c.Restart != "no" {
+ if c.Rm && (c.Restart != "" && c.Restart != "no" && c.Restart != "on-failure") {
return errors.Errorf(`the --rm option conflicts with --restart, when the restartPolicy is not "" and "no"`)
}
diff --git a/libpod/container_api.go b/libpod/container_api.go
index a9808a30e..6a7ddc421 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -714,3 +714,17 @@ func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOpti
defer c.newContainerEvent(events.Restore)
return c.restore(ctx, options)
}
+
+// Indicate whether or not the container should restart
+func (c *Container) ShouldRestart(ctx context.Context) bool {
+ logrus.Debugf("Checking if container %s should restart", c.ID())
+ if !c.batched {
+ c.lock.Lock()
+ defer c.lock.Unlock()
+
+ if err := c.syncContainer(); err != nil {
+ return false
+ }
+ }
+ return c.shouldRestart()
+}
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 108954bad..ffd0dc92f 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -206,37 +206,39 @@ func (c *Container) handleExitFile(exitFile string, fi os.FileInfo) error {
return nil
}
-// 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) (_ bool, retErr error) {
- // If we did not get a restart policy match, exit immediately.
+func (c *Container) shouldRestart() bool {
+ // If we did not get a restart policy match, return false
// 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
+ return false
}
// 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
+ return false
}
// 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", c.ID())
- return false, nil
+ if c.state.RestartCount >= c.config.RestartRetries {
+ return false
}
}
}
+ return true
+}
+// 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) (_ bool, retErr error) {
+ if !c.shouldRestart() {
+ 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.
diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go
index 7e6481321..14eb44831 100644
--- a/pkg/api/handlers/libpod/containers.go
+++ b/pkg/api/handlers/libpod/containers.go
@@ -344,3 +344,27 @@ func InitContainer(w http.ResponseWriter, r *http.Request) {
}
utils.WriteResponse(w, http.StatusNoContent, "")
}
+
+func ShouldRestart(w http.ResponseWriter, r *http.Request) {
+ runtime := r.Context().Value("runtime").(*libpod.Runtime)
+ // Now use the ABI implementation to prevent us from having duplicate
+ // code.
+ containerEngine := abi.ContainerEngine{Libpod: runtime}
+
+ name := utils.GetName(r)
+ report, err := containerEngine.ShouldRestart(r.Context(), name)
+ if err != nil {
+ if errors.Cause(err) == define.ErrNoSuchCtr {
+ utils.ContainerNotFound(w, name, err)
+ return
+ }
+ utils.InternalServerError(w, err)
+ return
+
+ }
+ if report.Value {
+ utils.WriteResponse(w, http.StatusNoContent, "")
+ } else {
+ utils.ContainerNotFound(w, name, define.ErrNoSuchCtr)
+ }
+}
diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go
index b5cd2128b..4331ae6c2 100644
--- a/pkg/bindings/containers/containers.go
+++ b/pkg/bindings/containers/containers.go
@@ -390,3 +390,15 @@ func ContainerInit(ctx context.Context, nameOrID string) error {
}
return response.Process(nil)
}
+
+func ShouldRestart(ctx context.Context, nameOrID string) (bool, error) {
+ conn, err := bindings.GetClient(ctx)
+ if err != nil {
+ return false, err
+ }
+ response, err := conn.DoRequest(nil, http.MethodPost, "/containers/%s/shouldrestart", nil, nil, nameOrID)
+ if err != nil {
+ return false, err
+ }
+ return response.IsSuccess(), nil
+}
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index 4b69ac74e..ff4277a2e 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -911,7 +911,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
} else {
report.ExitCode = int(ecode)
}
- if opts.Rm {
+ if opts.Rm && !ctr.ShouldRestart(ctx) {
if err := ic.Libpod.RemoveContainer(ctx, ctr, false, true); err != nil {
if errors.Cause(err) == define.ErrNoSuchCtr ||
errors.Cause(err) == define.ErrCtrRemoved {
@@ -992,7 +992,7 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st
return []*entities.ContainerCleanupReport{}, nil
}
- if options.Remove {
+ if options.Remove && !ctr.ShouldRestart(ctx) {
err = ic.Libpod.RemoveContainer(ctx, ctr, false, true)
if err != nil {
report.RmErr = errors.Wrapf(err, "failed to cleanup and remove container %v", ctr.ID())
@@ -1015,6 +1015,7 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st
_, err = ic.Libpod.RemoveImage(ctx, ctrImage, false)
report.RmiErr = err
}
+
reports = append(reports, &report)
}
return reports, nil
@@ -1314,3 +1315,13 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri
return statsChan, nil
}
+
+// ShouldRestart returns whether the container should be restarted
+func (ic *ContainerEngine) ShouldRestart(ctx context.Context, nameOrID string) (*entities.BoolReport, error) {
+ ctr, err := ic.Libpod.LookupContainer(nameOrID)
+ if err != nil {
+ return nil, err
+ }
+
+ return &entities.BoolReport{Value: ctr.ShouldRestart(ctx)}, nil
+}
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index 8066e1c00..1aa5afbe7 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -595,12 +595,20 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
// Defer the removal, so we can return early if needed and
// de-spaghetti the code.
defer func() {
- if err := containers.Remove(ic.ClientCxt, con.ID, bindings.PFalse, bindings.PTrue); err != nil {
- if errorhandling.Contains(err, define.ErrNoSuchCtr) ||
- errorhandling.Contains(err, define.ErrCtrRemoved) {
- logrus.Warnf("Container %s does not exist: %v", con.ID, err)
- } else {
- logrus.Errorf("Error removing container %s: %v", con.ID, err)
+ shouldRestart, err := containers.ShouldRestart(ic.ClientCxt, con.ID)
+ if err != nil {
+ logrus.Errorf("Failed to check if %s should restart: %v", con.ID, err)
+ return
+ }
+
+ if !shouldRestart {
+ if err := containers.Remove(ic.ClientCxt, con.ID, bindings.PFalse, bindings.PTrue); err != nil {
+ if errorhandling.Contains(err, define.ErrNoSuchCtr) ||
+ errorhandling.Contains(err, define.ErrCtrRemoved) {
+ logrus.Warnf("Container %s does not exist: %v", con.ID, err)
+ } else {
+ logrus.Errorf("Error removing container %s: %v", con.ID, err)
+ }
}
}
}()
@@ -737,3 +745,8 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri
}
return containers.Stats(ic.ClientCxt, namesOrIds, &options.Stream)
}
+
+// ShouldRestart reports back whether the containre will restart
+func (ic *ContainerEngine) ShouldRestart(_ context.Context, id string) (bool, error) {
+ return containers.ShouldRestart(ic.ClientCxt, id)
+}
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 5ee85efb9..0d65a3e59 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -75,11 +75,9 @@ var _ = Describe("Podman run", func() {
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
- // the --rm option conflicts with --restart, when the restartPolicy is not "" and "no"
- // so the exitCode should not equal 0
session = podmanTest.Podman([]string{"run", "--rm", "--restart", "on-failure", ALPINE})
session.WaitWithDefaultTimeout()
- Expect(session.ExitCode()).To(Not(Equal(0)))
+ Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"run", "--rm", "--restart", "always", ALPINE})
session.WaitWithDefaultTimeout()