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.

Sign up for GitHub

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

add ID to Mount / Unmount for docker 1.12 #2876

Closed
wants to merge 3 commits into from
Closed

add ID to Mount / Unmount for docker 1.12 #2876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca53d5f
first pass at adding ID to Mount Unmount for docker 1.12
Aug 1, 2016
2040fe7
fix test where ID value used as key
Aug 1, 2016
24bab0d
add default value for backward compat with docker < 1.12, add no_id t…
Aug 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions flocker/dockerplugin/_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,12 @@ def volumedriver_remove(self, Name):

@app.route("/VolumeDriver.Unmount", methods=["POST"])
@_endpoint(u"Unmount")
def volumedriver_unmount(self, Name):
def volumedriver_unmount(self, Name, ID=None):
"""
The Docker container is no longer using the given volume.

:param unicode Name: The name of the volume.
:param string ID: A unique ID for caller that requested the mount

For now this does nothing. In FLOC-2755 this will release the
lease acquired for the dataset by the ``VolumeDriver.Mount``
Expand Down Expand Up @@ -313,14 +314,19 @@ def got_state(datasets):

@app.route("/VolumeDriver.Mount", methods=["POST"])
@_endpoint(u"Mount")
def volumedriver_mount(self, Name):
def volumedriver_mount(self, Name, ID=None):
"""
Move a volume with the given name to the current node and mount it.

Since we need to return the filesystem path we wait until the
dataset is mounted locally.

:param unicode Name: The name of the volume.
:param string ID: A unique ID for caller that requested the mount

Right now we do nothing with ID, but it can be used to track
the mount calls when multiple containers are trying to mount
the same volume.

:return: Result that includes the mountpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Maybe double check whether the return format has changed. Do we have to include the supplied ID in the json that we return from the API.
If so, then we'll have to also update the JSON schema files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The return schema is the same, ID is only passed in the request and not the response. See here https://docs.docker.com/engine/extend/plugins_volume/

"""
Expand Down
75 changes: 71 additions & 4 deletions flocker/dockerplugin/test/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
Tests for the Volumes Plugin API provided by the plugin.
"""

import random
from uuid import uuid4

from bitmath import TiB, GiB, MiB, KiB, Byte
Expand Down Expand Up @@ -115,8 +116,22 @@ def test_unmount(self):
"""
``/VolumeDriver.Unmount`` returns a successful result.
"""
unmount_id = ''.join(random.choice(
'0123456789abcdef') for n in xrange(64))
return self.assertResult(b"POST", b"/VolumeDriver.Unmount",
Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at moby/moby#21015 and ID comes from https://github.com/docker/docker/blob/master/pkg/stringid/stringid.go ....and which seems to be a / the container ID.

You could do something like os.urandom(32).encode('hex') (we use that elsewhere in Flocker) which might be make the intended ID format a little clearer....although when I tested it seems to hang when there's insufficient entropy. And I guess that urandom may not be hooked up to trial's random seed option....so forget this comment :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, saw this too, tried to duplicate the stringid non cryptic id in python.

Im fine either way, the question is do we want to test multiple IDs with hypothesis or even test for malformed IDs ?

{u"Name": u"vol"}, OK, {u"Err": u""})
{u"Name": u"vol",
u"ID": unicode(unmount_id)},
OK, {u"Err": u""})

def test_unmount_no_id(self):
"""
``/VolumeDriver.Unmount`` returns a successful result.

No ID for backward compatability with Docker < 1.12
"""
return self.assertResult(b"POST", b"/VolumeDriver.Unmount",
{u"Name": u"vol"},
OK, {u"Err": u""})

def test_create_with_profile(self):
"""
Expand Down Expand Up @@ -320,6 +335,52 @@ def test_mount(self):
"""
name = u"myvol"
dataset_id = uuid4()
mount_id = ''.join(random.choice(
'0123456789abcdef') for n in xrange(64))

# Create dataset on a different node:
d = self.flocker_client.create_dataset(
self.NODE_B, int(DEFAULT_SIZE.to_Byte()),
metadata={NAME_FIELD: name},
dataset_id=dataset_id)

self._flush_volume_plugin_reactor_on_endpoint_render()

# Pretend that it takes 5 seconds for the dataset to get established on
# Node A.
self.volume_plugin_reactor.callLater(
5.0, self.flocker_client.synchronize_state)

d.addCallback(lambda _:
self.assertResult(
b"POST", b"/VolumeDriver.Mount",
{u"Name": name, u"ID": unicode(mount_id)}, OK,
{u"Err": u"",
u"Mountpoint": u"/flocker/{}".format(dataset_id)}))
d.addCallback(lambda _: self.flocker_client.list_datasets_state())

def final_assertions(datasets):
self.assertEqual([self.NODE_A],
[d.primary for d in datasets
if d.dataset_id == dataset_id])
# There should be less than 20 calls to list_datasets_state over
# the course of 5 seconds.
self.assertLess(
self.flocker_client.num_calls('list_datasets_state'), 20)
d.addCallback(final_assertions)

return d

def test_mount_no_id(self):
"""
``/VolumeDriver.Mount`` sets the primary of the dataset with matching
name to the current node and then waits for the dataset to
actually arrive.

No ID for backward compatability with Docker < 1.12
"""
name = u"myvol"
dataset_id = uuid4()

# Create dataset on a different node:
d = self.flocker_client.create_dataset(
Expand Down Expand Up @@ -363,6 +424,8 @@ def test_mount_timeout(self):
"""
name = u"myvol"
dataset_id = uuid4()
mount_id = ''.join(random.choice(
'0123456789abcdef') for n in xrange(64))
# Create dataset on a different node:
d = self.flocker_client.create_dataset(
self.NODE_B, int(DEFAULT_SIZE.to_Byte()),
Expand All @@ -379,7 +442,7 @@ def test_mount_timeout(self):
d.addCallback(lambda _:
self.assertResult(
b"POST", b"/VolumeDriver.Mount",
{u"Name": name}, OK,
{u"Name": name, u"ID": unicode(mount_id)}, OK,
{u"Err": u"Timed out waiting for dataset to mount.",
u"Mountpoint": u""}))
return d
Expand All @@ -392,6 +455,8 @@ def test_mount_already_exists(self):
don't have a special dataset ID.
"""
name = u"myvol"
mount_id = ''.join(random.choice(
'0123456789abcdef') for n in xrange(64))

d = self.flocker_client.create_dataset(
self.NODE_A, int(DEFAULT_SIZE.to_Byte()),
Expand All @@ -401,7 +466,7 @@ def created(dataset):
self.flocker_client.synchronize_state()
result = self.assertResult(
b"POST", b"/VolumeDriver.Mount",
{u"Name": name}, OK,
{u"Name": name, u"ID": unicode(mount_id)}, OK,
{u"Err": u"",
u"Mountpoint": u"/flocker/{}".format(
dataset.dataset_id)})
Expand All @@ -420,9 +485,11 @@ def test_unknown_mount(self):
non-existent volume.
"""
name = u"myvol"
mount_id = ''.join(random.choice(
'0123456789abcdef') for n in xrange(64))
return self.assertResult(
b"POST", b"/VolumeDriver.Mount",
{u"Name": name}, OK,
{u"Name": name, u"ID": unicode(mount_id)}, OK,
{u"Err": u"Could not find volume with given name."})

def test_path(self):
Expand Down