Inferred CSI volume's `readonly` field from volume mode.
Review: https://reviews.apache.org/r/72888
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 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 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 @@
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 @@
}
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 @@
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 @@
continue;
}
- if (info->volumes.contains(mount.volume)) {
+ if (info->volumes.contains(mount.csiVolume)) {
isVolumeInUse = true;
break;
}
@@ -478,7 +470,7 @@
*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 @@
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 @@
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 @@
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<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 @@
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 @@
return csi::paths::getMountTargetPath(
mountRootDir,
- volume.static_provisioning().volume_id());
+ csiVolume.static_provisioning().volume_id());
}));
}
@@ -431,14 +436,16 @@
}
-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<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 @@
// 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_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_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_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_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 @@
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_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_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_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 @@
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 @@
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;
}
