synology updates, dsm6 and dsm7 in ci

Signed-off-by: Travis Glenn Hansen <travisghansen@yahoo.com>

.github/workflows/main.yml

@@ -35,14 +35,14 @@ jobs:
           path: node_modules.tar.gz
           retention-days: 7
 
-  csi-sanity-synology:
+  csi-sanity-synology-dsm6:
     needs:
       - build-npm
     strategy:
       fail-fast: false
       matrix:
         config:
-          - synlogy/iscsi.yaml
+          - synlogy/dsm6/iscsi.yaml
     runs-on:
       - self-hosted
       - csi-sanity-synology
@@ -57,12 +57,41 @@ jobs:
           ci/bin/run.sh
         env:
           TEMPLATE_CONFIG_FILE: "./ci/configs/${{ matrix.config }}"
-          SYNOLOGY_HOST: ${{ secrets.SANITY_SYNOLOGY_HOST }}
+          SYNOLOGY_HOST: ${{ secrets.SANITY_SYNOLOGY_DSM6_HOST }}
-          SYNOLOGY_PORT: ${{ secrets.SANITY_SYNOLOGY_PORT }}
+          SYNOLOGY_PORT: ${{ secrets.SANITY_SYNOLOGY_DSM6_PORT }}
           SYNOLOGY_USERNAME: ${{ secrets.SANITY_SYNOLOGY_USERNAME }}
           SYNOLOGY_PASSWORD: ${{ secrets.SANITY_SYNOLOGY_PASSWORD }}
           SYNOLOGY_VOLUME: ${{ secrets.SANITY_SYNOLOGY_VOLUME }}
 
+  csi-sanity-synology-dsm7:
+    needs:
+      - build-npm
+    strategy:
+      fail-fast: false
+      matrix:
+        config:
+          - synlogy/dsm7/iscsi.yaml
+    runs-on:
+      - self-hosted
+      - csi-sanity-synology
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/download-artifact@v2
+        with:
+          name: node-modules
+      - name: csi-sanity
+        run: |
+          # run tests
+          ci/bin/run.sh
+        env:
+          TEMPLATE_CONFIG_FILE: "./ci/configs/${{ matrix.config }}"
+          SYNOLOGY_HOST: ${{ secrets.SANITY_SYNOLOGY_DSM7_HOST }}
+          SYNOLOGY_PORT: ${{ secrets.SANITY_SYNOLOGY_DSM7_PORT }}
+          SYNOLOGY_USERNAME: ${{ secrets.SANITY_SYNOLOGY_USERNAME }}
+          SYNOLOGY_PASSWORD: ${{ secrets.SANITY_SYNOLOGY_PASSWORD }}
+          SYNOLOGY_VOLUME: ${{ secrets.SANITY_SYNOLOGY_VOLUME }}
+
+
   # api-based drivers
   csi-sanity-truenas-scale-22_02:
     needs:
@@ -237,7 +266,8 @@ jobs:
 
   build-docker:
     needs:
-      - csi-sanity-synology
+      - csi-sanity-synology-dsm6
+      - csi-sanity-synology-dsm7
       - csi-sanity-truenas-scale-22_02
       - csi-sanity-truenas-core-12_0
       - csi-sanity-truenas-core-13_0

ci/configs/synlogy/dsm7/iscsi.yaml

