diff --git a/docs/manila-csi-plugin/using-manila-csi-plugin.md b/docs/manila-csi-plugin/using-manila-csi-plugin.md index f6914e6050..54e1488afa 100644 --- a/docs/manila-csi-plugin/using-manila-csi-plugin.md +++ b/docs/manila-csi-plugin/using-manila-csi-plugin.md @@ -21,7 +21,9 @@ # CSI Manila driver -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). +The CSI Manila driver is able to create, expand, snapshot, restore and mount OpenStack Manila shares. + +Currently supported Manila backends are NFS and native CephFS. ## Configuration diff --git a/go.mod b/go.mod index 6b0a239e5a..b2989f8b7d 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/MichaelTJones/walk v0.0.0-20161122175330-4748e29d5718 // indirect github.com/container-storage-interface/spec v1.3.0 github.com/golang/protobuf v1.4.3 - github.com/gophercloud/gophercloud v0.15.1-0.20210105012856-e34a44dc6580 + github.com/gophercloud/gophercloud v0.19.0 github.com/gophercloud/utils v0.0.0-20200423144003-7c72efc7435d github.com/gorilla/mux v1.8.0 github.com/hashicorp/go-version v1.2.0 diff --git a/go.sum b/go.sum index 9cc31ea862..ccf8fcd226 100644 --- a/go.sum +++ b/go.sum @@ -426,6 +426,8 @@ github.com/gophercloud/gophercloud v0.1.0/go.mod h1:vxM41WHh5uqHVBMZHzuwNOHh8XEo github.com/gophercloud/gophercloud v0.6.1-0.20191122030953-d8ac278c1c9d/go.mod h1:ozGNgr9KYOVATV5jsgHl/ceCDXGuguqOZAzoQ/2vcNM= github.com/gophercloud/gophercloud v0.15.1-0.20210105012856-e34a44dc6580 h1:EWFhxn19tw6t3LjFe95Kli30sBI02+fPYBqMvXmCvtU= github.com/gophercloud/gophercloud v0.15.1-0.20210105012856-e34a44dc6580/go.mod h1:VX0Ibx85B60B5XOrZr6kaNwrmPUzcmMpwxvQ1WQIIWM= +github.com/gophercloud/gophercloud v0.19.0 h1:zzaIh8W2K5M4AkJhPV1z6O4Sp0FOObzXm61NUmFz3Kw= +github.com/gophercloud/gophercloud v0.19.0/go.mod h1:wRtmUelyIIv3CSSDI47aUwbs075O6i+LY+pXsKCBsb4= github.com/gophercloud/utils v0.0.0-20200423144003-7c72efc7435d h1:fduaPzWwIfvOMLuHk2Al3GZH0XbUqG8MbElPop+Igzs= github.com/gophercloud/utils v0.0.0-20200423144003-7c72efc7435d/go.mod h1:ehWUbLQJPqS0Ep+CxeD559hsm9pthPXadJNKwZkp43w= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 h1:EGx4pi6eqNxGaHF6qqu48+N2wcFQ5qg5FXgOdqsJ5d8= @@ -884,6 +886,7 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20191202143827-86a70503ff7e/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 h1:/ZScEX8SfEmUGRHs0gxpqteO5nfNW6axyZbBdw9A12g= golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= diff --git a/pkg/csi/manila/controllerserver.go b/pkg/csi/manila/controllerserver.go index 9d27008430..4cf8a0cd39 100644 --- a/pkg/csi/manila/controllerserver.go +++ b/pkg/csi/manila/controllerserver.go @@ -45,7 +45,7 @@ var ( pendingSnapshots = sync.Map{} ) -func getVolumeCreator(source *csi.VolumeContentSource, shareOpts *options.ControllerVolumeContext, compatOpts *options.CompatibilityOptions, shareTypeCaps capabilities.ManilaCapabilities) (volumeCreator, error) { +func getVolumeCreator(source *csi.VolumeContentSource, shareOpts *options.ControllerVolumeContext, compatOpts *options.CompatibilityOptions) (volumeCreator, error) { if source == nil { return &blankVolume{}, nil } @@ -55,11 +55,6 @@ func getVolumeCreator(source *csi.VolumeContentSource, shareOpts *options.Contro } if source.GetSnapshot() != nil { - if tryCompatForVolumeSource(compatOpts, shareOpts, source, shareTypeCaps) != nil { - klog.Infof("share type %s does not advertise create_share_from_snapshot_support capability, compatibility mode is available", shareOpts.Type) - return &blankVolume{}, nil - } - return &volumeFromSnapshot{}, nil } @@ -121,7 +116,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol // Retrieve an existing share or create a new one - volCreator, err := getVolumeCreator(req.GetVolumeContentSource(), shareOpts, cs.d.compatOpts, shareTypeCaps) + volCreator, err := getVolumeCreator(req.GetVolumeContentSource(), shareOpts, cs.d.compatOpts) if err != nil { return nil, err } @@ -148,15 +143,6 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, status.Errorf(codes.Internal, "failed to grant access to volume %s: %v", share.Name, err) } - // Check if compatibility layer is needed and can be used - if compatLayer := tryCompatForVolumeSource(cs.d.compatOpts, shareOpts, req.GetVolumeContentSource(), shareTypeCaps); compatLayer != nil { - if err = compatLayer.SupplementCapability(cs.d.compatOpts, share, accessRight, req, cs.d.fwdEndpoint, manilaClient, cs.d.csiClientBuilder); err != nil { - // An error occurred, the user must clean the share manually - // TODO needs proper monitoring - return nil, err - } - } - var accessibleTopology []*csi.Topology if cs.d.withTopology { // All requisite/preferred topologies are considered valid. Nodes from those zones are required to be able to reach the storage. @@ -202,12 +188,6 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol } func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) { - if cs.d.shareProto == "CEPHFS" { - // Restoring shares from CephFS snapshots needs special handling that's not implemented yet. - // TODO: Creating CephFS snapshots is forbidden until CephFS restoration is in place. - return nil, status.Errorf(codes.InvalidArgument, "the driver doesn't support snapshotting CephFS shares yet") - } - if err := validateCreateSnapshotRequest(req); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -246,6 +226,17 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS sourceShare.ShareProto, req.GetSourceVolumeId(), cs.d.shareProto) } + // In order to satisfy CSI spec requirements around CREATE_DELETE_SNAPSHOT + // and the ability to populate volumes with snapshot contents, parent share + // must advertise snapshot_support and create_share_from_snapshot_support + // capabilities. + + if !sourceShare.SnapshotSupport || !sourceShare.CreateShareFromSnapshotSupport { + return nil, status.Errorf(codes.InvalidArgument, + "cannot create snapshot %s for volume %s: parent share must advertise snapshot_support and create_share_from_snapshot_support capabilities", + req.GetName(), req.GetSourceVolumeId()) + } + // Retrieve an existing snapshot or create a new one snapshot, err := getOrCreateSnapshot(req.GetName(), sourceShare.ID, manilaClient) @@ -258,11 +249,11 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS return nil, status.Errorf(codes.NotFound, "failed to create snapshot %s for volume %s because the volume doesn't exist: %v", req.GetName(), req.GetSourceVolumeId(), err) } - return nil, status.Errorf(codes.Internal, "failed to create a snapshot %s of volume %s: %v", req.GetName(), req.GetSourceVolumeId(), err) + return nil, status.Errorf(codes.Internal, "failed to create snapshot %s of volume %s: %v", req.GetName(), req.GetSourceVolumeId(), err) } if err = verifySnapshotCompatibility(snapshot, req); err != nil { - return nil, status.Errorf(codes.AlreadyExists, "a snapshot named %s already exists, but is incompatible with the request: %v", req.GetName(), err) + return nil, status.Errorf(codes.AlreadyExists, "snapshot %s already exists, but is incompatible with the request: %v", req.GetName(), err) } // Check for snapshot status, determine whether it's ready @@ -307,12 +298,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS } func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { - if cs.d.shareProto == "CEPHFS" { - // Restoring shares from CephFS snapshots needs special handling that's not implemented yet. - // TODO: Deleting CephFS snapshots is forbidden until CephFS restoration is in place. - return nil, status.Errorf(codes.InvalidArgument, "the driver doesn't support CephFS snapshots yet") - } - if err := validateDeleteSnapshotRequest(req); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } diff --git a/pkg/csi/manila/share.go b/pkg/csi/manila/share.go index 01269ba443..bb950f2bbc 100644 --- a/pkg/csi/manila/share.go +++ b/pkg/csi/manila/share.go @@ -33,16 +33,31 @@ const ( waitForAvailableShareTimeout = 3 waitForAvailableShareRetries = 10 - shareCreating = "creating" - shareDeleting = "deleting" - shareExtending = "extending" - shareError = "error" - shareErrorDeleting = "error_deleting" - shareAvailable = "available" + shareCreating = "creating" + shareCreatingFromSnapshot = "creating_from_snapshot" + shareDeleting = "deleting" + shareExtending = "extending" + shareError = "error" + shareErrorDeleting = "error_deleting" + shareErrorExtending = "extending_error" + shareAvailable = "available" shareDescription = "provisioned-by=manila.csi.openstack.org" ) +var ( + shareErrorStatuses = map[string]struct{}{ + shareError: {}, + shareErrorDeleting: {}, + shareErrorExtending: {}, + } +) + +func isShareInErrorState(s string) bool { + _, found := shareErrorStatuses[s] + return found +} + // getOrCreateShare first retrieves an existing share with name=shareName, or creates a new one if it doesn't exist yet. // Once the share is created, an exponential back-off is used to wait till the status of the share is "available". func getOrCreateShare(shareName string, createOpts *shares.CreateOpts, manilaClient manilaclient.Interface) (*shares.Share, manilaError, error) { @@ -75,7 +90,7 @@ func getOrCreateShare(shareName string, createOpts *shares.CreateOpts, manilaCli return share, 0, nil } - return waitForShareStatus(share.ID, shareCreating, shareAvailable, false, manilaClient) + return waitForShareStatus(share.ID, []string{shareCreating, shareCreatingFromSnapshot}, shareAvailable, false, manilaClient) } func deleteShare(shareID string, manilaClient manilaclient.Interface) error { @@ -101,7 +116,7 @@ func tryDeleteShare(share *shares.Share, manilaClient manilaclient.Interface) { return } - _, _, err := waitForShareStatus(share.ID, shareDeleting, "", true, manilaClient) + _, _, err := waitForShareStatus(share.ID, []string{shareDeleting}, "", true, manilaClient) if err != nil && err != wait.ErrWaitTimeout { klog.Errorf("couldn't retrieve volume %s in a roll-back procedure: %v", share.Name, err) } @@ -116,7 +131,7 @@ func extendShare(share *shares.Share, newSizeInGiB int, manilaClient manilaclien return err } - share, manilaErrCode, err := waitForShareStatus(share.ID, shareExtending, shareAvailable, false, manilaClient) + share, manilaErrCode, err := waitForShareStatus(share.ID, []string{shareExtending}, shareAvailable, false, manilaClient) if err != nil { if err == wait.ErrWaitTimeout { return status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume ID %s to become available", share.Name) @@ -128,7 +143,7 @@ func extendShare(share *shares.Share, newSizeInGiB int, manilaClient manilaclien return nil } -func waitForShareStatus(shareID, currentStatus, desiredStatus string, successOnNotFound bool, manilaClient manilaclient.Interface) (*shares.Share, manilaError, error) { +func waitForShareStatus(shareID string, validTransientStates []string, desiredStatus string, successOnNotFound bool, manilaClient manilaclient.Interface) (*shares.Share, manilaError, error) { var ( backoff = wait.Backoff{ Duration: time.Second * waitForAvailableShareTimeout, @@ -141,6 +156,15 @@ func waitForShareStatus(shareID, currentStatus, desiredStatus string, successOnN err error ) + isInTransientState := func(s string) bool { + for _, val := range validTransientStates { + if s == val { + return true + } + } + return false + } + return share, manilaErrCode, wait.ExponentialBackoff(backoff, func() (bool, error) { share, err = manilaClient.GetShareByID(shareID) @@ -152,25 +176,24 @@ func waitForShareStatus(shareID, currentStatus, desiredStatus string, successOnN return false, err } - var isAvailable bool + if share.Status == desiredStatus { + return true, nil + } + + if isInTransientState(share.Status) { + return false, nil + } - switch share.Status { - case currentStatus: - isAvailable = false - case desiredStatus: - isAvailable = true - case shareError: + if isShareInErrorState(share.Status) { manilaErrMsg, err := lastResourceError(shareID, manilaClient) if err != nil { return false, fmt.Errorf("share %s is in error state, error description could not be retrieved: %v", shareID, err) } manilaErrCode = manilaErrMsg.errCode - return false, fmt.Errorf("share %s is in error state: %s", shareID, manilaErrMsg.message) - default: - return false, fmt.Errorf("share %s is in an unexpected state: wanted either %s or %s, got %s", shareID, currentStatus, desiredStatus, share.Status) + return false, fmt.Errorf("share %s is in error state \"%s\": %s", shareID, share.Status, manilaErrMsg.message) } - return isAvailable, nil + return false, fmt.Errorf("share %s is in an unexpected state: wanted either %v or %s, got %s", shareID, validTransientStates, desiredStatus, share.Status) }) } diff --git a/pkg/csi/manila/util.go b/pkg/csi/manila/util.go index ec45e6aa8b..43efa7aacb 100644 --- a/pkg/csi/manila/util.go +++ b/pkg/csi/manila/util.go @@ -19,7 +19,6 @@ package manila import ( "errors" "fmt" - "strconv" "strings" "github.com/container-storage-interface/spec/lib/go/csi" @@ -28,7 +27,6 @@ import ( "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/snapshots" "google.golang.org/grpc/codes" "k8s.io/cloud-provider-openstack/pkg/csi/manila/capabilities" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/compatibility" "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" "k8s.io/klog/v2" @@ -156,18 +154,6 @@ func compareProtocol(protoA, protoB string) bool { return strings.ToUpper(protoA) == strings.ToUpper(protoB) } -func tryCompatForVolumeSource(compatOpts *options.CompatibilityOptions, shareOpts *options.ControllerVolumeContext, source *csi.VolumeContentSource, shareTypeCaps capabilities.ManilaCapabilities) compatibility.Layer { - if source != nil { - if source.GetSnapshot() != nil && shareTypeCaps[capabilities.ManilaCapabilitySnapshot] { - if createShareFromSnapshotEnabled, _ := strconv.ParseBool(compatOpts.CreateShareFromSnapshotEnabled); createShareFromSnapshotEnabled { - return compatibility.FindCompatibilityLayer(shareOpts.Protocol, capabilities.ManilaCapabilityShareFromSnapshot, shareTypeCaps) - } - } - } - - return nil -} - // // Controller service request validation // diff --git a/pkg/csi/manila/volumesource.go b/pkg/csi/manila/volumesource.go index 3aed08d8ff..8aa01ea123 100644 --- a/pkg/csi/manila/volumesource.go +++ b/pkg/csi/manila/volumesource.go @@ -67,11 +67,6 @@ type volumeFromSnapshot struct{} func (volumeFromSnapshot) create(req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, manilaClient manilaclient.Interface, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) { snapshotSource := req.GetVolumeContentSource().GetSnapshot() - if shareOpts.Protocol == "CEPHFS" { - // TODO: Restoring shares from CephFS snapshots needs special handling. - return nil, status.Errorf(codes.InvalidArgument, "restoring CephFS snapshots is not supported yet") - } - if snapshotSource.GetSnapshotId() == "" { return nil, status.Error(codes.InvalidArgument, "snapshot ID cannot be empty") }