diff --git a/cmd/manila-csi-plugin/main.go b/cmd/manila-csi-plugin/main.go index 8bf53f848d..652cf1d412 100644 --- a/cmd/manila-csi-plugin/main.go +++ b/cmd/manila-csi-plugin/main.go @@ -27,7 +27,6 @@ import ( "k8s.io/cloud-provider-openstack/pkg/csi/manila" "k8s.io/cloud-provider-openstack/pkg/csi/manila/csiclient" "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" "k8s.io/cloud-provider-openstack/pkg/csi/manila/runtimeconfig" "k8s.io/component-base/cli" "k8s.io/klog/v2" @@ -65,38 +64,6 @@ func validateShareProtocolSelector(v string) error { return fmt.Errorf("share protocol %q not supported; supported protocols are %v", v, supportedShareProtocols) } -func parseCompatOpts() (*options.CompatibilityOptions, error) { - data := make(map[string]string) - - if compatibilitySettings == "" { - return options.NewCompatibilityOptions(data) - } - - knownCompatSettings := map[string]interface{}{} - - isKnown := func(v string) bool { - _, ok := knownCompatSettings[v] - return ok - } - - settings := strings.Split(compatibilitySettings, ",") - for _, elem := range settings { - setting := strings.SplitN(elem, "=", 2) - - if len(setting) != 2 || setting[0] == "" || setting[1] == "" { - return nil, fmt.Errorf("invalid format in option %v, expected KEY=VALUE", setting) - } - - if !isKnown(setting[0]) { - return nil, fmt.Errorf("unrecognized option '%s'", setting[0]) - } - - data[setting[0]] = setting[1] - } - - return options.NewCompatibilityOptions(data) -} - func main() { if err := flag.CommandLine.Parse([]string{}); err != nil { klog.Fatalf("Unable to parse flags: %v", err) @@ -131,11 +98,6 @@ func main() { klog.Fatalf(err.Error()) } - compatOpts, err := parseCompatOpts() - if err != nil { - klog.Fatalf("failed to parse compatibility settings: %v", err) - } - manilaClientBuilder := &manilaclient.ClientBuilder{UserAgent: "manila-csi-plugin", ExtraUserAgentData: userAgentData} csiClientBuilder := &csiclient.ClientBuilder{} @@ -150,7 +112,6 @@ func main() { FwdCSIEndpoint: fwdEndpoint, ManilaClientBuilder: manilaClientBuilder, CSIClientBuilder: csiClientBuilder, - CompatOpts: compatOpts, ClusterID: clusterID, }, ) diff --git a/pkg/csi/manila/capabilities/manilacapabilities.go b/pkg/csi/manila/capabilities/manilacapabilities.go deleted file mode 100644 index a665e62412..0000000000 --- a/pkg/csi/manila/capabilities/manilacapabilities.go +++ /dev/null @@ -1,70 +0,0 @@ -/* -Copyright 2019 The Kubernetes 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 capabilities - -import ( - "fmt" - "strconv" - - "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" -) - -type ( - ManilaCapability int - ManilaCapabilities map[ManilaCapability]bool -) - -const ( - ManilaCapabilityNone ManilaCapability = iota - ManilaCapabilitySnapshot - ManilaCapabilityShareFromSnapshot - - extraSpecSnapshotSupport = "snapshot_support" - extraSpecCreateShareFromSnapshotSupport = "create_share_from_snapshot_support" -) - -func GetManilaCapabilities(shareType string, manilaClient manilaclient.Interface) (ManilaCapabilities, error) { - shareTypes, err := manilaClient.GetShareTypes() - if err != nil { - return nil, err - } - - for _, t := range shareTypes { - if t.Name == shareType || t.ID == shareType { - return readManilaCaps(t.ExtraSpecs), nil - } - } - - return nil, fmt.Errorf("unknown share type %s", shareType) -} - -func readManilaCaps(extraSpecs map[string]interface{}) ManilaCapabilities { - strToBool := func(ss interface{}) bool { - var b bool - if ss != nil { - if str, ok := ss.(string); ok { - b, _ = strconv.ParseBool(str) - } - } - return b - } - - return ManilaCapabilities{ - ManilaCapabilitySnapshot: strToBool(extraSpecs[extraSpecSnapshotSupport]), - ManilaCapabilityShareFromSnapshot: strToBool(extraSpecs[extraSpecCreateShareFromSnapshotSupport]), - } -} diff --git a/pkg/csi/manila/compatibility/compatibility.go b/pkg/csi/manila/compatibility/compatibility.go deleted file mode 100644 index f588ae4664..0000000000 --- a/pkg/csi/manila/compatibility/compatibility.go +++ /dev/null @@ -1,44 +0,0 @@ -/* -Copyright 2019 The Kubernetes 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 compatibility - -import ( - "github.com/container-storage-interface/spec/lib/go/csi" - "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/shares" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/capabilities" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/csiclient" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" -) - -type Layer interface { - SupplementCapability(compatOpts *options.CompatibilityOptions, dstShare *shares.Share, dstShareAccessRight *shares.AccessRight, req *csi.CreateVolumeRequest, fwdEndpoint string, manilaClient manilaclient.Interface, csiClientBuilder csiclient.Builder) error -} - -// Certain share protocols may not support certain Manila capabilities -// in a given share type. This map forms a compatibility layer which -// fills in the feature gap with in-driver functionality. -var compatCaps = map[string]map[capabilities.ManilaCapability]Layer{} - -func FindCompatibilityLayer(shareProto string, wantsCap capabilities.ManilaCapability, shareTypeCaps capabilities.ManilaCapabilities) Layer { - if layers, ok := compatCaps[shareProto]; ok { - if hasCapability := shareTypeCaps[wantsCap]; !hasCapability { - if compatCapability, ok := layers[wantsCap]; ok { - return compatCapability - } - } - } - - return nil -} diff --git a/pkg/csi/manila/controllerserver.go b/pkg/csi/manila/controllerserver.go index 1c9904ec9b..cdd9cf495d 100644 --- a/pkg/csi/manila/controllerserver.go +++ b/pkg/csi/manila/controllerserver.go @@ -27,7 +27,6 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/capabilities" "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" "k8s.io/cloud-provider-openstack/pkg/csi/manila/shareadapters" "k8s.io/cloud-provider-openstack/pkg/util" @@ -123,11 +122,6 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, status.Errorf(codes.Unauthenticated, "failed to create Manila v2 client: %v", err) } - shareTypeCaps, err := capabilities.GetManilaCapabilities(shareOpts.Type, manilaClient) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get Manila capabilities for share type %s: %v", shareOpts.Type, err) - } - requestedSize := req.GetCapacityRange().GetRequiredBytes() if requestedSize == 0 { // At least 1GiB @@ -159,12 +153,12 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, err } - share, err := volCreator.create(req, req.GetName(), sizeInGiB, manilaClient, shareOpts, shareMetadata) + share, err := volCreator.create(manilaClient, req, req.GetName(), sizeInGiB, shareOpts, shareMetadata) if err != nil { return nil, err } - err = verifyVolumeCompatibility(sizeInGiB, req, share, shareOpts, cs.d.compatOpts, shareTypeCaps) + err = verifyVolumeCompatibility(sizeInGiB, req, share, shareOpts) if err != nil { return nil, status.Errorf(codes.AlreadyExists, "volume %s already exists, but is incompatible with the request: %v", req.GetName(), err) } @@ -212,7 +206,7 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol return nil, status.Errorf(codes.Unauthenticated, "failed to create Manila v2 client: %v", err) } - if err := deleteShare(req.GetVolumeId(), manilaClient); err != nil { + if err := deleteShare(manilaClient, req.GetVolumeId()); err != nil { return nil, status.Errorf(codes.Internal, "failed to delete volume %s: %v", req.GetVolumeId(), err) } @@ -271,7 +265,7 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS // Retrieve an existing snapshot or create a new one - snapshot, err := getOrCreateSnapshot(req.GetName(), sourceShare.ID, manilaClient) + snapshot, err := getOrCreateSnapshot(manilaClient, req.GetName(), sourceShare.ID) if err != nil { if wait.Interrupted(err) { return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for snapshot %s of volume %s to become available", snapshot.ID, req.GetSourceVolumeId()) @@ -299,9 +293,9 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS readyToUse = true case snapshotError: // An error occurred, try to roll-back the snapshot - tryDeleteSnapshot(snapshot, manilaClient) + tryDeleteSnapshot(manilaClient, snapshot) - manilaErrMsg, err := lastResourceError(snapshot.ID, manilaClient) + manilaErrMsg, err := lastResourceError(manilaClient, snapshot.ID) if err != nil { return nil, status.Errorf(codes.Internal, "snapshot %s of volume %s is in error state, error description could not be retrieved: %v", snapshot.ID, req.GetSourceVolumeId(), err.Error()) } @@ -344,7 +338,7 @@ func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS return nil, status.Errorf(codes.Unauthenticated, "failed to create Manila v2 client: %v", err) } - if err := deleteSnapshot(req.GetSnapshotId(), manilaClient); err != nil { + if err := deleteSnapshot(manilaClient, req.GetSnapshotId()); err != nil { return nil, status.Errorf(codes.Internal, "failed to delete snapshot %s: %v", req.GetSnapshotId(), err) } @@ -482,7 +476,7 @@ func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi }, nil } - share, err = extendShare(share.ID, desiredSizeInGiB, manilaClient) + share, err = extendShare(manilaClient, share.ID, desiredSizeInGiB) if err != nil { return nil, err } diff --git a/pkg/csi/manila/driver.go b/pkg/csi/manila/driver.go index 97ec2f3206..3d977785c7 100644 --- a/pkg/csi/manila/driver.go +++ b/pkg/csi/manila/driver.go @@ -31,7 +31,6 @@ import ( "google.golang.org/grpc" "k8s.io/cloud-provider-openstack/pkg/csi/manila/csiclient" "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" "k8s.io/cloud-provider-openstack/pkg/version" "k8s.io/klog/v2" ) @@ -49,8 +48,6 @@ type DriverOpts struct { ManilaClientBuilder manilaclient.Builder CSIClientBuilder csiclient.Builder - - CompatOpts *options.CompatibilityOptions } type Driver struct { @@ -65,8 +62,6 @@ type Driver struct { serverEndpoint string fwdEndpoint string - compatOpts *options.CompatibilityOptions - ids *identityServer cs *controllerServer ns *nodeServer @@ -103,7 +98,14 @@ func argNotEmpty(val, name string) error { } func NewDriver(o *DriverOpts) (*Driver, error) { - for k, v := range map[string]string{"node ID": o.NodeID, "driver name": o.DriverName, "driver endpoint": o.ServerCSIEndpoint, "FWD endpoint": o.FwdCSIEndpoint, "share protocol selector": o.ShareProto} { + m := map[string]string{ + "node ID": o.NodeID, + "driver name": o.DriverName, + "driver endpoint": o.ServerCSIEndpoint, + "FWD endpoint": o.FwdCSIEndpoint, + "share protocol selector": o.ShareProto, + } + for k, v := range m { if err := argNotEmpty(v, k); err != nil { return nil, err } @@ -118,7 +120,6 @@ func NewDriver(o *DriverOpts) (*Driver, error) { serverEndpoint: o.ServerCSIEndpoint, fwdEndpoint: o.FwdCSIEndpoint, shareProto: strings.ToUpper(o.ShareProto), - compatOpts: o.CompatOpts, manilaClientBuilder: o.ManilaClientBuilder, csiClientBuilder: o.CSIClientBuilder, clusterID: o.ClusterID, diff --git a/pkg/csi/manila/manilaclient/builder.go b/pkg/csi/manila/manilaclient/builder.go index ef4f16dfdb..2d3bd88864 100644 --- a/pkg/csi/manila/manilaclient/builder.go +++ b/pkg/csi/manila/manilaclient/builder.go @@ -28,6 +28,14 @@ import ( "k8s.io/cloud-provider-openstack/pkg/client" ) +const ( + minimumManilaVersion = "2.37" +) + +var ( + manilaMicroversionRegexp = regexp.MustCompile(`^(\d+)\.(\d+)$`) +) + type ClientBuilder struct { UserAgent string ExtraUserAgentData []string @@ -37,13 +45,30 @@ func (cb *ClientBuilder) New(o *client.AuthOpts) (Interface, error) { return New(o, cb.UserAgent, cb.ExtraUserAgentData) } -const ( - minimumManilaVersion = "2.37" -) +func New(o *client.AuthOpts, userAgent string, extraUserAgentData []string) (*Client, error) { + // Authenticate and create Manila v2 client + provider, err := client.NewOpenStackClient(o, userAgent, extraUserAgentData...) + if err != nil { + return nil, fmt.Errorf("failed to authenticate: %v", err) + } -var ( - manilaMicroversionRegexp = regexp.MustCompile(`^(\d+)\.(\d+)$`) -) + client, err := openstack.NewSharedFileSystemV2(provider, gophercloud.EndpointOpts{ + Region: o.Region, + Availability: o.EndpointType, + }) + if err != nil { + return nil, fmt.Errorf("failed to create Manila v2 client: %v", err) + } + + // Check client's and server's versions for compatibility + + client.Microversion = minimumManilaVersion + if err = validateManilaClient(client); err != nil { + return nil, fmt.Errorf("Manila v2 client validation failed: %v", err) + } + + return &Client{c: client}, nil +} func splitManilaMicroversion(microversion string) (major, minor int) { if err := validateManilaMicroversion(microversion); err != nil { @@ -96,32 +121,3 @@ func validateManilaClient(c *gophercloud.ServiceClient) error { return nil } - -func New(o *client.AuthOpts, userAgent string, extraUserAgentData []string) (*Client, error) { - // Authenticate and create Manila v2 client - provider, err := client.NewOpenStackClient(o, userAgent, extraUserAgentData...) - if err != nil { - return nil, fmt.Errorf("failed to authenticate: %v", err) - } - - client, err := openstack.NewSharedFileSystemV2(provider, gophercloud.EndpointOpts{ - Region: o.Region, - Availability: o.EndpointType, - }) - if err != nil { - return nil, fmt.Errorf("failed to create Manila v2 client: %v", err) - } - - // Check client's and server's versions for compatibility - - client.Microversion = minimumManilaVersion - if err = validateManilaClient(client); err != nil { - return nil, fmt.Errorf("Manila v2 client validation failed: %v", err) - } - - return &Client{c: client}, nil -} - -func NewFromServiceClient(c *gophercloud.ServiceClient) *Client { - return &Client{c: c} -} diff --git a/pkg/csi/manila/nodeserver.go b/pkg/csi/manila/nodeserver.go index 10e417a961..1d8dfda264 100644 --- a/pkg/csi/manila/nodeserver.go +++ b/pkg/csi/manila/nodeserver.go @@ -128,8 +128,11 @@ func (ns *nodeServer) buildVolumeContext(volID volumeID, shareOpts *options.Node // Build volume context for fwd plugin sa := getShareAdapter(ns.d.shareProto) - - volumeContext, err = sa.BuildVolumeContext(&shareadapters.VolumeContextArgs{Locations: availableExportLocations, Options: shareOpts}) + opts := &shareadapters.VolumeContextArgs{ + Locations: availableExportLocations, + Options: shareOpts, + } + volumeContext, err = sa.BuildVolumeContext(opts) if err != nil { return nil, nil, status.Errorf(codes.InvalidArgument, "failed to build volume context for volume %s: %v", volID, err) } @@ -138,7 +141,10 @@ func (ns *nodeServer) buildVolumeContext(volID volumeID, shareOpts *options.Node } func buildNodePublishSecret(accessRight *shares.AccessRight, sa shareadapters.ShareAdapter, volID volumeID) (map[string]string, error) { - secret, err := sa.BuildNodePublishSecret(&shareadapters.SecretArgs{AccessRight: accessRight}) + opts := &shareadapters.SecretArgs{ + AccessRight: accessRight, + } + secret, err := sa.BuildNodePublishSecret(opts) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "failed to build publish secret for volume %s: %v", volID, err) } @@ -147,7 +153,10 @@ func buildNodePublishSecret(accessRight *shares.AccessRight, sa shareadapters.Sh } func buildNodeStageSecret(accessRight *shares.AccessRight, sa shareadapters.ShareAdapter, volID volumeID) (map[string]string, error) { - secret, err := sa.BuildNodeStageSecret(&shareadapters.SecretArgs{AccessRight: accessRight}) + opts := &shareadapters.SecretArgs{ + AccessRight: accessRight, + } + secret, err := sa.BuildNodeStageSecret(opts) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "failed to build stage secret for volume %s: %v", volID, err) } @@ -201,7 +210,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis secret, err = buildNodePublishSecret(accessRight, getShareAdapter(ns.d.shareProto), volID) } } - if err != nil { return nil, err } @@ -279,7 +287,6 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol } } ns.nodeStageCacheMtx.Unlock() - if err != nil { return nil, err } diff --git a/pkg/csi/manila/options/compatibilityoptions.go b/pkg/csi/manila/options/compatibilityoptions.go deleted file mode 100644 index 0ed4d6cfd2..0000000000 --- a/pkg/csi/manila/options/compatibilityoptions.go +++ /dev/null @@ -1,31 +0,0 @@ -/* -Copyright 2019 The Kubernetes 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 options - -import "k8s.io/cloud-provider-openstack/pkg/csi/manila/validator" - -type CompatibilityOptions struct { - CreateShareFromSnapshotEnabled string `name:"CreateShareFromSnapshotEnabled" value:"default:false" matches:"^true|false$"` - CreateShareFromSnapshotRetries string `name:"CreateShareFromSnapshotRetries" value:"default:10" matches:"^[0-9]+$"` - CreateShareFromSnapshotBackoffInterval string `name:"CreateShareFromSnapshotBackoffInterval" value:"default:5" matches:"^[0-9]+$"` -} - -var ( - compatOptionsValidator = validator.New(&CompatibilityOptions{}) -) - -func NewCompatibilityOptions(data map[string]string) (*CompatibilityOptions, error) { - opts := &CompatibilityOptions{} - return opts, compatOptionsValidator.Populate(data, opts) -} diff --git a/pkg/csi/manila/share.go b/pkg/csi/manila/share.go index 11b013c507..33badf09d5 100644 --- a/pkg/csi/manila/share.go +++ b/pkg/csi/manila/share.go @@ -60,7 +60,7 @@ func isShareInErrorState(s string) bool { // 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) { +func getOrCreateShare(manilaClient manilaclient.Interface, shareName string, createOpts *shares.CreateOpts) (*shares.Share, manilaError, error) { var ( share *shares.Share err error @@ -90,10 +90,10 @@ func getOrCreateShare(shareName string, createOpts *shares.CreateOpts, manilaCli return share, 0, nil } - return waitForShareStatus(share.ID, []string{shareCreating, shareCreatingFromSnapshot}, shareAvailable, false, manilaClient) + return waitForShareStatus(manilaClient, share.ID, []string{shareCreating, shareCreatingFromSnapshot}, shareAvailable, false) } -func deleteShare(shareID string, manilaClient manilaclient.Interface) error { +func deleteShare(manilaClient manilaclient.Interface, shareID string) error { if err := manilaClient.DeleteShare(shareID); err != nil { if clouderrors.IsNotFound(err) { klog.V(4).Infof("volume with share ID %s not found, assuming it to be already deleted", shareID) @@ -105,7 +105,7 @@ func deleteShare(shareID string, manilaClient manilaclient.Interface) error { return nil } -func tryDeleteShare(share *shares.Share, manilaClient manilaclient.Interface) { +func tryDeleteShare(manilaClient manilaclient.Interface, share *shares.Share) { if share == nil { return } @@ -116,13 +116,13 @@ func tryDeleteShare(share *shares.Share, manilaClient manilaclient.Interface) { return } - _, _, err := waitForShareStatus(share.ID, []string{shareDeleting}, "", true, manilaClient) + _, _, err := waitForShareStatus(manilaClient, share.ID, []string{shareDeleting}, "", true) if err != nil && !wait.Interrupted(err) { klog.Errorf("couldn't retrieve volume %s in a roll-back procedure: %v", share.Name, err) } } -func extendShare(shareID string, newSizeInGiB int, manilaClient manilaclient.Interface) (*shares.Share, error) { +func extendShare(manilaClient manilaclient.Interface, shareID string, newSizeInGiB int) (*shares.Share, error) { opts := shares.ExtendOpts{ NewSize: newSizeInGiB, } @@ -131,7 +131,7 @@ func extendShare(shareID string, newSizeInGiB int, manilaClient manilaclient.Int return nil, err } - share, manilaErrCode, err := waitForShareStatus(shareID, []string{shareExtending}, shareAvailable, false, manilaClient) + share, manilaErrCode, err := waitForShareStatus(manilaClient, shareID, []string{shareExtending}, shareAvailable, false) if err != nil { if wait.Interrupted(err) { return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume ID %s to become available", share.Name) @@ -143,7 +143,7 @@ func extendShare(shareID string, newSizeInGiB int, manilaClient manilaclient.Int return share, nil } -func waitForShareStatus(shareID string, validTransientStates []string, desiredStatus string, successOnNotFound bool, manilaClient manilaclient.Interface) (*shares.Share, manilaError, error) { +func waitForShareStatus(manilaClient manilaclient.Interface, shareID string, validTransientStates []string, desiredStatus string, successOnNotFound bool) (*shares.Share, manilaError, error) { var ( backoff = wait.Backoff{ Duration: time.Second * waitForAvailableShareTimeout, @@ -185,7 +185,7 @@ func waitForShareStatus(shareID string, validTransientStates []string, desiredSt } if isShareInErrorState(share.Status) { - manilaErrMsg, err := lastResourceError(shareID, manilaClient) + manilaErrMsg, err := lastResourceError(manilaClient, shareID) if err != nil { return false, fmt.Errorf("share %s is in error state, error description could not be retrieved: %v", shareID, err) } diff --git a/pkg/csi/manila/snapshot.go b/pkg/csi/manila/snapshot.go index afd58c1494..5fe2045421 100644 --- a/pkg/csi/manila/snapshot.go +++ b/pkg/csi/manila/snapshot.go @@ -38,7 +38,7 @@ const ( // getOrCreateSnapshot retrieves an existing snapshot with name=snapName, or creates a new one if it doesn't exist yet. // Instead of waiting for the snapshot to become available (as getOrCreateShare does), CSI's ready_to_use flag is used to signal readiness -func getOrCreateSnapshot(snapName, sourceShareID string, manilaClient manilaclient.Interface) (*snapshots.Snapshot, error) { +func getOrCreateSnapshot(manilaClient manilaclient.Interface, snapName, sourceShareID string) (*snapshots.Snapshot, error) { var ( snapshot *snapshots.Snapshot err error @@ -72,7 +72,7 @@ func getOrCreateSnapshot(snapName, sourceShareID string, manilaClient manilaclie return snapshot, nil } -func deleteSnapshot(snapID string, manilaClient manilaclient.Interface) error { +func deleteSnapshot(manilaClient manilaclient.Interface, snapID string) error { if err := manilaClient.DeleteSnapshot(snapID); err != nil { if clouderrors.IsNotFound(err) { klog.V(4).Infof("snapshot %s not found, assuming it to be already deleted", snapID) @@ -84,24 +84,24 @@ func deleteSnapshot(snapID string, manilaClient manilaclient.Interface) error { return nil } -func tryDeleteSnapshot(snapshot *snapshots.Snapshot, manilaClient manilaclient.Interface) { +func tryDeleteSnapshot(manilaClient manilaclient.Interface, snapshot *snapshots.Snapshot) { if snapshot == nil { return } - if err := deleteSnapshot(snapshot.ID, manilaClient); err != nil { + if err := deleteSnapshot(manilaClient, snapshot.ID); err != nil { // TODO failure to delete a snapshot in an error state needs proper monitoring support klog.Errorf("couldn't delete snapshot %s in a roll-back procedure: %v", snapshot.ID, err) return } - _, _, err := waitForSnapshotStatus(snapshot.ID, snapshotDeleting, "", true, manilaClient) + _, _, err := waitForSnapshotStatus(manilaClient, snapshot.ID, snapshotDeleting, "", true) if err != nil && !wait.Interrupted(err) { klog.Errorf("couldn't retrieve snapshot %s in a roll-back procedure: %v", snapshot.ID, err) } } -func waitForSnapshotStatus(snapshotID, currentStatus, desiredStatus string, successOnNotFound bool, manilaClient manilaclient.Interface) (*snapshots.Snapshot, manilaError, error) { +func waitForSnapshotStatus(manilaClient manilaclient.Interface, snapshotID, currentStatus, desiredStatus string, successOnNotFound bool) (*snapshots.Snapshot, manilaError, error) { var ( backoff = wait.Backoff{ Duration: time.Second * waitForAvailableShareTimeout, @@ -133,7 +133,7 @@ func waitForSnapshotStatus(snapshotID, currentStatus, desiredStatus string, succ case desiredStatus: isAvailable = true case shareError: - manilaErrMsg, err := lastResourceError(snapshotID, manilaClient) + manilaErrMsg, err := lastResourceError(manilaClient, snapshotID) if err != nil { return false, fmt.Errorf("snapshot %s is in error state, error description could not be retrieved: %v", snapshotID, err) } diff --git a/pkg/csi/manila/util.go b/pkg/csi/manila/util.go index 281bf0435b..090a6c80e4 100644 --- a/pkg/csi/manila/util.go +++ b/pkg/csi/manila/util.go @@ -26,7 +26,6 @@ import ( "github.com/gophercloud/gophercloud/openstack/sharedfilesystems/v2/shares" "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/manilaclient" "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" "k8s.io/klog/v2" @@ -68,6 +67,7 @@ const ( ) var ( + // TODO: add 001 and add a refernce to these codes manilaErrorCodesMap = map[string]manilaError{ "002": manilaErrNoValidHost, "003": manilaErrUnexpectedNetwork, @@ -129,7 +129,7 @@ func bytesToGiB(sizeInBytes int64) int { return sizeInGiB } -func lastResourceError(resourceID string, manilaClient manilaclient.Interface) (manilaErrorMessage, error) { +func lastResourceError(manilaClient manilaclient.Interface, resourceID string) (manilaErrorMessage, error) { msgs, err := manilaClient.GetUserMessages(&messages.ListOpts{ ResourceID: resourceID, MessageLevel: "ERROR", @@ -232,7 +232,7 @@ func coalesceValue(v string) string { return v } -func verifyVolumeCompatibility(sizeInGiB int, req *csi.CreateVolumeRequest, share *shares.Share, shareOpts *options.ControllerVolumeContext, compatOpts *options.CompatibilityOptions, shareTypeCaps capabilities.ManilaCapabilities) error { +func verifyVolumeCompatibility(sizeInGiB int, req *csi.CreateVolumeRequest, share *shares.Share, shareOpts *options.ControllerVolumeContext) error { if share.Size != sizeInGiB { return fmt.Errorf("size mismatch: wanted %d, got %d", share.Size, sizeInGiB) } diff --git a/pkg/csi/manila/volumesource.go b/pkg/csi/manila/volumesource.go index c2e7db0ce1..634040bef2 100644 --- a/pkg/csi/manila/volumesource.go +++ b/pkg/csi/manila/volumesource.go @@ -28,12 +28,12 @@ import ( ) type volumeCreator interface { - create(req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, manilaClient manilaclient.Interface, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) + create(manilaClient manilaclient.Interface, req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) } type blankVolume struct{} -func (blankVolume) create(req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, manilaClient manilaclient.Interface, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) { +func (blankVolume) create(manilaClient manilaclient.Interface, req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) { createOpts := &shares.CreateOpts{ AvailabilityZone: shareOpts.AvailabilityZone, ShareProto: shareOpts.Protocol, @@ -45,7 +45,7 @@ func (blankVolume) create(req *csi.CreateVolumeRequest, shareName string, sizeIn Metadata: shareMetadata, } - share, manilaErrCode, err := getOrCreateShare(shareName, createOpts, manilaClient) + share, manilaErrCode, err := getOrCreateShare(manilaClient, shareName, createOpts) if err != nil { if wait.Interrupted(err) { return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume %s to become available", shareName) @@ -53,7 +53,7 @@ func (blankVolume) create(req *csi.CreateVolumeRequest, shareName string, sizeIn if manilaErrCode != 0 { // An error has occurred, try to roll-back the share - tryDeleteShare(share, manilaClient) + tryDeleteShare(manilaClient, share) } return nil, status.Errorf(manilaErrCode.toRPCErrorCode(), "failed to create volume %s: %v", shareName, err) @@ -64,7 +64,7 @@ func (blankVolume) create(req *csi.CreateVolumeRequest, shareName string, sizeIn 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) { +func (volumeFromSnapshot) create(manilaClient manilaclient.Interface, req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) { snapshotSource := req.GetVolumeContentSource().GetSnapshot() if snapshotSource.GetSnapshotId() == "" { @@ -100,7 +100,7 @@ func (volumeFromSnapshot) create(req *csi.CreateVolumeRequest, shareName string, Metadata: shareMetadata, } - share, manilaErrCode, err := getOrCreateShare(shareName, createOpts, manilaClient) + share, manilaErrCode, err := getOrCreateShare(manilaClient, shareName, createOpts) if err != nil { if wait.Interrupted(err) { return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume %s to become available", share.Name) @@ -108,7 +108,7 @@ func (volumeFromSnapshot) create(req *csi.CreateVolumeRequest, shareName string, if manilaErrCode != 0 { // An error has occurred, try to roll-back the share - tryDeleteShare(share, manilaClient) + tryDeleteShare(manilaClient, share) } return nil, status.Errorf(manilaErrCode.toRPCErrorCode(), "failed to restore snapshot %s into volume %s: %v", snapshotSource.GetSnapshotId(), shareName, err) diff --git a/tests/sanity/manila/sanity_test.go b/tests/sanity/manila/sanity_test.go index 66249f9bdc..8fb7b17b8b 100644 --- a/tests/sanity/manila/sanity_test.go +++ b/tests/sanity/manila/sanity_test.go @@ -23,7 +23,6 @@ import ( "github.com/kubernetes-csi/csi-test/v5/pkg/sanity" "k8s.io/cloud-provider-openstack/pkg/csi/manila" - "k8s.io/cloud-provider-openstack/pkg/csi/manila/options" ) func TestDriver(t *testing.T) { @@ -44,7 +43,6 @@ func TestDriver(t *testing.T) { FwdCSIEndpoint: fwdEndpoint, ManilaClientBuilder: &fakeManilaClientBuilder{}, CSIClientBuilder: &fakeCSIClientBuilder{}, - CompatOpts: &options.CompatibilityOptions{}, }) if err != nil {