From 75857823aafc6f40330fba7621c51fc83a822459 Mon Sep 17 00:00:00 2001 From: Kim Wittenburg Date: Wed, 20 Apr 2022 17:28:09 +0200 Subject: [PATCH] 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);