diff --git a/README.md b/README.md index 2589b325c7f..09138310833 100644 --- a/README.md +++ b/README.md @@ -371,3 +371,4 @@ and YourKit .NET Profiler, innovative and intelligent tools for profiling Java and .NET applications. + diff --git a/changelog.md b/changelog.md index 88f7a1e9bd7..36bf25cce24 100644 --- a/changelog.md +++ b/changelog.md @@ -1,4 +1,64 @@ -## Changes from 1.8.194 to 1.8.xxx +## Changes from 1.8.218 to 1.8.xxx + +### External Volume Validation changes + +#### Relaxed name validation + +As there are some external volume providers which require options in the volume name, the strict validation of the name on the external volume is now removed. + +As the uniqueness check is based on the volume name, this may lead to some inconsistencies, for the sake of uniqueness, the following volumes are distinct: + +```json +"volumes": [ + { + "external": { + "name": "name=volumename,option1=value", + }, + } + ], +``` + +```json +"volumes": [ + { + "external": { + "name": "option1=value,name=volumename", + }, + } + ], +``` + +#### Optional uniqueness check + +Previously, Marathon would validate that an external volume with the same name is only used once across all apps. This was due to the initial implementation being focused on Rexray+EBS. However, multiple external volume providers now +allow shared access to mounted volumes, so we introduced a way to disable the uniqueness check: + +A new field, `container.volumes[n].external.shared` which defaults to `false`. If set to true, the same volume name can be used +by multiple containers. The `shared` flag has to be set to `true` on all external volumes with the same name, otherwise a conflict is reported on the volume without the `shared=true` flag. + +```json + "container": { + "type": "MESOS", + "volumes": [ + { + "external": { + "size": 5, + "name": "volumename", + "provider": "dvdi", + "shared": "true", + "options": { + "dvdi/driver": "pxd", + "dvdi/shared": "true" + } + }, + "mode": "RW", + "containerPath": "/mnt/nginx" + } + ], + } +``` + +## Changes from 1.8.194 to 1.8.218 ### Revive and Suppress Refactoring diff --git a/docs/docs/external-volumes.md b/docs/docs/external-volumes.md index c45c4ed12cb..5016b7e6c52 100644 --- a/docs/docs/external-volumes.md +++ b/docs/docs/external-volumes.md @@ -122,6 +122,33 @@ If you scale your app down to 0 instances, the volume is detached from the agent The default implicit volume size is 16 GB. If you are using the original Mesos containerizer or the UCR, you can modify this default for a particular volume by setting `volumes[x].external.size`. For the Mesos and Docker containerizers, you can modify the default size for all implicit volumes by [modifying the REX-Ray configuration][11]. +#### Shared Volumes + +By default, external volumes can not share the same name. As there are providers that allow this, there is a flag that prevents the uniqueness check on the volume name. `volumes[x].external.shared` can be set to true. In this case, this volume is not included when checking if the volume name already exists. It still verifies that no other external volumes with `volumes[x].external.shared=false` exist, so all volumes with the same name must have this flag set. + +``` + "container": { + "type": "MESOS", + "volumes": [ + { + "external": { + "size": 5, + "name": "volumename", + "provider": "dvdi", + "shared": "true", + "options": { + "dvdi/driver": "pxd", + "dvdi/shared": "true" + } + }, + "mode": "RW", + "containerPath": "/mnt/nginx" + } + ], + } +``` + + ### Potential Pitfalls * If one or more external volumes are declared for a Marathon app, and the Docker image specification includes one or more `VOLUME` entries, Docker may create anonymous external volumes. This is default Docker behavior with respect to volume management when the `--volume-driver` flag is passed to `docker run`. However, anonymous volumes are not automatically deleted and will accumulate over time unless you manually delete them. To prevent Docker from creating anonymous volumes, you can either use a Mesos container with a Docker image or follow these steps: diff --git a/docs/docs/rest-api/public/api/v2/types/volumes.raml b/docs/docs/rest-api/public/api/v2/types/volumes.raml index e07f0b2fe70..59332e3fe51 100644 --- a/docs/docs/rest-api/public/api/v2/types/volumes.raml +++ b/docs/docs/rest-api/public/api/v2/types/volumes.raml @@ -25,6 +25,10 @@ types: type: label.KVLabels (pragma.omitEmpty): description: Provider specific volume configuration options + shared?: + type: boolean + default: false + description: If true, excludes this volume from the global unique volume name check PersistentVolumeType: type: string description: | diff --git a/src/main/java/mesosphere/marathon/Protos.java b/src/main/java/mesosphere/marathon/Protos.java index f9bcec05562..1f74125f9dd 100644 --- a/src/main/java/mesosphere/marathon/Protos.java +++ b/src/main/java/mesosphere/marathon/Protos.java @@ -34585,6 +34585,15 @@ public interface ExternalVolumeInfoOrBuilder extends */ org.apache.mesos.Protos.LabelOrBuilder getOptionsOrBuilder( int index); + + /** + * optional bool shared = 5 [default = false]; + */ + boolean hasShared(); + /** + * optional bool shared = 5 [default = false]; + */ + boolean getShared(); } /** *
@@ -34606,6 +34615,7 @@ private ExternalVolumeInfo() {
         name_ = "";
         provider_ = "";
         options_ = java.util.Collections.emptyList();
+        shared_ = false;
       }
 
       @java.lang.Override
