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

feature: add container prune command #2856

Closed

Conversation

hellolijj
Copy link
Contributor

Signed-off-by: Junjun Li junjunli666@gmail.com

Ⅰ. Describe what this PR did

add pouch container prune command

Ⅱ. Does this pull request fix one issue?

#2213

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

$ pouch container prune
WARNING! This will remove all stopped containers.
Are you sure you want to continue? [y/N]y
Deleted Containers:
9beaac36a373b7f0bf9d26887cc4cc16b4b287ec4d411da98b7220ea985f5ed8

Total reclaimed space: 34B

Ⅴ. Special notes for reviews

@codecov
Copy link

codecov bot commented May 21, 2019

Codecov Report

Merging #2856 into master will increase coverage by 0.02%.
The diff coverage is 58.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2856      +/-   ##
==========================================
+ Coverage   67.21%   67.23%   +0.02%     
==========================================
  Files         291      293       +2     
  Lines       18238    18284      +46     
==========================================
+ Hits        12258    12294      +36     
- Misses       4525     4527       +2     
- Partials     1455     1463       +8
Flag Coverage Δ
#criv1alpha2_test 38.29% <2.17%> (-0.06%) ⬇️
#integration_test_1 36.02% <41.3%> (+0.39%) ⬆️
#integration_test_2 35.95% <2.17%> (-0.23%) ⬇️
#integration_test_3 35.71% <2.17%> (-0.01%) ⬇️
#node_e2e_test 34.07% <2.17%> (-0.1%) ⬇️
#unittest 28.01% <17.39%> (-0.04%) ⬇️
Impacted Files Coverage Δ
daemon/mgr/container.go 57.75% <ø> (-0.63%) ⬇️
daemon/mgr/container_state.go 100% <100%> (ø) ⬆️
apis/server/router.go 90.36% <100%> (+0.05%) ⬆️
client/container_prune.go 53.33% <53.33%> (ø)
daemon/mgr/container_prune.go 53.33% <53.33%> (ø)
apis/server/container_bridge.go 78.82% <61.53%> (-0.07%) ⬇️
ctrd/supervisord/daemon.go 49.32% <0%> (-1.36%) ⬇️
ctrd/container.go 50.47% <0%> (-0.39%) ⬇️
... and 7 more

continue
}

if c.SizeRw > 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, I take c.SizeRw as reclaimed space. is it right?

cli/container.go Outdated Show resolved Hide resolved
cli/container_prune.go Outdated Show resolved Hide resolved
cli/container_prune.go Outdated Show resolved Hide resolved
client/container_prune_test.go Show resolved Hide resolved
client/container_prune_test.go Outdated Show resolved Hide resolved
daemon/mgr/container_prune.go Outdated Show resolved Hide resolved
cli/container_prune.go Outdated Show resolved Hide resolved
@hellolijj hellolijj force-pushed the dev-prune-container branch from 0926c4d to 8e73f56 Compare May 29, 2019 07:56
@ZYecho ZYecho changed the title add container prune command feature: add container prune command Jun 3, 2019
daemon/mgr/container_prune.go Outdated Show resolved Hide resolved
daemon/mgr/container_prune.go Outdated Show resolved Hide resolved
test/api_container_prune_test.go Show resolved Hide resolved
test/cli_container_prune_test.go Outdated Show resolved Hide resolved
test/cli_container_prune_test.go Outdated Show resolved Hide resolved
}

// TestContainerPruneWork tests "pouch container prune" work.
func (suite *PouchContainerPruneSuite) TestContainerPruneWork(c *check.C) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could we add a test case for filter usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all right

Copy link
Contributor

Choose a reason for hiding this comment

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

why only add invalid test case here?

@@ -33,6 +33,11 @@ func (c *Container) IsRemoving() bool {
return c.State.Status == types.StatusRemoving
}

// IsStopped returns container is stopped or not.
func (c *Container) IsStopped() bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

just to confirm, does docker only delete stopped container?

Signed-off-by: Junjun Li <junjunli666@gmail.com>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hellolijj hellolijj closed this Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants