Rename graph drivers to storage drivers #22228

samoht · 2016-04-21T16:24:16Z

- What I did

I've renamed the occurrences of GraphDriver by StorageDriver in the code base to be more consistent with the docs.

The only externally-visible change of that PR is a direct API translation of the experimental storage driver plugins API

- How I did it

I've renamed things via grep/replace but I tried to do that carefully, so variable names and comments are also kept consistent.

- How to verify it

I've run make and make test and it seems to work. I need to update docker/go-plugins-helpers#42 to use the new StorageDriver naming scheme, I'll do that once/if that PR is merged.

I've tried to run the integration tests for the CLI. The experimental storage driver plugins seem broken (see below).

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

samoht · 2016-04-23T15:34:27Z

Hum I seem to have broken the integration-cli tests for experimental storage drivers. I'll investigate that next week:

----------------------------------------------------------------------
14:16:58 FAIL: docker_cli_external_storage_driver_unix_test.go:340: DockerExternalStorageDriverSuite.TestExternalStorageDriver
14:16:58 
14:16:58 [d51499870] waiting for daemon to start
14:16:58 [d51499870] exiting daemon
14:16:58 docker_cli_external_storage_driver_unix_test.go:341:
14:16:58     s.testExternalStorageDriver("test-external-storage-driver", "spec", c)
14:16:58 docker_cli_external_storage_driver_unix_test.go:348:
14:16:58     c.Assert(err, check.IsNil, check.Commentf("\n%s", string(b)))
14:16:58 ... value *errors.errorString = &errors.errorString{s:"[d51499870] Daemon exited during startup"} ("[d51499870] Daemon exited during startup")
14:16:58 ... 
14:16:58 time="2016-04-23T14:16:58Z" level=warning msg="Running experimental build" 
14:16:58 time="2016-04-23T14:16:58.364985085Z" level=debug msg="docker group found. gid: 999" 
14:16:58 time="2016-04-23T14:16:58.365020773Z" level=debug msg="Listener created for HTTP on unix (/go/src/github.com/docker/docker/bundles/1.12.0-dev/test-integration-cli/d51499870/docker.sock)" 
14:16:58 time="2016-04-23T14:16:58.366134145Z" level=debug msg="Using default logging driver json-file" 
14:16:58 time="2016-04-23T14:16:58.366173315Z" level=debug msg="Golang's threads limit set to 114570" 
14:16:58 time="2016-04-23T14:16:58.367042471Z" level=debug msg="containerd connection state change: READY" 
14:16:58 time="2016-04-23T14:16:58.367453406Z" level=debug msg="received past containerd event: &types.Event{Type:\"live\", Id:\"\", Status:0x0, Pid:\"\", Timestamp:0x571b83da}" 
14:16:58 time="2016-04-23T14:16:58.419219301Z" level=debug msg="[storage] trying provided driver \"test-external-storage-driver\"" 
14:16:58 time="2016-04-23T14:16:58.419982126Z" level=error msg="Failed to GetDriver storage test-external-storage-driver /go/src/github.com/docker/docker/bundles/1.12.0-dev/test-integration-cli/d51499870/root" 
14:16:58 time="2016-04-23T14:16:58.420009911Z" level=debug msg="Cleaning up old mountid : start." 
14:16:58 time="2016-04-23T14:16:58.420061568Z" level=fatal msg="Error starting daemon: error initializing storage: driver not supported" 
14:16:58

thaJeztah · 2016-05-12T00:16:01Z

ping @samoht have you had time to look into those failing tests?

samoht · 2016-05-12T14:22:00Z

I plan to look at that again in the following days. Do you think that renaming is worth continuing?

thaJeztah · 2016-05-12T14:26:00Z

Hm, not sure, I know @tiborvass proposed doing so in the past, but not sure if it's really important (well, it's more consistent probably)

samoht · 2016-05-12T14:30:32Z

ok, so I'll rebase and see what I can do with the failing tests.

thaJeztah · 2016-05-12T18:48:34Z

ok! We discussed this, and we like it, so moving to code review. Would be nice if we could give the graph driver plugin maintainers a notice :)

