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 2018/09/28 00:00:41 UTC
[mesos] 01/04: Added persistent volume support to the `disk/xfs`
isolator.
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 33d2cb42a21f3d90aaa5210c95aec021bf78fcae
Author: James Peach <jp...@apache.org>
AuthorDate: Thu Sep 27 16:17:06 2018 -0700
Added persistent volume support to the `disk/xfs` isolator.
Added persistent volume support to the `disk/xfs` isolator. This
implementation largely tracks the `disk/du` implementation in that
we now keep a map of paths in each container info structure. We now
defer quota clean up to project ID reclaimation time so that we can
use the same mechanism for sandbox and persistent volume paths.
We explicitly exclude mount disks from XFS project quotas, but we still
track them so that we can correctly publish their usage information in
the container `DiskStatistics` message. This means that mount disks are
not required to be XFS filesystems or have project quotas configured.
Review: https://reviews.apache.org/r/68401/
---
.../containerizer/mesos/isolators/xfs/disk.cpp | 481 ++++++++++++++++-----
.../containerizer/mesos/isolators/xfs/disk.hpp | 35 +-
src/tests/containerizer/xfs_quota_tests.cpp | 62 ++-
3 files changed, 440 insertions(+), 138 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
index 783da04..44b7d05 100644
--- a/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
+++ b/src/slave/containerizer/mesos/isolators/xfs/disk.cpp
@@ -27,7 +27,9 @@
#include <stout/check.hpp>
#include <stout/foreach.hpp>
+#include <stout/fs.hpp>
#include <stout/os.hpp>
+#include <stout/unreachable.hpp>
#include <stout/utils.hpp>
#include <stout/os/stat.hpp>
@@ -37,6 +39,8 @@
#include "slave/paths.hpp"
using std::list;
+using std::make_pair;
+using std::pair;
using std::string;
using std::vector;
@@ -75,7 +79,35 @@ static Try<IntervalSet<prid_t>> getIntervalSet(
}
-static Option<Bytes> getDiskResource(
+static Try<Nothing> isPathSupported(const string& path)
+{
+ if (!xfs::isPathXfs(path)) {
+ return Error("'" + path + "' is not an XFS filesystem");
+ }
+
+ Try<bool> enabled = xfs::isQuotaEnabled(path);
+ if (enabled.isError()) {
+ return Error(
+ "Failed to get quota status for '" + path + "': " + enabled.error());
+ }
+
+ if (!enabled.get()) {
+ return Error(
+ "XFS project quotas are not enabled on '" + path + "'");
+ }
+
+ return Nothing();
+}
+
+
+static bool isMountDisk(const Resource::DiskInfo& info)
+{
+ return info.has_source() &&
+ info.source().type() == Resource::DiskInfo::Source::MOUNT;
+}
+
+
+static Option<Bytes> getSandboxDisk(
const Resources& resources)
{
Option<Bytes> bytes = None();
@@ -85,16 +117,7 @@ static Option<Bytes> getDiskResource(
continue;
}
- // TODO(jpeach): Ignore persistent volume resources. The problem here is
- // that we need to guarantee that we can track the removal of every
- // directory for which we assign a project ID. Since destruction of
- // persistent is not visible to the isolator, we don't want to risk
- // leaking the project ID, or spuriously reusing it.
- if (Resources::isPersistentVolume(resource)) {
- continue;
- }
-
- if (resource.has_disk() && resource.disk().has_volume()) {
+ if (Resources::isPersistentVolume(resource) || resource.has_disk()) {
continue;
}
@@ -111,21 +134,32 @@ static Option<Bytes> getDiskResource(
Try<Isolator*> XfsDiskIsolatorProcess::create(const Flags& flags)
{
- if (!xfs::isPathXfs(flags.work_dir)) {
- return Error("'" + flags.work_dir + "' is not an XFS filesystem");
+ Try<Nothing> supported = isPathSupported(flags.work_dir);
+ if (supported.isError()) {
+ return Error(supported.error());
}
- Try<bool> enabled = xfs::isQuotaEnabled(flags.work_dir);
- if (enabled.isError()) {
- return Error(
- "Failed to get quota status for '" +
- flags.work_dir + "': " + enabled.error());
- }
+ // In general, we can't be sure of the filesystem configuration of
+ // PATH disks. We verify prior to use so that we don't later have to
+ // emit a confusing error (like "inappropriate IOCTL") when attempting
+ // to sample the project ID.
+ vector<Resource> resources = CHECK_NOTERROR(Resources::fromString(
+ flags.resources.getOrElse(""), flags.default_role));
- if (!enabled.get()) {
- return Error(
- "XFS project quotas are not enabled on '" +
- flags.work_dir + "'");
+ foreach (const Resource& resource, resources) {
+ if (resource.name() != "disk" || !resource.has_disk()) {
+ continue;
+ }
+
+ const Resource::DiskInfo& diskInfo = resource.disk();
+
+ if (diskInfo.has_source() &&
+ diskInfo.source().type() == Resource::DiskInfo::Source::PATH) {
+ supported = isPathSupported(diskInfo.source().path().root());
+ if (supported.isError()) {
+ return Error(supported.error());
+ }
+ }
}
Result<uid_t> uid = os::getuid();
@@ -199,7 +233,8 @@ XfsDiskIsolatorProcess::XfsDiskIsolatorProcess(
// At the beginning, the free project range is the same as the
// configured project range.
- LOG(INFO) << "Allocating XFS project IDs from the range " << totalProjectIds;
+ LOG(INFO) << "Allocating " << totalProjectIds.size()
+ << " XFS project IDs from the range " << totalProjectIds;
metrics.project_ids_total = totalProjectIds.size();
metrics.project_ids_free = totalProjectIds.size();
@@ -282,6 +317,53 @@ Future<Nothing> XfsDiskIsolatorProcess::recover(
}
}
+ Try<list<string>> volumes = paths::getPersistentVolumePaths(workDir);
+
+ if (volumes.isError()) {
+ return Failure(
+ "Failed to scan persistent volume directories: " + volumes.error());
+ }
+
+ // Track any project IDs that we have assigned to persistent volumes. Note
+ // that is is possible for operators to delete persistent volumes while
+ // the agent isn't running. If that happened, the quota record would be
+ // stale, but eventually the project ID would be re-used and the quota
+ // updated correctly.
+ foreach (const string& directory, volumes.get()) {
+ Result<prid_t> projectId = xfs::getProjectId(directory);
+ if (projectId.isError()) {
+ return Failure(projectId.error());
+ }
+
+ if (projectId.isNone()) {
+ continue;
+ }
+
+ // We should never have duplicate projects IDs assigned
+ // to persistent volumes, and since we haven't checked the
+ // recovered container resources yet, we can't already know
+ // about this persistent volume.
+ CHECK(!scheduledProjects.contains(projectId.get()))
+ << "Duplicate project ID " << projectId.get()
+ << " assigned to '" << directory << "'"
+ << " and '" << scheduledProjects.at(projectId.get()).second << "'";
+
+ freeProjectIds -= projectId.get();
+ if (totalProjectIds.contains(projectId.get())) {
+ --metrics.project_ids_free;
+ }
+
+ Try<string> devname = xfs::getDeviceForPath(directory);
+ if (devname.isError()) {
+ LOG(ERROR) << "Unable to schedule project ID " << projectId.get()
+ << " for reclaimation: " << devname.error();
+ continue;
+ }
+
+ scheduledProjects.put(
+ projectId.get(), make_pair(devname.get(), directory));
+ }
+
return Nothing();
}
@@ -344,41 +426,27 @@ Future<ContainerLimitation> XfsDiskIsolatorProcess::watch(
}
-Future<Nothing> XfsDiskIsolatorProcess::update(
- const ContainerID& containerId,
- const Resources& resources)
+static Try<xfs::QuotaInfo> applyProjectQuota(
+ const string& path,
+ prid_t projectId,
+ Bytes limit,
+ xfs::QuotaPolicy quotaPolicy)
{
- if (!infos.contains(containerId)) {
- LOG(INFO) << "Ignoring update for unknown container " << containerId;
- return Nothing();
- }
-
- const Owned<Info>& info = infos[containerId];
- Option<Bytes> needed = getDiskResource(resources);
-
- if (needed.isNone()) {
- // TODO(jpeach) If there's no disk resource attached, we should set the
- // minimum quota (1 block), since a zero quota would be unconstrained.
- LOG(WARNING) << "Ignoring quota update with no disk resources";
- return Nothing();
- }
-
switch (quotaPolicy) {
case xfs::QuotaPolicy::ACCOUNTING: {
- Try<Nothing> status = xfs::clearProjectQuota(
- info->directory, info->projectId);
+ Try<Nothing> status = xfs::clearProjectQuota(path, projectId);
if (status.isError()) {
- return Failure("Failed to clear quota for project " +
- stringify(info->projectId) + ": " + status.error());
+ return Error("Failed to clear quota for project " +
+ stringify(projectId) + ": " + status.error());
}
- break;
+ return xfs::QuotaInfo();
}
case xfs::QuotaPolicy::ENFORCING_ACTIVE:
case xfs::QuotaPolicy::ENFORCING_PASSIVE: {
- Bytes hardLimit = needed.get();
+ Bytes hardLimit = limit;
// The purpose behind adding to the hard limit is so that the soft
// limit can be exceeded thereby allowing us to check if the limit
@@ -389,56 +457,183 @@ Future<Nothing> XfsDiskIsolatorProcess::update(
}
Try<Nothing> status = xfs::setProjectQuota(
- info->directory, info->projectId, needed.get(), hardLimit);
+ path, projectId, limit, hardLimit);
if (status.isError()) {
- return Failure("Failed to update quota for project " +
- stringify(info->projectId) + ": " + status.error());
+ return Error("Failed to update quota for project " +
+ stringify(projectId) + ": " + status.error());
}
- LOG(INFO) << "Set quota on container " << containerId
- << " for project " << info->projectId
- << " to " << needed.get() << "/" << hardLimit;
-
- break;
+ return xfs::QuotaInfo{limit, hardLimit, 0};
}
}
- info->quota = needed.get();
- return Nothing();
+ UNREACHABLE();
}
-void XfsDiskIsolatorProcess::check()
+Future<Nothing> XfsDiskIsolatorProcess::update(
+ const ContainerID& containerId,
+ const Resources& resources)
{
- CHECK(quotaPolicy == xfs::QuotaPolicy::ENFORCING_ACTIVE);
+ if (!infos.contains(containerId)) {
+ LOG(INFO) << "Ignoring update for unknown container " << containerId;
+ return Nothing();
+ }
- foreachpair(const ContainerID& containerId, const Owned<Info>& info, infos) {
- Result<xfs::QuotaInfo> quotaInfo = xfs::getProjectQuota(
- info->directory, info->projectId);
+ const Owned<Info>& info = infos[containerId];
- if (quotaInfo.isError()) {
- LOG(WARNING) << "Failed to check disk usage for container '"
- << containerId << "': " << quotaInfo.error();
+ // First, apply the disk quota to the sandbox.
+ Option<Bytes> sandboxQuota = getSandboxDisk(resources);
+ if (sandboxQuota.isSome()) {
+ foreachpair (
+ const string& directory, Info::PathInfo& pathInfo, info->paths) {
+ if (pathInfo.disk.isNone()) {
+ pathInfo.quota = sandboxQuota.get();
+
+ Try<xfs::QuotaInfo> status = applyProjectQuota(
+ directory, pathInfo.projectId, sandboxQuota.get(), quotaPolicy);
+ if (status.isError()) {
+ return Failure(status.error());
+ }
+
+ LOG(INFO) << "Set quota on container " << containerId
+ << " for project " << pathInfo.projectId
+ << " to " << status->softLimit << "/" << status->hardLimit;
+ break;
+ }
+ }
+ }
+
+ // Make sure that we have project IDs assigned to all persistent volumes.
+ foreach (const Resource& resource, resources.persistentVolumes()) {
+ CHECK(resource.disk().has_volume());
+
+ const Bytes size = Megabytes(resource.scalar().value());
+ const string directory = paths::getPersistentVolumePath(workDir, resource);
+ // Don't apply project quotas to mount disks, since they are never
+ // subdivided and we can't guarantee that they are XFS filesystems. We
+ // still track the path for the container so that we can generate disk
+ // statistics correctly.
+ if (isMountDisk(resource.disk())) {
+ info->paths.put(directory, Info::PathInfo{size, 0, resource.disk()});
continue;
}
- // If the soft limit is exceeded the container should be killed.
- if (quotaInfo->used > quotaInfo->softLimit) {
- Resource resource;
- resource.set_name("disk");
- resource.set_type(Value::SCALAR);
- resource.mutable_scalar()->set_value(
- quotaInfo->used.bytes() / Bytes::MEGABYTES);
+ Result<prid_t> projectId = xfs::getProjectId(directory);
+ if (projectId.isError()) {
+ return Failure(projectId.error());
+ }
+
+ // If this volume already has a project ID, we must have assigned it
+ // here, or assigned or recovered it during project recovery.
+ if (projectId.isSome()) {
+ CHECK(scheduledProjects.contains(projectId.get()))
+ << "untracked project ID " << projectId.get()
+ << " for volume ID " << resource.disk().persistence().id()
+ << " on " << directory;
+ }
+
+ if (projectId.isNone()) {
+ Try<Nothing> supported = isPathSupported(directory);
+ if (supported.isError()) {
+ return Failure(supported.error());
+ }
+
+ projectId = nextProjectId();
+
+ 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<xfs::QuotaInfo> status =
+ applyProjectQuota(directory, projectId.get(), size, quotaPolicy);
+ if (status.isError()) {
+ return Failure(status.error());
+ }
+
+ info->paths.put(
+ directory, Info::PathInfo{size, projectId.get(), resource.disk()});
+
+ LOG(INFO) << "Set quota on volume " << resource.disk().persistence().id()
+ << " 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));
+ }
+ }
+
+ return Nothing();
+}
+
+
+void XfsDiskIsolatorProcess::check()
+{
+ CHECK(quotaPolicy == xfs::QuotaPolicy::ENFORCING_ACTIVE);
+
+ foreachpair(const ContainerID& containerId, const Owned<Info>& info, infos) {
+ foreachpair(
+ const string& directory, const Info::PathInfo& pathInfo, info->paths) {
+ Result<xfs::QuotaInfo> quotaInfo = xfs::getProjectQuota(
+ directory, pathInfo.projectId);
+
+ if (quotaInfo.isError()) {
+ LOG(WARNING)
+ << "Failed to check disk usage for container '"
+ << containerId << "' in '" << directory << "': "
+ << quotaInfo.error();
+
+ continue;
+ }
- info->limitation.set(
- protobuf::slave::createContainerLimitation(
- Resources(resource),
- "Disk usage (" + stringify(quotaInfo->used) +
- ") exceeds quota (" +
- stringify(quotaInfo->softLimit) + ")",
- TaskStatus::REASON_CONTAINER_LIMITATION_DISK));
+ // If the soft limit is exceeded the container should be killed.
+ if (quotaInfo->used > quotaInfo->softLimit) {
+ Resource resource;
+ resource.set_name("disk");
+ resource.set_type(Value::SCALAR);
+ resource.mutable_scalar()->set_value(
+ quotaInfo->used.bytes() / Bytes::MEGABYTES);
+
+ string volumeInfo;
+
+ if (pathInfo.disk.isSome()) {
+ resource.mutable_disk()->CopyFrom(pathInfo.disk.get());
+ volumeInfo =
+ " for volume '" + pathInfo.disk->persistence().id() + "'";
+ }
+
+ LOG(INFO)
+ << "Container " << stringify(containerId)
+ << " disk usage " << stringify(quotaInfo->used)
+ << " exceeded quota " << stringify(quotaInfo->softLimit)
+ << volumeInfo
+ << " in '" << directory << "'";
+
+ info->limitation.set(
+ protobuf::slave::createContainerLimitation(
+ Resources(resource),
+ "Disk usage (" + stringify(quotaInfo->used) +
+ ") exceeded quota (" + stringify(quotaInfo->softLimit) + ")" +
+ volumeInfo,
+ TaskStatus::REASON_CONTAINER_LIMITATION_DISK));
+ }
}
}
}
@@ -452,24 +647,55 @@ Future<ResourceStatistics> XfsDiskIsolatorProcess::usage(
return ResourceStatistics();
}
- ResourceStatistics statistics;
const Owned<Info>& info = infos[containerId];
+ ResourceStatistics statistics;
- Result<xfs::QuotaInfo> quota = xfs::getProjectQuota(
- info->directory, info->projectId);
+ foreachpair(
+ const string& directory, const Info::PathInfo& pathInfo, info->paths) {
+ DiskStatistics* disk = nullptr;
- if (quota.isError()) {
- return Failure(quota.error());
- }
+ if (pathInfo.disk.isSome()) {
+ disk = statistics.add_disk_statistics();
+ disk->mutable_persistence()->CopyFrom(pathInfo.disk->persistence());
+ disk->mutable_source()->CopyFrom(pathInfo.disk->source());
+ }
- // If we didn't set the quota (ie. we are in ACCOUNTING mode),
- // the quota limit will be 0. Since we are already tracking
- // what the quota ought to be in the Info, we just always
- // use that.
- statistics.set_disk_limit_bytes(info->quota.bytes());
+ // We don't require XFS on MOUNT disks, but we still want the isolator
+ // to publish usage for them to make them visible to operational tooling.
+ if (disk && isMountDisk(pathInfo.disk.get())) {
+ Try<Bytes> used = fs::used(directory);
+ if (used.isError()) {
+ return Failure("Failed to query disk usage for '" + directory +
+ "': " + used.error());
+ }
+
+ disk->set_used_bytes(used->bytes());
+ continue;
+ }
+
+ Result<xfs::QuotaInfo> quotaInfo =
+ xfs::getProjectQuota(directory, pathInfo.projectId);
+ if (quotaInfo.isError()) {
+ return Failure(quotaInfo.error());
+ }
+
+ if (quotaInfo.isSome()) {
+ if (disk) {
+ disk->set_used_bytes(quotaInfo->used.bytes());
+ } else {
+ statistics.set_disk_used_bytes(quotaInfo->used.bytes());
+ }
+ }
- if (quota.isSome()) {
- statistics.set_disk_used_bytes(quota->used.bytes());
+ // If we didn't set the quota (ie. we are in ACCOUNTING mode),
+ // the quota limit will be 0. Since we are already tracking
+ // what the quota ought to be in the Info, we just always
+ // use that.
+ if (disk) {
+ disk->set_limit_bytes(pathInfo.quota.bytes());
+ } else {
+ statistics.set_disk_limit_bytes(pathInfo.quota.bytes());
+ }
}
return statistics;
@@ -486,14 +712,9 @@ Future<Nothing> XfsDiskIsolatorProcess::cleanup(const ContainerID& containerId)
return Nothing();
}
- // Take a copy of the Info we are removing so that we can use it
- // to construct the Failure message if necessary.
- const std::string directory = infos[containerId]->directory;
- const prid_t projectId = infos[containerId]->projectId;
-
- infos.erase(containerId);
+ const Owned<Info>& info = infos[containerId];
- // Schedule the directory for project ID reclaiming.
+ // Schedule the directory for project ID reclaimation.
//
// We don't reclaim project ID here but wait until sandbox GC time.
// This is because the sandbox can potentially contain symlinks,
@@ -501,24 +722,30 @@ Future<Nothing> XfsDiskIsolatorProcess::cleanup(const ContainerID& containerId)
// limitations. Such symlinks would then contribute to disk usage
// of another container if the project ID was reused causing small
// inaccuracies in accounting.
- scheduledProjects.put(projectId, directory);
-
- LOG(INFO) << "Removing quota from project " << projectId
- << " for '" << directory << "'";
+ //
+ // Fortunately, this behaviour also suffices to reclaim project IDs from
+ // persistent volumes. We can just leave the project quota in place until
+ // 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<Nothing> quotaStatus = xfs::clearProjectQuota(
- directory, projectId);
+ Try<string> devname = xfs::getDeviceForPath(directory);
+ if (devname.isError()) {
+ LOG(ERROR) << "Unable to schedule project " << pathInfo.projectId
+ << " for reclaimation: " << devname.error();
+ continue;
+ }
- // Note that if we failed to clear the quota, we will still eventually
- // reclaim the project ID. If there is a persistent error will the quota
- // system, then we would ultimately fail to re-use that project ID since
- // the quota update would fail.
- if (quotaStatus.isError()) {
- LOG(ERROR) << "Failed to clear quota for '"
- << directory << "': " << quotaStatus.error();
- return Failure("Failed to cleanup '" + directory + "'");
+ scheduledProjects.put(
+ pathInfo.projectId, make_pair(devname.get(), directory));
}
+ infos.erase(containerId);
return Nothing();
}
@@ -552,14 +779,28 @@ void XfsDiskIsolatorProcess::returnProjectId(
void XfsDiskIsolatorProcess::reclaimProjectIds()
{
+ // Note that we need both the directory we assigned the project ID to,
+ // and the device node for the block device hosting the directory. Since
+ // we can only reclaim the project ID if the former doesn't exist, we
+ // need the latter to make the corresponding quota record updates.
+
foreachpair (
- prid_t projectId, const string& dir, utils::copy(scheduledProjects)) {
- if (!os::exists(dir)) {
- returnProjectId(projectId);
- scheduledProjects.erase(projectId);
- LOG(INFO) << "Reclaimed project ID " << projectId
- << " from '" << dir << "'";
+ prid_t projectId, const auto& dir, utils::copy(scheduledProjects)) {
+ if (os::exists(dir.second)) {
+ continue;
}
+
+ Try<Nothing> status = xfs::clearProjectQuota(dir.first, projectId);
+ if (status.isError()) {
+ LOG(ERROR) << "Failed to clear quota for '"
+ << dir.second << "': " << status.error();
+ }
+
+ returnProjectId(projectId);
+ scheduledProjects.erase(projectId);
+
+ LOG(INFO) << "Reclaimed project ID " << projectId
+ << " from '" << dir.second << "'";
}
}
diff --git a/src/slave/containerizer/mesos/isolators/xfs/disk.hpp b/src/slave/containerizer/mesos/isolators/xfs/disk.hpp
index 38c467b..94d44e7 100644
--- a/src/slave/containerizer/mesos/isolators/xfs/disk.hpp
+++ b/src/slave/containerizer/mesos/isolators/xfs/disk.hpp
@@ -18,6 +18,7 @@
#define __XFS_DISK_ISOLATOR_HPP__
#include <string>
+#include <utility>
#include <process/owned.hpp>
@@ -74,6 +75,24 @@ protected:
void initialize() override;
private:
+ struct Info
+ {
+ struct PathInfo
+ {
+ Bytes quota;
+ const prid_t projectId;
+ const Option<Resource::DiskInfo> disk;
+ };
+
+ Info(const std::string& directory, prid_t projectId)
+ {
+ paths.put(directory, PathInfo{Bytes(0), projectId, None()});
+ }
+
+ hashmap<std::string, PathInfo> paths;
+ process::Promise<mesos::slave::ContainerLimitation> limitation;
+ };
+
XfsDiskIsolatorProcess(
Duration watchInterval,
xfs::QuotaPolicy quotaPolicy,
@@ -94,17 +113,6 @@ private:
// that are not.
void reclaimProjectIds();
- struct Info
- {
- explicit Info(const std::string& _directory, prid_t _projectId)
- : directory(_directory), quota(0), projectId(_projectId) {}
-
- const std::string directory;
- Bytes quota;
- const prid_t projectId;
- process::Promise<mesos::slave::ContainerLimitation> limitation;
- };
-
const Duration watchInterval;
const Duration projectWatchInterval;
xfs::QuotaPolicy quotaPolicy;
@@ -112,7 +120,10 @@ private:
const IntervalSet<prid_t> totalProjectIds;
IntervalSet<prid_t> freeProjectIds;
hashmap<ContainerID, process::Owned<Info>> infos;
- hashmap<prid_t, std::string> scheduledProjects;
+
+ // Track the device and filesystem path of unused project IDs we want
+ // to reclaim.
+ hashmap<prid_t, std::pair<std::string, std::string>> scheduledProjects;
// Metrics used by the XFS disk isolator.
struct Metrics
diff --git a/src/tests/containerizer/xfs_quota_tests.cpp b/src/tests/containerizer/xfs_quota_tests.cpp
index 2b3a2e2..4043154 100644
--- a/src/tests/containerizer/xfs_quota_tests.cpp
+++ b/src/tests/containerizer/xfs_quota_tests.cpp
@@ -986,8 +986,8 @@ TEST_F(ROOT_XFS_QuotaTest, NoCheckpointRecovery)
// One sandbox and one symlink.
ASSERT_EQ(2u, sandboxes->size());
- // Scan the remaining sandboxes and check that project ID is still assigned
- // but quota is unset.
+ // Scan the remaining sandboxes and check that project ID is still
+ // assigned and that the quota is set.
foreach (const string& sandbox, sandboxes.get()) {
// Skip the "latest" symlink.
if (os::stat::islink(sandbox)) {
@@ -997,7 +997,7 @@ TEST_F(ROOT_XFS_QuotaTest, NoCheckpointRecovery)
Result<prid_t> projectId = xfs::getProjectId(sandbox);
ASSERT_SOME(projectId);
- EXPECT_NONE(xfs::getProjectQuota(sandbox, projectId.get()));
+ EXPECT_SOME(xfs::getProjectQuota(sandbox, projectId.get()));
}
driver.stop();
@@ -1315,18 +1315,21 @@ TEST_F(ROOT_XFS_QuotaTest, ProjectIdReclaiming)
ASSERT_SOME(sandboxes);
ASSERT_EQ(2u, sandboxes->size());
- // Scan the remaining sandboxes and check that project ID is still assigned
- // but quota is unset.
+ // Scan the remaining sandboxes and check that project ID is still
+ // assigned and the quota is set.
Option<prid_t> usedProjectId;
+
foreach (const string& sandbox, sandboxes.get()) {
if (!os::stat::islink(sandbox)) {
Result<prid_t> projectId = xfs::getProjectId(sandbox);
ASSERT_SOME(projectId);
+
usedProjectId = projectId.get();
- EXPECT_NONE(xfs::getProjectQuota(sandbox, projectId.get()));
+ EXPECT_SOME(xfs::getProjectQuota(sandbox, projectId.get()));
}
}
+
ASSERT_SOME(usedProjectId);
// Advance the clock to trigger sandbox GC and project ID usage check.
@@ -1342,6 +1345,13 @@ TEST_F(ROOT_XFS_QuotaTest, ProjectIdReclaiming)
ASSERT_SOME(sandboxes);
ASSERT_TRUE(sandboxes->empty());
+ // After the sandbox is removed, we should have reclaimed the project ID,
+ // removed all files for the project ID and cleared the quota record.
+ // This means that we ought to receive ENOENT when looking up the quota
+ // record.
+ EXPECT_ERROR(
+ xfs::getProjectQuota(mountPoint.get(), usedProjectId.get()));
+
AWAIT_READY(offers2);
ASSERT_FALSE(offers2->empty());
offer = offers2->at(0);
@@ -1430,6 +1440,46 @@ TEST_F(ROOT_XFS_QuotaTest, IsolatorFlags)
flags = CreateSlaveFlags();
flags.xfs_project_range = "100";
ASSERT_ERROR(StartSlave(detector.get(), flags));
+
+ // PATH disks that are not on XFS volumes should be rejected.
+ flags = CreateSlaveFlags();
+ flags.resources = R"(
+ [
+ {
+ "name": "cpus",
+ "type": "SCALAR",
+ "scalar": {
+ "value": 2
+ }
+ },
+ {
+ "name": "mem",
+ "type": "SCALAR",
+ "scalar": {
+ "value": 1024
+ }
+ },
+ {
+ "name": "disk",
+ "type": "SCALAR",
+ "scalar": {
+ "value": 1024
+ }
+ },
+ {
+ "name": "disk",
+ "type": "SCALAR",
+ "scalar": { "value": 1024 },
+ "disk": {
+ "source": {
+ "type": "PATH",
+ "path": { "root": "/sys" }
+ }
+ }
+ }
+ ]
+ )";
+ ASSERT_ERROR(StartSlave(detector.get(), flags));
}