Delegated the container root filesystem provisioning to the filesystem isolator.
Review: https://reviews.apache.org/r/37159 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8ba2f351 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8ba2f351 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8ba2f351 Branch: refs/heads/master Commit: 8ba2f3511e0b2e5975b72f69079210ad2e1169b5 Parents: 39d5e5f Author: Jie Yu <[email protected]> Authored: Wed Aug 5 20:58:37 2015 -0700 Committer: Jie Yu <[email protected]> Committed: Fri Aug 7 16:54:14 2015 -0700 ---------------------------------------------------------------------- include/mesos/slave/isolator.hpp | 1 - include/mesos/slave/isolator.proto | 3 + src/slave/containerizer/isolator.cpp | 2 - src/slave/containerizer/isolator.hpp | 2 - .../isolators/cgroups/cpushare.cpp | 1 - .../isolators/cgroups/cpushare.hpp | 1 - .../containerizer/isolators/cgroups/mem.cpp | 1 - .../containerizer/isolators/cgroups/mem.hpp | 1 - .../isolators/cgroups/perf_event.cpp | 1 - .../isolators/cgroups/perf_event.hpp | 1 - .../isolators/filesystem/posix.cpp | 12 +- .../isolators/filesystem/posix.hpp | 1 - .../isolators/filesystem/shared.cpp | 1 - .../isolators/filesystem/shared.hpp | 1 - .../containerizer/isolators/namespaces/pid.cpp | 1 - .../containerizer/isolators/namespaces/pid.hpp | 1 - .../isolators/network/port_mapping.cpp | 1 - .../isolators/network/port_mapping.hpp | 1 - src/slave/containerizer/isolators/posix.hpp | 1 - .../containerizer/isolators/posix/disk.cpp | 1 - .../containerizer/isolators/posix/disk.hpp | 1 - src/slave/containerizer/mesos/containerizer.cpp | 156 +++---------------- src/slave/containerizer/mesos/containerizer.hpp | 39 +---- src/tests/containerizer/isolator.hpp | 1 - src/tests/containerizer/isolator_tests.cpp | 11 -- .../containerizer/mesos_containerizer_tests.cpp | 27 ++-- src/tests/containerizer/port_mapping_tests.cpp | 14 -- 27 files changed, 46 insertions(+), 238 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/include/mesos/slave/isolator.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/slave/isolator.hpp b/include/mesos/slave/isolator.hpp index 22f1e36..970730f 100644 --- a/include/mesos/slave/isolator.hpp +++ b/include/mesos/slave/isolator.hpp @@ -74,7 +74,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user) = 0; // Isolate the executor. http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/include/mesos/slave/isolator.proto ---------------------------------------------------------------------- diff --git a/include/mesos/slave/isolator.proto b/include/mesos/slave/isolator.proto index 39d20c7..12ea6ac 100644 --- a/include/mesos/slave/isolator.proto +++ b/include/mesos/slave/isolator.proto @@ -68,4 +68,7 @@ message ContainerPrepareInfo { repeated CommandInfo commands = 1; optional Environment environment = 2; + + // The root filesystem for the container. + optional string rootfs = 3; } http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolator.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolator.cpp b/src/slave/containerizer/isolator.cpp index ed610f9..7973100 100644 --- a/src/slave/containerizer/isolator.cpp +++ b/src/slave/containerizer/isolator.cpp @@ -68,7 +68,6 @@ Future<Option<ContainerPrepareInfo>> MesosIsolator::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { return dispatch(process.get(), @@ -76,7 +75,6 @@ Future<Option<ContainerPrepareInfo>> MesosIsolator::prepare( containerId, executorInfo, directory, - rootfs, user); } http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolator.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolator.hpp b/src/slave/containerizer/isolator.hpp index 710c584..fbb7c8a 100644 --- a/src/slave/containerizer/isolator.hpp +++ b/src/slave/containerizer/isolator.hpp @@ -53,7 +53,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( @@ -93,7 +92,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user) = 0; virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/cgroups/cpushare.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/cgroups/cpushare.cpp b/src/slave/containerizer/isolators/cgroups/cpushare.cpp index 907d7e7..ba748c6 100644 --- a/src/slave/containerizer/isolators/cgroups/cpushare.cpp +++ b/src/slave/containerizer/isolators/cgroups/cpushare.cpp @@ -249,7 +249,6 @@ Future<Option<ContainerPrepareInfo>> CgroupsCpushareIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (infos.contains(containerId)) { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/cgroups/cpushare.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/cgroups/cpushare.hpp b/src/slave/containerizer/isolators/cgroups/cpushare.hpp index 6b980f2..54b83a7 100644 --- a/src/slave/containerizer/isolators/cgroups/cpushare.hpp +++ b/src/slave/containerizer/isolators/cgroups/cpushare.hpp @@ -58,7 +58,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/cgroups/mem.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/cgroups/mem.cpp b/src/slave/containerizer/isolators/cgroups/mem.cpp index e343d0b..48d7fbd 100644 --- a/src/slave/containerizer/isolators/cgroups/mem.cpp +++ b/src/slave/containerizer/isolators/cgroups/mem.cpp @@ -235,7 +235,6 @@ Future<Option<ContainerPrepareInfo>> CgroupsMemIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (infos.contains(containerId)) { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/cgroups/mem.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/cgroups/mem.hpp b/src/slave/containerizer/isolators/cgroups/mem.hpp index e831878..47f73c3 100644 --- a/src/slave/containerizer/isolators/cgroups/mem.hpp +++ b/src/slave/containerizer/isolators/cgroups/mem.hpp @@ -53,7 +53,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/cgroups/perf_event.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/cgroups/perf_event.cpp b/src/slave/containerizer/isolators/cgroups/perf_event.cpp index 0e421cb..8c3018d 100644 --- a/src/slave/containerizer/isolators/cgroups/perf_event.cpp +++ b/src/slave/containerizer/isolators/cgroups/perf_event.cpp @@ -221,7 +221,6 @@ Future<Option<ContainerPrepareInfo>> CgroupsPerfEventIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (infos.contains(containerId)) { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/cgroups/perf_event.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/cgroups/perf_event.hpp b/src/slave/containerizer/isolators/cgroups/perf_event.hpp index 73f245b..c1578b1 100644 --- a/src/slave/containerizer/isolators/cgroups/perf_event.hpp +++ b/src/slave/containerizer/isolators/cgroups/perf_event.hpp @@ -49,7 +49,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/filesystem/posix.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/filesystem/posix.cpp b/src/slave/containerizer/isolators/filesystem/posix.cpp index 4861ee1..eec510c 100644 --- a/src/slave/containerizer/isolators/filesystem/posix.cpp +++ b/src/slave/containerizer/isolators/filesystem/posix.cpp @@ -74,7 +74,6 @@ Future<Option<ContainerPrepareInfo>> PosixFilesystemIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (infos.contains(containerId)) { @@ -83,8 +82,15 @@ Future<Option<ContainerPrepareInfo>> PosixFilesystemIsolatorProcess::prepare( // Return failure if the container change the filesystem root // because the symlinks will become invalid in the new root. - if (rootfs.isSome()) { - return Failure("Container root filesystems not supported"); + if (executorInfo.has_container()) { + CHECK_EQ(executorInfo.container().type(), ContainerInfo::MESOS); + + if (executorInfo.container().mesos().has_image()) { + return Failure("Container root filesystems not supported"); + } + + // TODO(jieyu): Also return a failure if there exists images in + // the specified volumes. } infos.put(containerId, Owned<Info>(new Info(directory))); http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/filesystem/posix.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/filesystem/posix.hpp b/src/slave/containerizer/isolators/filesystem/posix.hpp index 4c7a6f2..61b81dd 100644 --- a/src/slave/containerizer/isolators/filesystem/posix.hpp +++ b/src/slave/containerizer/isolators/filesystem/posix.hpp @@ -44,7 +44,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/filesystem/shared.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/filesystem/shared.cpp b/src/slave/containerizer/isolators/filesystem/shared.cpp index b30ab3f..4b4520e 100644 --- a/src/slave/containerizer/isolators/filesystem/shared.cpp +++ b/src/slave/containerizer/isolators/filesystem/shared.cpp @@ -84,7 +84,6 @@ Future<Option<ContainerPrepareInfo>> SharedFilesystemIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (executorInfo.has_container() && http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/filesystem/shared.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/filesystem/shared.hpp b/src/slave/containerizer/isolators/filesystem/shared.hpp index 45e4ba0..a21bc79 100644 --- a/src/slave/containerizer/isolators/filesystem/shared.hpp +++ b/src/slave/containerizer/isolators/filesystem/shared.hpp @@ -49,7 +49,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/namespaces/pid.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/namespaces/pid.cpp b/src/slave/containerizer/isolators/namespaces/pid.cpp index 8e643f4..35cb664 100644 --- a/src/slave/containerizer/isolators/namespaces/pid.cpp +++ b/src/slave/containerizer/isolators/namespaces/pid.cpp @@ -163,7 +163,6 @@ Future<Option<ContainerPrepareInfo>> NamespacesPidIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { ContainerPrepareInfo prepareInfo; http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/namespaces/pid.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/namespaces/pid.hpp b/src/slave/containerizer/isolators/namespaces/pid.hpp index 858e436..b22f5ba 100644 --- a/src/slave/containerizer/isolators/namespaces/pid.hpp +++ b/src/slave/containerizer/isolators/namespaces/pid.hpp @@ -66,7 +66,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/network/port_mapping.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.cpp b/src/slave/containerizer/isolators/network/port_mapping.cpp index 8244c34..88c0cbc 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/isolators/network/port_mapping.cpp @@ -2086,7 +2086,6 @@ Future<Option<ContainerPrepareInfo>> PortMappingIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (unmanaged.contains(containerId)) { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/network/port_mapping.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.hpp b/src/slave/containerizer/isolators/network/port_mapping.hpp index 2599c98..4bca0b8 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.hpp +++ b/src/slave/containerizer/isolators/network/port_mapping.hpp @@ -162,7 +162,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/posix.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/posix.hpp b/src/slave/containerizer/isolators/posix.hpp index ef19749..ee9d275 100644 --- a/src/slave/containerizer/isolators/posix.hpp +++ b/src/slave/containerizer/isolators/posix.hpp @@ -66,7 +66,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user) { if (promises.contains(containerId)) { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/posix/disk.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/posix/disk.cpp b/src/slave/containerizer/isolators/posix/disk.cpp index 6dda77b..c324c79 100644 --- a/src/slave/containerizer/isolators/posix/disk.cpp +++ b/src/slave/containerizer/isolators/posix/disk.cpp @@ -107,7 +107,6 @@ Future<Option<ContainerPrepareInfo>> PosixDiskIsolatorProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { if (infos.contains(containerId)) { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/isolators/posix/disk.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/posix/disk.hpp b/src/slave/containerizer/isolators/posix/disk.hpp index 9fa584f..85df5d2 100644 --- a/src/slave/containerizer/isolators/posix/disk.hpp +++ b/src/slave/containerizer/isolators/posix/disk.hpp @@ -86,7 +86,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user); virtual process::Future<Nothing> isolate( http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/mesos/containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp index 758ed1c..e34dec9 100644 --- a/src/slave/containerizer/mesos/containerizer.cpp +++ b/src/slave/containerizer/mesos/containerizer.cpp @@ -197,20 +197,12 @@ Try<MesosContainerizer*> MesosContainerizer::create( return Error("Failed to create launcher: " + launcher.error()); } - Try<hashmap<Image::Type, Owned<Provisioner>>> provisioners = - Provisioner::create(flags, fetcher); - - if (provisioners.isError()) { - return Error("Failed to create provisioner(s): " + provisioners.error()); - } - return new MesosContainerizer( flags_, local, fetcher, Owned<Launcher>(launcher.get()), - isolators, - provisioners.get()); + isolators); } @@ -219,15 +211,13 @@ MesosContainerizer::MesosContainerizer( bool local, Fetcher* fetcher, const Owned<Launcher>& launcher, - const vector<Owned<Isolator>>& isolators, - const hashmap<Image::Type, Owned<Provisioner>>& provisioners) + const vector<Owned<Isolator>>& isolators) : process(new MesosContainerizerProcess( flags, local, fetcher, launcher, - isolators, - provisioners)) + isolators)) { spawn(process.get()); } @@ -441,17 +431,12 @@ Future<Nothing> MesosContainerizerProcess::_recover( { list<Future<Nothing>> futures; - // Then recover the provisioners. - foreachvalue (const Owned<Provisioner>& provisioner, provisioners) { - futures.push_back(provisioner->recover(recoverable, orphans)); - } - // Then recover the isolators. foreach (const Owned<Isolator>& isolator, isolators) { futures.push_back(isolator->recover(recoverable, orphans)); } - // If all provisioners and isolators recover then continue. + // If all isolators recover then continue. return collect(futures) .then(defer(self(), &Self::__recover, recoverable, orphans)); } @@ -616,71 +601,13 @@ Future<bool> MesosContainerizerProcess::launch( containers_.put(containerId, Owned<Container>(container)); - return provision(containerId, executorInfo) + return prepare(containerId, executorInfo, directory, user) .then(defer(self(), &Self::_launch, containerId, executorInfo, directory, user, - lambda::_1, - slaveId, - slavePid, - checkpoint)); -} - - -Future<Option<string>> MesosContainerizerProcess::provision( - const ContainerID& containerId, - const ExecutorInfo& executorInfo) -{ - if (!executorInfo.has_container()) { - // TODO(idownes): Consider refusing to run a container if the - // containerizer is configured with an image provisioner but the - // executor doesn't specify an image. - return None(); - } - - if (executorInfo.container().type() != ContainerInfo::MESOS) { - return Failure("Mesos containerizer can only provision Mesos containers"); - } - - if (!executorInfo.container().mesos().has_image()) { - // No image to provision. - return None(); - } - - Image image = executorInfo.container().mesos().image(); - - if (!provisioners.contains(image.type())) { - return Failure("ExecutorInfo specifies container image type '" + - stringify(image.type()) + - "' but no suitable provisioner available"); - } - - return provisioners[image.type()]->provision(containerId, image) - .then([] (const string& rootfs) -> Option<string> { return rootfs; }); -} - - -Future<bool> MesosContainerizerProcess::_launch( - const ContainerID& containerId, - const ExecutorInfo& executorInfo, - const string& directory, - const Option<string>& user, - const Option<string>& rootfs, - const SlaveID& slaveId, - const PID<Slave>& slavePid, - bool checkpoint) -{ - return prepare(containerId, executorInfo, directory, user, rootfs) - .then(defer(self(), - &Self::__launch, - containerId, - executorInfo, - directory, - user, - rootfs, slaveId, slavePid, checkpoint, @@ -702,12 +629,11 @@ static Future<list<Option<ContainerPrepareInfo>>> _prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user, const list<Option<ContainerPrepareInfo>> prepareInfos) { // Propagate any failure. - return isolator->prepare(containerId, executorInfo, directory, rootfs, user) + return isolator->prepare(containerId, executorInfo, directory, user) .then(lambda::bind(&accumulate, prepareInfos, lambda::_1)); } @@ -716,8 +642,7 @@ Future<list<Option<ContainerPrepareInfo>>> MesosContainerizerProcess::prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& user, - const Option<string>& rootfs) + const Option<string>& user) { CHECK(containers_.contains(containerId)); @@ -734,7 +659,6 @@ Future<list<Option<ContainerPrepareInfo>>> MesosContainerizerProcess::prepare( containerId, executorInfo, directory, - rootfs, user, lambda::_1)); } @@ -766,12 +690,11 @@ Future<Nothing> MesosContainerizerProcess::fetch( } -Future<bool> MesosContainerizerProcess::__launch( +Future<bool> MesosContainerizerProcess::_launch( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, const Option<string>& user, - const Option<string>& rootfs, const SlaveID& slaveId, const PID<Slave>& slavePid, bool checkpoint, @@ -785,6 +708,19 @@ Future<bool> MesosContainerizerProcess::__launch( return Failure("Container is currently being destroyed"); } + // Determine the root filesystem for the container. Only one + // isolator should return the container root filesystem. + Option<string> rootfs; + foreach (const Option<ContainerPrepareInfo>& prepareInfo, prepareInfos) { + if (prepareInfo.isSome() && prepareInfo.get().has_rootfs()) { + if (rootfs.isSome()) { + return Failure("Only one isolator should return the container rootfs"); + } else { + rootfs = prepareInfo.get().rootfs(); + } + } + } + // Prepare environment variables for the executor. map<string, string> environment = executorEnvironment( executorInfo, @@ -1245,56 +1181,6 @@ void MesosContainerizerProcess::____destroy( } } - // Clean up root filesystems used for this container. - // NOTE: We need to call 'destroy' for each provisioner because - // multiple provisioners might be used for a container. - list<Future<bool>> futures; - foreachvalue (const Owned<Provisioner>& provisioner, provisioners) { - futures.push_back(provisioner->destroy(containerId)); - } - - await(futures) - .onAny(defer(self(), - &Self::_____destroy, - containerId, - status, - message, - killed, - lambda::_1)); - - return; -} - - -void MesosContainerizerProcess::_____destroy( - const ContainerID& containerId, - const Future<Option<int>>& status, - Option<string> message, - bool killed, - const Future<list<Future<bool>>>& futures) -{ - CHECK(containers_.contains(containerId)); - - Container* container = containers_[containerId].get(); - - CHECK_READY(futures); - - foreach (const Future<bool> future, futures.get()) { - if (future.isReady()) { - continue; - } - - container->promise.fail( - "Failed to clean up the root filesystem when destroying container: " + - (future.isFailed() ? future.failure() : "discarded future")); - - containers_.erase(containerId); - - ++metrics.container_destroy_errors; - - return; - } - // If any isolator limited the container then we mark it to killed. // Note: We may not see a limitation in time for it to be // registered. This could occur if the limitation (e.g., an OOM) http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/slave/containerizer/mesos/containerizer.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/containerizer.hpp b/src/slave/containerizer/mesos/containerizer.hpp index 066d541..4c14192 100644 --- a/src/slave/containerizer/mesos/containerizer.hpp +++ b/src/slave/containerizer/mesos/containerizer.hpp @@ -33,7 +33,6 @@ #include "slave/containerizer/containerizer.hpp" #include "slave/containerizer/launcher.hpp" -#include "slave/containerizer/provisioner.hpp" namespace mesos { namespace internal { @@ -57,9 +56,7 @@ public: bool local, Fetcher* fetcher, const process::Owned<Launcher>& launcher, - const std::vector<process::Owned<mesos::slave::Isolator>>& isolators, - const hashmap<Image::Type, process::Owned<Provisioner>>& provisioners); - + const std::vector<process::Owned<mesos::slave::Isolator>>& isolators); // Used for testing. MesosContainerizer(const process::Owned<MesosContainerizerProcess>& _process); @@ -116,14 +113,12 @@ public: bool _local, Fetcher* _fetcher, const process::Owned<Launcher>& _launcher, - const std::vector<process::Owned<mesos::slave::Isolator>>& _isolators, - const hashmap<Image::Type, process::Owned<Provisioner>>& _provisioners) + const std::vector<process::Owned<mesos::slave::Isolator>>& _isolators) : flags(_flags), local(_local), fetcher(_fetcher), launcher(_launcher), - isolators(_isolators), - provisioners(_provisioners) {} + isolators(_isolators) {} virtual ~MesosContainerizerProcess() {} @@ -181,16 +176,11 @@ private: const std::list<mesos::slave::ContainerState>& recovered, const hashset<ContainerID>& orphans); - process::Future<Option<std::string>> provision( - const ContainerID& containerId, - const ExecutorInfo& executorInfo); - process::Future<std::list<Option<mesos::slave::ContainerPrepareInfo>>> prepare(const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& user, - const Option<std::string>& rootfs); + const Option<std::string>& user); process::Future<Nothing> fetch( const ContainerID& containerId, @@ -204,17 +194,6 @@ private: const ExecutorInfo& executorInfo, const std::string& directory, const Option<std::string>& user, - const Option<std::string>& rootfs, - const SlaveID& slaveId, - const process::PID<Slave>& slavePid, - bool checkpoint); - - process::Future<bool> __launch( - const ContainerID& containerId, - const ExecutorInfo& executorInfo, - const std::string& directory, - const Option<std::string>& user, - const Option<std::string>& rootfs, const SlaveID& slaveId, const process::PID<Slave>& slavePid, bool checkpoint, @@ -249,15 +228,6 @@ private: Option<std::string> message, bool killed); - // Continues (and completes) '____destroy()' once any root filessystem - // has been cleaned up. - void _____destroy( - const ContainerID& containerId, - const process::Future<Option<int>>& status, - Option<std::string> message, - bool killed, - const process::Future<std::list<process::Future<bool>>>& futures); - // Call back for when an isolator limits a container and impacts the // processes. This will trigger container destruction. void limited( @@ -278,7 +248,6 @@ private: Fetcher* fetcher; const process::Owned<Launcher> launcher; const std::vector<process::Owned<mesos::slave::Isolator>> isolators; - hashmap<Image::Type, process::Owned<Provisioner>> provisioners; enum State { http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/tests/containerizer/isolator.hpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/isolator.hpp b/src/tests/containerizer/isolator.hpp index fa2fc9b..56ac27b 100644 --- a/src/tests/containerizer/isolator.hpp +++ b/src/tests/containerizer/isolator.hpp @@ -49,7 +49,6 @@ public: const ContainerID& containerId, const ExecutorInfo& executorInfo, const std::string& directory, - const Option<std::string>& rootfs, const Option<std::string>& user) { return prepareInfo; http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/tests/containerizer/isolator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp index ff6e2b7..dd1ae22 100644 --- a/src/tests/containerizer/isolator_tests.cpp +++ b/src/tests/containerizer/isolator_tests.cpp @@ -172,7 +172,6 @@ TYPED_TEST(CpuIsolatorTest, UserCpuUsage) containerId, executorInfo, dir.get(), - None(), None())); const string& file = path::join(dir.get(), "mesos_isolator_test_ready"); @@ -282,7 +281,6 @@ TYPED_TEST(CpuIsolatorTest, SystemCpuUsage) containerId, executorInfo, dir.get(), - None(), None())); const string& file = path::join(dir.get(), "mesos_isolator_test_ready"); @@ -394,7 +392,6 @@ TEST_F(RevocableCpuIsolatorTest, ROOT_CGROUPS_RevocableCpu) containerId, executorInfo, os::getcwd(), - None(), None())); vector<string> argv{"sleep", "100"}; @@ -475,7 +472,6 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs) containerId, executorInfo, dir.get(), - None(), None())); // Generate random numbers to max out a single core. We'll run this for 0.5 @@ -587,7 +583,6 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota) containerId, executorInfo, dir.get(), - None(), None())); int pipes[2]; @@ -671,7 +666,6 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids) containerId, executorInfo, dir.get(), - None(), None())); // Right after the creation of the cgroup, which happens in @@ -795,7 +789,6 @@ TYPED_TEST(MemIsolatorTest, MemUsage) containerId, executorInfo, dir.get(), - None(), None())); MemoryTestHelper helper; @@ -858,7 +851,6 @@ TEST_F(PerfEventIsolatorTest, ROOT_CGROUPS_Sample) containerId, executorInfo, dir.get(), - None(), None())); // This first sample is likely to be empty because perf hasn't @@ -956,7 +948,6 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume) containerId, executorInfo, flags.work_dir, - None(), None()); AWAIT_READY(prepare); @@ -1061,7 +1052,6 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume) containerId, executorInfo, flags.work_dir, - None(), None()); AWAIT_READY(prepare); @@ -1230,7 +1220,6 @@ TYPED_TEST(UserCgroupIsolatorTest, ROOT_CGROUPS_UserCgroup) containerId, executorInfo, os::getcwd(), - None(), UNPRIVILEGED_USERNAME)); // Isolators don't provide a way to determine the cgroups they use http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/tests/containerizer/mesos_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp b/src/tests/containerizer/mesos_containerizer_tests.cpp index 4afa986..5bc7d40 100644 --- a/src/tests/containerizer/mesos_containerizer_tests.cpp +++ b/src/tests/containerizer/mesos_containerizer_tests.cpp @@ -55,7 +55,6 @@ using mesos::internal::slave::Launcher; using mesos::internal::slave::MesosContainerizer; using mesos::internal::slave::MesosContainerizerProcess; using mesos::internal::slave::PosixLauncher; -using mesos::internal::slave::Provisioner; using mesos::internal::slave::Slave; using mesos::internal::slave::state::ExecutorState; @@ -116,8 +115,7 @@ public: false, fetcher, Owned<Launcher>(launcher.get()), - isolators, - hashmap<Image::Type, Owned<Provisioner>>()); + isolators); } Try<MesosContainerizer*> CreateContainerizer( @@ -429,15 +427,13 @@ public: bool local, Fetcher* fetcher, const Owned<Launcher>& launcher, - const vector<Owned<Isolator>>& isolators, - const hashmap<Image::Type, Owned<Provisioner>>& provisioners) + const vector<Owned<Isolator>>& isolators) : MesosContainerizerProcess( flags, local, fetcher, launcher, - isolators, - provisioners) + isolators) { // NOTE: See TestContainerizer::setup for why we use // 'EXPECT_CALL' and 'WillRepeatedly' here instead of @@ -477,7 +473,7 @@ public: EXPECT_CALL(*this, cleanup(_)) .WillRepeatedly(Return(Nothing())); - EXPECT_CALL(*this, prepare(_, _, _, _, _)) + EXPECT_CALL(*this, prepare(_, _, _, _)) .WillRepeatedly(Invoke(this, &MockIsolator::_prepare)); } @@ -487,20 +483,18 @@ public: const list<ContainerState>&, const hashset<ContainerID>&)); - MOCK_METHOD5( + MOCK_METHOD4( prepare, Future<Option<ContainerPrepareInfo>>( const ContainerID&, const ExecutorInfo&, const string&, - const Option<string>&, const Option<string>&)); virtual Future<Option<ContainerPrepareInfo>> _prepare( const ContainerID& containerId, const ExecutorInfo& executorInfo, const string& directory, - const Option<string>& rootfs, const Option<string>& user) { return None(); @@ -546,8 +540,7 @@ TEST_F(MesosContainerizerDestroyTest, DestroyWhileFetching) true, &fetcher, Owned<Launcher>(launcher.get()), - vector<Owned<Isolator>>(), - hashmap<Image::Type, Owned<Provisioner>>()); + vector<Owned<Isolator>>()); Future<Nothing> exec; Promise<bool> promise; @@ -602,7 +595,7 @@ TEST_F(MesosContainerizerDestroyTest, DestroyWhilePreparing) Promise<Option<ContainerPrepareInfo>> promise; // Simulate a long prepare from the isolator. - EXPECT_CALL(*isolator, prepare(_, _, _, _, _)) + EXPECT_CALL(*isolator, prepare(_, _, _, _)) .WillOnce(DoAll(FutureSatisfy(&prepare), Return(promise.future()))); @@ -613,8 +606,7 @@ TEST_F(MesosContainerizerDestroyTest, DestroyWhilePreparing) true, &fetcher, Owned<Launcher>(launcher.get()), - {Owned<Isolator>(isolator)}, - hashmap<Image::Type, Owned<Provisioner>>()); + {Owned<Isolator>(isolator)}); MesosContainerizer containerizer((Owned<MesosContainerizerProcess>(process))); @@ -691,8 +683,7 @@ TEST_F(MesosContainerizerDestroyTest, LauncherDestroyFailure) true, &fetcher, Owned<Launcher>(launcher), - vector<Owned<Isolator>>(), - hashmap<Image::Type, Owned<Provisioner>>()); + vector<Owned<Isolator>>()); MesosContainerizer containerizer((Owned<MesosContainerizerProcess>(process))); http://git-wip-us.apache.org/repos/asf/mesos/blob/8ba2f351/src/tests/containerizer/port_mapping_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp index 4bee74a..3c9b7c8 100644 --- a/src/tests/containerizer/port_mapping_tests.cpp +++ b/src/tests/containerizer/port_mapping_tests.cpp @@ -461,7 +461,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerTCP) containerId1, executorInfo, dir1.get(), - None(), None()); AWAIT_READY(preparation1); @@ -529,7 +528,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerTCP) containerId2, executorInfo, dir2.get(), - None(), None()); AWAIT_READY(preparation2); @@ -623,7 +621,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerUDP) containerId1, executorInfo, dir1.get(), - None(), None()); AWAIT_READY(preparation1); @@ -691,7 +688,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerUDP) containerId2, executorInfo, dir2.get(), - None(), None()); AWAIT_READY(preparation2); @@ -787,7 +783,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_HostToContainerUDP) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -905,7 +900,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_HostToContainerTCP) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1031,7 +1025,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_ContainerICMPExternal) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1118,7 +1111,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_ContainerICMPInternal) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1208,7 +1200,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_ContainerARPExternal) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1304,7 +1295,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_DNS) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1396,7 +1386,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_TooManyContainers) containerId1, executorInfo, dir1.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1448,7 +1437,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_TooManyContainers) containerId2, executorInfo, dir2.get(), - None(), None()); AWAIT_FAILED(preparation2); @@ -1514,7 +1502,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_SmallEgressLimit) containerId, executorInfo, dir.get(), - None(), None()); AWAIT_READY(preparation1); @@ -1669,7 +1656,6 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_PortMappingStatistics) containerId, executorInfo, dir1.get(), - None(), None()); AWAIT_READY(preparation1);
