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

GCE translate underscores #580

Merged

Conversation

codenrhoden
Copy link
Contributor

This is a bit experimental, but worked completely in my testing with Docker.

New config option, gcepd.convertUnderscores, enables the GCEPD driver to
replace any underscores with dashes during volume creation. When a
VolumeInspectByName request comes in, that name is also converted, and
the volume that was created with the dashes is returned.

It is important to note that the returned data from the driver is always
correct -- the name is never displayed as having underscores instead of
dashes, because no such volume exists.

Fixes #458

without patch:

$ docker volume create -d rexray test_me3 --opt size=10
Error response from daemon: create test_me3: VolumeDriver.Create: {"Error":"Volume name does not meet GCE naming requirements"}

with patch:

$ docker volume create -d rexray test_me4 --opt size=10
test_me4
$ ./rexray -s gcepd volume ls
ID          Name        Status     Size
test-me4    test-me4    available  10
$ docker volume ls
DRIVER              VOLUME NAME
rexray              test-me4
$ docker volume inspect test_me4
[
    {
        "Driver": "rexray",
        "Labels": {},
        "Mountpoint": "/",
        "Name": "test-me4",
        "Options": {
            "size": "10"
        },
        "Scope": "global",
        "Status": {
            "availabilityZone": "us-west1-b",
            "fields": null,
            "iops": 0,
            "name": "test-me4",
            "server": "gcepd",
            "service": "gcepd",
            "size": 10,
            "type": "pd-ssd"
        }
    }
]
$ docker run --rm --volume-driver=rexray -v test_me4:/data busybox mount | grep \/data
/dev/disk/by-id/google-test-me4 on /data type ext4 (rw,seclabel,relatime,data=ordered)
# docker volume rm test_me4
test_me4

So, at this point for docker, test_me4 and test-me4 are pretty much aliases for each other. Referencing either one works.

This PR depends on #579, and includes the commit from that PR. Once that PR is merged, this can be rebased to remove the additional commit.

@codecov-io
Copy link

codecov-io commented Jun 21, 2017

Codecov Report

Merging #580 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #580   +/-   ##
=======================================
  Coverage   28.42%   28.42%           
=======================================
  Files          34       34           
  Lines        2016     2016           
=======================================
  Hits          573      573           
  Misses       1381     1381           
  Partials       62       62

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb8a4bc...4d2ab33. Read the comment docs.

New config option, gcepd.convertUnderscores, enables the GCEPD driver to
replace any underscores with dashes during volume creation. When a
VolumeInspectByName request comes in, that name is also converted, and
the volume that was created with the dashes is returned.

It is important to note that the returned data from the driver is always
correct -- the name is never displayed as having underscores instead of
dashes, because no such volume exists.
@codenrhoden codenrhoden force-pushed the enhancement/gce_translate_underscores branch from 28dad55 to 4d2ab33 Compare June 22, 2017 17:46
@akutz
Copy link
Collaborator

akutz commented Jun 26, 2017

Hi @codenrhoden,

Can you rebase this PR? It has two commits on it. For some reason f30c7cf isn't going away, even though #579 has been merged (and it has a single commit...the aforementioned f30c7cf). I don't want to accidentally screw anything up. Please rebase this, and then we can merge it cleanly.

@akutz akutz merged commit b156064 into thecodeteam:master Jun 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants