From f0ea73fce6a10b20f9f9a56a4926f152e8f6a669 Mon Sep 17 00:00:00 2001 From: Travis Glenn Hansen Date: Sat, 11 Dec 2021 13:46:44 -0700 Subject: [PATCH] more conformant behavior regarding volume/snapshot sizes Signed-off-by: Travis Glenn Hansen --- README.md | 1 + docs/Nomad/examples/volume-iscsi.hcl | 7 +++++ docs/Nomad/examples/volume-nfs.hcl | 4 +++ src/driver/controller-zfs-ssh/index.js | 30 ++++++++++-------- src/driver/freenas/api.js | 43 ++++++++++++++++---------- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 94fdaed..23b4417 100644 --- a/README.md +++ b/README.md @@ -347,6 +347,7 @@ Install `democratic-csi` as usual with `volumeSnapshotClasses` defined as approp - https://kubernetes.io/docs/concepts/storage/volume-snapshots/ - https://github.com/kubernetes-csi/external-snapshotter#usage +- https://github.com/democratic-csi/democratic-csi/issues/129#issuecomment-961489810 # Migrating from freenas-provisioner and freenas-iscsi-provisioner diff --git a/docs/Nomad/examples/volume-iscsi.hcl b/docs/Nomad/examples/volume-iscsi.hcl index 6a25c0c..ac9a9ce 100644 --- a/docs/Nomad/examples/volume-iscsi.hcl +++ b/docs/Nomad/examples/volume-iscsi.hcl @@ -9,3 +9,10 @@ capability { access_mode = "single-node-writer" attachment_mode = "file-system" } + +mount_options { + # ext4|xfs + # default is ext4 when left unset + #fs_type = "ext4" + #mount_flags = ["noatime"] +} diff --git a/docs/Nomad/examples/volume-nfs.hcl b/docs/Nomad/examples/volume-nfs.hcl index 6e54674..21d01b9 100644 --- a/docs/Nomad/examples/volume-nfs.hcl +++ b/docs/Nomad/examples/volume-nfs.hcl @@ -9,3 +9,7 @@ capability { access_mode = "multi-node-multi-writer" attachment_mode = "file-system" } + +mount_options { + mount_flags = ["noatime", "nfsvers=3"] +} diff --git a/src/driver/controller-zfs-ssh/index.js b/src/driver/controller-zfs-ssh/index.js index 7764887..cfa2ccc 100644 --- a/src/driver/controller-zfs-ssh/index.js +++ b/src/driver/controller-zfs-ssh/index.js @@ -571,6 +571,7 @@ class ControllerZfsSshBaseDriver extends CsiBaseDriver { ); } + // if no capacity_range specified set a required_bytes at least if ( !call.request.capacity_range || Object.keys(call.request.capacity_range).length === 0 @@ -592,16 +593,19 @@ class ControllerZfsSshBaseDriver extends CsiBaseDriver { ); } - /** - * NOTE: avoid the urge to templatize this given the name length limits for zvols - * ie: namespace-name may quite easily exceed 58 chars - */ - const datasetName = datasetParentName + "/" + name; let capacity_bytes = call.request.capacity_range.required_bytes || call.request.capacity_range.limit_bytes; - if (capacity_bytes && driverZfsResourceType == "volume") { + if (!capacity_bytes) { + //should never happen, value must be set + throw new GrpcError( + grpc.status.INVALID_ARGUMENT, + `volume capacity is required (either required_bytes or limit_bytes)` + ); + } + + if (capacity_bytes && driverZfsResourceType == "volume") { //make sure to align capacity_bytes with zvol blocksize //volume size must be a multiple of volume block size capacity_bytes = zb.helpers.generateZvolSize( @@ -609,13 +613,7 @@ class ControllerZfsSshBaseDriver extends CsiBaseDriver { zvolBlocksize ); } - if (!capacity_bytes) { - //should never happen, value must be set - throw new GrpcError( - grpc.status.INVALID_ARGUMENT, - `volume capacity is required (either required_bytes or limit_bytes)` - ); - } + // ensure *actual* capacity is not greater than limit if ( @@ -629,6 +627,12 @@ class ControllerZfsSshBaseDriver extends CsiBaseDriver { ); } + /** + * NOTE: avoid the urge to templatize this given the name length limits for zvols + * ie: namespace-name may quite easily exceed 58 chars + */ + const datasetName = datasetParentName + "/" + name; + // ensure volumes with the same name being requested a 2nd time but with a different size fails try { let properties = await zb.zfs.get(datasetName, ["volsize", "refquota"]); diff --git a/src/driver/freenas/api.js b/src/driver/freenas/api.js index 12431b0..b2247da 100644 --- a/src/driver/freenas/api.js +++ b/src/driver/freenas/api.js @@ -2044,6 +2044,7 @@ class FreeNASApiDriver extends CsiBaseDriver { ); } + // if no capacity_range specified set a required_bytes at least if ( !call.request.capacity_range || Object.keys(call.request.capacity_range).length === 0 @@ -2065,23 +2066,10 @@ class FreeNASApiDriver extends CsiBaseDriver { ); } - /** - * NOTE: avoid the urge to templatize this given the name length limits for zvols - * ie: namespace-name may quite easily exceed 58 chars - */ - const datasetName = datasetParentName + "/" + name; let capacity_bytes = call.request.capacity_range.required_bytes || call.request.capacity_range.limit_bytes; - if (capacity_bytes && driverZfsResourceType == "volume") { - //make sure to align capacity_bytes with zvol blocksize - //volume size must be a multiple of volume block size - capacity_bytes = zb.helpers.generateZvolSize( - capacity_bytes, - zvolBlocksize - ); - } if (!capacity_bytes) { //should never happen, value must be set throw new GrpcError( @@ -2091,10 +2079,24 @@ class FreeNASApiDriver extends CsiBaseDriver { } // ensure *actual* capacity is not too small - if (minimum_volume_size > 0 && capacity_bytes < minimum_volume_size) { - throw new GrpcError( - grpc.status.OUT_OF_RANGE, - `volume capacity is smaller than the minimum: ${minimum_volume_size}` + if ( + capacity_bytes > 0 && + minimum_volume_size > 0 && + capacity_bytes < minimum_volume_size + ) { + //throw new GrpcError( + // grpc.status.OUT_OF_RANGE, + // `volume capacity is smaller than the minimum: ${minimum_volume_size}` + //); + capacity_bytes = minimum_volume_size; + } + + if (capacity_bytes && driverZfsResourceType == "volume") { + //make sure to align capacity_bytes with zvol blocksize + //volume size must be a multiple of volume block size + capacity_bytes = zb.helpers.generateZvolSize( + capacity_bytes, + zvolBlocksize ); } @@ -2110,6 +2112,12 @@ class FreeNASApiDriver extends CsiBaseDriver { ); } + /** + * NOTE: avoid the urge to templatize this given the name length limits for zvols + * ie: namespace-name may quite easily exceed 58 chars + */ + const datasetName = datasetParentName + "/" + name; + // ensure volumes with the same name being requested a 2nd time but with a different size fails try { let properties = await httpApiClient.DatasetGet(datasetName, [ @@ -3992,6 +4000,7 @@ class FreeNASApiDriver extends CsiBaseDriver { size_bytes = getLargestNumber( properties.referenced.rawvalue, properties.logicalreferenced.rawvalue + // TODO: perhaps include minimum volume size here? ); } else { // get the size of the parent volume