Merge pull request #157 from democratic-csi/next

more robust code for setting filesystem mode/ownership, use TrueNAS a…

CHANGELOG.md

@@ -1,3 +1,12 @@
+# v1.5.3
+
+Released 2022-03-02
+
+- support for running `freenas-iscsi` and `freenas-nfs` sudo-less (see #151)
+- more robust chown / chmod logic for all zfs drivers
+- all for setting extent comment/description in `freenas-iscsi` and
+  `freenas-api-iscsi` (see #158)
+
 # v1.5.2
 
 Released 2022-02-24
@@ -22,8 +31,8 @@ Released 2022-02-23
   - only build `node_modules` once by using artifacts
   - support allow/block listing specific tests
   - better logic waiting for driver socket to appear
-- introduce `zfs-local-dataset` driver
+- introduce `zfs-local-dataset` driver (see #148)
-- introduce `zfs-local-zvol` driver
+- introduce `zfs-local-zvol` driver (see #148)
 - introduce `local-hostpath` driver
 - support manually provisioned (`node-manual`) `oneclient` volumes
 

README.md

@@ -157,7 +157,7 @@ In the name of ease-of-use these drivers by default report `MULTI_NODE` support
 node where originally provisioned. Topology contraints manage this in an
 automated fashion preventing any undesirable behavior. So while you may
 provision `MULTI_NODE` / `RWX` volumes, any workloads using the volume will
-always land a single node and that node will always be the node where the
+always land on a single node and that node will always be the node where the
 volume is/was provisioned.
 
 ### local-hostpath
@@ -165,6 +165,17 @@ volume is/was provisioned.
 This `driver` provisions node-local storage. Each node should have an
 identically name folder where volumes will be created.
 
+In the name of ease-of-use these drivers by default report `MULTI_NODE` support
+(`ReadWriteMany` in k8s) however the volumes will implicity only work on the
+node where originally provisioned. Topology contraints manage this in an
+automated fashion preventing any undesirable behavior. So while you may
+provision `MULTI_NODE` / `RWX` volumes, any workloads using the volume will
+always land on a single node and that node will always be the node where the
+volume is/was provisioned.
+
+The nature of this `driver` also prevents the enforcement of quotas. In short
+the requested volume size is generally ignored.
+
 ## Server Prep
 
 Server preparation depends slightly on which `driver` you are using.

examples/freenas-api-iscsi.yaml

@@ -37,7 +37,8 @@ zfs:
   # total volume name (zvol/<datasetParentName>/<pvc name>) length cannot exceed 63 chars
   # https://www.ixsystems.com/documentation/freenas/11.2-U5/storage.html#zfs-zvol-config-opts-tab
   # standard volume naming overhead is 46 chars
-  # datasetParentName should therefore be 17 chars or less when using TrueNAS 12 or below
+  # datasetParentName should therefore be 17 chars or less when using TrueNAS 12 or below (SCALE and 13+ do not have the same limits)
+  # for work-arounds see https://github.com/democratic-csi/democratic-csi/issues/54
   datasetParentName: tank/k8s/b/vols
   # do NOT make datasetParentName and detachedSnapshotsDatasetParentName overlap
   # they may be siblings, but neither should be nested in the other 
@@ -62,6 +63,7 @@ iscsi:
   #nameTemplate: "{{ parameters.[csi.storage.k8s.io/pvc/namespace] }}-{{ parameters.[csi.storage.k8s.io/pvc/name] }}"
   namePrefix: csi-
   nameSuffix: "-clustera"
+
   # add as many as needed
   targetGroups:
     # get the correct ID from the "portal" section in the UI
@@ -74,6 +76,7 @@ iscsi:
       # only required if using Chap
       targetGroupAuthGroup:
 
+  #extentCommentTemplate: "{{ parameters.[csi.storage.k8s.io/pvc/namespace] }}/{{ parameters.[csi.storage.k8s.io/pvc/name] }}"
   extentInsecureTpc: true
   extentXenCompat: false
   extentDisablePhysicalBlocksize: true

examples/freenas-iscsi.yaml

@@ -72,6 +72,7 @@ iscsi:
   #nameTemplate: "{{ parameters.[csi.storage.k8s.io/pvc/namespace] }}-{{ parameters.[csi.storage.k8s.io/pvc/name] }}"
   namePrefix: csi-
   nameSuffix: "-clustera"
+
   # add as many as needed
   targetGroups:
     # get the correct ID from the "portal" section in the UI
@@ -84,6 +85,7 @@ iscsi:
       # only required if using Chap
       targetGroupAuthGroup:
 
+  #extentCommentTemplate: "{{ parameters.[csi.storage.k8s.io/pvc/namespace] }}/{{ parameters.[csi.storage.k8s.io/pvc/name] }}"
   extentInsecureTpc: true
   extentXenCompat: false
   extentDisablePhysicalBlocksize: true

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "democratic-csi",
-  "version": "1.5.2",
+  "version": "1.5.3",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
@@ -20,9 +20,9 @@
       }
     },
     "@eslint/eslintrc": {
-      "version": "1.1.0",
+      "version": "1.2.0",
-      "resolved": "https://registry.npmjs.org/@eslint/eslintrc/-/eslintrc-1.1.0.tgz",
+      "resolved": "https://registry.npmjs.org/@eslint/eslintrc/-/eslintrc-1.2.0.tgz",
-      "integrity": "sha512-C1DfL7XX4nPqGd6jcP01W9pVM1HYCuUkFk1432D7F0v3JSlUIeOYn9oCoi3eoLZ+iwBSb29BMFxxny0YrrEZqg==",
+      "integrity": "sha512-igm9SjJHNEJRiUnecP/1R5T3wKLEJ7pL6e2P+GUSfCd0dGjPYYZve08uzw8L2J8foVHFz+NGu12JxRcU2gGo6w==",
       "dev": true,
       "requires": {
         "ajv": "^6.12.4",
@@ -45,9 +45,9 @@
       }
     },
     "@grpc/grpc-js": {
-      "version": "1.5.6",
+      "version": "1.5.7",
-      "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.5.6.tgz",
+      "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.5.7.tgz",
-      "integrity": "sha512-Q9dT3LkFuTnT2HHo8dQnQiFHZIfKHx/e5hDTMzK9uZ+bjZ1RAwgH5oUURVsGxBfsnH34RGeV/+51S6ZFe5KdNw==",
+      "integrity": "sha512-RAlSbZ9LXo0wNoHKeUlwP9dtGgVBDUbnBKFpfAv5iSqMG4qWz9um2yLH215+Wow1I48etIa1QMS+WAGmsE/7HQ==",
       "requires": {
         "@grpc/proto-loader": "^0.6.4",
         "@types/node": ">=12.12.47"
@@ -87,9 +87,9 @@
       }
     },
     "@humanwhocodes/config-array": {
-      "version": "0.9.3",
+      "version": "0.9.5",
-      "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.9.3.tgz",
+      "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.9.5.tgz",
-      "integrity": "sha512-3xSMlXHh03hCcCmFc0rbKp3Ivt2PFEJnQUJDDMTJQ2wkECZWdq4GePs2ctc5H8zV+cHPaq8k2vU8mrQjA6iHdQ==",
+      "integrity": "sha512-ObyMyWxZiCu/yTisA7uzx81s40xR2fD5Cg/2Kq7G02ajkNubJf6BopgDTmDyc3U7sXpNKM8cYOw7s7Tyr+DnCw==",
       "dev": true,
       "requires": {
         "@humanwhocodes/object-schema": "^1.2.1",
@@ -777,12 +777,12 @@
       "dev": true
     },
     "eslint": {
-      "version": "8.9.0",
+      "version": "8.10.0",
-      "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.9.0.tgz",
+      "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.10.0.tgz",
-      "integrity": "sha512-PB09IGwv4F4b0/atrbcMFboF/giawbBLVC7fyDamk5Wtey4Jh2K+rYaBhCAbUyEI4QzB1ly09Uglc9iCtFaG2Q==",
+      "integrity": "sha512-tcI1D9lfVec+R4LE1mNDnzoJ/f71Kl/9Cv4nG47jOueCMBrCCKYXr4AUVS7go6mWYGFD4+EoN6+eXSrEbRzXVw==",
       "dev": true,
       "requires": {
-        "@eslint/eslintrc": "^1.1.0",
+        "@eslint/eslintrc": "^1.2.0",
         "@humanwhocodes/config-array": "^0.9.2",
         "ajv": "^6.10.0",
         "chalk": "^4.0.0",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "democratic-csi",
-  "version": "1.5.2",
+  "version": "1.5.3",
   "description": "kubernetes csi driver framework",
   "main": "bin/democratic-csi",
   "scripts": {
@@ -18,7 +18,7 @@
     "url": "https://github.com/democratic-csi/democratic-csi.git"
   },
   "dependencies": {
-    "@grpc/grpc-js": "^1.5.6",
+    "@grpc/grpc-js": "^1.5.7",
     "@grpc/proto-loader": "^0.6.0",
     "@kubernetes/client-node": "^0.16.3",
     "async-mutex": "^0.3.1",
@@ -38,6 +38,6 @@
     "yargs": "^17.0.1"
   },
   "devDependencies": {
-    "eslint": "^8.9.0"
+    "eslint": "^8.10.0"
   }
 }

src/driver/controller-zfs/index.js

@@ -1,3 +1,4 @@
+const _ = require("lodash");
 const { CsiBaseDriver } = require("../index");
 const { GrpcError, grpc } = require("../../utils/grpc");
 const sleep = require("../../utils/general").sleep;
@@ -453,6 +454,64 @@ class ControllerZfsBaseDriver extends CsiBaseDriver {
     }
   }
 
+  async setFilesystemMode(path, mode) {
+    const driver = this;
+    const execClient = this.getExecClient();
+
+    let command = execClient.buildCommand("chmod", [mode, path]);
+    if ((await driver.getWhoAmI()) != "root") {
+      command = (await driver.getSudoPath()) + " " + command;
+    }
+
+    driver.ctx.logger.verbose("set permission command: %s", command);
+
+    let response = await execClient.exec(command);
+    if (response.code != 0) {
+      throw new GrpcError(
+        grpc.status.UNKNOWN,
+        `error setting permissions on dataset: ${JSON.stringify(response)}`
+      );
+    }
+  }
+
+  async setFilesystemOwnership(path, user = false, group = false) {
+    const driver = this;
+    const execClient = this.getExecClient();
+
+    if (user === false || typeof user == "undefined" || user === null) {
+      user = "";
+    }
+
+    if (group === false || typeof group == "undefined" || group === null) {
+      group = "";
+    }
+
+    user = String(user);
+    group = String(group);
+
+    if (user.length < 1 && group.length < 1) {
+      return;
+    }
+
+    let command = execClient.buildCommand("chown", [
+      (user.length > 0 ? user : "") + ":" + (group.length > 0 ? group : ""),
+      path,
+    ]);
+    if ((await driver.getWhoAmI()) != "root") {
+      command = (await driver.getSudoPath()) + " " + command;
+    }
+
+    driver.ctx.logger.verbose("set ownership command: %s", command);
+
+    let response = await execClient.exec(command);
+    if (response.code != 0) {
+      throw new GrpcError(
+        grpc.status.UNKNOWN,
+        `error setting ownership on dataset: ${JSON.stringify(response)}`
+      );
+    }
+  }
+
   /**
    * Ensure sane options are used etc
    * true = ready
@@ -1013,10 +1072,6 @@ class ControllerZfsBaseDriver extends CsiBaseDriver {
           await zb.zfs.set(datasetName, properties);
         }
 
-        //datasetPermissionsMode: 0777,
-        //datasetPermissionsUser: "root",
-        //datasetPermissionsGroup: "wheel",
-
         // get properties needed for remaining calls
         properties = await zb.zfs.get(datasetName, [
           "mountpoint",
@@ -1031,53 +1086,24 @@ class ControllerZfsBaseDriver extends CsiBaseDriver {
 
         // set mode
         if (this.options.zfs.datasetPermissionsMode) {
-          command = execClient.buildCommand("chmod", [
+          await driver.setFilesystemMode(
-            this.options.zfs.datasetPermissionsMode,
             properties.mountpoint.value,
-          ]);
+            this.options.zfs.datasetPermissionsMode
-          if ((await this.getWhoAmI()) != "root") {
+          );
-            command = (await this.getSudoPath()) + " " + command;
-          }
-
-          driver.ctx.logger.verbose("set permission command: %s", command);
-          response = await execClient.exec(command);
-          if (response.code != 0) {
-            throw new GrpcError(
-              grpc.status.UNKNOWN,
-              `error setting permissions on dataset: ${JSON.stringify(
-                response
-              )}`
-            );
-          }
         }
 
         // set ownership
         if (
-          this.options.zfs.datasetPermissionsUser ||
+          String(_.get(this.options, "zfs.datasetPermissionsUser", "")).length >
-          this.options.zfs.datasetPermissionsGroup
+            0 ||
+          String(_.get(this.options, "zfs.datasetPermissionsGroup", ""))
+            .length > 0
         ) {
-          command = execClient.buildCommand("chown", [
+          await driver.setFilesystemOwnership(
-            (this.options.zfs.datasetPermissionsUser
-              ? this.options.zfs.datasetPermissionsUser
-              : "") +
-              ":" +
-              (this.options.zfs.datasetPermissionsGroup
-                ? this.options.zfs.datasetPermissionsGroup
-                : ""),
             properties.mountpoint.value,
-          ]);
+            this.options.zfs.datasetPermissionsUser,
-          if ((await this.getWhoAmI()) != "root") {
+            this.options.zfs.datasetPermissionsGroup
-            command = (await this.getSudoPath()) + " " + command;
+          );
-          }
-
-          driver.ctx.logger.verbose("set ownership command: %s", command);
-          response = await execClient.exec(command);
-          if (response.code != 0) {
-            throw new GrpcError(
-              grpc.status.UNKNOWN,
-              `error setting ownership on dataset: ${JSON.stringify(response)}`
-            );
-          }
         }
 
         // set acls

src/driver/freenas/api.js

@@ -640,6 +640,25 @@ class FreeNASApiDriver extends CsiBaseDriver {
           "FreeNAS creating iscsi assets with name: " + iscsiName
         );
 
+        let extentComment;
+        if (this.options.iscsi.extentCommentTemplate) {
+          extentComment = Handlebars.compile(
+            this.options.iscsi.extentCommentTemplate
+          )({
+            name: call.request.name,
+            parameters: call.request.parameters,
+            csi: {
+              name: this.ctx.args.csiName,
+              version: this.ctx.args.csiVersion,
+            },
+            zfs: {
+              datasetName: datasetName,
+            },
+          });
+        } else {
+          extentComment = "";
+        }
+
         const extentInsecureTpc = this.options.iscsi.hasOwnProperty(
           "extentInsecureTpc"
         )
@@ -863,7 +882,7 @@ class FreeNASApiDriver extends CsiBaseDriver {
               }
 
               let extent = {
-                iscsi_target_extent_comment: "", // TODO: allow template for this value
+                iscsi_target_extent_comment: extentComment,
                 iscsi_target_extent_type: "Disk", // Disk/File, after save Disk becomes "ZVOL"
                 iscsi_target_extent_name: iscsiName,
                 iscsi_target_extent_insecure_tpc: extentInsecureTpc,
@@ -1114,7 +1133,7 @@ class FreeNASApiDriver extends CsiBaseDriver {
               });
 
               let extent = {
-                comment: "", // TODO: allow this to be templated
+                comment: extentComment,
                 type: "DISK", // Disk/File, after save Disk becomes "ZVOL"
                 name: iscsiName,
                 //iscsi_target_extent_naa: "0x3822690834aae6c5",

src/driver/freenas/ssh.js

@@ -671,6 +671,25 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver {
           "FreeNAS creating iscsi assets with name: " + iscsiName
         );
 
+        let extentComment;
+        if (this.options.iscsi.extentCommentTemplate) {
+          extentComment = Handlebars.compile(
+            this.options.iscsi.extentCommentTemplate
+          )({
+            name: call.request.name,
+            parameters: call.request.parameters,
+            csi: {
+              name: this.ctx.args.csiName,
+              version: this.ctx.args.csiVersion,
+            },
+            zfs: {
+              datasetName: datasetName,
+            },
+          });
+        } else {
+          extentComment = "";
+        }
+
         const extentInsecureTpc = this.options.iscsi.hasOwnProperty(
           "extentInsecureTpc"
         )
@@ -894,7 +913,7 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver {
               }
 
               let extent = {
-                iscsi_target_extent_comment: "", // TODO: allow template for this value
+                iscsi_target_extent_comment: extentComment,
                 iscsi_target_extent_type: "Disk", // Disk/File, after save Disk becomes "ZVOL"
                 iscsi_target_extent_name: iscsiName,
                 iscsi_target_extent_insecure_tpc: extentInsecureTpc,
@@ -1145,7 +1164,7 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver {
               });
 
               let extent = {
-                comment: "", // TODO: allow this to be templated
+                comment: extentComment,
                 type: "DISK", // Disk/File, after save Disk becomes "ZVOL"
                 name: iscsiName,
                 //iscsi_target_extent_naa: "0x3822690834aae6c5",
@@ -1657,9 +1676,143 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver {
     }
   }
 
+  async setFilesystemMode(path, mode) {
+    const httpClient = await this.getHttpClient();
+    const apiVersion = httpClient.getApiVersion();
+
+    switch (apiVersion) {
+      case 1:
+        return super.setFilesystemMode(...arguments);
+      case 2:
+        let perms = {
+          path,
+          mode: String(mode),
+        };
+
+        /*
+        {
+          "path": "string",
+          "mode": "string",
+          "uid": 0,
+          "gid": 0,
+          "options": {
+            "stripacl": false,
+            "recursive": false,
+            "traverse": false
+          }
+        }
+        */
+
+        let response;
+        let endpoint;
+
+        endpoint = `/filesystem/setperm`;
+        response = await httpClient.post(endpoint, perms);
+
+        if (response.statusCode == 200) {
+          return;
+        }
+
+        throw new Error(JSON.stringify(response.body));
+
+        break;
+      default:
+        throw new GrpcError(
+          grpc.status.FAILED_PRECONDITION,
+          `invalid configuration: unknown apiVersion ${apiVersion}`
+        );
+    }
+  }
+
+  async setFilesystemOwnership(path, user = false, group = false) {
+    const httpClient = await this.getHttpClient();
+    const apiVersion = httpClient.getApiVersion();
+
+    if (user === false || typeof user == "undefined" || user === null) {
+      user = "";
+    }
+
+    if (group === false || typeof group == "undefined" || group === null) {
+      group = "";
+    }
+
+    user = String(user);
+    group = String(group);
+
+    if (user.length < 1 && group.length < 1) {
+      return;
+    }
+
+    switch (apiVersion) {
+      case 1:
+        return super.setFilesystemOwnership(...arguments);
+      case 2:
+        let perms = {
+          path,
+        };
+        // set ownership
+
+        // user
+        if (user.length > 0) {
+          if (String(user).match(/^[0-9]+$/) == null) {
+            throw new GrpcError(
+              grpc.status.FAILED_PRECONDITION,
+              `datasetPermissionsUser must be numeric: ${user}`
+            );
+          }
+          perms.uid = Number(user);
+        }
+
+        // group
+        if (group.length > 0) {
+          if (String(group).match(/^[0-9]+$/) == null) {
+            throw new GrpcError(
+              grpc.status.FAILED_PRECONDITION,
+              `datasetPermissionsGroup must be numeric: ${group}`
+            );
+          }
+          perms.gid = Number(group);
+        }
+
+        /*
+        {
+          "path": "string",
+          "mode": "string",
+          "uid": 0,
+          "gid": 0,
+          "options": {
+            "stripacl": false,
+            "recursive": false,
+            "traverse": false
+          }
+        }
+        */
+
+        let response;
+        let endpoint;
+
+        endpoint = `/filesystem/setperm`;
+        response = await httpClient.post(endpoint, perms);
+
+        if (response.statusCode == 200) {
+          return;
+        }
+
+        throw new Error(JSON.stringify(response.body));
+        break;
+      default:
+        throw new GrpcError(
+          grpc.status.FAILED_PRECONDITION,
+          `invalid configuration: unknown apiVersion ${apiVersion}`
+        );
+    }
+  }
+
   async expandVolume(call, datasetName) {
     const driverShareType = this.getDriverShareType();
     const execClient = this.getExecClient();
+    const httpClient = await this.getHttpClient();
+    const apiVersion = httpClient.getApiVersion();
     const zb = await this.getZetabyte();
 
     switch (driverShareType) {
@@ -1696,8 +1849,39 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver {
           ]);
           reload = true;
         } else {
-          command = execClient.buildCommand("/etc/rc.d/ctld", ["reload"]);
+          switch (apiVersion) {
-          reload = true;
+            case 1:
+              // use cli for now
+              command = execClient.buildCommand("/etc/rc.d/ctld", ["reload"]);
+              reload = true;
+              break;
+            case 2:
+              this.ctx.logger.verbose(
+                "FreeNAS reloading iscsi daemon using api"
+              );
+              // POST /service/reload
+              let payload = {
+                service: "iscsitarget", // api version of ctld, same name in SCALE as well
+                "service-control": {
+                  ha_propagate: true,
+                },
+              };
+              let response = await httpClient.post("/service/reload", payload);
+              if (![200, 204].includes(response.statusCode)) {
+                throw new GrpcError(
+                  grpc.status.UNKNOWN,
+                  `error reloading iscsi daemon - code: ${
+                    response.statusCode
+                  } body: ${JSON.stringify(response.body)}`
+                );
+              }
+              return;
+            default:
+              throw new GrpcError(
+                grpc.status.FAILED_PRECONDITION,
+                `invalid configuration: unknown apiVersion ${apiVersion}`
+              );
+          }
         }
 
         if (reload) {