Skip to content

Conversation

@cpuguy83
Copy link
Member

Refactors volume driver storage to not use a global store.
This makes things a bit nicer to deal with, especially in tests which are now much more easily parallelized.

@codecov
Copy link

codecov bot commented Mar 22, 2018

Codecov Report

Merging #36637 into master will decrease coverage by 0.05%.
The diff coverage is 29.35%.

@@            Coverage Diff             @@
##           master   #36637      +/-   ##
==========================================
- Coverage    35.2%   35.15%   -0.06%     
==========================================
  Files         614      613       -1     
  Lines       45698    45623      -75     
==========================================
- Hits        16090    16040      -50     
+ Misses      27473    27463      -10     
+ Partials     2135     2120      -15

Copy link
Member

Choose a reason for hiding this comment

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

Does something else need to be updated, because this is generated code?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it just takes the package name from the package it's being run from.

Copy link
Member

Choose a reason for hiding this comment

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

This change should probably go into the other commit that removes the old code

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@anusha-ragunathan
Copy link
Contributor

Looks good overall.

The change description threw me off a bit. "No global volume driver store" can also be interpreted "No more metadata.db" which is a big change. Can you reword the description to mean that you are talking about an internal data structure change?

@cpuguy83
Copy link
Member Author

cpuguy83 commented Apr 3, 2018

@anusha-ragunathan This isn't removing metadata.db

@anusha-ragunathan
Copy link
Contributor

@cpuguy83 : Yes I know, but the change description can be misinterpreted to mean that. I'm just asking you to re-word the commit description. i.e "No global volume driver store" can be reworded.

@thaJeztah
Copy link
Member

ugh needs a rebase now 😥

Since the volume store already provides this functionality, we should
just use it rather than duplicating it.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Instead of using a global store for volume drivers, scope the driver
store to the caller (e.g. the volume store). This makes testing much
simpler.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83
Copy link
Member Author

Rebased and added more details to the commit message.

@cpuguy83 cpuguy83 force-pushed the no_global_driver_store branch from 75de57c to 977109d Compare April 17, 2018 18:08
@thaJeztah
Copy link
Member

