Skip to content

Return zero exit-code when force-removing non-existing containers#2678

Merged
cpuguy83 merged 1 commit into
docker:masterfrom
thaJeztah:fix_rm_force_exit_status
Aug 27, 2020
Merged

Return zero exit-code when force-removing non-existing containers#2678
cpuguy83 merged 1 commit into
docker:masterfrom
thaJeztah:fix_rm_force_exit_status

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Aug 14, 2020

Copy link
Copy Markdown
Member

When using docker rm / docker container rm with the -f / --force option, attempts to remove non-existing containers should print a warning, but should return a zero exit code ("successful").

Currently, a non-zero exit code is returned, marking the removal as "failed";

$ docker rm -fv 798c9471b695
Error: No such container: 798c9471b695
$ echo $?
1

The command should match the behavior of rm / rm -f, with the exception that
a warning is printed (instead of silently ignored):

Running rm with -f silences output and returns a zero exit code:

touch some-file && rm -f no-such-file some-file; echo exit code: $?; ls -la
# exit code: 0
# total 0
# drwxr-xr-x    2 sebastiaan  staff    64 Aug 14 12:17 .
# drwxr-xr-x  199 sebastiaan  staff  6368 Aug 14 12:13 ..

mkdir some-directory && rm -rf no-such-directory some-directory; echo exit code: $?; ls -la
# exit code: 0
# total 0
# drwxr-xr-x    2 sebastiaan  staff    64 Aug 14 12:17 .
# drwxr-xr-x  199 sebastiaan  staff  6368 Aug 14 12:13 ..

Note that other reasons for a delete to fail should still result in a non-zero
exit code, matching the behavior of rm. For instance, in the example below,
the rm failed because directories can only be removed if the -r option is used;

touch some-file && mkdir some-directory && rm -f some-directory no-such-file some-file; echo exit code: $?; ls -la
# rm: some-directory: is a directory
# exit code: 1
# total 0
# drwxr-xr-x    3 sebastiaan  staff    96 Aug 14 14:15 .
# drwxr-xr-x  199 sebastiaan  staff  6368 Aug 14 12:13 ..
# drwxr-xr-x    2 sebastiaan  staff    64 Aug 14 14:15 some-directory

This patch updates the docker rm / docker container rm command to not produce
an error when attempting to remove a missing containers, and instead only print
the error, but return a zero (0) exit code.

With this patch applied:

docker create --name mycontainer busybox \
&& docker rm nosuchcontainer mycontainer; \
echo exit code: $?; \
docker ps -a --filter name=mycontainer
# df23cc8573f00e97d6e948b48d9ea7d75ce3b4faaab4fe1d3458d3bfa451f39d
# mycontainer
# Error: No such container: nosuchcontainer
# exit code: 0
# CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES

Signed-off-by: Sebastiaan van Stijn github@gone.nl

- What I did

- How I did it

- How to verify it

- Description for the changelog

Fix `docker rm --force` returning a non-zero exit code if one or more containers did not exist

- A picture of a cute animal (not mandatory but encouraged)

When using `docker rm` / `docker container rm` with the `-f` / `--force` option, attempts to remove non-existing containers should print a warning, but should return a zero exit code ("successful").

Currently, a non-zero exit code is returned, marking the removal as "failed";

	$ docker rm -fv 798c9471b695
	Error: No such container: 798c9471b695
	$ echo $?
	1

The command should match the behavior of `rm` / `rm -f`, with the exception that
a warning is printed (instead of silently ignored):

Running `rm` with `-f` silences output and returns a zero exit code:

    touch some-file && rm -f no-such-file some-file; echo exit code: $?; ls -la
    # exit code: 0
    # total 0
    # drwxr-xr-x    2 sebastiaan  staff    64 Aug 14 12:17 .
    # drwxr-xr-x  199 sebastiaan  staff  6368 Aug 14 12:13 ..

    mkdir some-directory && rm -rf no-such-directory some-directory; echo exit code: $?; ls -la
    # exit code: 0
    # total 0
    # drwxr-xr-x    2 sebastiaan  staff    64 Aug 14 12:17 .
    # drwxr-xr-x  199 sebastiaan  staff  6368 Aug 14 12:13 ..

Note that other reasons for a delete to fail should still result in a non-zero
exit code, matching the behavior of `rm`. For instance, in the example below,
the `rm` failed because directories can only be removed if the `-r` option is used;

    touch some-file && mkdir some-directory && rm -f some-directory no-such-file some-file; echo exit code: $?; ls -la
    # rm: some-directory: is a directory
    # exit code: 1
    # total 0
    # drwxr-xr-x    3 sebastiaan  staff    96 Aug 14 14:15 .
    # drwxr-xr-x  199 sebastiaan  staff  6368 Aug 14 12:13 ..
    # drwxr-xr-x    2 sebastiaan  staff    64 Aug 14 14:15 some-directory

This patch updates the `docker rm` / `docker container rm` command to not produce
an error when attempting to remove a missing containers, and instead only print
the error, but return a zero (0) exit code.

With this patch applied:

    docker create --name mycontainer busybox \
    && docker rm nosuchcontainer mycontainer; \
    echo exit code: $?; \
    docker ps -a --filter name=mycontainer
    # df23cc8573f00e97d6e948b48d9ea7d75ce3b4faaab4fe1d3458d3bfa451f39d
    # mycontainer
    # Error: No such container: nosuchcontainer
    # exit code: 0
    # CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fix_rm_force_exit_status branch from adf6cd5 to 9a071a9 Compare August 14, 2020 14:17
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #2678 into master will increase coverage by 0.25%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2678      +/-   ##
==========================================
+ Coverage   58.16%   58.42%   +0.25%     
==========================================
  Files         295      295              
  Lines       21207    21210       +3     
==========================================
+ Hits        12336    12391      +55     
+ Misses       7963     7909      -54     
- Partials      908      910       +2     
@thaJeztah

Copy link
Copy Markdown
Member Author

@silvin-lubecki silvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cpuguy83 cpuguy83 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Probably we should do this in the API, though.

@cpuguy83 cpuguy83 merged commit 612567c into docker:master Aug 27, 2020
@thaJeztah thaJeztah deleted the fix_rm_force_exit_status branch August 27, 2020 22:41
@thaJeztah

Copy link
Copy Markdown
Member Author

Probably we should do this in the API, though

hm, possibly; also feels a bit odd to have the API ignore a non-existing resource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment