diff options
-rw-r--r-- | contrib/systemd/auto-update/podman-auto-update.timer | 1 | ||||
-rw-r--r-- | pkg/domain/infra/abi/containers.go | 47 | ||||
-rw-r--r-- | pkg/domain/infra/tunnel/containers.go | 26 | ||||
-rw-r--r-- | test/e2e/start_test.go | 1 | ||||
-rw-r--r-- | test/python/docker/compat/test_containers.py | 1 | ||||
-rw-r--r-- | test/system/070-build.bats | 3 | ||||
-rw-r--r-- | test/system/120-load.bats | 17 |
7 files changed, 64 insertions, 32 deletions
diff --git a/contrib/systemd/auto-update/podman-auto-update.timer b/contrib/systemd/auto-update/podman-auto-update.timer index 3e50ffa9b..50be391d3 100644 --- a/contrib/systemd/auto-update/podman-auto-update.timer +++ b/contrib/systemd/auto-update/podman-auto-update.timer @@ -3,6 +3,7 @@ Description=Podman auto-update timer [Timer] OnCalendar=daily +RandomizedDelaySec=900 Persistent=true [Install] diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 2c5300ccb..50751aa12 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -261,6 +261,24 @@ func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []st return reports, nil } +func (ic *ContainerEngine) removeContainer(ctx context.Context, ctr *libpod.Container, options entities.RmOptions) error { + err := ic.Libpod.RemoveContainer(ctx, ctr, options.Force, options.Volumes) + if err == nil { + return nil + } + logrus.Debugf("Failed to remove container %s: %s", ctr.ID(), err.Error()) + switch errors.Cause(err) { + case define.ErrNoSuchCtr: + if options.Ignore { + logrus.Debugf("Ignoring error (--allow-missing): %v", err) + return nil + } + case define.ErrCtrRemoved: + return nil + } + return err +} + func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, options entities.RmOptions) ([]*entities.RmReport, error) { reports := []*entities.RmReport{} @@ -318,21 +336,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, } errMap, err := parallelctr.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { - err := ic.Libpod.RemoveContainer(ctx, c, options.Force, options.Volumes) - if err == nil { - return nil - } - logrus.Debugf("Failed to remove container %s: %s", c.ID(), err.Error()) - switch errors.Cause(err) { - case define.ErrNoSuchCtr: - if options.Ignore { - logrus.Debugf("Ignoring error (--allow-missing): %v", err) - return nil - } - case define.ErrCtrRemoved: - return nil - } - return err + return ic.removeContainer(ctx, c, options) }) if err != nil { return nil, err @@ -791,6 +795,11 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri Err: err, ExitCode: exitCode, }) + if ctr.AutoRemove() { + if err := ic.removeContainer(ctx, ctr, entities.RmOptions{}); err != nil { + logrus.Errorf("Error removing container %s: %v", ctr.ID(), err) + } + } return reports, errors.Wrapf(err, "unable to start container %s", ctr.ID()) } @@ -827,9 +836,6 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri ExitCode: 125, } if err := ctr.Start(ctx, true); err != nil { - // if lastError != nil { - // fmt.Fprintln(os.Stderr, lastError) - // } report.Err = err if errors.Cause(err) == define.ErrWillDeadlock { report.Err = errors.Wrapf(err, "please run 'podman system renumber' to resolve deadlocks") @@ -838,6 +844,11 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri } report.Err = errors.Wrapf(err, "unable to start container %q", ctr.ID()) reports = append(reports, report) + if ctr.AutoRemove() { + if err := ic.removeContainer(ctx, ctr, entities.RmOptions{}); err != nil { + logrus.Errorf("Error removing container %s: %v", ctr.ID(), err) + } + } continue } report.ExitCode = 0 diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 56315f46f..c17d7b54f 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -541,6 +541,17 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri return nil, err } removeOptions := new(containers.RemoveOptions).WithVolumes(true).WithForce(false) + removeContainer := func(id string) { + if err := containers.Remove(ic.ClientCtx, id, removeOptions); err != nil { + if errorhandling.Contains(err, define.ErrNoSuchCtr) || + errorhandling.Contains(err, define.ErrCtrRemoved) { + logrus.Debugf("Container %s does not exist: %v", id, err) + } else { + logrus.Errorf("Error removing container %s: %v", id, err) + } + } + } + // There can only be one container if attach was used for i, ctr := range ctrs { name := ctr.ID @@ -568,6 +579,9 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri } if err != nil { + if ctr.AutoRemove { + removeContainer(ctr.ID) + } report.ExitCode = define.ExitCode(report.Err) report.Err = err reports = append(reports, &report) @@ -582,16 +596,10 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri logrus.Errorf("Failed to check if %s should restart: %v", ctr.ID, err) return } + logrus.Errorf("Should restart: %v", shouldRestart) - if !shouldRestart { - if err := containers.Remove(ic.ClientCtx, ctr.ID, removeOptions); err != nil { - if errorhandling.Contains(err, define.ErrNoSuchCtr) || - errorhandling.Contains(err, define.ErrCtrRemoved) { - logrus.Debugf("Container %s does not exist: %v", ctr.ID, err) - } else { - logrus.Errorf("Error removing container %s: %v", ctr.ID, err) - } - } + if !shouldRestart && ctr.AutoRemove { + removeContainer(ctr.ID) } }() } diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go index 7799055d9..073631ef7 100644 --- a/test/e2e/start_test.go +++ b/test/e2e/start_test.go @@ -65,7 +65,6 @@ var _ = Describe("Podman start", func() { }) It("podman start --rm --attach removed on failure", func() { - Skip("FIXME: #10935, race condition removing container") session := podmanTest.Podman([]string{"create", "--rm", ALPINE, "foo"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) diff --git a/test/python/docker/compat/test_containers.py b/test/python/docker/compat/test_containers.py index 38ac5b59f..9fcdf49ea 100644 --- a/test/python/docker/compat/test_containers.py +++ b/test/python/docker/compat/test_containers.py @@ -206,6 +206,7 @@ class TestContainers(unittest.TestCase): self.assertEqual(len(ctnrs), 1) def test_copy_to_container(self): + self.skipTest("FIXME: #10948 - test is broken") ctr: Optional[Container] = None try: test_file_content = b"Hello World!" diff --git a/test/system/070-build.bats b/test/system/070-build.bats index 06ff0c9e2..7b76c585f 100644 --- a/test/system/070-build.bats +++ b/test/system/070-build.bats @@ -922,6 +922,9 @@ function teardown() { run_podman '?' rm -a -f run_podman '?' rmi -f build_test + # Many of the tests above leave interim layers behind. Clean them up. + run_podman '?' image prune -f + basic_teardown } diff --git a/test/system/120-load.bats b/test/system/120-load.bats index 67687a5b0..97ea0f528 100644 --- a/test/system/120-load.bats +++ b/test/system/120-load.bats @@ -134,8 +134,16 @@ verify_iid_and_name() { } @test "podman load - multi-image archive" { - img1="quay.io/libpod/testimage:00000000" - img2="quay.io/libpod/testimage:20200902" + # img1 & 2 should be images that are not locally present; they must also + # be usable on the host arch. The nonlocal image (:000000xx) is kept + # up-to-date for all RHEL/Fedora arches; the other image we use is + # the one tagged ':multiimage', which as of 2021-07-15 is :20210610 + # but that tag will grow stale over time. If/when this test fails, + # your first approach should be to manually update :multiimage to + # point to a more recent testimage. (Use the quay.io GUI, it's waaay + # easier than pulling/pushing the correct manifest.) + img1=${PODMAN_NONLOCAL_IMAGE_FQN} + img2="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/$PODMAN_TEST_IMAGE_NAME:multiimage" archive=$PODMAN_TMPDIR/myimage-$(random_string 8).tar run_podman pull $img1 @@ -151,8 +159,9 @@ verify_iid_and_name() { } @test "podman load - multi-image archive with redirect" { - img1="quay.io/libpod/testimage:00000000" - img2="quay.io/libpod/testimage:20200902" + # (see comments in test above re: img1 & 2) + img1=${PODMAN_NONLOCAL_IMAGE_FQN} + img2="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/$PODMAN_TEST_IMAGE_NAME:multiimage" archive=$PODMAN_TMPDIR/myimage-$(random_string 8).tar run_podman pull $img1 |