From 41d4502d7e5e587fc90a30fc7c466bae544b354a Mon Sep 17 00:00:00 2001 From: Travis Glenn Hansen Date: Wed, 23 Feb 2022 11:02:23 -0700 Subject: [PATCH 1/2] ci flakes, delegated zfs setups Signed-off-by: Travis Glenn Hansen --- CHANGELOG.md | 8 ++++ README.md | 2 +- package.json | 2 +- src/driver/controller-client-common/index.js | 8 ++++ src/driver/controller-zfs-generic/index.js | 11 ++---- src/driver/controller-zfs-local/index.js | 10 +---- src/driver/controller-zfs/index.js | 39 ++++++++++++++------ src/driver/freenas/api.js | 22 +++++++---- src/driver/freenas/ssh.js | 11 ++---- 9 files changed, 69 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1a2d8b..5f05093 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# v1.5.1 + +Released 2022-02-23 + +- fix ci flakes +- better support running `zfs` commands as non-root with `delegated` + permissions + # v1.5.0 Released 2022-02-23 diff --git a/README.md b/README.md index 4c2c528..15e4b4b 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ volume is/was provisioned. ### local-hostpath -This `driver` provisions node-local storage. Each node should and an +This `driver` provisions node-local storage. Each node should have an identically name folder where volumes will be created. ## Server Prep diff --git a/package.json b/package.json index 523728c..8382c69 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "democratic-csi", - "version": "1.5.0", + "version": "1.5.1", "description": "kubernetes csi driver framework", "main": "bin/democratic-csi", "scripts": { diff --git a/src/driver/controller-client-common/index.js b/src/driver/controller-client-common/index.js index 7ab1709..4593b42 100644 --- a/src/driver/controller-client-common/index.js +++ b/src/driver/controller-client-common/index.js @@ -320,6 +320,10 @@ class ControllerClientCommonDriver extends CsiBaseDriver { return response.stdout.split("\n")[1].trim(); } + async createDir(path) { + await this.exec("mkdir", ["-p", path]); + } + async deleteDir(path) { await this.exec("rm", ["-rf", path]); @@ -600,6 +604,10 @@ class ControllerClientCommonDriver extends CsiBaseDriver { } } + if (!(await driver.directoryExists(driver.getControllerBasePath()))) { + await driver.createDir(driver.getControllerBasePath()); + } + const available_capacity = await driver.getAvailableSpaceAtPath( driver.getControllerBasePath() ); diff --git a/src/driver/controller-zfs-generic/index.js b/src/driver/controller-zfs-generic/index.js index 42b690f..bb44049 100644 --- a/src/driver/controller-zfs-generic/index.js +++ b/src/driver/controller-zfs-generic/index.js @@ -1,3 +1,4 @@ +const _ = require("lodash"); const { ControllerZfsBaseDriver } = require("../controller-zfs"); const { GrpcError, grpc } = require("../../utils/grpc"); const SshClient = require("../../utils/ssh").SshClient; @@ -30,13 +31,7 @@ class ControllerZfsGenericDriver extends ControllerZfsBaseDriver { options.paths = this.options.zfs.cli.paths; } - if ( - this.options.zfs.hasOwnProperty("cli") && - this.options.zfs.cli && - this.options.zfs.cli.hasOwnProperty("sudoEnabled") - ) { - options.sudo = this.getSudoEnabled(); - } + options.sudo = _.get(this.options, "zfs.cli.sudoEnabled", false); if (typeof this.setZetabyteCustomOptions === "function") { await this.setZetabyteCustomOptions(options); @@ -367,7 +362,7 @@ delete ${iscsiName} taregetCliCommand.push("|"); taregetCliCommand.push("targetcli"); - if (this.options.iscsi.shareStrategyTargetCli.sudoEnabled) { + if (_.get(this.options, "iscsi.shareStrategyTargetCli.sudoEnabled", false)) { command = "sudo"; args.unshift("sh"); } diff --git a/src/driver/controller-zfs-local/index.js b/src/driver/controller-zfs-local/index.js index e9e8363..f48b586 100644 --- a/src/driver/controller-zfs-local/index.js +++ b/src/driver/controller-zfs-local/index.js @@ -59,13 +59,7 @@ class ControllerZfsLocalDriver extends ControllerZfsBaseDriver { chroot: "chroot", }; - if ( - this.options.zfs.hasOwnProperty("cli") && - this.options.zfs.cli && - this.options.zfs.cli.hasOwnProperty("sudoEnabled") - ) { - options.sudo = this.getSudoEnabled(); - } + options.sudo = _.get(this.options, "zfs.cli.sudoEnabled", false); if (typeof this.setZetabyteCustomOptions === "function") { await this.setZetabyteCustomOptions(options); @@ -105,7 +99,7 @@ class ControllerZfsLocalDriver extends ControllerZfsBaseDriver { * default want to provision volumes of RWX. The topology contraints * implicity will enforce only a single node can use the volume at a given * time. - * + * * @returns Array */ getAccessModes() { diff --git a/src/driver/controller-zfs/index.js b/src/driver/controller-zfs/index.js index 456a602..62c398f 100644 --- a/src/driver/controller-zfs/index.js +++ b/src/driver/controller-zfs/index.js @@ -149,8 +149,17 @@ class ControllerZfsBaseDriver extends CsiBaseDriver { } } - getSudoEnabled() { - return this.options.zfs.cli && this.options.zfs.cli.sudoEnabled === true; + async getWhoAmI() { + const driver = this; + const execClient = driver.getExecClient(); + const command = "whoami"; + driver.ctx.logger.verbose("whoami command: %s", command); + const response = await execClient.exec(command); + if (response.code !== 0) { + throw new Error("failed to run uname to determine max zvol name length"); + } else { + return response.stdout.trim(); + } } async getSudoPath() { @@ -321,6 +330,13 @@ class ControllerZfsBaseDriver extends CsiBaseDriver { return; } + if ( + !zb.helpers.isPropertyValueSet(row[SHARE_VOLUME_CONTEXT_PROPERTY_NAME]) + ) { + driver.ctx.logger.warn(`${row.name} is missing share context`); + return; + } + let volume_content_source; let volume_context = JSON.parse(row[SHARE_VOLUME_CONTEXT_PROPERTY_NAME]); if ( @@ -1019,7 +1035,7 @@ class ControllerZfsBaseDriver extends CsiBaseDriver { this.options.zfs.datasetPermissionsMode, properties.mountpoint.value, ]); - if (this.getSudoEnabled()) { + if ((await this.getWhoAmI()) != "root") { command = (await this.getSudoPath()) + " " + command; } @@ -1050,7 +1066,7 @@ class ControllerZfsBaseDriver extends CsiBaseDriver { : ""), properties.mountpoint.value, ]); - if (this.getSudoEnabled()) { + if ((await this.getWhoAmI()) != "root") { command = (await this.getSudoPath()) + " " + command; } @@ -1073,7 +1089,7 @@ class ControllerZfsBaseDriver extends CsiBaseDriver { acl, properties.mountpoint.value, ]); - if (this.getSudoEnabled()) { + if ((await this.getWhoAmI()) != "root") { command = (await this.getSudoPath()) + " " + command; } @@ -1641,12 +1657,13 @@ class ControllerZfsBaseDriver extends CsiBaseDriver { } let volume = await driver.populateCsiVolumeFromData(row); - let status = await driver.getVolumeStatus(datasetName); - - entries.push({ - volume, - status, - }); + if (volume) { + let status = await driver.getVolumeStatus(datasetName); + entries.push({ + volume, + status, + }); + } } if (max_entries && entries.length > max_entries) { diff --git a/src/driver/freenas/api.js b/src/driver/freenas/api.js index a2db18e..f7fca64 100644 --- a/src/driver/freenas/api.js +++ b/src/driver/freenas/api.js @@ -1657,7 +1657,7 @@ class FreeNASApiDriver extends CsiBaseDriver { } if (reload) { - if (this.getSudoEnabled()) { + if ((await this.getWhoAmI()) != "root") { command = (await this.getSudoPath()) + " " + command; } @@ -1726,6 +1726,13 @@ class FreeNASApiDriver extends CsiBaseDriver { return; } + if ( + !zb.helpers.isPropertyValueSet(row[SHARE_VOLUME_CONTEXT_PROPERTY_NAME]) + ) { + driver.ctx.logger.warn(`${row.name} is missing share context`); + return; + } + let volume_content_source; let volume_context = JSON.parse(row[SHARE_VOLUME_CONTEXT_PROPERTY_NAME]); if ( @@ -3208,12 +3215,13 @@ class FreeNASApiDriver extends CsiBaseDriver { ); let volume = await driver.populateCsiVolumeFromData(row); - let status = await driver.getVolumeStatus(volume_id); - - entries.push({ - volume, - status, - }); + if (volume) { + let status = await driver.getVolumeStatus(volume_id); + entries.push({ + volume, + status, + }); + } } if (max_entries && entries.length > max_entries) { diff --git a/src/driver/freenas/ssh.js b/src/driver/freenas/ssh.js index 579f92c..8be042e 100644 --- a/src/driver/freenas/ssh.js +++ b/src/driver/freenas/ssh.js @@ -1,3 +1,4 @@ +const _ = require("lodash"); const { ControllerZfsBaseDriver } = require("../controller-zfs"); const { GrpcError, grpc } = require("../../utils/grpc"); const SshClient = require("../../utils/ssh").SshClient; @@ -42,13 +43,7 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver { options.paths = this.options.zfs.cli.paths; } - if ( - this.options.zfs.hasOwnProperty("cli") && - this.options.zfs.cli && - this.options.zfs.cli.hasOwnProperty("sudoEnabled") - ) { - options.sudo = this.getSudoEnabled(); - } + options.sudo = _.get(this.options, "zfs.cli.sudoEnabled", false); if (typeof this.setZetabyteCustomOptions === "function") { await this.setZetabyteCustomOptions(options); @@ -1706,7 +1701,7 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver { } if (reload) { - if (this.getSudoEnabled()) { + if ((await this.getWhoAmI()) != "root") { command = (await this.getSudoPath()) + " " + command; } From 0bbbadd0c4bc8de2a1f157637c27f5f81afcc2e3 Mon Sep 17 00:00:00 2001 From: Travis Glenn Hansen Date: Wed, 23 Feb 2022 12:05:56 -0700 Subject: [PATCH 2/2] assume assets are deleted already deleted if the response is 404 Signed-off-by: Travis Glenn Hansen --- src/driver/freenas/api.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/driver/freenas/api.js b/src/driver/freenas/api.js index f7fca64..2da1887 100644 --- a/src/driver/freenas/api.js +++ b/src/driver/freenas/api.js @@ -1521,7 +1521,7 @@ class FreeNASApiDriver extends CsiBaseDriver { if (deleteAsset) { response = await httpClient.delete(endpoint); - if (![200, 204].includes(response.statusCode)) { + if (![200, 204, 404].includes(response.statusCode)) { throw new GrpcError( grpc.status.UNKNOWN, `received error deleting iscsi target - target: ${targetId} code: ${ @@ -1583,7 +1583,7 @@ class FreeNASApiDriver extends CsiBaseDriver { if (deleteAsset) { response = await httpClient.delete(endpoint); - if (![200, 204].includes(response.statusCode)) { + if (![200, 204, 404].includes(response.statusCode)) { throw new GrpcError( grpc.status.UNKNOWN, `received error deleting iscsi extent - extent: ${extentId} code: ${