You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2019/08/08 04:54:48 UTC

[mesos] branch master updated (04ef1dc -> 543fb80)

This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 04ef1dc  Updated include guards in the sorter header files.
     new 8a47fdd  Propagate ephemeral volume information from rootfs.
     new 3a209b0  Supported multiple quota paths in the `disk/xfs` isolator.
     new 3446ca6  Add `disk/xfs` isolator support for ephemeral volumes.
     new abf9ae2  Update `disk/xfs` tests for rootfs quotas.
     new 8ba0682  Updated the `disk/du` isolator to support rootfs checks.
     new 543fb80  Updated the `disk/du` disk isolator tests with rootfs cases.

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 include/mesos/slave/containerizer.proto            |   6 +
 src/slave/containerizer/mesos/containerizer.cpp    |  18 +-
 .../containerizer/mesos/isolators/posix/disk.cpp   | 144 ++++---
 .../containerizer/mesos/isolators/posix/disk.hpp   |  15 +-
 .../containerizer/mesos/isolators/xfs/disk.cpp     | 242 +++++++++---
 .../containerizer/mesos/isolators/xfs/disk.hpp     |  11 +-
 .../containerizer/mesos/provisioner/backend.hpp    |   7 +-
 .../mesos/provisioner/backends/aufs.cpp            |   8 +-
 .../mesos/provisioner/backends/aufs.hpp            |   2 +-
 .../mesos/provisioner/backends/bind.cpp            |   9 +-
 .../mesos/provisioner/backends/bind.hpp            |   2 +-
 .../mesos/provisioner/backends/copy.cpp            |   9 +-
 .../mesos/provisioner/backends/copy.hpp            |   2 +-
 .../mesos/provisioner/backends/overlay.cpp         |  32 +-
 .../mesos/provisioner/backends/overlay.hpp         |   8 +-
 .../mesos/provisioner/provisioner.cpp              |   5 +-
 .../mesos/provisioner/provisioner.hpp              |   5 +
 src/tests/containerizer/rootfs.cpp                 |   1 +
 src/tests/containerizer/xfs_quota_tests.cpp        | 419 +++++++++++++++++----
 src/tests/disk_quota_tests.cpp                     | 192 ++++++++--
 src/tests/mesos.cpp                                |  33 ++
 src/tests/mesos.hpp                                |  18 +
 22 files changed, 958 insertions(+), 230 deletions(-)


[mesos] 01/06: Propagate ephemeral volume information from rootfs.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 8a47fdd86c1ca9536eeca4f23b6cac5bd8ea4f2d
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Aug 7 20:21:40 2019 -0700

    Propagate ephemeral volume information from rootfs.
    
    Propagate the overlayfs upperdir path to the containerization
    layer in a general form as a set of ephemeral volume paths.
    
    Review: https://reviews.apache.org/r/71192/
---
 include/mesos/slave/containerizer.proto                |  6 ++++++
 src/slave/containerizer/mesos/containerizer.cpp        | 18 +++++++++++++++++-
 src/slave/containerizer/mesos/provisioner/backend.hpp  |  7 ++++++-
 .../containerizer/mesos/provisioner/backends/aufs.cpp  |  8 ++++----
 .../containerizer/mesos/provisioner/backends/aufs.hpp  |  2 +-
 .../containerizer/mesos/provisioner/backends/bind.cpp  |  9 +++++----
 .../containerizer/mesos/provisioner/backends/bind.hpp  |  2 +-
 .../containerizer/mesos/provisioner/backends/copy.cpp  |  9 +++++----
 .../containerizer/mesos/provisioner/backends/copy.hpp  |  2 +-
 .../mesos/provisioner/backends/overlay.cpp             | 11 +++++++----
 .../mesos/provisioner/backends/overlay.hpp             |  2 +-
 .../containerizer/mesos/provisioner/provisioner.cpp    |  5 +++--
 .../containerizer/mesos/provisioner/provisioner.hpp    |  5 +++++
 13 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/include/mesos/slave/containerizer.proto b/include/mesos/slave/containerizer.proto
index a60c963..3e5b667 100644
--- a/include/mesos/slave/containerizer.proto
+++ b/include/mesos/slave/containerizer.proto
@@ -79,6 +79,9 @@ message ContainerState {
 
   // The sandbox directory.
   required string directory = 4;
+
+  // Ephemeral path volumes subject to container disk quota.
+  repeated string ephemeral_volumes = 5;
 }
 
 
