This is an automated email from the ASF dual-hosted git repository. qianzhang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push: new 8700dd8 Inferred CSI volume's `readonly` field from volume mode. 8700dd8 is described below commit 8700dd8d5ece658804d7b7a40863800dcc5c72bc Author: Qian Zhang <zhq527...@gmail.com> AuthorDate: Sat Sep 19 11:11:04 2020 +0800 Inferred CSI volume's `readonly` field from volume mode. Review: https://reviews.apache.org/r/72888 --- include/mesos/mesos.proto | 7 ++-- include/mesos/v1/mesos.proto | 7 ++-- .../mesos/isolators/volume/csi/isolator.cpp | 18 +++------- .../mesos/isolators/volume/csi/isolator.hpp | 5 ++- src/slave/csi_server.cpp | 42 ++++++++++++---------- src/slave/csi_server.hpp | 3 +- .../containerizer/volume_csi_isolator_tests.cpp | 39 +++++++++++--------- src/tests/mesos.hpp | 41 +++------------------ 8 files changed, 65 insertions(+), 97 deletions(-) diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto index a100844..a51d6fa 100644 --- a/include/mesos/mesos.proto +++ b/include/mesos/mesos.proto @@ -3205,16 +3205,15 @@ message Volume { message StaticProvisioning { required string volume_id = 1; required VolumeCapability volume_capability = 2; - optional bool readonly = 3; // The secrets needed for staging/publishing the volume, e.g.: // { // "username": {"type": REFERENCE, "reference": {"name": "U_SECRET"}}, // "password": {"type": REFERENCE, "reference": {"name": "P_SECRET"}} // } - map<string, Secret> node_stage_secrets = 4; - map<string, Secret> node_publish_secrets = 5; - map<string, string> volume_context = 6; + map<string, Secret> node_stage_secrets = 3; + map<string, Secret> node_publish_secrets = 4; + map<string, string> volume_context = 5; } optional StaticProvisioning static_provisioning = 2; diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto index 09973ab..ad7092e 100644 --- a/include/mesos/v1/mesos.proto +++ b/include/mesos/v1/mesos.proto @@ -3194,16 +3194,15 @@ message Volume { message StaticProvisioning { required string volume_id = 1; required VolumeCapability volume_capability = 2; - optional bool readonly = 3; // The secrets needed for staging/publishing the volume, e.g.: // { // "username": {"type": REFERENCE, "reference": {"name": "U_SECRET"}}, // "password": {"type": REFERENCE, "reference": {"name": "P_SECRET"}} // } - map<string, Secret> node_stage_secrets = 4; - map<string, Secret> node_publish_secrets = 5; - map<string, string> volume_context = 6; + map<string, Secret> node_stage_secrets = 3; + map<string, Secret> node_publish_secrets = 4; + map<string, string> volume_context = 5; } optional StaticProvisioning static_provisioning = 2; diff --git a/src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp b/src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp index 79a6860..8180b19 100644 --- a/src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp +++ b/src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp @@ -273,13 +273,6 @@ Future<Option<ContainerLaunchInfo>> VolumeCSIIsolatorProcess::prepare( const AccessMode& accessMode = csiVolume.static_provisioning().volume_capability().access_mode(); - if (csiVolume.static_provisioning().readonly() && - _volume.mode() == Volume::RW) { - return Failure( - "Cannot publish the volume '" + volumeId + - "' in read-only mode but use it in read-write mode"); - } - if ((accessMode.mode() == AccessMode::SINGLE_NODE_READER_ONLY || accessMode.mode() == AccessMode::MULTI_NODE_READER_ONLY) && _volume.mode() == Volume::RW) { @@ -355,10 +348,9 @@ Future<Option<ContainerLaunchInfo>> VolumeCSIIsolatorProcess::prepare( } Mount mount; - mount.csiVolume = csiVolume; - mount.volume = volume; + mount.volume = _volume; + mount.csiVolume = volume; mount.target = target; - mount.volumeMode = _volume.mode(); mounts.push_back(mount); volumeSet.insert(volume); @@ -390,7 +382,7 @@ Future<Option<ContainerLaunchInfo>> VolumeCSIIsolatorProcess::prepare( vector<Future<string>> futures; futures.reserve(mounts.size()); foreach (const Mount& mount, mounts) { - futures.push_back(csiServer->publishVolume(mount.csiVolume)); + futures.push_back(csiServer->publishVolume(mount.volume)); } return await(futures) @@ -449,7 +441,7 @@ Future<Option<ContainerLaunchInfo>> VolumeCSIIsolatorProcess::_prepare( continue; } - if (info->volumes.contains(mount.volume)) { + if (info->volumes.contains(mount.csiVolume)) { isVolumeInUse = true; break; } @@ -478,7 +470,7 @@ Future<Option<ContainerLaunchInfo>> VolumeCSIIsolatorProcess::_prepare( *launchInfo.add_mounts() = protobuf::slave::createContainerMount( source, mount.target, - MS_BIND | MS_REC | (mount.volumeMode == Volume::RO ? MS_RDONLY : 0)); + MS_BIND | MS_REC | (mount.volume.mode() == Volume::RO ? MS_RDONLY : 0)); } return launchInfo; diff --git a/src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp b/src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp index 4349acd..990aa06 100644 --- a/src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp +++ b/src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp @@ -67,10 +67,9 @@ public: private: struct Mount { - Volume::Source::CSIVolume csiVolume; - CSIVolume volume; + Volume volume; + CSIVolume csiVolume; std::string target; - Volume::Mode volumeMode; }; struct Info diff --git a/src/slave/csi_server.cpp b/src/slave/csi_server.cpp index 14fa866..3df6200 100644 --- a/src/slave/csi_server.cpp +++ b/src/slave/csi_server.cpp @@ -75,8 +75,7 @@ namespace slave { constexpr char DEFAULT_CSI_CONTAINER_PREFIX[] = "mesos-internal-csi-"; -static VolumeState createVolumeState( - const Volume::Source::CSIVolume::StaticProvisioning& volume); +static VolumeState createVolumeState(const Volume& volume); static hashset<CSIPluginContainerInfo::Service> extractServices( @@ -101,7 +100,7 @@ public: Future<Nothing> start(const SlaveID& _agentId); - Future<string> publishVolume(const Volume::Source::CSIVolume& volume); + Future<string> publishVolume(const Volume& volume); Future<Nothing> unpublishVolume( const string& pluginName, @@ -363,12 +362,18 @@ Future<Nothing> CSIServerProcess::start(const SlaveID& _agentId) } -Future<string> CSIServerProcess::publishVolume( - const Volume::Source::CSIVolume& volume) +Future<string> CSIServerProcess::publishVolume(const Volume& volume) { - CHECK(volume.has_static_provisioning()); + CHECK(volume.has_source() && + volume.source().has_type() && + volume.source().type() == Volume::Source::CSI_VOLUME); - const string& name = volume.plugin_name(); + CHECK(volume.source().has_csi_volume() && + volume.source().csi_volume().has_static_provisioning()); + + const Volume::Source::CSIVolume& csiVolume = volume.source().csi_volume(); + + const string& name = csiVolume.plugin_name(); if (!plugins.contains(name)) { // This will attempt to load the plugin's configuration, initialize the @@ -388,13 +393,13 @@ Future<string> CSIServerProcess::publishVolume( CHECK(plugins.contains(name)); return plugins.at(name).volumeManager->publishVolume( - volume.static_provisioning().volume_id(), - createVolumeState(volume.static_provisioning())); + csiVolume.static_provisioning().volume_id(), + createVolumeState(volume)); })) .then(defer(self(), [=]() { CHECK(plugins.contains(name)); - const CSIPluginInfo& info = plugins.at(volume.plugin_name()).info; + const CSIPluginInfo& info = plugins.at(csiVolume.plugin_name()).info; const string mountRootDir = info.has_target_path_root() ? info.target_path_root() @@ -402,7 +407,7 @@ Future<string> CSIServerProcess::publishVolume( return csi::paths::getMountTargetPath( mountRootDir, - volume.static_provisioning().volume_id()); + csiVolume.static_provisioning().volume_id()); })); } @@ -431,14 +436,16 @@ Future<Nothing> CSIServerProcess::unpublishVolume( } -VolumeState createVolumeState( - const Volume::Source::CSIVolume::StaticProvisioning& volume) +VolumeState createVolumeState(const Volume& volume) { + const Volume::Source::CSIVolume::StaticProvisioning& staticProvisioning = + volume.source().csi_volume().static_provisioning(); + VolumeState result; result.set_state(VolumeState::NODE_READY); - *result.mutable_volume_capability() = volume.volume_capability(); - *result.mutable_volume_context() = volume.volume_context(); - result.set_readonly(volume.readonly()); + *result.mutable_volume_capability() = staticProvisioning.volume_capability(); + *result.mutable_volume_context() = staticProvisioning.volume_context(); + result.set_readonly(volume.mode() == Volume::RO); result.set_pre_provisioned(true); return result; @@ -533,8 +540,7 @@ Future<Nothing> CSIServer::start(const SlaveID& agentId) } -Future<string> CSIServer::publishVolume( - const Volume::Source::CSIVolume& volume) +Future<string> CSIServer::publishVolume(const Volume& volume) { return started.future() .then(process::defer( diff --git a/src/slave/csi_server.hpp b/src/slave/csi_server.hpp index de5c6b6..fb8ef03 100644 --- a/src/slave/csi_server.hpp +++ b/src/slave/csi_server.hpp @@ -66,8 +66,7 @@ public: // been called, then the publishing of this volume will not be completed until // the CSI server is started. // Returns the target path at which the volume has been published. - process::Future<std::string> publishVolume( - const Volume::Source::CSIVolume& volume); + process::Future<std::string> publishVolume(const Volume& volume); // Unpublishes a CSI volume from this agent. If the `start()` method has not // yet been called, then the unpublishing of this volume will not be completed diff --git a/src/tests/containerizer/volume_csi_isolator_tests.cpp b/src/tests/containerizer/volume_csi_isolator_tests.cpp index d51d3c9..688af4d 100644 --- a/src/tests/containerizer/volume_csi_isolator_tests.cpp +++ b/src/tests/containerizer/volume_csi_isolator_tests.cpp @@ -396,9 +396,9 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_INTERNET_CURL_CommandTaskWithVolume) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false)})); + ::AccessMode::SINGLE_NODE_WRITER)})); Future<v1::scheduler::Event::Update> startingUpdate; Future<v1::scheduler::Event::Update> runningUpdate; @@ -480,9 +480,9 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_INTERNET_CURL_CommandTaskWithVolume) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false)})); + ::AccessMode::SINGLE_NODE_WRITER)})); EXPECT_CALL( *scheduler, @@ -611,9 +611,9 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_INTERNET_CURL_TaskGroupWithVolume) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false); + ::AccessMode::SINGLE_NODE_WRITER); taskInfo1.mutable_container()->CopyFrom( v1::createContainerInfo("alpine", {volume})); @@ -804,9 +804,9 @@ TEST_P(VolumeCSIIsolatorTest, UNPRIVILEGED_USER_NonRootTaskUser) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false)})); + ::AccessMode::SINGLE_NODE_WRITER)})); Future<v1::scheduler::Event::Update> startingUpdate; Future<v1::scheduler::Event::Update> runningUpdate; @@ -906,9 +906,14 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_PluginConfigAddedAtRuntime) const string pluginName = "org.apache.mesos.csi.added-at-runtime"; - Volume::Source::CSIVolume volume; - volume.set_plugin_name(pluginName); - volume.mutable_static_provisioning()->CopyFrom(staticVol); + Volume::Source::CSIVolume csiVolume; + csiVolume.set_plugin_name(pluginName); + csiVolume.mutable_static_provisioning()->CopyFrom(staticVol); + + Volume volume; + Volume::Source* source = volume.mutable_source(); + source->set_type(Volume::Source::CSI_VOLUME); + source->mutable_csi_volume()->CopyFrom(csiVolume); // First, perform publish/unpublish calls before we have written the // configuration to disk. @@ -1077,9 +1082,9 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_UnmanagedPlugin) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false)})); + ::AccessMode::SINGLE_NODE_WRITER)})); Future<v1::scheduler::Event::Update> startingUpdate; Future<v1::scheduler::Event::Update> runningUpdate; @@ -1207,9 +1212,9 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_INTERNET_CURL_UnpublishAfterAgentFailover) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false)})); + ::AccessMode::SINGLE_NODE_WRITER)})); Future<v1::scheduler::Event::Update> startingUpdate; Future<v1::scheduler::Event::Update> runningUpdate; @@ -1374,9 +1379,9 @@ TEST_P(VolumeCSIIsolatorTest, ROOT_INTERNET_CURL_FinishedWhileAgentDown) TEST_CSI_PLUGIN_TYPE, TEST_VOLUME_ID, TEST_CONTAINER_PATH, + mesos::v1::Volume::RW, mesos::v1::Volume::Source::CSIVolume::VolumeCapability - ::AccessMode::SINGLE_NODE_WRITER, - false)})); + ::AccessMode::SINGLE_NODE_WRITER)})); v1::ExecutorInfo executorInfo = v1::createExecutorInfo( v1::DEFAULT_EXECUTOR_ID, diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index 49abfc2..30e8f04 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -856,14 +856,15 @@ inline TVolume createVolumeFromDockerImage( template <typename TVolume> inline TVolume createVolumeCsi( const std::string& pluginName, - const std::string volumeId, + const std::string& volumeId, const std::string& containerPath, + const typename TVolume::Mode& mode, const typename TVolume::Source::CSIVolume::VolumeCapability - ::AccessMode::Mode mode, - bool readonly) + ::AccessMode::Mode& accessMode) { TVolume volume; volume.set_container_path(containerPath); + volume.set_mode(mode); typename TVolume::Source* source = volume.mutable_source(); source->set_type(TVolume::Source::CSI_VOLUME); @@ -873,41 +874,9 @@ inline TVolume createVolumeCsi( source->mutable_csi_volume()->mutable_static_provisioning(); staticInfo->set_volume_id(volumeId); - staticInfo->set_readonly(readonly); staticInfo->mutable_volume_capability()->mutable_mount(); staticInfo->mutable_volume_capability() - ->mutable_access_mode()->set_mode(mode); - - typedef typename TVolume::Source::CSIVolume::VolumeCapability::AccessMode - CSIAccessMode; - - // Set the top-level `mode` field of the volume based on the values of the - // CSI access mode and the `readonly` field. - typename TVolume::Mode mesosMode; - - switch (mode) { - case CSIAccessMode::SINGLE_NODE_WRITER: - case CSIAccessMode::MULTI_NODE_SINGLE_WRITER: - case CSIAccessMode::MULTI_NODE_MULTI_WRITER: { - if (readonly) { - mesosMode = TVolume::RO; - } else { - mesosMode = TVolume::RW; - } - - break; - } - - case CSIAccessMode::SINGLE_NODE_READER_ONLY: - case CSIAccessMode::MULTI_NODE_READER_ONLY: - default: { - mesosMode = TVolume::RO; - - break; - } - } - - volume.set_mode(mesosMode); + ->mutable_access_mode()->set_mode(accessMode); return volume; }