Failure on s390 (https://jenkins.dockerproject.org/job/Docker-PRs-s390x/9399/console) is being tracked through #36877;

18:54:00 ----------------------------------------------------------------------
18:54:00 FAIL: docker_cli_exec_unix_test.go:18: DockerSuite.TestExecInteractiveStdinClose
18:54:00 
18:54:00 docker_cli_exec_unix_test.go:37:
18:54:00     c.Assert(strings.TrimSpace(output), checker.Equals, "hello")
18:54:00 ... obtained string = "hello\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
18:54:00 ... expected string = "hello"
18:54:00 

@thaJeztah
Copy link
Member

Failure on experimentalalso should not be related https://jenkins.dockerproject.org/job/Docker-PRs-experimental/40285/console:

19:20:37 ----------------------------------------------------------------------
19:20:37 FAIL: check_test.go:347: DockerSwarmSuite.TearDownTest
19:20:37 
19:20:37 check_test.go:352:
19:20:37     d.Stop(c)
19:20:37 /go/src/github.com/docker/docker/internal/test/daemon/daemon.go:375:
19:20:37     t.Fatalf("Error while stopping the daemon %s : %v", d.id, err)
19:20:37 ... Error: Error while stopping the daemon d503e943de173 : exit status 130
19:20:37 
19:20:37 
19:20:37 ----------------------------------------------------------------------
19:20:37 PANIC: docker_api_swarm_test.go:360: DockerSwarmSuite.TestAPISwarmRaftQuorum
19:20:37 
19:20:37 [d503e943de173] waiting for daemon to start
19:20:37 [d503e943de173] daemon started
19:20:37 
19:20:37 [d255549be4adf] waiting for daemon to start
19:20:37 [d255549be4adf] daemon started
19:20:37 
19:20:37 [dbc1b9063b506] waiting for daemon to start
19:20:37 [dbc1b9063b506] daemon started
19:20:37 
19:20:37 [d255549be4adf] exiting daemon
19:20:37 [dbc1b9063b506] exiting daemon
19:20:37 [d255549be4adf] waiting for daemon to start
19:20:37 [d255549be4adf] daemon started
19:20:37 
19:20:37 [d503e943de173] daemon started
19:20:37 Attempt #2: daemon is still running with pid 24530
19:20:37 Attempt #3: daemon is still running with pid 24530
19:20:37 Attempt #4: daemon is still running with pid 24530
19:20:37 [d503e943de173] exiting daemon
19:20:37 ... Panic: Fixture has panicked (see related PANIC)
19:20:37 

@anusha-ragunathan
Copy link
Contributor

LGTM

@thaJeztah
Copy link
Member

Experimental failing again, on another test; https://jenkins.dockerproject.org/job/Docker-PRs-experimental/40287/console

22:37:31 FAIL: docker_api_swarm_service_test.go:616: DockerSwarmSuite.TestAPISwarmServicesPlugin
22:37:31 
22:37:31 [de70b26cd54f9] waiting for daemon to start
22:37:31 [de70b26cd54f9] daemon started
22:37:31 
22:37:31 [dcf6df5dd9e37] waiting for daemon to start
22:37:31 [dcf6df5dd9e37] daemon started
22:37:31 
22:37:31 [d9abf2e5613d9] waiting for daemon to start
22:37:31 [d9abf2e5613d9] daemon started
22:37:31 
22:37:31 docker_api_swarm_service_test.go:667:
22:37:31     waitAndAssert(c, defaultReconciliationTimeout, d3.CheckPluginRunning(name), checker.False)
22:37:31 docker_utils_test.go:435:
22:37:31     c.Assert(v, checker, args...)
22:37:31 ... obtained bool = true
22:37:31 ... &{Config:{Args:{Description: Name: Settable:[] Value:[]} Description: DockerVersion:dev Documentation: Entrypoint:[/basic] Env:[] Interface:{Socket:basic.sock Types:[.docker.dummy/1.0/]} IpcHost:false Linux:{AllowAllDevices:false Capabilities:[] Devices:[]} Mounts:[] Network:{Type:} PidHost:false PropagatedMount: User:{GID:0 UID:0} WorkDir: Rootfs:0xc423459ce0} Enabled:true ID:f8de84d69c24ebc51b24800c4f23152d225206e0336c63550baa3417b08da106 Name:test:latest PluginReference:127.0.0.1:5000/swarm/test:v2 Settings:{Args:[] Devices:[] Env:[] Mounts:[]}}
22:37:31 
22:37:31 [de70b26cd54f9] exiting daemon
22:37:31 [dcf6df5dd9e37] exiting daemon
22:37:31 [d9abf2e5613d9] exiting daemon
22:37:56 

@cpuguy83
Copy link
Member Author

09:17:27 FAIL: docker_cli_swarm_test.go:1372: DockerSwarmSuite.TestSwarmClusterRotateUnlockKey
09:17:27 
09:17:27 [d29e800d72ded] waiting for daemon to start
09:17:27 [d29e800d72ded] daemon started
09:17:27 
09:17:27 [de1846413ada2] waiting for daemon to start
09:17:27 [de1846413ada2] daemon started
09:17:27 
09:17:27 [dff9c430c0e5c] waiting for daemon to start
09:17:27 [dff9c430c0e5c] daemon started
09:17:27 
09:17:27 [de1846413ada2] exiting daemon
09:17:27 [de1846413ada2] waiting for daemon to start
09:17:27 [de1846413ada2] daemon started
09:17:27 
09:17:27 [dff9c430c0e5c] exiting daemon
09:17:27 [dff9c430c0e5c] waiting for daemon to start
09:17:27 [dff9c430c0e5c] daemon started
09:17:27 
09:17:27 [de1846413ada2] exiting daemon
09:17:27 [de1846413ada2] waiting for daemon to start
09:17:27 [de1846413ada2] daemon started
09:17:27 
09:17:27 [dff9c430c0e5c] exiting daemon
09:17:27 [dff9c430c0e5c] waiting for daemon to start
09:17:27 [dff9c430c0e5c] daemon started
09:17:27 
09:17:27 [de1846413ada2] exiting daemon
09:17:27 [de1846413ada2] waiting for daemon to start
09:17:27 [de1846413ada2] daemon started
09:17:27 
09:17:27 [dff9c430c0e5c] exiting daemon
09:17:27 [dff9c430c0e5c] waiting for daemon to start
09:17:27 [dff9c430c0e5c] daemon started
09:17:27 
09:17:27 docker_cli_swarm_test.go:1449:
09:17:27     c.Assert(err, checker.IsNil)
09:17:27 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc4204f64e0), Stderr:[]uint8(nil)} ("exit status 1")
09:17:27 
09:17:27 [d29e800d72ded] exiting daemon
09:17:27 [de1846413ada2] exiting daemon
09:17:27 [dff9c430c0e5c] exiting daemon

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.

8 participants