@@ -34662,6 +34672,11 @@ private ExternalVolumeInfo(
                     input.readMessage(org.apache.mesos.Protos.Label.PARSER, extensionRegistry));
                 break;
               }
+              case 40: {
+                bitField0_ |= 0x00000008;
+                shared_ = input.readBool();
+                break;
+              }
             }
           }
         } catch (com.google.protobuf.InvalidProtocolBufferException e) {
@@ -34824,6 +34839,21 @@ public org.apache.mesos.Protos.LabelOrBuilder getOptionsOrBuilder(
         return options_.get(index);
       }
 
+      public static final int SHARED_FIELD_NUMBER = 5;
+      private boolean shared_;
+      /**
+       * optional bool shared = 5 [default = false];
+       */
+      public boolean hasShared() {
+        return ((bitField0_ & 0x00000008) == 0x00000008);
+      }
+      /**
+       * optional bool shared = 5 [default = false];
+       */
+      public boolean getShared() {
+        return shared_;
+      }
+
       private byte memoizedIsInitialized = -1;
       public final boolean isInitialized() {
         byte isInitialized = memoizedIsInitialized;
@@ -34862,6 +34892,9 @@ public void writeTo(com.google.protobuf.CodedOutputStream output)
         for (int i = 0; i < options_.size(); i++) {
           output.writeMessage(4, options_.get(i));
         }
+        if (((bitField0_ & 0x00000008) == 0x00000008)) {
+          output.writeBool(5, shared_);
+        }
         unknownFields.writeTo(output);
       }
 
@@ -34884,6 +34917,10 @@ public int getSerializedSize() {
           size += com.google.protobuf.CodedOutputStream
             .computeMessageSize(4, options_.get(i));
         }
+        if (((bitField0_ & 0x00000008) == 0x00000008)) {
+          size += com.google.protobuf.CodedOutputStream
+            .computeBoolSize(5, shared_);
+        }
         size += unknownFields.getSerializedSize();
         memoizedSize = size;
         return size;
@@ -34918,6 +34955,11 @@ public boolean equals(final java.lang.Object obj) {
         }
         result = result && getOptionsList()
             .equals(other.getOptionsList());
+        result = result && (hasShared() == other.hasShared());
+        if (hasShared()) {
+          result = result && (getShared()
+              == other.getShared());
+        }
         result = result && unknownFields.equals(other.unknownFields);
         return result;
       }
@@ -34946,6 +34988,11 @@ public int hashCode() {
           hash = (37 * hash) + OPTIONS_FIELD_NUMBER;
           hash = (53 * hash) + getOptionsList().hashCode();
         }
+        if (hasShared()) {
+          hash = (37 * hash) + SHARED_FIELD_NUMBER;
+          hash = (53 * hash) + com.google.protobuf.Internal.hashBoolean(
+              getShared());
+        }
         hash = (29 * hash) + unknownFields.hashCode();
         memoizedHashCode = hash;
         return hash;
@@ -35092,6 +35139,8 @@ public Builder clear() {
           } else {
             optionsBuilder_.clear();
           }
+          shared_ = false;
+          bitField0_ = (bitField0_ & ~0x00000010);
           return this;
         }
 
@@ -35137,6 +35186,10 @@ public mesosphere.marathon.Protos.Volume.ExternalVolumeInfo buildPartial() {
           } else {
             result.options_ = optionsBuilder_.build();
           }
+          if (((from_bitField0_ & 0x00000010) == 0x00000010)) {
+            to_bitField0_ |= 0x00000008;
+          }
+          result.shared_ = shared_;
           result.bitField0_ = to_bitField0_;
           onBuilt();
           return result;
@@ -35218,6 +35271,9 @@ public Builder mergeFrom(mesosphere.marathon.Protos.Volume.ExternalVolumeInfo ot
               }
             }
           }
+          if (other.hasShared()) {
+            setShared(other.getShared());
+          }
           this.mergeUnknownFields(other.unknownFields);
           onChanged();
           return this;
