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

Add option to keep trying to remove containers, even if one fails #1251

Open
wants to merge 1 commit into
base: master
from

Conversation

@peterjeschke
Copy link

commented Aug 8, 2019

This PR adds an option to continue removing container and networks if one fails. A possible scenario is that the stop goal might try to remove multiple containers and one does not exist anymore (for whatever reason). This change will allow (with a new configuration option) to continue removing containers. The build will still fail though, to make the user aware of the problem.

Copy link
Collaborator

left a comment

Thanks for the PR ! I would even go so far and do not introduce an extra configuration option for that but use this as the default behaviour as this makes much sense for me in any case.

Could you please adapt the PR to remove the config option and continue the removal/stop process always ?

Signed-off-by: Jeschke, Peter Rene <jeschke@gemtec.eu>
@peterjeschke peterjeschke force-pushed the Gemtec:continueShutdown branch from 4399681 to 8e451f8 Aug 13, 2019
@codecov

This comment has been minimized.

Copy link

commented Aug 13, 2019

Codecov Report

Merging #1251 into master will decrease coverage by 2.15%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #1251      +/-   ##
============================================
- Coverage     55.53%   53.37%   -2.16%     
+ Complexity     1762     1637     -125     
============================================
  Files           156      155       -1     
  Lines          8474     8331     -143     
  Branches       1302     1280      -22     
============================================
- Hits           4706     4447     -259     
- Misses         3314     3441     +127     
+ Partials        454      443      -11
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/io/fabric8/maven/docker/StopMojo.java 0% <0%> (-65.75%) 0 <0> (-33)
...va/io/fabric8/maven/docker/service/RunService.java 46.41% <0%> (-5.71%) 25 <0> (ø)
.../main/java/io/fabric8/maven/docker/RemoveMojo.java 0% <0%> (-96.73%) 0% <0%> (-33%)
.../io/fabric8/maven/docker/util/NamePatternUtil.java 84.61% <0%> (-13.57%) 12% <0%> (-16%)
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java 77.02% <0%> (-10.82%) 34% <0%> (-4%)
...va/io/fabric8/maven/docker/AbstractDockerMojo.java 2.94% <0%> (-8.83%) 3% <0%> (-5%)
...aven/docker/model/ImageArchiveManifestAdapter.java 83.33% <0%> (-8.34%) 5% <0%> (-1%)
...io/fabric8/maven/docker/util/ImageArchiveUtil.java 86.95% <0%> (-7.25%) 31% <0%> (-2%)
...abric8/maven/docker/config/ImageConfiguration.java 53.52% <0%> (-6.22%) 15% <0%> (-5%)
... and 16 more
@codecov

This comment has been minimized.

Copy link

commented Aug 13, 2019

Codecov Report

Merging #1251 into master will decrease coverage by 0.2%.
The diff coverage is 16.66%.

@@             Coverage Diff              @@
##             master    #1251      +/-   ##
============================================
- Coverage     55.53%   55.32%   -0.21%     
  Complexity     1762     1762              
============================================
  Files           156      156              
  Lines          8474     8513      +39     
  Branches       1302     1309       +7     
============================================
+ Hits           4706     4710       +4     
- Misses         3314     3348      +34     
- Partials        454      455       +1
Impacted Files Coverage Δ Complexity Δ
...va/io/fabric8/maven/docker/service/RunService.java 47.49% <0%> (-4.63%) 25 <0> (ø)
...rc/main/java/io/fabric8/maven/docker/StopMojo.java 60.48% <40%> (-5.26%) 33 <0> (ø)
@peterjeschke

This comment has been minimized.

Copy link
Author

commented Aug 13, 2019

@rhuss I removed the configuration option and updated the branch to the latest commit on master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.