@@ -142,6 +145,9 @@ message ContainerConfig {
   // The work directory for the container in the host filesystem.
   required string directory = 3;
 
+  // Ephemeral path volumes subject to container disk quota.
+  repeated string ephemeral_volumes = 15;
+
   // The user that should be used to run the `command_info`.
   // The sandbox directory and any artifacts from the Mesos fetcher will
   // be made accessible to this user.
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index a01edc8..75e7cda 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -904,7 +904,7 @@ Future<Nothing> MesosContainerizerProcess::recover(
   }
 
   // Recover the containers from 'SlaveState'.
-  foreach (const ContainerState& state, recoverable) {
+  foreach (ContainerState& state, recoverable) {
     const ContainerID& containerId = state.container_id();
 
     // Contruct the structure for containers from the 'SlaveState'
@@ -931,6 +931,10 @@ Future<Nothing> MesosContainerizerProcess::recover(
 
     if (config.isSome()) {
       container->config = config.get();
+
+      // Copy the ephemeral volume paths to the ContainerState, since this
+      // information is otherwise only available to the prepare() callback.
+      *state.mutable_ephemeral_volumes() = config->ephemeral_volumes();
     } else {
       VLOG(1) << "No config is recovered for container " << containerId
               << ", this means image pruning will be disabled.";
@@ -1094,6 +1098,12 @@ Future<Nothing> MesosContainerizerProcess::recover(
             container->pid.get(),
             container->directory.get());
 
+      if (config.isSome()) {
+        // Copy the ephemeral volume paths to the ContainerState, since this
+        // information is otherwise only available to the prepare() callback.
+        *state.mutable_ephemeral_volumes() = config->ephemeral_volumes();
+      }
+
       recoverable.push_back(state);
       continue;
     }
@@ -1517,6 +1527,12 @@ Future<Nothing> MesosContainerizerProcess::prepare(
   if (provisionInfo.isSome()) {
     container->config->set_rootfs(provisionInfo->rootfs);
 
+    if (provisionInfo->ephemeralVolumes.isSome()) {
+      foreach (const Path& path, provisionInfo->ephemeralVolumes.get()) {
+        container->config->add_ephemeral_volumes(path);
+      }
+    }
+
     if (provisionInfo->dockerManifest.isSome() &&
         provisionInfo->appcManifest.isSome()) {
       return Failure("Container cannot have both Docker and Appc manifests");
diff --git a/src/slave/containerizer/mesos/provisioner/backend.hpp b/src/slave/containerizer/mesos/provisioner/backend.hpp
index 7257d3a..55f007f 100644
--- a/src/slave/containerizer/mesos/provisioner/backend.hpp
+++ b/src/slave/containerizer/mesos/provisioner/backend.hpp
@@ -24,6 +24,8 @@
 #include <process/owned.hpp>
 
 #include <stout/hashmap.hpp>
+#include <stout/option.hpp>
+#include <stout/path.hpp>
 #include <stout/try.hpp>
 
 #include "slave/flags.hpp"
@@ -48,7 +50,10 @@ public:
   // directory by applying the specified list of root filesystem layers in
   // the list order, i.e., files in a layer can overwrite/shadow those from
   // another layer earlier in the list.
-  virtual process::Future<Nothing> provision(
+  //
+  // Optionally returns a set of paths whose contents should be included
+  // in the ephemeral sandbox disk quota.
+  virtual process::Future<Option<std::vector<Path>>> provision(
       const std::vector<std::string>& layers,
       const std::string& rootfs,
       const std::string& backendDir) = 0;
diff --git a/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp b/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp
index 2eba552..e401e5b 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp
@@ -51,7 +51,7 @@ public:
   AufsBackendProcess()
     : ProcessBase(process::ID::generate("aufs-provisioner-backend")) {}
 
-  Future<Nothing> provision(
+  Future<Option<vector<Path>>> provision(
       const vector<string>& layers,
       const string& rootfs,
       const string& backendDir);
@@ -87,7 +87,7 @@ AufsBackend::AufsBackend(Owned<AufsBackendProcess> _process)
 }
 
 
-Future<Nothing> AufsBackend::provision(
+Future<Option<vector<Path>>> AufsBackend::provision(
     const vector<string>& layers,
     const string& rootfs,
     const string& backendDir)
@@ -113,7 +113,7 @@ Future<bool> AufsBackend::destroy(
 }
 
 
-Future<Nothing> AufsBackendProcess::provision(
+Future<Option<vector<Path>>> AufsBackendProcess::provision(
     const vector<string>& layers,
     const string& rootfs,
     const string& backendDir)
@@ -236,7 +236,7 @@ Future<Nothing> AufsBackendProcess::provision(
         "' as a shared mount: " + mount.error());
   }
 
-  return Nothing();
+  return None();
 }
 
 
diff --git a/src/slave/containerizer/mesos/provisioner/backends/aufs.hpp b/src/slave/containerizer/mesos/provisioner/backends/aufs.hpp
index 2c25187..8a08339 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/aufs.hpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/aufs.hpp
@@ -48,7 +48,7 @@ public:
 
   static Try<process::Owned<Backend>> create(const Flags&);
 
-  process::Future<Nothing> provision(
+  process::Future<Option<std::vector<Path>>> provision(
       const std::vector<std::string>& layers,
       const std::string& rootfs,
       const std::string& backendDir) override;
diff --git a/src/slave/containerizer/mesos/provisioner/backends/bind.cpp b/src/slave/containerizer/mesos/provisioner/backends/bind.cpp
index 4cc4c52..e906a88 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/bind.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/bind.cpp
@@ -47,7 +47,8 @@ public:
   BindBackendProcess()
     : ProcessBase(process::ID::generate("bind-provisioner-backend")) {}
 
-  Future<Nothing> provision(const vector<string>& layers, const string& rootfs);
+  Future<Option<vector<Path>>> provision(
+      const vector<string>& layers, const string& rootfs);
 
   Future<bool> destroy(const string& rootfs);
 
@@ -86,7 +87,7 @@ BindBackend::BindBackend(Owned<BindBackendProcess> _process)
 }
 
 
-Future<Nothing> BindBackend::provision(
+Future<Option<std::vector<Path>>> BindBackend::provision(
     const vector<string>& layers,
     const string& rootfs,
     const string& backendDir)
@@ -104,7 +105,7 @@ Future<bool> BindBackend::destroy(
 }
 
 
-Future<Nothing> BindBackendProcess::provision(
+Future<Option<std::vector<Path>>> BindBackendProcess::provision(
     const vector<string>& layers,
     const string& rootfs)
 {
@@ -164,7 +165,7 @@ Future<Nothing> BindBackendProcess::provision(
         "' as a shared mount: " + mount.error());
   }
 
-  return Nothing();
+  return None();
 }
 
 
diff --git a/src/slave/containerizer/mesos/provisioner/backends/bind.hpp b/src/slave/containerizer/mesos/provisioner/backends/bind.hpp
index c4a1d5f..9c908fd 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/bind.hpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/bind.hpp
@@ -55,7 +55,7 @@ public:
   // BindBackend doesn't use any flag.
   static Try<process::Owned<Backend>> create(const Flags&);
 
-  process::Future<Nothing> provision(
+  process::Future<Option<std::vector<Path>>> provision(
       const std::vector<std::string>& layers,
       const std::string& rootfs,
       const std::string& backendDir) override;
diff --git a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
index 10516ca..4afef7f 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
@@ -52,7 +52,8 @@ public:
   CopyBackendProcess()
     : ProcessBase(process::ID::generate("copy-provisioner-backend")) {}
 
-  Future<Nothing> provision(const vector<string>& layers, const string& rootfs);
+  Future<Option<vector<Path>>> provision(
+      const vector<string>& layers, const string& rootfs);
 
   Future<bool> destroy(const string& rootfs);
 
@@ -82,7 +83,7 @@ CopyBackend::CopyBackend(Owned<CopyBackendProcess> _process)
 }
 
 
-Future<Nothing> CopyBackend::provision(
+Future<Option<vector<Path>>> CopyBackend::provision(
     const vector<string>& layers,
     const string& rootfs,
     const string& backendDir)
@@ -100,7 +101,7 @@ Future<bool> CopyBackend::destroy(
 }
 
 
-Future<Nothing> CopyBackendProcess::provision(
+Future<Option<vector<Path>>> CopyBackendProcess::provision(
     const vector<string>& layers,
     const string& rootfs)
 {
@@ -126,7 +127,7 @@ Future<Nothing> CopyBackendProcess::provision(
   }
 
   return collect(futures)
-    .then([]() -> Future<Nothing> { return Nothing(); });
+    .then([]() -> Future<Option<vector<Path>>> { return None(); });
 }
 
 
diff --git a/src/slave/containerizer/mesos/provisioner/backends/copy.hpp b/src/slave/containerizer/mesos/provisioner/backends/copy.hpp
index 5dc9a2f..9e8bd97 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/copy.hpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/copy.hpp
@@ -45,7 +45,7 @@ public:
 
   // Provisions a rootfs given the layers' paths and target rootfs
   // path.
-  process::Future<Nothing> provision(
+  process::Future<Option<std::vector<Path>>> provision(
       const std::vector<std::string>& layers,
       const std::string& rootfs,
       const std::string& backendDir) override;
diff --git a/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp b/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
index f2040cf..77d6711 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
@@ -52,7 +52,7 @@ public:
   OverlayBackendProcess()
     : ProcessBase(process::ID::generate("overlay-provisioner-backend")) {}
 
-  Future<Nothing> provision(
+  Future<Option<vector<Path>>> provision(
       const vector<string>& layers,
       const string& rootfs,
       const string& backendDir);
@@ -88,7 +88,7 @@ OverlayBackend::OverlayBackend(Owned<OverlayBackendProcess> _process)
 }
 
 
-Future<Nothing> OverlayBackend::provision(
+Future<Option<vector<Path>>> OverlayBackend::provision(
     const vector<string>& layers,
     const string& rootfs,
     const string& backendDir)
@@ -114,7 +114,7 @@ Future<bool> OverlayBackend::destroy(
 }
 
 
-Future<Nothing> OverlayBackendProcess::provision(
+Future<Option<vector<Path>>> OverlayBackendProcess::provision(
     const vector<string>& layers,
     const string& rootfs,
     const string& backendDir)
@@ -237,7 +237,10 @@ Future<Nothing> OverlayBackendProcess::provision(
         "' as a shared mount: " + mount.error());
   }
 
-  return Nothing();
+  // Note that both upperdir and workdir are ephemeral. The `disk/xfs`
+  // isolator needs this because XFS will error with EXDEV when renaming
+  // a file into a tree with a different project ID (see xfs_rename).
+  return vector<Path>{Path(upperdir), Path(workdir)};
 }
 
 
diff --git a/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp b/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp
index 362e021..78896b6 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp
@@ -49,7 +49,7 @@ public:
 
   static Try<process::Owned<Backend>> create(const Flags&);
 
-  process::Future<Nothing> provision(
+  process::Future<Option<std::vector<Path>>> provision(
       const std::vector<std::string>& layers,
       const std::string& rootfs,
       const std::string& backendDir) override;
diff --git a/src/slave/containerizer/mesos/provisioner/provisioner.cpp b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
index a081fb0..bf3908d 100644
--- a/src/slave/containerizer/mesos/provisioner/provisioner.cpp
+++ b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
@@ -574,7 +574,8 @@ Future<ProvisionInfo> ProvisionerProcess::_provision(
       imageInfo.layers,
       rootfs,
       backendDir)
-    .then(defer(self(), [=]() -> Future<ProvisionInfo> {
+    .then(defer(self(), [=](const Option<vector<Path>>& ephemeral)
+    -> Future<ProvisionInfo> {
       const string path =
         provisioner::paths::getLayersFilePath(rootDir, containerId);
 
@@ -596,7 +597,7 @@ Future<ProvisionInfo> ProvisionerProcess::_provision(
       }
 
       return ProvisionInfo{
-          rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};
+          rootfs, ephemeral, imageInfo.dockerManifest, imageInfo.appcManifest};
     }));
 }
 
diff --git a/src/slave/containerizer/mesos/provisioner/provisioner.hpp b/src/slave/containerizer/mesos/provisioner/provisioner.hpp
index 7f84aa4..3866417 100644
--- a/src/slave/containerizer/mesos/provisioner/provisioner.hpp
+++ b/src/slave/containerizer/mesos/provisioner/provisioner.hpp
@@ -30,6 +30,7 @@
 #include <mesos/slave/isolator.hpp> // For ContainerState.
 
 #include <stout/nothing.hpp>
+#include <stout/path.hpp>
 #include <stout/try.hpp>
 
 #include <process/future.hpp>
@@ -61,6 +62,10 @@ struct ProvisionInfo
 {
   std::string rootfs;
 
+  // Ephemeral volumes are any additional paths the Provisioner backend
+  // may have created that should be counted towards the sandbox disk quota.
+  Option<std::vector<Path>> ephemeralVolumes;
+
   // Docker v1 image manifest.
   Option<::docker::spec::v1::ImageManifest> dockerManifest;
 


[mesos] 03/06: Add `disk/xfs` isolator support for ephemeral volumes.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 3446ca6f05152fb34f607bfe69ad3175e6a5e806
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Aug 7 20:22:03 2019 -0700

    Add `disk/xfs` isolator support for ephemeral volumes.
    
    Add support for labeling ephemeral volumes with the sandbox XFS
    project ID. This makes changes to the container rootfs share the
    same disk quota as the sandbox.
    
    Review: https://reviews.apache.org/r/71194/
---
 .../containerizer/mesos/isolators/xfs/disk.cpp     | 152 +++++++++++++++++++--
 .../mesos/provisioner/backends/overlay.cpp         |  21 ++-
 .../mesos/provisioner/backends/overlay.hpp         |   6 +
 3 files changed, 166 insertions(+), 13 deletions(-)

diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
index 5454432..1680a59 100644
--- a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
+++ b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
@@ -38,6 +38,8 @@
 
 #include "slave/paths.hpp"
 
+#include "slave/containerizer/mesos/provisioner/backends/overlay.hpp"
+
 using std::list;
 using std::make_pair;
 using std::pair;
@@ -317,6 +319,64 @@ Future<Nothing> XfsDiskIsolatorProcess::recover(
     }
   }
 
+  foreach (const ContainerState& state, states) {
+    foreach (const string& directory, state.ephemeral_volumes()) {
+      Result<prid_t> projectId = xfs::getProjectId(directory);
+      if (projectId.isError()) {
+        return Failure(projectId.error());
+      }
+
+      // Ephemeral volumes should have been assigned a project ID, but
+      // that's not atommic, so if we were killed during a container
+      // launch, we can't guarantee that we labeled all the ephemeral
+      // volumes.
+      if (projectId.isNone()) {
+        if (!infos.contains(state.container_id())) {
+          LOG(WARNING) << "Missing project ID for ephemeral volume at '"
+                       << directory << "'";
+          continue;
+        }
+
+        // We have an unlabeled ephemeral volume for a known container. Find
+        // the corresponding sandbox path to get the right project ID.
+        const Owned<Info>& info = infos.at(state.container_id());
+
+        foreachpair (
+            const string& directory,
+            const Info::PathInfo& pathInfo,
+            info->paths) {
+          // Skip persistent volumes.
+          if (pathInfo.disk.isSome()) {
+            continue;
+          }
+
+          Try<Nothing> status =
+            xfs::setProjectId(directory, pathInfo.projectId);
+          if (status.isError()) {
+            return Failure(
+                "Failed to assign project " +
+                stringify(projectId.get()) + ": " + status.error());
+          }
+
+          break;
+        }
+      }
+
+      Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
+      if (scheduled.isError()) {
+        return Failure(
+            "Unable to schedule project ID " + stringify(projectId.get()) +
+            " for reclaimation: " + scheduled.error());
+      }
+
+      // If we are still managing this project ID, we should have
+      // tracked it when we added sandboxes above.
+      if (totalProjectIds.contains(projectId.get())) {
+          CHECK_NOT_CONTAINS(freeProjectIds, projectId.get());
+      }
+    }
+  }
+
   Try<list<string>> volumes = paths::getPersistentVolumePaths(workDir);
 
   if (volumes.isError()) {
@@ -355,8 +415,52 @@ Future<Nothing> XfsDiskIsolatorProcess::recover(
 
     Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
     if (scheduled.isError()) {
-      LOG(ERROR) << "Unable to schedule project ID " << projectId.get()
-                 << " for reclaimation: " << scheduled.error();
+      return Failure(
+          "Unable to schedule project ID " + stringify(projectId.get()) +
+          " for reclaimation: " + scheduled.error());
+    }
+  }
+
+  // Scan ephemeral provisioner directories to pick up any project IDs that
+  // aren't already captured by the recovered container states. Admittedly,
+  // it's a bit hacky to specifically check the overlay backend here,
+  // but it's not really worse than the sandbox scanning we do above.
+  Try<list<string>> provisionerDirs =
+    OverlayBackend::listEphemeralVolumes(workDir);
+
+  if (provisionerDirs.isError()) {
+    return Failure("Failed to scan overlay provisioner directories: " +
+                   provisionerDirs.error());
+  }
+
+  foreach (const string& directory, provisionerDirs.get()) {
+    if (!os::stat::isdir(directory)) {
+      continue;
+    }
+
+    Result<prid_t> projectId = xfs::getProjectId(directory);
+    if (projectId.isError()) {
+      return Failure(projectId.error());
+    }
+
+    // Not all provisioner directories will have a project IDs.
+    if (projectId.isNone()) {
+      continue;
+    }
+
+    // It is likely we counted this project ID when we recovered the
+    // containers, so don't double-count.
+    if (totalProjectIds.contains(projectId.get()) &&
+        freeProjectIds.contains(projectId.get())) {
+      --metrics.project_ids_free;
+      freeProjectIds -= projectId.get();
+    }
+
+    Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
+    if (scheduled.isError()) {
+      return Failure(
+          "Unable to schedule project ID " + stringify(projectId.get()) +
+          " for reclaimation: " + scheduled.error());
     }
   }
 
@@ -395,8 +499,30 @@ Future<Option<ContainerLaunchInfo>> XfsDiskIsolatorProcess::prepare(
         status.error());
   }
 
-  LOG(INFO) << "Assigned project " << stringify(projectId.get()) << " to '"
-            << containerConfig.directory() << "'";
+  LOG(INFO) << "Assigned project " << stringify(projectId.get())
+            << " to '" << containerConfig.directory() << "'";
+
+  // The ephemeral volumes share the same quota as the sandbox, so label
+  // them with the project ID now.
+  foreach (const string& directory, containerConfig.ephemeral_volumes()) {
+    Try<Nothing> status = xfs::setProjectId(directory, projectId.get());
+
+    if (status.isError()) {
+      return Failure(
+          "Failed to assign project " + stringify(projectId.get()) + ": " +
+          status.error());
+    }
+
+    LOG(INFO) << "Assigned project " << stringify(projectId.get())
+              << " to '" << directory << "'";
+
+    Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
+    if (scheduled.isError()) {
+      return Failure(
+          "Unable to schedule project ID " + stringify(projectId.get()) +
+          " for reclaimation: " + scheduled.error());
+    }
+  }
 
   return update(containerId, containerConfig.resources())
     .then([]() -> Future<Option<ContainerLaunchInfo>> {
@@ -409,7 +535,7 @@ Future<ContainerLimitation> XfsDiskIsolatorProcess::watch(
     const ContainerID& containerId)
 {
   if (infos.contains(containerId)) {
-    return infos[containerId]->limitation.future();
+    return infos.at(containerId)->limitation.future();
   }
 
   // Any container that did not have a project ID assigned when
@@ -477,7 +603,7 @@ Future<Nothing> XfsDiskIsolatorProcess::update(
     return Nothing();
   }
 
-  const Owned<Info>& info = infos[containerId];
+  const Owned<Info>& info = infos.at(containerId);
 
   // First, apply the disk quota to the sandbox.
   Option<Bytes> sandboxQuota = getSandboxDisk(resources);
@@ -565,8 +691,9 @@ Future<Nothing> XfsDiskIsolatorProcess::update(
 
     Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
     if (scheduled.isError()) {
-      LOG(ERROR) << "Unable to schedule project " << projectId.get()
-                  << " for reclaimation: " << scheduled.error();
+      return Failure(
+          "Unable to schedule project " + stringify(projectId.get()) +
+          " for reclaimation: " + scheduled.error());
     }
   }
 
@@ -637,7 +764,7 @@ Future<ResourceStatistics> XfsDiskIsolatorProcess::usage(
     return ResourceStatistics();
   }
 
-  const Owned<Info>& info = infos[containerId];
+  const Owned<Info>& info = infos.at(containerId);
   ResourceStatistics statistics;
 
   foreachpair(
@@ -706,7 +833,7 @@ Future<Nothing> XfsDiskIsolatorProcess::cleanup(const ContainerID& containerId)
     return Nothing();
   }
 
-  const Owned<Info>& info = infos[containerId];
+  const Owned<Info>& info = infos.at(containerId);
 
   // Schedule the directory for project ID reclaimation.
   //
@@ -724,8 +851,9 @@ Future<Nothing> XfsDiskIsolatorProcess::cleanup(const ContainerID& containerId)
       const string& directory, const Info::PathInfo& pathInfo, info->paths) {
     Try<Nothing> scheduled = scheduleProjectRoot(pathInfo.projectId, directory);
     if (scheduled.isError()) {
-      LOG(ERROR) << "Unable to schedule project " << pathInfo.projectId
-                 << " for reclaimation: " << scheduled.error();
+      return Failure(
+          "Unable to schedule project " + stringify(pathInfo.projectId) +
+          " for reclaimation: " + scheduled.error());
     }
   }
 
diff --git a/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp b/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
index 77d6711..cf261d1 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
@@ -14,6 +14,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "slave/containerizer/mesos/provisioner/backends/overlay.hpp"
+
 #include <process/dispatch.hpp>
 #include <process/id.hpp>
 #include <process/process.hpp>
@@ -27,7 +29,9 @@
 
 #include "linux/fs.hpp"
 
-#include "slave/containerizer/mesos/provisioner/backends/overlay.hpp"
+#include "slave/paths.hpp"
+
+#include "slave/containerizer/mesos/provisioner/constants.hpp"
 
 using process::Failure;
 using process::Future;
@@ -63,6 +67,21 @@ public:
 };
 
 
+Try<std::list<std::string>> OverlayBackend::listEphemeralVolumes(
+    const string& workDir)
+{
+  return os::glob(path::join(
+    paths::getProvisionerDir(workDir),
+    "containers",
+    "*", /* ContainerID */
+    "backends",
+    OVERLAY_BACKEND, /* backendDir */
+    "scratch"
+    "*", /* rootfs ID */
+    "*"));
+}
+
+
 Try<Owned<Backend>> OverlayBackend::create(const Flags&)
 {
   if (geteuid() != 0) {
diff --git a/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp b/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp
index 78896b6..e116064 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/overlay.hpp
@@ -17,6 +17,9 @@
 #ifndef __MESOS_PROVISIONER_OVERLAY_HPP__
 #define __MESOS_PROVISIONER_OVERLAY_HPP__
 
+#include <list>
+#include <string>
+
 #include "slave/containerizer/mesos/provisioner/backend.hpp"
 
 namespace mesos {
@@ -49,6 +52,9 @@ public:
 
   static Try<process::Owned<Backend>> create(const Flags&);
 
+  static Try<std::list<std::string>> listEphemeralVolumes(
+      const std::string& workDir);
+
   process::Future<Option<std::vector<Path>>> provision(
       const std::vector<std::string>& layers,
       const std::string& rootfs,


[mesos] 02/06: Supported multiple quota paths in the `disk/xfs` isolator.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 3a209b0a83751e500395844e1d1e87d15543b681
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Aug 7 20:21:56 2019 -0700

    Supported multiple quota paths in the `disk/xfs` isolator.
    
    The `disk/xfs` isolator assumed that there would only be a single
    directory path for each project quota. When we apply project quotas
    to the overlayfs upperdir, that won't be true any more, since the
    upperdir will come under the same quota as the task sandbox.
    
    Update the quota reclamation tracking to keep a set of disk paths that
    the quota has been applied to, and only reclaim the project ID once all
    those paths have been removed.
    
    Review: https://reviews.apache.org/r/71193/
---
 .../containerizer/mesos/isolators/xfs/disk.cpp     | 102 ++++++++++++---------
 .../containerizer/mesos/isolators/xfs/disk.hpp     |  11 ++-
 2 files changed, 68 insertions(+), 45 deletions(-)

diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
index 646330c..5454432 100644
--- a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
+++ b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
@@ -345,23 +345,19 @@ Future<Nothing> XfsDiskIsolatorProcess::recover(
     // about this persistent volume.
     CHECK(!scheduledProjects.contains(projectId.get()))
         << "Duplicate project ID " << projectId.get()
-        << " assigned to '" << directory << "'"
-        << " and '" << scheduledProjects.at(projectId.get()).second << "'";
+        << " assigned to '" << directory << "' and '"
+        << *scheduledProjects.at(projectId.get()).directories.begin() << "'";
 
     freeProjectIds -= projectId.get();
     if (totalProjectIds.contains(projectId.get())) {
       --metrics.project_ids_free;
     }
 
-    Try<string> devname = xfs::getDeviceForPath(directory);
-    if (devname.isError()) {
+    Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
+    if (scheduled.isError()) {
       LOG(ERROR) << "Unable to schedule project ID " << projectId.get()
-                 << " for reclaimation: " << devname.error();
-      continue;
+                 << " for reclaimation: " << scheduled.error();
     }
-
-    scheduledProjects.put(
-        projectId.get(), make_pair(devname.get(), directory));
   }
 
   return Nothing();
@@ -567,16 +563,10 @@ Future<Nothing> XfsDiskIsolatorProcess::update(
               << " for project " << projectId.get()
               << " to " << status->softLimit << "/" << status->hardLimit;
 
-    if (!scheduledProjects.contains(projectId.get())) {
-      Try<string> devname = xfs::getDeviceForPath(directory);
-      if (devname.isError()) {
-        LOG(ERROR) << "Unable to schedule project " << projectId.get()
-                   << " for reclaimation: " << devname.error();
-        continue;
-      }
-
-      scheduledProjects.put(
-          projectId.get(), make_pair(devname.get(), directory));
+    Try<Nothing> scheduled = scheduleProjectRoot(projectId.get(), directory);
+    if (scheduled.isError()) {
+      LOG(ERROR) << "Unable to schedule project " << projectId.get()
+                  << " for reclaimation: " << scheduled.error();
     }
   }
 
@@ -732,21 +722,11 @@ Future<Nothing> XfsDiskIsolatorProcess::cleanup(const ContainerID& containerId)
   // we determine that the persistent volume is no longer present.
   foreachpair (
       const string& directory, const Info::PathInfo& pathInfo, info->paths) {
-    // If we assigned a project ID to a persistent volume, it might
-    // already be scheduled for reclaimation.
-    if (scheduledProjects.contains(pathInfo.projectId)) {
-      continue;
-    }
-
-    Try<string> devname = xfs::getDeviceForPath(directory);
-    if (devname.isError()) {
+    Try<Nothing> scheduled = scheduleProjectRoot(pathInfo.projectId, directory);
+    if (scheduled.isError()) {
       LOG(ERROR) << "Unable to schedule project " << pathInfo.projectId
-                 << " for reclaimation: " << devname.error();
-      continue;
+                 << " for reclaimation: " << scheduled.error();
     }
-
-    scheduledProjects.put(
-        pathInfo.projectId, make_pair(devname.get(), directory));
   }
 
   infos.erase(containerId);
@@ -781,6 +761,33 @@ void XfsDiskIsolatorProcess::returnProjectId(
 }
 
 
+Try<Nothing> XfsDiskIsolatorProcess::scheduleProjectRoot(
+    prid_t projectId,
+    const string& rootDir)
+{
+  Try<string> devname = xfs::getDeviceForPath(rootDir);
+
+  if (devname.isError()) {
+    return Error(devname.error());
+  }
+
+  if (!scheduledProjects.contains(projectId)) {
+    scheduledProjects.put(projectId, ProjectRoots{devname.get(), {rootDir}});
+  } else {
+    ProjectRoots& roots = scheduledProjects.at(projectId);
+
+    if (roots.deviceName != devname.get()) {
+      return Error(strings::format(
+            "Conflicting device names '%s' and '%s' for project ID %s",
+            roots.deviceName, devname.get(), projectId).get());
+    }
+
+    roots.directories.insert(rootDir);
+  }
+
+  return Nothing();
+}
+
 void XfsDiskIsolatorProcess::reclaimProjectIds()
 {
   // Note that we need both the directory we assigned the project ID to,
@@ -789,22 +796,29 @@ void XfsDiskIsolatorProcess::reclaimProjectIds()
   // need the latter to make the corresponding quota record updates.
 
   foreachpair (
-      prid_t projectId, const auto& dir, utils::copy(scheduledProjects)) {
-    if (os::exists(dir.second)) {
-      continue;
+      prid_t projectId, auto& roots, utils::copy(scheduledProjects)) {
+    // Stop tracking any directories that have already been removed.
+    foreach (const string& directory, utils::copy(roots.directories)) {
+      if (!os::exists(directory)) {
+        roots.directories.erase(directory);
+
+        VLOG(1) << "Droppped path '" << directory
+                << "' from project ID " << projectId;
+      }
     }
 
-    Try<Nothing> status = xfs::clearProjectQuota(dir.first, projectId);
-    if (status.isError()) {
-      LOG(ERROR) << "Failed to clear quota for '"
-                 << dir.second << "': " << status.error();
-    }
+    if (roots.directories.empty()) {
+      Try<Nothing> status = xfs::clearProjectQuota(roots.deviceName, projectId);
+      if (status.isError()) {
+        LOG(ERROR) << "Failed to clear quota for project ID "
+                   << projectId << "': " << status.error();
+      }
 
-    returnProjectId(projectId);
-    scheduledProjects.erase(projectId);
+      returnProjectId(projectId);
+      scheduledProjects.erase(projectId);
 
-    LOG(INFO) << "Reclaimed project ID " << projectId
-              << " from '" << dir.second << "'";
+      LOG(INFO) << "Reclaimed project ID " << projectId;
+    }
   }
 }
 
diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.hpp b/src/slave/containerizer/mesos/isolators/xfs/disk.hpp
index 94d44e7..16bb432 100644
--- a/src/slave/containerizer/mesos/isolators/xfs/disk.hpp
+++ b/src/slave/containerizer/mesos/isolators/xfs/disk.hpp
@@ -27,6 +27,7 @@
 #include <stout/bytes.hpp>
 #include <stout/duration.hpp>
 #include <stout/hashmap.hpp>
+#include <stout/hashset.hpp>
 
 #include "slave/flags.hpp"
 
@@ -93,6 +94,11 @@ private:
     process::Promise<mesos::slave::ContainerLimitation> limitation;
   };
 
+  struct ProjectRoots {
+    std::string deviceName;
+    hashset<std::string> directories;
+  };
+
   XfsDiskIsolatorProcess(
       Duration watchInterval,
       xfs::QuotaPolicy quotaPolicy,
@@ -113,6 +119,9 @@ private:
   // that are not.
   void reclaimProjectIds();
 
+  Try<Nothing> scheduleProjectRoot(
+      prid_t projectId, const std::string& rootDir);
+
   const Duration watchInterval;
   const Duration projectWatchInterval;
   xfs::QuotaPolicy quotaPolicy;
@@ -123,7 +132,7 @@ private:
 
   // Track the device and filesystem path of unused project IDs we want
   // to reclaim.
-  hashmap<prid_t, std::pair<std::string, std::string>> scheduledProjects;
+  hashmap<prid_t, ProjectRoots> scheduledProjects;
 
   // Metrics used by the XFS disk isolator.
   struct Metrics


[mesos] 06/06: Updated the `disk/du` disk isolator tests with rootfs cases.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 543fb80a736ed336bf9441ac45d53f335bf7e1c9
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Aug 7 20:22:27 2019 -0700

    Updated the `disk/du` disk isolator tests with rootfs cases.
    
    Added the disk enforcement type parameter to the `disk/du` disk
    isolator tests to verify rootfs ephemeral storage quota handling.
    
    Review: https://reviews.apache.org/r/71197/
---
 src/tests/disk_quota_tests.cpp | 192 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 170 insertions(+), 22 deletions(-)

diff --git a/src/tests/disk_quota_tests.cpp b/src/tests/disk_quota_tests.cpp
index cbb1ccf..edf9597 100644
--- a/src/tests/disk_quota_tests.cpp
+++ b/src/tests/disk_quota_tests.cpp
@@ -47,6 +47,10 @@
 #include "tests/mesos.hpp"
 #include "tests/utils.hpp"
 
+#if __linux__
+#include "tests/containerizer/docker_archive.hpp"
+#endif
+
 using namespace process;
 
 using std::string;
@@ -175,10 +179,44 @@ TEST_F(DiskUsageCollectorTest, ExcludeRelativePath)
 class DiskQuotaTest : public MesosTest {};
 
 
+class DiskQuotaEnforcement
+  : public DiskQuotaTest,
+    public ::testing::WithParamInterface<ParamDiskQuota::Type>
+{
+public:
+  DiskQuotaEnforcement() :DiskQuotaTest() {}
+
+  static Future<Nothing> CreateDockerImage(
+      const string& registry, const string& name)
+  {
+#if __linux__
+    return DockerArchive::create(registry, name);
+#else
+    return Failure("DockerArchive is only supported on Linux");
+#endif
+  }
+};
+
+
+INSTANTIATE_TEST_CASE_P(
+    Enforcing,
+    DiskQuotaEnforcement,
+    ::testing::ValuesIn(ParamDiskQuota::parameters()),
+    ParamDiskQuota::Printer());
+
+
 // This test verifies that the container will be killed if the disk
 // usage exceeds its quota.
-TEST_F(DiskQuotaTest, DiskUsageExceedsQuota)
+TEST_P(DiskQuotaEnforcement, DiskUsageExceedsQuota)
 {
+  // NOTE: We don't use the "ROOT_" tag on the test because that
+  // would require the SANDBOX tests to be run as root as well.
+  // If we did that, then we would lose disk isolator test coverage
+  // in the default CI configuration.
+  if (GetParam() == ParamDiskQuota::ROOTFS && ::geteuid() != 0) {
+    return;
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
@@ -190,6 +228,17 @@ TEST_F(DiskQuotaTest, DiskUsageExceedsQuota)
   flags.container_disk_watch_interval = Milliseconds(1);
   flags.enforce_container_disk_quota = true;
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.isolation += ",filesystem/linux,docker/runtime";
+
+    flags.docker_registry = path::join(sandbox.get(), "registry");
+    flags.docker_store_dir = path::join(sandbox.get(), "store");
+    flags.image_providers = "docker";
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(CreateDockerImage(flags.docker_registry, "test_image"));
+  }
+
   Owned<MasterDetector> detector = master.get()->createDetector();
   Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
   ASSERT_SOME(slave);
@@ -212,12 +261,25 @@ TEST_F(DiskQuotaTest, DiskUsageExceedsQuota)
 
   const Offer& offer = offers.get()[0];
 
+  TaskInfo task;
+
   // Create a task which requests 1MB disk, but actually uses more
   // than 2MB disk.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      Resources::parse("cpus:1;mem:128;disk:1").get(),
-      "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 1000");
+  switch (GetParam()) {
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+        offer.slave_id(),
+        Resources::parse("cpus:1;mem:128;disk:1").get(),
+        "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 1000");
+      break;
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+        offer.slave_id(),
+        Resources::parse("cpus:1;mem:128;disk:1").get(),
+        "dd if=/dev/zero of=/tmp/file bs=1048576 count=2 && sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+  }
 
   Future<TaskStatus> status0;
   Future<TaskStatus> status1;
@@ -353,8 +415,12 @@ TEST_F(DiskQuotaTest, VolumeUsageExceedsQuota)
 // This test verifies that the container will not be killed if
 // disk_enforce_quota flag is false (even if the disk usage exceeds
 // its quota).
-TEST_F(DiskQuotaTest, NoQuotaEnforcement)
+TEST_P(DiskQuotaEnforcement, NoQuotaEnforcement)
 {
+  if (GetParam() == ParamDiskQuota::ROOTFS && ::geteuid() != 0) {
+    return;
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
@@ -366,6 +432,17 @@ TEST_F(DiskQuotaTest, NoQuotaEnforcement)
   flags.container_disk_watch_interval = Milliseconds(1);
   flags.enforce_container_disk_quota = false;
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.isolation += ",filesystem/linux,docker/runtime";
+
+    flags.docker_registry = path::join(sandbox.get(), "registry");
+    flags.docker_store_dir = path::join(sandbox.get(), "store");
+    flags.image_providers = "docker";
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(CreateDockerImage(flags.docker_registry, "test_image"));
+  }
+
   Fetcher fetcher(flags);
 
   Try<MesosContainerizer*> _containerizer =
@@ -401,11 +478,24 @@ TEST_F(DiskQuotaTest, NoQuotaEnforcement)
 
   const Offer& offer = offers.get()[0];
 
+  TaskInfo task;
+
   // Create a task that uses 2MB disk.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      Resources::parse("cpus:1;mem:128;disk:1").get(),
-      "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 1000");
+  switch (GetParam()) {
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+        offer.slave_id(),
+        Resources::parse("cpus:1;mem:128;disk:1").get(),
+        "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 1000");
+      break;
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+        offer.slave_id(),
+        Resources::parse("cpus:1;mem:128;disk:1").get(),
+        "dd if=/dev/zero of=/tmp/file bs=1048576 count=2 && sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+  }
 
   Future<TaskStatus> statusStarting;
   Future<TaskStatus> statusRunning;
@@ -459,8 +549,12 @@ TEST_F(DiskQuotaTest, NoQuotaEnforcement)
 }
 
 
-TEST_F(DiskQuotaTest, ResourceStatistics)
+TEST_P(DiskQuotaEnforcement, ResourceStatistics)
 {
+  if (GetParam() == ParamDiskQuota::ROOTFS && ::geteuid() != 0) {
+    return;
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
@@ -473,6 +567,17 @@ TEST_F(DiskQuotaTest, ResourceStatistics)
   // the 'du' subprocess.
   flags.container_disk_watch_interval = Milliseconds(1);
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.isolation += ",filesystem/linux,docker/runtime";
+
+    flags.docker_registry = path::join(sandbox.get(), "registry");
+    flags.docker_store_dir = path::join(sandbox.get(), "store");
+    flags.image_providers = "docker";
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(CreateDockerImage(flags.docker_registry, "test_image"));
+  }
+
   Fetcher fetcher(flags);
 
   Try<MesosContainerizer*> _containerizer =
@@ -533,13 +638,28 @@ TEST_F(DiskQuotaTest, ResourceStatistics)
 
   taskResources += volume;
 
+  TaskInfo task;
+
   // Create a task that uses 2MB disk.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      taskResources,
-      "dd if=/dev/zero of=file bs=1048576 count=2 && "
-      "dd if=/dev/zero of=path1/file bs=1048576 count=2 && "
-      "sleep 1000");
+  switch (GetParam()) {
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+        offer.slave_id(),
+        taskResources,
+        "dd if=/dev/zero of=file bs=1048576 count=2 && "
+        "dd if=/dev/zero of=path1/file bs=1048576 count=2 && "
+        "sleep 1000");
+      break;
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+        offer.slave_id(),
+        taskResources,
+        "dd if=/dev/zero of=/tmp/file bs=1048576 count=2 && "
+        "dd if=/dev/zero of=path1/file bs=1048576 count=2 && "
+        "sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+  }
 
   Future<TaskStatus> status0;
   Future<TaskStatus> status1;
@@ -624,8 +744,12 @@ TEST_F(DiskQuotaTest, ResourceStatistics)
 
 // This test verifies that disk quota isolator recovers properly after
 // the slave restarts.
-TEST_F(DiskQuotaTest, SlaveRecovery)
+TEST_P(DiskQuotaEnforcement, SlaveRecovery)
 {
+  if (GetParam() == ParamDiskQuota::ROOTFS && ::geteuid() != 0) {
+    return;
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
@@ -633,6 +757,17 @@ TEST_F(DiskQuotaTest, SlaveRecovery)
   flags.isolation = "posix/cpu,posix/mem,disk/du";
   flags.container_disk_watch_interval = Milliseconds(1);
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.isolation += ",filesystem/linux,docker/runtime";
+
+    flags.docker_registry = path::join(sandbox.get(), "registry");
+    flags.docker_store_dir = path::join(sandbox.get(), "store");
+    flags.image_providers = "docker";
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(CreateDockerImage(flags.docker_registry, "test_image"));
+  }
+
   Fetcher fetcher(flags);
 
   Try<MesosContainerizer*> _containerizer =
@@ -672,11 +807,24 @@ TEST_F(DiskQuotaTest, SlaveRecovery)
 
   const Offer& offer = offers.get()[0];
 
+  TaskInfo task;
+
   // Create a task that uses 2MB disk.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      Resources::parse("cpus:1;mem:128;disk:3").get(),
-      "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 1000");
+  switch (GetParam()) {
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+        offer.slave_id(),
+        Resources::parse("cpus:1;mem:128;disk:3").get(),
+        "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 1000");
+      break;
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+        offer.slave_id(),
+        Resources::parse("cpus:1;mem:128;disk:3").get(),
+        "dd if=/dev/zero of=/tmp/file bs=1048576 count=2 && sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+  }
 
   Future<TaskStatus> statusStarting;
   Future<TaskStatus> statusRunning;


[mesos] 04/06: Update `disk/xfs` tests for rootfs quotas.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit abf9ae28205947ec967d7d7a33321517b0ba06f3
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Aug 7 20:22:10 2019 -0700

    Update `disk/xfs` tests for rootfs quotas.
    
    Parameterize the `disk/xfs` isolator tests on whether we
    are enforcing quotas on the sandbox or on the container
    rootfs. Separating out the rootfs and sandbox cases wth
    parameterization helps to ensure that quota is correctly
    applied in both cases and tests don't accidentally pass.
    
    Review: https://reviews.apache.org/r/71195/
---
 src/tests/containerizer/rootfs.cpp          |   1 +
 src/tests/containerizer/xfs_quota_tests.cpp | 419 ++++++++++++++++++++++------
 src/tests/mesos.cpp                         |  33 +++
 src/tests/mesos.hpp                         |  18 ++
 4 files changed, 390 insertions(+), 81 deletions(-)

diff --git a/src/tests/containerizer/rootfs.cpp b/src/tests/containerizer/rootfs.cpp
index 206cab6..35a5509 100644
--- a/src/tests/containerizer/rootfs.cpp
+++ b/src/tests/containerizer/rootfs.cpp
@@ -133,6 +133,7 @@ Try<process::Owned<Rootfs>> LinuxRootfs::create(const string& root)
     "/bin/ls",
     "/bin/mkdir",
     "/bin/ping",
+    "/bin/rm",
     "/bin/sh",
     "/bin/sleep",
   };
diff --git a/src/tests/containerizer/xfs_quota_tests.cpp b/src/tests/containerizer/xfs_quota_tests.cpp
index 96cd40d..6703a1c 100644
--- a/src/tests/containerizer/xfs_quota_tests.cpp
+++ b/src/tests/containerizer/xfs_quota_tests.cpp
@@ -49,14 +49,20 @@
 #include "slave/slave.hpp"
 
 #include "slave/containerizer/fetcher.hpp"
+
 #include "slave/containerizer/mesos/containerizer.hpp"
+
 #include "slave/containerizer/mesos/isolators/xfs/disk.hpp"
 #include "slave/containerizer/mesos/isolators/xfs/utils.hpp"
 
+#include "slave/containerizer/mesos/provisioner/backends/overlay.hpp"
+
 #include "tests/environment.hpp"
 #include "tests/mesos.hpp"
 #include "tests/utils.hpp"
 
+#include "tests/containerizer/docker_archive.hpp"
+
 using namespace mesos::internal::xfs;
 
 using namespace process;
@@ -205,8 +211,15 @@ public:
     // We only need an XFS-specific directory for the work directory. We
     // don't mind that other flags refer to a different temp directory.
     flags.work_dir = mountPoint.get();
-    flags.isolation = "disk/xfs,filesystem/linux";
+    flags.isolation = "disk/xfs,filesystem/linux,docker/runtime";
     flags.enforce_container_disk_quota = true;
+
+    // Note that the docker registry doesn't need to be on XFS. The
+    // provisioner store does, but we get that via work_dir.
+    flags.docker_registry = path::join(sandbox.get(), "registry");
+    flags.docker_store_dir = path::join(sandbox.get(), "store");
+    flags.image_providers = "docker";
+
     return flags;
   }
 
@@ -303,6 +316,23 @@ public:
 };
 
 
+class ROOT_XFS_QuotaEnforcement
+  : public ROOT_XFS_TestBase,
+    public ::testing::WithParamInterface<ParamDiskQuota::Type>
+{
+public:
+  ROOT_XFS_QuotaEnforcement()
+    : ROOT_XFS_TestBase("prjquota") {}
+};
+
+
+INSTANTIATE_TEST_CASE_P(
+    Enforcing,
+    ROOT_XFS_QuotaEnforcement,
+    ::testing::ValuesIn(ParamDiskQuota::parameters()),
+    ParamDiskQuota::Printer());
+
+
 TEST_F(ROOT_XFS_QuotaTest, QuotaGetSet)
 {
   prid_t projectId = 44;
@@ -454,14 +484,22 @@ TEST_F(ROOT_XFS_QuotaTest, DirectoryTree)
 // is only allowed to consume exactly the assigned resources. We tell dd
 // to write 2MB but only give it 1MB of resources and (roughly) verify that
 // it exits with a failure (that should be a write error).
-TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuota)
+TEST_P(ROOT_XFS_QuotaEnforcement, DiskUsageExceedsQuota)
 {
+  slave::Flags flags = CreateSlaveFlags();
+
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
   Owned<MasterDetector> detector = master.get()->createDetector();
-  Try<Owned<cluster::Slave>> slave =
-    StartSlave(detector.get(), CreateSlaveFlags());
+
+  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
   ASSERT_SOME(slave);
 
   MockScheduler sched;
@@ -484,10 +522,23 @@ TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuota)
 
   // Create a task which requests 1MB disk, but actually uses more
   // than 2MB disk.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      Resources::parse("cpus:1;mem:128;disk:1").get(),
-      "dd if=/dev/zero of=file bs=1048576 count=2");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offer.slave_id(),
+          Resources::parse("cpus:1;mem:128;disk:1").get(),
+          "pwd; dd if=/dev/zero of=/tmp/file bs=1048576 count=2");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offer.slave_id(),
+          Resources::parse("cpus:1;mem:128;disk:1").get(),
+          "pwd; dd if=/dev/zero of=file bs=1048576 count=2");
+      break;
+  }
 
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
@@ -843,16 +894,23 @@ TEST_F(ROOT_XFS_QuotaTest, VolumeUsageExceedsQuotaWithKill)
 
 // This is the same logic as DiskUsageExceedsQuota except we turn off disk quota
 // enforcement, so exceeding the quota should be allowed.
-TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaNoEnforce)
+TEST_P(ROOT_XFS_QuotaEnforcement, DiskUsageExceedsQuotaNoEnforce)
 {
+  slave::Flags flags = CreateSlaveFlags();
+
+  flags.enforce_container_disk_quota = false;
+
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
   Owned<MasterDetector> detector = master.get()->createDetector();
 
-  slave::Flags flags = CreateSlaveFlags();
-  flags.enforce_container_disk_quota = false;
-
   Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
   ASSERT_SOME(slave);
 
@@ -876,10 +934,24 @@ TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaNoEnforce)
 
   // Create a task which requests 1MB disk, but actually uses more
   // than 2MB disk.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      Resources::parse("cpus:1;mem:128;disk:1").get(),
-      "dd if=/dev/zero of=file bs=1048576 count=2");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offer.slave_id(),
+          Resources::parse("cpus:1;mem:128;disk:1").get(),
+          "pwd; dd if=/dev/zero of=/tmp/file bs=1048576 count=2");
+
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offer.slave_id(),
+          Resources::parse("cpus:1;mem:128;disk:1").get(),
+          "pwd; dd if=/dev/zero of=file bs=1048576 count=2");
+      break;
+  }
 
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
@@ -912,13 +984,8 @@ TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaNoEnforce)
 
 // Verify that when the `xfs_kill_containers` flag is enabled, tasks that
 // exceed their disk quota are killed with the correct container limitation.
-TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaWithKill)
+TEST_P(ROOT_XFS_QuotaEnforcement, DiskUsageExceedsQuotaWithKill)
 {
-  Try<Owned<cluster::Master>> master = StartMaster();
-  ASSERT_SOME(master);
-
-  Owned<MasterDetector> detector = master.get()->createDetector();
-
   slave::Flags flags = CreateSlaveFlags();
 
   // Enable killing containers on disk quota violations.
@@ -928,6 +995,17 @@ TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaWithKill)
   // the quota violation as soon as possible.
   flags.container_disk_watch_interval = Milliseconds(1);
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
   Try<Owned<cluster::Slave>> slave =
     StartSlave(detector.get(), flags);
   ASSERT_SOME(slave);
@@ -953,10 +1031,23 @@ TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaWithKill)
   // Create a task which requests 1MB disk, but actually uses 2MB. This
   // waits a long time to ensure that the task lives long enough for the
   // isolator to impose a container limitation.
-  TaskInfo task = createTask(
-      offer.slave_id(),
-      Resources::parse("cpus:1;mem:128;disk:1").get(),
-      "dd if=/dev/zero of=file bs=1048576 count=2 && sleep 100000");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offer.slave_id(),
+          Resources::parse("cpus:1;mem:128;disk:1").get(),
+          "pwd; dd if=/dev/zero of=/tmp/file bs=1048576 count=2");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offer.slave_id(),
+          Resources::parse("cpus:1;mem:128;disk:1").get(),
+          "pwd; dd if=/dev/zero of=file bs=1048576 count=2");
+      break;
+  }
 
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
@@ -1007,18 +1098,25 @@ TEST_F(ROOT_XFS_QuotaTest, DiskUsageExceedsQuotaWithKill)
 
 // Verify that we can get accurate resource statistics from the XFS
 // disk isolator.
-TEST_F(ROOT_XFS_QuotaTest, ResourceStatistics)
+TEST_P(ROOT_XFS_QuotaEnforcement, ResourceStatistics)
 {
+  slave::Flags flags = CreateSlaveFlags();
+
+  flags.resources = strings::format(
+      "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
+
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
   FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
   frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
 
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
-  slave::Flags flags = CreateSlaveFlags();
-  flags.resources = strings::format(
-      "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
-
   Fetcher fetcher(flags);
   Owned<MasterDetector> detector = master.get()->createDetector();
 
@@ -1062,15 +1160,33 @@ TEST_F(ROOT_XFS_QuotaTest, ResourceStatistics)
     createDiskResource("3", DEFAULT_TEST_ROLE, None(), None()) +
     volume;
 
-  TaskInfo task = createTask(
-      offers->at(0).slave_id(),
-      taskResources,
-      "touch path1/working && "
-      "touch path1/started && "
-      "dd if=/dev/zero of=file bs=1048576 count=1 && "
-      "dd if=/dev/zero of=path1/file bs=1048576 count=1 && "
-      "rm path1/working && "
-      "sleep 1000");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "echo touch > path1/working && "
+          "echo touch > path1/started && "
+          "dd if=/dev/zero of=/tmp/file bs=1048576 count=1 && "
+          "dd if=/dev/zero of=path1/file bs=1048576 count=1 && "
+          "rm path1/working && "
+          "sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "echo touch > path1/working && "
+          "echo touch > path1/started && "
+          "dd if=/dev/zero of=file bs=1048576 count=1 && "
+          "dd if=/dev/zero of=path1/file bs=1048576 count=1 && "
+          "rm path1/working && "
+          "sleep 1000");
+      break;
+  }
 
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
@@ -1137,7 +1253,11 @@ TEST_F(ROOT_XFS_QuotaTest, ResourceStatistics)
       ASSERT_TRUE(statistics.has_used_bytes());
 
       EXPECT_EQ(Megabytes(3), Bytes(statistics.limit_bytes()));
-      EXPECT_EQ(Megabytes(1), Bytes(statistics.used_bytes()));
+
+      // We can't precisely control how much ephemeral space is consumed
+      // by the rootfs, so check a reasonable range.
+      EXPECT_GE(Bytes(statistics.used_bytes()), Megabytes(1));
+      EXPECT_LE(Bytes(statistics.used_bytes()), Kilobytes(1400));
 
       EXPECT_EQ("id1", statistics.persistence().id());
       EXPECT_EQ(
@@ -1161,19 +1281,27 @@ TEST_F(ROOT_XFS_QuotaTest, ResourceStatistics)
 // This is the same logic as ResourceStatistics, except the task should
 // be allowed to exceed the disk quota, and usage statistics should report
 // that the quota was exceeded.
-TEST_F(ROOT_XFS_QuotaTest, ResourceStatisticsNoEnforce)
+TEST_P(ROOT_XFS_QuotaEnforcement, ResourceStatisticsNoEnforce)
 {
+  slave::Flags flags = CreateSlaveFlags();
+
+  flags.enforce_container_disk_quota = false;
+
+  flags.resources = strings::format(
+      "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
+
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
   FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
   frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
 
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
-  slave::Flags flags = CreateSlaveFlags();
-  flags.enforce_container_disk_quota = false;
-  flags.resources = strings::format(
-      "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
-
   Fetcher fetcher(flags);
   Owned<MasterDetector> detector = master.get()->createDetector();
 
@@ -1217,15 +1345,33 @@ TEST_F(ROOT_XFS_QuotaTest, ResourceStatisticsNoEnforce)
     createDiskResource("1", DEFAULT_TEST_ROLE, None(), None()) +
     volume;
 
-  TaskInfo task = createTask(
-      offers->at(0).slave_id(),
-      taskResources,
-      "touch path1/working && "
-      "touch path1/started && "
-      "dd if=/dev/zero of=file bs=1048576 count=2 && "
-      "dd if=/dev/zero of=path1/file bs=1048576 count=2 && "
-      "rm path1/working && "
-      "sleep 1000");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "echo touch > path1/working && "
+          "echo touch > path1/started && "
+          "dd if=/dev/zero of=/tmp/file bs=1048576 count=2 && "
+          "dd if=/dev/zero of=path1/file bs=1048576 count=2 && "
+          "rm path1/working && "
+          "sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "echo touch > path1/working && "
+          "echo touch > path1/started && "
+          "dd if=/dev/zero of=file bs=1048576 count=2 && "
+          "dd if=/dev/zero of=path1/file bs=1048576 count=2 && "
+          "rm path1/working && "
+          "sleep 1000");
+      break;
+  }
 
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
@@ -1291,7 +1437,10 @@ TEST_F(ROOT_XFS_QuotaTest, ResourceStatisticsNoEnforce)
       ASSERT_TRUE(statistics.has_used_bytes());
 
       EXPECT_EQ(Megabytes(1), Bytes(statistics.limit_bytes()));
-      EXPECT_EQ(Megabytes(2), Bytes(statistics.used_bytes()));
+
+      // We can't precisely control how much ephemeral space is consumed
+      // by the rootfs, so check a reasonable range.
+      EXPECT_GE(Bytes(statistics.used_bytes()), Megabytes(2));
 
       EXPECT_EQ("id1", statistics.persistence().id());
       EXPECT_EQ(
@@ -1315,18 +1464,25 @@ TEST_F(ROOT_XFS_QuotaTest, ResourceStatisticsNoEnforce)
 // In this test, the framework is not checkpointed. This ensures that when we
 // stop the slave, the executor is killed and we will need to recover the
 // working directories without getting any checkpointed recovery state.
-TEST_F(ROOT_XFS_QuotaTest, NoCheckpointRecovery)
+TEST_P(ROOT_XFS_QuotaEnforcement, NoCheckpointRecovery)
 {
+  slave::Flags flags = CreateSlaveFlags();
+
+  flags.resources = strings::format(
+      "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
+
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
   FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
   frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
 
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
-  slave::Flags flags = CreateSlaveFlags();
-  flags.resources = strings::format(
-      "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
-
   Fetcher fetcher(flags);
   Try<MesosContainerizer*> _containerizer =
     MesosContainerizer::create(flags, true, &fetcher);
@@ -1374,10 +1530,23 @@ TEST_F(ROOT_XFS_QuotaTest, NoCheckpointRecovery)
     createDiskResource("1", DEFAULT_TEST_ROLE, None(), None()) +
     volume;
 
-  TaskInfo task = createTask(
-      offers->at(0).slave_id(),
-      taskResources,
-      "dd if=/dev/zero of=file bs=1048576 count=1; sleep 1000");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "dd if=/dev/zero of=/tmp/file bs=1048576 count=1; sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "dd if=/dev/zero of=file bs=1048576 count=1; sleep 1000");
+      break;
+  }
 
   Future<TaskStatus> runningStatus;
   Future<TaskStatus> startingStatus;
@@ -1475,6 +1644,28 @@ TEST_F(ROOT_XFS_QuotaTest, NoCheckpointRecovery)
     EXPECT_SOME(xfs::getProjectQuota(path, projectId.get()));
   }
 
+  // Since we are not checkpointing, the rootfs should be gone.
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    int count = 0;
+
+    Try<list<string>> dirs =
+      slave::OverlayBackend::listEphemeralVolumes(flags.work_dir);
+
+    ASSERT_SOME(dirs);
+
+    foreach (const string& dir, dirs.get()) {
+      Result<prid_t> projectId = xfs::getProjectId(dir);
+      ASSERT_FALSE(projectId.isError()) << projectId.error();
+
+      EXPECT_NONE(projectId);
+      if (projectId.isSome()) {
+        ++count;
+      }
+    }
+
+    EXPECT_EQ(0, count);
+  }
+
   // We should have project IDs still allocated for the persistent volume and
   // for the task sandbox (since it is not GC'd yet).
   JSON::Object metrics = Metrics();
@@ -1492,16 +1683,23 @@ TEST_F(ROOT_XFS_QuotaTest, NoCheckpointRecovery)
 // In this test, the framework is checkpointed so we expect the executor to
 // persist across the slave restart and to have the same resource usage before
 // and after.
-TEST_F(ROOT_XFS_QuotaTest, CheckpointRecovery)
+TEST_P(ROOT_XFS_QuotaEnforcement, CheckpointRecovery)
 {
-  FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
-  frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
-  frameworkInfo.set_checkpoint(true);
-
   slave::Flags flags = CreateSlaveFlags();
+
   flags.resources = strings::format(
       "disk(%s):%d", DEFAULT_TEST_ROLE, DISK_SIZE_MB).get();
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
+  FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
+  frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
+  frameworkInfo.set_checkpoint(true);
+
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
@@ -1539,10 +1737,23 @@ TEST_F(ROOT_XFS_QuotaTest, CheckpointRecovery)
     createDiskResource("1", DEFAULT_TEST_ROLE, None(), None()) +
     volume;
 
-  TaskInfo task = createTask(
-      offers->at(0).slave_id(),
-      taskResources,
-      "dd if=/dev/zero of=file bs=1048576 count=1; sleep 1000");
+  TaskInfo task;
+
+  switch (GetParam()) {
+    case ParamDiskQuota::ROOTFS:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "dd if=/dev/zero of=/tmp/file bs=1048576 count=1; sleep 1000");
+      task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+      break;
+    case ParamDiskQuota::SANDBOX:
+      task = createTask(
+          offers->at(0).slave_id(),
+          taskResources,
+          "dd if=/dev/zero of=file bs=1048576 count=1; sleep 1000");
+      break;
+  }
 
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
@@ -1624,6 +1835,28 @@ TEST_F(ROOT_XFS_QuotaTest, CheckpointRecovery)
     EXPECT_SOME(xfs::getProjectQuota(path, projectId.get()));
   }
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    int count = 0;
+    Try<list<string>> dirs =
+      slave::OverlayBackend::listEphemeralVolumes(flags.work_dir);
+
+    ASSERT_SOME(dirs);
+    EXPECT_FALSE(dirs->empty());
+
+    foreach (const string& dir, dirs.get()) {
+      Result<prid_t> projectId = xfs::getProjectId(dir);
+      ASSERT_FALSE(projectId.isError()) << projectId.error();
+
+      if (projectId.isSome()) {
+        ++count;
+        EXPECT_SOME(xfs::getProjectQuota(dir, projectId.get()));
+      }
+    }
+
+    EXPECT_GT(1, count)
+      << "overlay provisioner backend is missing project IDs";
+  }
+
   JSON::Object metrics = Metrics();
   EXPECT_EQ(
       metrics.at<JSON::Number>("containerizer/mesos/disk/project_ids_total")
@@ -1638,9 +1871,9 @@ TEST_F(ROOT_XFS_QuotaTest, CheckpointRecovery)
 
 // In this test, the agent initially doesn't enable disk isolation
 // but then restarts with XFS disk isolation enabled. We verify that
-// the old container launched before the agent restart is
+// the old container that was launched before the agent restart is
 // successfully recovered.
-TEST_F(ROOT_XFS_QuotaTest, RecoverOldContainers)
+TEST_P(ROOT_XFS_QuotaEnforcement, RecoverOldContainers)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
@@ -1650,8 +1883,14 @@ TEST_F(ROOT_XFS_QuotaTest, RecoverOldContainers)
   slave::Flags flags = CreateSlaveFlags();
 
   // `CreateSlaveFlags()` enables `disk/xfs` so here we reset
-  // `isolation` to empty.
-  flags.isolation.clear();
+  // `isolation` to remove it.
+  flags.isolation = "filesystem/linux,docker/runtime";
+
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
 
   Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
   ASSERT_SOME(slave);
@@ -1677,11 +1916,17 @@ TEST_F(ROOT_XFS_QuotaTest, RecoverOldContainers)
 
   Offer offer = offers.get()[0];
 
-  TaskInfo task = createTask(
+  TaskInfo task;
+
+  task = createTask(
       offer.slave_id(),
       Resources::parse("cpus:1;mem:128;disk:1").get(),
       "dd if=/dev/zero of=file bs=1024 count=1; sleep 1000");
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+  }
+
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningstatus;
   EXPECT_CALL(sched, statusUpdate(&driver, _))
@@ -1736,6 +1981,7 @@ TEST_F(ROOT_XFS_QuotaTest, RecoverOldContainers)
     const ResourceUsage_Executor& executor = usage->executors().Get(0);
     ASSERT_TRUE(executor.has_statistics());
     ASSERT_FALSE(executor.statistics().has_disk_limit_bytes());
+    ASSERT_FALSE(executor.statistics().has_disk_used_bytes());
   }
 
   // Verify that we haven't allocated any project IDs.
@@ -1753,7 +1999,7 @@ TEST_F(ROOT_XFS_QuotaTest, RecoverOldContainers)
 
 // Verify that XFS project IDs are reclaimed when sandbox directories they were
 // set on are garbage collected.
-TEST_F(ROOT_XFS_QuotaTest, ProjectIdReclaiming)
+TEST_P(ROOT_XFS_QuotaEnforcement, ProjectIdReclaiming)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
@@ -1761,9 +2007,16 @@ TEST_F(ROOT_XFS_QuotaTest, ProjectIdReclaiming)
   Owned<MasterDetector> detector = master.get()->createDetector();
 
   slave::Flags flags = CreateSlaveFlags();
+
   flags.gc_delay = Seconds(10);
   flags.disk_watch_interval = Seconds(10);
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    flags.image_provisioner_backend = "overlay";
+
+    AWAIT_READY(DockerArchive::create(flags.docker_registry, "test_image"));
+  }
+
   Try<Resource> projects =
     Resources::parse("projects", flags.xfs_project_range, "*");
   ASSERT_SOME(projects);
@@ -1813,6 +2066,10 @@ TEST_F(ROOT_XFS_QuotaTest, ProjectIdReclaiming)
       Resources::parse("cpus:1;mem:128;disk:2").get(),
       "dd if=/dev/zero of=file bs=1048576 count=1 && sleep 1000");
 
+  if (GetParam() == ParamDiskQuota::ROOTFS) {
+    task.mutable_container()->CopyFrom(createContainerInfo("test_image"));
+  }
+
   Future<TaskStatus> startingStatus;
   Future<TaskStatus> runningStatus;
   Future<TaskStatus> exitStatus;
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index df363ab..0396ce7 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -35,6 +35,7 @@
 
 #ifdef __linux__
 #include "linux/cgroups.hpp"
+#include "linux/fs.hpp"
 #endif
 
 #ifdef ENABLE_PORT_MAPPING_ISOLATOR
@@ -56,6 +57,7 @@ using mesos::master::detector::MasterDetector;
 using std::list;
 using std::shared_ptr;
 using std::string;
+using std::vector;
 
 using testing::_;
 
@@ -935,6 +937,37 @@ void ContainerizerTest<slave::MesosContainerizer>::TearDown()
 }
 #endif // __linux__
 
+
+string ParamDiskQuota::Printer::operator()(
+  const ::testing::TestParamInfo<ParamDiskQuota::Type>& info) const
+{
+  switch (info.param) {
+    case SANDBOX:
+      return "Sandbox";
+    case ROOTFS:
+      return "Rootfs";
+    default:
+      UNREACHABLE();
+  }
+}
+
+
+vector<ParamDiskQuota::Type> ParamDiskQuota::parameters()
+{
+  vector<Type> params{SANDBOX};
+
+  // ROOTFS tests depend on overlayfs being available, since that is
+  // the only provisioner backend that supports ephemeral volumes.
+#if __linux__
+  Try<bool> overlayfsSupported = fs::supported("overlayfs");
+  if (overlayfsSupported.isSome() && overlayfsSupported.get()) {
+    params.push_back(ROOTFS);
+  }
+#endif
+
+  return params;
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index cd50673..73b6e42 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -3919,6 +3919,24 @@ private:
   Type type;
 };
 
+
+struct ParamDiskQuota
+{
+  enum Type
+  {
+    SANDBOX,
+    ROOTFS,
+  };
+
+  struct Printer
+  {
+    std::string operator()(
+        const ::testing::TestParamInfo<ParamDiskQuota::Type>& info) const;
+  };
+
+  static std::vector<Type> parameters();
+};
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {


[mesos] 05/06: Updated the `disk/du` isolator to support rootfs checks.

Posted by jp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 8ba0682521c6051b42f33b3dd96a37f4d46a290d
Author: James Peach <jp...@apache.org>
AuthorDate: Wed Aug 7 20:22:21 2019 -0700

    Updated the `disk/du` isolator to support rootfs checks.
    
    Updated the `disk/du` isolator to support isolating rootfs ephemeral
    volumes. We need to keep track of the ephemeral paths and start a `du`
    check for each one of them, but otherwise treat them in the same way
    as the sandbox path.
    
    Review: https://reviews.apache.org/r/71196/
---
 .../containerizer/mesos/isolators/posix/disk.cpp   | 144 ++++++++++++++-------
 .../containerizer/mesos/isolators/posix/disk.hpp   |  15 ++-
 2 files changed, 108 insertions(+), 51 deletions(-)

diff --git a/src/slave/containerizer/mesos/isolators/posix/disk.cpp b/src/slave/containerizer/mesos/isolators/posix/disk.cpp
index 4869c72..29bdbe6 100644
--- a/src/slave/containerizer/mesos/isolators/posix/disk.cpp
+++ b/src/slave/containerizer/mesos/isolators/posix/disk.cpp
@@ -135,7 +135,13 @@ Future<Nothing> PosixDiskIsolatorProcess::recover(
     CHECK(os::exists(state.directory()))
       << "Executor work directory " << state.directory() << " doesn't exist";
 
-    infos.put(state.container_id(), Owned<Info>(new Info(state.directory())));
+    Owned<Info> info(new Info(state.directory()));
+
+    foreach (const string& path, state.ephemeral_volumes()) {
+      info->directories.insert(path);
+    }
+
+    infos.put(state.container_id(), info);
   }
 
   return Nothing();
@@ -157,8 +163,13 @@ Future<Option<ContainerLaunchInfo>> PosixDiskIsolatorProcess::prepare(
     return Failure("Container has already been prepared");
   }
 
-  infos.put(containerId, Owned<Info>(new Info(containerConfig.directory())));
+  Owned<Info> info(new Info(containerConfig.directory()));
+
+  foreach (const string& path, containerConfig.ephemeral_volumes()) {
+    info->directories.insert(path);
+  }
 
+  infos.put(containerId, info);
   return None();
 }
 
@@ -232,7 +243,7 @@ Future<Nothing> PosixDiskIsolatorProcess::update(
       // If either DiskInfo or DiskInfo.Volume are not set we're
       // dealing with the working directory of the executor (aka the
       // sandbox).
-      path = info->directory;
+      path = info->sandbox;
     } else {
       // Otherwise it is a disk resource (such as a persistent volume)
       // and we extract the path from the protobuf.
@@ -242,7 +253,7 @@ Future<Nothing> PosixDiskIsolatorProcess::update(
       // is relative to the working directory of the executor. We
       // always store the absolute path.
       if (!path::absolute(path)) {
-        path = path::join(info->directory, path);
+        path = path::join(info->sandbox, path);
       }
     }
 
@@ -253,6 +264,16 @@ Future<Nothing> PosixDiskIsolatorProcess::update(
     quotas[path] += resource;
   }
 
+  // If we still have a sandbox quota, include all the ephemeral
+  // quota directories so we include them in the collection state
+  // updates below.
+  if (quotas.contains(info->sandbox)) {
+    const Resources quota = quotas[info->sandbox];
+    foreach (const string& path, info->directories) {
+      quotas[path] = quota;
+    }
+  }
+
   // Update the quota for paths. For each new path we also initiate
   // the disk usage collection.
   foreachpair (const string& path, const Resources& quota, quotas) {
@@ -291,9 +312,9 @@ Future<Bytes> PosixDiskIsolatorProcess::collect(
   // 'du' process to incorrectly include the disk usage of the newly
   // added persistent volume to the usage of the sandbox.
   vector<string> excludes;
-  if (path == info->directory) {
+  if (info->directories.contains(path)) {
     foreachkey (const string& exclude, info->paths) {
-      if (exclude != info->directory) {
+      if (!info->directories.contains(exclude)) {
         excludes.push_back(exclude);
       }
     }
@@ -302,7 +323,7 @@ Future<Bytes> PosixDiskIsolatorProcess::collect(
   // We append "/" at the end to make sure that 'du' runs on actual
   // directory pointed by the symlink (and not the symlink itself).
   string _path = path;
-  if (path != info->directory && os::stat::islink(path)) {
+  if (!info->directories.contains(path) && os::stat::islink(path)) {
     _path = path::join(path, "");
   }
 
@@ -350,30 +371,40 @@ void PosixDiskIsolatorProcess::_collect(
     // Save the last disk usage.
     info->paths[path].lastUsage = future.get();
 
-    // We need to ignore the quota enforcement check for MOUNT type
-    // disk resources because its quota will be enforced by the
-    // underlying filesystem.
-    bool isDiskSourceMount = false;
-    foreach (const Resource& resource, info->paths[path].quota) {
-      if (resource.has_disk() &&
-          resource.disk().has_source() &&
-          resource.disk().source().type() ==
-            Resource::DiskInfo::Source::MOUNT) {
-        isDiskSourceMount = true;
+    if (flags.enforce_container_disk_quota) {
+      Bytes currentUsage = future.get();
+
+      // We need to ignore the quota enforcement check for MOUNT type
+      // disk resources because its quota will be enforced by the
+      // underlying filesystem.
+      bool isDiskSourceMount = false;
+      foreach (const Resource& resource, info->paths[path].quota) {
+        if (resource.has_disk() &&
+            resource.disk().has_source() &&
+            resource.disk().source().type() ==
+              Resource::DiskInfo::Source::MOUNT) {
+          isDiskSourceMount = true;
+        }
       }
-    }
 
-    if (flags.enforce_container_disk_quota && !isDiskSourceMount) {
-      Option<Bytes> quota = info->paths[path].quota.disk();
-      CHECK_SOME(quota);
-
-      if (future.get() > quota.get()) {
-        info->limitation.set(
-            protobuf::slave::createContainerLimitation(
-                Resources(info->paths[path].quota),
-                "Disk usage (" + stringify(future.get()) +
-                ") exceeds quota (" + stringify(quota.get()) + ")",
-                TaskStatus::REASON_CONTAINER_LIMITATION_DISK));
+      if (!isDiskSourceMount) {
+        // If this path is using the ephemeral quota, we need to
+        // estimate the total current usage.
+        if (info->directories.contains(path)) {
+          currentUsage = info->ephemeralUsage();
+        }
+
+        Option<Bytes> quota = info->paths[path].quota.disk();
+        CHECK_SOME(quota);
+
+        if (currentUsage > quota.get()) {
+          info->limitation.set(
+              protobuf::slave::createContainerLimitation(
+                  Resources(info->paths[path].quota),
+                  "Disk usage (" + stringify(currentUsage) +
+                  ") exceeds quota (" + stringify(quota.get()) + ")",
+                  TaskStatus::REASON_CONTAINER_LIMITATION_DISK));
+        }
       }
     }
   }
@@ -400,42 +431,49 @@ Future<ResourceStatistics> PosixDiskIsolatorProcess::usage(
   foreachpair (const string& path,
                const Info::PathInfo& pathInfo,
                info->paths) {
+    // Skip ephemeral paths. We explicitly deal with those below.
+    if (info->directories.contains(path)) {
+      continue;
+    }
+
     DiskStatistics *statistics = result.add_disk_statistics();
 
     Option<Bytes> quota = pathInfo.quota.disk();
     CHECK_SOME(quota);
 
     statistics->set_limit_bytes(quota->bytes());
-    if (path == info->directory) {
-      result.set_disk_limit_bytes(quota->bytes());
-    }
 
     // NOTE: There may be a large delay (# of containers * interval)
     // until an initial cached value is returned here!
     if (pathInfo.lastUsage.isSome()) {
       statistics->set_used_bytes(pathInfo.lastUsage->bytes());
-      if (path == info->directory) {
-        result.set_disk_used_bytes(pathInfo.lastUsage->bytes());
-      }
     }
 
-    // Set meta information for persistent volumes.
-    if (path != info->directory) {
-      // TODO(jieyu): For persistent volumes, validate that there is
-      // only one Resource object associated with it.
-      Resource resource = *pathInfo.quota.begin();
+    // TODO(jieyu): For persistent volumes, validate that there is
+    // only one Resource object associated with it.
+    Resource resource = *pathInfo.quota.begin();
 
-      if (resource.has_disk() && resource.disk().has_source()) {
-        statistics->mutable_source()->CopyFrom(resource.disk().source());
-      }
+    if (resource.has_disk() && resource.disk().has_source()) {
+      statistics->mutable_source()->CopyFrom(resource.disk().source());
+    }
 
-      if (resource.has_disk() && resource.disk().has_persistence()) {
-        statistics->mutable_persistence()->CopyFrom(
-            resource.disk().persistence());
-      }
+    if (resource.has_disk() && resource.disk().has_persistence()) {
+      statistics->mutable_persistence()->CopyFrom(
+          resource.disk().persistence());
     }
   }
 
+  result.set_disk_used_bytes(info->ephemeralUsage().bytes());
+
+  // It doesn't matter which ephemeral path we use to get the quota,
+  // since it's replicated there.
+  result.set_disk_limit_bytes(
+      info->paths[info->sandbox].quota.disk()->bytes());
+
+  DiskStatistics *statistics = result.add_disk_statistics();
+  statistics->set_limit_bytes(result.disk_limit_bytes());
+  statistics->set_used_bytes(result.disk_used_bytes());
+
   return result;
 }
 
@@ -460,6 +498,18 @@ Future<Nothing> PosixDiskIsolatorProcess::cleanup(
 }
 
 
+Bytes PosixDiskIsolatorProcess::Info::ephemeralUsage() const
+{
+  Bytes usage;
+
+  foreach (const string& path, directories) {
+    usage += paths.at(path).lastUsage.getOrElse(0);
+  }
+
+  return usage;
+}
+
+
 class DiskUsageCollectorProcess : public Process<DiskUsageCollectorProcess>
 {
 public:
diff --git a/src/slave/containerizer/mesos/isolators/posix/disk.hpp b/src/slave/containerizer/mesos/isolators/posix/disk.hpp
index f513dfc..2e9b481 100644
--- a/src/slave/containerizer/mesos/isolators/posix/disk.hpp
+++ b/src/slave/containerizer/mesos/isolators/posix/disk.hpp
@@ -24,6 +24,7 @@
 #include <stout/bytes.hpp>
 #include <stout/duration.hpp>
 #include <stout/hashmap.hpp>
+#include <stout/hashset.hpp>
 
 #include "slave/flags.hpp"
 
@@ -122,11 +123,17 @@ private:
 
   struct Info
   {
-    explicit Info(const std::string& _directory) : directory(_directory) {}
+    explicit Info(const std::string& _directory)
+      : directories({_directory}), sandbox(_directory) {}
 
-    // We save executor working directory here so that we know where
-    // to collect disk usage for disk resources without DiskInfo.
-    const std::string directory;
+    Bytes ephemeralUsage() const;
+
+    // Save the executor ephemeral storage (sandbox and rootfs)
+    // directories so that we know where to collect disk usage
+    // for disk resources without DiskInfo.
+    hashset<std::string> directories;
+
+    std::string sandbox;
 
     process::Promise<mesos::slave::ContainerLimitation> limitation;