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

return prune data when context canceled #33979

Merged
merged 1 commit into from Jul 11, 2017

Conversation

Projects
None yet
6 participants
@allencloud
Contributor

allencloud commented Jul 6, 2017

Signed-off-by: allencloud allen.sun@daocloud.io

This PR takes into user's cancel request when prune data into consideration. When context is canceled, docker daemon still returns the data which is already pruned.

Prune Object includes: container, network, image, volume, and build cache.

fixes #33957

- What I did

  1. make engine return prune data even if context is canceled.

- How I did it

- How to verify it

- Description for the changelog

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

/cc @mlaventure

Show outdated Hide outdated daemon/prune.go
@mlaventure

LGTM

Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated builder/fscache/fscache.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Jul 9, 2017

Contributor

Thanks for your review again. @cpuguy83
I have updated that, PTAL.

Contributor

allencloud commented Jul 9, 2017

Thanks for your review again. @cpuguy83
I have updated that, PTAL.

@thaJeztah

Left some questions / comments, thanks!

Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
Show outdated Hide outdated daemon/prune.go
@@ -427,8 +429,8 @@ func (daemon *Daemon) NetworksPrune(ctx context.Context, pruneFilters filters.Ar
select {
case <-ctx.Done():
logrus.Warnf("NetworksPrune operation cancelled: %#v", *rep)
return nil, ctx.Err()
logrus.Debugf("NetworksPrune operation cancelled: %#v", *rep)

This comment has been minimized.

@thaJeztah

thaJeztah Jul 10, 2017

Member

I see @cpuguy83 suggested "debug"; just wondering if changing from "warn" in the current code to "debug" is too big a change, and if "info" would be a better fit.

Not a show stopper, just thinking aloud

@thaJeztah

thaJeztah Jul 10, 2017

Member

I see @cpuguy83 suggested "debug"; just wondering if changing from "warn" in the current code to "debug" is too big a change, and if "info" would be a better fit.

Not a show stopper, just thinking aloud

This comment has been minimized.

@mlaventure

mlaventure Jul 10, 2017

Contributor

I think debug is fine. Not sure if that message being in the log by default has any benefits

@mlaventure

mlaventure Jul 10, 2017

Contributor

I think debug is fine. Not sure if that message being in the log by default has any benefits

return prune data when context canceled
Signed-off-by: allencloud <allen.sun@daocloud.io>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 10, 2017

Member

hm, flaky? https://jenkins.dockerproject.org/job/Docker-PRs/44175/console

03:33:45 ----------------------------------------------------------------------
03:33:45 FAIL: docker_cli_swarm_test.go:1146: DockerSwarmSuite.TestSwarmLockUnlockCluster
03:33:45 
03:33:45 [d6209096216f9] waiting for daemon to start
03:33:45 [d6209096216f9] daemon started
03:33:45 
03:33:45 [df75e109fdee3] waiting for daemon to start
03:33:45 [df75e109fdee3] daemon started
03:33:45 
03:33:45 [ded4392fb23e8] waiting for daemon to start
03:33:45 [ded4392fb23e8] daemon started
03:33:45 
03:33:45 [df75e109fdee3] exiting daemon
03:33:45 [df75e109fdee3] waiting for daemon to start
03:33:45 [df75e109fdee3] daemon started
03:33:45 
03:33:45 [df75e109fdee3] exiting daemon
03:33:45 [d6209096216f9] exiting daemon
03:33:45 [d6209096216f9] waiting for daemon to start
03:33:45 [d6209096216f9] daemon started
03:33:45 
03:33:45 [ded4392fb23e8] exiting daemon
03:33:45 [ded4392fb23e8] waiting for daemon to start
03:33:45 [ded4392fb23e8] daemon started
03:33:45 
03:33:45 [df75e109fdee3] waiting for daemon to start
03:33:45 [df75e109fdee3] daemon started
03:33:45 
03:33:45 docker_cli_swarm_test.go:1192:
03:33:45     // d2 is now set to lock
03:33:45     checkSwarmUnlockedToLocked(c, d2)
03:33:45 docker_utils_test.go:471:
03:33:45     c.Assert(v, checker, args...)
03:33:45 ... obtained bool = false
03:33:45 ... expected bool = true
03:33:45 
03:33:45 [d6209096216f9] exiting daemon
03:33:45 [df75e109fdee3] exiting daemon
03:33:45 [ded4392fb23e8] exiting daemon
03:34:06 
Member

thaJeztah commented Jul 10, 2017

hm, flaky? https://jenkins.dockerproject.org/job/Docker-PRs/44175/console

03:33:45 ----------------------------------------------------------------------
03:33:45 FAIL: docker_cli_swarm_test.go:1146: DockerSwarmSuite.TestSwarmLockUnlockCluster
03:33:45 
03:33:45 [d6209096216f9] waiting for daemon to start
03:33:45 [d6209096216f9] daemon started
03:33:45 
03:33:45 [df75e109fdee3] waiting for daemon to start
03:33:45 [df75e109fdee3] daemon started
03:33:45 
03:33:45 [ded4392fb23e8] waiting for daemon to start
03:33:45 [ded4392fb23e8] daemon started
03:33:45 
03:33:45 [df75e109fdee3] exiting daemon
03:33:45 [df75e109fdee3] waiting for daemon to start
03:33:45 [df75e109fdee3] daemon started
03:33:45 
03:33:45 [df75e109fdee3] exiting daemon
03:33:45 [d6209096216f9] exiting daemon
03:33:45 [d6209096216f9] waiting for daemon to start
03:33:45 [d6209096216f9] daemon started
03:33:45 
03:33:45 [ded4392fb23e8] exiting daemon
03:33:45 [ded4392fb23e8] waiting for daemon to start
03:33:45 [ded4392fb23e8] daemon started
03:33:45 
03:33:45 [df75e109fdee3] waiting for daemon to start
03:33:45 [df75e109fdee3] daemon started
03:33:45 
03:33:45 docker_cli_swarm_test.go:1192:
03:33:45     // d2 is now set to lock
03:33:45     checkSwarmUnlockedToLocked(c, d2)
03:33:45 docker_utils_test.go:471:
03:33:45     c.Assert(v, checker, args...)
03:33:45 ... obtained bool = false
03:33:45 ... expected bool = true
03:33:45 
03:33:45 [d6209096216f9] exiting daemon
03:33:45 [df75e109fdee3] exiting daemon
03:33:45 [ded4392fb23e8] exiting daemon
03:34:06 
@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Jul 11, 2017

Contributor

Great to see all green. ☀️ 😄

Contributor

allencloud commented Jul 11, 2017

Great to see all green. ☀️ 😄

@thaJeztah

LGTM

@cpuguy83 PTAL

@cpuguy83

LGTM

@cpuguy83 cpuguy83 merged commit 73e8f56 into moby:master Jul 11, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 35563 has succeeded
Details
janky Jenkins build Docker-PRs 44200 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 4548 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 15550 has succeeded
Details
z Jenkins build Docker-PRs-s390x 4251 has succeeded
Details

@allencloud allencloud deleted the allencloud:return-prune-data-when-context-canceled branch Jul 11, 2017

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