From 291d04bdf9064f1003db04daf4a38203303a537b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B3bert=20Va=C5=A1ek?= Date: Thu, 1 Jul 2021 07:21:54 +0200 Subject: [PATCH] [manila-csi-plugin] Add support for online volume expansion (#1579) * implemented ControllerExpandVolume with online volume expansion support * updated CSI sanity tests * updated Helm chart * updated k8s manifests * updated docs * updated examples * docs: extend -> expand --- charts/manila-csi-plugin/Chart.yaml | 2 +- .../controllerplugin-rules-clusterrole.yaml | 5 +- .../controllerplugin-statefulset.yaml | 15 +++++ charts/manila-csi-plugin/values.yaml | 7 +++ .../using-manila-csi-plugin.md | 2 +- .../dynamic-provisioning/storageclass.yaml | 3 + .../csi-controllerplugin-rbac.yaml | 5 +- .../csi-controllerplugin.yaml | 13 +++++ pkg/csi/manila/controllerserver.go | 55 ++++++++++++++++++- pkg/csi/manila/driver.go | 1 + pkg/csi/manila/identityserver.go | 7 +++ pkg/csi/manila/manilaclient/client.go | 4 ++ pkg/csi/manila/manilaclient/interface.go | 1 + pkg/csi/manila/share.go | 24 ++++++++ pkg/csi/manila/util.go | 16 ++++++ tests/sanity/manila/fake-secrets.yaml | 7 +++ tests/sanity/manila/fakemanilaclient.go | 24 +++++++- 17 files changed, 185 insertions(+), 6 deletions(-) diff --git a/charts/manila-csi-plugin/Chart.yaml b/charts/manila-csi-plugin/Chart.yaml index 42e2103071..d2184de0e1 100644 --- a/charts/manila-csi-plugin/Chart.yaml +++ b/charts/manila-csi-plugin/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v1 appVersion: latest description: Manila CSI Chart for OpenStack name: openstack-manila-csi -version: 1.2.1 +version: 1.3.0 home: http://github.com/kubernetes/cloud-provider-openstack icon: https://github.com/kubernetes/kubernetes/blob/master/logo/logo.png maintainers: diff --git a/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml b/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml index 1571fa1a92..bbdf8033ac 100644 --- a/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml +++ b/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml @@ -18,10 +18,13 @@ rules: verbs: ["get", "list"] - apiGroups: [""] resources: ["persistentvolumes"] - verbs: ["get", "list", "watch", "create", "delete"] + verbs: ["get", "list", "watch", "create", "delete", "patch"] - apiGroups: [""] resources: ["persistentvolumeclaims"] verbs: ["get", "list", "watch", "update"] + - apiGroups: [""] + resources: ["persistentvolumeclaims/status"] + verbs: ["patch"] - apiGroups: [""] resources: ["events"] verbs: ["list", "watch", "create", "update", "patch"] diff --git a/charts/manila-csi-plugin/templates/controllerplugin-statefulset.yaml b/charts/manila-csi-plugin/templates/controllerplugin-statefulset.yaml index aaabfb8fb5..fc5e6ca5a9 100644 --- a/charts/manila-csi-plugin/templates/controllerplugin-statefulset.yaml +++ b/charts/manila-csi-plugin/templates/controllerplugin-statefulset.yaml @@ -59,6 +59,21 @@ spec: mountPath: /var/lib/kubelet/plugins/{{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }} resources: {{ toYaml $.Values.controllerplugin.snapshotter.resources | indent 12 }} + - name: {{ .protocolSelector | lower }}-resizer + image: "{{ $.Values.controllerplugin.resizer.image.repository }}:{{ $.Values.controllerplugin.resizer.image.tag }}" + args: + - "--v=5" + - "--csi-address=$(ADDRESS)" + - "--handle-volume-inuse-error=false" + env: + - name: ADDRESS + value: "unix:///var/lib/kubelet/plugins/{{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }}/csi-controllerplugin.sock" + imagePullPolicy: {{ $.Values.controllerplugin.resizer.image.pullPolicy }} + volumeMounts: + - name: {{ .protocolSelector | lower }}-plugin-dir + mountPath: /var/lib/kubelet/plugins/{{ printf "%s.%s" .protocolSelector $.Values.driverName | lower }} + resources: +{{ toYaml $.Values.controllerplugin.resizer.resources | indent 12 }} - name: {{ .protocolSelector | lower }}-nodeplugin securityContext: privileged: true diff --git a/charts/manila-csi-plugin/values.yaml b/charts/manila-csi-plugin/values.yaml index adc368d1f7..1db39db028 100644 --- a/charts/manila-csi-plugin/values.yaml +++ b/charts/manila-csi-plugin/values.yaml @@ -84,6 +84,13 @@ controllerplugin: tag: v2.1.3 pullPolicy: IfNotPresent resources: {} + # CSI external-resizer container spec + resizer: + image: + repository: k8s.gcr.io/sig-storage/csi-resizer + tag: v1.2.0 + pullPolicy: IfNotPresent + resources: {} nodeSelector: {} tolerations: [] affinity: {} diff --git a/docs/manila-csi-plugin/using-manila-csi-plugin.md b/docs/manila-csi-plugin/using-manila-csi-plugin.md index f582322afc..4fa9b98929 100644 --- a/docs/manila-csi-plugin/using-manila-csi-plugin.md +++ b/docs/manila-csi-plugin/using-manila-csi-plugin.md @@ -21,7 +21,7 @@ # CSI Manila driver -The CSI Manila driver is able to create and mount OpenStack Manila shares. Snapshots and recovering shares from snapshots is supported as well (support for CephFS snapshots will be added soon). +The CSI Manila driver is able to create, expand and mount OpenStack Manila shares. Snapshots and recovering shares from snapshots is supported as well (support for CephFS snapshots will be added soon). ## Configuration diff --git a/examples/manila-csi-plugin/nfs/dynamic-provisioning/storageclass.yaml b/examples/manila-csi-plugin/nfs/dynamic-provisioning/storageclass.yaml index ad690767ce..31a5c838e1 100644 --- a/examples/manila-csi-plugin/nfs/dynamic-provisioning/storageclass.yaml +++ b/examples/manila-csi-plugin/nfs/dynamic-provisioning/storageclass.yaml @@ -3,12 +3,15 @@ kind: StorageClass metadata: name: csi-manila-nfs provisioner: nfs.manila.csi.openstack.org +allowVolumeExpansion: true parameters: # Manila share type type: default csi.storage.k8s.io/provisioner-secret-name: csi-manila-secrets csi.storage.k8s.io/provisioner-secret-namespace: default + csi.storage.k8s.io/controller-expand-secret-name: csi-manila-secrets + csi.storage.k8s.io/controller-expand-secret-namespace: default csi.storage.k8s.io/node-stage-secret-name: csi-manila-secrets csi.storage.k8s.io/node-stage-secret-namespace: default csi.storage.k8s.io/node-publish-secret-name: csi-manila-secrets diff --git a/manifests/manila-csi-plugin/csi-controllerplugin-rbac.yaml b/manifests/manila-csi-plugin/csi-controllerplugin-rbac.yaml index e05d8a439b..71b8270407 100644 --- a/manifests/manila-csi-plugin/csi-controllerplugin-rbac.yaml +++ b/manifests/manila-csi-plugin/csi-controllerplugin-rbac.yaml @@ -36,10 +36,13 @@ rules: verbs: ["get", "list"] - apiGroups: [""] resources: ["persistentvolumes"] - verbs: ["get", "list", "watch", "create", "delete"] + verbs: ["get", "list", "watch", "create", "delete", "patch"] - apiGroups: [""] resources: ["persistentvolumeclaims"] verbs: ["get", "list", "watch", "update"] + - apiGroups: [""] + resources: ["persistentvolumeclaims/status"] + verbs: ["patch"] - apiGroups: [""] resources: ["events"] verbs: ["list", "watch", "create", "update", "patch"] diff --git a/manifests/manila-csi-plugin/csi-controllerplugin.yaml b/manifests/manila-csi-plugin/csi-controllerplugin.yaml index 7c9ea85faa..4faaf7318e 100644 --- a/manifests/manila-csi-plugin/csi-controllerplugin.yaml +++ b/manifests/manila-csi-plugin/csi-controllerplugin.yaml @@ -61,6 +61,19 @@ spec: volumeMounts: - name: plugin-dir mountPath: /var/lib/kubelet/plugins/manila.csi.openstack.org + - name: resizer + image: "k8s.gcr.io/sig-storage/csi-resizer:v1.2.0" + args: + - "--v=5" + - "--csi-address=$(ADDRESS)" + - "--handle-volume-inuse-error=false" + env: + - name: ADDRESS + value: "unix:///var/lib/kubelet/plugins/manila.csi.openstack.org/csi-controllerplugin.sock" + imagePullPolicy: IfNotPresent + volumeMounts: + - name: plugin-dir + mountPath: /var/lib/kubelet/plugins/manila.csi.openstack.org - name: nodeplugin securityContext: privileged: true diff --git a/pkg/csi/manila/controllerserver.go b/pkg/csi/manila/controllerserver.go index 057edd3958..317843205e 100644 --- a/pkg/csi/manila/controllerserver.go +++ b/pkg/csi/manila/controllerserver.go @@ -424,7 +424,60 @@ func (cs *controllerServer) ListSnapshots(context.Context, *csi.ListSnapshotsReq } func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) { - return nil, status.Error(codes.Unimplemented, "") + if err := validateControllerExpandVolumeRequest(req); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + // Configuration + + osOpts, err := options.NewOpenstackOptions(req.GetSecrets()) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "invalid OpenStack secrets: %v", err) + } + + manilaClient, err := cs.d.manilaClientBuilder.New(osOpts) + if err != nil { + return nil, status.Errorf(codes.Unauthenticated, "failed to create Manila v2 client: %v", err) + } + + // Retrieve the share by its ID + + share, err := manilaClient.GetShareByID(req.GetVolumeId()) + if err != nil { + if clouderrors.IsNotFound(err) { + return nil, status.Errorf(codes.NotFound, "share %s not found: %v", req.GetVolumeId(), err) + } + + return nil, status.Errorf(codes.Internal, "failed to retrieve share %s: %v", req.GetVolumeId(), err) + } + + // Check for pending operations on this volume + if _, isPending := pendingVolumes.LoadOrStore(share.Name, true); isPending { + return nil, status.Errorf(codes.Aborted, "volume named %s is already being processed", share.Name) + } + defer pendingVolumes.Delete(share.Name) + + // Try to expand the share + + currentSizeInBytes := int64(share.Size * bytesInGiB) + + if currentSizeInBytes >= req.GetCapacityRange().GetRequiredBytes() { + // Share is already larger than requested size + + return &csi.ControllerExpandVolumeResponse{ + CapacityBytes: currentSizeInBytes, + }, nil + } + + desiredSizeInGiB := int(req.GetCapacityRange().GetRequiredBytes() / bytesInGiB) + + if err = extendShare(share, desiredSizeInGiB, manilaClient); err != nil { + return nil, err + } + + return &csi.ControllerExpandVolumeResponse{ + CapacityBytes: int64(desiredSizeInGiB * bytesInGiB), + }, nil } func (cs *controllerServer) ControllerGetVolume(context.Context, *csi.ControllerGetVolumeRequest) (*csi.ControllerGetVolumeResponse, error) { diff --git a/pkg/csi/manila/driver.go b/pkg/csi/manila/driver.go index 630c4c71bb..bb7b09419e 100644 --- a/pkg/csi/manila/driver.go +++ b/pkg/csi/manila/driver.go @@ -153,6 +153,7 @@ func NewDriver(o *DriverOpts) (*Driver, error) { d.addControllerServiceCapabilities([]csi.ControllerServiceCapability_RPC_Type{ csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT, + csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, }) d.addVolumeCapabilityAccessModes([]csi.VolumeCapability_AccessMode_Mode{ diff --git a/pkg/csi/manila/identityserver.go b/pkg/csi/manila/identityserver.go index 5ffdad8624..df2b00a847 100644 --- a/pkg/csi/manila/identityserver.go +++ b/pkg/csi/manila/identityserver.go @@ -57,6 +57,13 @@ func (ids *identityServer) GetPluginCapabilities(ctx context.Context, req *csi.G }, }, }, + { + Type: &csi.PluginCapability_VolumeExpansion_{ + VolumeExpansion: &csi.PluginCapability_VolumeExpansion{ + Type: csi.PluginCapability_VolumeExpansion_ONLINE, + }, + }, + }, } if ids.d.withTopology { diff --git a/pkg/csi/manila/manilaclient/client.go b/pkg/csi/manila/manilaclient/client.go index 4f507551bc..46a9a0cc06 100644 --- a/pkg/csi/manila/manilaclient/client.go +++ b/pkg/csi/manila/manilaclient/client.go @@ -52,6 +52,10 @@ func (c Client) DeleteShare(shareID string) error { return shares.Delete(c.c, shareID).ExtractErr() } +func (c Client) ExtendShare(shareID string, opts shares.ExtendOptsBuilder) error { + return shares.Extend(c.c, shareID, opts).ExtractErr() +} + func (c Client) GetExportLocations(shareID string) ([]shares.ExportLocation, error) { return shares.ListExportLocations(c.c, shareID).Extract() } diff --git a/pkg/csi/manila/manilaclient/interface.go b/pkg/csi/manila/manilaclient/interface.go index 3671ac354b..0e01c64138 100644 --- a/pkg/csi/manila/manilaclient/interface.go +++ b/pkg/csi/manila/manilaclient/interface.go @@ -29,6 +29,7 @@ type Interface interface { GetShareByName(shareName string) (*shares.Share, error) CreateShare(opts shares.CreateOptsBuilder) (*shares.Share, error) DeleteShare(shareID string) error + ExtendShare(shareID string, opts shares.ExtendOptsBuilder) error GetExportLocations(shareID string) ([]shares.ExportLocation, error) diff --git a/pkg/csi/manila/share.go b/pkg/csi/manila/share.go index 92dd6e6ab6..1c358084f5 100644 --- a/pkg/csi/manila/share.go +++ b/pkg/csi/manila/share.go @@ -21,6 +21,8 @@ import ( "time" "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/shares" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" clouderrors "k8s.io/cloud-provider-openstack/pkg/util/errors" @@ -33,6 +35,7 @@ const ( shareCreating = "creating" shareDeleting = "deleting" + shareExtending = "extending" shareError = "error" shareErrorDeleting = "error_deleting" shareAvailable = "available" @@ -104,6 +107,27 @@ func tryDeleteShare(share *shares.Share, manilaClient manilaclient.Interface) { } } +func extendShare(share *shares.Share, newSizeInGiB int, manilaClient manilaclient.Interface) error { + opts := shares.ExtendOpts{ + NewSize: newSizeInGiB, + } + + if err := manilaClient.ExtendShare(share.ID, opts); err != nil { + return err + } + + share, manilaErrCode, err := waitForShareStatus(share.ID, shareExtending, shareAvailable, false, manilaClient) + if err != nil { + if err == wait.ErrWaitTimeout { + return status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for share %s to become available", share.ID) + } + + return status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to resize share %s: %v", share.ID, err) + } + + return nil +} + func waitForShareStatus(shareID, currentStatus, desiredStatus string, successOnNotFound bool, manilaClient manilaclient.Interface) (*shares.Share, manilaError, error) { var ( backoff = wait.Backoff{ diff --git a/pkg/csi/manila/util.go b/pkg/csi/manila/util.go index 642d49e54e..ec45e6aa8b 100644 --- a/pkg/csi/manila/util.go +++ b/pkg/csi/manila/util.go @@ -303,6 +303,22 @@ func validateValidateVolumeCapabilitiesRequest(req *csi.ValidateVolumeCapabiliti return nil } +func validateControllerExpandVolumeRequest(req *csi.ControllerExpandVolumeRequest) error { + if req.GetVolumeId() == "" { + return errors.New("volume ID missing in request") + } + + if req.GetCapacityRange() == nil { + return errors.New("capacity range cannot be nil") + } + + if req.GetSecrets() == nil || len(req.GetSecrets()) == 0 { + return errors.New("volume expand secrets cannot be nil or empty") + } + + return nil +} + // // Node service request validation // diff --git a/tests/sanity/manila/fake-secrets.yaml b/tests/sanity/manila/fake-secrets.yaml index 4cb4a79885..ae6930c339 100644 --- a/tests/sanity/manila/fake-secrets.yaml +++ b/tests/sanity/manila/fake-secrets.yaml @@ -33,6 +33,13 @@ ControllerValidateVolumeCapabilitiesSecret: os-password: fake-password os-domainID: fake-domain-id os-projectID: fake-project-id +ControllerExpandVolumeSecret: + os-authURL: fake-url + os-region: fake-region + os-userID: fake-user-id + os-password: fake-password + os-domainID: fake-domain-id + os-projectID: fake-project-id NodeStageVolumeSecret: os-authURL: fake-url os-region: fake-region diff --git a/tests/sanity/manila/fakemanilaclient.go b/tests/sanity/manila/fakemanilaclient.go index d8cec1da98..a3ee1fc47d 100644 --- a/tests/sanity/manila/fakemanilaclient.go +++ b/tests/sanity/manila/fakemanilaclient.go @@ -17,12 +17,13 @@ limitations under the License. package sanity import ( - "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/sharetypes" + "fmt" "strconv" "github.com/gophercloud/gophercloud" "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/messages" "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/shares" + "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/sharetypes" "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/snapshots" "k8s.io/cloud-provider-openstack/pkg/client" "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" @@ -117,6 +118,27 @@ func (c fakeManilaClient) DeleteShare(shareID string) error { return nil } +func (c fakeManilaClient) ExtendShare(shareID string, opts shares.ExtendOptsBuilder) error { + share, err := c.GetShareByID(shareID) + if err != nil { + return err + } + + var res shares.ExtendResult + res.Body = opts + + extendOpts := &shares.ExtendOpts{} + if err := res.ExtractInto(extendOpts); err != nil { + return err + } + + if extendOpts.NewSize < share.Size { + return fmt.Errorf("new size is smaller than old size: %d < %d", extendOpts.NewSize, share.Size) + } + + return nil +} + func (c fakeManilaClient) GetExportLocations(shareID string) ([]shares.ExportLocation, error) { if !shareExists(shareID) { return nil, gophercloud.ErrResourceNotFound{}