Skip to content

Commit

Permalink
Add multishare backup flag
Browse files Browse the repository at this point in the history
  • Loading branch information
hsadoyan authored and tyuchn committed Oct 3, 2023
1 parent 46a38fe commit 23ad3df
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 33 deletions.
6 changes: 6 additions & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -177,6 +180,9 @@ func main() {
LeaderElectionRenewDeadline: *leaderElectionRenewDeadline,
LeaderElectionRetryPeriod: *leaderElectionRetryPeriod,
},
FeatureMultishareBackups: &driver.FeatureMultishareBackups{
Enabled: *featureMultishareBackups,
},
}

mounter := mount.New("")
Expand Down
20 changes: 4 additions & 16 deletions pkg/csi_driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions pkg/csi_driver/gcfs_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions pkg/csi_driver/multishare_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type MultishareController struct {
isRegional bool
clustername string
featureMaxSharePerInstance bool
featureMultishareBackups bool

// Filestore instance description overrides
descOverrideMaxSharesPerInstance string
Expand Down Expand Up @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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\"")
}
Expand Down
85 changes: 68 additions & 17 deletions pkg/csi_driver/multishare_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1194,6 +1195,11 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
},
},
}
features := &GCFSDriverFeatureOptions{
FeatureMultishareBackups: &FeatureMultishareBackups{
Enabled: true,
},
}

defaultBackup := &BackupTestInfo{
backup: &file.BackupInfo{
Expand Down Expand Up @@ -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{
Expand All @@ -1243,6 +1278,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
},
},
},
features: features,
resp: &csi.CreateVolumeResponse{
Volume: &csi.Volume{
CapacityBytes: 100 * util.Gb,
Expand Down Expand Up @@ -1327,6 +1363,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
},
},
},
features: features,
resp: &csi.CreateVolumeResponse{
Volume: &csi.Volume{
CapacityBytes: 100 * util.Gb,
Expand Down Expand Up @@ -1385,6 +1422,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
},
},
},
features: features,
resp: &csi.CreateVolumeResponse{
Volume: &csi.Volume{
CapacityBytes: 100 * util.Gb,
Expand Down Expand Up @@ -1463,6 +1501,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
},
},
},
features: features,
resp: &csi.CreateVolumeResponse{
Volume: &csi.Volume{
CapacityBytes: 100 * util.Gb,
Expand Down Expand Up @@ -1500,6 +1539,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
},
},
},
features: features,
ops: []OpItem{
{
id: "op1",
Expand Down Expand Up @@ -1555,6 +1595,7 @@ func TestMultishareCreateVolumeFromBackup(t *testing.T) {
cloud: cloudProvider,
volumeLocks: util.NewVolumeLocks(),
ecfsDescription: "",
features: tc.features,
}
mcs := NewMultishareController(config)

Expand All @@ -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)
}
Expand Down Expand Up @@ -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{
Expand All @@ -2469,6 +2527,7 @@ func TestCreateMultishareSnapshot(t *testing.T) {
util.VolumeSnapshotTypeKey: "backup",
},
},
features: features,
initialBackup: &BackupTestInfo{
backup: &file.BackupInfo{
Project: testProject,
Expand All @@ -2491,6 +2550,7 @@ func TestCreateMultishareSnapshot(t *testing.T) {
util.VolumeSnapshotTypeKey: "backup",
},
},
features: features,
initialBackup: &BackupTestInfo{
backup: &file.BackupInfo{
Project: testProject,
Expand All @@ -2513,6 +2573,7 @@ func TestCreateMultishareSnapshot(t *testing.T) {
util.VolumeSnapshotTypeKey: "backup",
},
},
features: features,
initialBackup: &BackupTestInfo{
backup: &file.BackupInfo{
Project: testProject,
Expand All @@ -2536,6 +2597,7 @@ func TestCreateMultishareSnapshot(t *testing.T) {
util.VolumeSnapshotTypeKey: "backup",
},
},
features: features,
initialBackup: &BackupTestInfo{
backup: &file.BackupInfo{
Project: testProject,
Expand All @@ -2560,6 +2622,7 @@ func TestCreateMultishareSnapshot(t *testing.T) {
util.VolumeSnapshotTypeKey: "backup",
},
},
features: features,
resp: &csi.CreateSnapshotResponse{
Snapshot: &csi.Snapshot{
SizeBytes: 1 * util.Tb,
Expand All @@ -2579,6 +2642,7 @@ func TestCreateMultishareSnapshot(t *testing.T) {
util.VolumeSnapshotTypeKey: "backup",
},
},
features: features,
initialBackup: &BackupTestInfo{
backup: &file.BackupInfo{
Project: testProject,
Expand All @@ -2595,29 +2659,16 @@ 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)
if tc.initialBackup.state != "" {
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)
}
Expand Down

0 comments on commit 23ad3df

Please sign in to comment.