Prune from JVM hook if Ryuk is disabled #4960
Conversation
a.k.a. Poor Man's Ryuk :D
| .exec(); | ||
|
|
||
| containers.parallelStream().forEach(container -> { | ||
| stopContainer(container.getId(), container.getImage()); |
There was a problem hiding this comment.
I wonder if the stopContainer method name has become misleading and should be renamed. When I read this, my first thought was that this overall switch construct would stop containers but not explicitly remove them.
There was a problem hiding this comment.
stop behaves like this for a long time (since forever?) of course (and differentiates from the semantics of Docker's container stop). So while I was always in favor of aligning our domain language to the Docker one, by now this would a such a breaking change of semantics in our API, I am unsure how to easily deliver it?
There was a problem hiding this comment.
stopContainer specifically is a private method, and IIRC is only called by this and a stopAndRemove (public) method.
So maybe more do-able?
(+100 for aliging to docker's domain language)
There was a problem hiding this comment.
Happy to add it to the scope - do you have naming recommendations? reapContainer? :D
There was a problem hiding this comment.
@rnorth Ok you are right, I was mixing it up with our public stop() method, which will call stopAndRemoveContainer (which I consider confusing but hard to change).
@bsideup WDTY about just removeContainer, since this would be aligned with the methods for other Docker resources?
public synchronized void performCleanup() {
registeredContainers.forEach(this::stopContainer);
registeredNetworks.forEach(this::removeNetwork);
registeredImages.forEach(this::removeImage);
}
Co-authored-by: Richard North <rich.north@gmail.com>
a.k.a. Poor Man's Ryuk :D