From f3510112eed6a51c82933a3595cd9393db6e012f Mon Sep 17 00:00:00 2001 From: Ritesh Ghorse Date: Mon, 11 Nov 2024 06:57:16 +0000 Subject: [PATCH] filestore nfsv4 support --- pkg/cloud_provider/file/fake.go | 1 + pkg/cloud_provider/file/file.go | 26 ++-- pkg/csi_driver/controller.go | 63 +++++++-- pkg/csi_driver/controller_test.go | 74 ++++++++-- pkg/csi_driver/multishare_controller.go | 21 ++- pkg/csi_driver/multishare_controller_test.go | 139 ++++++++++++++++--- pkg/csi_driver/multishare_ops_manager.go | 25 +++- pkg/csi_driver/node.go | 9 +- pkg/csi_driver/reconciler.go | 4 +- pkg/util/ip_reservation_test.go | 2 +- pkg/webhook/webhook.go | 4 +- 11 files changed, 301 insertions(+), 67 deletions(-) diff --git a/pkg/cloud_provider/file/fake.go b/pkg/cloud_provider/file/fake.go index bc46089d2..ae4856c42 100644 --- a/pkg/cloud_provider/file/fake.go +++ b/pkg/cloud_provider/file/fake.go @@ -95,6 +95,7 @@ func (manager *fakeServiceManager) CreateInstance(ctx context.Context, obj *Serv State: "READY", BackupSource: obj.BackupSource, NfsExportOptions: obj.NfsExportOptions, + Protocol: obj.Protocol, } manager.createdInstances[obj.Name] = instance diff --git a/pkg/cloud_provider/file/file.go b/pkg/cloud_provider/file/file.go index 86b9dd33e..b1cba396f 100644 --- a/pkg/cloud_provider/file/file.go +++ b/pkg/cloud_provider/file/file.go @@ -85,6 +85,7 @@ type MultishareInstance struct { KmsKeyName string Description string MaxShareCount int + Protocol string } func (i *MultishareInstance) String() string { @@ -109,6 +110,7 @@ type ServiceInstance struct { KmsKeyName string BackupSource string NfsExportOptions []*NfsExportOptions + Protocol string } type Volume struct { @@ -124,9 +126,10 @@ type Network struct { } type Backup struct { - Backup *filev1beta1.Backup - SourceInstance string - SourceShare string + Backup *filev1beta1.Backup + SourceInstance string + SourceShare string + FileSystemProtocl string } type BackupInfo struct { @@ -291,9 +294,10 @@ func (manager *gcfsServiceManager) CreateInstance(ctx context.Context, obj *Serv KmsKeyName: obj.KmsKeyName, Labels: obj.Labels, State: obj.State, + Protocol: obj.Protocol, } - klog.V(4).Infof("Creating instance %q: location %v, tier %q, capacity %v, network %q, ipRange %q, connectMode %q, KmsKeyName %q, labels %v backup source %q", + klog.V(4).Infof("Creating instance %q: location %v, tier %q, capacity %v, network %q, ipRange %q, connectMode %q, KmsKeyName %q, labels %v, backup source %q, protocol %v", obj.Name, obj.Location, instance.Tier, @@ -303,7 +307,8 @@ func (manager *gcfsServiceManager) CreateInstance(ctx context.Context, obj *Serv instance.Networks[0].ConnectMode, instance.KmsKeyName, instance.Labels, - instance.FileShares[0].SourceBackup) + instance.FileShares[0].SourceBackup, + obj.Protocol) op, err := manager.instancesService.Create(locationURI(obj.Project, obj.Location), instance).InstanceId(obj.Name).Context(ctx).Do() if err != nil { klog.Errorf("CreateInstance operation failed for instance %v: %v", obj.Name, err) @@ -370,6 +375,7 @@ func cloudInstanceToServiceInstance(instance *filev1beta1.Instance) (*ServiceIns Labels: instance.Labels, State: instance.State, BackupSource: instance.FileShares[0].SourceBackup, + Protocol: instance.Protocol, }, nil } @@ -513,9 +519,10 @@ func (manager *gcfsServiceManager) GetBackup(ctx context.Context, backupUri stri return nil, err } return &Backup{ - Backup: backup, - SourceInstance: backup.SourceInstance, - SourceShare: backup.SourceFileShare, + Backup: backup, + SourceInstance: backup.SourceInstance, + SourceShare: backup.SourceFileShare, + FileSystemProtocl: backup.FileSystemProtocol, }, nil } @@ -990,6 +997,7 @@ func (manager *gcfsServiceManager) StartCreateMultishareInstanceOp(ctx context.C Labels: instance.Labels, Description: instance.Description, MaxShareCount: int64(instance.MaxShareCount), + Protocol: instance.Protocol, } op, err := manager.multishareInstancesService.Create(locationURI(instance.Project, instance.Location), targetinstance).InstanceId(instance.Name).Context(ctx).Do() @@ -1019,6 +1027,7 @@ func (manager *gcfsServiceManager) StartResizeMultishareInstanceOp(ctx context.C KmsKeyName: obj.KmsKeyName, Labels: obj.Labels, Description: obj.Description, + Protocol: obj.Protocol, } op, err := manager.multishareInstancesService.Patch(instanceuri, targetinstance).UpdateMask(multishareCapacityUpdateMask).Context(ctx).Do() if err != nil { @@ -1138,7 +1147,6 @@ func (manager *gcfsServiceManager) ListShares(ctx context.Context, filter *ListF klog.Errorf("Failed to parse share url :%s", sobj.Name) return nil, err } - s := &Share{ Name: shareName, Parent: &MultishareInstance{ diff --git a/pkg/csi_driver/controller.go b/pkg/csi_driver/controller.go index 056947040..a923a0114 100644 --- a/pkg/csi_driver/controller.go +++ b/pkg/csi_driver/controller.go @@ -38,14 +38,16 @@ const ( modeInstance = "modeInstance" newInstanceVolume = "vol1" - defaultTier = "standard" - enterpriseTier = "enterprise" - premiumTier = "premium" - basicHDDTier = "basic_hdd" - basicSSDTier = "basic_ssd" - highScaleTier = "high_scale_ssd" - zonalTier = "zonal" - defaultNetwork = "default" + defaultTier = "standard" + enterpriseTier = "enterprise" + premiumTier = "premium" + basicHDDTier = "basic_hdd" + basicSSDTier = "basic_ssd" + highScaleTier = "high_scale_ssd" + zonalTier = "zonal" + defaultNetwork = "default" + v3FileProtocol = "NFS_V3" + v4_1FileProtocol = "NFS_V4_1" defaultTierMinSize = 1 * util.Tb defaultTierMaxSize = 639 * util.Tb / 10 @@ -72,6 +74,7 @@ const ( attrIP = "ip" attrVolume = "volume" attrSupportLockRelease = "supportLockRelease" + attrFileProtocol = "fileProtocol" ) // CreateVolume parameters @@ -87,6 +90,7 @@ const ( ParamMultishareInstanceScLabel = "instance-storageclass-label" ParamNfsExportOptions = "nfs-export-options-on-create" paramMaxVolumeSize = "max-volume-size" + paramFileProtocol = "protocol" // Keys for PV and PVC parameters as reported by external-provisioner ParameterKeyPVCName = "csi.storage.k8s.io/pvc/name" @@ -279,13 +283,13 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu // Check if the filestore instance is in the process of getting created. if filer.State == "CREATING" { msg := fmt.Sprintf("Volume %v not ready, current state: %v", name, filer.State) - klog.V(4).Infof(msg) + klog.V(4).Info(msg) return nil, status.Error(codes.DeadlineExceeded, msg) } if filer.State != "READY" { msg := fmt.Sprintf("Volume %v not ready, current state: %v", name, filer.State) - klog.V(4).Infof(msg) - return nil, status.Errorf(codes.Unavailable, msg) + klog.V(4).Info(msg) + return nil, status.Error(codes.Unavailable, msg) } } else { param := req.GetParameters() @@ -603,6 +607,14 @@ func invalidVolumeExpansionRequest(capRange *csi.CapacityRange, currentCapacity return false } +func isBasicTier(tier string) bool { + switch tier { + case defaultTier, premiumTier, basicHDDTier, basicSSDTier: + return true + } + return false +} + // generateNewFileInstance populates the GCFS Instance object using // CreateVolume parameters func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, params map[string]string, topo *csi.TopologyRequirement) (*file.ServiceInstance, error) { @@ -617,6 +629,7 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, network := defaultNetwork connectMode := directPeering kmsKeyName := "" + fileProtocol := "" // Validate parameters (case-insensitive). for k, v := range params { @@ -655,12 +668,25 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, continue case cloud.ParameterKeyResourceTags: continue + case paramFileProtocol: + fileProtocol = v case ParameterKeyLabels, ParameterKeyPVCName, ParameterKeyPVCNamespace, ParameterKeyPVName: case "csiprovisionersecretname", "csiprovisionersecretnamespace": default: return nil, fmt.Errorf("invalid parameter %q", k) } } + + switch fileProtocol { + case v4_1FileProtocol: + if isBasicTier(tier) { + return nil, status.Errorf(codes.FailedPrecondition, "Filestore does not support NFSv4.1 protocol with Basic tiers") + } + case v3FileProtocol: + default: + fileProtocol = v3FileProtocol + } + return &file.ServiceInstance{ Project: s.config.cloud.Project, Name: name, @@ -676,6 +702,7 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, }, KmsKeyName: kmsKeyName, NfsExportOptions: nfsExportOptions, + Protocol: fileProtocol, }, nil } @@ -699,9 +726,19 @@ func (s *controllerServer) fileInstanceToCSIVolume(instance *file.ServiceInstanc } resp.ContentSource = contentSource } - if s.config.features.FeatureLockRelease.Enabled && strings.ToLower(instance.Tier) == enterpriseTier { - resp.VolumeContext[attrSupportLockRelease] = "true" + + switch instance.Protocol { + case v4_1FileProtocol: + resp.VolumeContext[attrFileProtocol] = v4_1FileProtocol + case v3FileProtocol: + resp.VolumeContext[attrFileProtocol] = v3FileProtocol + if s.config.features.FeatureLockRelease.Enabled && strings.ToLower(instance.Tier) == enterpriseTier { + resp.VolumeContext[attrSupportLockRelease] = "true" + } + default: + resp.VolumeContext[attrFileProtocol] = v3FileProtocol } + return resp } diff --git a/pkg/csi_driver/controller_test.go b/pkg/csi_driver/controller_test.go index da2756c3e..86cfc61e6 100644 --- a/pkg/csi_driver/controller_test.go +++ b/pkg/csi_driver/controller_test.go @@ -144,8 +144,9 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { CapacityBytes: defaultTierMinSize, VolumeId: testVolumeID, VolumeContext: map[string]string{ - attrIP: testIP, - attrVolume: newInstanceVolume, + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v3FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -191,8 +192,9 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { CapacityBytes: premiumTierMinSize, VolumeId: testVolumeID, VolumeContext: map[string]string{ - attrIP: testIP, - attrVolume: newInstanceVolume, + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v3FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -230,7 +232,7 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { }, }, }, - Parameters: map[string]string{"tier": enterpriseTier}, + Parameters: map[string]string{"tier": enterpriseTier, paramFileProtocol: v4_1FileProtocol}, VolumeCapabilities: volumeCapabilities, }, resp: &csi.CreateVolumeResponse{ @@ -238,8 +240,9 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { CapacityBytes: testBytes, VolumeId: testVolumeID, VolumeContext: map[string]string{ - attrIP: testIP, - attrVolume: newInstanceVolume, + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v4_1FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -306,8 +309,9 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { CapacityBytes: testBytes, VolumeId: testVolumeID, VolumeContext: map[string]string{ - attrIP: testIP, - attrVolume: newInstanceVolume, + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v3FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -459,14 +463,50 @@ func TestCreateVolume(t *testing.T) { }, }, }, + Parameters: map[string]string{ + "tier": zonalTier, + "protocol": v4_1FileProtocol, + }, + }, + resp: &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: 1 * util.Tb, + VolumeId: testVolumeID, + VolumeContext: map[string]string{ + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v4_1FileProtocol, + }, + }, + }, + features: features, + }, + { + name: "create volume without providing protocol for basic", + req: &csi.CreateVolumeRequest{ + Name: testCSIVolume, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + Parameters: map[string]string{ + "tier": basicHDDTier, + }, }, resp: &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: 1 * util.Tb, VolumeId: testVolumeID, VolumeContext: map[string]string{ - attrIP: testIP, - attrVolume: newInstanceVolume, + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v3FileProtocol, }, }, }, @@ -655,8 +695,9 @@ func TestCreateVolume(t *testing.T) { CapacityBytes: 1 * util.Tb, VolumeId: testVolumeID, VolumeContext: map[string]string{ - attrIP: testIP, - attrVolume: newInstanceVolume, + attrIP: testIP, + attrVolume: newInstanceVolume, + attrFileProtocol: v3FileProtocol, }, }, }, @@ -1106,6 +1147,7 @@ func TestGenerateNewFileInstance(t *testing.T) { Name: newInstanceVolume, SizeBytes: testBytes, }, + Protocol: v3FileProtocol, }, }, { @@ -1145,6 +1187,7 @@ func TestGenerateNewFileInstance(t *testing.T) { Name: newInstanceVolume, SizeBytes: testBytes, }, + Protocol: v3FileProtocol, }, }, { @@ -1189,6 +1232,7 @@ func TestGenerateNewFileInstance(t *testing.T) { Name: newInstanceVolume, SizeBytes: testBytes, }, + Protocol: v3FileProtocol, }, }, { @@ -1221,6 +1265,7 @@ func TestGenerateNewFileInstance(t *testing.T) { Name: newInstanceVolume, SizeBytes: testBytes, }, + Protocol: v3FileProtocol, }, }, { @@ -1261,6 +1306,7 @@ func TestGenerateNewFileInstance(t *testing.T) { Name: newInstanceVolume, SizeBytes: testBytes, }, + Protocol: v3FileProtocol, }, }, { @@ -1285,6 +1331,7 @@ func TestGenerateNewFileInstance(t *testing.T) { SizeBytes: testBytes, }, KmsKeyName: "foo-key", + Protocol: v3FileProtocol, }, }, { @@ -1310,6 +1357,7 @@ func TestGenerateNewFileInstance(t *testing.T) { SizeBytes: testBytes, }, KmsKeyName: "foo-key", + Protocol: v3FileProtocol, }, }, { diff --git a/pkg/csi_driver/multishare_controller.go b/pkg/csi_driver/multishare_controller.go index 001a0a9bd..1ae7d6c87 100644 --- a/pkg/csi_driver/multishare_controller.go +++ b/pkg/csi_driver/multishare_controller.go @@ -350,7 +350,7 @@ func (m *MultishareController) getShareAndGenerateCSICreateVolumeResponse(ctx co if share.State != "READY" { return nil, status.Errorf(codes.Aborted, "share %s not ready, state %s", share.Name, share.State) } - return m.generateCSICreateVolumeResponse(instancePrefix, share, maxShareSizeSizeBytes) + return m.generateCSICreateVolumeResponse(instancePrefix, s, maxShareSizeSizeBytes) } func (m *MultishareController) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { @@ -573,6 +573,7 @@ func (m *MultishareController) generateNewMultishareInstance(instanceName string network := defaultNetwork connectMode := directPeering kmsKeyName := "" + fileProtocol := "" for k, v := range req.GetParameters() { switch strings.ToLower(k) { case paramTier: @@ -592,6 +593,8 @@ func (m *MultishareController) generateNewMultishareInstance(instanceName string return nil, status.Error(codes.InvalidArgument, "nfsExportOptions are disabled") } continue + case paramFileProtocol: + fileProtocol = v // Ignore the cidr flag as it is not passed to the cloud provider // It will be used to get unreserved IP in the reserveIPV4Range function // ignore IPRange flag as it will be handled at the same place as cidr @@ -623,7 +626,11 @@ func (m *MultishareController) generateNewMultishareInstance(instanceName string } labels, err := extractInstanceLabels(req.GetParameters(), m.extraVolumeLabels, m.driver.config.Name, m.clustername, location) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, err.Error()) + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + if fileProtocol == "" { + fileProtocol = v3FileProtocol } f := &file.MultishareInstance{ @@ -639,6 +646,7 @@ func (m *MultishareController) generateNewMultishareInstance(instanceName string KmsKeyName: kmsKeyName, Labels: labels, Description: generateInstanceDescFromEcfsDesc(m.ecfsDescription), + Protocol: fileProtocol, } if m.featureMaxSharePerInstance { f.MaxShareCount = maxShareCount @@ -672,6 +680,7 @@ func (m *MultishareController) checkVolumeContentSource(ctx context.Context, req return "", nil } + func generateNewShare(name string, parent *file.MultishareInstance, req *csi.CreateVolumeRequest, sourceSnapshotId string) (*file.Share, error) { if parent == nil { return nil, status.Error(codes.Internal, "parent multishare instance is empty") @@ -816,7 +825,7 @@ func getShareRequestCapacity(capRange *csi.CapacityRange, minShareSizeBytes, max func (m *MultishareController) generateCSICreateVolumeResponse(instancePrefix string, s *file.Share, maxShareSizeBytes int64) (*csi.CreateVolumeResponse, error) { volId, err := generateMultishareVolumeIdFromShare(instancePrefix, s) if err != nil { - return nil, status.Errorf(codes.Internal, err.Error()) + return nil, status.Error(codes.Internal, err.Error()) } resp := &csi.CreateVolumeResponse{ @@ -844,6 +853,12 @@ func (m *MultishareController) generateCSICreateVolumeResponse(instancePrefix st if m.featureMaxSharePerInstance { resp.Volume.VolumeContext[attrMaxShareSize] = strconv.Itoa(int(maxShareSizeBytes)) } + + if s.Parent.Protocol == v4_1FileProtocol { + resp.Volume.VolumeContext[attrFileProtocol] = v4_1FileProtocol + } else { + resp.Volume.VolumeContext[attrFileProtocol] = v3FileProtocol + } klog.Infof("CreateVolume resp: %+v", resp) return resp, nil } diff --git a/pkg/csi_driver/multishare_controller_test.go b/pkg/csi_driver/multishare_controller_test.go index f74086643..0119d3e9c 100644 --- a/pkg/csi_driver/multishare_controller_test.go +++ b/pkg/csi_driver/multishare_controller_test.go @@ -690,6 +690,7 @@ func TestGenerateNewMultishareInstance(t *testing.T) { TagKeyClusterName: testClusterName, util.ParamMultishareInstanceScLabelKey: testInstanceScPrefix, }, + Protocol: v3FileProtocol, }, }, } @@ -704,7 +705,7 @@ func TestGenerateNewMultishareInstance(t *testing.T) { t.Errorf("unexpected error: %q", err) } if !reflect.DeepEqual(filer, tc.expectedInstance) { - t.Errorf("got filer %+v, want %+v", filer, tc.expectedInstance) + t.Errorf("got filer %v, want %+v", filer, tc.expectedInstance) } }) } @@ -749,6 +750,7 @@ func TestGenerateCSICreateVolumeResponse(t *testing.T) { Network: file.Network{ Ip: "1.1.1.1", }, + Protocol: v3FileProtocol, }, CapacityBytes: 1 * util.Tb, }, @@ -758,7 +760,8 @@ func TestGenerateCSICreateVolumeResponse(t *testing.T) { VolumeId: modeMultishare + "/" + testInstanceScPrefix + "/" + testProject + "/" + testLocation + "/" + testInstanceName + "/" + testShareName, CapacityBytes: 1 * util.Tb, VolumeContext: map[string]string{ - attrIP: "1.1.1.1", + attrIP: "1.1.1.1", + attrFileProtocol: v3FileProtocol, }, }, }, @@ -780,6 +783,7 @@ func TestGenerateCSICreateVolumeResponse(t *testing.T) { Network: file.Network{ Ip: "1.1.1.1", }, + Protocol: v4_1FileProtocol, }, CapacityBytes: 1 * util.Tb, }, @@ -791,6 +795,7 @@ func TestGenerateCSICreateVolumeResponse(t *testing.T) { VolumeContext: map[string]string{ attrIP: "1.1.1.1", attrMaxShareSize: strconv.Itoa(util.Tb), + attrFileProtocol: v4_1FileProtocol, }, }, }, @@ -823,6 +828,7 @@ func TestGenerateCSICreateVolumeResponse(t *testing.T) { VolumeContext: map[string]string{ attrIP: "1.1.1.1", attrMaxShareSize: strconv.Itoa(100 * util.Gb), + attrFileProtocol: v3FileProtocol, }, }, }, @@ -1159,6 +1165,81 @@ func TestMultishareCreateVolume(t *testing.T) { }, }, }, + { + name: "1 initial ready 1Tib instances with 0 shares having NFSv3 Protocol, 1 initial ready 1Tib instances with 0 shares having NFSv4 Protocol, create 100Gib share in a NFSv4 instance, success response", + initInstances: []*file.MultishareInstance{ + { + Name: testInstanceName1, + Location: "us-central1", + Project: "test-project", + Labels: map[string]string{ + util.ParamMultishareInstanceScLabelKey: testInstanceScPrefix, + TagKeyClusterLocation: testLocation, + TagKeyClusterName: "", + }, + CapacityBytes: 1 * util.Tb, + Tier: "enterprise", + Network: file.Network{ + Ip: testIP, + Name: defaultNetwork, + ConnectMode: directPeering, + }, + State: "READY", + Protocol: v3FileProtocol, + }, + { + Name: testInstanceName2, + Location: "us-central1", + Project: "test-project", + Labels: map[string]string{ + util.ParamMultishareInstanceScLabelKey: testInstanceScPrefix, + TagKeyClusterLocation: testLocation, + TagKeyClusterName: "", + }, + CapacityBytes: 1 * util.Tb, + Tier: "enterprise", + Network: file.Network{ + Ip: testIP, + Name: defaultNetwork, + ConnectMode: directPeering, + }, + State: "READY", + Protocol: v4_1FileProtocol, + }, + }, + ops: []OpItem{}, + req: &csi.CreateVolumeRequest{ + Name: testVolName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: 100 * util.Gb, + }, + Parameters: map[string]string{ + ParamMultishareInstanceScLabel: testInstanceScPrefix, + paramTier: "enterprise", + paramFileProtocol: v4_1FileProtocol, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + resp: &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: 100 * util.Gb, + VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName2, testShareName), + VolumeContext: map[string]string{ + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, + }, + }, + }, + }, { name: "1 initial ready 1Tib instances with 0 shares, 1 busy instance, create 100Gib share and use the ready instance, success response", initInstances: []*file.MultishareInstance{ @@ -1178,7 +1259,8 @@ func TestMultishareCreateVolume(t *testing.T) { Name: defaultNetwork, ConnectMode: directPeering, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, { Name: testInstanceName2, @@ -1196,7 +1278,8 @@ func TestMultishareCreateVolume(t *testing.T) { Name: defaultNetwork, ConnectMode: directPeering, }, - State: "READY", + State: "CREATING", + Protocol: v4_1FileProtocol, }, }, ops: []OpItem{ @@ -1214,6 +1297,7 @@ func TestMultishareCreateVolume(t *testing.T) { Parameters: map[string]string{ ParamMultishareInstanceScLabel: testInstanceScPrefix, paramTier: "enterprise", + paramFileProtocol: v4_1FileProtocol, }, VolumeCapabilities: []*csi.VolumeCapability{ { @@ -1231,7 +1315,8 @@ func TestMultishareCreateVolume(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, }, }, }, @@ -1300,7 +1385,8 @@ func TestMultishareCreateVolume(t *testing.T) { Network: file.Network{ Ip: testIP, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, }, initShares: []*file.Share{ @@ -1318,7 +1404,8 @@ func TestMultishareCreateVolume(t *testing.T) { Network: file.Network{ Ip: testIP, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, CapacityBytes: 100 * util.Gb, MountPointName: testShareName, @@ -1332,6 +1419,7 @@ func TestMultishareCreateVolume(t *testing.T) { }, Parameters: map[string]string{ ParamMultishareInstanceScLabel: testInstanceScPrefix, + paramFileProtocol: v4_1FileProtocol, }, VolumeCapabilities: []*csi.VolumeCapability{ { @@ -1349,7 +1437,8 @@ func TestMultishareCreateVolume(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, }, }, }, @@ -1539,6 +1628,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { "squashMode": "NO_ROOT_SQUASH" } ]`, + paramFileProtocol: v4_1FileProtocol, }, VolumeCapabilities: volumeCapabilities, VolumeContentSource: &csi.VolumeContentSource{ @@ -1569,7 +1659,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -1608,7 +1699,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v3FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -1641,7 +1733,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Name: defaultNetwork, ConnectMode: directPeering, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, { Name: testInstanceName2, @@ -1659,7 +1752,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Name: defaultNetwork, ConnectMode: directPeering, }, - State: "READY", + State: "CREATING", + Protocol: v4_1FileProtocol, }, }, ops: []OpItem{ @@ -1677,6 +1771,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Parameters: map[string]string{ ParamMultishareInstanceScLabel: testInstanceScPrefix, paramTier: "enterprise", + paramFileProtocol: v4_1FileProtocol, }, VolumeCapabilities: volumeCapabilities, VolumeContentSource: &csi.VolumeContentSource{ @@ -1693,7 +1788,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -1725,7 +1821,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Name: defaultNetwork, ConnectMode: directPeering, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, }, req: &csi.CreateVolumeRequest{ @@ -1736,6 +1833,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Parameters: map[string]string{ ParamMultishareInstanceScLabel: testInstanceScPrefix, paramTier: "enterprise", + paramFileProtocol: v4_1FileProtocol, }, VolumeCapabilities: volumeCapabilities, VolumeContentSource: &csi.VolumeContentSource{ @@ -1752,7 +1850,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ @@ -1782,7 +1881,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Network: file.Network{ Ip: testIP, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, }, initShares: []*file.Share{ @@ -1800,7 +1900,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { Network: file.Network{ Ip: testIP, }, - State: "READY", + State: "READY", + Protocol: v4_1FileProtocol, }, CapacityBytes: 100 * util.Gb, MountPointName: testShareName, @@ -1815,6 +1916,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, Parameters: map[string]string{ ParamMultishareInstanceScLabel: testInstanceScPrefix, + paramFileProtocol: v4_1FileProtocol, }, VolumeCapabilities: volumeCapabilities, VolumeContentSource: &csi.VolumeContentSource{ @@ -1831,7 +1933,8 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { CapacityBytes: 100 * util.Gb, VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), VolumeContext: map[string]string{ - attrIP: testIP, + attrIP: testIP, + attrFileProtocol: v4_1FileProtocol, }, ContentSource: &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ diff --git a/pkg/csi_driver/multishare_ops_manager.go b/pkg/csi_driver/multishare_ops_manager.go index 500f3473c..2292d0442 100644 --- a/pkg/csi_driver/multishare_ops_manager.go +++ b/pkg/csi_driver/multishare_ops_manager.go @@ -78,7 +78,7 @@ func (m *MultishareOpsManager) setupEligibleInstanceAndStartWorkflow(ctx context createShareOp := containsOpWithShareName(shareName, util.ShareCreate, ops) if createShareOp != nil { msg := fmt.Sprintf("Share create op %s in progress", createShareOp.Id) - klog.Infof(msg) + klog.Info(msg) return nil, nil, status.Error(codes.Aborted, msg) } @@ -94,7 +94,7 @@ func (m *MultishareOpsManager) setupEligibleInstanceAndStartWorkflow(ctx context return nil, nil, err } for _, s := range shares { - if s.Name == shareName { + if s.Name == shareName && s.Parent.Protocol == instance.Protocol { return nil, s, nil } } @@ -418,7 +418,7 @@ func (m *MultishareOpsManager) runEligibleInstanceCheck(ctx context.Context, req } } - return nil, status.Errorf(codes.Aborted, errorString) + return nil, status.Error(codes.Aborted, errorString) } @@ -704,22 +704,33 @@ func (m *MultishareOpsManager) listMatchedInstances(ctx context.Context, req *cs // A source instance will be considered as "matched" with the target instance // if and only if the following requirements were met: +// // 1. Both source and target instance should have a label with key // "storage_gke_io_storage-class-id", and the value should be the same. +// // 2. (Check if exists) The ip address of the target instance should be // within the ip range specified in "reserved-ipv4-cidr". +// // 3. (Check if exists) The ip address of the target instance should be // within the ip range specified in "reserved-ip-range". +// // 4. Both source and target instance should be in the same location. +// // 5. Both source and target instance should be under the same tier. +// // 6. Both source and target instance should be in the same VPC network. // -// 7, Both source and target instance should have the same connect mode. +// 7. Both source and target instance should have the same connect mode. +// // 8. Both source and target instance should have the same KmsKeyName. +// // 9. Both source and target instance should have a label with key // "gke_cluster_location", and the value should be the same. +// // 10. Both source and target instance should have a label with key // "gke_cluster_name", and the value should be the same. +// +// 11. Both source and target instance should have the same FileSystem protocol. func isMatchedInstance(source, target *file.MultishareInstance, req *csi.CreateVolumeRequest) (bool, error) { matchLabels := [3]string{util.ParamMultishareInstanceScLabelKey, TagKeyClusterLocation, TagKeyClusterName} for _, labelKey := range matchLabels { @@ -740,6 +751,11 @@ func isMatchedInstance(source, target *file.MultishareInstance, req *csi.CreateV return false, nil } } + + if source.Protocol != target.Protocol { + return false, nil + } + // Skip validation for parameter "reserved-ip-range" since it requires // extra compute api auth and not clear if it's required. if strings.EqualFold(source.Location, target.Location) && @@ -749,5 +765,6 @@ func isMatchedInstance(source, target *file.MultishareInstance, req *csi.CreateV strings.EqualFold(source.KmsKeyName, target.KmsKeyName) { return true, nil } + return false, nil } diff --git a/pkg/csi_driver/node.go b/pkg/csi_driver/node.go index 8e2599732..3ad1bb083 100644 --- a/pkg/csi_driver/node.go +++ b/pkg/csi_driver/node.go @@ -300,8 +300,13 @@ func (s *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolu } } + fileProtocol, ok := attr[attrFileProtocol] + if !ok { + fileProtocol = v3FileProtocol + } + if mounted { - if s.features.FeatureLockRelease.Enabled { + if fileProtocol == v3FileProtocol && s.features.FeatureLockRelease.Enabled { klog.V(4).Infof("NodeStageVolume mounted volume %v to staging target path %s, mount already exists on node %s. Proceed to lock info configmap updates", volumeID, stagingTargetPath, s.driver.config.NodeName) if err := s.nodeStageVolumeUpdateLockInfo(ctx, req); err != nil { return nil, status.Errorf(codes.Internal, "failed to store lock info after NodeStageVolume succeeded on volume %v to path %s: %v", volumeID, stagingTargetPath, err.Error()) @@ -335,7 +340,7 @@ func (s *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolu return nil, status.Errorf(codes.Internal, "mount %q failed on node %s: %v", stagingTargetPath, s.driver.config.NodeName, err.Error()) } - if s.features.FeatureLockRelease.Enabled { + if fileProtocol == v3FileProtocol && s.features.FeatureLockRelease.Enabled { klog.V(4).Infof("NodeStageVolume mounted volume %v to staging target path %s on node %s, proceed to lock info configmap updates.", volumeID, stagingTargetPath, s.driver.config.NodeName) if err := s.nodeStageVolumeUpdateLockInfo(ctx, req); err != nil { return nil, status.Errorf(codes.Internal, "failed to store lock info after NodeStageVolume succeeded on volume %v to path %s: %v", volumeID, stagingTargetPath, err.Error()) diff --git a/pkg/csi_driver/reconciler.go b/pkg/csi_driver/reconciler.go index 3fbf14360..c392bb2d5 100644 --- a/pkg/csi_driver/reconciler.go +++ b/pkg/csi_driver/reconciler.go @@ -248,7 +248,7 @@ func (recon *MultishareReconciler) sendShareRequests(instanceInfos map[string]*v if assignedInstanceInfo.Status == nil || assignedInstanceInfo.Status.InstanceStatus != v1.READY || assignedInstanceInfo.Status.CapacityBytes < assignedInstanceInfo.Spec.CapacityBytes { klog.Infof("Instance %s is not ready", assignedInstanceInfo.Name) if assignedInstanceInfo.Status != nil && assignedInstanceInfo.Status.Error != "" { - recon.updateShareInfoErr(shareInfo, fmt.Errorf(assignedInstanceInfo.Status.Error)) + recon.updateShareInfoErr(shareInfo, fmt.Errorf("%v", assignedInstanceInfo.Status.Error)) } continue } @@ -487,7 +487,7 @@ func (recon *MultishareReconciler) generateNewMultishareInstance(instanceInfo *v labels, err := extractInstanceLabels(params, recon.config.ExtraVolumeLabels, recon.config.Name, recon.config.ClusterName, clusterLocation) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, err.Error()) + return nil, status.Error(codes.InvalidArgument, err.Error()) } instance := &file.MultishareInstance{ diff --git a/pkg/util/ip_reservation_test.go b/pkg/util/ip_reservation_test.go index ad6f35351..4a3c47655 100644 --- a/pkg/util/ip_reservation_test.go +++ b/pkg/util/ip_reservation_test.go @@ -387,7 +387,7 @@ func getIPRanges(cidr string, ipRangesCount int, ipRangeSize int, t *testing.T) ipRanges[ipRangeString] = true } if err != nil { - t.Fatalf(err.Error()) + t.Fatal(err.Error()) } else if i != ipRangesCount { t.Fatalf("The required number of IP ranges %d are not available in the CIDR %s", ipRangesCount, cidr) } diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 4355b96b2..f7ff57bca 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -95,7 +95,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) { contentType := r.Header.Get("Content-Type") if contentType != "application/json" { msg := fmt.Sprintf("contentType=%s, expect application/json", contentType) - klog.Errorf(msg) + klog.Error(msg) http.Error(w, msg, http.StatusBadRequest) return } @@ -117,7 +117,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) { requestedAdmissionReview, ok := obj.(*v1.AdmissionReview) if !ok { msg := fmt.Sprintf("Expected v1.AdmissionReview but got: %T", obj) - klog.Errorf(msg) + klog.Error(msg) http.Error(w, msg, http.StatusBadRequest) return }