@@ -35680,6 +35736,38 @@ public org.apache.mesos.Protos.Label.Builder addOptionsBuilder(
           }
           return optionsBuilder_;
         }
+
+        private boolean shared_ ;
+        /**
+         * optional bool shared = 5 [default = false];
+         */
+        public boolean hasShared() {
+          return ((bitField0_ & 0x00000010) == 0x00000010);
+        }
+        /**
+         * optional bool shared = 5 [default = false];
+         */
+        public boolean getShared() {
+          return shared_;
+        }
+        /**
+         * optional bool shared = 5 [default = false];
+         */
+        public Builder setShared(boolean value) {
+          bitField0_ |= 0x00000010;
+          shared_ = value;
+          onChanged();
+          return this;
+        }
+        /**
+         * optional bool shared = 5 [default = false];
+         */
+        public Builder clearShared() {
+          bitField0_ = (bitField0_ & ~0x00000010);
+          shared_ = false;
+          onChanged();
+          return this;
+        }
         public final Builder setUnknownFields(
             final com.google.protobuf.UnknownFieldSet unknownFields) {
           return super.setUnknownFields(unknownFields);
@@ -50695,7 +50783,7 @@ public mesosphere.marathon.Protos.EnvVarSecretRef getDefaultInstanceForType() {
       "host_port\030\001 \001(\r\022\026\n\016container_port\030\002 \002(\r\022" +
       "\020\n\010protocol\030\003 \001(\t\022\014\n\004name\030\004 \001(\t\022\034\n\006label" +
       "s\030\005 \003(\0132\014.mesos.Label\022\027\n\014service_port\030\006 " +
-      "\001(\r:\0010\022\025\n\rnetwork_names\030\007 \003(\t\"\366\004\n\006Volume" +
+      "\001(\r:\0010\022\025\n\rnetwork_names\030\007 \003(\t\"\215\005\n\006Volume" +
       "\022 \n\004mode\030\003 \002(\0162\022.mesos.Volume.Mode\022\026\n\016co" +
       "ntainer_path\030\001 \002(\t\022\021\n\thost_path\030\002 \001(\t\022\033\n" +
       "\005image\030\004 \001(\0132\014.mesos.Image\022D\n\npersistent" +
@@ -50708,55 +50796,56 @@ public mesosphere.marathon.Protos.EnvVarSecretRef getDefaultInstanceForType() {
       "mesos.Resource.DiskInfo.Source.Type\0224\n\013c" +
       "onstraints\030\003 \003(\0132\037.mesosphere.marathon.C" +
       "onstraint\022\017\n\007maxSize\030\004 \001(\004\022\023\n\013profileNam" +
-      "e\030\005 \001(\t\032a\n\022ExternalVolumeInfo\022\014\n\004size\030\001 " +
+      "e\030\005 \001(\t\032x\n\022ExternalVolumeInfo\022\014\n\004size\030\001 " +
       "\001(\004\022\014\n\004name\030\002 \002(\t\022\020\n\010provider\030\003 \002(\t\022\035\n\007o" +
-      "ptions\030\004 \003(\0132\014.mesos.Label\032\"\n\020SecretVolu" +
-      "meInfo\022\016\n\006secret\030\001 \002(\t\"\274\001\n\016StorageVersio",
-      "n\022\r\n\005major\030\001 \002(\r\022\r\n\005minor\030\002 \002(\r\022\r\n\005patch" +
-      "\030\003 \002(\r\022I\n\006format\030\004 \001(\01621.mesosphere.mara" +
-      "thon.StorageVersion.StorageFormat:\006LEGAC" +
-      "Y\"2\n\rStorageFormat\022\n\n\006LEGACY\020\000\022\025\n\021PERSIS" +
-      "TENCE_STORE\020\001\"Z\n\031UpgradeStrategyDefiniti" +
-      "on\022\035\n\025minimumHealthCapacity\030\001 \002(\001\022\036\n\023max" +
-      "imumOverCapacity\030\002 \001(\001:\0011\"\236\003\n\017GroupDefin" +
-      "ition\022\n\n\002id\030\001 \002(\t\022\017\n\007version\030\002 \002(\t\022?\n\017de" +
-      "precated_apps\030\003 \003(\0132&.mesosphere.maratho" +
-      "n.ServiceDefinition\0222\n\017deprecated_pods\030\010",
-      " \003(\0132\031.mesosphere.marathon.Json\0224\n\006group" +
-      "s\030\004 \003(\0132$.mesosphere.marathon.GroupDefin" +
-      "ition\022\024\n\014dependencies\030\005 \003(\t\022?\n\004apps\030\006 \003(" +
-      "\01321.mesosphere.marathon.GroupDefinition." +
-      "AppReference\022?\n\004pods\030\007 \003(\01321.mesosphere." +
-      "marathon.GroupDefinition.AppReference\032+\n" +
-      "\014AppReference\022\n\n\002id\030\001 \002(\t\022\017\n\007version\030\002 \002" +
-      "(\t\"\371\001\n\030DeploymentPlanDefinition\022\n\n\002id\030\001 " +
-      "\002(\t\022\021\n\ttimestamp\030\002 \001(\t\022A\n\023deprecated_ori" +
-      "ginal\030\004 \001(\0132$.mesosphere.marathon.GroupD",
-      "efinition\022?\n\021deprecated_target\030\005 \001(\0132$.m" +
-      "esosphere.marathon.GroupDefinition\022\035\n\025or" +
-      "iginal_root_version\030\006 \001(\t\022\033\n\023target_root" +
-      "_version\030\007 \001(\t\"\306\001\n\013TaskFailure\022\016\n\006app_id" +
-      "\030\001 \002(\t\022\036\n\007task_id\030\002 \002(\0132\r.mesos.TaskID\022\037" +
-      "\n\005state\030\003 \002(\0162\020.mesos.TaskState\022\021\n\007messa" +
-      "ge\030\004 \001(\t:\000\022\016\n\004host\030\005 \001(\t:\000\022\017\n\007version\030\006 " +
-      "\002(\t\022\021\n\ttimestamp\030\007 \002(\t\022\037\n\007slaveId\030\010 \001(\0132" +
-      "\016.mesos.SlaveID\"T\n\014ZKStoreEntry\022\014\n\004name\030" +
-      "\001 \002(\t\022\014\n\004uuid\030\002 \002(\014\022\r\n\005value\030\003 \002(\014\022\031\n\nco",
-      "mpressed\030\004 \001(\010:\005false\"\326\001\n\023ResidencyDefin" +
-      "ition\022(\n relaunchEscalationTimeoutSecond" +
-      "s\030\001 \001(\003\022S\n\020taskLostBehavior\030\002 \001(\01629.meso" +
-      "sphere.marathon.ResidencyDefinition.Task" +
-      "LostBehavior\"@\n\020TaskLostBehavior\022\032\n\026RELA" +
-      "UNCH_AFTER_TIMEOUT\020\000\022\020\n\014WAIT_FOREVER\020\001\"$" +
-      "\n\006Secret\022\n\n\002id\030\001 \002(\t\022\016\n\006source\030\002 \002(\t\"\262\001\n" +
-      "\017EnvVarReference\0227\n\004type\030\001 \002(\0162).mesosph" +
-      "ere.marathon.EnvVarReference.Type\022\014\n\004nam" +
-      "e\030\002 \002(\t\0227\n\tsecretRef\030\003 \001(\0132$.mesosphere.",
-      "marathon.EnvVarSecretRef\"\037\n\004Type\022\013\n\007UNKN" +
-      "OWN\020\000\022\n\n\006SECRET\020\001\"#\n\017EnvVarSecretRef\022\020\n\010" +
-      "secretId\030\001 \002(\t*3\n\rKillSelection\022\021\n\rYoung" +
-      "estFirst\020\001\022\017\n\013OldestFirst\020\002B\035\n\023mesospher" +
-      "e.marathonB\006Protos"
+      "ptions\030\004 \003(\0132\014.mesos.Label\022\025\n\006shared\030\005 \001" +
+      "(\010:\005false\032\"\n\020SecretVolumeInfo\022\016\n\006secret\030",
+      "\001 \002(\t\"\274\001\n\016StorageVersion\022\r\n\005major\030\001 \002(\r\022" +
+      "\r\n\005minor\030\002 \002(\r\022\r\n\005patch\030\003 \002(\r\022I\n\006format\030" +
+      "\004 \001(\01621.mesosphere.marathon.StorageVersi" +
+      "on.StorageFormat:\006LEGACY\"2\n\rStorageForma" +
+      "t\022\n\n\006LEGACY\020\000\022\025\n\021PERSISTENCE_STORE\020\001\"Z\n\031" +
+      "UpgradeStrategyDefinition\022\035\n\025minimumHeal" +
+      "thCapacity\030\001 \002(\001\022\036\n\023maximumOverCapacity\030" +
+      "\002 \001(\001:\0011\"\236\003\n\017GroupDefinition\022\n\n\002id\030\001 \002(\t" +
+      "\022\017\n\007version\030\002 \002(\t\022?\n\017deprecated_apps\030\003 \003" +
+      "(\0132&.mesosphere.marathon.ServiceDefiniti",
+      "on\0222\n\017deprecated_pods\030\010 \003(\0132\031.mesosphere" +
+      ".marathon.Json\0224\n\006groups\030\004 \003(\0132$.mesosph" +
+      "ere.marathon.GroupDefinition\022\024\n\014dependen" +
+      "cies\030\005 \003(\t\022?\n\004apps\030\006 \003(\01321.mesosphere.ma" +
+      "rathon.GroupDefinition.AppReference\022?\n\004p" +
+      "ods\030\007 \003(\01321.mesosphere.marathon.GroupDef" +
+      "inition.AppReference\032+\n\014AppReference\022\n\n\002" +
+      "id\030\001 \002(\t\022\017\n\007version\030\002 \002(\t\"\371\001\n\030Deployment" +
+      "PlanDefinition\022\n\n\002id\030\001 \002(\t\022\021\n\ttimestamp\030" +
+      "\002 \001(\t\022A\n\023deprecated_original\030\004 \001(\0132$.mes",
+      "osphere.marathon.GroupDefinition\022?\n\021depr" +
+      "ecated_target\030\005 \001(\0132$.mesosphere.maratho" +
+      "n.GroupDefinition\022\035\n\025original_root_versi" +
+      "on\030\006 \001(\t\022\033\n\023target_root_version\030\007 \001(\t\"\306\001" +
+      "\n\013TaskFailure\022\016\n\006app_id\030\001 \002(\t\022\036\n\007task_id" +
+      "\030\002 \002(\0132\r.mesos.TaskID\022\037\n\005state\030\003 \002(\0162\020.m" +
+      "esos.TaskState\022\021\n\007message\030\004 \001(\t:\000\022\016\n\004hos" +
+      "t\030\005 \001(\t:\000\022\017\n\007version\030\006 \002(\t\022\021\n\ttimestamp\030" +
+      "\007 \002(\t\022\037\n\007slaveId\030\010 \001(\0132\016.mesos.SlaveID\"T" +
+      "\n\014ZKStoreEntry\022\014\n\004name\030\001 \002(\t\022\014\n\004uuid\030\002 \002",
+      "(\014\022\r\n\005value\030\003 \002(\014\022\031\n\ncompressed\030\004 \001(\010:\005f" +
+      "alse\"\326\001\n\023ResidencyDefinition\022(\n relaunch" +
+      "EscalationTimeoutSeconds\030\001 \001(\003\022S\n\020taskLo" +
+      "stBehavior\030\002 \001(\01629.mesosphere.marathon.R" +
+      "esidencyDefinition.TaskLostBehavior\"@\n\020T" +
+      "askLostBehavior\022\032\n\026RELAUNCH_AFTER_TIMEOU" +
+      "T\020\000\022\020\n\014WAIT_FOREVER\020\001\"$\n\006Secret\022\n\n\002id\030\001 " +
+      "\002(\t\022\016\n\006source\030\002 \002(\t\"\262\001\n\017EnvVarReference\022" +
+      "7\n\004type\030\001 \002(\0162).mesosphere.marathon.EnvV" +
+      "arReference.Type\022\014\n\004name\030\002 \002(\t\0227\n\tsecret",
+      "Ref\030\003 \001(\0132$.mesosphere.marathon.EnvVarSe" +
+      "cretRef\"\037\n\004Type\022\013\n\007UNKNOWN\020\000\022\n\n\006SECRET\020\001" +
+      "\"#\n\017EnvVarSecretRef\022\020\n\010secretId\030\001 \002(\t*3\n" +
+      "\rKillSelection\022\021\n\rYoungestFirst\020\001\022\017\n\013Old" +
+      "estFirst\020\002B\035\n\023mesosphere.marathonB\006Proto" +
+      "s"
     };
     com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner assigner =
         new com.google.protobuf.Descriptors.FileDescriptor.    InternalDescriptorAssigner() {
@@ -50914,7 +51003,7 @@ public com.google.protobuf.ExtensionRegistry assignDescriptors(
     internal_static_mesosphere_marathon_Volume_ExternalVolumeInfo_fieldAccessorTable = new
       com.google.protobuf.GeneratedMessageV3.FieldAccessorTable(
         internal_static_mesosphere_marathon_Volume_ExternalVolumeInfo_descriptor,
-        new java.lang.String[] { "Size", "Name", "Provider", "Options", });
+        new java.lang.String[] { "Size", "Name", "Provider", "Options", "Shared", });
     internal_static_mesosphere_marathon_Volume_SecretVolumeInfo_descriptor =
       internal_static_mesosphere_marathon_Volume_descriptor.getNestedTypes().get(2);
     internal_static_mesosphere_marathon_Volume_SecretVolumeInfo_fieldAccessorTable = new
diff --git a/src/main/proto/marathon.proto b/src/main/proto/marathon.proto
index 3e7e4e9d5be..c81c40ec42d 100644
--- a/src/main/proto/marathon.proto
+++ b/src/main/proto/marathon.proto
@@ -342,6 +342,7 @@ message Volume {
     required string name = 2;
     required string provider = 3;
     repeated mesos.Label options = 4;
+    optional bool shared = 5 [default = false];
   }
 
   // Defining properties of secret volumes
diff --git a/src/main/scala/mesosphere/marathon/api/serialization/ContainerSerializer.scala b/src/main/scala/mesosphere/marathon/api/serialization/ContainerSerializer.scala
index e8241b6438d..029376cd059 100644
--- a/src/main/scala/mesosphere/marathon/api/serialization/ContainerSerializer.scala
+++ b/src/main/scala/mesosphere/marathon/api/serialization/ContainerSerializer.scala
@@ -205,6 +205,7 @@ object ExternalVolumeInfoSerializer {
     val builder = Protos.Volume.ExternalVolumeInfo.newBuilder()
       .setName(info.name)
       .setProvider(info.provider)
+      .setShared(info.shared)
 
     info.size.foreach(builder.setSize)
     info.options.map{
diff --git a/src/main/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProvider.scala b/src/main/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProvider.scala
index bbc3ee76983..b91a3c282d2 100644
--- a/src/main/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProvider.scala
+++ b/src/main/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProvider.scala
@@ -83,6 +83,9 @@ private[externalvolume] case object DVDIProvider extends ExternalVolumeProvider
 
   val driverOption = "dvdi/driver"
   val quotedDriverOption = '"' + driverOption + '"'
+
+  val driverValueRexRay = "rexray"
+
 }
 
 private[impl] object DVDIProviderValidations extends ExternalVolumeValidations {
@@ -284,7 +287,7 @@ private[impl] object DVDIProviderValidations extends ExternalVolumeValidations {
       v.external.options.get(driverOption) as s""""external/options($quotedDriverOption)"""" is
         definedAnd(validLabel)
       v.external.options as "external/options" is
-        conditional[Map[String, String]](_.get(driverOption).contains("rexray"))(validRexRayOptions)
+        conditional[Map[String, String]](_.get(driverOption).contains(driverValueRexRay))(validRexRayOptions)
     }
   }
 
@@ -309,7 +312,7 @@ private[impl] object DVDIProviderValidations extends ExternalVolumeValidations {
       v.name is definedAnd(notEmpty)
       v.provider is definedAnd(equalTo(name))
       v.options.get(driverOption) as s"options($quotedDriverOption)" is definedAnd(validLabel)
-      v.options is conditional[Map[String, String]](_.get(driverOption).contains("rexray"))(validRexRayOptions)
+      v.options is conditional[Map[String, String]](_.get(driverOption).contains(driverValueRexRay))(validRexRayOptions)
     }
     forAll(
       validator[AppExternalVolume] { v =>
@@ -326,10 +329,20 @@ private[impl] object DVDIProviderValidations extends ExternalVolumeValidations {
   private[this] def matchesProvider(volume: ExternalVolume): Boolean = volume.external.provider == name
   private[this] def matchesProviderRaml(volume: AppExternalVolume): Boolean = volume.external.provider.contains(name)
 
+  private[this] def isForUniquenessCheck(volume: ExternalVolume): Boolean = !volume.external.shared
+  private[this] def isForUniquenessCheckRaml(volume: AppExternalVolume): Boolean = !volume.external.shared
+
   private[this] def namesOfMatchingVolumes(app: AppDefinition): Seq[String] =
-    app.externalVolumes.withFilter(matchesProvider).map(_.external.name)
+    app
+      .externalVolumes
+      .withFilter(matchesProvider)
+      .withFilter(isForUniquenessCheck)
+      .map(_.external.name)
 
   private[this] def namesOfMatchingVolumes(app: App): Seq[String] =
-    app.container.fold(Seq.empty[AppExternalVolume])(_.volumes.collect{ case v: AppExternalVolume => v })
-      .withFilter(matchesProviderRaml).flatMap(_.external.name)
+    app.container
+      .fold(Seq.empty[AppExternalVolume])(_.volumes.collect{ case v: AppExternalVolume => v })
+      .withFilter(matchesProviderRaml)
+      .withFilter(isForUniquenessCheckRaml)
+      .flatMap(_.external.name)
 }
diff --git a/src/main/scala/mesosphere/marathon/raml/VolumeConversion.scala b/src/main/scala/mesosphere/marathon/raml/VolumeConversion.scala
index 3f3642ec694..d0a2039f4b7 100644
--- a/src/main/scala/mesosphere/marathon/raml/VolumeConversion.scala
+++ b/src/main/scala/mesosphere/marathon/raml/VolumeConversion.scala
@@ -92,7 +92,7 @@ trait VolumeConversion extends ConstraintConversion with DefaultConversions {
   implicit val volumeWrites: Writes[state.VolumeWithMount[Volume], AppVolume] = Writes { volumeWithMount =>
 
     implicit val externalVolumeWrites: Writes[state.ExternalVolumeInfo, ExternalVolumeInfo] = Writes { ev =>
-      ExternalVolumeInfo(size = ev.size, name = Some(ev.name), provider = Some(ev.provider), options = ev.options)
+      ExternalVolumeInfo(size = ev.size, name = Some(ev.name), provider = Some(ev.provider), options = ev.options, shared = ev.shared)
     }
 
     val volume = volumeWithMount.volume
@@ -132,7 +132,8 @@ trait VolumeConversion extends ConstraintConversion with DefaultConversions {
         throw SerializationFailedException("external volume requires a name")),
       provider = volumeRaml.external.provider.getOrElse(
         throw SerializationFailedException("external volume requires a provider")),
-      options = volumeRaml.external.options
+      options = volumeRaml.external.options,
+      shared = volumeRaml.external.shared
     )
     val volume = state.ExternalVolume(name = None, external = info)
     val mount = state.VolumeMount(
@@ -201,7 +202,8 @@ trait VolumeConversion extends ConstraintConversion with DefaultConversions {
         options = volume.whenOrElse(
           _.getOptionsCount > 0,
           _.getOptionsList.map { x => x.getKey -> x.getValue }(collection.breakOut),
-          ExternalVolumeInfo.DefaultOptions)
+          ExternalVolumeInfo.DefaultOptions),
+        shared = volume.when(_.hasShared, _.getShared).getOrElse(ExternalVolumeInfo.DefaultShared)
       )
     }
 
diff --git a/src/main/scala/mesosphere/marathon/state/Volume.scala b/src/main/scala/mesosphere/marathon/state/Volume.scala
index 7475071760f..2e64f2d50dc 100644
--- a/src/main/scala/mesosphere/marathon/state/Volume.scala
+++ b/src/main/scala/mesosphere/marathon/state/Volume.scala
@@ -323,6 +323,8 @@ object PathPatterns {
   *  
  • A volume name MUST be unique within the scope of a volume provider. *
  • A fully-qualified volume name is expected to be unique across the cluster and may formed, for example, * by concatenating the volume provider name with the volume name. E.g “dvdi.volume123” + *
  • As there are multiple providers that allow shared access to the same volume, there is + * a parameter `shared` that excludes that volume from the uniqueness check. * * `provider` is optional; if specified it indicates which storage provider will implement volume * lifecycle management operations for the external volume. if unspecified, “agent” is assumed. @@ -340,12 +342,14 @@ object PathPatterns { * @param name identifies the volume within the context of the storage provider. * @param provider identifies the storage provider responsible for volume lifecycle operations. * @param options contains storage provider-specific configuration configuration + * @param shared if true, this volume is excluded from the uniqueness check */ case class ExternalVolumeInfo( size: Option[Long] = None, name: String, provider: String, - options: Map[String, String] = Map.empty[String, String]) + options: Map[String, String] = Map.empty[String, String], + shared: Boolean = false) object OptionLabelPatterns { val OptionNamespaceSeparator = "/" @@ -364,17 +368,17 @@ object ExternalVolumeInfo { implicit val validExternalVolumeInfo = validator[ExternalVolumeInfo] { info => info.size.each should be > 0L - info.name should matchRegex(LabelRegex) info.provider should matchRegex(LabelRegex) info.options is validOptions } def fromProto(evi: Protos.Volume.ExternalVolumeInfo): ExternalVolumeInfo = ExternalVolumeInfo( - if (evi.hasSize) Some(evi.getSize) else None, - evi.getName, - evi.getProvider, - evi.getOptionsList.map { p => p.getKey -> p.getValue }(collection.breakOut) + size = if (evi.hasSize) Some(evi.getSize) else None, + name = evi.getName, + provider = evi.getProvider, + shared = if (evi.hasShared) evi.getShared else false, + options = evi.getOptionsList.map { p => p.getKey -> p.getValue }(collection.breakOut) ) } diff --git a/src/test/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProviderRootGroupValidationTest.scala b/src/test/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProviderRootGroupValidationTest.scala index 4c8f45083f3..668adce6eab 100644 --- a/src/test/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProviderRootGroupValidationTest.scala +++ b/src/test/scala/mesosphere/marathon/core/externalvolume/impl/providers/DVDIProviderRootGroupValidationTest.scala @@ -74,8 +74,63 @@ class DVDIProviderRootGroupValidationTest extends UnitTest with GroupCreation { ) } + "two volumes with same name and shared true result in no error" in { + val f = new Fixture + Given("a root group with two apps and conflicting volumes") + val app1 = f.appWithDVDIVolume(appId = PathId("/nested/app1"), volumeName = "vol", shared = true) + val app2 = f.appWithDVDIVolume(appId = PathId("/nested/app2"), volumeName = "vol", shared = true) + val rootGroup = createRootGroup( + groups = Set( + createGroup( + id = PathId("/nested"), + apps = Map( + app1.id -> app1, + app2.id -> app2 + ), + validate = false + ) + ), + validate = false + ) + + f.checkResult( + rootGroup, + expectedViolations = Set.empty + ) + } + + "two volumes with same name and only one has shared true result in an error" in { + val f = new Fixture + Given("a root group with two apps and conflicting volumes") + val app1 = f.appWithDVDIVolume(appId = PathId("/nested/app1"), volumeName = "vol", shared = true) + val app2 = f.appWithDVDIVolume(appId = PathId("/nested/app2"), volumeName = "vol", shared = false) + val rootGroup = createRootGroup( + groups = Set( + createGroup( + id = PathId("/nested"), + apps = Map( + app1.id -> app1, + app2.id -> app2 + ), + validate = false + ) + ), + validate = false + ) + + f.checkResult( + rootGroup, + expectedViolations = Set( + ConstraintViolation( + constraint = "Volume name 'vol' in /nested/app1 conflicts with volume(s) of same name in app(s): /nested/app2", + path = "/groups(0)/apps(0)/externalVolumes(0)" + ) + ) + ) + } + class Fixture { - def appWithDVDIVolume(appId: PathId, volumeName: String, provider: String = DVDIProvider.name): AppDefinition = { + def appWithDVDIVolume(appId: PathId, volumeName: String, provider: String = DVDIProvider.name, shared: Boolean = false): AppDefinition = { AppDefinition( id = appId, cmd = Some("sleep 123"), @@ -88,6 +143,7 @@ class DVDIProviderRootGroupValidationTest extends UnitTest with GroupCreation { name = None, external = ExternalVolumeInfo( name = volumeName, + shared = shared, provider = provider, options = Map( DVDIProvider.driverOption -> "rexray"))), diff --git a/src/test/scala/mesosphere/marathon/raml/VolumeConversionTest.scala b/src/test/scala/mesosphere/marathon/raml/VolumeConversionTest.scala index 83af1ba6498..b39f1439c68 100644 --- a/src/test/scala/mesosphere/marathon/raml/VolumeConversionTest.scala +++ b/src/test/scala/mesosphere/marathon/raml/VolumeConversionTest.scala @@ -49,7 +49,7 @@ class VolumeConversionTest extends UnitTest with TableDrivenPropertyChecks { } "core ExternalVolume conversion" when { - val external = state.ExternalVolumeInfo(Some(123L), "external", "foo", Map("foo" -> "bla")) + val external = state.ExternalVolumeInfo(Some(123L), "external", "foo", Map("foo" -> "bla"), shared = true) val externalVolume = state.ExternalVolume(None, external) val mount = state.VolumeMount(None, "/container") val volume = state.VolumeWithMount(externalVolume, mount) @@ -65,6 +65,7 @@ class VolumeConversionTest extends UnitTest with TableDrivenPropertyChecks { externalRaml.external.options should be(external.options) externalRaml.external.provider should be(Some(external.provider)) externalRaml.external.size should be(external.size) + externalRaml.external.shared should be(true) } } } @@ -72,7 +73,7 @@ class VolumeConversionTest extends UnitTest with TableDrivenPropertyChecks { "RAML external volume conversion" when { val volume = AppExternalVolume( "/container", - ExternalVolumeInfo(Some(1L), Some("vol-name"), Some("provider"), Map("foo" -> "bla")), ReadMode.Rw) + ExternalVolumeInfo(Some(1L), Some("vol-name"), Some("provider"), Map("foo" -> "bla"), shared = true), ReadMode.Rw) "converting to core ExternalVolume" should { val (externalVolume, mount) = Some(volume.fromRaml).collect { case state.VolumeWithMount(v: state.ExternalVolume, m) => (v, m) @@ -84,6 +85,7 @@ class VolumeConversionTest extends UnitTest with TableDrivenPropertyChecks { externalVolume.external.provider should be(volume.external.provider.head) externalVolume.external.size should be(volume.external.size) externalVolume.external.options should be(volume.external.options) + externalVolume.external.shared should be(volume.external.shared) } } }