Repository: mesos Updated Branches: refs/heads/1.0.x 7ba7b1ab7 -> c9b70582e
Fixed Docker daemon args generation. This fix prevents the Docker containerizer from specifying --volume_driver multiple times. Review: https://reviews.apache.org/r/49958/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/36144dab Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/36144dab Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/36144dab Branch: refs/heads/1.0.x Commit: 36144dabf403ef0acd2065eaf73d24d7cecf226f Parents: 7ba7b1a Author: Gastón Kleiman <gas...@mesosphere.com> Authored: Tue Jul 12 15:16:54 2016 -0700 Committer: Jie Yu <yujie....@gmail.com> Committed: Fri Jul 22 16:40:51 2016 -0700 ---------------------------------------------------------------------- src/docker/docker.cpp | 20 +++++- src/tests/containerizer/docker_tests.cpp | 91 +++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/36144dab/src/docker/docker.cpp ---------------------------------------------------------------------- diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index aad5380..e96b4b6 100755 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -555,6 +555,7 @@ Future<Option<int>> Docker::run( argv.push_back("-e"); argv.push_back("MESOS_CONTAINER_NAME=" + name); + Option<string> volumeDriver; foreach (const Volume& volume, containerInfo.volumes()) { string volumeConfig = volume.container_path(); @@ -587,8 +588,14 @@ Future<Option<int>> Docker::run( ":" + volumeConfig; if (volume.source().docker_volume().has_driver()) { - argv.push_back("--volume-driver=" + - volume.source().docker_volume().driver()); + const string& currentDriver = volume.source().docker_volume().driver(); + + if (volumeDriver.isSome() && + volumeDriver.get() != currentDriver) { + return Failure("Only one volume driver is supported"); + } + + volumeDriver = currentDriver; } switch (volume.mode()) { @@ -611,6 +618,15 @@ Future<Option<int>> Docker::run( // TODO(gyliu513): Deprecate this after the release cycle of 1.0. // It will be replaced by Volume.Source.DockerVolume.driver. if (dockerInfo.has_volume_driver()) { + if (volumeDriver.isSome() && + volumeDriver.get() != dockerInfo.volume_driver()) { + return Failure("Only one volume driver per task is supported"); + } + + volumeDriver = dockerInfo.volume_driver(); + } + + if (volumeDriver.isSome()) { argv.push_back("--volume-driver=" + dockerInfo.volume_driver()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/36144dab/src/tests/containerizer/docker_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/docker_tests.cpp b/src/tests/containerizer/docker_tests.cpp index 00ccc2f..ab6dc3c 100644 --- a/src/tests/containerizer/docker_tests.cpp +++ b/src/tests/containerizer/docker_tests.cpp @@ -67,6 +67,26 @@ class DockerTest : public MesosTest AWAIT_READY_FOR(docker.get()->rm(container.id, true), Seconds(30)); } } + +protected: + Volume createDockerVolume( + const string& driver, + const string& name, + const string& containerPath) + { + Volume volume; + volume.set_mode(Volume::RW); + volume.set_container_path(containerPath); + + Volume::Source* source = volume.mutable_source(); + source->set_type(Volume::Source::DOCKER_VOLUME); + + Volume::Source::DockerVolume* docker = source->mutable_docker_volume(); + docker->set_driver(driver); + docker->set_name(name); + + return volume; + } }; @@ -783,6 +803,77 @@ TEST_F(DockerTest, ROOT_DOCKER_NVIDIA_GPU_InspectDevices) EXPECT_EQ(128 + SIGKILL, WEXITSTATUS(status->get())) << status->get(); } + +// This tests verifies that a task requiring more than one volume driver (in +// multiple Volumes) is rejected. +TEST_F(DockerTest, ROOT_DOCKER_ConflictingVolumeDriversInMultipleVolumes) +{ + Owned<Docker> docker = Docker::create( + tests::flags.docker, + tests::flags.docker_socket, + false).get(); + + ContainerInfo containerInfo; + containerInfo.set_type(ContainerInfo::DOCKER); + + Volume volume1 = createDockerVolume("driver1", "name1", "/tmp/test1"); + containerInfo.add_volumes()->CopyFrom(volume1); + + Volume volume2 = createDockerVolume("driver2", "name2", "/tmp/test2"); + containerInfo.add_volumes()->CopyFrom(volume2); + + ContainerInfo::DockerInfo dockerInfo; + dockerInfo.set_image("alpine"); + containerInfo.mutable_docker()->CopyFrom(dockerInfo); + + CommandInfo commandInfo; + commandInfo.set_shell(false); + + Future<Option<int>> run = docker->run( + containerInfo, + commandInfo, + "testContainer", + "dir", + "/mnt/mesos/sandbox"); + + ASSERT_TRUE(run.isFailed()); +} + + +// This tests verifies that a task requiring more than one volume driver (via +// Volume.Source.DockerInfo.driver and ContainerInfo.DockerInfo.volume_driver) +// is rejected. +TEST_F(DockerTest, ROOT_DOCKER_ConflictingVolumeDrivers) +{ + Owned<Docker> docker = Docker::create( + tests::flags.docker, + tests::flags.docker_socket, + false).get(); + + ContainerInfo containerInfo; + containerInfo.set_type(ContainerInfo::DOCKER); + + Volume volume1 = createDockerVolume("driver", "name1", "/tmp/test1"); + containerInfo.add_volumes()->CopyFrom(volume1); + + ContainerInfo::DockerInfo dockerInfo; + dockerInfo.set_image("alpine"); + dockerInfo.set_volume_driver("driver1"); + containerInfo.mutable_docker()->CopyFrom(dockerInfo); + + CommandInfo commandInfo; + commandInfo.set_shell(false); + + Future<Option<int>> run = docker->run( + containerInfo, + commandInfo, + "testContainer", + "dir", + "/mnt/mesos/sandbox"); + + ASSERT_TRUE(run.isFailed()); +} + } // namespace tests { } // namespace internal { } // namespace mesos {
