d2iq-archive / marathon

Deploy and manage containers (including Docker) on top of Apache Mesos at scale.
https://mesosphere.github.io/marathon/
Apache License 2.0
4.07k stars 843 forks source link

CSI support in Marathon #7213

Closed pierrebeitz closed 4 years ago

pierrebeitz commented 4 years ago

Jira: MARATHON-8765

supersedes #7212.

after talking to @timcharper and @kaiwalyajoshi, we figured that going with the approach of pushing the CSI Volume configuration into the existing AppExternalVolume is more coherent with existing API while introducing less concepts.

  1. ⚠️ i chose to make mode optional so users don't need to specify it for csi-volumes, as accessMode seems to hold all information we need. if we go with this, we'd need to add an additional validation for DVDI volumes.

  2. @timcharper as discussed yesterday: it's a union type for now but does not need to stay one, if the error messages are confusing this way.

  3. i chose not to prefix all the options with csi/ like it seems to be usual for dvdi-options. should we add those for consistency or do you think that it's fine this way?

JSON

here's an app definition in JSON as that requires less brain cycles to understand.

App with CSI Volume

{
  "id": "hello",
  "cmd": "sleep 100",
  "container": {
    "type": "MESOS",
    "volumes": [
      {
        "containerPath": "mypath",
        "external": {
          "provider": "csi",
          "name": "my-UNIQUE-volume-id",
          "options": { 
            "pluginName": "...",
            "access": {
              "type": "mount",
              "mode": "MULTI_NODE_MULTI_WRITER"
              "fsType": "xfs",
              "mountFlags": ["some", "mount", "flags"]
            }
            "nodeStageSecret": {
              "username": "username_secret_ref",
              "password": "password_secret_ref"
            },
            "nodePublishSecret": {
              "username": "username_secret_ref",
              "password": "password_secret_ref"
            },
            "volumeContext": {
              "some": "context"
            }
          }
        }
      }
    ]
  }
}

With Minimal Config

here's an example without optional properties. note how accessType now is specified as a string, as block volumes don't need any configuration.

{
  "id": "hello",
  "cmd": "sleep 100",
  "container": {
    "type": "MESOS",
    "volumes": [
      {
        "containerPath": "mypath",
        "external": {
          "provider": "csi",
          "name": "my-UNIQUE-volume-id",
          "options": {
            "accessType": "block",
            "accessMode": "MULTI_NODE_MULTI_WRITER"
          }
        }
      }
    ]
  }
}

App with DVDI Volume for Comparison

{
  "id": "hello",
  "cmd": "sleep 100",
  "container": {
    "type": "MESOS",
    "volumes": [
      {
        "containerPath": "mypath",
        "external": {
          "provider": "dvdi",
          "size": 100,
          "name": "my-test-vol",
          "options": { "dvdi/driver": "rexray" }
          },
        "mode": "RW"
      }
    ]
  }
}
pierrebeitz commented 4 years ago

we also need to add readonly for CSIVolumeInfo as per @qianzhangxa's comment in slack:

readonly==true indicates CSI plugin must publish the volume in readonly mode, it is actually orthogonal to accessMode.

timcharper commented 4 years ago

This looks good!

Just a side note, we're going to want to extend the secrets validation logic here, I believe. The Mesos secrets directive should also validate per the DCOS_SPACE label which is auto-set by the Marathon dcos plugins logic.

See https://github.com/mesosphere/marathon-dcos-plugins/blob/79297c47ca65b91ca9331fd3c30901301b7274d0/dcos-secrets/src/main/scala/mesosphere/marathon/dcos/plugin/secrets/DCOSSecretsDirectiveValidator.scala#L56