@@ -0,0 +1,77 @@
+driver: synology-iscsi
+httpConnection:
+  protocol: http
+  host: ${SYNOLOGY_HOST}
+  port: ${SYNOLOGY_PORT}
+  username: ${SYNOLOGY_USERNAME}
+  password: ${SYNOLOGY_PASSWORD}
+  allowInsecure: true
+  session: "democratic-csi-${CI_BUILD_KEY}"
+  serialize: true
+
+synology:
+  volume: ${SYNOLOGY_VOLUME}
+
+iscsi:
+  targetPortal: ${SYNOLOGY_HOST}
+  targetPortals: []
+  baseiqn: "iqn.2000-01.com.synology:XpenoDsm62x."
+  namePrefix: "csi-${CI_BUILD_KEY}-"
+  nameSuffix: "-ci"
+
+  lunTemplate:
+    # btrfs thin provisioning
+    type: "BLUN"
+    # tpws = Hardware-assisted zeroing
+    # caw = Hardware-assisted locking
+    # 3pc = Hardware-assisted data transfer
+    # tpu = Space reclamation
+    # can_snapshot = Snapshot
+    #dev_attribs:
+    #- dev_attrib: emulate_tpws
+    #  enable: 1
+    #- dev_attrib: emulate_caw
+    #  enable: 1
+    #- dev_attrib: emulate_3pc
+    #  enable: 1
+    #- dev_attrib: emulate_tpu
+    #  enable: 0
+    #- dev_attrib: can_snapshot
+    #  enable: 1
+
+    # btfs thick provisioning
+    # only zeroing and locking supported
+    #type: "BLUN_THICK"
+    # tpws = Hardware-assisted zeroing
+    # caw = Hardware-assisted locking
+    #dev_attribs:
+    #- dev_attrib: emulate_tpws
+    #  enable: 1
+    #- dev_attrib: emulate_caw
+    #  enable: 1
+
+    # ext4 thinn provisioning UI sends everything with enabled=0
+    #type: "THIN"
+
+    # ext4 thin with advanced legacy features set
+    # can only alter tpu (all others are set as enabled=1)
+    #type: "ADV"
+    #dev_attribs:
+    #- dev_attrib: emulate_tpu
+    #  enable: 1
+
+    # ext4 thick
+    # can only alter caw
+    #type: "FILE"
+    #dev_attribs:
+    #- dev_attrib: emulate_caw
+    #  enable: 1
+
+  lunSnapshotTemplate:
+    is_locked: true
+    # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot
+    is_app_consistent: true
+
+  targetTemplate:
+    auth_type: 0
+    max_sessions: 0

examples/synology-iscsi.yaml

@@ -34,6 +34,9 @@ iscsi:
   # These options can also be configured per storage-class:
   # See https://github.com/democratic-csi/democratic-csi/blob/master/docs/storage-class-parameters.md
   lunTemplate:
+    # can be static value or handlebars template
+    #description: "{{ parameters.[csi.storage.k8s.io/pvc/namespace] }}-{{ parameters.[csi.storage.k8s.io/pvc/name] }}"
+    
     # btrfs thin provisioning
     type: "BLUN"
     # tpws = Hardware-assisted zeroing

src/driver/controller-synology/http/index.js

