From bc8ef898112169b15b8a749d1f4f6b4d84284680 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 21 Dec 2021 19:31:13 +0530 Subject: [PATCH 01/30] ci: strict rule for commit body length body-max-line-length is added as a warning even if the commit body is crossing the length limit of 80 its considered as a warning not an error. This commit moves the check from warning to error. Signed-off-by: Madhu Rajanna --- .commitlintrc.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.commitlintrc.yml b/.commitlintrc.yml index b55fcfb1b..f63f4f828 100644 --- a/.commitlintrc.yml +++ b/.commitlintrc.yml @@ -19,7 +19,7 @@ rules: # Body shouldn't be empty body-min-length: [2, always, 1] # Wrap the lines to 80 characters. - body-max-line-length: [1, always, 80] + body-max-line-length: [2, always, 80] # always sign off the commit trailer-exists: [2, always, "Signed-off-by:"] From 6be0e8cb51338f9b8f685c53f1bc3be831a62aac Mon Sep 17 00:00:00 2001 From: Steven Reitsma Date: Tue, 21 Dec 2021 14:54:34 +0100 Subject: [PATCH 02/30] helm: Fix missing ClusterRoleBinding for nodeplugin ServiceAccount When topology is disabled, the ClusterRoleBinding is not created in the Helm chart. However, the nodeplugin needs access to volumeattachments for the volume healer. Signed-off-by: Steven Reitsma --- .../ceph-csi-rbd/templates/nodeplugin-clusterrolebinding.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/charts/ceph-csi-rbd/templates/nodeplugin-clusterrolebinding.yaml b/charts/ceph-csi-rbd/templates/nodeplugin-clusterrolebinding.yaml index fdc79be4a..bf52865e1 100644 --- a/charts/ceph-csi-rbd/templates/nodeplugin-clusterrolebinding.yaml +++ b/charts/ceph-csi-rbd/templates/nodeplugin-clusterrolebinding.yaml @@ -1,5 +1,4 @@ {{- if .Values.rbac.create -}} -{{- if .Values.topology.enabled }} kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: @@ -18,5 +17,4 @@ roleRef: kind: ClusterRole name: {{ include "ceph-csi-rbd.nodeplugin.fullname" . }} apiGroup: rbac.authorization.k8s.io -{{- end }} {{- end -}} From 74965fef41d8e08f18b99b02372d370cccc3b22a Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 22 Dec 2021 08:46:53 +0100 Subject: [PATCH 03/30] deploy: fix default URL for --csi-addons-endpoint option The --csi-addons-endpoint= option has been added recently, but was not configured in the deployment files yet. The socket was incorrectly created as `/csi-addons.sock`, by correcting the URL to the socket, the socket now gets created as `/tmp/csi-addons.sock` in the same directory as other sockets. If an endpoint is a UNIX Domain Socket, the format needs to be `unix:///path/to/socket`. The `unix://` URL format allows an authentication provider to be added directly after the `//`. If there is no authentication provider needed, the field can remain empty. After the authetication provider, the full path needs to be specified, starting with a `/`. This means that URLs to a UDS should start with `unix:///` in normal cases. Signed-off-by: Niels de Vos --- cmd/cephcsi.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cephcsi.go b/cmd/cephcsi.go index 3bbd67770..f419790b0 100644 --- a/cmd/cephcsi.go +++ b/cmd/cephcsi.go @@ -59,7 +59,7 @@ var conf util.Config func init() { // common flags flag.StringVar(&conf.Vtype, "type", "", "driver type [rbd|cephfs|liveness|controller]") - flag.StringVar(&conf.Endpoint, "endpoint", "unix://tmp/csi.sock", "CSI endpoint") + flag.StringVar(&conf.Endpoint, "endpoint", "unix:///tmp/csi.sock", "CSI endpoint") flag.StringVar(&conf.DriverName, "drivername", "", "name of the driver") flag.StringVar(&conf.DriverNamespace, "drivernamespace", defaultNS, "namespace in which driver is deployed") flag.StringVar(&conf.NodeID, "nodeid", "", "node id") @@ -129,7 +129,7 @@ func init() { flag.BoolVar(&conf.EnableProfiling, "enableprofiling", false, "enable go profiling") // CSI-Addons configuration - flag.StringVar(&conf.CSIAddonsEndpoint, "csi-addons-endpoint", "unix://tmp/csi-addons.sock", "CSI-Addons endpoint") + flag.StringVar(&conf.CSIAddonsEndpoint, "csi-addons-endpoint", "unix:///tmp/csi-addons.sock", "CSI-Addons endpoint") klog.InitFlags(nil) if err := flag.Set("logtostderr", "true"); err != nil { From ee2e97b62db842cae0b8ac666c16f87466774608 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 22 Dec 2021 08:09:27 +0100 Subject: [PATCH 04/30] deploy: add CSI-Addons endpoint Deployments place all sockets for communicating with CSI components in the shared `/csi` directory. The CSI-Addons socket was introduced recently, but not configured to be in the same location (by default placed in `/tmp`). Signed-off-by: Niels de Vos --- charts/ceph-csi-rbd/templates/nodeplugin-daemonset.yaml | 3 +++ charts/ceph-csi-rbd/templates/provisioner-deployment.yaml | 3 +++ deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml | 3 +++ deploy/rbd/kubernetes/csi-rbdplugin.yaml | 3 +++ 4 files changed, 12 insertions(+) diff --git a/charts/ceph-csi-rbd/templates/nodeplugin-daemonset.yaml b/charts/ceph-csi-rbd/templates/nodeplugin-daemonset.yaml index 1ccc49d08..422ad0874 100644 --- a/charts/ceph-csi-rbd/templates/nodeplugin-daemonset.yaml +++ b/charts/ceph-csi-rbd/templates/nodeplugin-daemonset.yaml @@ -71,6 +71,7 @@ spec: - "--nodeserver=true" - "--pidlimit=-1" - "--endpoint=$(CSI_ENDPOINT)" + - "--csi-addons-endpoint=$(CSI_ADDONS_ENDPOINT)" - "--v={{ .Values.logLevel }}" - "--drivername=$(DRIVER_NAME)" {{- if .Values.topology.enabled }} @@ -92,6 +93,8 @@ spec: fieldPath: spec.nodeName - name: CSI_ENDPOINT value: "unix:///csi/{{ .Values.pluginSocketFile }}" + - name: CSI_ADDONS_ENDPOINT + value: "unix:///csi/csi-addons.sock" securityContext: privileged: true capabilities: diff --git a/charts/ceph-csi-rbd/templates/provisioner-deployment.yaml b/charts/ceph-csi-rbd/templates/provisioner-deployment.yaml index 5c20f545b..86a04db23 100644 --- a/charts/ceph-csi-rbd/templates/provisioner-deployment.yaml +++ b/charts/ceph-csi-rbd/templates/provisioner-deployment.yaml @@ -137,6 +137,7 @@ spec: - "--controllerserver=true" - "--pidlimit=-1" - "--endpoint=$(CSI_ENDPOINT)" + - "--csi-addons-endpoint=$(CSI_ADDONS_ENDPOINT)" - "--v={{ .Values.logLevel }}" - "--drivername=$(DRIVER_NAME)" - "--rbdhardmaxclonedepth={{ .Values.provisioner.hardMaxCloneDepth }}" @@ -162,6 +163,8 @@ spec: fieldPath: spec.nodeName - name: CSI_ENDPOINT value: "unix:///csi/{{ .Values.provisionerSocketFile }}" + - name: CSI_ADDONS_ENDPOINT + value: "unix:///csi/csi-addons.sock" volumeMounts: - name: socket-dir mountPath: /csi diff --git a/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml b/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml index 034d5f814..d2fd8b231 100644 --- a/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml +++ b/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml @@ -118,6 +118,7 @@ spec: - "--type=rbd" - "--controllerserver=true" - "--endpoint=$(CSI_ENDPOINT)" + - "--csi-addons-endpoint=$(CSI_ADDONS_ENDPOINT)" - "--v=5" - "--drivername=rbd.csi.ceph.com" - "--pidlimit=-1" @@ -141,6 +142,8 @@ spec: # value: encryptionConfig - name: CSI_ENDPOINT value: unix:///csi/csi-provisioner.sock + - name: CSI_ADDONS_ENDPOINT + value: unix:///csi/csi-addons.sock imagePullPolicy: "IfNotPresent" volumeMounts: - name: socket-dir diff --git a/deploy/rbd/kubernetes/csi-rbdplugin.yaml b/deploy/rbd/kubernetes/csi-rbdplugin.yaml index 999444170..a0cb2adfd 100644 --- a/deploy/rbd/kubernetes/csi-rbdplugin.yaml +++ b/deploy/rbd/kubernetes/csi-rbdplugin.yaml @@ -58,6 +58,7 @@ spec: - "--type=rbd" - "--nodeserver=true" - "--endpoint=$(CSI_ENDPOINT)" + - "--csi-addons-endpoint=$(CSI_ADDONS_ENDPOINT)" - "--v=5" - "--drivername=rbd.csi.ceph.com" - "--enableprofiling=false" @@ -83,6 +84,8 @@ spec: # value: encryptionConfig - name: CSI_ENDPOINT value: unix:///csi/csi.sock + - name: CSI_ADDONS_ENDPOINT + value: unix:///csi/csi-addons.sock imagePullPolicy: "IfNotPresent" volumeMounts: - name: socket-dir From 3ca8b1e006cee4bbd72b5fd8ecf93305e3ecadb0 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 22 Dec 2021 08:40:40 +0100 Subject: [PATCH 05/30] doc: add --csi-addons-endpoint option to rbd deployment Signed-off-by: Niels de Vos --- docs/deploy-rbd.md | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/docs/deploy-rbd.md b/docs/deploy-rbd.md index 6e20b5f98..31ec1c030 100644 --- a/docs/deploy-rbd.md +++ b/docs/deploy-rbd.md @@ -28,23 +28,24 @@ make image-cephcsi | Option | Default value | Description | | ------------------------ | --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `--endpoint` | `unix://tmp/csi.sock` | CSI endpoint, must be a UNIX socket | -| `--drivername` | `rbd.csi.ceph.com` | Name of the driver (Kubernetes: `provisioner` field in StorageClass must correspond to this value) | -| `--nodeid` | _empty_ | This node's ID | -| `--type` | _empty_ | Driver type: `[rbd/cephfs]`. If the driver type is set to `rbd` it will act as a `rbd plugin` or if it's set to `cephfs` will act as a `cephfs plugin` | -| `--instanceid` | "default" | Unique ID distinguishing this instance of Ceph CSI among other instances, when sharing Ceph clusters across CSI instances for provisioning | -| `--pidlimit` | _0_ | Configure the PID limit in cgroups. The container runtime can restrict the number of processes/tasks which can cause problems while provisioning (or deleting) a large number of volumes. A value of `-1` configures the limit to the maximum, `0` does not configure limits at all. | -| `--metricsport` | `8080` | TCP port for liveness metrics requests | -| `--metricspath` | `"/metrics"` | Path of prometheus endpoint where metrics will be available | -| `--enablegrpcmetrics` | `false` | [Deprecated] Enable grpc metrics collection and start prometheus server | -| `--polltime` | `"60s"` | Time interval in between each poll | -| `--timeout` | `"3s"` | Probe timeout in seconds | -| `--histogramoption` | `0.5,2,6` | [Deprecated] Histogram option for grpc metrics, should be comma separated value (ex:= "0.5,2,6" where start=0.5 factor=2, count=6) | -| `--domainlabels` | _empty_ | Kubernetes node labels to use as CSI domain labels for topology aware provisioning, should be a comma separated value (ex:= "failure-domain/region,failure-domain/zone") | -| `--rbdhardmaxclonedepth` | `8` | Hard limit for maximum number of nested volume clones that are taken before a flatten occurs | -| `--rbdsoftmaxclonedepth` | `4` | Soft limit for maximum number of nested volume clones that are taken before a flatten occurs | -| `--skipforceflatten` | `false` | skip image flattening on kernel < 5.2 which support mapping of rbd images which has the deep-flatten feature | -| `--maxsnapshotsonimage` | `450` | Maximum number of snapshots allowed on rbd image without flattening | +| `--endpoint` | `unix:///tmp/csi.sock` | CSI endpoint, must be a UNIX socket | +| `--csi-addons-endpoint` | `unix:///tmp/csi-addons.sock` | CSI-Addons endpoint, must be a UNIX socket | +| `--drivername` | `rbd.csi.ceph.com` | Name of the driver (Kubernetes: `provisioner` field in StorageClass must correspond to this value) | +| `--nodeid` | _empty_ | This node's ID | +| `--type` | _empty_ | Driver type: `[rbd/cephfs]`. If the driver type is set to `rbd` it will act as a `rbd plugin` or if it's set to `cephfs` will act as a `cephfs plugin` | +| `--instanceid` | "default" | Unique ID distinguishing this instance of Ceph CSI among other instances, when sharing Ceph clusters across CSI instances for provisioning | +| `--pidlimit` | _0_ | Configure the PID limit in cgroups. The container runtime can restrict the number of processes/tasks which can cause problems while provisioning (or deleting) a large number of volumes. A value of `-1` configures the limit to the maximum, `0` does not configure limits at all. | +| `--metricsport` | `8080` | TCP port for liveness metrics requests | +| `--metricspath` | `"/metrics"` | Path of prometheus endpoint where metrics will be available | +| `--enablegrpcmetrics` | `false` | [Deprecated] Enable grpc metrics collection and start prometheus server | +| `--polltime` | `"60s"` | Time interval in between each poll | +| `--timeout` | `"3s"` | Probe timeout in seconds | +| `--histogramoption` | `0.5,2,6` | [Deprecated] Histogram option for grpc metrics, should be comma separated value (ex:= "0.5,2,6" where start=0.5 factor=2, count=6) | +| `--domainlabels` | _empty_ | Kubernetes node labels to use as CSI domain labels for topology aware provisioning, should be a comma separated value (ex:= "failure-domain/region,failure-domain/zone") | +| `--rbdhardmaxclonedepth` | `8` | Hard limit for maximum number of nested volume clones that are taken before a flatten occurs | +| `--rbdsoftmaxclonedepth` | `4` | Soft limit for maximum number of nested volume clones that are taken before a flatten occurs | +| `--skipforceflatten` | `false` | skip image flattening on kernel < 5.2 which support mapping of rbd images which has the deep-flatten feature | +| `--maxsnapshotsonimage` | `450` | Maximum number of snapshots allowed on rbd image without flattening | **Available volume parameters:** From f3ed883df994be2231826572c030289f5cc2c163 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 22 Dec 2021 19:06:32 +0530 Subject: [PATCH 06/30] ci: use install.sh from golangci-lint repo The golangci-lint install script at goreleaser.com is deprecated. Docs now advise to install from a github link: goreleaser/godownloader#207 Signed-off-by: Madhu Rajanna --- scripts/Dockerfile.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Dockerfile.test b/scripts/Dockerfile.test index e5b000c7f..70cd9f1f9 100644 --- a/scripts/Dockerfile.test +++ b/scripts/Dockerfile.test @@ -46,7 +46,7 @@ RUN source /build.env \ && mkdir -p ${GOROOT} \ && curl https://storage.googleapis.com/golang/go${GOLANG_VERSION}.linux-${GOARCH}.tar.gz \ | tar xzf - -C ${GOROOT} --strip-components=1 \ - && curl -sf "https://install.goreleaser.com/github.com/golangci/golangci-lint.sh" \ + && curl -sf "https://raw.githubusercontent.com/golangci/golangci-lint/${GOLANGCI_VERSION}/install.sh" \ | bash -s -- -b ${GOPATH}/bin "${GOLANGCI_VERSION}" \ && curl -L https://git.io/get_helm.sh | bash -s -- --version "${HELM_VERSION}" \ && mkdir /opt/commitlint && pushd /opt/commitlint \ From b9a8d37c3d5d908f202f91f1507d6e9609ee1344 Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Wed, 22 Dec 2021 11:29:09 +0530 Subject: [PATCH 07/30] rbd: enable expand operation for intree volumes This commit enable the resize operation[1] for in-tree volumes. new helper has been introduced here to aid the enablement or to make it clean with existing code base. [1] https://github.com/ceph/ceph-csi/blob/devel/docs/design/proposals/intree-migrate.md?plain=1#L66 Signed-off-by: Humble Chirammal --- internal/rbd/controllerserver.go | 5 ++--- internal/rbd/migration.go | 36 +++++++++++++++++++------------- internal/rbd/rbd_util.go | 19 +++++++++++++++++ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index da464ad53..87819ab79 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -1447,9 +1447,7 @@ func (cs *ControllerServer) ControllerExpandVolume( return nil, status.Error(codes.InvalidArgument, err.Error()) } defer cr.DeleteCredentials() - - rbdVol, err := GenVolFromVolID(ctx, volID, cr, req.GetSecrets()) - defer rbdVol.Destroy() + rbdVol, err := genVolFromVolIDWithMigration(ctx, volID, cr, req.GetSecrets()) if err != nil { switch { case errors.Is(err, ErrImageNotFound): @@ -1463,6 +1461,7 @@ func (cs *ControllerServer) ControllerExpandVolume( return nil, err } + defer rbdVol.Destroy() // NodeExpansion is needed for PersistentVolumes with, // 1. Filesystem VolumeMode with & without Encryption and diff --git a/internal/rbd/migration.go b/internal/rbd/migration.go index ca8c48e4c..d021514a9 100644 --- a/internal/rbd/migration.go +++ b/internal/rbd/migration.go @@ -77,35 +77,41 @@ func parseMigrationVolID(vh string) (*migrationVolID, error) { // deleteMigratedVolume get rbd volume details from the migration volID // and delete the volume from the cluster, return err if there was an error on the process. func deleteMigratedVolume(ctx context.Context, parsedMigHandle *migrationVolID, cr *util.Credentials) error { + rv, err := genVolFromMigVolID(ctx, parsedMigHandle, cr) + if err != nil { + return err + } + defer rv.Destroy() + err = deleteImage(ctx, rv, cr) + if err != nil { + log.ErrorLog(ctx, "failed to delete rbd image: %s, err: %v", rv, err) + } + + return err +} + +// genVolFromMigVolID populate rbdVol struct from the migration volID. +func genVolFromMigVolID(ctx context.Context, migVolID *migrationVolID, cr *util.Credentials) (*rbdVolume, error) { var err error rv := &rbdVolume{} // fill details to rv struct from parsed migration handle - rv.RbdImageName = parsedMigHandle.imageName - rv.Pool = parsedMigHandle.poolName - rv.ClusterID = parsedMigHandle.clusterID + rv.RbdImageName = migVolID.imageName + rv.Pool = migVolID.poolName + rv.ClusterID = migVolID.clusterID rv.Monitors, err = util.Mons(util.CsiConfigFile, rv.ClusterID) if err != nil { log.ErrorLog(ctx, "failed to fetch monitors using clusterID: %s, err: %v", rv.ClusterID, err) - return err + return nil, err } - // connect to the volume. err = rv.Connect(cr) if err != nil { log.ErrorLog(ctx, "failed to get connected to the rbd image : %s, err: %v", rv.RbdImageName, err) - return err - } - defer rv.Destroy() - // if connected , delete it - err = deleteImage(ctx, rv, cr) - if err != nil { - log.ErrorLog(ctx, "failed to delete rbd image : %s, err: %v", rv.RbdImageName, err) - - return err + return nil, err } - return nil + return rv, nil } diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index ef3a15416..9fc149d14 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -2071,3 +2071,22 @@ func strategicActionOnLogFile(ctx context.Context, logStrategy, logFile string) log.ErrorLog(ctx, "unknown cephLogStrategy option %q: hint: 'remove'|'compress'|'preserve'", logStrategy) } } + +// genVolFromVolIDWithMigration populate a rbdVol structure based on the volID format. +func genVolFromVolIDWithMigration( + ctx context.Context, volID string, cr *util.Credentials, secrets map[string]string) (*rbdVolume, error) { + if isMigrationVolID(volID) { + pmVolID, pErr := parseMigrationVolID(volID) + if pErr != nil { + return nil, pErr + } + + return genVolFromMigVolID(ctx, pmVolID, cr) + } + rv, err := GenVolFromVolID(ctx, volID, cr, secrets) + if err != nil { + rv.Destroy() + } + + return rv, err +} From ca2932855488b9773403d6ad22c1f581c8c8c295 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 10:59:41 +0530 Subject: [PATCH 08/30] csi: remove size check when creating volume remove the bigger size validation when creating a volume from a snapshot or when creation a clone from a volume as we resized the volume after cloning. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 46 +++----------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 87819ab79..7e14b27e1 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -192,48 +192,8 @@ func getGRPCErrorForCreateVolume(err error) error { return status.Error(codes.Internal, err.Error()) } -// validateRequestedVolumeSize validates the request volume size with the -// source snapshot or volume size, if there is a size mismatches it returns an error. -func validateRequestedVolumeSize(rbdVol, parentVol *rbdVolume, rbdSnap *rbdSnapshot, cr *util.Credentials) error { - if rbdSnap != nil { - vol := generateVolFromSnap(rbdSnap) - err := vol.Connect(cr) - if err != nil { - return status.Error(codes.Internal, err.Error()) - } - defer vol.Destroy() - - err = vol.getImageInfo() - if err != nil { - return status.Error(codes.Internal, err.Error()) - } - if rbdVol.VolSize != vol.VolSize { - return status.Errorf( - codes.InvalidArgument, - "size mismatches, requested volume size %d and source snapshot size %d", - rbdVol.VolSize, - vol.VolSize) - } - } - if parentVol != nil { - if rbdVol.VolSize != parentVol.VolSize { - return status.Errorf( - codes.InvalidArgument, - "size mismatches, requested volume size %d and source volume size %d", - rbdVol.VolSize, - parentVol.VolSize) - } - } - - return nil -} - -func checkValidCreateVolumeRequest(rbdVol, parentVol *rbdVolume, rbdSnap *rbdSnapshot, cr *util.Credentials) error { - err := validateRequestedVolumeSize(rbdVol, parentVol, rbdSnap, cr) - if err != nil { - return err - } - +func checkValidCreateVolumeRequest(rbdVol, parentVol *rbdVolume, rbdSnap *rbdSnapshot) error { + var err error switch { case rbdSnap != nil: err = rbdSnap.isCompatibleEncryption(&rbdVol.rbdImage) @@ -309,7 +269,7 @@ func (cs *ControllerServer) CreateVolume( return cs.repairExistingVolume(ctx, req, cr, rbdVol, parentVol, rbdSnap) } - err = checkValidCreateVolumeRequest(rbdVol, parentVol, rbdSnap, cr) + err = checkValidCreateVolumeRequest(rbdVol, parentVol, rbdSnap) if err != nil { return nil, err } From 22365ab77f6650f2ba3723829509ef55dbf523a2 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 11:10:50 +0530 Subject: [PATCH 09/30] cleanup: add cleanup helper for incorrect thick volume added a new helper function called cleanupThickClone to cleanup the snapshot and clone if the thick provisioning is not fully completed. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 33 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 7e14b27e1..0cd9dcc0b 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -403,18 +403,7 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C rbdVol, err) } else if !thick { - err = cleanUpSnapshot(ctx, parentVol, rbdSnap, rbdVol, cr) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to remove partially cloned volume %q: %s", rbdVol, err) - } - err = undoVolReservation(ctx, rbdVol, cr) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to remove volume %q from journal: %s", rbdVol, err) - } - - return nil, status.Errorf( - codes.Internal, - "cloning thick-provisioned volume %q has been interrupted, please retry", rbdVol) + return nil, cleanupThickClone(ctx, parentVol, rbdVol, rbdSnap, cr) } } } @@ -422,6 +411,26 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C return buildCreateVolumeResponse(req, rbdVol), nil } +// cleanupThickClone will delete the snapshot and volume and undo the reservation. +func cleanupThickClone(ctx context.Context, + rbdVol, + parentVol *rbdVolume, + rbdSnap *rbdSnapshot, + cr *util.Credentials) error { + err := cleanUpSnapshot(ctx, parentVol, rbdSnap, rbdVol, cr) + if err != nil { + return status.Errorf(codes.Internal, "failed to remove partially cloned volume %q: %s", rbdVol, err) + } + err = undoVolReservation(ctx, rbdVol, cr) + if err != nil { + return status.Errorf(codes.Internal, "failed to remove volume %q from journal: %s", rbdVol, err) + } + + return status.Errorf( + codes.Internal, + "cloning thick-provisioned volume %q has been interrupted, please retry", rbdVol) +} + // check snapshots on the rbd image, as we have limit from krbd that an image // cannot have more than 510 snapshot at a given point of time. If the // snapshots are more than the `maxSnapshotsOnImage` Add a task to flatten all From 124281519f0c3ccfcefbb7f401ec8ea3ad91d4ec Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 19:19:35 +0530 Subject: [PATCH 10/30] rbd: add RequestedVolSize to rbdVolume struct when doing the internal operation to get the latest details the rbd image size is also getting updated and this will update the volume size also without actual requested size we cannot do the resize operation for bigger clones. This commit adds a new field called RequestedVolSize to rbdVolume struct to hold the user requested size. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 2 ++ internal/rbd/rbd_util.go | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 0cd9dcc0b..7d576cf22 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -136,6 +136,8 @@ func (cs *ControllerServer) parseVolCreateRequest( // always round up the request size in bytes to the nearest MiB/GiB rbdVol.VolSize = util.RoundOffBytes(volSizeBytes) + // RequestedVolSize has the size of the volume requested by the user. + rbdVol.RequestedVolSize = rbdVol.VolSize // start with pool the same as journal pool, in case there is a topology // based split, pool for the image will be updated subsequently diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 9fc149d14..ed758d8c7 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -164,7 +164,10 @@ type rbdVolume struct { VolName string `json:"volName"` MonValueFromSecret string `json:"monValueFromSecret"` VolSize int64 `json:"volSize"` - DisableInUseChecks bool `json:"disableInUseChecks"` + // RequestedVolSize has the size of the volume requested by the user and + // this value will not be updated when doing getImageInfo() on rbdVolume. + RequestedVolSize int64 + DisableInUseChecks bool `json:"disableInUseChecks"` readOnly bool Primary bool ThickProvision bool From a0829e9e936239c95d6c6294e64ee701617b0589 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 19:33:52 +0530 Subject: [PATCH 11/30] rbd: remove json tag from rbdVolume struct as we are no longer supporting the v1.x version of cephcsi. removing the json tag used to store rbd volume details in configmap. Signed-off-by: Madhu Rajanna --- internal/rbd/rbd_util.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index ed758d8c7..915412e8a 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -153,21 +153,21 @@ type rbdVolume struct { // Parent Pool is the pool that contains the parent image. ParentPool string imageFeatureSet librbd.FeatureSet - AdminID string `json:"adminId"` - UserID string `json:"userId"` - Mounter string `json:"mounter"` + AdminID string + UserID string + Mounter string ReservedID string MapOptions string UnmapOptions string LogDir string LogStrategy string - VolName string `json:"volName"` - MonValueFromSecret string `json:"monValueFromSecret"` - VolSize int64 `json:"volSize"` + VolName string + MonValueFromSecret string + VolSize int64 // RequestedVolSize has the size of the volume requested by the user and // this value will not be updated when doing getImageInfo() on rbdVolume. RequestedVolSize int64 - DisableInUseChecks bool `json:"disableInUseChecks"` + DisableInUseChecks bool readOnly bool Primary bool ThickProvision bool From b1a0bb471402e52e43164a9185fa561eb1fd86b0 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 19:36:24 +0530 Subject: [PATCH 12/30] rbd: move VolSize to rbdImage struct move the Volsize to the rbdImage struct as size is more applicable for rbdImage as rbdImage is used for both rbdVolume and rbdSnapshot. Signed-off-by: Madhu Rajanna --- internal/rbd/rbd_util.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 915412e8a..69bcc913a 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -107,6 +107,9 @@ type rbdImage struct { // identifying this rbd image VolID string `json:"volID"` + // VolSize is the size of the RBD image backing this rbdImage. + VolSize int64 + Monitors string // JournalPool is the ceph pool in which the CSI Journal/CSI snapshot Journal is // stored @@ -163,7 +166,6 @@ type rbdVolume struct { LogStrategy string VolName string MonValueFromSecret string - VolSize int64 // RequestedVolSize has the size of the volume requested by the user and // this value will not be updated when doing getImageInfo() on rbdVolume. RequestedVolSize int64 From 6a82baf5d348eb6b2427eba26c1122072145d9b1 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 19:44:10 +0530 Subject: [PATCH 13/30] rbd: remove SizeBytes from rbdSnapshot struct as we are moving the VolSize to rbdImage struct we should reuse the same instead of maintaining one more field in rbdSnapshot struct. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 4 ++-- internal/rbd/rbd_journal.go | 2 +- internal/rbd/rbd_util.go | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 7d576cf22..64c6175cf 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -1001,7 +1001,7 @@ func (cs *ControllerServer) CreateSnapshot( return nil, status.Error(codes.Internal, err.Error()) } rbdSnap.RbdImageName = rbdVol.RbdImageName - rbdSnap.SizeBytes = rbdVol.VolSize + rbdSnap.VolSize = rbdVol.VolSize rbdSnap.SourceVolumeID = req.GetSourceVolumeId() rbdSnap.RequestName = req.GetName() @@ -1134,7 +1134,7 @@ func cloneFromSnapshot( return &csi.CreateSnapshotResponse{ Snapshot: &csi.Snapshot{ - SizeBytes: rbdSnap.SizeBytes, + SizeBytes: rbdSnap.VolSize, SnapshotId: rbdSnap.VolID, SourceVolumeId: rbdSnap.SourceVolumeID, CreationTime: rbdSnap.CreatedAt, diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index a1633506e..5f81fafc5 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -176,7 +176,7 @@ func checkSnapCloneExists( // Code from here on, rolls the transaction forward. rbdSnap.CreatedAt = vol.CreatedAt - rbdSnap.SizeBytes = vol.VolSize + rbdSnap.VolSize = vol.VolSize // found a snapshot already available, process and return its information rbdSnap.VolID, err = util.GenerateVolID(ctx, rbdSnap.Monitors, cr, snapData.ImagePoolID, rbdSnap.Pool, rbdSnap.ClusterID, snapUUID, volIDVersion) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 69bcc913a..91068d399 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -184,7 +184,6 @@ type rbdSnapshot struct { SourceVolumeID string ReservedID string RbdSnapName string - SizeBytes int64 } // imageFeature represents required image features and value. From da60d221dfa354e915ab1e4bd2e81497898359a0 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 11:35:34 +0530 Subject: [PATCH 14/30] rbd: update size for rbdSnapshot struct we need actual size of the rbdVolume created for the snapshot, as we are not storing the size of the snapshot in OMAP we need to fetch the size from ceph cluster and update the same on rbdSnapshot struct. Signed-off-by: Madhu Rajanna --- internal/rbd/rbd_util.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 91068d399..eed7b7924 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1055,9 +1055,33 @@ func genSnapFromSnapID( } } + err = updateSnapshotDetails(rbdSnap) + if err != nil { + return fmt.Errorf("failed to update snapshot details for %q: %w", rbdSnap, err) + } + return err } +// updateSnapshotDetails will copies the details from the rbdVolume to the +// rbdSnapshot. example copying size from rbdVolume to rbdSnapshot. +func updateSnapshotDetails(rbdSnap *rbdSnapshot) error { + vol := generateVolFromSnap(rbdSnap) + err := vol.Connect(rbdSnap.conn.Creds) + if err != nil { + return err + } + defer vol.Destroy() + + err = vol.getImageInfo() + if err != nil { + return err + } + rbdSnap.VolSize = vol.VolSize + + return nil +} + // generateVolumeFromVolumeID generates a rbdVolume structure from the provided identifier. func generateVolumeFromVolumeID( ctx context.Context, From f7f662678a42ac19ab9f36f1a1aa96318a81b449 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 12:00:41 +0530 Subject: [PATCH 15/30] rbd: consider ErrImageNotFound during DeleteSnapshot added a check to consider ErrImageNotFound error during DeleteSnapshot operation, if the error is ErrImageNotFound we need to ensure that image is removed from the trash and also the rados OMAP data is removed. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 70 +++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 64c6175cf..10acab35e 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -1327,6 +1327,17 @@ func (cs *ControllerServer) DeleteSnapshot( return &csi.DeleteSnapshotResponse{}, nil } + // if the error is ErrImageNotFound, We need to cleanup the image from + // trash and remove the metadata in OMAP. + if errors.Is(err, ErrImageNotFound) { + err = cleanUpImageAndSnapReservation(ctx, rbdSnap, cr) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + + return &csi.DeleteSnapshotResponse{}, nil + } + return nil, status.Error(codes.Internal, err.Error()) } @@ -1350,28 +1361,14 @@ func (cs *ControllerServer) DeleteSnapshot( } defer rbdVol.Destroy() - err = rbdVol.getImageInfo() + rbdVol.ImageID = rbdSnap.ImageID + // update parent name to delete the snapshot + rbdSnap.RbdImageName = rbdVol.RbdImageName + err = cleanUpSnapshot(ctx, rbdVol, rbdSnap, rbdVol, cr) if err != nil { - if errors.Is(err, ErrImageNotFound) { - err = rbdVol.ensureImageCleanup(ctx) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } - } else { - log.ErrorLog(ctx, "failed to delete rbd image: %s/%s with error: %v", rbdVol.Pool, rbdVol.VolName, err) + log.ErrorLog(ctx, "failed to delete image: %v", err) - return nil, status.Error(codes.Internal, err.Error()) - } - } else { - rbdVol.ImageID = rbdSnap.ImageID - // update parent name to delete the snapshot - rbdSnap.RbdImageName = rbdVol.RbdImageName - err = cleanUpSnapshot(ctx, rbdVol, rbdSnap, rbdVol, cr) - if err != nil { - log.ErrorLog(ctx, "failed to delete image: %v", err) - - return nil, status.Error(codes.Internal, err.Error()) - } + return nil, status.Error(codes.Internal, err.Error()) } err = undoSnapReservation(ctx, rbdSnap, cr) if err != nil { @@ -1384,6 +1381,39 @@ func (cs *ControllerServer) DeleteSnapshot( return &csi.DeleteSnapshotResponse{}, nil } +// cleanUpImageAndSnapReservation cleans up the image from the trash and +// snapshot reservation in rados OMAP. +func cleanUpImageAndSnapReservation(ctx context.Context, rbdSnap *rbdSnapshot, cr *util.Credentials) error { + rbdVol := generateVolFromSnap(rbdSnap) + err := rbdVol.Connect(cr) + if err != nil { + return status.Error(codes.Internal, err.Error()) + } + defer rbdVol.Destroy() + + err = rbdVol.openIoctx() + if err != nil { + return status.Error(codes.Internal, err.Error()) + } + + // cleanup the image from trash if the error is image not found. + err = rbdVol.ensureImageCleanup(ctx) + if err != nil { + log.ErrorLog(ctx, "failed to delete rbd image: %q with error: %v", rbdVol.Pool, rbdVol.VolName, err) + + return status.Error(codes.Internal, err.Error()) + } + err = undoSnapReservation(ctx, rbdSnap, cr) + if err != nil { + log.ErrorLog(ctx, "failed to remove reservation for snapname (%s) with backing snap %q", + rbdSnap.RequestName, rbdSnap, err) + + return status.Error(codes.Internal, err.Error()) + } + + return nil +} + // ControllerExpandVolume expand RBD Volumes on demand based on resizer request. func (cs *ControllerServer) ControllerExpandVolume( ctx context.Context, From a28a4a4285f90efbdec65197b1eb6fdeb2c7fac5 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 11:14:24 +0530 Subject: [PATCH 16/30] rbd: resize the volume created from volume If the requested volume size is greater than the parent volume size, resize the cloned volume after creating a final clone from a parent volume. Signed-off-by: Madhu Rajanna --- internal/rbd/clone.go | 8 ++++++++ internal/rbd/controllerserver.go | 7 +++++++ internal/rbd/rbd_util.go | 10 ++++++++++ 3 files changed, 25 insertions(+) diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go index 3b91e47be..1fe0e42d8 100644 --- a/internal/rbd/clone.go +++ b/internal/rbd/clone.go @@ -181,6 +181,14 @@ func (rv *rbdVolume) createCloneFromImage(ctx context.Context, parentVol *rbdVol return err } + // expand the image if the requested size is greater than the current size + err = rv.expand() + if err != nil { + log.ErrorLog(ctx, "failed to resize volume %s: %v", rv, err) + + return err + } + return nil } diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 10acab35e..640079a54 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -408,6 +408,13 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C return nil, cleanupThickClone(ctx, parentVol, rbdVol, rbdSnap, cr) } } + // expand the image if the requested size is greater than the current size + err := rbdVol.expand() + if err != nil { + log.ErrorLog(ctx, "failed to resize volume %s: %v", rbdVol, err) + + return nil, err + } } return buildCreateVolumeResponse(req, rbdVol), nil diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index eed7b7924..3b18c8e2c 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1703,6 +1703,16 @@ func cleanupRBDImageMetadataStash(metaDataPath string) error { return nil } +// expand checks if the requestedVolume size and the existing image size both +// are same. If they are same, it returns nil else it resizes the image. +func (rv *rbdVolume) expand() error { + if rv.RequestedVolSize == rv.VolSize { + return nil + } + + return rv.resize(rv.RequestedVolSize) +} + // resize the given volume to new size. // updates Volsize of rbdVolume object to newSize in case of success. func (rv *rbdVolume) resize(newSize int64) error { From 69ae19e0cb242c5277cadae4f8a404094d8cb714 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 11:02:13 +0530 Subject: [PATCH 17/30] rbd: resize the volume created from snapshot If the requested volume size is greater than the snapshot size, resize the cloned volume after creating a clone from a snapshot. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 640079a54..50bb8a4cc 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -389,6 +389,14 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C return nil, err } + // expand the image if the requested size is greater than the current size + err = rbdVol.expand() + if err != nil { + log.ErrorLog(ctx, "failed to resize volume %s: %v", rbdVol, err) + + return nil, err + } + // rbdVol is a clone from parentVol case vcs.GetVolume() != nil: // When cloning into a thick-provisioned volume was happening, @@ -590,6 +598,15 @@ func (cs *ControllerServer) createVolumeFromSnapshot( log.DebugLog(ctx, "create volume %s from snapshot %s", rbdVol.RequestName, rbdSnap.RbdSnapName) + // resize the volume if the size is different + // expand the image if the requested size is greater than the current size + err = rbdVol.expand() + if err != nil { + log.ErrorLog(ctx, "failed to resize volume %s: %v", rbdVol, err) + + return err + } + return nil } From 3169c8e23ab9dda2cb4401c82f9abc6b439fa1dc Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 15 Dec 2021 12:55:19 +0530 Subject: [PATCH 18/30] rbd: expand filesystem during NodeStageVolume If the volume with a bigger size is created from a snapshot or from another volume we need to exapand the filesystem also in the csidriver as nodeExpand request is not triggered for this one, During NodeStageVolume we can expand the filesystem by checking filesystem needs expansion or not. If its a encrypted device, check the device size of rbd device and the LUKS device if required the device will be expanded before expanding the filesystem. Signed-off-by: Madhu Rajanna --- internal/rbd/nodeserver.go | 127 +++++++++++++++++++++++++++++++------ 1 file changed, 108 insertions(+), 19 deletions(-) diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 52da74dde..8f30451c1 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -427,26 +427,15 @@ func (ns *NodeServer) stageTransaction( } transaction.isMounted = true - // resize if its fileSystemType static volume. - if staticVol && !isBlock { - var ok bool - resizer := mount.NewResizeFs(utilexec.New()) - ok, err = resizer.NeedResize(devicePath, stagingTargetPath) - if err != nil { - return transaction, status.Errorf(codes.Internal, - "need resize check failed on devicePath %s and staingPath %s, error: %v", - devicePath, - stagingTargetPath, - err) - } - if ok { - ok, err = resizer.Resize(devicePath, stagingTargetPath) - if !ok { - return transaction, status.Errorf(codes.Internal, - "resize failed on path %s, error: %v", stagingTargetPath, err) - } - } + // As we are supporting the restore of a volume to a bigger size and + // creating bigger size clone from a volume, we need to check filesystem + // resize is required, if required resize filesystem. + // in case of encrypted block PVC resize only the LUKS device. + err = resizeNodeStagePath(ctx, isBlock, transaction, req.GetVolumeId(), stagingTargetPath) + if err != nil { + return transaction, err } + if !readOnly { // #nosec - allow anyone to write inside the target path err = os.Chmod(stagingTargetPath, 0o777) @@ -455,6 +444,87 @@ func (ns *NodeServer) stageTransaction( return transaction, err } +// resizeNodeStagePath resizes the device if its encrypted and it also resizes +// the stagingTargetPath if filesystem needs resize. +func resizeNodeStagePath(ctx context.Context, + isBlock bool, + transaction *stageTransaction, + volID, + stagingTargetPath string) error { + var err error + devicePath := transaction.devicePath + var ok bool + + // if its a non encrypted block device we dont need any expansion + if isBlock && !transaction.isEncrypted { + return nil + } + + resizer := mount.NewResizeFs(utilexec.New()) + + if transaction.isEncrypted { + devicePath, err = resizeEncryptedDevice(ctx, volID, stagingTargetPath, devicePath) + if err != nil { + return status.Error(codes.Internal, err.Error()) + } + } + // check stagingPath needs resize. + ok, err = resizer.NeedResize(devicePath, stagingTargetPath) + if err != nil { + return status.Errorf(codes.Internal, + "need resize check failed on devicePath %s and staingPath %s, error: %v", + devicePath, + stagingTargetPath, + err) + } + // return nil if no resize is required + if !ok { + return nil + } + ok, err = resizer.Resize(devicePath, stagingTargetPath) + if !ok { + return status.Errorf(codes.Internal, + "resize failed on path %s, error: %v", stagingTargetPath, err) + } + + return nil +} + +func resizeEncryptedDevice(ctx context.Context, volID, stagingTargetPath, devicePath string) (string, error) { + rbdDevSize, err := getDeviceSize(ctx, devicePath) + if err != nil { + return "", fmt.Errorf( + "failed to get device size of %s and staingPath %s, error: %w", + devicePath, + stagingTargetPath, + err) + } + _, mapperPath := util.VolumeMapper(volID) + encDevSize, err := getDeviceSize(ctx, mapperPath) + if err != nil { + return "", fmt.Errorf( + "failed to get device size of %s and staingPath %s, error: %w", + mapperPath, + stagingTargetPath, + err) + } + // if the rbd device `/dev/rbd0` size is greater than LUKS device size + // we need to resize the LUKS device. + if rbdDevSize > encDevSize { + // The volume is encrypted, resize an active mapping + err = util.ResizeEncryptedVolume(ctx, mapperPath) + if err != nil { + log.ErrorLog(ctx, "failed to resize device %s: %v", + mapperPath, err) + + return "", fmt.Errorf( + "failed to resize device %s: %w", mapperPath, err) + } + } + + return mapperPath, nil +} + func flattenImageBeforeMapping( ctx context.Context, volOptions *rbdVolume, @@ -1168,3 +1238,22 @@ func blockNodeGetVolumeStats(ctx context.Context, targetPath string) (*csi.NodeG }, }, nil } + +// getDeviceSize gets the block device size. +func getDeviceSize(ctx context.Context, devicePath string) (uint64, error) { + output, _, err := util.ExecCommand(ctx, "blockdev", "--getsize64", devicePath) + if err != nil { + return 0, fmt.Errorf("blockdev %v returned an error: %w", devicePath, err) + } + + outStr := strings.TrimSpace(output) + if err != nil { + return 0, fmt.Errorf("failed to read size of device %s: %s: %w", devicePath, outStr, err) + } + size, err := strconv.ParseUint(outStr, 10, 64) + if err != nil { + return 0, fmt.Errorf("failed to parse size of device %s %s: %w", devicePath, outStr, err) + } + + return size, nil +} From 494c9b91cb1d9213b835717826459963e71b406e Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 16 Dec 2021 10:57:46 +0530 Subject: [PATCH 19/30] e2e: add e2e for bigger size restore added e2e for below cases Normal PVC snapshot restore to a bigger size PVC (without encryption) * Filesystem pvc restore to a bigger size * Block pvc restore to a bigger size Encrypted PVC snapshot restore to a bigger size PVC * Filesystem pvc restore to a bigger size * Block pvc restore to a bigger size Signed-off-by: Madhu Rajanna --- e2e/rbd.go | 106 ++++++++++++++++++++++++++++ e2e/snapshot.go | 87 +++++++++++++++++++++++ examples/rbd/pod-block-restore.yaml | 17 +++++ examples/rbd/pvc-block-restore.yaml | 17 +++++ 4 files changed, 227 insertions(+) create mode 100644 examples/rbd/pod-block-restore.yaml create mode 100644 examples/rbd/pvc-block-restore.yaml diff --git a/e2e/rbd.go b/e2e/rbd.go index 124d9b988..3f6cc729b 100644 --- a/e2e/rbd.go +++ b/e2e/rbd.go @@ -60,6 +60,8 @@ var ( appClonePath = rbdExamplePath + "pod-restore.yaml" appSmartClonePath = rbdExamplePath + "pod-clone.yaml" appBlockSmartClonePath = rbdExamplePath + "block-pod-clone.yaml" + pvcBlockRestorePath = rbdExamplePath + "pvc-block-restore.yaml" + appBlockRestorePath = rbdExamplePath + "pod-block-restore.yaml" appEphemeralPath = rbdExamplePath + "pod-ephemeral.yaml" snapshotPath = rbdExamplePath + "snapshot.yaml" deployFSAppPath = e2eTemplatesPath + "rbd-fs-deployment.yaml" @@ -3338,6 +3340,110 @@ var _ = Describe("RBD", func() { } }) + By("restore snapshot to a bigger size PVC", func() { + By("restore snapshot to bigger size pvc", func() { + err := deleteResource(rbdExamplePath + "storageclass.yaml") + if err != nil { + e2elog.Failf("failed to delete storageclass: %v", err) + } + err = createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, nil, deletePolicy) + if err != nil { + e2elog.Failf("failed to create storageclass: %v", err) + } + defer func() { + err = deleteResource(rbdExamplePath + "storageclass.yaml") + if err != nil { + e2elog.Failf("failed to delete storageclass: %v", err) + } + }() + err = createRBDSnapshotClass(f) + if err != nil { + e2elog.Failf("failed to create VolumeSnapshotClass: %v", err) + } + defer func() { + err = deleteRBDSnapshotClass() + if err != nil { + e2elog.Failf("failed to delete VolumeSnapshotClass: %v", err) + } + }() + // validate filesystem mode PVC + err = validateBiggerPVCFromSnapshot(f, + pvcPath, + appPath, + snapshotPath, + pvcClonePath, + appClonePath) + if err != nil { + e2elog.Failf("failed to validate restore bigger size clone: %v", err) + } + // validate block mode PVC + err = validateBiggerPVCFromSnapshot(f, + rawPvcPath, + rawAppPath, + snapshotPath, + pvcBlockRestorePath, + appBlockRestorePath) + if err != nil { + e2elog.Failf("failed to validate restore bigger size clone: %v", err) + } + }) + + By("restore snapshot to bigger size encrypted PVC with VaultKMS", func() { + scOpts := map[string]string{ + "encrypted": "true", + "encryptionKMSID": "vault-test", + } + err := createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, scOpts, deletePolicy) + if err != nil { + e2elog.Failf("failed to create storageclass: %v", err) + } + defer func() { + err = deleteResource(rbdExamplePath + "storageclass.yaml") + if err != nil { + e2elog.Failf("failed to delete storageclass: %v", err) + } + }() + err = createRBDSnapshotClass(f) + if err != nil { + e2elog.Failf("failed to create VolumeSnapshotClass: %v", err) + } + defer func() { + err = deleteRBDSnapshotClass() + if err != nil { + e2elog.Failf("failed to delete VolumeSnapshotClass: %v", err) + } + }() + // validate filesystem mode PVC + err = validateBiggerPVCFromSnapshot(f, + pvcPath, + appPath, + snapshotPath, + pvcClonePath, + appClonePath) + if err != nil { + e2elog.Failf("failed to validate restore bigger size clone: %v", err) + } + // validate block mode PVC + err = validateBiggerPVCFromSnapshot(f, + rawPvcPath, + rawAppPath, + snapshotPath, + pvcBlockRestorePath, + appBlockRestorePath) + if err != nil { + e2elog.Failf("failed to validate restore bigger size clone: %v", err) + } + }) + + By("validate image deletion", func() { + validateRBDImageCount(f, 0, defaultRBDPool) + err := waitToRemoveImagesFromTrash(f, defaultRBDPool, deployTimeout) + if err != nil { + e2elog.Failf("failed to validate rbd images in pool %s trash: %v", defaultRBDPool, err) + } + }) + }) + // Make sure this should be last testcase in this file, because // it deletes pool By("Create a PVC and delete PVC when backend pool deleted", func() { diff --git a/e2e/snapshot.go b/e2e/snapshot.go index 390a1a09b..b6d15c750 100644 --- a/e2e/snapshot.go +++ b/e2e/snapshot.go @@ -9,7 +9,9 @@ import ( snapapi "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1" snapclient "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/typed/volumesnapshot/v1" . "github.com/onsi/gomega" // nolint + v1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/test/e2e/framework" @@ -218,3 +220,88 @@ func getVolumeSnapshotContent(namespace, snapshotName string) (*snapapi.VolumeSn return volumeSnapshotContent, nil } + +func validateBiggerPVCFromSnapshot(f *framework.Framework, + pvcPath, + appPath, + snapPath, + pvcClonePath, + appClonePath string) error { + const ( + size = "1Gi" + newSize = "2Gi" + ) + pvc, err := loadPVC(pvcPath) + if err != nil { + return fmt.Errorf("failed to load PVC: %w", err) + } + label := make(map[string]string) + pvc.Namespace = f.UniqueName + pvc.Spec.Resources.Requests[v1.ResourceStorage] = resource.MustParse(size) + app, err := loadApp(appPath) + if err != nil { + return fmt.Errorf("failed to load app: %w", err) + } + label[appKey] = appLabel + app.Namespace = f.UniqueName + app.Labels = label + opt := metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", appKey, label[appKey]), + } + err = createPVCAndApp("", f, pvc, app, deployTimeout) + if err != nil { + return fmt.Errorf("failed to create pvc and application: %w", err) + } + + snap := getSnapshot(snapPath) + snap.Namespace = f.UniqueName + snap.Spec.Source.PersistentVolumeClaimName = &pvc.Name + err = createSnapshot(&snap, deployTimeout) + if err != nil { + return fmt.Errorf("failed to create snapshot: %w", err) + } + err = deletePVCAndApp("", f, pvc, app) + if err != nil { + return fmt.Errorf("failed to delete pvc and application: %w", err) + } + pvcClone, err := loadPVC(pvcClonePath) + if err != nil { + e2elog.Failf("failed to load PVC: %v", err) + } + pvcClone.Namespace = f.UniqueName + pvcClone.Spec.DataSource.Name = snap.Name + pvcClone.Spec.Resources.Requests[v1.ResourceStorage] = resource.MustParse(newSize) + appClone, err := loadApp(appClonePath) + if err != nil { + e2elog.Failf("failed to load application: %v", err) + } + appClone.Namespace = f.UniqueName + appClone.Labels = label + err = createPVCAndApp("", f, pvcClone, appClone, deployTimeout) + if err != nil { + return fmt.Errorf("failed to create pvc clone and application: %w", err) + } + err = deleteSnapshot(&snap, deployTimeout) + if err != nil { + return fmt.Errorf("failed to delete snapshot: %w", err) + } + if pvcClone.Spec.VolumeMode == nil || *pvcClone.Spec.VolumeMode == v1.PersistentVolumeFilesystem { + err = checkDirSize(appClone, f, &opt, newSize) + if err != nil { + return fmt.Errorf("failed to validate directory size: %w", err) + } + } + + if pvcClone.Spec.VolumeMode != nil && *pvcClone.Spec.VolumeMode == v1.PersistentVolumeBlock { + err = checkDeviceSize(appClone, f, &opt, newSize) + if err != nil { + return fmt.Errorf("failed to validate device size: %w", err) + } + } + err = deletePVCAndApp("", f, pvcClone, appClone) + if err != nil { + return fmt.Errorf("failed to delete pvc and application: %w", err) + } + + return nil +} diff --git a/examples/rbd/pod-block-restore.yaml b/examples/rbd/pod-block-restore.yaml new file mode 100644 index 000000000..230b2fd87 --- /dev/null +++ b/examples/rbd/pod-block-restore.yaml @@ -0,0 +1,17 @@ +--- +apiVersion: v1 +kind: Pod +metadata: + name: pod-block-volume-restore +spec: + containers: + - name: centos + image: quay.io/centos/centos:latest + command: ["/bin/sleep", "infinity"] + volumeDevices: + - name: data + devicePath: /dev/xvda + volumes: + - name: data + persistentVolumeClaim: + claimName: rbd-block-pvc-restore diff --git a/examples/rbd/pvc-block-restore.yaml b/examples/rbd/pvc-block-restore.yaml new file mode 100644 index 000000000..deb333cf3 --- /dev/null +++ b/examples/rbd/pvc-block-restore.yaml @@ -0,0 +1,17 @@ +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: rbd-block-pvc-restore +spec: + storageClassName: csi-rbd-sc + dataSource: + name: rbd-pvc-snapshot + kind: VolumeSnapshot + apiGroup: snapshot.storage.k8s.io + accessModes: + - ReadWriteOnce + volumeMode: Block + resources: + requests: + storage: 1Gi From 210f95de0414c01fa23ab0d146bd5d982b933188 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 16 Dec 2021 12:04:45 +0530 Subject: [PATCH 20/30] e2e: add e2e for bigger size clone added e2e for below cases Normal PVC clone to a bigger size PVC (without encryption) * Filesystem pvc clone to a bigger size * Block pvc clone to a bigger size Encrypted PVC clone to a bigger size PVC * Filesystem pvc clone to a bigger size * Block pvc clone to a bigger size Signed-off-by: Madhu Rajanna --- e2e/clone.go | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++ e2e/rbd.go | 71 ++++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 e2e/clone.go diff --git a/e2e/clone.go b/e2e/clone.go new file mode 100644 index 000000000..4f508ecad --- /dev/null +++ b/e2e/clone.go @@ -0,0 +1,100 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/kubernetes/test/e2e/framework" + e2elog "k8s.io/kubernetes/test/e2e/framework/log" +) + +func validateBiggerCloneFromPVC(f *framework.Framework, + pvcPath, + appPath, + pvcClonePath, + appClonePath string) error { + const ( + size = "1Gi" + newSize = "2Gi" + ) + pvc, err := loadPVC(pvcPath) + if err != nil { + return fmt.Errorf("failed to load PVC: %w", err) + } + label := make(map[string]string) + pvc.Namespace = f.UniqueName + pvc.Spec.Resources.Requests[v1.ResourceStorage] = resource.MustParse(size) + app, err := loadApp(appPath) + if err != nil { + return fmt.Errorf("failed to load app: %w", err) + } + label[appKey] = appLabel + app.Namespace = f.UniqueName + app.Labels = label + opt := metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", appKey, label[appKey]), + } + err = createPVCAndApp("", f, pvc, app, deployTimeout) + if err != nil { + return fmt.Errorf("failed to create pvc and application: %w", err) + } + + pvcClone, err := loadPVC(pvcClonePath) + if err != nil { + e2elog.Failf("failed to load PVC: %v", err) + } + pvcClone.Namespace = f.UniqueName + pvcClone.Spec.DataSource.Name = pvc.Name + pvcClone.Spec.Resources.Requests[v1.ResourceStorage] = resource.MustParse(newSize) + appClone, err := loadApp(appClonePath) + if err != nil { + e2elog.Failf("failed to load application: %v", err) + } + appClone.Namespace = f.UniqueName + appClone.Labels = label + err = createPVCAndApp("", f, pvcClone, appClone, deployTimeout) + if err != nil { + return fmt.Errorf("failed to create pvc clone and application: %w", err) + } + err = deletePVCAndApp("", f, pvc, app) + if err != nil { + return fmt.Errorf("failed to delete pvc and application: %w", err) + } + if pvcClone.Spec.VolumeMode == nil || *pvcClone.Spec.VolumeMode == v1.PersistentVolumeFilesystem { + err = checkDirSize(appClone, f, &opt, newSize) + if err != nil { + return err + } + } + + if pvcClone.Spec.VolumeMode != nil && *pvcClone.Spec.VolumeMode == v1.PersistentVolumeBlock { + err = checkDeviceSize(appClone, f, &opt, newSize) + if err != nil { + return err + } + } + err = deletePVCAndApp("", f, pvcClone, appClone) + if err != nil { + return fmt.Errorf("failed to delete pvc and application: %w", err) + } + + return nil +} diff --git a/e2e/rbd.go b/e2e/rbd.go index 3f6cc729b..3b6d42a7e 100644 --- a/e2e/rbd.go +++ b/e2e/rbd.go @@ -3444,6 +3444,77 @@ var _ = Describe("RBD", func() { }) }) + By("clone PVC to a bigger size PVC", func() { + By("clone PVC to bigger size encrypted PVC with VaultKMS", func() { + scOpts := map[string]string{ + "encrypted": "true", + "encryptionKMSID": "vault-test", + } + err := createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, scOpts, deletePolicy) + if err != nil { + e2elog.Failf("failed to create storageclass: %v", err) + } + defer func() { + err = deleteResource(rbdExamplePath + "storageclass.yaml") + if err != nil { + e2elog.Failf("failed to delete storageclass: %v", err) + } + }() + + // validate filesystem mode PVC + err = validateBiggerCloneFromPVC(f, + pvcPath, + appPath, + pvcSmartClonePath, + appSmartClonePath) + if err != nil { + e2elog.Failf("failed to validate bigger size clone: %v", err) + } + // validate block mode PVC + err = validateBiggerCloneFromPVC(f, + rawPvcPath, + rawAppPath, + pvcBlockSmartClonePath, + appBlockSmartClonePath) + if err != nil { + e2elog.Failf("failed to validate bigger size clone: %v", err) + } + }) + + By("clone PVC to bigger size pvc", func() { + err := createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, nil, deletePolicy) + if err != nil { + e2elog.Failf("failed to create storageclass: %v", err) + } + // validate filesystem mode PVC + err = validateBiggerCloneFromPVC(f, + pvcPath, + appPath, + pvcSmartClonePath, + appSmartClonePath) + if err != nil { + e2elog.Failf("failed to validate bigger size clone: %v", err) + } + // validate block mode PVC + err = validateBiggerCloneFromPVC(f, + rawPvcPath, + rawAppPath, + pvcBlockSmartClonePath, + appBlockSmartClonePath) + if err != nil { + e2elog.Failf("failed to validate bigger size clone: %v", err) + } + }) + + By("validate image deletion", func() { + validateRBDImageCount(f, 0, defaultRBDPool) + err := waitToRemoveImagesFromTrash(f, defaultRBDPool, deployTimeout) + if err != nil { + e2elog.Failf("failed to validate rbd images in pool %s trash: %v", defaultRBDPool, err) + } + }) + }) + // Make sure this should be last testcase in this file, because // it deletes pool By("Create a PVC and delete PVC when backend pool deleted", func() { From c6b288779aa1f04a787edb9fa129759a7cce67f8 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 20:31:11 +0530 Subject: [PATCH 21/30] rbd: correct logging for clone log the rbdVolume and the rbdSnapshot after creating the clone from snapshot. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 50bb8a4cc..7f6cb93e7 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -596,7 +596,7 @@ func (cs *ControllerServer) createVolumeFromSnapshot( } } - log.DebugLog(ctx, "create volume %s from snapshot %s", rbdVol.RequestName, rbdSnap.RbdSnapName) + log.DebugLog(ctx, "create volume %s from snapshot %s", rbdVol, rbdSnap) // resize the volume if the size is different // expand the image if the requested size is greater than the current size From edcb2b529b0c58b924fb1ab54dc944ee9a9614f1 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 20:44:28 +0530 Subject: [PATCH 22/30] rbd: move core fields to rbdImage struct moved ParentName, ParentPool and ImageFeatureSet fields to the rbdImage struct as these are the first citizens on the rbdImage. Signed-off-by: Madhu Rajanna --- internal/rbd/clone.go | 2 +- internal/rbd/controllerserver.go | 2 +- internal/rbd/rbd_util.go | 34 +++++++++++---------- internal/rbd/rbd_util_test.go | 2 +- internal/rbd/replicationcontrollerserver.go | 2 +- 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go index 1fe0e42d8..c72dbeb8c 100644 --- a/internal/rbd/clone.go +++ b/internal/rbd/clone.go @@ -128,7 +128,7 @@ func (rv *rbdVolume) generateTempClone() *rbdVolume { tempClone.conn = rv.conn.Copy() // The temp clone image need to have deep flatten feature f := []string{librbd.FeatureNameLayering, librbd.FeatureNameDeepFlatten} - tempClone.imageFeatureSet = librbd.FeatureSetFromNames(f) + tempClone.ImageFeatureSet = librbd.FeatureSetFromNames(f) tempClone.ClusterID = rv.ClusterID tempClone.Monitors = rv.Monitors tempClone.Pool = rv.Pool diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 7f6cb93e7..1c72357a8 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -1204,7 +1204,7 @@ func (cs *ControllerServer) doSnapshotClone( defer cloneRbd.Destroy() // add image feature for cloneRbd f := []string{librbd.FeatureNameLayering, librbd.FeatureNameDeepFlatten} - cloneRbd.imageFeatureSet = librbd.FeatureSetFromNames(f) + cloneRbd.ImageFeatureSet = librbd.FeatureSetFromNames(f) err := cloneRbd.Connect(cr) if err != nil { diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 3b18c8e2c..b91fb1107 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -127,6 +127,12 @@ type rbdImage struct { RequestName string NamePrefix string + // ParentName represents the parent image name of the image. + ParentName string + // Parent Pool is the pool that contains the parent image. + ParentPool string + ImageFeatureSet librbd.FeatureSet + // encryption provides access to optional VolumeEncryption functions encryption *util.VolumeEncryption // Owner is the creator (tenant, Kubernetes Namespace) of the volume @@ -151,11 +157,7 @@ type rbdVolume struct { Topology map[string]string // DataPool is where the data for images in `Pool` are stored, this is used as the `--data-pool` // argument when the pool is created, and is not used anywhere else - DataPool string - ParentName string - // Parent Pool is the pool that contains the parent image. - ParentPool string - imageFeatureSet librbd.FeatureSet + DataPool string AdminID string UserID string Mounter string @@ -349,10 +351,10 @@ func createImage(ctx context.Context, pOpts *rbdVolume, cr *util.Credentials) er } } log.DebugLog(ctx, logMsg, - pOpts, volSzMiB, pOpts.imageFeatureSet.Names(), pOpts.Monitors) + pOpts, volSzMiB, pOpts.ImageFeatureSet.Names(), pOpts.Monitors) - if pOpts.imageFeatureSet != 0 { - err := options.SetUint64(librbd.RbdImageOptionFeatures, uint64(pOpts.imageFeatureSet)) + if pOpts.ImageFeatureSet != 0 { + err := options.SetUint64(librbd.RbdImageOptionFeatures, uint64(pOpts.ImageFeatureSet)) if err != nil { return fmt.Errorf("failed to set image features: %w", err) } @@ -925,7 +927,7 @@ func (rv *rbdVolume) flatten() error { } func (rv *rbdVolume) hasFeature(feature uint64) bool { - return (uint64(rv.imageFeatureSet) & feature) == feature + return (uint64(rv.ImageFeatureSet) & feature) == feature } func (rv *rbdVolume) checkImageChainHasFeature(ctx context.Context, feature uint64) (bool, error) { @@ -1315,7 +1317,7 @@ func genVolFromVolumeOptions( ctx, "setting disableInUseChecks: %t image features: %v mounter: %s", disableInUseChecks, - rbdVol.imageFeatureSet.Names(), + rbdVol.ImageFeatureSet.Names(), rbdVol.Mounter) rbdVol.DisableInUseChecks = disableInUseChecks @@ -1353,7 +1355,7 @@ func (rv *rbdVolume) validateImageFeatures(imageFeatures string) error { return fmt.Errorf("feature %s requires rbd-nbd for mounter", f) } } - rv.imageFeatureSet = librbd.FeatureSetFromNames(arr) + rv.ImageFeatureSet = librbd.FeatureSetFromNames(arr) return nil } @@ -1386,7 +1388,7 @@ func genSnapFromOptions(ctx context.Context, rbdVol *rbdVolume, snapOptions map[ // hasSnapshotFeature checks if Layering is enabled for this image. func (rv *rbdVolume) hasSnapshotFeature() bool { - return (uint64(rv.imageFeatureSet) & librbd.FeatureLayering) == librbd.FeatureLayering + return (uint64(rv.ImageFeatureSet) & librbd.FeatureLayering) == librbd.FeatureLayering } func (rv *rbdVolume) createSnapshot(ctx context.Context, pOpts *rbdSnapshot) error { @@ -1450,10 +1452,10 @@ func (rv *rbdVolume) cloneRbdImageFromSnapshot( } log.DebugLog(ctx, logMsg, - pSnapOpts, rv, rv.imageFeatureSet.Names(), rv.Monitors) + pSnapOpts, rv, rv.ImageFeatureSet.Names(), rv.Monitors) - if rv.imageFeatureSet != 0 { - err = options.SetUint64(librbd.RbdImageOptionFeatures, uint64(rv.imageFeatureSet)) + if rv.ImageFeatureSet != 0 { + err = options.SetUint64(librbd.RbdImageOptionFeatures, uint64(rv.ImageFeatureSet)) if err != nil { return fmt.Errorf("failed to set image features: %w", err) } @@ -1527,7 +1529,7 @@ func (rv *rbdVolume) getImageInfo() error { if err != nil { return err } - rv.imageFeatureSet = librbd.FeatureSet(features) + rv.ImageFeatureSet = librbd.FeatureSet(features) // Get parent information. parentInfo, err := image.GetParent() diff --git a/internal/rbd/rbd_util_test.go b/internal/rbd/rbd_util_test.go index 2174ca2ea..6007106ba 100644 --- a/internal/rbd/rbd_util_test.go +++ b/internal/rbd/rbd_util_test.go @@ -41,7 +41,7 @@ func TestHasSnapshotFeature(t *testing.T) { rv := rbdVolume{} for _, test := range tests { - rv.imageFeatureSet = librbd.FeatureSetFromNames(strings.Split(test.features, ",")) + rv.ImageFeatureSet = librbd.FeatureSetFromNames(strings.Split(test.features, ",")) if got := rv.hasSnapshotFeature(); got != test.hasFeature { t.Errorf("hasSnapshotFeature(%s) = %t, want %t", test.features, got, test.hasFeature) } diff --git a/internal/rbd/replicationcontrollerserver.go b/internal/rbd/replicationcontrollerserver.go index d41fa7c6a..51c0ab6ac 100644 --- a/internal/rbd/replicationcontrollerserver.go +++ b/internal/rbd/replicationcontrollerserver.go @@ -324,7 +324,7 @@ func createDummyImage(ctx context.Context, rbdVol *rbdVolume) error { librbd.FeatureNameFastDiff, } features := librbd.FeatureSetFromNames(f) - dummyVol.imageFeatureSet = features + dummyVol.ImageFeatureSet = features // create 1MiB dummy image. 1MiB=1048576 bytes dummyVol.VolSize = 1048576 err = createImage(ctx, &dummyVol, dummyVol.conn.Creds) From ff91b7edbd91f65c2b369b496f04c22b910b22b5 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 20:48:35 +0530 Subject: [PATCH 23/30] rbd: get image details after creating clone after creating the clone get the current image details like size, creationTime, imageFeatures etc from the ceph cluster. Signed-off-by: Madhu Rajanna --- internal/rbd/rbd_util.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index b91fb1107..94571ea6f 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -1503,6 +1503,12 @@ func (rv *rbdVolume) cloneRbdImageFromSnapshot( } } + // get image latest information + err = rv.getImageInfo() + if err != nil { + return fmt.Errorf("failed to get image info of %s: %w", rv, err) + } + // Success! Do not delete the cloned image now :) deleteClone = false From 8c9105f09e6a618f1d573b031bcf516c5e8c7935 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 20:50:04 +0530 Subject: [PATCH 24/30] rbd: remove extra getImageInfo API call as getImageInfo is already called inside cloneRbdImageFromSnapshot function right after creating the clone. remove the extra API call to get the details again. Signed-off-by: Madhu Rajanna --- internal/rbd/snapshot.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/internal/rbd/snapshot.go b/internal/rbd/snapshot.go index f15f41db1..62191666b 100644 --- a/internal/rbd/snapshot.go +++ b/internal/rbd/snapshot.go @@ -64,17 +64,6 @@ func createRBDClone( return err } - err = cloneRbdVol.getImageInfo() - if err != nil { - log.ErrorLog(ctx, "failed to get rbd image: %s details with error: %v", cloneRbdVol, err) - delErr := deleteImage(ctx, cloneRbdVol, cr) - if delErr != nil { - log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", cloneRbdVol, delErr) - } - - return err - } - return nil } From 549bfedc94745e3558f4d2a836631a6b96c3905b Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 20:59:37 +0530 Subject: [PATCH 25/30] rbd: remove extra logging from createBackingImage we are already logging the rbd image details and the snapshot details after creating the clone. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 1c72357a8..060e862d3 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -637,7 +637,6 @@ func (cs *ControllerServer) createBackingImage( if err != nil { return err } - log.DebugLog(ctx, "created volume %s from snapshot %s", rbdVol.RequestName, rbdSnap.RbdSnapName) case parentVol != nil: if err = cs.OperationLocks.GetCloneLock(parentVol.VolID); err != nil { log.ErrorLog(ctx, err.Error()) From 9499e73b93d1ef2343fe84e8df5f3709d160f7e8 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 20 Dec 2021 21:01:17 +0530 Subject: [PATCH 26/30] rbd: correct logging in createBackingImage after creating the rbd image log the image details corresponding for the request along with the request name. Signed-off-by: Madhu Rajanna --- internal/rbd/controllerserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 060e862d3..b689f89fe 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -655,7 +655,7 @@ func (cs *ControllerServer) createBackingImage( } } - log.DebugLog(ctx, "created volume %s backed by image %s", rbdVol.RequestName, rbdVol.RbdImageName) + log.DebugLog(ctx, "created image %s backed for request name %s", rbdVol, rbdVol.RequestName) defer func() { if err != nil { From e87c0ac275b847ecb5082c1ac7c7b897553f834a Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 21 Dec 2021 19:53:26 +0530 Subject: [PATCH 27/30] e2e: add Copyright header of e2e files added Copyright header for files inside the e2e folder. Signed-off-by: Madhu Rajanna --- e2e/ceph_user.go | 16 ++++++++++++++++ e2e/cephfs.go | 16 ++++++++++++++++ e2e/cephfs_helper.go | 16 ++++++++++++++++ e2e/configmap.go | 16 ++++++++++++++++ e2e/deploy-vault.go | 16 ++++++++++++++++ e2e/e2e_test.go | 16 ++++++++++++++++ e2e/kms.go | 16 ++++++++++++++++ e2e/migration.go | 16 ++++++++++++++++ e2e/namespace.go | 16 ++++++++++++++++ e2e/node.go | 16 ++++++++++++++++ e2e/pod.go | 16 ++++++++++++++++ e2e/pvc.go | 16 ++++++++++++++++ e2e/rbd.go | 16 ++++++++++++++++ e2e/rbd_helper.go | 16 ++++++++++++++++ e2e/resize.go | 16 ++++++++++++++++ e2e/snapshot.go | 16 ++++++++++++++++ e2e/staticpvc.go | 16 ++++++++++++++++ e2e/upgrade-cephfs.go | 16 ++++++++++++++++ e2e/upgrade-rbd.go | 16 ++++++++++++++++ e2e/upgrade.go | 16 ++++++++++++++++ e2e/utils.go | 16 ++++++++++++++++ 21 files changed, 336 insertions(+) diff --git a/e2e/ceph_user.go b/e2e/ceph_user.go index f3f5ba5cd..8860546ef 100644 --- a/e2e/ceph_user.go +++ b/e2e/ceph_user.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/cephfs.go b/e2e/cephfs.go index 452b480e4..147d74d2f 100644 --- a/e2e/cephfs.go +++ b/e2e/cephfs.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/cephfs_helper.go b/e2e/cephfs_helper.go index 28e8d3eb9..7c6c941bb 100644 --- a/e2e/cephfs_helper.go +++ b/e2e/cephfs_helper.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/configmap.go b/e2e/configmap.go index b02609b24..932f37fe0 100644 --- a/e2e/configmap.go +++ b/e2e/configmap.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/deploy-vault.go b/e2e/deploy-vault.go index c69c66da3..6fc8f5237 100644 --- a/e2e/deploy-vault.go +++ b/e2e/deploy-vault.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index ff9f0cbe7..b7d39e917 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/kms.go b/e2e/kms.go index cf387f5ed..0d3ae9655 100644 --- a/e2e/kms.go +++ b/e2e/kms.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/migration.go b/e2e/migration.go index 2173f1202..114718b98 100644 --- a/e2e/migration.go +++ b/e2e/migration.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/namespace.go b/e2e/namespace.go index 4ba690ff6..24cc2bddc 100644 --- a/e2e/namespace.go +++ b/e2e/namespace.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/node.go b/e2e/node.go index b195b1db9..a8e624529 100644 --- a/e2e/node.go +++ b/e2e/node.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/pod.go b/e2e/pod.go index 9f0127d09..6e76881ec 100644 --- a/e2e/pod.go +++ b/e2e/pod.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/pvc.go b/e2e/pvc.go index 91fb123ea..a46765d5d 100644 --- a/e2e/pvc.go +++ b/e2e/pvc.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/rbd.go b/e2e/rbd.go index 3b6d42a7e..9f75791f1 100644 --- a/e2e/rbd.go +++ b/e2e/rbd.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/rbd_helper.go b/e2e/rbd_helper.go index 8ebcfc4b1..1d909e7f7 100644 --- a/e2e/rbd_helper.go +++ b/e2e/rbd_helper.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/resize.go b/e2e/resize.go index 429377f60..9a6b42a3a 100644 --- a/e2e/resize.go +++ b/e2e/resize.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/snapshot.go b/e2e/snapshot.go index b6d15c750..6f2f98d3c 100644 --- a/e2e/snapshot.go +++ b/e2e/snapshot.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/staticpvc.go b/e2e/staticpvc.go index 28cd945a8..4e75a051d 100644 --- a/e2e/staticpvc.go +++ b/e2e/staticpvc.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/upgrade-cephfs.go b/e2e/upgrade-cephfs.go index 236675841..235302abd 100644 --- a/e2e/upgrade-cephfs.go +++ b/e2e/upgrade-cephfs.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/upgrade-rbd.go b/e2e/upgrade-rbd.go index 2db1e419a..3625b827d 100644 --- a/e2e/upgrade-rbd.go +++ b/e2e/upgrade-rbd.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/upgrade.go b/e2e/upgrade.go index 58859ad3d..29f3ea60e 100644 --- a/e2e/upgrade.go +++ b/e2e/upgrade.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( diff --git a/e2e/utils.go b/e2e/utils.go index 2c6b3ece9..b2f00b1f8 100644 --- a/e2e/utils.go +++ b/e2e/utils.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package e2e import ( From 2b7fb1bbb1c2b4fc1409dc53f41cb2aa54ef530f Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 21 Dec 2021 11:22:45 +0530 Subject: [PATCH 28/30] doc: lift supported ceph version for snapshot/clone snapshot and clone should be supported from octopus 15.2.4 instead of 15.2.3. fixes #2732 Signed-off-by: Madhu Rajanna --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4a54fc276..e5600f232 100644 --- a/README.md +++ b/README.md @@ -96,9 +96,9 @@ for its support details. | CephFS | Dynamically provision, de-provision File mode RWO volume | Beta | >= v1.1.0 | >= v1.0.0 | Nautilus (>=14.2.2) | >= v1.14.0 | | | Dynamically provision, de-provision File mode RWX volume | Beta | >= v1.1.0 | >= v1.0.0 | Nautilus (>=v14.2.2) | >= v1.14.0 | | | Dynamically provision, de-provision File mode ROX volume | Alpha | >= v3.0.0 | >= v1.0.0 | Nautilus (>=v14.2.2) | >= v1.14.0 | -| | Creating and deleting snapshot | Beta | >= v3.1.0 | >= v1.0.0 | Octopus (>=v15.2.3) | >= v1.17.0 | -| | Provision volume from snapshot | Beta | >= v3.1.0 | >= v1.0.0 | Octopus (>=v15.2.3) | >= v1.17.0 | -| | Provision volume from another volume | Beta | >= v3.1.0 | >= v1.0.0 | Octopus (>=v15.2.3) | >= v1.16.0 | +| | Creating and deleting snapshot | Beta | >= v3.1.0 | >= v1.0.0 | Octopus (>=v15.2.4) | >= v1.17.0 | +| | Provision volume from snapshot | Beta | >= v3.1.0 | >= v1.0.0 | Octopus (>=v15.2.4) | >= v1.17.0 | +| | Provision volume from another volume | Beta | >= v3.1.0 | >= v1.0.0 | Octopus (>=v15.2.4) | >= v1.16.0 | | | Expand volume | Beta | >= v2.0.0 | >= v1.1.0 | Nautilus (>=v14.2.2) | >= v1.15.0 | | | Volume/PV Metrics of File Mode Volume | Beta | >= v1.2.0 | >= v1.1.0 | Nautilus (>=v14.2.2) | >= v1.15.0 | From 95e9595c1fa673e57aed07ab0409182a6e1a4692 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 22 Dec 2021 08:37:25 +0530 Subject: [PATCH 29/30] util: add helper ExecCommandWithTimeout function added ExecCommandWithTimeout helper function to execute the commands with the timeout option, if the command does not return any response with in the timeout time the process will be terminated and error will be returned back to the user. Signed-off-by: Madhu Rajanna --- internal/util/cephcmds.go | 54 +++++++++++++++++++++ internal/util/cephcmds_test.go | 89 ++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 internal/util/cephcmds_test.go diff --git a/internal/util/cephcmds.go b/internal/util/cephcmds.go index a405f4973..66c32b196 100644 --- a/internal/util/cephcmds.go +++ b/internal/util/cephcmds.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "os/exec" + "time" "github.com/ceph/ceph-csi/internal/util/log" @@ -65,6 +66,59 @@ func ExecCommand(ctx context.Context, program string, args ...string) (string, s return stdout, stderr, nil } +// ExecCommandWithTimeout executes passed in program with args, timeout and +// returns separate stdout and stderr streams. If the command is not executed +// within given timeout, the process will be killed. In case ctx is not set to +// context.TODO(), the command will be logged after it was executed. +func ExecCommandWithTimeout( + ctx context.Context, + timeout time.Duration, + program string, + args ...string) ( + string, + string, + error) { + var ( + sanitizedArgs = StripSecretInArgs(args) + stdoutBuf bytes.Buffer + stderrBuf bytes.Buffer + ) + + cctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + cmd := exec.CommandContext(cctx, program, args...) // #nosec:G204, commands executing not vulnerable. + cmd.Stdout = &stdoutBuf + cmd.Stderr = &stderrBuf + + err := cmd.Run() + stdout := stdoutBuf.String() + stderr := stderrBuf.String() + if err != nil { + // if its a timeout log return context deadline exceeded error message + if errors.Is(cctx.Err(), context.DeadlineExceeded) { + err = fmt.Errorf("timeout: %w", cctx.Err()) + } + err = fmt.Errorf("an error (%w) and stderror (%s) occurred while running %s args: %v", + err, + stderr, + program, + sanitizedArgs) + + if ctx != context.TODO() { + log.ErrorLog(ctx, "%s", err) + } + + return stdout, stderr, err + } + + if ctx != context.TODO() { + log.UsefulLog(ctx, "command succeeded: %s %v", program, sanitizedArgs) + } + + return stdout, stderr, nil +} + // GetPoolID fetches the ID of the pool that matches the passed in poolName // parameter. func GetPoolID(monitors string, cr *Credentials, poolName string) (int64, error) { diff --git a/internal/util/cephcmds_test.go b/internal/util/cephcmds_test.go new file mode 100644 index 000000000..134eb34f0 --- /dev/null +++ b/internal/util/cephcmds_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2021 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestExecCommandWithTimeout(t *testing.T) { + t.Parallel() + type args struct { + ctx context.Context + program string + timeout time.Duration + args []string + } + tests := []struct { + name string + args args + stdout string + expectedErr error + wantErr bool + }{ + { + name: "echo hello", + args: args{ + ctx: context.TODO(), + program: "echo", + timeout: time.Second, + args: []string{"hello"}, + }, + stdout: "hello\n", + expectedErr: nil, + wantErr: false, + }, + { + name: "sleep with timeout", + args: args{ + ctx: context.TODO(), + program: "sleep", + timeout: time.Second, + args: []string{"3"}, + }, + stdout: "", + expectedErr: context.DeadlineExceeded, + wantErr: true, + }, + } + for _, tt := range tests { + newtt := tt + t.Run(newtt.name, func(t *testing.T) { + t.Parallel() + stdout, _, err := ExecCommandWithTimeout(newtt.args.ctx, + newtt.args.timeout, + newtt.args.program, + newtt.args.args...) + if (err != nil) != newtt.wantErr { + t.Errorf("ExecCommandWithTimeout() error = %v, wantErr %v", err, newtt.wantErr) + + return + } + + if newtt.wantErr && !errors.Is(err, newtt.expectedErr) { + t.Errorf("ExecCommandWithTimeout() error expected got = %v, want %v", err, newtt.expectedErr) + } + + if stdout != newtt.stdout { + t.Errorf("ExecCommandWithTimeout() got = %v, want %v", stdout, newtt.stdout) + } + }) + } +} From e4b7943bacde6ce5af593b7e97b1902eaa32956c Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 22 Dec 2021 09:06:37 +0530 Subject: [PATCH 30/30] rbd: add workaround for force promote use ExecCommandWithTimeout with timeout of 1 minute for the promote operation. If the command doesnot returns error/response in 1 minute the process will be killed and error will be returned to the user. Signed-off-by: Madhu Rajanna --- internal/rbd/mirror.go | 33 +++++++++++++++++++++ internal/rbd/replicationcontrollerserver.go | 8 ++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/internal/rbd/mirror.go b/internal/rbd/mirror.go index 0d010be01..504268a12 100644 --- a/internal/rbd/mirror.go +++ b/internal/rbd/mirror.go @@ -16,7 +16,11 @@ limitations under the License. package rbd import ( + "context" "fmt" + "time" + + "github.com/ceph/ceph-csi/internal/util" librbd "github.com/ceph/go-ceph/rbd" ) @@ -84,6 +88,35 @@ func (ri *rbdImage) promoteImage(force bool) error { return nil } +// forcePromoteImage promotes image to primary with force option with 1 minute +// timeout. If there is no response within 1 minute,the rbd CLI process will be +// killed and an error is returned. +func (rv *rbdVolume) forcePromoteImage(cr *util.Credentials) error { + promoteArgs := []string{ + "mirror", "image", "promote", + rv.String(), + "--force", + "--id", cr.ID, + "-m", rv.Monitors, + "--keyfile=" + cr.KeyFile, + } + _, stderr, err := util.ExecCommandWithTimeout( + context.TODO(), + time.Minute, + "rbd", + promoteArgs..., + ) + if err != nil { + return fmt.Errorf("failed to promote image %q with error: %w", rv, err) + } + + if stderr != "" { + return fmt.Errorf("failed to promote image %q with stderror: %s", rv, stderr) + } + + return nil +} + // demoteImage demotes image to secondary. func (ri *rbdImage) demoteImage() error { image, err := ri.open() diff --git a/internal/rbd/replicationcontrollerserver.go b/internal/rbd/replicationcontrollerserver.go index 51c0ab6ac..521b4c951 100644 --- a/internal/rbd/replicationcontrollerserver.go +++ b/internal/rbd/replicationcontrollerserver.go @@ -557,7 +557,13 @@ func (rs *ReplicationServer) PromoteVolume(ctx context.Context, // promote secondary to primary if !mirroringInfo.Primary { - err = rbdVol.promoteImage(req.Force) + if req.GetForce() { + // workaround for https://github.com/ceph/ceph-csi/issues/2736 + // TODO: remove this workaround when the issue is fixed + err = rbdVol.forcePromoteImage(cr) + } else { + err = rbdVol.promoteImage(req.GetForce()) + } if err != nil { log.ErrorLog(ctx, err.Error()) // In case of the DR the image on the primary site cannot be