diff --git a/cmd/main.go b/cmd/main.go index e5f6ef070..bb14e1cb4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -61,7 +61,8 @@ var ( coreInformerResyncPeriod = flag.Duration("core-informer-resync-repriod", 15*time.Minute, "Core informer resync period.") // Feature multishare backups enabled - featureMultishareBackups = flag.Bool("feature-multishare-backups", false, "if set to true, the multishare backups will be enabled. enable-multishare must be set to true as well") + featureMultishareBackups = flag.Bool("feature-multishare-backups", false, "if set to true, the multishare backups will be enabled. enable-multishare must be set to true as well") + featureNFSExportOptionsOnCreate = flag.Bool("feature-nfs-export-options", false, "if set to true, the driver will accpet nfs-export-options-on-create parameter and configure IP Access rules") // Feature stateful CSI driver specific parameters featureStateful = flag.Bool("feature-stateful-multishare", false, "if set to true, the controller will run stateful multishare controller, if set to true, enable-multishare must be set to true as well") @@ -183,6 +184,9 @@ func main() { FeatureMultishareBackups: &driver.FeatureMultishareBackups{ Enabled: *featureMultishareBackups, }, + FeatureNFSExportOptionsOnCreate: &driver.FeatureNFSExportOptionsOnCreate{ + Enabled: *featureNFSExportOptionsOnCreate, + }, } mounter := mount.New("") diff --git a/pkg/cloud_provider/file/fake.go b/pkg/cloud_provider/file/fake.go index 9ecb666d7..bc46089d2 100644 --- a/pkg/cloud_provider/file/fake.go +++ b/pkg/cloud_provider/file/fake.go @@ -91,8 +91,10 @@ func (manager *fakeServiceManager) CreateInstance(ctx context.Context, obj *Serv Ip: "1.1.1.1", ReservedIpRange: obj.Network.ReservedIpRange, }, - Labels: obj.Labels, - State: "READY", + Labels: obj.Labels, + State: "READY", + BackupSource: obj.BackupSource, + NfsExportOptions: obj.NfsExportOptions, } manager.createdInstances[obj.Name] = instance @@ -204,29 +206,6 @@ func (manager *fakeServiceManager) GetBackup(ctx context.Context, backupUri stri return backupInfo, nil } -func (manager *fakeServiceManager) CreateInstanceFromBackupSource(ctx context.Context, obj *ServiceInstance, sourceSnapshotId string) (*ServiceInstance, error) { - instance := &ServiceInstance{ - Project: defaultProject, - Location: defaultZone, - Name: obj.Name, - Tier: obj.Tier, - Volume: Volume{ - Name: obj.Volume.Name, - SizeBytes: obj.Volume.SizeBytes, - }, - Network: Network{ - Name: obj.Network.Name, - Ip: "1.1.1.1", - ReservedIpRange: obj.Network.ReservedIpRange, - }, - Labels: obj.Labels, - State: "READY", - } - - manager.createdInstances[obj.Name] = instance - return instance, nil -} - func (m *fakeServiceManager) HasOperations(ctx context.Context, obj *ServiceInstance, operationType string, done bool) (bool, error) { return false, nil } @@ -380,13 +359,14 @@ func (manager *fakeServiceManager) StartCreateShareOp(ctx context.Context, obj * State: "READY", } share := &Share{ - Name: obj.Name, - Parent: parent, - CapacityBytes: obj.CapacityBytes, - Labels: obj.Labels, - MountPointName: obj.Name, - BackupId: obj.BackupId, - State: "READY", + Name: obj.Name, + Parent: parent, + CapacityBytes: obj.CapacityBytes, + Labels: obj.Labels, + MountPointName: obj.Name, + BackupId: obj.BackupId, + State: "READY", + NfsExportOptions: obj.NfsExportOptions, } manager.createdMultishares[share.Name] = share diff --git a/pkg/cloud_provider/file/file.go b/pkg/cloud_provider/file/file.go index 7632e2581..d14879296 100644 --- a/pkg/cloud_provider/file/file.go +++ b/pkg/cloud_provider/file/file.go @@ -50,15 +50,23 @@ type PollOpts struct { Interval time.Duration Timeout time.Duration } +type NfsExportOptions struct { + AccessMode string `json:"accessMode,omitempty"` + AnonGid int64 `json:"anonGid,omitempty,string"` + AnonUid int64 `json:"anonUid,omitempty,string"` + IpRanges []string `json:"ipRanges,omitempty"` + SquashMode string `json:"squashMode,omitempty"` +} type Share struct { - Name string // only the share name - Parent *MultishareInstance // parent captures the project, location details. - State string - MountPointName string - Labels map[string]string - CapacityBytes int64 - BackupId string + Name string // only the share name + Parent *MultishareInstance // parent captures the project, location details. + State string + MountPointName string + Labels map[string]string + CapacityBytes int64 + BackupId string + NfsExportOptions []*NfsExportOptions } type MultishareInstance struct { @@ -88,15 +96,17 @@ type ListFilter struct { } type ServiceInstance struct { - Project string - Name string - Location string - Tier string - Network Network - Volume Volume - Labels map[string]string - State string - KmsKeyName string + Project string + Name string + Location string + Tier string + Network Network + Volume Volume + Labels map[string]string + State string + KmsKeyName string + BackupSource string + NfsExportOptions []*NfsExportOptions } type Volume struct { @@ -154,7 +164,6 @@ type Service interface { GetBackup(ctx context.Context, backupUri string) (*Backup, error) CreateBackup(ctx context.Context, backupInfo *BackupInfo) (*filev1beta1.Backup, error) DeleteBackup(ctx context.Context, backupId string) error - CreateInstanceFromBackupSource(ctx context.Context, obj *ServiceInstance, volumeSourceSnapshotId string) (*ServiceInstance, error) HasOperations(ctx context.Context, obj *ServiceInstance, operationType string, done bool) (bool, error) // Multishare ops GetMultishareInstance(ctx context.Context, obj *MultishareInstance) (*MultishareInstance, error) @@ -251,64 +260,14 @@ func NewGCFSService(version string, client *http.Client, primaryFilestoreService } func (manager *gcfsServiceManager) CreateInstance(ctx context.Context, obj *ServiceInstance) (*ServiceInstance, error) { - betaObj := &filev1beta1.Instance{ - Tier: obj.Tier, - FileShares: []*filev1beta1.FileShareConfig{ - { - Name: obj.Volume.Name, - CapacityGb: util.RoundBytesToGb(obj.Volume.SizeBytes), - }, - }, - Networks: []*filev1beta1.NetworkConfig{ - { - Network: obj.Network.Name, - Modes: []string{"MODE_IPV4"}, - ReservedIpRange: obj.Network.ReservedIpRange, - ConnectMode: obj.Network.ConnectMode, - }, - }, - KmsKeyName: obj.KmsKeyName, - Labels: obj.Labels, - } - - klog.V(4).Infof("Creating instance %q: location %q, tier %q, capacity %v, network %q, ipRange %q, connectMode %q, KmsKeyName %q, labels %v", - obj.Name, - obj.Location, - betaObj.Tier, - betaObj.FileShares[0].CapacityGb, - betaObj.Networks[0].Network, - betaObj.Networks[0].ReservedIpRange, - betaObj.Networks[0].ConnectMode, - betaObj.KmsKeyName, - betaObj.Labels) - op, err := manager.instancesService.Create(locationURI(obj.Project, obj.Location), betaObj).InstanceId(obj.Name).Context(ctx).Do() - if err != nil { - klog.Errorf("CreateInstance operation failed for instance %s: %w", obj.Name, err) - return nil, err - } - - klog.V(4).Infof("For instance %v, waiting for create instance op %v to complete", obj.Name, op.Name) - err = manager.waitForOp(ctx, op) - if err != nil { - klog.Errorf("WaitFor CreateInstance op %s failed: %w", op.Name, err) - return nil, err - } - instance, err := manager.GetInstance(ctx, obj) - if err != nil { - klog.Errorf("failed to get instance after creation: %w", err) - return nil, err - } - return instance, nil -} - -func (manager *gcfsServiceManager) CreateInstanceFromBackupSource(ctx context.Context, obj *ServiceInstance, sourceSnapshotId string) (*ServiceInstance, error) { instance := &filev1beta1.Instance{ Tier: obj.Tier, FileShares: []*filev1beta1.FileShareConfig{ { - Name: obj.Volume.Name, - CapacityGb: util.RoundBytesToGb(obj.Volume.SizeBytes), - SourceBackup: sourceSnapshotId, + Name: obj.Volume.Name, + CapacityGb: util.RoundBytesToGb(obj.Volume.SizeBytes), + SourceBackup: obj.BackupSource, + NfsExportOptions: extractNfsShareExportOptions(obj.NfsExportOptions), }, }, Networks: []*filev1beta1.NetworkConfig{ @@ -394,9 +353,10 @@ func cloudInstanceToServiceInstance(instance *filev1beta1.Instance) (*ServiceIns ReservedIpRange: instance.Networks[0].ReservedIpRange, ConnectMode: instance.Networks[0].ConnectMode, }, - KmsKeyName: instance.KmsKeyName, - Labels: instance.Labels, - State: instance.State, + KmsKeyName: instance.KmsKeyName, + Labels: instance.Labels, + State: instance.State, + BackupSource: instance.FileShares[0].SourceBackup, }, nil } @@ -1006,10 +966,11 @@ func (manager *gcfsServiceManager) StartResizeMultishareInstanceOp(ctx context.C func (manager *gcfsServiceManager) StartCreateShareOp(ctx context.Context, share *Share) (*filev1beta1multishare.Operation, error) { instanceuri := instanceURI(share.Parent.Project, share.Parent.Location, share.Parent.Name) targetshare := &filev1beta1multishare.Share{ - CapacityGb: util.BytesToGb(share.CapacityBytes), - Labels: share.Labels, - MountName: share.MountPointName, - Backup: share.BackupId, + CapacityGb: util.BytesToGb(share.CapacityBytes), + Labels: share.Labels, + MountName: share.MountPointName, + Backup: share.BackupId, + NfsExportOptions: extractNfsShareExportOptions(share.NfsExportOptions), } op, err := manager.multishareInstancesSharesService.Create(instanceuri, targetshare).ShareId(share.Name).Context(ctx).Do() @@ -1326,3 +1287,18 @@ func GenerateShareURI(s *Share) (string, error) { func isMultishareVolId(volId string) bool { return strings.Contains(volId, "modeMultishare") } + +func extractNfsShareExportOptions(options []*NfsExportOptions) []*filev1beta1multishare.NfsExportOptions { + var filerOpts []*filev1beta1multishare.NfsExportOptions + for _, opt := range options { + filerOpts = append(filerOpts, + &filev1beta1multishare.NfsExportOptions{ + AccessMode: opt.AccessMode, + AnonGid: opt.AnonGid, + AnonUid: opt.AnonUid, + IpRanges: opt.IpRanges, + SquashMode: opt.SquashMode, + }) + } + return filerOpts +} diff --git a/pkg/csi_driver/controller.go b/pkg/csi_driver/controller.go index 8a5a0b986..65ca2b40c 100644 --- a/pkg/csi_driver/controller.go +++ b/pkg/csi_driver/controller.go @@ -17,6 +17,8 @@ limitations under the License. package driver import ( + "bytes" + "encoding/json" "fmt" "strings" "time" @@ -81,6 +83,7 @@ const ( paramMultishare = "multishare" ParamInstanceEncryptionKmsKey = "instance-encryption-kms-key" ParamMultishareInstanceScLabel = "instance-storageclass-label" + ParamNfsExportOptions = "nfs-export-options-on-create" paramMaxVolumeSize = "max-volume-size" // Keys for PV and PVC parameters as reported by external-provisioner @@ -218,7 +221,6 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu } defer s.config.volumeLocks.Release(volumeID) - sourceSnapshotId := "" if req.GetVolumeContentSource() != nil { if req.GetVolumeContentSource().GetVolume() != nil { return nil, status.Error(codes.InvalidArgument, "Unsupported volume content source") @@ -235,7 +237,7 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu klog.Errorf("Failed to get volume %v source snapshot %v: %v", name, id, err.Error()) return nil, file.StatusError(err) } - sourceSnapshotId = id + newFiler.BackupSource = id } } @@ -302,17 +304,13 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu // Create the instance var createErr error - if sourceSnapshotId != "" { - filer, createErr = s.config.fileService.CreateInstanceFromBackupSource(ctx, newFiler, sourceSnapshotId) - } else { - filer, createErr = s.config.fileService.CreateInstance(ctx, newFiler) - } + filer, createErr = s.config.fileService.CreateInstance(ctx, newFiler) if createErr != nil { klog.Errorf("Create volume for volume Id %s failed: %v", volumeID, createErr.Error()) return nil, file.StatusError(createErr) } } - resp := &csi.CreateVolumeResponse{Volume: s.fileInstanceToCSIVolume(filer, modeInstance, sourceSnapshotId)} + resp := &csi.CreateVolumeResponse{Volume: s.fileInstanceToCSIVolume(filer, modeInstance)} klog.Infof("CreateVolume succeeded: %+v", resp) return resp, nil } @@ -587,6 +585,7 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, // Set default parameters tier := defaultTier + var nfsExportOptions []*file.NfsExportOptions network := defaultNetwork connectMode := directPeering kmsKeyName := "" @@ -604,6 +603,14 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, } location = region } + case ParamNfsExportOptions: + if s.config.features.FeatureNFSExportOptionsOnCreate == nil || !s.config.features.FeatureNFSExportOptionsOnCreate.Enabled { + return nil, fmt.Errorf("nfsExportOptions are disabled") + } + nfsExportOptions, err = parseNfsExportOptions(v) + if err != nil { + return nil, fmt.Errorf("failed to parse nfs-export-options-on-create %s: %v", v, err) + } case paramNetwork: network = v case ParamConnectMode: @@ -637,12 +644,13 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64, Name: newInstanceVolume, SizeBytes: capBytes, }, - KmsKeyName: kmsKeyName, + KmsKeyName: kmsKeyName, + NfsExportOptions: nfsExportOptions, }, nil } // fileInstanceToCSIVolume generates a CSI volume spec from the cloud Instance -func (s *controllerServer) fileInstanceToCSIVolume(instance *file.ServiceInstance, mode, sourceSnapshotId string) *csi.Volume { +func (s *controllerServer) fileInstanceToCSIVolume(instance *file.ServiceInstance, mode string) *csi.Volume { resp := &csi.Volume{ VolumeId: getVolumeIDFromFileInstance(instance, mode), CapacityBytes: instance.Volume.SizeBytes, @@ -651,11 +659,11 @@ func (s *controllerServer) fileInstanceToCSIVolume(instance *file.ServiceInstanc attrVolume: instance.Volume.Name, }, } - if sourceSnapshotId != "" { + if instance.BackupSource != "" { contentSource := &csi.VolumeContentSource{ Type: &csi.VolumeContentSource_Snapshot{ Snapshot: &csi.VolumeContentSource_SnapshotSource{ - SnapshotId: sourceSnapshotId, + SnapshotId: instance.BackupSource, }, }, } @@ -1015,3 +1023,21 @@ func (s *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSn return &csi.DeleteSnapshotResponse{}, nil } + +func parseNfsExportOptions(optionsString string) ([]*file.NfsExportOptions, error) { + if optionsString == "" { + return nil, nil + } + var parsedOptions []*file.NfsExportOptions + err := strictUnmarshal([]byte(optionsString), &parsedOptions) + if err != nil { + return nil, err + } + return parsedOptions, nil +} + +func strictUnmarshal(data []byte, v interface{}) error { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + return dec.Decode(v) +} diff --git a/pkg/csi_driver/controller_test.go b/pkg/csi_driver/controller_test.go index ed69a889b..dfe0ff05f 100644 --- a/pkg/csi_driver/controller_test.go +++ b/pkg/csi_driver/controller_test.go @@ -104,13 +104,21 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { }, }, } + features := &GCFSDriverFeatureOptions{ + FeatureNFSExportOptionsOnCreate: &FeatureNFSExportOptionsOnCreate{ + Enabled: true, + }, + FeatureLockRelease: &FeatureLockRelease{}, + } cases := []struct { - name string - req *csi.CreateVolumeRequest - resp *csi.CreateVolumeResponse - initialBackup *BackupInfo - expectErr bool + name string + req *csi.CreateVolumeRequest + resp *csi.CreateVolumeResponse + initialBackup *BackupInfo + expectedOptions []*file.NfsExportOptions + expectErr bool + features *GCFSDriverFeatureOptions }{ { name: "from default tier snapshot", @@ -253,10 +261,95 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { SourceVolumeId: modeInstance + "/" + testRegion + "/" + instanceName + "/" + shareName, }, }, + { + name: "from enterprise tier snapshot and nfsExportOptions set", + req: &csi.CreateVolumeRequest{ + Name: testCSIVolume, + VolumeContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{ + SnapshotId: "projects/test-project/locations/us-central1/backups/mybackup", + }, + }, + }, + Parameters: map[string]string{ + "tier": enterpriseTier, + ParamNfsExportOptions: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + }, + VolumeCapabilities: volumeCapabilities, + }, + features: features, + resp: &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: testBytes, + VolumeId: testVolumeID, + VolumeContext: map[string]string{ + attrIP: testIP, + attrVolume: newInstanceVolume, + }, + ContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{ + SnapshotId: "projects/test-project/locations/us-central1/backups/mybackup", + }, + }, + }, + }, + }, + expectedOptions: []*file.NfsExportOptions{ + { + AccessMode: "READ_WRITE", + IpRanges: []string{"10.0.0.0/24"}, + SquashMode: "ROOT_SQUASH", + AnonGid: 1003, + AnonUid: 1003, + }, + { + AccessMode: "READ_ONLY", + IpRanges: []string{"10.0.0.0/28"}, + SquashMode: "NO_ROOT_SQUASH", + }, + }, + initialBackup: &BackupInfo{ + s: &file.ServiceInstance{ + Project: testProject, + Location: testRegion, + Name: instanceName, + Tier: enterpriseTier, + Volume: file.Volume{ + Name: shareName, + SizeBytes: testBytes, + }, + }, + backupName: backupName, + backupLocation: testRegion, + SourceVolumeId: modeInstance + "/" + testRegion + "/" + instanceName + "/" + shareName, + }, + }, } for _, test := range cases { cs := initTestController(t).(*controllerServer) + if test.features != nil { + cs.config.features = test.features + } //Create initial backup backupInfo := &file.BackupInfo{ @@ -280,6 +373,18 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { if test.expectErr && err == nil { t.Errorf("test %q failed; got success", test.name) } + if !test.expectErr && test.req.Parameters[ParamNfsExportOptions] != "" { + instance, err := cs.config.fileService.GetInstance(context.TODO(), &file.ServiceInstance{Name: test.req.Name}) + if err != nil { + t.Errorf("test %q failed: couldn't get instance %v: %v", test.name, resp.Volume.VolumeId, err) + return + } + for i := range test.expectedOptions { + if !reflect.DeepEqual(instance.NfsExportOptions[i], test.expectedOptions[i]) { + t.Errorf("test %q failed; nfs export options not equal at index %d: got %+v, expected %+v", test.name, i, instance.NfsExportOptions[i], test.expectedOptions[i]) + } + } + } if !reflect.DeepEqual(resp, test.resp) { t.Errorf("test %q failed: got resp %+v, expected %+v", test.name, resp, test.resp) } @@ -287,11 +392,19 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { } func TestCreateVolume(t *testing.T) { + features := &GCFSDriverFeatureOptions{ + FeatureNFSExportOptionsOnCreate: &FeatureNFSExportOptionsOnCreate{ + Enabled: true, + }, + FeatureLockRelease: &FeatureLockRelease{}, + } cases := []struct { - name string - req *csi.CreateVolumeRequest - resp *csi.CreateVolumeResponse - expectErr bool + name string + req *csi.CreateVolumeRequest + resp *csi.CreateVolumeResponse + expectErr bool + features *GCFSDriverFeatureOptions + expectedOptions []*file.NfsExportOptions }{ { name: "valid defaults", @@ -318,7 +431,9 @@ func TestCreateVolume(t *testing.T) { }, }, }, + features: features, }, + // Failure Scenarios { name: "name empty", req: &csi.CreateVolumeRequest{ @@ -373,11 +488,140 @@ func TestCreateVolume(t *testing.T) { // TODO: instance already exists error // TODO: instance already exists invalid // TODO: instance already exists valid + { + name: "nfsExportOptions feature is disabled", + req: &csi.CreateVolumeRequest{ + Name: testCSIVolume, + VolumeContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{ + SnapshotId: "projects/test-project/locations/us-central1/backups/mybackup", + }, + }, + }, + Parameters: map[string]string{ + "tier": enterpriseTier, + ParamNfsExportOptions: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + features: &GCFSDriverFeatureOptions{ + FeatureNFSExportOptionsOnCreate: &FeatureNFSExportOptionsOnCreate{ + Enabled: false, + }, + FeatureLockRelease: &FeatureLockRelease{}, + }, + expectErr: true, + }, + // Success Scenarios + { + name: "adding nfs-export-options", + req: &csi.CreateVolumeRequest{ + Name: testCSIVolume, + Parameters: map[string]string{ + ParamNfsExportOptions: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + features: features, + expectedOptions: []*file.NfsExportOptions{ + { + AccessMode: "READ_WRITE", + IpRanges: []string{"10.0.0.0/24"}, + SquashMode: "ROOT_SQUASH", + AnonGid: 1003, + AnonUid: 1003, + }, + { + AccessMode: "READ_ONLY", + IpRanges: []string{"10.0.0.0/28"}, + SquashMode: "NO_ROOT_SQUASH", + }, + }, + resp: &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: 1 * util.Tb, + VolumeId: testVolumeID, + VolumeContext: map[string]string{ + attrIP: testIP, + attrVolume: newInstanceVolume, + }, + }, + }, + }, } for _, test := range cases { - cs := initTestController(t) + fileService, err := file.NewFakeService() + if err != nil { + t.Fatalf("failed to initialize GCFS service: %v", err) + } + + cloudProvider, err := cloud.NewFakeCloud() + if err != nil { + t.Fatalf("Failed to get cloud provider: %v", err) + } + cs := newControllerServer(&controllerServerConfig{ + driver: initTestDriver(t), + fileService: fileService, + cloud: cloudProvider, + volumeLocks: util.NewVolumeLocks(), + features: test.features, + }) resp, err := cs.CreateVolume(context.TODO(), test.req) + if !test.expectErr && err != nil { t.Errorf("test %q failed: %v", test.name, err) } @@ -387,6 +631,19 @@ func TestCreateVolume(t *testing.T) { if !reflect.DeepEqual(resp, test.resp) { t.Errorf("test %q failed: got resp %+v, expected %+v", test.name, resp, test.resp) } + + if !test.expectErr && test.req.Parameters[ParamNfsExportOptions] != "" { + instance, err := fileService.GetInstance(context.TODO(), &file.ServiceInstance{Name: test.req.Name}) + if err != nil { + t.Errorf("test %q failed: couldn't get instance %v: %v", test.name, resp.Volume.VolumeId, err) + return + } + for i := range test.expectedOptions { + if !reflect.DeepEqual(instance.NfsExportOptions[i], test.expectedOptions[i]) { + t.Errorf("test %q failed; nfs export options not equal at index %d: got %+v, expected %+v", test.name, i, instance.NfsExportOptions[i], test.expectedOptions[i]) + } + } + } } } @@ -1734,3 +1991,115 @@ func TestGetCloudInstancesReservedIPRanges(t *testing.T) { } } } + +func TestParsingNfsExportOptions(t *testing.T) { + cases := []struct { + name string + optionsString string + expectedOptions []*file.NfsExportOptions + expectErr bool + }{ + { + name: "Base case single options", + optionsString: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + } + ]`, + expectedOptions: []*file.NfsExportOptions{ + { + AccessMode: "READ_WRITE", + IpRanges: []string{"10.0.0.0/24"}, + SquashMode: "ROOT_SQUASH", + AnonGid: 1003, + AnonUid: 1003, + }, + }, + expectErr: false, + }, + { + name: "Base case multiple options", + optionsString: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + expectedOptions: []*file.NfsExportOptions{ + { + AccessMode: "READ_WRITE", + IpRanges: []string{"10.0.0.0/24"}, + SquashMode: "ROOT_SQUASH", + AnonGid: 1003, + AnonUid: 1003, + }, + { + AccessMode: "READ_ONLY", + IpRanges: []string{"10.0.0.0/28"}, + SquashMode: "NO_ROOT_SQUASH", + }, + }, + expectErr: false, + }, + { + name: "Invalid extra keys throw an error", + optionsString: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003", + "INVALID_KEY": "INVALID_VALUE" + } + ]`, + expectErr: true, + }, + { + name: "Empty string returns nil", + optionsString: "", + expectedOptions: nil, + expectErr: false, + }, + { + name: "Invalid json returns error", + optionsString: ` + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + `, + expectErr: true, + }, + } + for _, test := range cases { + parsedOptions, err := parseNfsExportOptions(test.optionsString) + if !test.expectErr && err != nil { + t.Errorf("test %q failed: %v", test.name, err) + } + if !reflect.DeepEqual(test.expectedOptions, parsedOptions) { + t.Errorf("test %q failed; expected: %#v; got %#v", test.name, test.expectedOptions, parsedOptions) + } + } +} diff --git a/pkg/csi_driver/gcfs_driver.go b/pkg/csi_driver/gcfs_driver.go index b5ae767c8..9032872a4 100644 --- a/pkg/csi_driver/gcfs_driver.go +++ b/pkg/csi_driver/gcfs_driver.go @@ -96,15 +96,20 @@ type GCFSDriverFeatureOptions struct { // FeatureLockRelease will enable the NFS lock release feature if sets to true. FeatureLockRelease *FeatureLockRelease // FeatureMaxSharesPerInstance will enable CSI driver to pack configurable number of max shares per Filestore instance (multishare) - FeatureMaxSharesPerInstance *FeatureMaxSharesPerInstance - FeatureStateful *FeatureStateful - FeatureMultishareBackups *FeatureMultishareBackups + FeatureMaxSharesPerInstance *FeatureMaxSharesPerInstance + FeatureStateful *FeatureStateful + FeatureMultishareBackups *FeatureMultishareBackups + FeatureNFSExportOptionsOnCreate *FeatureNFSExportOptionsOnCreate } type FeatureMultishareBackups struct { Enabled bool } +type FeatureNFSExportOptionsOnCreate struct { + Enabled bool +} + type FeatureStateful struct { Enabled bool KubeAPIQPS float64 diff --git a/pkg/csi_driver/multishare_controller.go b/pkg/csi_driver/multishare_controller.go index 8d32ce44f..2e95520d1 100644 --- a/pkg/csi_driver/multishare_controller.go +++ b/pkg/csi_driver/multishare_controller.go @@ -57,16 +57,17 @@ const ( // MultishareController handles CSI calls for volumes which use Filestore multishare instances. type MultishareController struct { - driver *GCFSDriver - fileService file.Service - cloud *cloud.Cloud - opsManager *MultishareOpsManager - volumeLocks *util.VolumeLocks - ecfsDescription string - isRegional bool - clustername string - featureMaxSharePerInstance bool - featureMultishareBackups bool + driver *GCFSDriver + fileService file.Service + cloud *cloud.Cloud + opsManager *MultishareOpsManager + volumeLocks *util.VolumeLocks + ecfsDescription string + isRegional bool + clustername string + featureMaxSharePerInstance bool + featureMultishareBackups bool + featureNFSExportOptionsOnCreate bool // Filestore instance description overrides descOverrideMaxSharesPerInstance string @@ -103,6 +104,9 @@ func NewMultishareController(config *controllerServerConfig) *MultishareControll if config.features != nil && config.features.FeatureMultishareBackups != nil { c.featureMultishareBackups = config.features.FeatureMultishareBackups.Enabled } + if config.features != nil && config.features.FeatureNFSExportOptionsOnCreate != nil { + c.featureNFSExportOptionsOnCreate = config.features.FeatureNFSExportOptionsOnCreate.Enabled + } return c } @@ -570,6 +574,12 @@ func (m *MultishareController) generateNewMultishareInstance(instanceName string } case ParamInstanceEncryptionKmsKey: kmsKeyName = v + // Ensure we don't flag the nfsExportOptions param as invalid. Value will be used when creating a new share + case ParamNfsExportOptions: + if !m.featureNFSExportOptionsOnCreate { + return nil, status.Error(codes.InvalidArgument, "nfsExportOptions are disabled") + } + continue // 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 @@ -657,14 +667,22 @@ func generateNewShare(name string, parent *file.MultishareInstance, req *csi.Cre if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } + var nfsExportOptions []*file.NfsExportOptions + if req.GetParameters()[ParamNfsExportOptions] != "" { + nfsExportOptions, err = parseNfsExportOptions(req.GetParameters()[ParamNfsExportOptions]) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + } share := &file.Share{ - Name: name, - Parent: parent, - CapacityBytes: targetSizeBytes, - Labels: extractShareLabels(req.Parameters), - MountPointName: name, - BackupId: sourceSnapshotId, + Name: name, + Parent: parent, + CapacityBytes: targetSizeBytes, + Labels: extractShareLabels(req.Parameters), + NfsExportOptions: nfsExportOptions, + MountPointName: name, + BackupId: sourceSnapshotId, } return share, nil } diff --git a/pkg/csi_driver/multishare_controller_test.go b/pkg/csi_driver/multishare_controller_test.go index c75e49ff6..7692bbd08 100644 --- a/pkg/csi_driver/multishare_controller_test.go +++ b/pkg/csi_driver/multishare_controller_test.go @@ -792,6 +792,14 @@ func TestMultishareCreateVolume(t *testing.T) { testShareName := util.ConvertVolToShareName(testVolName) testInstanceName1 := "fs-" + string(uuid.NewUUID()) testInstanceName2 := "fs-" + string(uuid.NewUUID()) + features := &GCFSDriverFeatureOptions{ + FeatureMultishareBackups: &FeatureMultishareBackups{ + Enabled: true, + }, + FeatureNFSExportOptionsOnCreate: &FeatureNFSExportOptionsOnCreate{ + Enabled: true, + }, + } type OpItem struct { id string target string @@ -806,8 +814,10 @@ func TestMultishareCreateVolume(t *testing.T) { initShares []*file.Share req *csi.CreateVolumeRequest resp *csi.CreateVolumeResponse + expectedOptions []*file.NfsExportOptions errorExpected bool checkOnlyVolidFmt bool // for auto generated instance, the instance name is not known + features *GCFSDriverFeatureOptions }{ { name: "create volume called with volume size < 100G in required bytes", @@ -924,6 +934,126 @@ func TestMultishareCreateVolume(t *testing.T) { }, checkOnlyVolidFmt: true, }, + { + name: "no initial instances, create instance and share, success response", + req: &csi.CreateVolumeRequest{ + Name: testVolName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: 100 * util.Gb, + }, + Parameters: map[string]string{ + ParamMultishareInstanceScLabel: testInstanceScPrefix, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + checkOnlyVolidFmt: true, + }, + { + name: "nfs-export-options feature is disabled", + req: &csi.CreateVolumeRequest{ + Name: testVolName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: 100 * util.Gb, + }, + Parameters: map[string]string{ + ParamMultishareInstanceScLabel: testInstanceScPrefix, + ParamNfsExportOptions: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + errorExpected: true, + }, + { + name: "add nfs-export-options", + req: &csi.CreateVolumeRequest{ + Name: testVolName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: 100 * util.Gb, + }, + Parameters: map[string]string{ + ParamMultishareInstanceScLabel: testInstanceScPrefix, + ParamNfsExportOptions: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + features: features, + checkOnlyVolidFmt: true, + expectedOptions: []*file.NfsExportOptions{ + { + AccessMode: "READ_WRITE", + IpRanges: []string{"10.0.0.0/24"}, + SquashMode: "ROOT_SQUASH", + AnonGid: 1003, + AnonUid: 1003, + }, + { + AccessMode: "READ_ONLY", + IpRanges: []string{"10.0.0.0/28"}, + SquashMode: "NO_ROOT_SQUASH", + }, + }, + }, { 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{ @@ -1148,6 +1278,7 @@ func TestMultishareCreateVolume(t *testing.T) { cloud: cloudProvider, volumeLocks: util.NewVolumeLocks(), ecfsDescription: "", + features: tc.features, } mcs := NewMultishareController(config) resp, err := mcs.CreateVolume(context.Background(), tc.req) @@ -1155,7 +1286,19 @@ func TestMultishareCreateVolume(t *testing.T) { t.Errorf("expected error not found") } if !tc.errorExpected && err != nil { - t.Errorf("unexpected error") + t.Errorf("unexpected error %s", err) + } + if !tc.errorExpected && tc.req.Parameters[ParamNfsExportOptions] != "" { + instance, err := s.GetShare(context.TODO(), &file.Share{Name: util.ConvertVolToShareName(tc.req.Name)}) + if err != nil { + t.Errorf("test %q failed: couldn't get instance %v: %v", tc.name, tc.req.Name, err) + return + } + for i := range tc.expectedOptions { + if !reflect.DeepEqual(instance.NfsExportOptions[i], tc.expectedOptions[i]) { + t.Errorf("tc %q failed; nfs export options not equal at index %d: got %+v, expected %+v", tc.name, i, instance.NfsExportOptions[i], tc.expectedOptions[i]) + } + } } if tc.checkOnlyVolidFmt { if !strings.Contains(resp.Volume.VolumeId, modeMultishare) || !strings.Contains(resp.Volume.VolumeId, testShareName) { @@ -1199,6 +1342,9 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { FeatureMultishareBackups: &FeatureMultishareBackups{ Enabled: true, }, + FeatureNFSExportOptionsOnCreate: &FeatureNFSExportOptionsOnCreate{ + Enabled: true, + }, } defaultBackup := &BackupTestInfo{ @@ -1227,6 +1373,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { req *csi.CreateVolumeRequest resp *csi.CreateVolumeResponse checkOnlyVolidFmt bool + expectedOptions []*file.NfsExportOptions initialBackup *BackupTestInfo features *GCFSDriverFeatureOptions errorExpected bool @@ -1259,6 +1406,78 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { checkOnlyVolidFmt: true, errorExpected: true, }, + { + name: "create volume called with volume content source and nfsExportOptions set", + req: &csi.CreateVolumeRequest{ + Name: testVolName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: 100 * util.Gb, + }, + Parameters: map[string]string{ + ParamMultishareInstanceScLabel: testInstanceScPrefix, + ParamNfsExportOptions: `[ + { + "accessMode": "READ_WRITE", + "ipRanges": [ + "10.0.0.0/24", + "10.124.124.0/28" + ], + "squashMode": "ROOT_SQUASH", + "anonUid": "1003", + "anonGid": "1003" + }, + { + "accessMode": "READ_ONLY", + "ipRanges": [ + "10.0.0.0/28" + ], + "squashMode": "NO_ROOT_SQUASH" + } + ]`, + }, + VolumeCapabilities: volumeCapabilities, + VolumeContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{ + SnapshotId: "projects/test-project/locations/us-central1/backups/mybackup", + }, + }, + }, + }, + features: features, + expectedOptions: []*file.NfsExportOptions{ + { + AccessMode: "READ_WRITE", + IpRanges: []string{"10.0.0.0/24", "10.124.124.0/28"}, + SquashMode: "ROOT_SQUASH", + AnonGid: 1003, + AnonUid: 1003, + }, + { + AccessMode: "READ_ONLY", + IpRanges: []string{"10.0.0.0/28"}, + SquashMode: "NO_ROOT_SQUASH", + }, + }, + resp: &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: 100 * util.Gb, + VolumeId: fmt.Sprintf(multishareVolIdFmt, testInstanceScPrefix, testProject, testRegion, testInstanceName1, testShareName), + VolumeContext: map[string]string{ + attrIP: testIP, + }, + ContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{ + SnapshotId: "projects/test-project/locations/us-central1/backups/mybackup", + }, + }, + }, + }, + }, + initialBackup: defaultBackup, + checkOnlyVolidFmt: true, + }, { name: "create volume called with volume content source, no existing instance or share", req: &csi.CreateVolumeRequest{ @@ -1612,6 +1831,18 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { if !tc.errorExpected && err != nil { t.Errorf("unexpected error") } + if !tc.errorExpected && tc.req.Parameters[ParamNfsExportOptions] != "" { + instance, err := s.GetShare(context.TODO(), &file.Share{Name: util.ConvertVolToShareName(tc.req.Name)}) + if err != nil { + t.Errorf("test %q failed: couldn't get instance %v: %v", tc.name, tc.req.Name, err) + return + } + for i := range tc.expectedOptions { + if !reflect.DeepEqual(instance.NfsExportOptions[i], tc.expectedOptions[i]) { + t.Errorf("tc %q failed; nfs export options not equal at index %d: got %+v, expected %+v", tc.name, i, instance.NfsExportOptions[i], tc.expectedOptions[i]) + } + } + } if tc.checkOnlyVolidFmt && !tc.errorExpected { if !strings.Contains(resp.Volume.VolumeId, modeMultishare) || !strings.Contains(resp.Volume.VolumeId, testShareName) { t.Errorf("unexpected vol id %s", resp.Volume.VolumeId)