@@ -10,17 +10,46 @@ const USER_AGENT = "democratic-csi";
 const __REGISTRY_NS__ = "SynologyHttpClient";
 
 SYNO_ERRORS = {
-  18990002: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The synology volume is out of disk space." },
+  400: {
-  18990318: { status: grpc.status.INVALID_ARGUMENT, message: "The requested lun type is incompatible with the Synology filesystem." },
+    status: grpc.status.UNAUTHENTICATED,
-  18990538: { status: grpc.status.ALREADY_EXISTS, message: "A LUN with this name already exists." },
+    message: "Failed to authenticate to the Synology DSM",
-  18990541: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The maximum number of LUNS has been reached." },
+  },
-  18990542: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The maximum number if iSCSI target has been reached." },
+  18990002: {
-  18990744: { status: grpc.status.ALREADY_EXISTS, message: "An iSCSI target with this name already exists." },
+    status: grpc.status.RESOURCE_EXHAUSTED,
+    message: "The synology volume is out of disk space.",
+  },
+  18990318: {
+    status: grpc.status.INVALID_ARGUMENT,
+    message:
+      "The requested lun type is incompatible with the Synology filesystem.",
+  },
+  18990538: {
+    status: grpc.status.ALREADY_EXISTS,
+    message: "A LUN with this name already exists.",
+  },
+  18990541: {
+    status: grpc.status.RESOURCE_EXHAUSTED,
+    message: "The maximum number of LUNS has been reached.",
+  },
+  18990542: {
+    status: grpc.status.RESOURCE_EXHAUSTED,
+    message: "The maximum number if iSCSI target has been reached.",
+  },
+  18990744: {
+    status: grpc.status.ALREADY_EXISTS,
+    message: "An iSCSI target with this name already exists.",
+  },
   18990532: { status: grpc.status.NOT_FOUND, message: "No such snapshot." },
   18990500: { status: grpc.status.INVALID_ARGUMENT, message: "Bad LUN type" },
-  18990543: { status: grpc.status.RESOURCE_EXHAUSTED, message: "Maximum number of snapshots reached." },
+  18990543: {
-  18990635: { status: grpc.status.INVALID_ARGUMENT, message: "Invalid ioPolicy." }
+    status: grpc.status.RESOURCE_EXHAUSTED,
-}
+    message: "Maximum number of snapshots reached.",
+  },
+  18990635: {
+    status: grpc.status.INVALID_ARGUMENT,
+    message: "Invalid ioPolicy.",
+  },
+};
 
 class SynologyError extends GrpcError {
   constructor(code, httpCode = undefined) {
@@ -28,9 +57,11 @@ class SynologyError extends GrpcError {
     this.synoCode = code;
     this.httpCode = httpCode;
     if (code > 0) {
-      const error = SYNO_ERRORS[code]
+      const error = SYNO_ERRORS[code];
       this.code = error?.status ?? grpc.status.UNKNOWN;
-      this.message = error?.message ?? `An unknown error occurred when executing a synology command (code = ${code}).`;
+      this.message =
+        error?.message ??
+        `An unknown error occurred when executing a synology command (code = ${code}).`;
     } else {
       this.code = grpc.status.UNKNOWN;
       this.message = `The synology webserver returned a status code ${httpCode}`;
@@ -95,6 +126,15 @@ class SynologyHttpClient {
       _.set(options, prop, "redacted");
     }
 
+    prop = "params._sid";
+    val = _.get(options, prop, false);
+    if (val) {
+      _.set(options, prop, "redacted");
+    }
+
+    delete options.httpAgent;
+    delete options.httpsAgent;
+
     this.logger.debug("SYNOLOGY HTTP REQUEST: " + stringify(options));
     this.logger.debug("SYNOLOGY HTTP ERROR: " + error);
     this.logger.debug("SYNOLOGY HTTP STATUS: " + response.statusCode);
@@ -179,7 +219,7 @@ class SynologyHttpClient {
           }
 
           if (response.statusCode > 299 || response.statusCode < 200) {
-            reject(new SynologyError(null, response.statusCode))
+            reject(new SynologyError(null, response.statusCode));
           }
 
           if (response.body.success === false) {
@@ -187,7 +227,9 @@ class SynologyHttpClient {
             if (response.body.error.code == 119 && sid == client.sid) {
               client.sid = null;
             }
-            reject(new SynologyError(response.body.error.code, response.statusCode));
+            reject(
+              new SynologyError(response.body.error.code, response.statusCode)
+            );
           }
 
           resolve(response);
@@ -602,7 +644,12 @@ class SynologyHttpClient {
     );
   }
 
-  async CreateClonedVolume(src_lun_uuid, dst_lun_name, dst_location, description) {
+  async CreateClonedVolume(
+    src_lun_uuid,
+    dst_lun_name,
+    dst_location,
+    description
+  ) {
     const create_cloned_volume = {
       api: "SYNO.Core.ISCSI.LUN",
       version: 1,
@@ -619,7 +666,12 @@ class SynologyHttpClient {
     return await this.do_request("GET", "entry.cgi", create_cloned_volume);
   }
 
-  async CreateVolumeFromSnapshot(src_lun_uuid, snapshot_uuid, cloned_lun_name, description) {
+  async CreateVolumeFromSnapshot(
+    src_lun_uuid,
+    snapshot_uuid,
+    cloned_lun_name,
+    description
+  ) {
     const create_volume_from_snapshot = {
       api: "SYNO.Core.ISCSI.LUN",
       version: 1,

src/driver/controller-synology/index.js

@@ -1,11 +1,12 @@
+const _ = require("lodash");
 const { CsiBaseDriver } = require("../index");
+const GeneralUtils = require("../../utils/general");
 const { GrpcError, grpc } = require("../../utils/grpc");
+const Handlebars = require("handlebars");
 const registry = require("../../utils/registry");
 const SynologyHttpClient = require("./http").SynologyHttpClient;
 const semver = require("semver");
-const sleep = require("../../utils/general").sleep;
 const yaml = require("js-yaml");
-const GeneralUtils = require("../../utils/general");
 
 const __REGISTRY_NS__ = "ControllerSynologyDriver";
 
@@ -146,19 +147,33 @@ class ControllerSynologyDriver extends CsiBaseDriver {
 
   getObjectFromDevAttribs(list = []) {
     if (!list) {
-      return {}
+      return {};
     }
     return list.reduce(
-      (obj, item) => Object.assign(obj, {[item.dev_attrib]: item.enable}), {}
+      (obj, item) => Object.assign(obj, { [item.dev_attrib]: item.enable }),
-    )
+      {}
+    );
   }
 
   getDevAttribsFromObject(obj, keepNull = false) {
-    return Object.entries(obj).filter(
+    return Object.entries(obj)
-      e => keepNull || (e[1] != null)
+      .filter((e) => keepNull || e[1] != null)
-    ).map(
+      .map((e) => ({ dev_attrib: e[0], enable: e[1] }));
-      e => ({dev_attrib: e[0], enable: e[1]})
+  }
+
+  parseParameterYamlData(data, fieldHint = "") {
+    try {
+      return yaml.load(data);
+    } catch {
+      if (err instanceof yaml.YAMLException) {
+        throw new GrpcError(
+          grpc.status.INVALID_ARGUMENT,
+          `${fieldHint} not a valid YAML document.`.trim()
         );
+      } else {
+        throw err;
+      }
+    }
   }
 
   buildIscsiName(name) {
@@ -183,14 +198,14 @@ class ControllerSynologyDriver extends CsiBaseDriver {
    * @returns {String} The location of the volume.
    */
   getLocation() {
-    let location = this.options?.synology?.volume;
+    let location = _.get(this.options, "synology.volume");
-    if (location === undefined) {
+    if (!location) {
-      location = "volume1"
+      location = "volume1";
     }
-    if (!location.startsWith('/')) {
+    if (!location.startsWith("/")) {
-      location = "/" + location
+      location = "/" + location;
     }
-    return location
+    return location;
   }
 
   assertCapabilities(capabilities) {
@@ -350,7 +365,9 @@ class ControllerSynologyDriver extends CsiBaseDriver {
     }
 
     let volume_context = {};
-    const normalizedParameters = driver.getNormalizedParameters(call.request.parameters);
+    const normalizedParameters = driver.getNormalizedParameters(
+      call.request.parameters
+    );
     switch (driver.getDriverShareType()) {
       case "nfs":
         // TODO: create volume here
@@ -368,13 +385,53 @@ class ControllerSynologyDriver extends CsiBaseDriver {
         break;
       case "iscsi":
         let iscsiName = driver.buildIscsiName(name);
-        let storageClassTemplate;
+        let lunTemplate;
+        let targetTemplate;
         let data;
         let target;
         let lun_mapping;
         let lun_uuid;
         let existingLun;
 
+        lunTemplate = Object.assign(
+          {},
+          _.get(driver.options, "iscsi.lunTemplate", {}),
+          driver.parseParameterYamlData(
+            _.get(normalizedParameters, "lunTemplate", "{}"),
+            "parameters.lunTemplate"
+          ),
+          driver.parseParameterYamlData(
+            _.get(call.request, "secrets.lunTemplate", "{}"),
+            "secrets.lunTemplate"
+          )
+        );
+        targetTemplate = Object.assign(
+          {},
+          _.get(driver.options, "iscsi.targetTemplate", {}),
+          driver.parseParameterYamlData(
+            _.get(normalizedParameters, "targetTemplate", "{}"),
+            "parameters.targetTemplate"
+          ),
+          driver.parseParameterYamlData(
+            _.get(call.request, "secrets.targetTemplate", "{}"),
+            "secrets.targetTemplate"
+          )
+        );
+
+        // render the template for description
+        if (lunTemplate.description) {
+          lunTemplate.description = Handlebars.compile(lunTemplate.description)(
+            {
+              name: call.request.name,
+              parameters: call.request.parameters,
+              csi: {
+                name: this.ctx.args.csiName,
+                version: this.ctx.args.csiVersion,
+              },
+            }
+          );
+        }
+
         // ensure volumes with the same name being requested a 2nd time but with a different size fails
         try {
           let lun = await httpClient.GetLunByName(iscsiName);
@@ -429,7 +486,8 @@ class ControllerSynologyDriver extends CsiBaseDriver {
                 src_lun_uuid = await httpClient.GetLunByID(src_lun_uuid).uuid;
               }
 
-              let snapshot = await httpClient.GetSnapshotByLunUUIDAndSnapshotUUID(
+              let snapshot =
+                await httpClient.GetSnapshotByLunUUIDAndSnapshotUUID(
                   src_lun_uuid,
                   snapshot_uuid
                 );
@@ -446,7 +504,7 @@ class ControllerSynologyDriver extends CsiBaseDriver {
                   src_lun_uuid,
                   snapshot_uuid,
                   iscsiName,
-                  normalizedParameters.description
+                  lunTemplate.description
                 );
               }
               break;
@@ -474,7 +532,7 @@ class ControllerSynologyDriver extends CsiBaseDriver {
                   src_lun_uuid,
                   iscsiName,
                   driver.getLocation(),
-                  normalizedParameters.description
+                  lunTemplate.description
                 );
               }
               break;
@@ -494,62 +552,22 @@ class ControllerSynologyDriver extends CsiBaseDriver {
           }
         } else {
           // create lun
-          try {
+          data = Object.assign({}, lunTemplate, {
-            storageClassTemplate = yaml.load(normalizedParameters.lunTemplate ?? "")
-            const devAttribs = driver.getDevAttribsFromObject(Object.assign(
-              {},
-              driver.getObjectFromDevAttribs(driver.options.iscsi.lunTemplate?.dev_attribs),
-              driver.getObjectFromDevAttribs(storageClassTemplate?.dev_attribs)
-            ))
-            data = Object.assign({}, driver.options.iscsi.lunTemplate, storageClassTemplate, {
             name: iscsiName,
             location: driver.getLocation(),
             size: capacity_bytes,
-              dev_attribs: devAttribs
           });
 
           lun_uuid = await httpClient.CreateLun(data);
-          } catch (err) {
-            if (err instanceof yaml.YAMLException) {
-              throw new GrpcError(
-                grpc.status.INVALID_ARGUMENT,
-                `The lunTemplate on StorageClass is not a valid YAML document.`
-              );
-            } else {
-              throw err
-            }
-          }
         }
 
         // create target
         let iqn = driver.options.iscsi.baseiqn + iscsiName;
-        try {
+        data = Object.assign({}, targetTemplate, {
-          storageClassTemplate = yaml.load(normalizedParameters.targetTemplate ?? "")
-        } catch (err) {
-          throw new GrpcError(
-            grpc.status.INVALID_ARGUMENT,
-            `The targetTemplate on StorageClass is not a valid YAML document.`
-          );
-        }
-        data = Object.assign({}, driver.options.iscsi.targetTemplate, storageClassTemplate, {
           name: iscsiName,
           iqn,
         });
 
-        if ('user' in call.request.secrets && 'password' in call.request.secrets) {
-          data.user = call.request.secrets.user;
-          data.password = call.request.secrets.password;
-          data.chap = true;
-          if ('mutualUser' in call.request.secrets && 'mutualPassword' in call.request.secrets) {
-            data.mutual_user = call.request.secrets.mutualUser;
-            data.mutual_password = call.request.secrets.mutualPassword;
-            data.auth_type = 2;
-            data.mutual_chap = true;
-          } else {
-            data.auth_type = 1;
-            data.mutual_chap = false;
-          }
-        }
         let target_id = await httpClient.CreateTarget(data);
         //target = await httpClient.GetTargetByTargetID(target_id);
         target = await httpClient.GetTargetByIQN(iqn);
@@ -924,6 +942,24 @@ class ControllerSynologyDriver extends CsiBaseDriver {
       );
     }
 
+    const normalizedParameters = driver.getNormalizedParameters(
+      call.request.parameters
+    );
+    let lunSnapshotTemplate;
+
+    lunSnapshotTemplate = Object.assign(
+      {},
+      _.get(driver.options, "iscsi.lunSnapshotTemplate", {}),
+      driver.parseParameterYamlData(
+        _.get(normalizedParameters, "lunSnapshotTemplate", "{}"),
+        "parameters.lunSnapshotTemplate"
+      ),
+      driver.parseParameterYamlData(
+        _.get(call.request, "secrets.lunSnapshotTemplate", "{}"),
+        "secrets.lunSnapshotTemplate"
+      )
+    );
+
     // check for other snapshopts with the same name on other volumes and fail as appropriate
     // TODO: technically this should only be checking lun/snapshots relevant to this specific install of the driver
     // but alas an isolation/namespacing mechanism does not exist in synology
@@ -939,19 +975,9 @@ class ControllerSynologyDriver extends CsiBaseDriver {
 
     // check for already exists
     let snapshot;
-    let snapshotClassTemplate;
     snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name);
     if (!snapshot) {
-      const normalizedParameters = driver.getNormalizedParameters(call.request.parameters);
+      let data = Object.assign({}, lunSnapshotTemplate, {
-      try {
-        snapshotClassTemplate = yaml.load(normalizedParameters.lunSnapshotTemplate ?? "");
-      } catch (err) {
-        throw new GrpcError(
-          grpc.status.INVALID_ARGUMENT,
-          `The snapshotTemplate on VolumeSnapshotClass is not a valid YAML document.`
-        );
-      }
-      let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, snapshotClassTemplate, {
         src_lun_uuid: lun.uuid,
         taken_by: "democratic-csi",
         description: name, //check

src/driver/controller-zfs-generic/index.js

@@ -65,7 +65,7 @@ class ControllerZfsGenericDriver extends ControllerZfsBaseDriver {
     const driver = this;
 
     driver.ctx.logger.verbose(
-      `generating smb share name for dataset: ${typeof datasetName} ${datasetName}`
+      `generating smb share name for dataset: ${datasetName}`
     );
 
     let name = datasetName || "";