You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/12/14 02:03:13 UTC
[mesos] branch master updated (3ade731 -> b6fc970)
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 3ade731 Fixed SIGINT handling in parallel test runner.
new 65ec2b0 Added an optional `vendor` field to `Resource.DiskInfo.Source`.
new 48211c1 Set up the `Resource.DiskInfo.Source.vendor` field in SLRP.
new fd5b28b Fixed `AgentResourceProviderConfigApiTest.Update` for `vendor` field.
new a3a8b5f Added MESOS-9321 to the 1.7.1 CHANGELOG.
new 7c852a9 Tested the `Resource.DiskInfo.Source.vendor` field in related tests.
new b6fc970 Added the missing MESOS-7947 to the 1.7.0 CHANGELOG.
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:
CHANGELOG | 7 +++
include/mesos/mesos.proto | 28 ++++++++----
include/mesos/v1/mesos.proto | 28 ++++++++----
src/common/resources.cpp | 36 +++++++--------
src/resource_provider/storage/provider.cpp | 33 ++++++++++----
.../agent_resource_provider_config_api_tests.cpp | 49 +++++++++++++++-----
src/tests/resources_tests.cpp | 32 ++++++++-----
.../storage_local_resource_provider_tests.cpp | 53 +++++++++++++++++++---
src/v1/resources.cpp | 36 +++++++--------
9 files changed, 213 insertions(+), 89 deletions(-)
[mesos] 05/06: Tested the `Resource.DiskInfo.Source.vendor` field
in related tests.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 7c852a984c61f8ccd60eaf45749603f8f08e3043
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Thu Dec 6 15:26:34 2018 -0800
Tested the `Resource.DiskInfo.Source.vendor` field in related tests.
Review: https://reviews.apache.org/r/69522
---
src/tests/resources_tests.cpp | 32 ++++++++-----
.../storage_local_resource_provider_tests.cpp | 53 +++++++++++++++++++---
2 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index ab42374..5337a5c 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -19,6 +19,7 @@
#include <set>
#include <sstream>
#include <string>
+#include <tuple>
#include <gtest/gtest.h>
@@ -2370,10 +2371,11 @@ TEST(DiskResourcesTest, DiskSourceEquals)
class DiskResourcesSourceTest
: public ::testing::Test,
- public ::testing::WithParamInterface<std::tr1::tuple<
+ public ::testing::WithParamInterface<std::tuple<
Resource::DiskInfo::Source::Type,
- bool,
- bool>> {};
+ bool, // Whether the disk has the `vendor` field set.
+ bool, // Whether the disk has the `id` field set.
+ bool>> {}; // Whether the disk has the `profile` field set.
INSTANTIATE_TEST_CASE_P(
@@ -2386,11 +2388,14 @@ INSTANTIATE_TEST_CASE_P(
Resource::DiskInfo::Source::PATH,
Resource::DiskInfo::Source::BLOCK,
Resource::DiskInfo::Source::MOUNT),
- // We test the case where the source has identity (i.e., has
- // an `id` set) and where not.
+ // We test the cases where the source has a vendor (i.e., has the
+ // `vendor` field set) and where not.
::testing::Bool(),
- // We test the case where the source has profile (i.e., has
- // an `profile` set) and where not.
+ // We test the cases where the source has an identity (i.e., has the
+ // `id` field set) and where not.
+ ::testing::Bool(),
+ // We test the cases where the source has a profile (i.e., has the
+ // `profile` field set) and where not.
::testing::Bool()));
@@ -2398,14 +2403,19 @@ TEST_P(DiskResourcesSourceTest, SourceIdentity)
{
auto parameters = GetParam();
- Resource::DiskInfo::Source::Type type = std::tr1::get<0>(parameters);
- bool hasIdentity = std::tr1::get<1>(parameters);
- bool hasProfile = std::tr1::get<2>(parameters);
+ Resource::DiskInfo::Source::Type type = std::get<0>(parameters);
+ bool hasVendor = std::get<1>(parameters);
+ bool hasIdentity = std::get<2>(parameters);
+ bool hasProfile = std::get<3>(parameters);
- // Create a disk, possibly with an id to signify identiy.
+ // Create a disk, possibly with an id to signify identity.
Resource::DiskInfo::Source source;
source.set_type(type);
+ if (hasVendor) {
+ source.set_vendor("vendor");
+ }
+
if (hasIdentity) {
source.set_id("id");
}
diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp
index 422a152..7887bd5 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -81,6 +81,9 @@ constexpr char URI_DISK_PROFILE_ADAPTOR_NAME[] =
constexpr char TEST_SLRP_TYPE[] = "org.apache.mesos.rp.local.storage";
constexpr char TEST_SLRP_NAME[] = "test";
+constexpr char TEST_CSI_PLUGIN_TYPE[] = "org.apache.mesos.csi.test";
+constexpr char TEST_CSI_PLUGIN_NAME[] = "local";
+constexpr char TEST_CSI_VENDOR[] = "org.apache.mesos.csi.test.local";
class StorageLocalResourceProviderTest
@@ -196,13 +199,10 @@ public:
const Option<string> volumes = None(),
const Option<string> createParameters = None())
{
- const string testCsiPluginName = "test_csi_plugin";
-
const string testCsiPluginPath =
path::join(tests::flags.build_dir, "src", "test-csi-plugin");
- const string testCsiPluginWorkDir =
- path::join(sandbox.get(), testCsiPluginName);
+ const string testCsiPluginWorkDir = path::join(sandbox.get(), "storage");
ASSERT_SOME(os::mkdir(testCsiPluginWorkDir));
Try<string> resourceProviderConfig = strings::format(
@@ -218,7 +218,7 @@ public:
],
"storage": {
"plugin": {
- "type": "org.apache.mesos.csi.test",
+ "type": "%s",
"name": "%s",
"containers": [
{
@@ -261,7 +261,8 @@ public:
)~",
TEST_SLRP_TYPE,
TEST_SLRP_NAME,
- testCsiPluginName,
+ TEST_CSI_PLUGIN_TYPE,
+ TEST_CSI_PLUGIN_NAME,
testCsiPluginPath,
testCsiPluginPath,
stringify(capacity),
@@ -516,6 +517,8 @@ TEST_F(StorageLocalResourceProviderTest, DISABLED_SmallDisk)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
!r.disk().source().has_id() &&
r.disk().source().has_profile();
};
@@ -523,6 +526,8 @@ TEST_F(StorageLocalResourceProviderTest, DISABLED_SmallDisk)
auto isPreExistingVolume = [](const Resource& r) {
return r.has_disk() &&
r.disk().has_source() &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
r.disk().source().has_id() &&
!r.disk().source().has_profile();
};
@@ -677,6 +682,8 @@ TEST_F(StorageLocalResourceProviderTest, ProfileAppeared)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
!r.disk().source().has_id() &&
r.disk().source().has_profile() &&
r.disk().source().profile() == profile;
@@ -812,6 +819,8 @@ TEST_F(StorageLocalResourceProviderTest, CreateDestroyDisk)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -855,6 +864,8 @@ TEST_F(StorageLocalResourceProviderTest, CreateDestroyDisk)
}
ASSERT_SOME(destroyed);
+ ASSERT_TRUE(destroyed->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, destroyed->disk().source().vendor());
ASSERT_FALSE(destroyed->disk().source().has_id());
ASSERT_FALSE(destroyed->disk().source().has_metadata());
ASSERT_FALSE(destroyed->disk().source().has_mount());
@@ -992,6 +1003,8 @@ TEST_F(StorageLocalResourceProviderTest, CreateDestroyDiskRecovery)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -1062,6 +1075,8 @@ TEST_F(StorageLocalResourceProviderTest, CreateDestroyDiskRecovery)
}
ASSERT_SOME(destroyed);
+ ASSERT_TRUE(destroyed->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, destroyed->disk().source().vendor());
ASSERT_FALSE(destroyed->disk().source().has_id());
ASSERT_FALSE(destroyed->disk().source().has_metadata());
ASSERT_FALSE(destroyed->disk().source().has_mount());
@@ -1504,6 +1519,8 @@ TEST_F(StorageLocalResourceProviderTest, AgentRegisteredWithNewId)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
!r.disk().source().has_id() &&
r.disk().source().has_profile() &&
r.disk().source().profile() == profile;
@@ -1526,6 +1543,8 @@ TEST_F(StorageLocalResourceProviderTest, AgentRegisteredWithNewId)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::MOUNT &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
r.disk().source().has_id() &&
r.disk().source().has_profile() &&
r.disk().source().profile() == profile;
@@ -1554,6 +1573,8 @@ TEST_F(StorageLocalResourceProviderTest, AgentRegisteredWithNewId)
.begin();
ASSERT_TRUE(created.has_provider_id());
+ ASSERT_TRUE(created.disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, created.disk().source().vendor());
ASSERT_TRUE(created.disk().source().has_id());
ASSERT_TRUE(created.disk().source().has_metadata());
ASSERT_TRUE(created.disk().source().has_mount());
@@ -1604,6 +1625,8 @@ TEST_F(StorageLocalResourceProviderTest, AgentRegisteredWithNewId)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
r.disk().source().has_id() &&
!r.disk().source().has_profile();
};
@@ -1813,6 +1836,8 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_PublishResources)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -2030,6 +2055,8 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_PublishResourcesRecovery)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -2299,6 +2326,8 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_PublishResourcesReboot)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -2633,6 +2662,8 @@ TEST_F(
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -2807,6 +2838,8 @@ TEST_F(StorageLocalResourceProviderTest, ImportPreprovisionedVolume)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
!r.disk().source().has_id() &&
r.disk().source().has_profile() &&
r.disk().source().profile() == profile;
@@ -2831,6 +2864,8 @@ TEST_F(StorageLocalResourceProviderTest, ImportPreprovisionedVolume)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
r.disk().source().has_id() &&
!r.disk().source().has_profile();
};
@@ -2860,6 +2895,8 @@ TEST_F(StorageLocalResourceProviderTest, ImportPreprovisionedVolume)
return r.has_disk() &&
r.disk().has_source() &&
r.disk().source().type() == Resource::DiskInfo::Source::MOUNT &&
+ r.disk().source().has_vendor() &&
+ r.disk().source().vendor() == TEST_CSI_VENDOR &&
r.disk().source().has_id() &&
r.disk().source().has_profile() &&
r.disk().source().profile() == profile;
@@ -3466,6 +3503,8 @@ TEST_F(StorageLocalResourceProviderTest, OperationStateMetrics)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
@@ -3742,6 +3781,8 @@ TEST_F(StorageLocalResourceProviderTest, CsiPluginRpcMetrics)
}
ASSERT_SOME(volume);
+ ASSERT_TRUE(volume->disk().source().has_vendor());
+ EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
ASSERT_TRUE(volume->disk().source().has_id());
ASSERT_TRUE(volume->disk().source().has_metadata());
ASSERT_TRUE(volume->disk().source().has_mount());
[mesos] 03/06: Fixed `AgentResourceProviderConfigApiTest.Update`
for `vendor` field.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit fd5b28b5176efb03d6b68921f2c264e3c0066ad2
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Thu Dec 6 20:20:46 2018 -0800
Fixed `AgentResourceProviderConfigApiTest.Update` for `vendor` field.
The introduction of the `Resource.DiskInfo.Source.vendor` changes the
resource comparison and thus breaks this test. This patch fixes the test
and make it more robust.
NOTE: The updated test will fail unless the previous patch (which sets
up the `vendor` field) is also applied.
Review: https://reviews.apache.org/r/69521
---
.../agent_resource_provider_config_api_tests.cpp | 49 ++++++++++++++++------
1 file changed, 37 insertions(+), 12 deletions(-)
diff --git a/src/tests/agent_resource_provider_config_api_tests.cpp b/src/tests/agent_resource_provider_config_api_tests.cpp
index 15c83b3..7185ac0 100644
--- a/src/tests/agent_resource_provider_config_api_tests.cpp
+++ b/src/tests/agent_resource_provider_config_api_tests.cpp
@@ -151,17 +151,18 @@ public:
// This extra closure is necessary in order to use `ASSERT_*`, as
// these macros require a void return type.
[&] {
- // Randomize the plugin name so we get a clean work directory for
- // each created config.
+ // Randomize the plugin name so every created config uses its own CSI
+ // plugin instance. We use this to check if the config has been updated in
+ // some tests.
const string testCsiPluginName =
- "test_csi_plugin_" +
- strings::remove(id::UUID::random().toString(), "-");
+ "local_" + strings::remove(id::UUID::random().toString(), "-");
const string testCsiPluginPath =
path::join(tests::flags.build_dir, "src", "test-csi-plugin");
const string testCsiPluginWorkDir =
path::join(sandbox.get(), testCsiPluginName);
+
ASSERT_SOME(os::mkdir(testCsiPluginWorkDir));
Try<string> resourceProviderConfig = strings::format(
@@ -579,11 +580,13 @@ TEST_P(AgentResourceProviderConfigApiTest, Update)
Owned<MasterDetector> detector = master.get()->createDetector();
// Generate a pre-existing config.
+ const string volumes = "volume1:4GB";
const string configPath =
path::join(resourceProviderConfigDir.get(), "test.json");
+
ASSERT_SOME(os::write(
configPath,
- stringify(JSON::protobuf(createResourceProviderInfo("volume1:4GB")))));
+ stringify(JSON::protobuf(createResourceProviderInfo(volumes)))));
Future<SlaveRegisteredMessage> slaveRegisteredMessage =
FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
@@ -616,14 +619,22 @@ TEST_P(AgentResourceProviderConfigApiTest, Update)
Future<vector<Offer>> oldOffers;
EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
- std::bind(&Resources::hasResourceProvider, lambda::_1))))
+ &Resources::hasResourceProvider)))
.WillOnce(FutureArg<1>(&oldOffers));
driver.start();
// Wait for an offer having the old provider resource.
AWAIT_READY(oldOffers);
- ASSERT_FALSE(oldOffers->empty());
+ ASSERT_EQ(1u, oldOffers->size());
+
+ Resource oldResource = *Resources(oldOffers->at(0).resources())
+ .filter(&Resources::hasResourceProvider)
+ .begin();
+
+ ASSERT_TRUE(oldResource.has_disk());
+ ASSERT_TRUE(oldResource.disk().has_source());
+ ASSERT_TRUE(oldResource.disk().source().has_vendor());
Future<OfferID> rescinded;
@@ -636,7 +647,8 @@ TEST_P(AgentResourceProviderConfigApiTest, Update)
std::bind(&Resources::hasResourceProvider, lambda::_1))))
.WillOnce(FutureArg<1>(&newOffers));
- ResourceProviderInfo info = createResourceProviderInfo("volume1:2GB");
+ // Create a new config that serves the same volumes with a different vendor.
+ ResourceProviderInfo info = createResourceProviderInfo(volumes);
AWAIT_EXPECT_RESPONSE_STATUS_EQ(
http::OK().status,
@@ -655,6 +667,7 @@ TEST_P(AgentResourceProviderConfigApiTest, Update)
Try<ResourceProviderInfo> _info =
::protobuf::parse<ResourceProviderInfo>(json.get());
+
ASSERT_SOME(_info);
EXPECT_EQ(_info.get(), info);
@@ -663,10 +676,22 @@ TEST_P(AgentResourceProviderConfigApiTest, Update)
// Wait for an offer having the new provider resource.
AWAIT_READY(newOffers);
-
- // The new provider resource is smaller than the old provider resource.
- EXPECT_FALSE(Resources(newOffers->at(0).resources()).contains(
- oldOffers->at(0).resources()));
+ ASSERT_EQ(1u, newOffers->size());
+
+ Resource newResource = *Resources(newOffers->at(0).resources())
+ .filter(&Resources::hasResourceProvider)
+ .begin();
+
+ ASSERT_TRUE(newResource.has_disk());
+ ASSERT_TRUE(newResource.disk().has_source());
+ ASSERT_TRUE(newResource.disk().source().has_vendor());
+
+ // The resource from the new resource provider has the same provider ID but a
+ // different vendor as that from the old one.
+ EXPECT_EQ(oldResource.provider_id(), newResource.provider_id());
+ EXPECT_NE(
+ oldResource.disk().source().vendor(),
+ newResource.disk().source().vendor());
}
[mesos] 06/06: Added the missing MESOS-7947 to the 1.7.0 CHANGELOG.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit b6fc9706c03c93239128f4a27051997c9647d8ff
Author: Joseph Wu <jo...@apache.org>
AuthorDate: Wed Sep 5 16:52:40 2018 -0700
Added the missing MESOS-7947 to the 1.7.0 CHANGELOG.
---
CHANGELOG | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/CHANGELOG b/CHANGELOG
index 3a517c1..434b0df 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -81,6 +81,11 @@ This release contains the following highlights:
along with other cgroups related options
(e.g., `cgroups/cpu`), those options will be just ignored.
+ * [MESOS-7947] - Added a new `--gc_non_executor_container_sandboxes`
+ option which tells the agent to garbage collect sandboxes created
+ via the LAUNCH_NESTED_CONTAINER API. The same flag will apply to
+ standalone container sandboxes in future.
+
* [MESOS-8327] - Added container-specific cgroups mounts under
`/sys/fs/cgroup` to containers with image launched by Mesos
containerizer.
@@ -299,6 +304,7 @@ All Resolved Issues:
** Improvement
* [MESOS-6451] - Add timer and percentile for docker pull latency distribution.
* [MESOS-7691] - Support local enabled cgroups subsystems automatically.
+ * [MESOS-7947] - Add GC capability to nested containers
* [MESOS-8064] - Add capability so mesos can programmatically decode .zip, .tar, .gzip, and other common file compression schemes
* [MESOS-8106] - Docker fetcher plugin unsupported scheme failure message is not accurate.
* [MESOS-8340] - Add a no-enforce option to the `network/ports` isolator.
[mesos] 04/06: Added MESOS-9321 to the 1.7.1 CHANGELOG.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit a3a8b5f81963c5dd0344b91131defea53fd4f7c3
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Thu Dec 13 14:24:23 2018 -0800
Added MESOS-9321 to the 1.7.1 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index 85bc05a..3a517c1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -39,6 +39,7 @@ Release Notes - Mesos - Version 1.7.1 (WIP)
* [MESOS-9249] - Avoid dirtying the DRF sorter when allocating resources.
* [MESOS-9255] - Use consistent "totals" across role / framework DRF.
* [MESOS-9305] - Create cgoup recursively to workaround systemd deleting cgroups_root.
+ * [MESOS-9321] - Add an optional `vendor` field in `Resource.DiskInfo.Source`.
* [MESOS-9340] - Log all socket errors in libprocess.
[mesos] 02/06: Set up the `Resource.DiskInfo.Source.vendor` field
in SLRP.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 48211c1def9cbd8885764fee136a644e29bd7d79
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Thu Dec 6 15:28:16 2018 -0800
Set up the `Resource.DiskInfo.Source.vendor` field in SLRP.
For a CSI-backed disk resource, this field is set to the concatenation
of the type and name of its CSI plugin, with a dot in between. This
enables frameworks to identify the storage backend the disk resource.
Review: https://reviews.apache.org/r/69520
---
src/resource_provider/storage/provider.cpp | 33 ++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index 955471c..d6e20a5 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -246,32 +246,41 @@ static inline Resource createRawDiskResource(
const ResourceProviderInfo& info,
const Bytes& capacity,
const Option<string>& profile,
+ const Option<string>& vendor,
const Option<string>& id = None(),
const Option<Labels>& metadata = None())
{
CHECK(info.has_id());
+ CHECK(info.has_storage());
Resource resource;
resource.set_name("disk");
resource.set_type(Value::SCALAR);
resource.mutable_scalar()
- ->set_value((double) capacity.bytes() / Bytes::MEGABYTES);
+ ->set_value(static_cast<double>(capacity.bytes()) / Bytes::MEGABYTES);
+
resource.mutable_provider_id()->CopyFrom(info.id()),
resource.mutable_reservations()->CopyFrom(info.default_reservations());
- resource.mutable_disk()->mutable_source()
- ->set_type(Resource::DiskInfo::Source::RAW);
+
+ Resource::DiskInfo::Source* source =
+ resource.mutable_disk()->mutable_source();
+
+ source->set_type(Resource::DiskInfo::Source::RAW);
if (profile.isSome()) {
- resource.mutable_disk()->mutable_source()->set_profile(profile.get());
+ source->set_profile(profile.get());
+ }
+
+ if (vendor.isSome()) {
+ source->set_vendor(vendor.get());
}
if (id.isSome()) {
- resource.mutable_disk()->mutable_source()->set_id(id.get());
+ source->set_id(id.get());
}
if (metadata.isSome()) {
- resource.mutable_disk()->mutable_source()->mutable_metadata()
- ->CopyFrom(metadata.get());
+ source->mutable_metadata()->CopyFrom(metadata.get());
}
return resource;
@@ -296,6 +305,9 @@ public:
metaDir(slave::paths::getMetaRootDir(_workDir)),
contentType(ContentType::PROTOBUF),
info(_info),
+ vendor(
+ info.storage().plugin().type() + "." +
+ info.storage().plugin().name()),
slaveId(_slaveId),
authToken(_authToken),
strict(_strict),
@@ -455,6 +467,7 @@ private:
const string metaDir;
const ContentType contentType;
ResourceProviderInfo info;
+ const string vendor;
const SlaveID slaveId;
const Option<string> authToken;
const bool strict;
@@ -1378,6 +1391,8 @@ ResourceConversion StorageLocalResourceProviderProcess::reconcileResources(
Bytes(resource.scalar().value() * Bytes::MEGABYTES),
resource.disk().source().has_profile()
? resource.disk().source().profile() : Option<string>::none(),
+ resource.disk().source().has_vendor()
+ ? resource.disk().source().vendor() : Option<string>::none(),
resource.disk().source().has_id()
? resource.disk().source().id() : Option<string>::none(),
resource.disk().source().has_metadata()
@@ -2917,6 +2932,7 @@ Future<Resources> StorageLocalResourceProviderProcess::listVolumes()
volumesToProfiles.contains(entry.volume().id())
? volumesToProfiles.at(entry.volume().id())
: Option<string>::none(),
+ vendor,
entry.volume().id(),
entry.volume().attributes().empty()
? Option<Labels>::none()
@@ -2964,7 +2980,8 @@ Future<Resources> StorageLocalResourceProviderProcess::getCapacities()
return createRawDiskResource(
info,
Bytes(response.available_capacity()),
- profile);
+ profile,
+ vendor);
})));
}
[mesos] 01/06: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
Posted by ch...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 65ec2b0149970d2790d18f9f3493b247c698baae
Author: Chun-Hung Hsiao <ch...@apache.org>
AuthorDate: Thu Nov 15 23:06:22 2018 -0800
Added an optional `vendor` field to `Resource.DiskInfo.Source`.
The new `vendor` field together with the `id` field provide the means of
uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
there is a change in the agent ID).
Review: https://reviews.apache.org/r/69037/
---
include/mesos/mesos.proto | 28 ++++++++++++++++++++--------
include/mesos/v1/mesos.proto | 28 ++++++++++++++++++++--------
src/common/resources.cpp | 36 ++++++++++++++++++------------------
src/v1/resources.cpp | 36 ++++++++++++++++++------------------
4 files changed, 76 insertions(+), 52 deletions(-)
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index ef4d785..137df1f 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1026,7 +1026,7 @@ message CSIPluginContainerInfo {
* Describes a CSI plugin.
*/
message CSIPluginInfo {
- // The type of the CSI service. This uniquely identifies a CSI
+ // The type of the CSI plugin. This uniquely identifies a CSI
// implementation. For instance:
// org.apache.mesos.csi.test
//
@@ -1035,11 +1035,17 @@ message CSIPluginInfo {
// to avoid conflicts on type names.
required string type = 1;
- // The name of the CSI service. There could be mutliple instances of a
- // type of CSI service. The name field is used to distinguish these
- // instances. It should be a legal Java identifier
+ // The name of the CSI plugin. There could be multiple instances of a
+ // type of CSI plugin within a Mesos cluster. The name field is used to
+ // distinguish these instances. It should be a legal Java identifier
// (https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html)
// to avoid conflicts on concatenation of type and name.
+ //
+ // The type and name together provide the means to uniquely identify a storage
+ // backend and its resources in the cluster, so the operator should ensure
+ // that the concatenation of type and name is unique in the cluster, and it
+ // remains the same if the instance is migrated to another agent (e.g., there
+ // is a change in the agent ID).
required string name = 2;
// A list of container configurations to run CSI plugin components.
@@ -1452,12 +1458,18 @@ message Resource {
optional Path path = 2;
optional Mount mount = 3;
- // An identifier for this source. This field maps onto CSI
- // volume IDs and is not expected to be set by frameworks.
+ // The vendor of this source. If present, this field provides the means to
+ // uniquely identify the storage backend of this source in the cluster.
+ optional string vendor = 7; // EXPERIMENTAL.
+
+ // The identifier of this source. This field maps onto CSI volume IDs and
+ // is not expected to be set by frameworks. If both `vendor` and `id` are
+ // present, these two fields together provide the means to uniquely
+ // identify this source in the cluster.
optional string id = 4; // EXPERIMENTAL.
- // Additional metadata for this source. This field maps onto CSI
- // volume metadata and is not expected to be set by frameworks.
+ // Additional metadata for this source. This field maps onto CSI volume
+ // attributes and is not expected to be set by frameworks.
optional Labels metadata = 5; // EXPERIMENTAL.
// This field serves as an indirection to a set of storage
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index 4e8f7a1..f121709 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1018,7 +1018,7 @@ message CSIPluginContainerInfo {
* Describes a CSI plugin.
*/
message CSIPluginInfo {
- // The type of the CSI service. This uniquely identifies a CSI
+ // The type of the CSI plugin. This uniquely identifies a CSI
// implementation. For instance:
// org.apache.mesos.csi.test
//
@@ -1027,11 +1027,17 @@ message CSIPluginInfo {
// to avoid conflicts on type names.
required string type = 1;
- // The name of the CSI service. There could be mutliple instances of a
- // type of CSI service. The name field is used to distinguish these
- // instances. It should be a legal Java identifier
+ // The name of the CSI plugin. There could be multiple instances of a
+ // type of CSI plugin within a Mesos cluster. The name field is used to
+ // distinguish these instances. It should be a legal Java identifier
// (https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html)
// to avoid conflicts on concatenation of type and name.
+ //
+ // The type and name together provide the means to uniquely identify a storage
+ // backend and its resources in the cluster, so the operator should ensure
+ // that the concatenation of type and name is unique in the cluster, and it
+ // remains the same if the instance is migrated to another agent (e.g., there
+ // is a change in the agent ID).
required string name = 2;
// A list of container configurations to run CSI plugin components.
@@ -1444,12 +1450,18 @@ message Resource {
optional Path path = 2;
optional Mount mount = 3;
- // An identifier for this source. This field maps onto CSI
- // volume IDs and is not expected to be set by frameworks.
+ // The vendor of this source. If present, this field provides the means to
+ // uniquely identify the storage backend of this source in the cluster.
+ optional string vendor = 7; // EXPERIMENTAL.
+
+ // The identifier of this source. This field maps onto CSI volume IDs and
+ // is not expected to be set by frameworks. If both `vendor` and `id` are
+ // present, these two fields together provide the means to uniquely
+ // identify this source in the cluster.
optional string id = 4; // EXPERIMENTAL.
- // Additional metadata for this source. This field maps onto CSI
- // volume metadata and is not expected to be set by frameworks.
+ // Additional metadata for this source. This field maps onto CSI volume
+ // attributes and is not expected to be set by frameworks.
optional Labels metadata = 5; // EXPERIMENTAL.
// This field serves as an indirection to a set of storage
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 1585e30..758b5a2 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -192,6 +192,14 @@ bool operator==(
return false;
}
+ if (left.has_vendor() != right.has_vendor()) {
+ return false;
+ }
+
+ if (left.has_vendor() && left.vendor() != right.vendor()) {
+ return false;
+ }
+
if (left.has_id() != right.has_id()) {
return false;
}
@@ -2381,29 +2389,21 @@ Resources& Resources::operator-=(const Resources& that)
ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
{
+ const Option<string> csiSource = source.has_id() || source.has_profile()
+ ? "(" + source.vendor() + "," + source.id() + "," + source.profile() + ")"
+ : Option<string>::none();
+
switch (source.type()) {
case Resource::DiskInfo::Source::MOUNT:
- return stream
- << "MOUNT"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")"
- : (source.mount().has_root() ? ":" + source.mount().root() : ""));
+ return stream << "MOUNT" << csiSource.getOrElse(
+ source.mount().has_root() ? ":" + source.mount().root() : "");
case Resource::DiskInfo::Source::PATH:
- return stream
- << "PATH"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")"
- : (source.path().has_root() ? ":" + source.path().root() : ""));
+ return stream << "PATH" << csiSource.getOrElse(
+ source.path().has_root() ? ":" + source.path().root() : "");
case Resource::DiskInfo::Source::BLOCK:
- return stream
- << "BLOCK"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")" : "");
+ return stream << "BLOCK" << csiSource.getOrElse("");
case Resource::DiskInfo::Source::RAW:
- return stream
- << "RAW"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")" : "");
+ return stream << "RAW" << csiSource.getOrElse("");
case Resource::DiskInfo::Source::UNKNOWN:
return stream << "UNKNOWN";
}
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 0248f1d..d717f53 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -194,6 +194,14 @@ bool operator==(
return false;
}
+ if (left.has_vendor() != right.has_vendor()) {
+ return false;
+ }
+
+ if (left.has_vendor() && left.vendor() != right.vendor()) {
+ return false;
+ }
+
if (left.has_id() != right.has_id()) {
return false;
}
@@ -2382,29 +2390,21 @@ Resources& Resources::operator-=(const Resources& that)
ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
{
+ const Option<string> csiSource = source.has_id() || source.has_profile()
+ ? "(" + source.vendor() + "," + source.id() + "," + source.profile() + ")"
+ : Option<string>::none();
+
switch (source.type()) {
case Resource::DiskInfo::Source::MOUNT:
- return stream
- << "MOUNT"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")"
- : (source.mount().has_root() ? ":" + source.mount().root() : ""));
+ return stream << "MOUNT" << csiSource.getOrElse(
+ source.mount().has_root() ? ":" + source.mount().root() : "");
case Resource::DiskInfo::Source::PATH:
- return stream
- << "PATH"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")"
- : (source.path().has_root() ? ":" + source.path().root() : ""));
+ return stream << "PATH" << csiSource.getOrElse(
+ source.path().has_root() ? ":" + source.path().root() : "");
case Resource::DiskInfo::Source::BLOCK:
- return stream
- << "BLOCK"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")" : "");
+ return stream << "BLOCK" << csiSource.getOrElse("");
case Resource::DiskInfo::Source::RAW:
- return stream
- << "RAW"
- << ((source.has_id() || source.has_profile())
- ? "(" + source.id() + "," + source.profile() + ")" : "");
+ return stream << "RAW" << csiSource.getOrElse("");
case Resource::DiskInfo::Source::UNKNOWN:
return stream << "UNKNOWN";
}