diff --git a/cmd/main.go b/cmd/main.go index 62d7b028c..e5f6ef070 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -60,6 +60,9 @@ var ( descOverrideMinShareSizeGB = flag.String("desc-override-min-shares-size-gb", "", "If non-empty, the filestore instance description override is used to configure min share size. This flag is ignored if 'feature-max-shares-per-instance' flag is false. Both 'desc-override-max-shares-per-instance' and 'desc-override-min-shares-size-gb' must be provided. 'ecfsDescription' is ignored, if this flag is provided.") 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") + // 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") statefulResyncPeriod = flag.Duration("stateful-resync-period", 15*time.Minute, "Resync interval of the stateful driver.") @@ -177,6 +180,9 @@ func main() { LeaderElectionRenewDeadline: *leaderElectionRenewDeadline, LeaderElectionRetryPeriod: *leaderElectionRetryPeriod, }, + FeatureMultishareBackups: &driver.FeatureMultishareBackups{ + Enabled: *featureMultishareBackups, + }, } mounter := mount.New("") diff --git a/pkg/csi_driver/controller_test.go b/pkg/csi_driver/controller_test.go index 3732a7665..ed69a889b 100644 --- a/pkg/csi_driver/controller_test.go +++ b/pkg/csi_driver/controller_test.go @@ -1523,17 +1523,6 @@ func TestDeleteSnapshot(t *testing.T) { }, expectErr: false, }, - { - name: "Create mutltishare snapshot and delete it", - createReq: &csi.CreateSnapshotRequest{ - SourceVolumeId: fmt.Sprintf("modeMultishare/%s/%s/%s", zone, instanceName, shareName), - Name: backupName, - }, - deleteReq: &csi.DeleteSnapshotRequest{ - SnapshotId: fmt.Sprintf("projects/%s/locations/%s/backups/%s", project, region, backupName), - }, - expectErr: false, - }, { name: "Backup is already in state DELETING. Expect error", createReq: &csi.CreateSnapshotRequest{ @@ -1559,11 +1548,10 @@ func TestDeleteSnapshot(t *testing.T) { } cs := newControllerServer(&controllerServerConfig{ - driver: initTestDriver(t), - fileService: fileService, - cloud: cloudProvider, - volumeLocks: util.NewVolumeLocks(), - enableMultishare: true, + driver: initTestDriver(t), + fileService: fileService, + cloud: cloudProvider, + volumeLocks: util.NewVolumeLocks(), }) _, err = cs.CreateSnapshot(context.TODO(), test.createReq) if err != nil { diff --git a/pkg/csi_driver/gcfs_driver.go b/pkg/csi_driver/gcfs_driver.go index 5ddc1f7ba..b5ae767c8 100644 --- a/pkg/csi_driver/gcfs_driver.go +++ b/pkg/csi_driver/gcfs_driver.go @@ -98,6 +98,11 @@ type GCFSDriverFeatureOptions struct { // FeatureMaxSharesPerInstance will enable CSI driver to pack configurable number of max shares per Filestore instance (multishare) FeatureMaxSharesPerInstance *FeatureMaxSharesPerInstance FeatureStateful *FeatureStateful + FeatureMultishareBackups *FeatureMultishareBackups +} + +type FeatureMultishareBackups struct { + Enabled bool } type FeatureStateful struct { diff --git a/pkg/csi_driver/multishare_controller.go b/pkg/csi_driver/multishare_controller.go index 197950999..8d32ce44f 100644 --- a/pkg/csi_driver/multishare_controller.go +++ b/pkg/csi_driver/multishare_controller.go @@ -66,6 +66,7 @@ type MultishareController struct { isRegional bool clustername string featureMaxSharePerInstance bool + featureMultishareBackups bool // Filestore instance description overrides descOverrideMaxSharesPerInstance string @@ -99,6 +100,10 @@ func NewMultishareController(config *controllerServerConfig) *MultishareControll c.pvListerSynced = pvInformer.Informer().HasSynced } + if config.features != nil && config.features.FeatureMultishareBackups != nil { + c.featureMultishareBackups = config.features.FeatureMultishareBackups.Enabled + } + return c } @@ -124,6 +129,7 @@ func (m *MultishareController) CreateVolume(ctx context.Context, req *csi.Create if err := m.driver.validateVolumeCapabilities(req.GetVolumeCapabilities()); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } + sourceSnapshotId, err := m.checkVolumeContentSource(ctx, req) if err != nil { return nil, err @@ -222,6 +228,9 @@ func (m *MultishareController) CreateVolume(ctx context.Context, req *csi.Create } func (m *MultishareController) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) { + if !m.featureMultishareBackups { + return nil, status.Error(codes.InvalidArgument, "CreateSnapshot is not supported for multishare backed volumes") + } klog.Infof("CreateSnapshot called for multishare with request %+v", req) name := req.GetName() volumeID := req.GetSourceVolumeId() @@ -615,6 +624,9 @@ func (m *MultishareController) generateNewMultishareInstance(instanceName string func (m *MultishareController) checkVolumeContentSource(ctx context.Context, req *csi.CreateVolumeRequest) (string, error) { if req.GetVolumeContentSource() != nil { + if !m.featureMultishareBackups { + return "", status.Error(codes.InvalidArgument, "Multishare backed volumes do not support volume content source") + } if req.GetVolumeContentSource().GetVolume() != nil { return "", status.Error(codes.InvalidArgument, "Unsupported volume content source type \"volume\"") } diff --git a/pkg/csi_driver/multishare_controller_test.go b/pkg/csi_driver/multishare_controller_test.go index 2de31c32e..c75e49ff6 100644 --- a/pkg/csi_driver/multishare_controller_test.go +++ b/pkg/csi_driver/multishare_controller_test.go @@ -78,6 +78,7 @@ func initTestMultishareControllerWithFeatureOpts(t *testing.T, features *GCFSDri if err != nil { t.Fatalf("Failed to get cloud provider: %v", err) } + cloudProvider.File = fileService config := &controllerServerConfig{ driver: initTestDriver(t), fileService: fileService, @@ -1194,6 +1195,11 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, }, } + features := &GCFSDriverFeatureOptions{ + FeatureMultishareBackups: &FeatureMultishareBackups{ + Enabled: true, + }, + } defaultBackup := &BackupTestInfo{ backup: &file.BackupInfo{ @@ -1222,8 +1228,37 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { resp *csi.CreateVolumeResponse checkOnlyVolidFmt bool initialBackup *BackupTestInfo + features *GCFSDriverFeatureOptions errorExpected bool }{ + { + name: "create volume called with volume content source, but multishare backup feature is disabled", + req: &csi.CreateVolumeRequest{ + Name: testVolName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: 100 * util.Gb, + }, + Parameters: map[string]string{ + ParamMultishareInstanceScLabel: testInstanceScPrefix, + }, + VolumeCapabilities: volumeCapabilities, + VolumeContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{ + SnapshotId: "projects/test-project/locations/us-central1/backups/mybackup", + }, + }, + }, + }, + features: &GCFSDriverFeatureOptions{ + FeatureMultishareBackups: &FeatureMultishareBackups{ + Enabled: false, + }, + }, + initialBackup: defaultBackup, + checkOnlyVolidFmt: true, + errorExpected: true, + }, { name: "create volume called with volume content source, no existing instance or share", req: &csi.CreateVolumeRequest{ @@ -1243,6 +1278,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, }, }, + features: features, resp: &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: 100 * util.Gb, @@ -1327,6 +1363,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, }, }, + features: features, resp: &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: 100 * util.Gb, @@ -1385,6 +1422,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, }, }, + features: features, resp: &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: 100 * util.Gb, @@ -1463,6 +1501,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, }, }, + features: features, resp: &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: 100 * util.Gb, @@ -1500,6 +1539,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { }, }, }, + features: features, ops: []OpItem{ { id: "op1", @@ -1555,6 +1595,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { cloud: cloudProvider, volumeLocks: util.NewVolumeLocks(), ecfsDescription: "", + features: tc.features, } mcs := NewMultishareController(config) @@ -1571,7 +1612,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) { if !tc.errorExpected && err != nil { t.Errorf("unexpected error") } - if tc.checkOnlyVolidFmt { + 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) } @@ -2452,14 +2493,31 @@ func TestCreateMultishareSnapshot(t *testing.T) { defaultSourceVolumeID := modeMultishare + "/" + testRegion + "/" + testInstanceName1 + "/" + testShareName defaultBackupUri := fmt.Sprintf("projects/%s/locations/%s/backups/%s", testProject, testRegion, backupName) + features := &GCFSDriverFeatureOptions{ + FeatureMultishareBackups: &FeatureMultishareBackups{ + Enabled: true, + }, + } cases := []struct { name string req *csi.CreateSnapshotRequest resp *csi.CreateSnapshotResponse initialBackup *BackupTestInfo + features *GCFSDriverFeatureOptions expectErr bool }{ //Failure test cases/ + { + name: "Feature multisharebackups is not enabled", + req: &csi.CreateSnapshotRequest{ + SourceVolumeId: modeMultishare + "/" + testRegion + "/" + "differnetInstanceName" + "/" + testShareName, + Name: backupName, + Parameters: map[string]string{ + util.VolumeSnapshotTypeKey: "backup", + }, + }, + expectErr: true, + }, { name: "Existing backup found, with different instance ID, error expected", req: &csi.CreateSnapshotRequest{ @@ -2469,6 +2527,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { util.VolumeSnapshotTypeKey: "backup", }, }, + features: features, initialBackup: &BackupTestInfo{ backup: &file.BackupInfo{ Project: testProject, @@ -2491,6 +2550,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { util.VolumeSnapshotTypeKey: "backup", }, }, + features: features, initialBackup: &BackupTestInfo{ backup: &file.BackupInfo{ Project: testProject, @@ -2513,6 +2573,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { util.VolumeSnapshotTypeKey: "backup", }, }, + features: features, initialBackup: &BackupTestInfo{ backup: &file.BackupInfo{ Project: testProject, @@ -2536,6 +2597,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { util.VolumeSnapshotTypeKey: "backup", }, }, + features: features, initialBackup: &BackupTestInfo{ backup: &file.BackupInfo{ Project: testProject, @@ -2560,6 +2622,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { util.VolumeSnapshotTypeKey: "backup", }, }, + features: features, resp: &csi.CreateSnapshotResponse{ Snapshot: &csi.Snapshot{ SizeBytes: 1 * util.Tb, @@ -2579,6 +2642,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { util.VolumeSnapshotTypeKey: "backup", }, }, + features: features, initialBackup: &BackupTestInfo{ backup: &file.BackupInfo{ Project: testProject, @@ -2595,21 +2659,8 @@ func TestCreateMultishareSnapshot(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - fileService, err := file.NewFakeService() - if err != nil { - t.Fatalf("failed to initialize GCFS service: %v", err) - } - - cloudProvider, _ := cloud.NewFakeCloud() - cloudProvider.File = fileService - config := &controllerServerConfig{ - driver: initTestDriver(t), - fileService: fileService, - cloud: cloudProvider, - volumeLocks: util.NewVolumeLocks(), - ecfsDescription: "", - } - mcs := NewMultishareController(config) + m := initTestMultishareControllerWithFeatureOpts(t, tc.features) + fileService := m.fileService if tc.initialBackup != nil { existingBackup, _ := fileService.CreateBackup(context.TODO(), tc.initialBackup.backup) @@ -2617,7 +2668,7 @@ func TestCreateMultishareSnapshot(t *testing.T) { existingBackup.State = tc.initialBackup.state } } - resp, err := mcs.CreateSnapshot(context.TODO(), tc.req) + resp, err := m.CreateSnapshot(context.TODO(), tc.req) if !tc.expectErr && err != nil { t.Errorf("test %q failed: %v", tc.name, err) }