Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm --rm and -d #10545

Closed
timbertson opened this issue Feb 4, 2015 · 18 comments
Closed

rm --rm and -d #10545

timbertson opened this issue Feb 4, 2015 · 18 comments
Assignees
Labels
area/runtime kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny

Comments

@timbertson
Copy link

It would be good if I could supply a flag to docker rm to signal that I don't care if the container doesn't exist, rather than have it fail (and then parsing the error message to make sure it failed with "No such container").

-f would fit with rm, but it's not exactly forcing anything - perhaps something more explicit like --ignore-missing would be better (and may be applicable to more commands in the future).


Rationale:

  • docker run --rm, for various reasons, is not always reliable - a common workaround is to run docker rm <container> before I run docker run --rm <container>, to ensure that a previous unclean exit doesn't prevent me from starting a container with the same name.
  • even when you're not using --rm, and are just using well-known container names, you often have reason to "remove container if it exists, ignore otherwise". e.g when using --detached you can't use --rm anyway, so you need to perform manual cleanup of stopped containers at some point. Most methods of scripting this are racey, which is why I think rm should handle it internally.
@timbertson
Copy link
Author

This might be helpful for #7245 as well (at least for the --rm=dead proposal).

@cpuguy83
Copy link
Member

cpuguy83 commented Feb 4, 2015

Why not just ignore the error?

@timbertson
Copy link
Author

That's what I'm currently doing, but docker can fail for plenty of reasons that I don't want to ignore (an obvious example is a failure to connect to docker.sock, which isn't that rare).

@jessfraz
Copy link
Contributor

closing as a duplicate of #6757

@timbertson
Copy link
Author

@jfrazelle I don't think this is a duplicate. In fact, I've since stopped using --rm in my own code (because I need to use --detached), and still have exactly the same need for this feature. Making --rm better isn't really the point of this issue.

@jessfraz jessfraz reopened this Feb 10, 2015
@jessfraz
Copy link
Contributor

I reopened

@timbertson
Copy link
Author

Thanks :)

@jessfraz
Copy link
Contributor

Can you put the need for -d and --rm so others don't get confused because the initial comment is in fact the same issue as the other

@cpuguy83 cpuguy83 reopened this Feb 10, 2015
@cpuguy83
Copy link
Member

Dang phone closed it...

Anyway... Was thinking about a restart policy called "remove", wdyt?

@timbertson
Copy link
Author

Updated description to de-emphasize the relevance of docker run --rm.

@cpuguy83 is that so that you could have a reliable alternative to --rm even when you're using --detached? That could be useful, but feels a little hacky (it's hardly a "restart action", and took me a while to realize what you even meant).

I think the explicit rm is still going to be needed in some cases - currently whenever you use docker rm on a live system it's going to be racey, so that seems worth improving regardless of how well --rm and similar features work.

@estesp
Copy link
Contributor

estesp commented Feb 11, 2015

I've been looking at the various --rm issues recently, and I think one confusion is that there are two separate problems, sometimes conflated as the same issue.

Issue 1: --rm is a client-side action, using the API. Therefore it conflicts with -d as the client is long gone when it is time to remove the container.

Issue 2: server-side deletion can fail, leaving a mix of invalid state or content (leftover filesystem pieces, mounts, etc.).

I think the "right" fix for issue 1 that @duglin and I have both mentioned in various arenas is to make --rm generate a server-side action upon container exit. Container exit is a well-defined event, and a simple addition to the container.Config at create time (e.g. docker run) based on the existence of --rm will allow the server to perform deletion upon exit without any concern for detached vs. non-detached. That also fixes all problems with client errors or some other action getting in the way of the client successfully calling POST /delete.

Issue 2 has no "perfect fix" but can be mitigated by more robust deletion logic that tries as much as possible to perform the delete, handling errors as necessary or where feasible. @crosbymichael made this clear in the recent PR #10099 closure (#10099 (comment))

Pinging @jfrazelle @duglin and @tibor as I know this was discussed on IRC earlier but I wasn't around to give my 2c :)

@estesp
Copy link
Contributor

estesp commented Feb 11, 2015

...and maybe I should have added this comment to the other not-really-a-dup issue given @gfxmonk made it clear he is looking for "ignore-errors" flag to docker rm; although hopefully if --rm was valid with -d then your need for a docker rm that ignored errors goes down to some degree?

@jessfraz
Copy link
Contributor

Haha yes no worries about placement but I agree with you on all the above

On Tuesday, February 10, 2015, Phil Estes notifications@github.com wrote:

...and maybe I should have added this comment to the other
not-really-a-dup issue given @gfxmonk https://github.com/gfxmonk made
it clear he is looking for "ignore-errors" flag to docker rm; although
hopefully if --rm was valid with -d then your need for a docker rm that
ignored errors goes down to some degree?


Reply to this email directly or view it on GitHub
#10545 (comment).

@timbertson
Copy link
Author

although hopefully if --rm was valid with -d then your need for a docker rm that ignored errors goes down to some degree

That would be a convenient feature (and I'd use it), but unless it's completely impossible for --rm to ever fail, I'd still need the feature I described :)

@duglin
Copy link
Contributor

duglin commented Feb 11, 2015

Just to pull threads together, there's also: #10688

@jessfraz jessfraz changed the title idempotent rm rm --rm and -d Feb 17, 2015
@jessfraz jessfraz added Proposal kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny labels Feb 28, 2015
@duglin
Copy link
Contributor

duglin commented Jun 10, 2015

See PR #13855

@duglin duglin self-assigned this Jun 10, 2015
@jessfraz jessfraz added kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny and removed kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny kind/proposal labels Sep 8, 2015
@icecrime
Copy link
Contributor

--rm is now handled on the daemon-side, thanks!

@thaJeztah
Copy link
Member

Implemented in #20848 (Docker 1.13 and up)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/runtime kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny
Projects
None yet
Development

No branches or pull requests

7 participants