cpuguy83 · 2016-05-13T14:02:52Z

ping @kolyshkin

samoht · 2016-05-13T17:35:46Z

I think I've fixed the docker_cli_external_storage_driver_unix_test issue.

Normally, all my changes are not visible from the user, apart the changes to the experimental storage driver plugins. The remaining bits where GraphDriver still appears are:

in docker/engine-api
and in docker inspect

I don't plan to change theses. Next step for me is to update docker/go-plugins-helpers#42

kolyshkin · 2016-05-13T17:44:14Z

LGTM

samoht · 2016-05-13T19:56:58Z

A interesting new error for wind2lin:

19:22:28 + git clone https://github.com/docker/notary.git /tmp/tmp.vCngyuJWWd/src/github.com/docker/notary
19:22:28 Cloning into '/tmp/tmp.vCngyuJWWd/src/github.com/docker/notary'...
19:32:30 error: RPC failed; result=56, HTTP code = 200
19:32:30 fatal: The remote end hung up unexpectedly
19:32:30 fatal: early EOF
19:32:30 fatal: index-pack failed

For TP5, I'm not sure where this comes from...

17:42:03 ---> Making bundle: .ensure-frozen-images-windows (in bundles/1.12.0-dev/test-integration-cli)
17:42:03 ERROR: Could not find windowsservercore images

tiborvass · 2016-05-16T13:55:22Z

LGTM

This is already the case in most of the CLI. This PR updates the codebase to be more consistent. This should not change anything for the user. Signed-off-by: Thomas Gazagnaire <[email protected]>

Signed-off-by: Thomas Gazagnaire <[email protected]>

This is to keep consistent with the name that we are using in the documentation, where "storage drivers" is prefered over the now deprecated "graph drivers" terminology. Signed-off-by: Thomas Gazagnaire <[email protected]>

Signed-off-by: Thomas Gazagnaire <[email protected]>

This match with what the documentation for `--graph` says. Signed-off-by: Thomas Gazagnaire <[email protected]>

Signed-off-by: Thomas Gazagnaire <[email protected]>

samoht · 2016-05-16T23:16:33Z

Rebased on master, and got new funky errors (yay):

23:14:23 docker: Error response from daemon: devmapper: Error mounting '/dev/mapper/docker-202:1-75497728-1220c22c26ac33994ddca2ef67a0e6d2abb2f46ef1b6069e92e32469063ddd87-init' on '/var/lib/docker/devicemapper/mnt/1220c22c26ac33994ddca2ef67a0e6d2abb2f46ef1b6069e92e32469063ddd87-init': no space left on device.

samoht · 2016-05-19T23:26:47Z

Still an issue with TP5:

ERROR: Could not find windowsservercore images

Is that something expected?

thaJeztah · 2016-05-19T23:49:18Z

Restarted the Windows build, doesn't look like its related

thaJeztah · 2016-06-03T00:33:23Z

oh boy; this needs a rebase now

thaJeztah · 2016-06-10T15:14:47Z

let me close this for now, because it's carried in #23237, but we still need to figure out the Windows issue 😢

GordonTheTurtle added the status/0-triage label Apr 21, 2016

samoht force-pushed the storage-drivers branch from f0b26fe to 5aac54d Compare April 21, 2016 17:21

samoht changed the title ~~Storage drivers~~ Rename graph drivers to storage drivers Apr 21, 2016

samoht force-pushed the storage-drivers branch 4 times, most recently from b50f38f to f49deef Compare April 23, 2016 13:44

thaJeztah added status/failing-ci Indicates that the PR in its current state fails the test suite status/1-design-review and removed status/0-triage labels Apr 25, 2016

samoht force-pushed the storage-drivers branch from f49deef to edcf328 Compare May 12, 2016 16:00

thaJeztah added status/2-code-review and removed status/1-design-review labels May 12, 2016

samoht force-pushed the storage-drivers branch 2 times, most recently from 77c3f88 to c768b24 Compare May 13, 2016 09:53

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 13, 2016

samoht force-pushed the storage-drivers branch from c768b24 to 0461640 Compare May 13, 2016 09:54

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 13, 2016

