diff options
35 files changed, 486 insertions, 176 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index 9cab8f0f0..17902a430 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -503,8 +503,7 @@ docker-py_test_task: always: *runner_stats -# Does exactly what it says, execute the podman unit-tests on all primary -# platforms and release versions. +# Does exactly what it says, execute the podman unit-tests on Fedora. unit_test_task: name: "Unit tests on $DISTRO_NV" alias: unit_test @@ -515,9 +514,6 @@ unit_test_task: - validate matrix: - env: *stdenvars - # Fedora 35 skipped for podman4 - #- env: *priorfedora_envvars - - env: *ubuntu_envvars # Special-case: Rootless on latest Fedora (standard) VM - name: "Rootless unit on $DISTRO_NV" env: diff --git a/docs/README.md b/docs/README.md index 0f2af16d6..abe7d7758 100644 --- a/docs/README.md +++ b/docs/README.md @@ -13,7 +13,6 @@ link on that page. | ------------------------------------ | --------------------------- | | Markdown source for man pages | docs/source/markdown/ | | man pages aliases as .so files | docs/source/markdown/links/ | -| restructured text for readthedocs.io | docs/rst/ | | target for output | docs/build | | man pages | docs/build/man | | remote linux man pages | docs/build/remote/linux | @@ -60,16 +59,24 @@ incorrect, or the file isn't accessible for some other reason. ## Local Testing -Assuming that you have the [dependencies](https://podman.io/getting-started/installation#build-and-run-dependencies) +To build standard man pages, run `make docs`. Results will be in `docs/build/man`. + +To build HTMLized man pages: Assuming that you have the +[dependencies](https://podman.io/getting-started/installation#build-and-run-dependencies) installed, then also install (showing Fedora in the example): ``` -# dnf install python3-sphinx python3-recommonmark -# pip install sphinx-markdown-tables +$ sudo dnf install python3-sphinx python3-recommonmark +$ pip install sphinx-markdown-tables myst_parser ``` +(The above dependencies are current as of 2022-09-15. If you experience problems, +please see [requirements.txt](requirements.txt) in this directory, it will almost +certainly be more up-to-date than this README.) + After that completes, cd to the `docs` directory in your Podman sandbox and then do `make html`. You can then preview the html files in `docs/build/html` with: ``` python -m http.server 8000 --directory build/html ``` +...and point your web browser at `http://localhost:8000/` diff --git a/docs/source/conf.py b/docs/source/conf.py index b58bb3f46..345e289e1 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -86,12 +86,12 @@ def convert_markdown_title(app, docname, source): if docpath.endswith(".md"): # Convert pandoc title line into eval_rst block for myst_parser # - # Remove the ending "(1)" to avoid it from being displayed + # Remove the ending " 1" (section) to avoid it from being displayed # in the web tab. Often such a text indicates that # a web page got an update. For instance GitHub issues # shows the number of new comments that have been written # after the user's last visit. - source[0] = re.sub(r"^% (.*)(\(\d\))", r"```{title} \g<1>\n```", source[0]) + source[0] = re.sub(r"^% (.*)\s(\d)", r"```{title} \g<1>\n```", source[0]) def setup(app): app.connect("source-read", convert_markdown_title) diff --git a/docs/source/markdown/options/dns.md b/docs/source/markdown/options/dns.md new file mode 100644 index 000000000..39380ace2 --- /dev/null +++ b/docs/source/markdown/options/dns.md @@ -0,0 +1,11 @@ +#### **--dns**=*ipaddr* + +Set custom DNS servers. + +This option can be used to override the DNS +configuration passed to the container. Typically this is necessary when the +host DNS configuration is invalid for the container (e.g., **127.0.0.1**). When this +is the case the **--dns** flag is necessary for every run. + +The special value **none** can be specified to disable creation of _/etc/resolv.conf_ in the container by Podman. +The _/etc/resolv.conf_ file in the image will be used without changes. diff --git a/docs/source/markdown/podman-build.1.md.in b/docs/source/markdown/podman-build.1.md.in index 94644a051..e201806e5 100644 --- a/docs/source/markdown/podman-build.1.md.in +++ b/docs/source/markdown/podman-build.1.md.in @@ -223,18 +223,12 @@ specifying **--disable-compression=false**. @@option disable-content-trust -#### **--dns**=*dns* +@@option dns -Set custom DNS servers to be used during the build. +This option cannot be combined with **--network** that is set to **none**. -This option can be used to override the DNS configuration passed to the -container. Typically this is necessary when the host DNS configuration is -invalid for the container (e.g., 127.0.0.1). When this is the case the `--dns` -option is necessary for every run. - -The special value **none** can be specified to disable creation of -/etc/resolv.conf in the container by Podman. The /etc/resolv.conf file in the -image will be used without changes. +Note: this option takes effect only during *RUN* instructions in the build. +It does not affect _/etc/resolv.conf_ in the final image. #### **--dns-option**=*option* diff --git a/docs/source/markdown/podman-create.1.md.in b/docs/source/markdown/podman-create.1.md.in index 835976eba..72f167a7e 100644 --- a/docs/source/markdown/podman-create.1.md.in +++ b/docs/source/markdown/podman-create.1.md.in @@ -141,17 +141,9 @@ flag to pass the user's supplementary group access into the container. @@option disable-content-trust -#### **--dns**=*dns* +@@option dns -Set custom DNS servers. Invalid if using **--dns** and **--network** that is set to 'none' or `container:<name|id>`. - -This option can be used to override the DNS -configuration passed to the container. Typically this is necessary when the -host DNS configuration is invalid for the container (e.g., 127.0.0.1). When this -is the case the **--dns** flag is necessary for every run. - -The special value **none** can be specified to disable creation of **/etc/resolv.conf** in the container by Podman. -The **/etc/resolv.conf** file in the image will be used without changes. +This option cannot be combined with **--network** that is set to **none** or **container:**_id_. @@option dns-opt.container diff --git a/docs/source/markdown/podman-run.1.md.in b/docs/source/markdown/podman-run.1.md.in index b4bb107d6..37d052646 100644 --- a/docs/source/markdown/podman-run.1.md.in +++ b/docs/source/markdown/podman-run.1.md.in @@ -177,17 +177,9 @@ flag to pass the user's supplementary group access into the container. @@option disable-content-trust -#### **--dns**=*ipaddr* +@@option dns -Set custom DNS servers. Invalid if using **--dns** with **--network** that is set to **none** or **container:**_id_. - -This option can be used to override the DNS -configuration passed to the container. Typically this is necessary when the -host DNS configuration is invalid for the container (e.g., **127.0.0.1**). When this -is the case the **--dns** flag is necessary for every run. - -The special value **none** can be specified to disable creation of _/etc/resolv.conf_ in the container by Podman. -The _/etc/resolv.conf_ file in the image will be used without changes. +This option cannot be combined with **--network** that is set to **none** or **container:**_id_. @@option dns-opt.container @@ -106,7 +106,7 @@ export PODMAN_ROOTLESS_USER=$(id -un) # Root if [ -z "$ROOTLESS_ONLY" ]; then - echo "# bats ${bats_filter[@]} $TESTS" + echo "# bats ${bats_filter[*]} $TESTS" sudo --preserve-env=PODMAN \ --preserve-env=PODMAN_TEST_DEBUG \ --preserve-env=OCI_RUNTIME \ @@ -119,7 +119,7 @@ fi # Rootless. (Only if we're not already root) if [[ -z "$ROOT_ONLY" && "$(id -u)" != 0 ]]; then echo "--------------------------------------------------" - echo "\$ bats ${bats_filter[@]} $TESTS" + echo "\$ bats ${bats_filter[*]} $TESTS" bats "${bats_opts[@]}" "${bats_filter[@]}" $TESTS rc=$((rc | $?)) fi diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index d579a180a..b95290841 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -38,10 +38,6 @@ our $Treadmill_PR_Title = 'DO NOT MERGE: buildah vendor treadmill'; # Github API; this is where we query to find out the active treadmill PR our $API_URL = 'https://api.github.com/graphql'; -# Temporary file used to preserve current treadmill patches. This file -# should only exist very briefly while we perform branch operations. -our $Patch_File = "0000-$ME.patch"; - # Use colors if available and if stdout is a tty our $C_Highlight = ''; our $C_Warning = ''; @@ -66,14 +62,14 @@ eval ' sub usage { print <<"END_USAGE"; -Usage: $ME [OPTIONS] [--sync | --pick | --reset ] +Usage: $ME [OPTIONS] [--sync | --pick [PR] | --reset ] $ME is (2022-04-20) **EXPERIMENTAL** $ME is intended to solve the problem of vendoring buildah into podman. -Call me with one of two options: +Call me with one of three options: --sync The usual case. Mostly used by Ed. Called from a development branch, this just updates everything so @@ -81,7 +77,8 @@ Call me with one of two options: latest-podman (main). With a few sanity checks. --pick Used for really-truly vendoring in a new buildah; will - cherry-pick a commit on your buildah-vendor working branch + cherry-pick a commit on your buildah-vendor working branch. + Optional PR arg is the ID of the treadmill PR on github. --reset Used after vendoring buildah into main, when there really aren't any buildah patches to keep rolling. @@ -103,6 +100,7 @@ END_USAGE our %action; our $debug = 0; our $force_old_main = 0; # in --pick, proceeds even if main is old +our $force_retry = 0; # in --sync, continue despite saved checkpoint our $force_testing = 0; # in --sync, test even no podman/buildah changes our $verbose = 0; our $NOT = ''; # print "blahing the blah$NOT\n" if $debug @@ -114,6 +112,7 @@ sub handle_opts { 'reset' => sub { $action{reset}++ }, 'force-old-main' => \$force_old_main, + 'force-retry' => \$force_retry, 'force-testing' => \$force_testing, 'debug!' => \$debug, @@ -140,11 +139,6 @@ sub main { # and there's no clean way to make it use @_. handle_opts(); # will set package globals - # Fetch command-line arguments. Barf if too many. - # FIXME: if called with arg, that's the --sync branch? - # FIXME: if called with --pick + arg, that's the PR? - die "$ME: Too many arguments; try $ME --help\n" if @ARGV; - my @action = keys(%action); die "$ME: Please invoke me with one of --sync or --pick\n" if ! @action; @@ -158,13 +152,15 @@ sub main { # that repo is clean. None of our actions can be run on a dirty repo. assert_clean_repo(); - $handler->(); + $handler->(@ARGV); } ############################################################################### # BEGIN sync and its helpers sub do_sync { + die "$ME: --sync takes no arguments; try $ME --help\n" if @_; + # Preserve current branch name, so we can come back after switching to main my $current_branch = git_current_branch(); @@ -188,11 +184,13 @@ sub do_sync { pull_main(); git('checkout', '-q', $current_branch); - # Preserve local patches. --always will generate empty patches (e.g., - # after a buildah vendor when everything is copacetic); --no-signature - # prevents a buildup of "-- 2.35" (git version) lines at the end. - git('format-patch', '--always', '--no-signature', "--output=$Patch_File", 'HEAD^'); - progress("Treadmill patches saved to $Patch_File"); + # Make a temporary copy of this branch + my $temp_branch = strftime("__buildah-treadmill-checkpoint/%Y%m%d-%H%M%S", localtime); + git('branch', $temp_branch, $current_branch); + progress("Current branch preserved as $temp_branch"); + + # Get the hash of the top (treadmill) commit, to cherry-pick later + my $treadmill_commit = git('rev-parse', 'HEAD'); # # Danger Will Robinson! This is where it gets scary: a failure here @@ -207,7 +205,11 @@ This is not something I can recover from. Your human judgment is needed. You will need to recover from this manually. Your best option is to look at the source code for this script. -Your treadmill patches are here: $Patch_File +Treadmill branch copy is preserved in $temp_branch + +To restore state to where you were before this sync: + \$ git checkout main + \$ git branch -f $current_branch $treadmill_commit END_FAIL_INSTRUCTIONS exit 1; @@ -260,12 +262,34 @@ END_FAIL_INSTRUCTIONS git_commit_buildah($buildah_new); # And, finally, this has the highest possibility of failing - progress('Reapplying preserved patches'); - git('am', '--empty=keep', $Patch_File); + local $SIG{__DIE__} = sub { + print STDERR $C_Warning, "@_", <<"END_FAIL_INSTRUCTIONS"; + +This is not something I can recover from. Your human judgment is needed. + +Chances are, you might be able to run 'git status', look for +merge conflicts, manually resolve those, 'git add', then +'git cherry-pick --continue'. If that works, run this script +again (you will probably need the --force-retry option). - # It worked! Clean up: remove our local die() handler and the patch file +If that DOES NOT work, your only option is to look at the source code +for this script. Sorry. There's only so much that can be done automatically. + +Treadmill branch copy is preserved in $temp_branch + +To restore state to where you were before this sync: + \$ git checkout main + \$ git branch -f $current_branch $treadmill_commit +END_FAIL_INSTRUCTIONS + + exit 1; + }; + progress('Reapplying treadmill patches'); + git('cherry-pick', '--allow-empty', $treadmill_commit); + + # It worked! Clean up: remove our local die() handler and the saved branch undef $SIG{__DIE__}; - unlink $Patch_File; + git('branch', '-D', $temp_branch); # if buildah is unchanged, and we did not pull main, exit cleanly my $change_message = ''; @@ -295,6 +319,13 @@ END_FAIL_INSTRUCTIONS progress("All OK. It's now up to you to 'git push --force'"); progress(" --- Reminder: $change_message"); + + # Kind of kludgy. If user had to retry a prior failed attempt, and + # things are now successful, remind them to delete old checkpoints. + # ($force_retry is a 'git branch -D' command string at this point.) + if ($force_retry) { + progress(" --- Retry worked! You may now $force_retry"); + } } ############### @@ -429,8 +460,9 @@ sub do_pick { assert_buildah_vendor_commit('HEAD'); progress("HEAD is a buildah vendor commit. Good."); - # Identify and pull the treadmill PR - my $treadmill_pr = treadmill_pr(); + # Identify and pull the treadmill PR. + my $treadmill_pr = shift || treadmill_pr(); + my $treadmill_branch = "$ME/pr$treadmill_pr/tmp$$"; progress("Fetching treadmill PR $treadmill_pr into $treadmill_branch"); git('fetch', '-q', git_upstream(), "pull/$treadmill_pr/head:$treadmill_branch"); @@ -465,6 +497,26 @@ sub do_pick { # treadmill_pr # Returns ID of open podman PR with the desired subject ################## sub treadmill_pr { + # Github API (or maybe just the search endpoint???) is restricted. + my $token = $ENV{GITHUB_TOKEN} + or do { + warn <<"END_NEED_PR"; +$ME: Cannot proceed without PR ID. + +If you have a github API token, please: export GITHUB_TOKEN=....... +and re-run me. + +If you do not have a github API token, please go here: + + https://github.com/containers/podman/pulls?q=is%3Apr+is%3Aopen+%22buildah+vendor+treadmill%22 + +...then reinvoke me, adding that PR ID to the command line args. + +As of 2022-09-12 the treadmill PR is 13808, but that may change over time. +END_NEED_PR + exit 1; + }; + my $query = <<'END_QUERY'; { search( @@ -481,16 +533,10 @@ END_QUERY $ua->agent("$ME " . $ua->agent); # Identify ourself my %headers = ( + 'Authorization' => "bearer $token", 'Accept' => "application/vnd.github.antiope-preview+json", 'Content-Type' => "application/json", ); - - # Use github token if available, but don't require it. (All it does is - # bump up our throttling limit, which shouldn't be an issue) (unless - # someone invokes this script hundreds of times per minute). - if (my $token = $ENV{GITHUB_TOKEN}) { - $headers{Authorization} = "bearer $token"; - } $ua->default_header($_ => $headers{$_}) for keys %headers; # Massage the query: escape quotes, put it all in one line, collapse spaces @@ -503,7 +549,9 @@ END_QUERY print $postquery, "\n" if $debug; my $res = $ua->post($API_URL, Content => $postquery); if ((my $code = $res->code) != 200) { - print $code, " ", $res->message, "\n"; + warn "$ME: GraphQL request failed on $API_URL:\n"; + print STDERR " ", $code, " ", $res->message, "\n"; + warn "Cannot continue.\n"; exit 1; } @@ -621,8 +669,8 @@ from the buildah vendor treadmill PR, #%s EOF # Strip the "DO NOT MERGE" header from the treadmill PR, print only -# the "Changes as of YYYY-MM-DD" and subsequent lines -sed -ne '/^Changes as of/,$ p' <$msgfile >>$tmpfile +# the "Changes since YYYY-MM-DD" and subsequent lines +sed -ne '/^Changes since /,$ p' <$msgfile >>$tmpfile mv $tmpfile $msgfile END_EDIT_SCRIPT @@ -639,6 +687,8 @@ END_EDIT_SCRIPT # BEGIN reset and its helpers sub do_reset { + die "$ME: --sync takes no arguments; try $ME --help\n" if @_; + my $current_branch = git_current_branch(); # Make sure side branch == main (i.e., there are no commits on the branch) @@ -681,20 +731,46 @@ sub progress { # assert_clean_repo # Don't even think of running with local changes ####################### sub assert_clean_repo { - # Our patch file should only exist for brief moments during a sync run. - # If it exists at any other time, something has gone very wrong. - if (-e $Patch_File) { - warn <<"END_WARN"; -$ME: File exists: $Patch_File + # During --sync we create a temporary copy of the treadmill branch, + # in case something goes wrong. The branch is deleted on success. + # If one exists, it means we may have lost work. + my @relics = grep { + m!^__buildah-treadmill-checkpoint/\d+-\d+$! + } git('branch', '--list', '--format=%(refname:lstrip=2)'); + if (@relics) { + if ($force_retry) { + warn <<"END_WARN"; +$ME: WARNING: leftover checkpoint(s): @relics + + ...continuing due to --force-retry. + + If things work out, you can 'git branch -D @relics' +END_WARN + + # OK, ugly override of a binary flag, but it's OK because + # it helps with user-friendliness: offer a reminder upon + # successful completion of the script. + $force_retry = "git branch -D @relics"; + } + else { + warn <<"END_WARN"; +$ME: FATAL: leftover checkpoint: @relics This means that something went very wrong during an earlier sync run. Your git branch may be in an inconsistent state. Your work to date - may be lost. This file may be your only hope of recovering it. + may be lost. This branch may be your only hope of recovering it. This is not something a script can resolve. You need to look at this - file, compare to your git HEAD, and manually reconcile any differences. + branch, compare to your git HEAD, and manually reconcile any differences. + + If you really know what you're doing, i.e., if you've reconciled + merge conflicts and have a pretty secure branch structure, try + rerunning me with --force-retry. Or, if that checkpoint is a + remnant from a past run, and you're ultra-certain that you don't + need it, you can git branch -D @relics END_WARN - exit 1; + exit 1; + } } # OK so far. Now check for modified files. @@ -727,7 +803,15 @@ sub git_current_branch() { # git_forkpoint # Hash at which branch (default: cur) branched from main ################### sub git_forkpoint { - return git('merge-base', '--fork-point', 'main', @_); + # '--fork-point vendor-branch' fails silently on Paul's git tree, + # but plain merge-base works fine. My head hurts from trying to + # understand the docs, so I give up. Just try fork-point first, + # and if it fails, try without. #cargocult #gitishard + my $forkpoint = eval { git('merge-base', '--fork-point', 'main', @_) }; + if ($@) { + $forkpoint = git('merge-base', 'main', @_); + } + return $forkpoint; } ##################### diff --git a/hack/check_root.sh b/hack/check_root.sh index 1f53887ff..2489a4edd 100755 --- a/hack/check_root.sh +++ b/hack/check_root.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash if ! [ $(id -u) = 0 ]; then - echo "Please run as root! '$@' requires root privileges." + echo "Please run as root! '$*' requires root privileges." exit 1 fi diff --git a/hack/man-page-checker b/hack/man-page-checker index 8ee0aaf6d..83e0b8b1d 100755 --- a/hack/man-page-checker +++ b/hack/man-page-checker @@ -87,7 +87,7 @@ function compare_usage() { # strip off command name from both from_man=$(sed -e "s/\*\*$cmd\*\*[[:space:]]*//" <<<"$from_man") - from_help=$(sed -e "s/^[[:space:]]*$cmd[[:space:]]*//" <<<"$from_help") + from_help=$(sed -e "s/^[[:space:]]*${cmd}[[:space:]]*//" <<<"$from_help") # man page lists 'foo [*options*]', help msg shows 'foo [flags]'. # Make sure if one has it, the other does too. @@ -153,7 +153,7 @@ for md in *.1.md;do # special case: the command is "auto-update", with a hyphen md_nodash='podman auto-update' fi - if [ "$cmd" != "$md_nodash" -a "$cmd" != "podman-remote" ]; then + if [[ "$cmd" != "$md_nodash" ]] && [[ "$cmd" != "podman-remote" ]]; then echo printf "Inconsistent program name in SYNOPSIS in %s:\n" $md printf " SYNOPSIS = %s (expected: '%s')\n" "$cmd" "$md_nodash" diff --git a/libpod/container_graph.go b/libpod/container_graph.go index 96d61b756..d43579e4a 100644 --- a/libpod/container_graph.go +++ b/libpod/container_graph.go @@ -281,3 +281,94 @@ func startNode(ctx context.Context, node *containerNode, setError bool, ctrError startNode(ctx, successor, ctrErrored, ctrErrors, ctrsVisited, restart) } } + +// Visit a node on the container graph and remove it, or set an error if it +// failed to remove. Only intended for use in pod removal; do *not* use when +// removing individual containers. +// All containers are assumed to be *UNLOCKED* on running this function. +// Container locks will be acquired as necessary. +// Pod and infraID are optional. If a pod is given it must be *LOCKED*. +func removeNode(ctx context.Context, node *containerNode, pod *Pod, force bool, timeout *uint, setError bool, ctrErrors map[string]error, ctrsVisited map[string]bool, ctrNamedVolumes map[string]*ContainerNamedVolume) { + // If we already visited this node, we're done. + if ctrsVisited[node.id] { + return + } + + // Someone who depends on us failed. + // Mark us as failed and recurse. + if setError { + ctrsVisited[node.id] = true + ctrErrors[node.id] = fmt.Errorf("a container that depends on container %s could not be removed: %w", node.id, define.ErrCtrStateInvalid) + + // Hit anyone who depends on us, set errors there as well. + for _, successor := range node.dependsOn { + removeNode(ctx, successor, pod, force, timeout, true, ctrErrors, ctrsVisited, ctrNamedVolumes) + } + } + + // Does anyone still depend on us? + // Cannot remove if true. Once all our dependencies have been removed, + // we will be removed. + for _, dep := range node.dependedOn { + // The container that depends on us hasn't been removed yet. + // OK to continue on + if ok := ctrsVisited[dep.id]; !ok { + return + } + } + + // Going to try to remove the node, mark us as visited + ctrsVisited[node.id] = true + + ctrErrored := false + + // Verify that all that depend on us are gone. + // Graph traversal should guarantee this is true, but this isn't that + // expensive, and it's better to be safe. + for _, dep := range node.dependedOn { + if _, err := node.container.runtime.GetContainer(dep.id); err == nil { + ctrErrored = true + ctrErrors[node.id] = fmt.Errorf("a container that depends on container %s still exists: %w", node.id, define.ErrDepExists) + } + } + + // Lock the container + node.container.lock.Lock() + + // Gate all subsequent bits behind a ctrErrored check - we don't want to + // proceed if a previous step failed. + if !ctrErrored { + if err := node.container.syncContainer(); err != nil { + ctrErrored = true + ctrErrors[node.id] = err + } + } + + if !ctrErrored { + for _, vol := range node.container.config.NamedVolumes { + ctrNamedVolumes[vol.Name] = vol + } + + if pod != nil && pod.state.InfraContainerID == node.id { + pod.state.InfraContainerID = "" + if err := pod.save(); err != nil { + ctrErrored = true + ctrErrors[node.id] = fmt.Errorf("error removing infra container %s from pod %s: %w", node.id, pod.ID(), err) + } + } + } + + if !ctrErrored { + if err := node.container.runtime.removeContainer(ctx, node.container, force, false, true, false, timeout); err != nil { + ctrErrored = true + ctrErrors[node.id] = err + } + } + + node.container.lock.Unlock() + + // Recurse to anyone who we depend on and remove them + for _, successor := range node.dependsOn { + removeNode(ctx, successor, pod, force, timeout, ctrErrored, ctrErrors, ctrsVisited, ctrNamedVolumes) + } +} diff --git a/libpod/pod_api.go b/libpod/pod_api.go index 1bd686ddc..924d43436 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -40,7 +40,7 @@ func (p *Pod) startInitContainers(ctx context.Context) error { icLock := initCon.lock icLock.Lock() var time *uint - if err := p.runtime.removeContainer(ctx, initCon, false, false, true, time); err != nil { + if err := p.runtime.removeContainer(ctx, initCon, false, false, true, false, time); err != nil { icLock.Unlock() return fmt.Errorf("failed to remove once init container %s: %w", initCon.ID(), err) } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 1f032dd6b..7b3cbadfa 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -581,7 +581,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai // be removed also if and only if the container is the sole user // Otherwise, RemoveContainer will return an error if the container is running func (r *Runtime) RemoveContainer(ctx context.Context, c *Container, force bool, removeVolume bool, timeout *uint) error { - return r.removeContainer(ctx, c, force, removeVolume, false, timeout) + return r.removeContainer(ctx, c, force, removeVolume, false, false, timeout) } // Internal function to remove a container. @@ -589,7 +589,9 @@ func (r *Runtime) RemoveContainer(ctx context.Context, c *Container, force bool, // removePod is used only when removing pods. It instructs Podman to ignore // infra container protections, and *not* remove from the database (as pod // remove will handle that). -func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, removeVolume, removePod bool, timeout *uint) error { +// ignoreDeps is *DANGEROUS* and should not be used outside of a very specific +// context (alternate pod removal code, where graph traversal is not possible). +func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, removeVolume, removePod, ignoreDeps bool, timeout *uint) error { if !c.valid { if ok, _ := r.state.HasContainer(c.ID()); !ok { // Container probably already removed @@ -618,25 +620,27 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo // pod. var pod *Pod runtime := c.runtime - if c.config.Pod != "" && !removePod { + if c.config.Pod != "" { pod, err = r.state.Pod(c.config.Pod) if err != nil { return fmt.Errorf("container %s is in pod %s, but pod cannot be retrieved: %w", c.ID(), pod.ID(), err) } - // Lock the pod while we're removing container - if pod.config.LockID == c.config.LockID { - return fmt.Errorf("container %s and pod %s share lock ID %d: %w", c.ID(), pod.ID(), c.config.LockID, define.ErrWillDeadlock) - } - pod.lock.Lock() - defer pod.lock.Unlock() - if err := pod.updatePod(); err != nil { - return err - } + if !removePod { + // Lock the pod while we're removing container + if pod.config.LockID == c.config.LockID { + return fmt.Errorf("container %s and pod %s share lock ID %d: %w", c.ID(), pod.ID(), c.config.LockID, define.ErrWillDeadlock) + } + pod.lock.Lock() + defer pod.lock.Unlock() + if err := pod.updatePod(); err != nil { + return err + } - infraID := pod.state.InfraContainerID - if c.ID() == infraID { - return fmt.Errorf("container %s is the infra container of pod %s and cannot be removed without removing the pod", c.ID(), pod.ID()) + infraID := pod.state.InfraContainerID + if c.ID() == infraID { + return fmt.Errorf("container %s is the infra container of pod %s and cannot be removed without removing the pod", c.ID(), pod.ID()) + } } } @@ -696,7 +700,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo // Check that no other containers depend on the container. // Only used if not removing a pod - pods guarantee that all // deps will be evicted at the same time. - if !removePod { + if !ignoreDeps { deps, err := r.state.ContainerInUse(c) if err != nil { return err @@ -777,13 +781,11 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo if c.config.Pod != "" { // If we're removing the pod, the container will be evicted // from the state elsewhere - if !removePod { - if err := r.state.RemoveContainerFromPod(pod, c); err != nil { - if cleanupErr == nil { - cleanupErr = err - } else { - logrus.Errorf("Removing container %s from database: %v", c.ID(), err) - } + if err := r.state.RemoveContainerFromPod(pod, c); err != nil { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("Removing container %s from database: %v", c.ID(), err) } } } else { @@ -872,7 +874,7 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol if err == nil { logrus.Infof("Container %s successfully retrieved from state, attempting normal removal", id) // Assume force = true for the evict case - err = r.removeContainer(ctx, tmpCtr, true, removeVolume, false, timeout) + err = r.removeContainer(ctx, tmpCtr, true, removeVolume, false, false, timeout) if !tmpCtr.valid { // If the container is marked invalid, remove succeeded // in kicking it out of the state - no need to continue. @@ -1034,7 +1036,7 @@ func (r *Runtime) RemoveDepend(ctx context.Context, rmCtr *Container, force bool } report := reports.RmReport{Id: rmCtr.ID(), RawInput: rmCtr.ID()} - report.Err = r.removeContainer(ctx, rmCtr, force, removeVolume, false, timeout) + report.Err = r.removeContainer(ctx, rmCtr, force, removeVolume, false, false, timeout) return append(rmReports, &report), nil } diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index 5510b2af6..dacbd752f 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -47,7 +47,7 @@ func (r *Runtime) RemoveContainersForImageCallback(ctx context.Context) libimage return fmt.Errorf("removing image %s: container %s using image could not be removed: %w", imageID, ctr.ID(), err) } } else { - if err := r.removeContainer(ctx, ctr, true, false, false, timeout); err != nil { + if err := r.removeContainer(ctx, ctr, true, false, false, false, timeout); err != nil { return fmt.Errorf("removing image %s: container %s using image could not be removed: %w", imageID, ctr.ID(), err) } } diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index 3eeef69d8..24e9f3da7 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -17,6 +17,7 @@ import ( "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/specgen" + "github.com/hashicorp/go-multierror" "github.com/sirupsen/logrus" ) @@ -191,29 +192,9 @@ func (r *Runtime) SavePod(pod *Pod) error { return nil } -func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool, timeout *uint) error { - if err := p.updatePod(); err != nil { - return err - } - - ctrs, err := r.state.PodContainers(p) - if err != nil { - return err - } - numCtrs := len(ctrs) - - // If the only running container in the pod is the pause container, remove the pod and container unconditionally. - pauseCtrID := p.state.InfraContainerID - if numCtrs == 1 && ctrs[0].ID() == pauseCtrID { - removeCtrs = true - force = true - } - if !removeCtrs && numCtrs > 0 { - return fmt.Errorf("pod %s contains containers and cannot be removed: %w", p.ID(), define.ErrCtrExists) - } - - ctrNamedVolumes := make(map[string]*ContainerNamedVolume) - +// DO NOT USE THIS FUNCTION DIRECTLY. Use removePod(), below. It will call +// removeMalformedPod() if necessary. +func (r *Runtime) removeMalformedPod(ctx context.Context, p *Pod, ctrs []*Container, force bool, timeout *uint, ctrNamedVolumes map[string]*ContainerNamedVolume) error { var removalErr error for _, ctr := range ctrs { err := func() error { @@ -231,7 +212,7 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool, ctrNamedVolumes[vol.Name] = vol } - return r.removeContainer(ctx, ctr, force, false, true, timeout) + return r.removeContainer(ctx, ctr, force, false, true, true, timeout) }() if removalErr == nil { @@ -261,6 +242,69 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool, return err } + return nil +} + +func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool, timeout *uint) error { + if err := p.updatePod(); err != nil { + return err + } + + ctrs, err := r.state.PodContainers(p) + if err != nil { + return err + } + numCtrs := len(ctrs) + + // If the only running container in the pod is the pause container, remove the pod and container unconditionally. + pauseCtrID := p.state.InfraContainerID + if numCtrs == 1 && ctrs[0].ID() == pauseCtrID { + removeCtrs = true + force = true + } + if !removeCtrs && numCtrs > 0 { + return fmt.Errorf("pod %s contains containers and cannot be removed: %w", p.ID(), define.ErrCtrExists) + } + + var removalErr error + ctrNamedVolumes := make(map[string]*ContainerNamedVolume) + + // Build a graph of all containers in the pod. + graph, err := BuildContainerGraph(ctrs) + if err != nil { + // We have to allow the pod to be removed. + // But let's only do it if force is set. + if !force { + return fmt.Errorf("cannot create container graph for pod %s: %w", p.ID(), err) + } + + removalErr = fmt.Errorf("creating container graph for pod %s failed, fell back to loop removal: %w", p.ID(), err) + + if err := r.removeMalformedPod(ctx, p, ctrs, force, timeout, ctrNamedVolumes); err != nil { + logrus.Errorf("Error creating container graph for pod %s: %v. Falling back to loop removal.", p.ID(), err) + return err + } + } else { + ctrErrors := make(map[string]error) + ctrsVisited := make(map[string]bool) + + for _, node := range graph.notDependedOnNodes { + removeNode(ctx, node, p, force, timeout, false, ctrErrors, ctrsVisited, ctrNamedVolumes) + } + + // This is gross, but I don't want to change the signature on + // removePod - especially since any change here eventually has + // to map down to one error unless we want to make a breaking + // API change. + if len(ctrErrors) > 0 { + var allErrs error + for id, err := range ctrErrors { + allErrs = multierror.Append(allErrs, fmt.Errorf("removing container %s from pod %s: %w", id, p.ID(), err)) + } + return allErrs + } + } + for volName := range ctrNamedVolumes { volume, err := r.state.Volume(volName) if err != nil && !errors.Is(err, define.ErrNoSuchVolume) { diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index c9a4a7dc1..08fdbf977 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -324,7 +324,7 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo logrus.Debugf("Removing container %s (depends on volume %q)", ctr.ID(), v.Name()) - if err := r.removeContainer(ctx, ctr, force, false, false, timeout); err != nil { + if err := r.removeContainer(ctx, ctr, force, false, false, false, timeout); err != nil { return fmt.Errorf("removing container %s that depends on volume %s: %w", ctr.ID(), v.Name(), err) } } diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index d1460569f..a76e3d988 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -266,16 +266,16 @@ func Checkpoint(w http.ResponseWriter, r *http.Request) { utils.InternalServerError(w, err) return } + if len(reports) != 1 { + utils.InternalServerError(w, fmt.Errorf("expected 1 restore report but got %d", len(reports))) + return + } + if reports[0].Err != nil { + utils.InternalServerError(w, reports[0].Err) + return + } if !query.Export { - if len(reports) != 1 { - utils.InternalServerError(w, fmt.Errorf("expected 1 restore report but got %d", len(reports))) - return - } - if reports[0].Err != nil { - utils.InternalServerError(w, reports[0].Err) - return - } utils.WriteResponse(w, http.StatusOK, reports[0]) return } diff --git a/pkg/bindings/containers/checkpoint.go b/pkg/bindings/containers/checkpoint.go index bcb944488..8c072f588 100644 --- a/pkg/bindings/containers/checkpoint.go +++ b/pkg/bindings/containers/checkpoint.go @@ -39,7 +39,7 @@ func Checkpoint(ctx context.Context, nameOrID string, options *CheckpointOptions } defer response.Body.Close() - if !export { + if response.StatusCode != http.StatusOK || !export { return &report, response.Process(&report) } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 572807ad6..0dc73081d 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -413,7 +413,7 @@ func (ic *ContainerEngine) ContainerCheckpoint(ctx context.Context, namesOrIds [ if err != nil { reports = append(reports, &entities.CheckpointReport{Id: c.ID, Err: err}) } else { - report.RawInput = idToRawInput[report.Id] + report.RawInput = idToRawInput[c.ID] reports = append(reports, report) } } diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index fe7c9a333..8510cfd42 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -46,6 +46,7 @@ type containerInfo struct { ExecStart string TimeoutStartSec uint TimeoutStopSec uint + ExecStop string ExecStopPost string GenerateNoHeader bool Pod *podInfo @@ -97,6 +98,9 @@ TimeoutStopSec={{{{.TimeoutStopSec}}}} ExecStartPre={{{{.ExecStartPre}}}} {{{{- end}}}} ExecStart={{{{.ExecStart}}}} +{{{{- if .ExecStop}}}} +ExecStop={{{{.ExecStop}}}} +{{{{- end}}}} {{{{- if .ExecStopPost}}}} ExecStopPost={{{{.ExecStopPost}}}} {{{{- end}}}} @@ -290,6 +294,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst info.Type = "forking" info.EnvVariable = define.EnvVariable info.ExecStart = "{{{{.Executable}}}} start {{{{.ContainerNameOrID}}}}" + info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}" info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}" for i, env := range info.AdditionalEnvVariables { info.AdditionalEnvVariables[i] = escapeSystemdArg(env) @@ -308,6 +313,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst info.PIDFile = "" info.ContainerIDFile = "%t/%n.ctr-id" info.ExecStartPre = "/bin/rm -f {{{{.ContainerIDFile}}}}" + info.ExecStop = "{{{{.Executable}}}} stop --ignore --cidfile={{{{.ContainerIDFile}}}}" info.ExecStopPost = "{{{{.Executable}}}} rm -f --ignore --cidfile={{{{.ContainerIDFile}}}}" // The create command must at least have three arguments: // /usr/bin/podman run $IMAGE diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 3bde305d7..7f92e75b8 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -57,6 +57,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=82 ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 +ExecStop=/usr/bin/podman stop -t 22 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 ExecStopPost=/usr/bin/podman stop -t 22 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401 PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -82,6 +83,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -105,6 +107,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -131,6 +134,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -157,6 +161,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -183,6 +188,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -211,6 +217,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -236,6 +243,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -268,6 +276,7 @@ ExecStart=/usr/bin/podman container run \ --replace \ --name jadda-jadda \ --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space" +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -300,6 +309,7 @@ ExecStart=/usr/bin/podman container run \ --sdnotify=container \ --name jadda-jadda \ --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space" +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -332,6 +342,7 @@ ExecStart=/usr/bin/podman container run \ --replace \ --name jadda-jadda \ --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space" +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -364,6 +375,7 @@ ExecStart=/usr/bin/podman run \ -d \ --name jadda-jadda \ --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -397,6 +409,7 @@ ExecStart=/usr/bin/podman run \ -d \ --name jadda-jadda \ --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -429,6 +442,7 @@ ExecStart=/usr/bin/podman run \ --detach \ --name jadda-jadda \ --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -458,6 +472,7 @@ ExecStart=/usr/bin/podman run \ --rm \ --sdnotify=conmon \ -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -490,6 +505,7 @@ ExecStart=/usr/bin/podman run \ ` + detachparam + ` awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -525,6 +541,7 @@ ExecStart=/usr/bin/podman run \ --name test \ -p 80:80 awesome-image:latest somecmd \ --detach=false +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -556,6 +573,7 @@ ExecStart=/usr/bin/podman \ --rm \ --sdnotify=conmon \ -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -585,6 +603,7 @@ ExecStart=/usr/bin/podman container run \ --rm \ --sdnotify=conmon \ -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -618,6 +637,7 @@ ExecStart=/usr/bin/podman run \ --name test \ --log-driver=journald \ --log-opt=tag={{.Name}} awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -650,6 +670,7 @@ ExecStart=/usr/bin/podman run \ --replace \ --name test awesome-image:latest sh \ -c "kill $$$$ && echo %%\\" +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -683,6 +704,7 @@ ExecStart=/usr/bin/podman run \ --cgroups=foo \ --conmon-pidfile=foo \ --cidfile=foo alpine +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -718,6 +740,7 @@ ExecStart=/usr/bin/podman run \ --conmon-pidfile=foo \ --cidfile=foo \ --pod-id-file /tmp/pod-foobar.pod-id-file alpine +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -752,6 +775,7 @@ ExecStart=/usr/bin/podman run \ --env=BAR \ --env=MYENV=2 \ -e USER awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -778,6 +802,7 @@ Environment=USER=%%a Restart=on-failure TimeoutStopSec=70 ExecStart=/usr/bin/podman start foobar +ExecStop=/usr/bin/podman stop -t 10 foobar ExecStopPost=/usr/bin/podman stop -t 10 foobar PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -808,6 +833,7 @@ ExecStart=/usr/bin/podman run \ --rm \ --sdnotify=conmon \ -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -838,6 +864,7 @@ ExecStart=/usr/bin/podman run \ --sdnotify=conmon \ -d \ -h hostname awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all @@ -869,6 +896,7 @@ ExecStart=/usr/bin/podman run \ --rm \ --sdnotify=conmon \ -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify NotifyAccess=all diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 13884dd1f..729a038a5 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -69,6 +69,8 @@ type podInfo struct { ExecStart string // TimeoutStopSec of the unit. TimeoutStopSec uint + // ExecStop of the unit. + ExecStop string // ExecStopPost of the unit. ExecStopPost string // Removes autogenerated by Podman and timestamp if set to true @@ -120,6 +122,7 @@ ExecStartPre={{{{.ExecStartPre1}}}} ExecStartPre={{{{.ExecStartPre2}}}} {{{{- end}}}} ExecStart={{{{.ExecStart}}}} +ExecStop={{{{.ExecStop}}}} ExecStopPost={{{{.ExecStopPost}}}} PIDFile={{{{.PIDFile}}}} Type=forking @@ -292,6 +295,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) info.EnvVariable = define.EnvVariable info.ExecStart = "{{{{.Executable}}}} start {{{{.InfraNameOrID}}}}" + info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}" info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}" // Assemble the ExecStart command when creating a new pod. @@ -370,6 +374,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}" info.ExecStartPre2 = strings.Join(startCommand, " ") info.ExecStart = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod start --pod-id-file {{{{.PodIDFile}}}}" + info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod stop --ignore --pod-id-file {{{{.PodIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}" info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}" } info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index 20dfaeb9a..000d73e9a 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -79,6 +79,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -106,6 +107,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -134,6 +136,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -161,6 +164,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -188,6 +192,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -217,6 +222,7 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -243,6 +249,7 @@ TimeoutStopSec=70 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --exit-policy=stop foo ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id +ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id PIDFile=%t/pod-123abc.pid Type=forking @@ -269,6 +276,7 @@ Restart=on-failure RestartSec=15 TimeoutStopSec=102 ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 42 jadda-jadda-infra ExecStopPost=/usr/bin/podman stop -t 42 jadda-jadda-infra PIDFile=/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid Type=forking @@ -296,6 +304,7 @@ TimeoutStopSec=70 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --exit-policy=stop --name foo "bar=arg with space" --replace ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id +ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id PIDFile=%t/pod-123abc.pid Type=forking @@ -323,6 +332,7 @@ TimeoutStopSec=70 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman --events-backend none --runroot /root pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --exit-policy=stop --name foo "bar=arg with space" --replace ExecStart=/usr/bin/podman --events-backend none --runroot /root pod start --pod-id-file %t/pod-123abc.pod-id +ExecStop=/usr/bin/podman --events-backend none --runroot /root pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 ExecStopPost=/usr/bin/podman --events-backend none --runroot /root pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id PIDFile=%t/pod-123abc.pid Type=forking @@ -350,6 +360,7 @@ TimeoutStopSec=70 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --exit-policy=stop --name foo --replace ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id +ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id PIDFile=%t/pod-123abc.pid Type=forking @@ -377,6 +388,7 @@ TimeoutStopSec=70 ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo --label key={{someval}} --exit-policy=continue --replace ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id +ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id PIDFile=%t/pod-123abc.pid Type=forking diff --git a/test/apiv2/test-apiv2 b/test/apiv2/test-apiv2 index b762cff9e..8132e6432 100755 --- a/test/apiv2/test-apiv2 +++ b/test/apiv2/test-apiv2 @@ -272,8 +272,8 @@ function t() { esac done if [[ -z "$curl_args" ]]; then - curl_args=(-d $(jsonify ${post_args[@]})) - testname="$testname [${curl_args[@]}]" + curl_args=(-d $(jsonify ${post_args[*]})) + testname="$testname [${curl_args[*]}]" fi fi @@ -336,7 +336,7 @@ function t() { # Any error from curl is instant bad news, from which we can't recover if [[ $rc -ne 0 ]]; then - die "curl failure ($rc) on $url - cannot continue" + die "curl failure ($rc) on $url - cannot continue. args=${curl_args[*]}" fi # Show returned headers (without trailing ^M or empty lines) in log file. @@ -384,7 +384,7 @@ function t() { # Special case: if response code does not match, dump the response body # and skip all further subtests. - if [[ $actual_code != $expected_code ]]; then + if [[ "$actual_code" != "$expected_code" ]]; then echo -e "# response: $output" for i; do _show_ok skip "$testname: $i # skip - wrong return code" @@ -671,11 +671,11 @@ echo -e "collected ${#tests_to_run[@]} items\n" start_service -for i in ${tests_to_run[@]}; do +for i in "${tests_to_run[@]}"; do TEST_CONTEXT="[$(basename $i .at)]" # Clear output from 'podman' helper - >| $WORKDIR/output.log + truncate --size=0 $WORKDIR/output.log source $i done diff --git a/test/buildah-bud/make-new-buildah-diffs b/test/buildah-bud/make-new-buildah-diffs index 3d0a77008..f6404fa51 100644 --- a/test/buildah-bud/make-new-buildah-diffs +++ b/test/buildah-bud/make-new-buildah-diffs @@ -17,7 +17,7 @@ if [[ ! $whereami =~ test-buildah-v ]]; then fi # FIXME: check that git repo is buildah -git remote -v | grep -q [BUILDAHREPO] \ +git remote -v | grep -q '[BUILDAHREPO]' \ || die "This does not look like a buildah repo (git remote -v)" # We could do the commit automatically, but it's prudent to require human diff --git a/test/compose/test-compose b/test/compose/test-compose index 99d063c25..fe2da9532 100755 --- a/test/compose/test-compose +++ b/test/compose/test-compose @@ -64,7 +64,7 @@ function is() { local expect=$2 local testname=$3 - if [[ $actual = $expect ]]; then + if [[ "$actual" = "$expect" ]]; then # On success, include expected value; this helps readers understand _show_ok 1 "$testname=$expect" return @@ -303,12 +303,12 @@ n_tests=0 # We aren't really TAP 13; this helps logformatter recognize our output as BATS echo "TAP version 13" -for t in ${tests_to_run[@]}; do +for t in "${tests_to_run[@]}"; do testdir="$(dirname $t)" testname="$(basename $testdir)" if [ -e $test_dir/SKIP ]; then - local reason="$(<$test_dir/SKIP)" + reason="$(<$test_dir/SKIP)" if [ -n "$reason" ]; then reason=" - $reason" fi diff --git a/test/e2e/checkpoint_test.go b/test/e2e/checkpoint_test.go index 16e43aa73..a33936549 100644 --- a/test/e2e/checkpoint_test.go +++ b/test/e2e/checkpoint_test.go @@ -95,13 +95,15 @@ var _ = Describe("Podman checkpoint", func() { It("podman checkpoint bogus container", func() { session := podmanTest.Podman([]string{"container", "checkpoint", "foobar"}) session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session).Should(Exit(125)) + Expect(session.ErrorToString()).To(ContainSubstring("no such container")) }) It("podman restore bogus container", func() { session := podmanTest.Podman([]string{"container", "restore", "foobar"}) session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session).Should(Exit(125)) + Expect(session.ErrorToString()).To(ContainSubstring("no such container or image")) }) It("podman checkpoint a running container by id", func() { @@ -585,6 +587,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -604,6 +607,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -623,6 +627,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -642,6 +647,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -659,6 +665,7 @@ var _ = Describe("Podman checkpoint", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(125)) + Expect(result.ErrorToString()).To(ContainSubstring("not supported")) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) Expect(podmanTest.NumberOfContainers()).To(Equal(1)) @@ -704,6 +711,7 @@ var _ = Describe("Podman checkpoint", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -754,6 +762,7 @@ var _ = Describe("Podman checkpoint", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -796,6 +805,7 @@ var _ = Describe("Podman checkpoint", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -834,6 +844,7 @@ var _ = Describe("Podman checkpoint", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -884,6 +895,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -954,6 +966,7 @@ var _ = Describe("Podman checkpoint", func() { result = podmanTest.Podman([]string{"container", "checkpoint", cid, "-e", checkpointFileName}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -1057,6 +1070,7 @@ var _ = Describe("Podman checkpoint", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.GetContainerStatus()).To(ContainSubstring("Exited")) @@ -1103,6 +1117,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -1317,6 +1332,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -1648,6 +1664,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) @@ -1802,6 +1819,7 @@ var _ = Describe("Podman checkpoint", func() { // As the container has been started with '--rm' it will be completely // cleaned up after checkpointing. Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(cid)) fixmeFixme14653(podmanTest, cid) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(0)) diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index 7ceea1b91..347440faf 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -566,6 +566,7 @@ var _ = Describe("Podman generate systemd", func() { Expect(session.OutputToString()).To(ContainSubstring("BindsTo=pod-foo.service")) Expect(session.OutputToString()).To(ContainSubstring("pod create --infra-conmon-pidfile %t/pod-foo.pid --pod-id-file %t/pod-foo.pod-id --exit-policy=stop --name foo")) Expect(session.OutputToString()).To(ContainSubstring("ExecStartPre=/bin/rm -f %t/pod-foo.pid %t/pod-foo.pod-id")) + Expect(session.OutputToString()).To(ContainSubstring("pod stop --ignore --pod-id-file %t/pod-foo.pod-id -t 10")) Expect(session.OutputToString()).To(ContainSubstring("pod rm --ignore -f --pod-id-file %t/pod-foo.pod-id")) }) diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go index a5eab7eed..364ef54d5 100644 --- a/test/e2e/pod_rm_test.go +++ b/test/e2e/pod_rm_test.go @@ -318,4 +318,31 @@ var _ = Describe("Podman pod rm", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) }) + + It("podman pod rm pod with infra container and running container", func() { + podName := "testPod" + ctrName := "testCtr" + + ctrAndPod := podmanTest.Podman([]string{"run", "-d", "--pod", fmt.Sprintf("new:%s", podName), "--name", ctrName, ALPINE, "top"}) + ctrAndPod.WaitWithDefaultTimeout() + Expect(ctrAndPod).Should(Exit(0)) + + removePod := podmanTest.Podman([]string{"pod", "rm", "-a"}) + removePod.WaitWithDefaultTimeout() + Expect(removePod).Should(Not(Exit(0))) + + ps := podmanTest.Podman([]string{"pod", "ps"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring(podName)) + + removePodForce := podmanTest.Podman([]string{"pod", "rm", "-af"}) + removePodForce.WaitWithDefaultTimeout() + Expect(removePodForce).Should(Exit(0)) + + ps2 := podmanTest.Podman([]string{"pod", "ps"}) + ps2.WaitWithDefaultTimeout() + Expect(ps2).Should(Exit(0)) + Expect(ps2.OutputToString()).To(Not(ContainSubstring(podName))) + }) }) diff --git a/test/system/015-help.bats b/test/system/015-help.bats index dd5a7ed44..927645f29 100644 --- a/test/system/015-help.bats +++ b/test/system/015-help.bats @@ -121,7 +121,7 @@ function check_help() { # Exceptions: these commands don't work rootless if is_rootless; then # "pause is not supported for rootless containers" - if [ "$cmd" = "pause" -o "$cmd" = "unpause" ]; then + if [[ "$cmd" = "pause" ]] || [[ "$cmd" = "unpause" ]]; then continue fi # "network rm" too @@ -162,17 +162,17 @@ function check_help() { # Any command that takes subcommands, prints its help and errors if called # without one. - dprint "podman $@" + dprint "podman $*" run_podman '?' "$@" is "$status" 125 "'podman $*' without any subcommand - exit status" - is "$output" ".*Usage:.*Error: missing command '.*$@ COMMAND'" \ + is "$output" ".*Usage:.*Error: missing command '.*$* COMMAND'" \ "'podman $*' without any subcommand - expected error message" # Assume that 'NoSuchCommand' is not a command - dprint "podman $@ NoSuchCommand" + dprint "podman $* NoSuchCommand" run_podman '?' "$@" NoSuchCommand is "$status" 125 "'podman $* NoSuchCommand' - exit status" - is "$output" "Error: unrecognized command .*$@ NoSuchCommand" \ + is "$output" "Error: unrecognized command .*$* NoSuchCommand" \ "'podman $* NoSuchCommand' - expected error message" # This can happen if the output of --help changes, such as between diff --git a/test/system/065-cp.bats b/test/system/065-cp.bats index 8f5abd228..c8ad8468c 100644 --- a/test/system/065-cp.bats +++ b/test/system/065-cp.bats @@ -436,7 +436,7 @@ load helpers run_podman cp cpcontainer:$src $destdir$dest is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description" is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description" - rm -rf $destdir/* + rm -rf ${destdir:?}/* done < <(parse_table "$tests") run_podman kill cpcontainer run_podman rm -t 0 -f cpcontainer @@ -456,7 +456,7 @@ load helpers run_podman cp cpcontainer:$src $destdir$dest is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description" is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description" - rm -rf $destdir/* + rm -rf ${destdir:?}/* done < <(parse_table "$tests") touch $destdir/testfile diff --git a/test/system/070-build.bats b/test/system/070-build.bats index 9fddbaa21..87979483e 100644 --- a/test/system/070-build.bats +++ b/test/system/070-build.bats @@ -541,7 +541,7 @@ Labels.$label_name | $label_value this-file-does-not-match-anything-in-ignore-file comment ) - for f in ${files[@]}; do + for f in "${files[@]}"; do # The magic '##-' strips off the '-' prefix echo "$f" > $tmpdir/${f##-} done diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index 6829c6a78..08baaf468 100644 --- a/test/system/160-volumes.bats +++ b/test/system/160-volumes.bats @@ -315,11 +315,11 @@ EOF # List available volumes for pruning after using 1,2,3 run_podman volume prune <<< N - is "$(echo $(sort <<<${lines[@]:1:3}))" "${v[4]} ${v[5]} ${v[6]}" "volume prune, with 1,2,3 in use, lists 4,5,6" + is "$(echo $(sort <<<${lines[*]:1:3}))" "${v[4]} ${v[5]} ${v[6]}" "volume prune, with 1,2,3 in use, lists 4,5,6" # List available volumes for pruning after using 1,2,3 and filtering; see #8913 run_podman volume prune --filter label=mylabel <<< N - is "$(echo $(sort <<<${lines[@]:1:2}))" "${v[5]} ${v[6]}" "volume prune, with 1,2,3 in use and 4 filtered out, lists 5,6" + is "$(echo $(sort <<<${lines[*]:1:2}))" "${v[5]} ${v[6]}" "volume prune, with 1,2,3 in use and 4 filtered out, lists 5,6" # prune should remove v4 run_podman volume prune --force diff --git a/test/system/400-unprivileged-access.bats b/test/system/400-unprivileged-access.bats index 0d6be2d60..d70c95973 100644 --- a/test/system/400-unprivileged-access.bats +++ b/test/system/400-unprivileged-access.bats @@ -119,7 +119,7 @@ EOF # Some of the above may not exist on our host. Find only the ones that do. local -a subset=() - for mp in ${mps[@]}; do + for mp in "${mps[@]}"; do if [ -e $mp ]; then subset+=($mp) fi @@ -128,7 +128,7 @@ EOF # Run 'stat' on all the files, plus /dev/null. Get path, file type, # number of links, major, and minor (see below for why). Do it all # in one go, to avoid multiple podman-runs - run_podman '?' run --rm $IMAGE stat -c'%n:%F:%h:%T:%t' /dev/null ${subset[@]} + run_podman '?' run --rm $IMAGE stat -c'%n:%F:%h:%T:%t' /dev/null "${subset[@]}" assert $status -le 1 "stat exit status: expected 0 or 1" local devnull= |