From 25f9380706ebaacb615d4b2c66cc319f7b895b3a Mon Sep 17 00:00:00 2001 From: batiati Date: Sun, 10 Apr 2022 17:56:22 -0300 Subject: [PATCH 01/10] Max iscsi volumes limit on FreeBSD --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 312829e..f934689 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,9 @@ Ensure the following services are configurged and running: - `curl --header "Accept: application/json" --user root: 'http(s):///api/v2.0/iscsi/portal'` - `curl --header "Accept: application/json" --user root: 'http(s):///api/v2.0/iscsi/initiator'` - `curl --header "Accept: application/json" --user root: 'http(s):///api/v2.0/iscsi/auth'` + - The maximum number of volumes is limited to 255 by default on FreeBSD (physical devices such as disks and CD-ROM drives count against this value). + Be sure to properly adjust both [tunables](https://www.freebsd.org/cgi/man.cgi?query=ctl&sektion=4#end) `kern.cam.ctl.max_ports` and `kern.cam.ctl.max_luns` to avoid running out of resources when dynamically provisioning iSCSI volumes on FreeNAS or TrueNAS Core. + - smb If you would prefer you can configure `democratic-csi` to use a From c76750a30343f5c5f4d3fa1a857e2024b084fd17 Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Wed, 13 Apr 2022 17:53:25 +0200 Subject: [PATCH 02/10] Improve Synology error handling --- src/driver/controller-synology/http/index.js | 50 ++++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index f1e7c0a..f25e3b3 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -4,10 +4,50 @@ const https = require("https"); const { axios_request, stringify } = require("../../../utils/general"); const Mutex = require("async-mutex").Mutex; const registry = require("../../../utils/registry"); +const { GrpcError, grpc } = require("../../../utils/grpc"); const USER_AGENT = "democratic-csi"; const __REGISTRY_NS__ = "SynologyHttpClient"; +SYNO_ERROR_MESSAGES = { + 18990002: "The synology volume is out of disk space.", + 18990538: "A LUN with this name already exists.", + 18990541: "The maximum number of LUNS has been reached.", + 18990542: "The maximum number if iSCSI target has been reached.", + 18990744: "An iSCSI target with this name already exists.", + 18990532: "No such snapshot.", + 18990500: "Bad LUN type", + 18990543: "Maximum number of snapshots reached.", + 18990635: "Invalid ioPolicy." +} + +SYNO_GRPC_CODES = { + 18990002: grpc.status.RESOURCE_EXHAUSTED, + 18990538: grpc.status.ALREADY_EXISTS, + 18990541: grpc.status.RESOURCE_EXHAUSTED, + 18990542: grpc.status.RESOURCE_EXHAUSTED, + 18990744: grpc.status.ALREADY_EXISTS, + 18990532: grpc.status.NOT_FOUND, + 18990500: grpc.status.INVALID_ARGUMENT, + 18990543: grpc.status.RESOURCE_EXHAUSTED, + 18990635: grpc.status.INVALID_ARGUMENT +} + +class SynologyError extends GrpcError { + constructor(code, httpCode = undefined) { + super(0, ""); + this.synoCode = code; + this.httpCode = httpCode; + if (code > 0) { + this.code = SYNO_GRPC_CODES[code] ?? grpc.status.UNKNOWN; + this.message = SYNO_ERROR_MESSAGES[code] ?? `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}`; + } + } +} + class SynologyHttpClient { constructor(options = {}) { this.options = JSON.parse(JSON.stringify(options)); @@ -149,7 +189,7 @@ class SynologyHttpClient { } if (response.statusCode > 299 || response.statusCode < 200) { - reject(response); + reject(new SynologyError(-1, response.statusCode)) } if (response.body.success === false) { @@ -157,7 +197,7 @@ class SynologyHttpClient { if (response.body.error.code == 119 && sid == client.sid) { client.sid = null; } - reject(response); + reject(new SynologyError(response.body.error.code, response.statusCode)); } resolve(response); @@ -412,7 +452,7 @@ class SynologyHttpClient { response = await this.do_request("GET", "entry.cgi", iscsi_lun_create); return response.body.data.uuid; } catch (err) { - if ([18990538].includes(err.body.error.code)) { + if (err.synoCode === 18990538) { response = await this.do_request("GET", "entry.cgi", lun_list); let lun = response.body.data.luns.find((i) => { return i.name == iscsi_lun_create.name; @@ -503,7 +543,7 @@ class SynologyHttpClient { return response.body.data.target_id; } catch (err) { - if ([18990744].includes(err.body.error.code)) { + if (err.synoCode === 18990744) { //do lookup const iscsi_target_list = { api: "SYNO.Core.ISCSI.Target", @@ -549,7 +589,7 @@ class SynologyHttpClient { /** * 18990710 = non-existant */ - //if (![18990710].includes(err.body.error.code)) { + //if (err.synoCode !== 18990710) { throw err; //} } From bd620025a06f0d64f8a95a163866d771b3debb8b Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Wed, 13 Apr 2022 18:44:11 +0200 Subject: [PATCH 03/10] Add StorageClass Parameters for Synology --- docs/storage-class-parameters.md | 100 +++++++++++ examples/synology-iscsi.yaml | 69 +------- src/driver/controller-synology/http/index.js | 6 +- src/driver/controller-synology/index.js | 170 ++++++++++++++++++- 4 files changed, 274 insertions(+), 71 deletions(-) create mode 100644 docs/storage-class-parameters.md diff --git a/docs/storage-class-parameters.md b/docs/storage-class-parameters.md new file mode 100644 index 0000000..d2a1684 --- /dev/null +++ b/docs/storage-class-parameters.md @@ -0,0 +1,100 @@ +# Storage Class Parameters + +Some drivers support different settings for volumes. These can be configured via the driver configuration and/or storage classes. + +## `synology-iscsi` +The `synology-iscsi` driver supports several storage class parameters. Note however that not all parameters/values are supported for all backing file systems and LUN type. The following options are available: + +### Configure Storage Classes +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: synology-iscsi +parameters: + fsType: ext4 + # The following options affect the LUN representing the volume + volume: /volume2 # Optional. Override the volume on which the LUN will be created. + lunType: BLUN # Btrfs thin provisioning + lunType: BLUN_THICK # Btrfs thick provisioning + lunType: THIN # Ext4 thin provisioning + lunType: ADV # Ext4 thin provisioning with legacy advanced feature set + lunType: FILE # Ext4 thick provisioning + lunDescription: Some Description + hardwareAssistedZeroing: true + hardwareAssistedLocking: true + hardwareAssistedDataTransfer: true + spaceReclamation: true + allowSnapshots: true + enableFuaWrite: false + enableSyncCache: false + ioPolicy: Buffered # or Direct + # The following options affect the iSCSI target + headerDigenst: false + dataDigest: false + maxSessions: 1 # Note that this option requires a compatible filesystem + maxRecieveSegmentBytes: 262144 + maxSendSegmentBytes: 262144 +... +``` + +About extended features: +- For `BLUN_THICK` volumes only hardware assisted zeroing and locking can be configured. +- For `THIN` volumes none of the extended features can be configured. +- For `ADV` volumes only space reclamation can be configured. +- For `FILE` volumes only hardware assisted locking can be configured. +- `ioPolicy` is only available for thick provisioned volumes. + +### Configure Snapshot Classes +`synology-iscsi` can also configure different parameters on snapshot classes: + +```yaml +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshotClass +metadata: + name: synology-iscsi-snapshot +parameters: + isLocked: true + # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot + consistency: AppConsistent # Or CrashConsistent +... +``` + +### Enabling CHAP Authentication +You can enable CHAP Authentication for `StorageClass`es by supplying an appropriate `StorageClass` secret (see the [documentation](https://kubernetes-csi.github.io/docs/secrets-and-credentials-storage-class.html) for more details). You can use the same password for alle volumes of a `StorageClass` or use different passwords per volume. + +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: synology-iscsi-chap +parameters: + fsType: ext4 + lunType: BLUN + lunDescription: iSCSI volumes with CHAP Authentication +secrets: + # Use this to configure a single set of credentials for all volumes of this StorageClass + csi.storage.k8s.io/provisioner-secret-name: chap-secret + csi.storage.k8s.io/provisioner-secret-namespace: default + # Use substitutions to use different credentials for volumes based on the PVC + csi.storage.k8s.io/provisioner-secret-name: "${pvc.name}-chap-secret" + csi.storage.k8s.io/provisioner-secret-namespace: "${pvc.namespace}" +... +--- +# Use a secret like this to supply CHAP credentials. +apiVersion: v1 +kind: Secret +metadata: + name: chap-secret +stringData: + # Client Credentials + user: client + password: MySecretPassword + # Mutual CHAP Credentials. If these are specified mutual CHAP will be enabled. + mutualUser: server + password: MyOtherPassword +``` + +Note that CHAP authentication will only be enabled if the secret is correctly configured. If e.g. a password is missing CHAP authentication will not be enabled (but the volume will still be created). You cannot automatically enable/disable CHAP or change the password after the volume has been created. + +If the secret itself is referenced but not present, the volume will not be created. diff --git a/examples/synology-iscsi.yaml b/examples/synology-iscsi.yaml index b8cd825..64629cb 100644 --- a/examples/synology-iscsi.yaml +++ b/examples/synology-iscsi.yaml @@ -10,9 +10,10 @@ httpConnection: session: "democratic-csi" serialize: true -synology: - # choose the proper volume for your system - volume: /volume1 +# choose the default volume for your system. The default value is /volume1. +# This can also be overridden by StorageClasses +# synology: +# volume: /volume1 iscsi: targetPortal: "server[:port]" @@ -27,63 +28,5 @@ iscsi: # full iqn limit is 223 bytes, plan accordingly namePrefix: "" nameSuffix: "" - - # documented below are several blocks - # pick the option appropriate for you based on what your backing fs is and desired features - # you do not need to alter dev_attribs under normal circumstances but they may be altered in advanced use-cases - 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 + # LUN options and CHAP authentication can be configured using StorageClasses. + # See https://github.com/democratic-csi/democratic-csi/blob/master/docs/storage-class-parameters.md diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index f25e3b3..2651f79 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -612,16 +612,20 @@ class SynologyHttpClient { ); } - async CreateClonedVolume(src_lun_uuid, dst_lun_name) { + async CreateClonedVolume(src_lun_uuid, dst_lun_name, dst_location, description) { const create_cloned_volume = { api: "SYNO.Core.ISCSI.LUN", version: 1, method: "clone", src_lun_uuid: JSON.stringify(src_lun_uuid), // src lun uuid dst_lun_name: dst_lun_name, // dst lun name + dst_location: dst_location, is_same_pool: true, // always true? string? clone_type: "democratic-csi", // check }; + if (description) { + create_cloned_volume.description = description; + } return await this.do_request("GET", "entry.cgi", create_cloned_volume); } diff --git a/src/driver/controller-synology/index.js b/src/driver/controller-synology/index.js index 6db629f..f00a0de 100644 --- a/src/driver/controller-synology/index.js +++ b/src/driver/controller-synology/index.js @@ -142,6 +142,24 @@ class ControllerSynologyDriver extends CsiBaseDriver { } } + /** + * Parses a boolean value (e.g. from the value of a parameter. This recognizes + * strings containing boolean literals as well as the numbers 1 and 0. + * + * @param {String} value - The value to be parsed. + * @returns {boolean} The parsed boolean value. + */ + parseBoolean(value) { + if (value === undefined) { + return undefined; + } + const parsed = parseInt(value) + if (!isNaN(parsed)) { + return Boolean(parsed) + } + return "true".localeCompare(value, undefined, {sensitivity: "accent"}) === 0 + } + buildIscsiName(name) { let iscsiName = name; if (this.options.iscsi.namePrefix) { @@ -155,6 +173,25 @@ class ControllerSynologyDriver extends CsiBaseDriver { return iscsiName.toLowerCase(); } + /** + * Returns the value for the 'location' parameter indicating on which volume + * a LUN is to be created. + * + * @param {Object} parameters - Parameters received from a StorageClass + * @param {String} parameters.volume - The volume specified by the StorageClass + * @returns {String} The location of the volume. + */ + getLocation({volume}) { + let location = volume ?? this.options?.synology?.volume + if (location === undefined) { + location = "volume1" + } + if (!location.startsWith('/')) { + location = "/" + location + } + return location + } + assertCapabilities(capabilities) { const driverResourceType = this.getDriverResourceType(); this.ctx.logger.verbose("validating capabilities: %j", capabilities); @@ -310,6 +347,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { } let volume_context = {}; + const normalizedParameters = driver.getNormalizedParameters(call.request.parameters); switch (driver.getDriverShareType()) { case "nfs": // TODO: create volume here @@ -425,7 +463,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { `invalid volume_id: ${volume_content_source.volume.volume_id}` ); } - await httpClient.CreateClonedVolume(src_lun_uuid, iscsiName); + await httpClient.CreateClonedVolume(src_lun_uuid, iscsiName, driver.getLocation(normalizedParameters)); } break; default: @@ -446,9 +484,59 @@ class ControllerSynologyDriver extends CsiBaseDriver { // create lun data = Object.assign({}, driver.options.iscsi.lunTemplate, { name: iscsiName, - location: driver.options.synology.volume, - size: capacity_bytes, + location: driver.getLocation(normalizedParameters), + size: capacity_bytes }); + data.type = normalizedParameters.lunType ?? data.type; + if ('lunDescription' in normalizedParameters) { + data.description = normalizedParameters.lunDescription; + } + if (normalizedParameters.ioPolicy === "Direct") { + data.direct_io_pattern = 3; + } else if (normalizedParameters.ioPolicy === "Buffered") { + data.direct_io_pattern = 0; + } else if (normalizedParameters.ioPolicy !== undefined) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `snapshot consistency must be either CrashConsistent or AppConsistent` + ); + } + + const dev_attribs = (data.dev_attribs ?? []).reduce( + (obj, item) => Object.assign(obj, {[item.dev_attrib]: driver.parseBoolean(item.enable)}), {} + ); + dev_attribs.emulate_tpws = driver.parseBoolean(normalizedParameters.hardwareAssistedZeroing) ?? dev_attribs.emulate_tpws; + dev_attribs.emulate_caw = driver.parseBoolean(normalizedParameters.hardwareAssistedLocking) ?? dev_attribs.emulate_caw; + dev_attribs.emulate_3pc = driver.parseBoolean(normalizedParameters.hardwareAssistedDataTransfer) ?? dev_attribs.emulate_3pc; + dev_attribs.emulate_tpu = driver.parseBoolean(normalizedParameters.spaceReclamation) ?? dev_attribs.emulate_tpu; + dev_attribs.emulate_fua_write = driver.parseBoolean(normalizedParameters.enableFuaWrite) ?? dev_attribs.emulate_fua_write; + dev_attribs.emulate_sync_cache = driver.parseBoolean(normalizedParameters.enableSyncCache) ?? dev_attribs.emulate_sync_cache; + dev_attribs.can_snapshot = driver.parseBoolean(normalizedParameters.allowSnapshots) ?? dev_attribs.can_snapshot; + data.dev_attribs = Object.entries(dev_attribs).filter( + e => e[1] !== undefined + ).map( + e => ({dev_attrib: e[0], enable: Number(e[1])}) + ); + + if (["BLUN", "THIN", "ADV"].includes(data.type) && 'direct_io_pattern' in data) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `ioPolicy can only be used with thick provisioning.` + ); + } + if (["BLUN_THICK", "FILE"].includes(data.type) && dev_attribs.emulate_tpu) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `spaceReclamation can only be used with thin provisioning.` + ); + } + if (["BLUN_THICK", "FILE"].includes(data.type) && dev_attribs.can_snapshot) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `allowSnapshots can only be used with thin provisioning.` + ); + } + lun_uuid = await httpClient.CreateLun(data); } @@ -458,6 +546,60 @@ class ControllerSynologyDriver extends CsiBaseDriver { name: iscsiName, iqn, }); + if ('headerChecksum' in normalizedParameters) { + data.has_data_checksum = normalizedParameters['headerChecksum']; + } + if ('dataChecksum' in normalizedParameters) { + data.has_data_checksum = normalizedParameters['dataChecksum']; + } + if ('maxSessions' in normalizedParameters) { + data.max_sessions = Number(normalizedParameters['maxSessions']); + if (isNaN(data.max_sessions)) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `maxSessions must be a number.` + ); + } + } + if (!('multi_sessions' in data) && 'max_sessions' in data) { + data.multi_sessions = data.max_sessions == 1; + } + if ('maxReceiveSegmentBytes' in normalizedParameters) { + data.max_recv_seg_bytes = Number(normalizedParameters['maxReceiveSegmentBytes']); + if (isNaN(data.max_recv_seg_bytes)) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `maxReceiveSegmentBytes must be a number.` + ); + } + } + if ('maxSendSegmentBytes' in normalizedParameters) { + data.max_send_seg_bytes = Number(normalizedParameters['maxSendSegmentBytes']); + if (isNaN(data.max_send_seg_bytes)) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `maxSendSegmentBytes must be a number.` + ); + } + } + + 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; + } + } else { + data.auth_type ??= 0; + data.chap ??= false; + } let target_id = await httpClient.CreateTarget(data); //target = await httpClient.GetTargetByTargetID(target_id); target = await httpClient.GetTargetByIQN(iqn); @@ -737,8 +879,10 @@ class ControllerSynologyDriver extends CsiBaseDriver { async GetCapacity(call) { const driver = this; const httpClient = await driver.getHttpClient(); + const normalizedParameters = driver.getNormalizedParameters(call.request.parameters) + const location = driver.getLocation(normalizedParameters); - if (!driver.options.synology.volume) { + if (!location) { throw new GrpcError( grpc.status.FAILED_PRECONDITION, `invalid configuration: missing volume` @@ -753,9 +897,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { } } - let response = await httpClient.GetVolumeInfo( - driver.options.synology.volume - ); + let response = await httpClient.GetVolumeInfo(location); return { available_capacity: response.body.data.volume.size_free_byte }; } @@ -850,11 +992,25 @@ class ControllerSynologyDriver extends CsiBaseDriver { let snapshot; snapshot = await httpClient.GetSnapshotByLunIDAndName(lun.lun_id, name); if (!snapshot) { + const normalizedParameters = driver.getNormalizedParameters(call.request.parameters); let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, { src_lun_uuid: lun.uuid, taken_by: "democratic-csi", description: name, //check }); + if ('isLocked' in normalizedParameters) { + data['is_locked'] = driver.parseBoolean(normalizedParameters.isLocked); + } + if (normalizedParameters.consistency === "AppConsistent") { + data['is_app_consistent'] = true; + } else if (normalizedParameters.consistency === 'CrashConsistent') { + data['is_app_consistent'] = false; + } else if ('consistency' in normalizedParameters.consistency) { + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `snapshot consistency must be either CrashConsistent or AppConsistent` + ); + } await httpClient.CreateSnapshot(data); snapshot = await httpClient.GetSnapshotByLunIDAndName(lun.lun_id, name); From 4de638b596cf91bb3dd10ff055bcbad71fe144cc Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Fri, 15 Apr 2022 23:24:18 +0200 Subject: [PATCH 04/10] Fix Snapshots for DSM 7 --- README.md | 2 +- src/driver/controller-synology/http/index.js | 26 +++++++------- src/driver/controller-synology/index.js | 37 ++++++++++++-------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index f934689..dc8d43b 100644 --- a/README.md +++ b/README.md @@ -283,7 +283,7 @@ Ensure ssh and zfs is installed on the nfs/iscsi server and that you have instal ### Synology (synology-iscsi) -Ensure iscsi manager has been installed and is generally setup/configured. +Ensure iscsi manager has been installed and is generally setup/configured. DSM 6.3+ is supported. ## Helm Installation diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index 2651f79..0412924 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -333,19 +333,19 @@ class SynologyHttpClient { return snapshots; } - async GetSnapshotByLunIDAndName(lun_id, name) { + async GetSnapshotByLunUUIDAndName(lun_uuid, name) { const get_snapshot_info = { - lid: lun_id, //check? - api: "SYNO.Core.Storage.iSCSILUN", - method: "load_snapshot", + api: "SYNO.Core.ISCSI.LUN", + method: "list_snapshot", version: 1, + src_lun_uuid: JSON.stringify(lun_uuid), }; let response = await this.do_request("GET", "entry.cgi", get_snapshot_info); - if (response.body.data) { - let snapshot = response.body.data.find((i) => { - return i.desc == name; + if (response.body.data.snapshots) { + let snapshot = response.body.data.snapshots.find((i) => { + return i.description == name; }); if (snapshot) { @@ -354,18 +354,18 @@ class SynologyHttpClient { } } - async GetSnapshotByLunIDAndSnapshotUUID(lun_id, snapshot_uuid) { + async GetSnapshotByLunUUIDAndSnapshotUUID(lun_uuid, snapshot_uuid) { const get_snapshot_info = { - lid: lun_id, //check? - api: "SYNO.Core.Storage.iSCSILUN", - method: "load_snapshot", + api: "SYNO.Core.ISCSI.LUN", + method: "list_snapshot", version: 1, + src_lun_uuid: JSON.stringify(lun_uuid), }; let response = await this.do_request("GET", "entry.cgi", get_snapshot_info); - if (response.body.data) { - let snapshot = response.body.data.find((i) => { + if (response.body.data.snapshots) { + let snapshot = response.body.data.snapshots.find((i) => { return i.uuid == snapshot_uuid; }); diff --git a/src/driver/controller-synology/index.js b/src/driver/controller-synology/index.js index f00a0de..dead5dd 100644 --- a/src/driver/controller-synology/index.js +++ b/src/driver/controller-synology/index.js @@ -399,13 +399,12 @@ class ControllerSynologyDriver extends CsiBaseDriver { if (volume_content_source) { let src_lun_uuid; - let src_lun_id; switch (volume_content_source.type) { case "snapshot": let parts = volume_content_source.snapshot.snapshot_id.split("/"); - src_lun_id = parts[2]; - if (!src_lun_id) { + src_lun_uuid = parts[2]; + if (!src_lun_uuid) { throw new GrpcError( grpc.status.NOT_FOUND, `invalid snapshot_id: ${volume_content_source.snapshot.snapshot_id}` @@ -420,11 +419,14 @@ class ControllerSynologyDriver extends CsiBaseDriver { ); } - let src_lun = await httpClient.GetLunByID(src_lun_id); - src_lun_uuid = src_lun.uuid; + // This is for backwards compatibility. Previous versions of this driver used the LUN ID instead of the + // UUID. If this is the case we need to get the LUN UUID before we can proceed. + if (!src_lun_uuid.includes("-")) { + src_lun_uuid = await httpClient.GetLunByID(src_lun_uuid).uuid; + } - let snapshot = await httpClient.GetSnapshotByLunIDAndSnapshotUUID( - src_lun_id, + let snapshot = await httpClient.GetSnapshotByLunUUIDAndSnapshotUUID( + src_lun_uuid, snapshot_uuid ); if (!snapshot) { @@ -990,7 +992,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { // check for already exists let snapshot; - snapshot = await httpClient.GetSnapshotByLunIDAndName(lun.lun_id, name); + snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name); if (!snapshot) { const normalizedParameters = driver.getNormalizedParameters(call.request.parameters); let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, { @@ -1013,7 +1015,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { } await httpClient.CreateSnapshot(data); - snapshot = await httpClient.GetSnapshotByLunIDAndName(lun.lun_id, name); + snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name); if (!snapshot) { throw new Error(`failed to create snapshot`); @@ -1027,7 +1029,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { * is needed to create a volume from this snapshot. */ size_bytes: snapshot.total_size, - snapshot_id: `/lun/${lun.lun_id}/${snapshot.uuid}`, // add shanpshot_uuid //fixme + snapshot_id: `/lun/${lun.uuid}/${snapshot.uuid}`, source_volume_id: source_volume_id, //https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/timestamp.proto creation_time: { @@ -1064,8 +1066,8 @@ class ControllerSynologyDriver extends CsiBaseDriver { } let parts = snapshot_id.split("/"); - let lun_id = parts[2]; - if (!lun_id) { + let lun_uuid = parts[2]; + if (!lun_uuid) { return {}; } @@ -1074,9 +1076,14 @@ class ControllerSynologyDriver extends CsiBaseDriver { return {}; } - // TODO: delete snapshot - let snapshot = await httpClient.GetSnapshotByLunIDAndSnapshotUUID( - lun_id, + // This is for backwards compatibility. Previous versions of this driver used the LUN ID instead of the UUID. If + // this is the case we need to get the LUN UUID before we can proceed. + if (!lun_uuid.includes("-")) { + lun_uuid = await httpClient.GetLunByID(lun_uuid).uuid; + } + + let snapshot = await httpClient.GetSnapshotByLunUUIDAndSnapshotUUID( + lun_uuid, snapshot_uuid ); From 20d4b3a7a3e3b51abcd123b35ee87d83a219aaad Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Fri, 15 Apr 2022 23:40:40 +0200 Subject: [PATCH 05/10] Update docs --- docs/storage-class-parameters.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/storage-class-parameters.md b/docs/storage-class-parameters.md index d2a1684..e7e6ba9 100644 --- a/docs/storage-class-parameters.md +++ b/docs/storage-class-parameters.md @@ -1,9 +1,11 @@ # Storage Class Parameters -Some drivers support different settings for volumes. These can be configured via the driver configuration and/or storage classes. +Some drivers support different settings for volumes. These can be configured via the driver configuration and/or storage +classes. ## `synology-iscsi` -The `synology-iscsi` driver supports several storage class parameters. Note however that not all parameters/values are supported for all backing file systems and LUN type. The following options are available: +The `synology-iscsi` driver supports several storage class parameters. Note however that not all parameters/values are +supported for all backing file systems and LUN type. The following options are available: ### Configure Storage Classes ```yaml @@ -56,12 +58,16 @@ metadata: parameters: isLocked: true # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot + # Note that AppConsistent snapshots require a working Synology Storage Console. Otherwise both values will have + # equivalent behavior. consistency: AppConsistent # Or CrashConsistent ... ``` ### Enabling CHAP Authentication -You can enable CHAP Authentication for `StorageClass`es by supplying an appropriate `StorageClass` secret (see the [documentation](https://kubernetes-csi.github.io/docs/secrets-and-credentials-storage-class.html) for more details). You can use the same password for alle volumes of a `StorageClass` or use different passwords per volume. +You can enable CHAP Authentication for `StorageClass`es by supplying an appropriate `StorageClass` secret (see the +[documentation](https://kubernetes-csi.github.io/docs/secrets-and-credentials-storage-class.html) for more details). You +can use the same password for alle volumes of a `StorageClass` or use different passwords per volume. ```yaml apiVersion: storage.k8s.io/v1 @@ -95,6 +101,8 @@ stringData: password: MyOtherPassword ``` -Note that CHAP authentication will only be enabled if the secret is correctly configured. If e.g. a password is missing CHAP authentication will not be enabled (but the volume will still be created). You cannot automatically enable/disable CHAP or change the password after the volume has been created. +Note that CHAP authentication will only be enabled if the secret contains a username and password. If e.g. a password is +missing CHAP authentication will not be enabled (but the volume will still be created). You cannot automatically +enable/disable CHAP or change the password after the volume has been created. If the secret itself is referenced but not present, the volume will not be created. From c8f50f3c6be6ddaf9c528ed243dea16f56f6bebc Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Sat, 16 Apr 2022 00:05:49 +0200 Subject: [PATCH 06/10] Consolidate Synology API errors --- src/driver/controller-synology/http/index.js | 37 +++++++------------- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index 0412924..cbb2c08 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -9,28 +9,16 @@ const { GrpcError, grpc } = require("../../../utils/grpc"); const USER_AGENT = "democratic-csi"; const __REGISTRY_NS__ = "SynologyHttpClient"; -SYNO_ERROR_MESSAGES = { - 18990002: "The synology volume is out of disk space.", - 18990538: "A LUN with this name already exists.", - 18990541: "The maximum number of LUNS has been reached.", - 18990542: "The maximum number if iSCSI target has been reached.", - 18990744: "An iSCSI target with this name already exists.", - 18990532: "No such snapshot.", - 18990500: "Bad LUN type", - 18990543: "Maximum number of snapshots reached.", - 18990635: "Invalid ioPolicy." -} - -SYNO_GRPC_CODES = { - 18990002: grpc.status.RESOURCE_EXHAUSTED, - 18990538: grpc.status.ALREADY_EXISTS, - 18990541: grpc.status.RESOURCE_EXHAUSTED, - 18990542: grpc.status.RESOURCE_EXHAUSTED, - 18990744: grpc.status.ALREADY_EXISTS, - 18990532: grpc.status.NOT_FOUND, - 18990500: grpc.status.INVALID_ARGUMENT, - 18990543: grpc.status.RESOURCE_EXHAUSTED, - 18990635: grpc.status.INVALID_ARGUMENT +SYNO_ERRORS = { + 18990002: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The synology volume is out of disk space." }, + 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." }, + 18990635: { status: grpc.status.INVALID_ARGUMENT, message: "Invalid ioPolicy." } } class SynologyError extends GrpcError { @@ -39,8 +27,9 @@ class SynologyError extends GrpcError { this.synoCode = code; this.httpCode = httpCode; if (code > 0) { - this.code = SYNO_GRPC_CODES[code] ?? grpc.status.UNKNOWN; - this.message = SYNO_ERROR_MESSAGES[code] ?? `An unknown error occurred when executing a synology command (code = ${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}).`; } else { this.code = grpc.status.UNKNOWN; this.message = `The synology webserver returned a status code ${httpCode}`; From 8dbe45a789fe0abd814f97a77b147668f115b06c Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Sat, 16 Apr 2022 00:12:09 +0200 Subject: [PATCH 07/10] Fix null value --- src/driver/controller-synology/http/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index cbb2c08..328b5bd 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -178,7 +178,7 @@ class SynologyHttpClient { } if (response.statusCode > 299 || response.statusCode < 200) { - reject(new SynologyError(-1, response.statusCode)) + reject(new SynologyError(null, response.statusCode)) } if (response.body.success === false) { From 7f998ebec2f1be2ae5024f610c76081ae77f10a3 Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Sat, 16 Apr 2022 15:56:39 +0200 Subject: [PATCH 08/10] Default Values & Code Style --- docs/storage-class-parameters.md | 3 +++ src/driver/controller-synology/http/index.js | 12 +++++++-- src/driver/controller-synology/index.js | 28 ++++++++++++-------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/docs/storage-class-parameters.md b/docs/storage-class-parameters.md index e7e6ba9..da50195 100644 --- a/docs/storage-class-parameters.md +++ b/docs/storage-class-parameters.md @@ -64,6 +64,9 @@ parameters: ... ``` +Note that it is currently not supported by Synology devices to restore a snapshot onto a different volume. You can +create volumes from snapshots, but you should use the same `StorageClass` as the original volume of the snapshot did. + ### Enabling CHAP Authentication You can enable CHAP Authentication for `StorageClass`es by supplying an appropriate `StorageClass` secret (see the [documentation](https://kubernetes-csi.github.io/docs/secrets-and-credentials-storage-class.html) for more details). You diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index 328b5bd..03d6013 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -39,7 +39,12 @@ class SynologyError extends GrpcError { class SynologyHttpClient { constructor(options = {}) { - this.options = JSON.parse(JSON.stringify(options)); + this.options = Object.assign({ + protocol: "https", + port: 5001, + allowInsecure: false, + session: "democratic-csi" + }, JSON.parse(JSON.stringify(options))); this.logger = console; this.doLoginMutex = new Mutex(); this.apiSerializeMutex = new Mutex(); @@ -618,7 +623,7 @@ class SynologyHttpClient { return await this.do_request("GET", "entry.cgi", create_cloned_volume); } - async CreateVolumeFromSnapshot(src_lun_uuid, snapshot_uuid, cloned_lun_name) { + async CreateVolumeFromSnapshot(src_lun_uuid, snapshot_uuid, cloned_lun_name, description) { const create_volume_from_snapshot = { api: "SYNO.Core.ISCSI.LUN", version: 1, @@ -628,6 +633,9 @@ class SynologyHttpClient { cloned_lun_name: cloned_lun_name, // cloned lun name clone_type: "democratic-csi", // check }; + if (description) { + create_volume_from_snapshot.description = description; + } return await this.do_request( "GET", "entry.cgi", diff --git a/src/driver/controller-synology/index.js b/src/driver/controller-synology/index.js index dead5dd..6670422 100644 --- a/src/driver/controller-synology/index.js +++ b/src/driver/controller-synology/index.js @@ -441,7 +441,8 @@ class ControllerSynologyDriver extends CsiBaseDriver { await httpClient.CreateVolumeFromSnapshot( src_lun_uuid, snapshot_uuid, - iscsiName + iscsiName, + normalizedParameters.description ); } break; @@ -465,7 +466,12 @@ class ControllerSynologyDriver extends CsiBaseDriver { `invalid volume_id: ${volume_content_source.volume.volume_id}` ); } - await httpClient.CreateClonedVolume(src_lun_uuid, iscsiName, driver.getLocation(normalizedParameters)); + await httpClient.CreateClonedVolume( + src_lun_uuid, + iscsiName, + driver.getLocation(normalizedParameters), + normalizedParameters.description + ); } break; default: @@ -549,13 +555,13 @@ class ControllerSynologyDriver extends CsiBaseDriver { iqn, }); if ('headerChecksum' in normalizedParameters) { - data.has_data_checksum = normalizedParameters['headerChecksum']; + data.has_data_checksum = normalizedParameters.headerChecksum; } if ('dataChecksum' in normalizedParameters) { - data.has_data_checksum = normalizedParameters['dataChecksum']; + data.has_data_checksum = normalizedParameters.dataChecksum; } if ('maxSessions' in normalizedParameters) { - data.max_sessions = Number(normalizedParameters['maxSessions']); + data.max_sessions = Number(normalizedParameters.maxSessions); if (isNaN(data.max_sessions)) { throw new GrpcError( grpc.status.INVALID_ARGUMENT, @@ -567,7 +573,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { data.multi_sessions = data.max_sessions == 1; } if ('maxReceiveSegmentBytes' in normalizedParameters) { - data.max_recv_seg_bytes = Number(normalizedParameters['maxReceiveSegmentBytes']); + data.max_recv_seg_bytes = Number(normalizedParameters.maxReceiveSegmentBytes); if (isNaN(data.max_recv_seg_bytes)) { throw new GrpcError( grpc.status.INVALID_ARGUMENT, @@ -576,7 +582,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { } } if ('maxSendSegmentBytes' in normalizedParameters) { - data.max_send_seg_bytes = Number(normalizedParameters['maxSendSegmentBytes']); + data.max_send_seg_bytes = Number(normalizedParameters.maxSendSegmentBytes); if (isNaN(data.max_send_seg_bytes)) { throw new GrpcError( grpc.status.INVALID_ARGUMENT, @@ -588,7 +594,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { 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; + 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; @@ -1001,12 +1007,12 @@ class ControllerSynologyDriver extends CsiBaseDriver { description: name, //check }); if ('isLocked' in normalizedParameters) { - data['is_locked'] = driver.parseBoolean(normalizedParameters.isLocked); + data.is_locked = driver.parseBoolean(normalizedParameters.isLocked); } if (normalizedParameters.consistency === "AppConsistent") { - data['is_app_consistent'] = true; + data.is_app_consistent = true; } else if (normalizedParameters.consistency === 'CrashConsistent') { - data['is_app_consistent'] = false; + data.is_app_consistent = false; } else if ('consistency' in normalizedParameters.consistency) { throw new GrpcError( grpc.status.INVALID_ARGUMENT, From 7b4d6a9a059ae4b70207f267ec6e72c0eb68c67a Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Sat, 16 Apr 2022 17:01:59 +0200 Subject: [PATCH 09/10] Custom templates, remove volume from StorageClass --- docs/storage-class-parameters.md | 17 ++++++++++++++--- examples/synology-iscsi.yaml | 3 +-- src/driver/controller-synology/http/index.js | 7 +------ src/driver/controller-synology/index.js | 18 +++++++++--------- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/docs/storage-class-parameters.md b/docs/storage-class-parameters.md index da50195..b9a6bd9 100644 --- a/docs/storage-class-parameters.md +++ b/docs/storage-class-parameters.md @@ -16,7 +16,6 @@ metadata: parameters: fsType: ext4 # The following options affect the LUN representing the volume - volume: /volume2 # Optional. Override the volume on which the LUN will be created. lunType: BLUN # Btrfs thin provisioning lunType: BLUN_THICK # Btrfs thick provisioning lunType: THIN # Ext4 thin provisioning @@ -34,10 +33,18 @@ parameters: # The following options affect the iSCSI target headerDigenst: false dataDigest: false - maxSessions: 1 # Note that this option requires a compatible filesystem + maxSessions: 1 # Note that this option requires a compatible filesystem. Use 0 for unlimited sessions (default). maxRecieveSegmentBytes: 262144 maxSendSegmentBytes: 262144 -... + lunTemplate: | + # This inline yaml object will be passed to the Synology API when creating the LUN. Use this for custom options. + dev_attribs: + - dev_attrib: emulate_caw + enable: 1 + targetTemplate: | + # This inline yaml object will be passed to the Synology API when creating the target. Use this for custom + # options. + max_sessions: 0 ``` About extended features: @@ -61,6 +68,10 @@ parameters: # Note that AppConsistent snapshots require a working Synology Storage Console. Otherwise both values will have # equivalent behavior. consistency: AppConsistent # Or CrashConsistent + lunSnapshotTemplate: | + # This inline yaml object will be passed to the Synology API when creating the snapshot. Use this for custom + # options. + is_locked: true ... ``` diff --git a/examples/synology-iscsi.yaml b/examples/synology-iscsi.yaml index 64629cb..0fba420 100644 --- a/examples/synology-iscsi.yaml +++ b/examples/synology-iscsi.yaml @@ -10,8 +10,7 @@ httpConnection: session: "democratic-csi" serialize: true -# choose the default volume for your system. The default value is /volume1. -# This can also be overridden by StorageClasses +# Choose the DSM volume this driver operates on. The default value is /volume1. # synology: # volume: /volume1 diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index 03d6013..9e00362 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -39,12 +39,7 @@ class SynologyError extends GrpcError { class SynologyHttpClient { constructor(options = {}) { - this.options = Object.assign({ - protocol: "https", - port: 5001, - allowInsecure: false, - session: "democratic-csi" - }, JSON.parse(JSON.stringify(options))); + this.options = JSON.parse(JSON.stringify(options)); this.logger = console; this.doLoginMutex = new Mutex(); this.apiSerializeMutex = new Mutex(); diff --git a/src/driver/controller-synology/index.js b/src/driver/controller-synology/index.js index 6670422..e1da1bf 100644 --- a/src/driver/controller-synology/index.js +++ b/src/driver/controller-synology/index.js @@ -4,6 +4,7 @@ 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 __REGISTRY_NS__ = "ControllerSynologyDriver"; @@ -181,8 +182,8 @@ class ControllerSynologyDriver extends CsiBaseDriver { * @param {String} parameters.volume - The volume specified by the StorageClass * @returns {String} The location of the volume. */ - getLocation({volume}) { - let location = volume ?? this.options?.synology?.volume + getLocation() { + let location = this.options?.synology?.volume; if (location === undefined) { location = "volume1" } @@ -469,7 +470,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { await httpClient.CreateClonedVolume( src_lun_uuid, iscsiName, - driver.getLocation(normalizedParameters), + driver.getLocation(), normalizedParameters.description ); } @@ -490,9 +491,9 @@ class ControllerSynologyDriver extends CsiBaseDriver { } } else { // create lun - data = Object.assign({}, driver.options.iscsi.lunTemplate, { + data = Object.assign({}, driver.options.iscsi.lunTemplate, yaml.load(normalizedParameters.lunTemplate), { name: iscsiName, - location: driver.getLocation(normalizedParameters), + location: driver.getLocation(), size: capacity_bytes }); data.type = normalizedParameters.lunType ?? data.type; @@ -550,7 +551,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { // create target let iqn = driver.options.iscsi.baseiqn + iscsiName; - data = Object.assign({}, driver.options.iscsi.targetTemplate, { + data = Object.assign({}, driver.options.iscsi.targetTemplate, yaml.load(normalizedParameters.targetTemplate), { name: iscsiName, iqn, }); @@ -887,8 +888,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { async GetCapacity(call) { const driver = this; const httpClient = await driver.getHttpClient(); - const normalizedParameters = driver.getNormalizedParameters(call.request.parameters) - const location = driver.getLocation(normalizedParameters); + const location = driver.getLocation(); if (!location) { throw new GrpcError( @@ -1001,7 +1001,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name); if (!snapshot) { const normalizedParameters = driver.getNormalizedParameters(call.request.parameters); - let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, { + let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, yaml.load(normalizedParameters.lunSnapshotTemplate), { src_lun_uuid: lun.uuid, taken_by: "democratic-csi", description: name, //check From 75857823aafc6f40330fba7621c51fc83a822459 Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Wed, 20 Apr 2022 17:28:09 +0200 Subject: [PATCH 10/10] Remove validation and custom options --- docs/storage-class-parameters.md | 98 +++++----- examples/synology-iscsi.yaml | 62 ++++++- src/driver/controller-synology/http/index.js | 1 + src/driver/controller-synology/index.js | 180 ++++++------------- 4 files changed, 176 insertions(+), 165 deletions(-) diff --git a/docs/storage-class-parameters.md b/docs/storage-class-parameters.md index b9a6bd9..d71d47f 100644 --- a/docs/storage-class-parameters.md +++ b/docs/storage-class-parameters.md @@ -15,44 +15,60 @@ metadata: name: synology-iscsi parameters: fsType: ext4 - # The following options affect the LUN representing the volume - lunType: BLUN # Btrfs thin provisioning - lunType: BLUN_THICK # Btrfs thick provisioning - lunType: THIN # Ext4 thin provisioning - lunType: ADV # Ext4 thin provisioning with legacy advanced feature set - lunType: FILE # Ext4 thick provisioning - lunDescription: Some Description - hardwareAssistedZeroing: true - hardwareAssistedLocking: true - hardwareAssistedDataTransfer: true - spaceReclamation: true - allowSnapshots: true - enableFuaWrite: false - enableSyncCache: false - ioPolicy: Buffered # or Direct - # The following options affect the iSCSI target - headerDigenst: false - dataDigest: false - maxSessions: 1 # Note that this option requires a compatible filesystem. Use 0 for unlimited sessions (default). - maxRecieveSegmentBytes: 262144 - maxSendSegmentBytes: 262144 + # The following options affect the LUN representing the volume. These options are passed directly to the Synology API. + # The following options are known. lunTemplate: | - # This inline yaml object will be passed to the Synology API when creating the LUN. Use this for custom options. + type: BLUN # Btrfs thin provisioning + type: BLUN_THICK # Btrfs thick provisioning + type: THIN # Ext4 thin provisioning + type: ADV # Ext4 thin provisioning with legacy advanced feature set + type: FILE # Ext4 thick provisioning + description: Some Description + + # Only for thick provisioned volumes. Known values: + # 0: Buffered Writes + # 3: Direct Write + direct_io_pattern: 0 + + # Device Attributes. See below for more info dev_attribs: - - dev_attrib: emulate_caw + - dev_attrib: emulate_tpws enable: 1 + - ... + + # The following options affect the iSCSI target. These options will be passed directly to the Synology API. + # The following options are known. targetTemplate: | - # This inline yaml object will be passed to the Synology API when creating the target. Use this for custom - # options. + has_header_checksum: false + has_data_checksum: false + + # Note that this option requires a compatible filesystem. Use 0 for unlimited sessions. max_sessions: 0 + multi_sessions: true + max_recv_seg_bytes: 262144 + max_send_seg_bytes: 262144 + + # Use this to disable authentication. To configure authentication see below + auth_type: 0 ``` -About extended features: -- For `BLUN_THICK` volumes only hardware assisted zeroing and locking can be configured. -- For `THIN` volumes none of the extended features can be configured. -- For `ADV` volumes only space reclamation can be configured. -- For `FILE` volumes only hardware assisted locking can be configured. -- `ioPolicy` is only available for thick provisioned volumes. +#### About LUN Types +The availability of the different types of LUNs depends on the filesystem used on your Synology volume. For Btrfs volumes +you can use `BLUN` and `BLUN_THICK` volumes. For Ext4 volumes you can use `THIN`, `ADV` or `FILE` volumes. These +correspond to the options available in the UI. + +#### About `dev_attribs` +Most of the LUN options are configured via the `dev_attribs` list. This list can be specified both in the `lunTemplate` +of the global configuration and in the `lunTemplate` of the `StorageClass`. If both lists are present they will be merged +(with the `StorageClass` taking precedence). The following `dev_attribs` are known to work: + +- `emulate_tpws`: Hardware-assisted zeroing +- `emulate_caw`: Hardware-assisted locking +- `emulate_3pc`: Hardware-assisted data transfer +- `emulate_tpu`: Space Reclamation +- `emulate_fua_write`: Enable the FUA iSCSI command (DSM 7+) +- `emulate_sync_cache`: Enable the Sync Cache iSCSI command (DSM 7+) +- `can_snapshot`: Enable snapshots for this volume. Only works for thin provisioned volumes. ### Configure Snapshot Classes `synology-iscsi` can also configure different parameters on snapshot classes: @@ -63,15 +79,14 @@ kind: VolumeSnapshotClass metadata: name: synology-iscsi-snapshot parameters: - isLocked: true - # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot - # Note that AppConsistent snapshots require a working Synology Storage Console. Otherwise both values will have - # equivalent behavior. - consistency: AppConsistent # Or CrashConsistent + # This inline yaml object will be passed to the Synology API when creating the snapshot. lunSnapshotTemplate: | - # This inline yaml object will be passed to the Synology API when creating the snapshot. Use this for custom - # options. is_locked: true + + # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot + # Note that app consistent snapshots require a working Synology Storage Console. Otherwise both values will have + # equivalent behavior. + is_app_consistent: true ... ``` @@ -90,8 +105,9 @@ metadata: name: synology-iscsi-chap parameters: fsType: ext4 - lunType: BLUN - lunDescription: iSCSI volumes with CHAP Authentication + lunTemplate: | + type: BLUN + description: iSCSI volumes with CHAP Authentication secrets: # Use this to configure a single set of credentials for all volumes of this StorageClass csi.storage.k8s.io/provisioner-secret-name: chap-secret @@ -112,7 +128,7 @@ stringData: password: MySecretPassword # Mutual CHAP Credentials. If these are specified mutual CHAP will be enabled. mutualUser: server - password: MyOtherPassword + mutualPassword: MyOtherPassword ``` Note that CHAP authentication will only be enabled if the secret contains a username and password. If e.g. a password is diff --git a/examples/synology-iscsi.yaml b/examples/synology-iscsi.yaml index 0fba420..cc8380f 100644 --- a/examples/synology-iscsi.yaml +++ b/examples/synology-iscsi.yaml @@ -27,5 +27,65 @@ iscsi: # full iqn limit is 223 bytes, plan accordingly namePrefix: "" nameSuffix: "" - # LUN options and CHAP authentication can be configured using StorageClasses. + + # documented below are several blocks + # pick the option appropriate for you based on what your backing fs is and desired features + # you do not need to alter dev_attribs under normal circumstances but they may be altered in advanced use-cases + # 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: + # 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 diff --git a/src/driver/controller-synology/http/index.js b/src/driver/controller-synology/http/index.js index 9e00362..00ac3f0 100644 --- a/src/driver/controller-synology/http/index.js +++ b/src/driver/controller-synology/http/index.js @@ -11,6 +11,7 @@ const __REGISTRY_NS__ = "SynologyHttpClient"; SYNO_ERRORS = { 18990002: { 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." }, diff --git a/src/driver/controller-synology/index.js b/src/driver/controller-synology/index.js index e1da1bf..a1e9b96 100644 --- a/src/driver/controller-synology/index.js +++ b/src/driver/controller-synology/index.js @@ -143,22 +143,21 @@ class ControllerSynologyDriver extends CsiBaseDriver { } } - /** - * Parses a boolean value (e.g. from the value of a parameter. This recognizes - * strings containing boolean literals as well as the numbers 1 and 0. - * - * @param {String} value - The value to be parsed. - * @returns {boolean} The parsed boolean value. - */ - parseBoolean(value) { - if (value === undefined) { - return undefined; + getObjectFromDevAttribs(list = []) { + if (!list) { + return {} } - const parsed = parseInt(value) - if (!isNaN(parsed)) { - return Boolean(parsed) - } - return "true".localeCompare(value, undefined, {sensitivity: "accent"}) === 0 + return list.reduce( + (obj, item) => Object.assign(obj, {[item.dev_attrib]: item.enable}), {} + ) + } + + getDevAttribsFromObject(obj, keepNull = false) { + return Object.entries(obj).filter( + e => keepNull || (e[1] != null) + ).map( + e => ({dev_attrib: e[0], enable: e[1]}) + ); } buildIscsiName(name) { @@ -366,6 +365,7 @@ class ControllerSynologyDriver extends CsiBaseDriver { break; case "iscsi": let iscsiName = driver.buildIscsiName(name); + let storageClassTemplate; let data; let target; let lun_mapping; @@ -491,106 +491,47 @@ class ControllerSynologyDriver extends CsiBaseDriver { } } else { // create lun - data = Object.assign({}, driver.options.iscsi.lunTemplate, yaml.load(normalizedParameters.lunTemplate), { - name: iscsiName, - location: driver.getLocation(), - size: capacity_bytes - }); - data.type = normalizedParameters.lunType ?? data.type; - if ('lunDescription' in normalizedParameters) { - data.description = normalizedParameters.lunDescription; - } - if (normalizedParameters.ioPolicy === "Direct") { - data.direct_io_pattern = 3; - } else if (normalizedParameters.ioPolicy === "Buffered") { - data.direct_io_pattern = 0; - } else if (normalizedParameters.ioPolicy !== undefined) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `snapshot consistency must be either CrashConsistent or AppConsistent` - ); - } + try { + 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 + }); - const dev_attribs = (data.dev_attribs ?? []).reduce( - (obj, item) => Object.assign(obj, {[item.dev_attrib]: driver.parseBoolean(item.enable)}), {} - ); - dev_attribs.emulate_tpws = driver.parseBoolean(normalizedParameters.hardwareAssistedZeroing) ?? dev_attribs.emulate_tpws; - dev_attribs.emulate_caw = driver.parseBoolean(normalizedParameters.hardwareAssistedLocking) ?? dev_attribs.emulate_caw; - dev_attribs.emulate_3pc = driver.parseBoolean(normalizedParameters.hardwareAssistedDataTransfer) ?? dev_attribs.emulate_3pc; - dev_attribs.emulate_tpu = driver.parseBoolean(normalizedParameters.spaceReclamation) ?? dev_attribs.emulate_tpu; - dev_attribs.emulate_fua_write = driver.parseBoolean(normalizedParameters.enableFuaWrite) ?? dev_attribs.emulate_fua_write; - dev_attribs.emulate_sync_cache = driver.parseBoolean(normalizedParameters.enableSyncCache) ?? dev_attribs.emulate_sync_cache; - dev_attribs.can_snapshot = driver.parseBoolean(normalizedParameters.allowSnapshots) ?? dev_attribs.can_snapshot; - data.dev_attribs = Object.entries(dev_attribs).filter( - e => e[1] !== undefined - ).map( - e => ({dev_attrib: e[0], enable: Number(e[1])}) - ); - - if (["BLUN", "THIN", "ADV"].includes(data.type) && 'direct_io_pattern' in data) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `ioPolicy can only be used with thick provisioning.` - ); + 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 + } } - if (["BLUN_THICK", "FILE"].includes(data.type) && dev_attribs.emulate_tpu) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `spaceReclamation can only be used with thin provisioning.` - ); - } - if (["BLUN_THICK", "FILE"].includes(data.type) && dev_attribs.can_snapshot) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `allowSnapshots can only be used with thin provisioning.` - ); - } - - lun_uuid = await httpClient.CreateLun(data); } // create target let iqn = driver.options.iscsi.baseiqn + iscsiName; - data = Object.assign({}, driver.options.iscsi.targetTemplate, yaml.load(normalizedParameters.targetTemplate), { + try { + 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 ('headerChecksum' in normalizedParameters) { - data.has_data_checksum = normalizedParameters.headerChecksum; - } - if ('dataChecksum' in normalizedParameters) { - data.has_data_checksum = normalizedParameters.dataChecksum; - } - if ('maxSessions' in normalizedParameters) { - data.max_sessions = Number(normalizedParameters.maxSessions); - if (isNaN(data.max_sessions)) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `maxSessions must be a number.` - ); - } - } - if (!('multi_sessions' in data) && 'max_sessions' in data) { - data.multi_sessions = data.max_sessions == 1; - } - if ('maxReceiveSegmentBytes' in normalizedParameters) { - data.max_recv_seg_bytes = Number(normalizedParameters.maxReceiveSegmentBytes); - if (isNaN(data.max_recv_seg_bytes)) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `maxReceiveSegmentBytes must be a number.` - ); - } - } - if ('maxSendSegmentBytes' in normalizedParameters) { - data.max_send_seg_bytes = Number(normalizedParameters.maxSendSegmentBytes); - if (isNaN(data.max_send_seg_bytes)) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `maxSendSegmentBytes must be a number.` - ); - } - } if ('user' in call.request.secrets && 'password' in call.request.secrets) { data.user = call.request.secrets.user; @@ -605,9 +546,6 @@ class ControllerSynologyDriver extends CsiBaseDriver { data.auth_type = 1; data.mutual_chap = false; } - } else { - data.auth_type ??= 0; - data.chap ??= false; } let target_id = await httpClient.CreateTarget(data); //target = await httpClient.GetTargetByTargetID(target_id); @@ -998,27 +936,23 @@ 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({}, driver.options.iscsi.lunSnapshotTemplate, yaml.load(normalizedParameters.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 }); - if ('isLocked' in normalizedParameters) { - data.is_locked = driver.parseBoolean(normalizedParameters.isLocked); - } - if (normalizedParameters.consistency === "AppConsistent") { - data.is_app_consistent = true; - } else if (normalizedParameters.consistency === 'CrashConsistent') { - data.is_app_consistent = false; - } else if ('consistency' in normalizedParameters.consistency) { - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `snapshot consistency must be either CrashConsistent or AppConsistent` - ); - } await httpClient.CreateSnapshot(data); snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name);