samoht force-pushed the storage-drivers branch from 0461640 to 7ac43c5 Compare May 13, 2016 10:08

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 13, 2016

samoht force-pushed the storage-drivers branch 3 times, most recently from a03d204 to 1561a8f Compare May 13, 2016 17:13

samoht force-pushed the storage-drivers branch from afb395d to 6d8ab5e Compare May 13, 2016 17:31

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 13, 2016

samoht force-pushed the storage-drivers branch from 6d8ab5e to 548ebe3 Compare May 13, 2016 17:32

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 13, 2016

samoht force-pushed the storage-drivers branch from 548ebe3 to dc2fc2f Compare May 13, 2016 17:37

samoht added 9 commits May 16, 2016 16:12

Rename Graphdriver into Storage Drivers in daemon/

95bbfdd

This is already the case in most of the CLI. This PR updates the codebase to be more consistent. This should not change anything for the user. Signed-off-by: Thomas Gazagnaire <[email protected]>

Rename GraphDrivers into Storage Drivers in layer/

5ce62b6

This is already the case in most of the CLI. This PR updates the codebase to be more consistent. This should not change anything for the user. Signed-off-by: Thomas Gazagnaire <[email protected]>

Rename GraphDriver into Storage Driver in pkg/archive comments

2c59872

Signed-off-by: Thomas Gazagnaire <[email protected]>

Rename GraphDriver in Storage Driver in container/ comments

f286a72

Signed-off-by: Thomas Gazagnaire <[email protected]>

Rename GraphDriver into StorageDriver in contrib/

cce36df

Signed-off-by: Thomas Gazagnaire <[email protected]>

Rename experimental GraphDriver plugin API to StorageDriver

61ac716

This is to keep consistent with the name that we are using in the documentation, where "storage drivers" is prefered over the now deprecated "graph drivers" terminology. Signed-off-by: Thomas Gazagnaire <[email protected]>

Update integration-cli tests to GraphDriver => StorageDriver API changes

4a527f1

Signed-off-by: Thomas Gazagnaire <[email protected]>

Rename defaultGraph to defaultRuntimeRoot in the daemon.

2d1d799

This match with what the documentation for `--graph` says. Signed-off-by: Thomas Gazagnaire <[email protected]>

Update internal docs to refer to storage driver instead of graphdriver

4fdd20a

Signed-off-by: Thomas Gazagnaire <[email protected]>

samoht force-pushed the storage-drivers branch from dc2fc2f to 4fdd20a Compare May 16, 2016 23:12

thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label May 19, 2016

vdemeester mentioned this pull request Jun 3, 2016

Carry #22228 : Rename graph drivers to storage drivers #23237

Closed

thaJeztah closed this Jun 10, 2016

thaJeztah mentioned this pull request Sep 14, 2016

Rename graphdriver to storage driver in Experimental.md #17996

Closed

thaJeztah mentioned this pull request Nov 22, 2016

Deprecate --graph flag; Replace with --data-root #28696

Merged

thaJeztah mentioned this pull request Aug 4, 2022

Make the snapshotter configurable rumpl/moby#40

Merged

Rename graph drivers to storage drivers #22228

Rename graph drivers to storage drivers #22228

Uh oh!

Conversation

samoht commented Apr 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samoht commented Apr 23, 2016

Uh oh!

thaJeztah commented May 12, 2016

Uh oh!

samoht commented May 12, 2016

Uh oh!

thaJeztah commented May 12, 2016

Uh oh!

samoht commented May 12, 2016

Uh oh!

thaJeztah commented May 12, 2016

Uh oh!

cpuguy83 commented May 13, 2016

Uh oh!

samoht commented May 13, 2016

Uh oh!

kolyshkin commented May 13, 2016

Uh oh!

samoht commented May 13, 2016

Uh oh!

tiborvass commented May 16, 2016

Uh oh!

samoht commented May 16, 2016

Uh oh!

samoht commented May 19, 2016

Uh oh!

thaJeztah commented May 19, 2016

Uh oh!

thaJeztah commented Jun 3, 2016

Uh oh!

thaJeztah commented Jun 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

samoht commented Apr 21, 2016 •

edited

Loading