diff --git a/cmd/kubectl-directpv/move.go b/cmd/kubectl-directpv/move.go index f2beafc15..7e270c4c4 100644 --- a/cmd/kubectl-directpv/move.go +++ b/cmd/kubectl-directpv/move.go @@ -145,7 +145,6 @@ func moveMain(ctx context.Context, src, dest string) { quietFlag, true, "insufficient free capacity on destination drive; required=%v free=%v\n", - destDrive.Name, printableBytes(requiredCapacity), printableBytes(destDrive.Status.FreeCapacity), ) diff --git a/pkg/client/dynamic_test.go b/pkg/client/dynamic_test.go index 13a8d1473..4fbda24d4 100644 --- a/pkg/client/dynamic_test.go +++ b/pkg/client/dynamic_test.go @@ -22,6 +22,7 @@ import ( "reflect" "testing" + "github.com/google/uuid" directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types" "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/types" @@ -102,27 +103,19 @@ func newFakeLatestVolumeClientForList(backendVersion, resource, value string, un return &latestVolumeClient{newFakeDynamicInterfaceForList(backendVersion, resource, value, unstructuredObjects)} } -func createTestDrive(node, drive, backendVersion string, labels map[string]string) *types.Drive { - return &types.Drive{ - TypeMeta: metav1.TypeMeta{ - APIVersion: consts.GroupName + "/" + backendVersion, - Kind: consts.DriveKind, +func createTestDrive(nodeID directpvtypes.NodeID, driveID directpvtypes.DriveID, accessTier directpvtypes.AccessTier) *types.Drive { + return types.NewDrive( + driveID, + types.DriveStatus{ + TotalCapacity: 20 * MiB, + FreeCapacity: 20 * MiB, + FSUUID: string(driveID), + Status: directpvtypes.DriveStatusReady, }, - ObjectMeta: metav1.ObjectMeta{ - Name: drive, - Finalizers: []string{ - string(consts.DriveFinalizerDataProtection), - }, - Labels: labels, - }, - Status: types.DriveStatus{ - NodeName: node, - Status: directpvtypes.DriveStatusOK, - FreeCapacity: 20 * MiB, - AllocatedCapacity: 0, - TotalCapacity: 20 * MiB, - }, - } + nodeID, + directpvtypes.DriveName("sda"), + accessTier, + ) } func getFakeLatestDriveClient(t *testing.T, i int, drive runtime.Object, version string) *latestDriveClient { @@ -142,7 +135,7 @@ func TestGetDrive(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "drive1", - object: createTestDrive("node1", "drive1", consts.LatestAPIVersion, map[string]string{}), + object: createTestDrive("node1", "drive1", directpvtypes.AccessTierDefault), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -170,8 +163,8 @@ func TestListDrive(t *testing.T) { apiVersion: consts.LatestAPIVersion, name: "drive1", objects: []runtime.Object{ - createTestDrive("node1", "drive1", consts.LatestAPIVersion, map[string]string{}), - createTestDrive("node1", "drive2", consts.LatestAPIVersion, map[string]string{}), + createTestDrive("node1", "drive1", directpvtypes.AccessTierDefault), + createTestDrive("node1", "drive2", directpvtypes.AccessTierDefault), }, }, } @@ -208,12 +201,9 @@ func TestListDriveWithOption(t *testing.T) { apiVersion: consts.LatestAPIVersion, names: []string{"drive2", "drive3"}, objects: []runtime.Object{ - createTestDrive("node1", "drive1", consts.LatestAPIVersion, - map[string]string{string(types.NodeLabelKey): "node1", string(types.AccessTierLabelKey): "Hot"}), - createTestDrive("node2", "drive2", consts.LatestAPIVersion, - map[string]string{string(types.NodeLabelKey): "node2", string(types.AccessTierLabelKey): "Hot"}), - createTestDrive("node2", "drive3", consts.LatestAPIVersion, - map[string]string{string(types.NodeLabelKey): "node2", string(types.AccessTierLabelKey): "Hot"}), + createTestDrive("node1", "drive1", directpvtypes.AccessTierHot), + createTestDrive("node2", "drive2", directpvtypes.AccessTierHot), + createTestDrive("node2", "drive3", directpvtypes.AccessTierHot), }, }, } @@ -230,13 +220,13 @@ func TestListDriveWithOption(t *testing.T) { } client := newFakeLatestDriveClientForList(testCase.apiVersion, consts.DriveResource, "DirectPVDriveList", unstructuredObjects...) - labelMap := map[types.LabelKey][]types.LabelValue{ - types.NodeLabelKey: {types.NewLabelValue("node2")}, + labelMap := map[directpvtypes.LabelKey][]directpvtypes.LabelValue{ + directpvtypes.NodeLabelKey: {directpvtypes.NewLabelValue("node2")}, } if testCase.apiVersion == consts.LatestAPIVersion { - labelMap[types.AccessTierLabelKey] = []types.LabelValue{types.NewLabelValue("Hot")} + labelMap[directpvtypes.AccessTierLabelKey] = []directpvtypes.LabelValue{directpvtypes.NewLabelValue("Hot")} } - driveList, err := client.List(ctx, metav1.ListOptions{LabelSelector: types.ToLabelSelector(labelMap)}) + driveList, err := client.List(ctx, metav1.ListOptions{LabelSelector: directpvtypes.ToLabelSelector(labelMap)}) if err != nil { t.Fatalf("case %v: unexpected error: %v", i+1, err) } @@ -264,8 +254,8 @@ func TestCreateDrive(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "drive3", - inputDrive: createTestDrive("node1", "drive3", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}), - newDrive: createTestDrive("node1", "new-drive3", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}), + inputDrive: createTestDrive("node1", "drive3", directpvtypes.AccessTierHot), + newDrive: createTestDrive("node1", "new-drive3", directpvtypes.AccessTierHot), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -291,7 +281,7 @@ func TestDeleteDrive(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "drive2", - inputDrive: createTestDrive("node1", "drive2", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}), + inputDrive: createTestDrive("node1", "drive2", directpvtypes.AccessTierDefault), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -316,7 +306,7 @@ func TestDeleteCollectionDrive(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "drive2", - inputDrive: createTestDrive("node1", "drive2", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}), + inputDrive: createTestDrive("node1", "drive2", directpvtypes.AccessTierDefault), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -343,14 +333,14 @@ func TestUpdateDrive(t *testing.T) { apiVersion: consts.LatestAPIVersion, name: "drive2", accessTier: directpvtypes.AccessTierHot, - inputDrive: createTestDrive("node1", "drive2", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}), + inputDrive: createTestDrive("node1", "drive2", directpvtypes.AccessTierDefault), }, } ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() for i, testCase := range testCases { client := getFakeLatestDriveClient(t, i, testCase.inputDrive, testCase.apiVersion) - testCase.inputDrive.Status.AccessTier = testCase.accessTier + testCase.inputDrive.SetAccessTier(testCase.accessTier) updatedDrive, err := client.Update(ctx, testCase.inputDrive, metav1.UpdateOptions{}) if err != nil { t.Fatalf("case %v: unexpected error: %v", i+1, err) @@ -358,8 +348,8 @@ func TestUpdateDrive(t *testing.T) { if expectedGV != updatedDrive.GetObjectKind().GroupVersionKind().GroupVersion() { t.Fatalf("case %v: groupVersion: expected: %v, got: %v", i+1, expectedGV, updatedDrive.GetObjectKind().GroupVersionKind().GroupVersion()) } - if updatedDrive.Status.AccessTier != testCase.accessTier { - t.Fatalf("case %v: accessTier: expected: %v, got: %v", i+1, updatedDrive.Status.AccessTier, testCase.accessTier) + if updatedDrive.GetAccessTier() != testCase.accessTier { + t.Fatalf("case %v: accessTier: expected: %v, got: %v", i+1, updatedDrive.GetAccessTier(), testCase.accessTier) } } } @@ -375,14 +365,14 @@ func TestUpdateStatusDrive(t *testing.T) { apiVersion: consts.LatestAPIVersion, name: "drive2", accessTier: directpvtypes.AccessTierHot, - inputDrive: createTestDrive("node1", "drive2", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}), + inputDrive: createTestDrive("node1", "drive2", directpvtypes.AccessTierDefault), }, } ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() for i, testCase := range testCases { client := getFakeLatestDriveClient(t, i, testCase.inputDrive, testCase.apiVersion) - testCase.inputDrive.Status.AccessTier = testCase.accessTier + testCase.inputDrive.SetAccessTier(testCase.accessTier) updatedDrive, err := client.UpdateStatus(ctx, testCase.inputDrive, metav1.UpdateOptions{}) if err != nil { t.Fatalf("case %v: unexpected error: %v", i+1, err) @@ -390,8 +380,8 @@ func TestUpdateStatusDrive(t *testing.T) { if expectedGV != updatedDrive.GetObjectKind().GroupVersionKind().GroupVersion() { t.Fatalf("case %v: groupVersion: expected: %v, got: %v", i+1, expectedGV, updatedDrive.GetObjectKind().GroupVersionKind().GroupVersion()) } - if updatedDrive.Status.AccessTier != testCase.accessTier { - t.Fatalf("case %v: accessTier: expected: %v, got: %v", i+1, updatedDrive.Status.AccessTier, testCase.accessTier) + if updatedDrive.GetAccessTier() != testCase.accessTier { + t.Fatalf("case %v: accessTier: expected: %v, got: %v", i+1, updatedDrive.GetAccessTier(), testCase.accessTier) } } } @@ -414,7 +404,7 @@ func TestWatcher(t *testing.T) { return nil } - inputDrive := createTestDrive("node1", "drive1", consts.LatestAPIVersion, map[string]string{string(types.NodeLabelKey): "node1"}) + inputDrive := createTestDrive("node1", "drive1", directpvtypes.AccessTierDefault) fakeDriveClient := getFakeLatestDriveClient(t, 0, inputDrive, consts.LatestAPIVersion) fakeWatchInterface, err := fakeDriveClient.Watch(ctx, metav1.ListOptions{}) if err != nil { @@ -473,24 +463,18 @@ func TestWatcher(t *testing.T) { } } -func createTestVolume(volName string, apiVersion string) *types.Volume { - return &types.Volume{ - TypeMeta: metav1.TypeMeta{ - APIVersion: string(consts.GroupName + "/" + apiVersion), - Kind: consts.VolumeKind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: volName, - Labels: map[string]string{ - tenantLabel: testTenantName, - }, - }, - Status: types.VolumeStatus{ - NodeName: testNodeName, - DriveName: testDrivePath, - TargetPath: "/path/targetpath", - }, - } +func createTestVolume(volName string) *types.Volume { + fsuuid := uuid.NewString() + volume := types.NewVolume( + volName, + fsuuid, + testNodeName, + directpvtypes.DriveID(fsuuid), + testDrivePath, + 10*MiB, + ) + volume.SetLabel(consts.GroupName+"/tenant", testTenantName) + return volume } func getFakeLatestVolumeClient(drive runtime.Object, i int, version string, t *testing.T) *latestVolumeClient { @@ -510,7 +494,7 @@ func TestGetVolume(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "volume2", - volume: createTestVolume("volume2", consts.LatestAPIVersion), + volume: createTestVolume("volume2"), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -536,7 +520,7 @@ func TestListVolume(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "drive2", - volumes: []runtime.Object{createTestVolume("volume1", consts.LatestAPIVersion), createTestVolume("volume2", consts.LatestAPIVersion)}, + volumes: []runtime.Object{createTestVolume("volume1"), createTestVolume("volume2")}, }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -572,8 +556,8 @@ func TestCreateVolume(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "volume2", - volume: createTestVolume("volume2", consts.LatestAPIVersion), - newVolume: createTestVolume("new-volume2", consts.LatestAPIVersion), + volume: createTestVolume("volume2"), + newVolume: createTestVolume("new-volume2"), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -599,7 +583,7 @@ func TestDeleteVolume(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "volume2", - volume: createTestVolume("volume2", consts.LatestAPIVersion), + volume: createTestVolume("volume2"), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -624,7 +608,7 @@ func TestVolumeDeleteCollection(t *testing.T) { { apiVersion: consts.LatestAPIVersion, name: "volume2", - volume: createTestVolume("volume2", consts.LatestAPIVersion), + volume: createTestVolume("volume2"), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -651,7 +635,7 @@ func TestUpdateVolume(t *testing.T) { apiVersion: consts.LatestAPIVersion, name: "volume2", availableCapacity: 20 * MiB, - volume: createTestVolume("volume2", consts.LatestAPIVersion), + volume: createTestVolume("volume2"), }, } ctx, cancelFunc := context.WithCancel(context.Background()) @@ -683,7 +667,7 @@ func TestUpdateStatusVolume(t *testing.T) { apiVersion: consts.LatestAPIVersion, name: "volume2", availableCapacity: 20 * MiB, - volume: createTestVolume("volume2", consts.LatestAPIVersion), + volume: createTestVolume("volume2"), }, } ctx, cancelFunc := context.WithCancel(context.Background()) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 4e380da94..208ae8579 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/google/uuid" directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types" "github.com/minio/directpv/pkg/client" clientsetfake "github.com/minio/directpv/pkg/clientset/fake" @@ -38,16 +39,6 @@ func init() { const MiB = 1024 * 1024 -func createFakeController() *Server { - return &Server{ - NodeID: "test-node-1", - Identity: "test-identity-1", - Rack: "test-rack-1", - Zone: "test-zone-1", - Region: "test-region-1", - } -} - func TestCreateAndDeleteVolumeRPCs(t *testing.T) { getTopologySegmentsForNode := func(node string) map[string]string { switch node { @@ -60,24 +51,20 @@ func TestCreateAndDeleteVolumeRPCs(t *testing.T) { } } - createTestDrive100MB := func(node, drive string) *types.Drive { - return &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: drive, - Finalizers: []string{ - string(consts.DriveFinalizerDataProtection), - }, - }, - Status: types.DriveStatus{ - NodeName: node, - Status: directpvtypes.DriveStatusOK, - FreeCapacity: 100 * MiB, - AllocatedCapacity: int64(0), - TotalCapacity: 100 * MiB, - Topology: getTopologySegmentsForNode(node), + createTestDrive100MB := func(nodeID directpvtypes.NodeID, driveID directpvtypes.DriveID) *types.Drive { + return types.NewDrive( + driveID, + types.DriveStatus{ + TotalCapacity: 100 * MiB, + FreeCapacity: 100 * MiB, + FSUUID: string(driveID), + Status: directpvtypes.DriveStatusReady, + Topology: getTopologySegmentsForNode(string(nodeID)), }, - } + nodeID, + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ) } create20MBVolumeRequest := func(volName string, requestedNode string) csi.CreateVolumeRequest { @@ -141,7 +128,7 @@ func TestCreateAndDeleteVolumeRPCs(t *testing.T) { } ctx := context.TODO() - cl := createFakeController() + cl := NewServer() clientset := types.NewExtFakeClientset(clientsetfake.NewSimpleClientset(testDriveObjects...)) client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) @@ -176,8 +163,8 @@ func TestCreateAndDeleteVolumeRPCs(t *testing.T) { volFinalizers := volObj.GetFinalizers() for _, f := range volFinalizers { switch f { - case consts.VolumeFinalizerPVProtection: - case consts.VolumeFinalizerPurgeProtection: + case consts.GroupName + "/pv-protection": + case consts.GroupName + "/purge-protection": default: t.Errorf("[%s] Unknown finalizer found: %v", volName, f) } @@ -190,7 +177,7 @@ func TestCreateAndDeleteVolumeRPCs(t *testing.T) { // Fetch the drive objects client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) - driveList, err := drive.GetDriveList(ctx, nil, nil, nil) + driveList, err := drive.NewLister().Get(ctx) if err != nil { t.Errorf("Listing drives failed: %v", err) } @@ -201,8 +188,8 @@ func TestCreateAndDeleteVolumeRPCs(t *testing.T) { if len(drive.GetFinalizers()) > 2 { t.Errorf("Volumes were not equally distributed among drives. Drive name: %v Finaliers: %v", drive.Name, drive.GetFinalizers()) } - if drive.Status.Status != directpvtypes.DriveStatusOK { - t.Errorf("Expected drive(%s) status: %s, But got: %v", drive.Name, string(directpvtypes.DriveStatusOK), string(drive.Status.Status)) + if drive.Status.Status != directpvtypes.DriveStatusReady { + t.Errorf("Expected drive(%s) status: %s, But got: %v", drive.Name, string(directpvtypes.DriveStatusReady), string(drive.Status.Status)) } if drive.Status.FreeCapacity != (100*MiB - 20*MiB) { t.Errorf("Expected drive(%s) FreeCapacity: %d, But got: %d", drive.Name, (100*MiB - 20*MiB), drive.Status.FreeCapacity) @@ -229,44 +216,23 @@ func TestCreateAndDeleteVolumeRPCs(t *testing.T) { } func TestAbnormalDeleteVolume(t1 *testing.T) { - testVolumeObjects := []runtime.Object{ - &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-1", - Finalizers: []string{ - string(consts.VolumeFinalizerPVProtection), - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: "node-1", - DriveName: "test-drive", - TotalCapacity: 100, - TargetPath: "", - StagingTargetPath: "/path/staging/targetpath", - UsedCapacity: 50, - }, - }, - &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume-2", - Finalizers: []string{ - string(consts.VolumeFinalizerPVProtection), - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: "node-1", - DriveName: "test-drive", - TotalCapacity: 100, - StagingTargetPath: "/path/staging/targetpath", - TargetPath: "/path/targetpath", - UsedCapacity: 50, - }, - }, - } + fsuuid := uuid.NewString() + volume1 := types.NewVolume( + "test-volume-1", + fsuuid, + "node-1", + directpvtypes.DriveID(fsuuid), + "test-drive", + 100, + ) + volume1.Status.StagingTargetPath = "/path/staging/targetpath" + volume1.Status.UsedCapacity = 50 + + volume2 := *volume1 + volume2.Name = "test-volume-2" + volume2.Status.TargetPath = "/path/targetpath" + + testVolumeObjects := []runtime.Object{volume1, &volume2} deleteVolumeRequests := []csi.DeleteVolumeRequest{ { @@ -278,7 +244,7 @@ func TestAbnormalDeleteVolume(t1 *testing.T) { } ctx := context.TODO() - cl := createFakeController() + cl := NewServer() clientset := types.NewExtFakeClientset(clientsetfake.NewSimpleClientset(testVolumeObjects...)) client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) @@ -292,7 +258,7 @@ func TestAbnormalDeleteVolume(t1 *testing.T) { } func TestControllerGetCapabilities(t *testing.T) { - result, err := createFakeController().ControllerGetCapabilities(context.TODO(), nil) + result, err := NewServer().ControllerGetCapabilities(context.TODO(), nil) if err != nil { t.Fatal(err) } @@ -346,7 +312,7 @@ func TestValidateVolumeCapabilities(t *testing.T) { }, } - controller := createFakeController() + controller := NewServer() for i, testCase := range testCases { result, err := controller.ValidateVolumeCapabilities(context.TODO(), testCase.request) if err != nil { @@ -360,55 +326,55 @@ func TestValidateVolumeCapabilities(t *testing.T) { } func TestListVolumes(t *testing.T) { - if _, err := createFakeController().ListVolumes(context.TODO(), nil); err == nil { + if _, err := NewServer().ListVolumes(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestControllerPublishVolume(t *testing.T) { - if _, err := createFakeController().ControllerPublishVolume(context.TODO(), nil); err == nil { + if _, err := NewServer().ControllerPublishVolume(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestControllerUnpublishVolume(t *testing.T) { - if _, err := createFakeController().ControllerUnpublishVolume(context.TODO(), nil); err == nil { + if _, err := NewServer().ControllerUnpublishVolume(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestControllerExpandVolume(t *testing.T) { - if _, err := createFakeController().ControllerExpandVolume(context.TODO(), nil); err == nil { + if _, err := NewServer().ControllerExpandVolume(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestControllerGetVolume(t *testing.T) { - if _, err := createFakeController().ControllerGetVolume(context.TODO(), nil); err == nil { + if _, err := NewServer().ControllerGetVolume(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestListSnapshots(t *testing.T) { - if _, err := createFakeController().ListSnapshots(context.TODO(), nil); err == nil { + if _, err := NewServer().ListSnapshots(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestCreateSnapshot(t *testing.T) { - if _, err := createFakeController().CreateSnapshot(context.TODO(), nil); err == nil { + if _, err := NewServer().CreateSnapshot(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestDeleteSnapshot(t *testing.T) { - if _, err := createFakeController().DeleteSnapshot(context.TODO(), nil); err == nil { + if _, err := NewServer().DeleteSnapshot(context.TODO(), nil); err == nil { t.Fatal("error expected") } } func TestGetCapacity(t *testing.T) { - if _, err := createFakeController().GetCapacity(context.TODO(), nil); err == nil { + if _, err := NewServer().GetCapacity(context.TODO(), nil); err == nil { t.Fatal("error expected") } } diff --git a/pkg/controller/utils_test.go b/pkg/controller/utils_test.go index 298a87869..c72b66485 100644 --- a/pkg/controller/utils_test.go +++ b/pkg/controller/utils_test.go @@ -25,7 +25,6 @@ import ( directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types" "github.com/minio/directpv/pkg/client" clientsetfake "github.com/minio/directpv/pkg/clientset/fake" - "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/types" "github.com/minio/directpv/pkg/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,94 +35,102 @@ const GiB = 1024 * 1024 * 1024 func TestGetFilteredDrives(t *testing.T) { case2Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "drive-1", - Finalizers: []string{consts.DriveFinalizerPrefix + "volume-1"}, - }, - Status: types.DriveStatus{}, - }, + *types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusReady}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), } case2Objects := []runtime.Object{&case2Result[0]} case2Request := &csi.CreateVolumeRequest{Name: "volume-1"} case3Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - }, - }, + *types.NewDrive( + "drive-2", + types.DriveStatus{Status: directpvtypes.DriveStatusReady}, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierDefault, + ), + *types.NewDrive( + "drive-3", + types.DriveStatus{Status: directpvtypes.DriveStatusReady}, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), } case3Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), &case3Result[0], &case3Result[1], } case3Request := &csi.CreateVolumeRequest{Name: "volume-1"} case4Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, + *types.NewDrive( + "drive-2", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, FreeCapacity: 4 * GiB, }, - }, - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierDefault, + ), + *types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, FreeCapacity: 2 * GiB, }, - }, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), } case4Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), &case4Result[0], &case4Result[1], } case4Request := &csi.CreateVolumeRequest{Name: "volume-1", CapacityRange: &csi.CapacityRange{RequiredBytes: 2 * GiB}} case5Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - FreeCapacity: 4 * GiB, - }, - }, + case4Result[0], } case5Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusFenced, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusLost, FreeCapacity: 2 * GiB, }, - }, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), &case5Result[0], } case5Request := &csi.CreateVolumeRequest{ @@ -133,27 +140,32 @@ func TestGetFilteredDrives(t *testing.T) { } case6Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - AccessTier: directpvtypes.AccessTierHot, - }, - }, + *types.NewDrive( + "drive-2", + types.DriveStatus{Status: directpvtypes.DriveStatusReady}, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierHot, + ), } case6Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusDeleted, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusLost, + FreeCapacity: 2 * GiB, }, - }, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), &case6Result[0], } case6Request := &csi.CreateVolumeRequest{ @@ -162,27 +174,32 @@ func TestGetFilteredDrives(t *testing.T) { } case7Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, + *types.NewDrive( + "drive-2", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region1"}, }, - }, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierHot, + ), } case7Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - }, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{Status: directpvtypes.DriveStatusReady}, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), &case7Result[0], } case7Request := &csi.CreateVolumeRequest{ @@ -193,29 +210,48 @@ func TestGetFilteredDrives(t *testing.T) { } case8Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, + *types.NewDrive( + "drive-2", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, }, - }, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierHot, + ), } case8Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, + types.NewDrive( + "drive-20", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, + Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region1"}, }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierHot, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region1"}, }, - }, - &case8Result[0], + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-2", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, + Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, + }, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierHot, + ), } case8Request := &csi.CreateVolumeRequest{ Name: "volume-1", @@ -225,18 +261,14 @@ func TestGetFilteredDrives(t *testing.T) { } case9Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - }, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + &case3Result[0], } case9Request := &csi.CreateVolumeRequest{ Name: "volume-1", @@ -247,28 +279,35 @@ func TestGetFilteredDrives(t *testing.T) { } case10Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, + *types.NewDrive( + "drive-2", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, }, - }, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierDefault, + ), } case10Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusDeleted, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusLost, Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, }, - }, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), &case10Result[0], } case10Request := &csi.CreateVolumeRequest{ @@ -279,19 +318,14 @@ func TestGetFilteredDrives(t *testing.T) { } case11Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusFenced, - Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, - }, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + case10Objects[1], } case11Request := &csi.CreateVolumeRequest{ Name: "volume-1", @@ -301,36 +335,30 @@ func TestGetFilteredDrives(t *testing.T) { } case12Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, - }, - }, + case10Result[0], } + + deletionTimestamp := metav1.Now() + drive := types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusLost, + Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, + }, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ) + drive.DeletionTimestamp = &deletionTimestamp case12Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{ - Name: "drive-1", - }, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusError, - }, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{ - Name: "drive-3", - DeletionTimestamp: func() *metav1.Time { - deleteTz := metav1.Now() - return &deleteTz - }(), - }, - Status: types.DriveStatus{ - Status: directpvtypes.DriveStatusOK, - Topology: map[string]string{"node": "node1", "rack": "rack1", "zone": "zone1", "region": "region2"}, - }, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + drive, &case10Result[0], } case12Request := &csi.CreateVolumeRequest{ @@ -341,20 +369,23 @@ func TestGetFilteredDrives(t *testing.T) { } case13Result := []types.Drive{ - { - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusOK, AccessTier: directpvtypes.AccessTierHot}, - }, + case7Result[0], } case13Objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusError}, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusDeleted}, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{Status: directpvtypes.DriveStatusLost}, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), &case13Result[0], } case13Request := &csi.CreateVolumeRequest{ @@ -397,12 +428,14 @@ func TestGetFilteredDrives(t *testing.T) { } func TestGetDrive(t *testing.T) { - case2Result := &types.Drive{ - ObjectMeta: metav1.ObjectMeta{ - Name: "drive-1", - Finalizers: []string{consts.DriveFinalizerPrefix + "volume-1"}, - }, - } + case2Result := types.NewDrive( + "drive-1", + types.DriveStatus{}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ) + case2Result.AddVolumeFinalizer("volume-1") case2Objects := []runtime.Object{case2Result} case2Request := &csi.CreateVolumeRequest{Name: "volume-1"} @@ -436,18 +469,33 @@ func TestGetDrive(t *testing.T) { } objects := []runtime.Object{ - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusError}, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-2"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusOK, FreeCapacity: 4 * GiB}, - }, - &types.Drive{ - ObjectMeta: metav1.ObjectMeta{Name: "drive-3"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusOK, FreeCapacity: 4 * GiB}, - }, + types.NewDrive( + "drive-1", + types.DriveStatus{Status: directpvtypes.DriveStatusError}, + "node-1", + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-2", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, + FreeCapacity: 4 * GiB, + }, + "node-1", + directpvtypes.DriveName("sdb"), + directpvtypes.AccessTierDefault, + ), + types.NewDrive( + "drive-3", + types.DriveStatus{ + Status: directpvtypes.DriveStatusReady, + FreeCapacity: 4 * GiB, + }, + "node-1", + directpvtypes.DriveName("sdc"), + directpvtypes.AccessTierDefault, + ), } request := &csi.CreateVolumeRequest{Name: "volume-1", CapacityRange: &csi.CapacityRange{RequiredBytes: 2 * GiB}} @@ -457,7 +505,7 @@ func TestGetDrive(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !utils.StringIn([]string{"drive-2", "drive-3"}, result.Name) { + if !utils.Contains([]string{"drive-2", "drive-3"}, result.Name) { t.Fatalf("result: expected: %v, got: %v", []string{"drive-2", "drive-3"}, result.Name) } } diff --git a/pkg/converter/converter_test.go b/pkg/converter/converter_test.go index a6f24d512..1cea54e5f 100644 --- a/pkg/converter/converter_test.go +++ b/pkg/converter/converter_test.go @@ -19,10 +19,10 @@ package converter import ( "testing" + "github.com/google/uuid" directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types" "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/types" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -30,6 +30,25 @@ import ( ) func TestMigrate(t *testing.T) { + fsuuid1 := uuid.NewString() + srcObject := types.NewDrive( + directpvtypes.DriveID(fsuuid1), + types.DriveStatus{ + TotalCapacity: 3072, + FreeCapacity: 2048, + AllocatedCapacity: 1024, + FSUUID: fsuuid1, + Status: directpvtypes.DriveStatusReady, + }, + directpvtypes.NodeID("node-name"), + directpvtypes.DriveName("sda"), + directpvtypes.AccessTierDefault, + ) + srcObject.AddVolumeFinalizer("volume-1") + srcObject.AddVolumeFinalizer("volume-2") + + destObject := *srcObject + testCases := []struct { srcObject runtime.Object destObject runtime.Object @@ -37,42 +56,8 @@ func TestMigrate(t *testing.T) { }{ // upgrade/downgrade drive LatestAPIVersion => LatestAPIVersion i.e. no-op { - srcObject: &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test-drive", - Finalizers: []string{ - string(consts.DriveFinalizerDataProtection), - consts.DriveFinalizerPrefix + "volume-1", - consts.DriveFinalizerPrefix + "volume-2", - }, - }, - Status: types.DriveStatus{ - NodeName: "node-name", - Status: directpvtypes.DriveStatusOK, - FreeCapacity: 2048, - AllocatedCapacity: 1024, - TotalCapacity: 3072, - }, - }, - destObject: &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test-drive", - Finalizers: []string{ - string(consts.DriveFinalizerDataProtection), - consts.DriveFinalizerPrefix + "volume-1", - consts.DriveFinalizerPrefix + "volume-2", - }, - }, - Status: types.DriveStatus{ - NodeName: "node-name", - Status: directpvtypes.DriveStatusOK, - FreeCapacity: 2048, - AllocatedCapacity: 1024, - TotalCapacity: 3072, - }, - }, + srcObject: srcObject, + destObject: &destObject, groupVersion: schema.GroupVersion{ Group: consts.GroupName, Version: consts.LatestAPIVersion, diff --git a/pkg/drive/event.go b/pkg/drive/event.go index bbe3eb4dc..d943c02d8 100644 --- a/pkg/drive/event.go +++ b/pkg/drive/event.go @@ -258,6 +258,7 @@ func (handler *driveEventHandler) move(ctx context.Context, drive *types.Drive) if volume.GetNodeID() != drive.GetNodeID() { return fmt.Errorf( "volume %v must be on same node of destination drive; volume node %v; desination node %v", + volume.Name, volume.GetNodeID(), drive.GetNodeID(), ) diff --git a/pkg/drive/list_test.go b/pkg/drive/list_test.go index 6a22282d5..70e6115d6 100644 --- a/pkg/drive/list_test.go +++ b/pkg/drive/list_test.go @@ -31,7 +31,7 @@ import ( func TestGetDriveList(t *testing.T) { clientset := types.NewExtFakeClientset(clientsetfake.NewSimpleClientset()) client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) - drives, err := GetDriveList(context.TODO(), nil, nil, nil) + drives, err := NewLister().Get(context.TODO()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -48,7 +48,7 @@ func TestGetDriveList(t *testing.T) { clientset = types.NewExtFakeClientset(clientsetfake.NewSimpleClientset(objects...)) client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) - drives, err = GetDriveList(context.TODO(), nil, nil, nil) + drives, err = NewLister().Get(context.TODO()) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/installer/install_test.go b/pkg/installer/install_test.go index a89895971..7a7c5f59a 100644 --- a/pkg/installer/install_test.go +++ b/pkg/installer/install_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "github.com/minio/directpv/pkg/admin" "github.com/minio/directpv/pkg/client" "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/k8s" @@ -42,6 +43,7 @@ func TestInstaller(t *testing.T) { SeccompProfile: "", ApparmorProfile: "", DryRun: false, + Credential: &admin.Credential{AccessKey: "directpvadmin", SecretKey: "directpvadmin"}, } getDiscoveryGroupsAndMethodsFn := func() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { diff --git a/pkg/listener/listener_test.go b/pkg/listener/listener_test.go index fe712261b..92710df23 100644 --- a/pkg/listener/listener_test.go +++ b/pkg/listener/listener_test.go @@ -25,7 +25,6 @@ import ( "github.com/minio/directpv/pkg/client" clientsetfake "github.com/minio/directpv/pkg/clientset/fake" - "github.com/minio/directpv/pkg/consts" pkgtypes "github.com/minio/directpv/pkg/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -86,22 +85,17 @@ func startTestController(ctx context.Context, t *testing.T, handler *testEventHa } func newVolume(name, uid string, capacity int64) *pkgtypes.Volume { - return &pkgtypes.Volume{ - TypeMeta: pkgtypes.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Finalizers: []string{ - string(consts.VolumeFinalizerPurgeProtection), - }, - UID: types.UID(uid), - }, - Status: pkgtypes.VolumeStatus{ - NodeName: nodeID, - DataPath: "datapath", - DriveName: "test-drive", - TotalCapacity: capacity, - }, - } + volume := pkgtypes.NewVolume( + name, + uid, + nodeID, + "sda", + "sda", + capacity, + ) + volume.UID = types.UID(uid) + volume.Status.DataPath = "datapath" + return volume } func getHandleFunc(t *testing.T, event EventType, volumes ...*pkgtypes.Volume) (<-chan struct{}, func(EventArgs) error) { diff --git a/pkg/metrics/collector_test.go b/pkg/metrics/collector_test.go index 99e784b6b..411735a19 100644 --- a/pkg/metrics/collector_test.go +++ b/pkg/metrics/collector_test.go @@ -44,35 +44,17 @@ const ( metricStatsBytesTotal metricType = consts.AppName + "_stats_bytes_total" ) -var volumes = []types.Volume{ - { - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test_volume_20MB", - Labels: map[string]string{tenantLabel: "tenant-1"}, - }, - Status: types.VolumeStatus{ - NodeName: "test-node-1", - DriveName: "test-drive-1", - TotalCapacity: 20 * MiB, - TargetPath: "/path/targetpath", - UsedCapacity: 10 * MiB, - }, - }, - { - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test_volume_30MB", - Labels: map[string]string{tenantLabel: "tenant-1"}, - }, - Status: types.VolumeStatus{ - NodeName: "test-node-1", - DriveName: "test-drive-1", - TotalCapacity: 30 * MiB, - TargetPath: "/path/targetpath", - UsedCapacity: 20 * MiB, - }, - }, +var volumes []types.Volume + +func init() { + volumes = []types.Volume{ + *types.NewVolume("test_volume_20MB", "fsuuid1", "test-node-1", "test-drive-1", "test-drive-1", 20*MiB), + *types.NewVolume("test_volume_30MB", "fsuuid1", "test-node-1", "test-drive-1", "test-drive-1", 30*MiB), + } + volumes[0].Status.UsedCapacity = 10 * MiB + volumes[0].Status.TargetPath = "/path/targetpath" + volumes[1].Status.UsedCapacity = 20 * MiB + volumes[1].Status.TargetPath = "/path/targetpath" } func createFakeMetricsCollector() *metricsCollector { diff --git a/pkg/node/publish_unpublish_test.go b/pkg/node/publish_unpublish_test.go index 5e9abb8bf..32a7bf699 100644 --- a/pkg/node/publish_unpublish_test.go +++ b/pkg/node/publish_unpublish_test.go @@ -24,8 +24,8 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "github.com/minio/directpv/pkg/client" clientsetfake "github.com/minio/directpv/pkg/clientset/fake" - "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/types" + "github.com/minio/directpv/pkg/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -78,20 +78,8 @@ func TestPublishUnpublishVolume(t *testing.T) { } defer os.RemoveAll(testTargetPath) - testVol := &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: testVolumeName50MB, - Finalizers: []string{ - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: testNodeName, - StagingTargetPath: testStagingPath, - TotalCapacity: 20 * MiB, - }, - } + testVol := types.NewVolume(testVolumeName50MB, "fsuuid1", testNodeName, "sda", "sda", 20*MiB) + testVol.Status.StagingTargetPath = testStagingPath publishVolumeRequest := csi.NodePublishVolumeRequest{ VolumeId: testVolumeName50MB, @@ -122,8 +110,8 @@ func TestPublishUnpublishVolume(t *testing.T) { ns := createFakeServer() // Publish volume test - ns.getMounts = func() (map[string][]string, map[string][]string, error) { - return map[string][]string{testStagingPath: {}}, nil, nil + ns.getMounts = func() (map[string]utils.StringSet, error) { + return map[string]utils.StringSet{testStagingPath: nil}, nil } _, err := ns.NodePublishVolume(ctx, &publishVolumeRequest) if err != nil { diff --git a/pkg/node/stage_unstage_test.go b/pkg/node/stage_unstage_test.go index 54a0c66c9..72b52af4a 100644 --- a/pkg/node/stage_unstage_test.go +++ b/pkg/node/stage_unstage_test.go @@ -28,6 +28,7 @@ import ( clientsetfake "github.com/minio/directpv/pkg/clientset/fake" "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/types" + "github.com/minio/directpv/pkg/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -44,55 +45,39 @@ func TestNodeStageVolume(t *testing.T) { }, } - case1Drive := &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - } - - case2Drive := &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{Name: "drive-1"}, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusOK}, - } - - case3Drive := &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "drive-1", - Finalizers: []string{consts.DriveFinalizerPrefix + "volume-id-1"}, - }, - Status: types.DriveStatus{Status: directpvtypes.DriveStatusOK}, - } + case1Drive := types.NewDrive("drive-1", types.DriveStatus{}, "node-1", "sda", directpvtypes.AccessTierDefault) + case2Drive := types.NewDrive("drive-1", types.DriveStatus{Status: directpvtypes.DriveStatusReady}, "node-1", "sda", directpvtypes.AccessTierDefault) + case3Drive := types.NewDrive("drive-1", types.DriveStatus{Status: directpvtypes.DriveStatusReady}, "node-1", "sda", directpvtypes.AccessTierDefault) + case3Drive.AddVolumeFinalizer("volume-id-1") testCases := []struct { req *csi.NodeStageVolumeRequest drive *types.Drive - mountInfo map[string][]string + mountInfo map[string]utils.StringSet }{ {case1Req, case1Drive, nil}, {case1Req, case2Drive, nil}, - {case1Req, case3Drive, map[string][]string{consts.MountRootDir: {}}}, - {case1Req, case3Drive, map[string][]string{consts.MountRootDir: {}}}, + {case1Req, case3Drive, map[string]utils.StringSet{consts.MountRootDir: nil}}, + {case1Req, case3Drive, map[string]utils.StringSet{consts.MountRootDir: nil}}, } for i, testCase := range testCases { - volume := &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{Name: testCase.req.VolumeId}, - Status: types.VolumeStatus{ - NodeName: testNodeName, - DriveName: testCase.drive.Name, - TotalCapacity: 100 * MiB, - }, - } + volume := types.NewVolume( + testCase.req.VolumeId, + "fsuuid1", + testNodeName, + "sda", + directpvtypes.DriveName(testCase.drive.Name), + 100*MiB, + ) clientset := types.NewExtFakeClientset(clientsetfake.NewSimpleClientset(volume, testCase.drive)) client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) client.SetVolumeInterface(clientset.DirectpvLatest().DirectPVVolumes()) nodeServer := createFakeServer() - nodeServer.getMounts = func() (map[string][]string, map[string][]string, error) { - return testCase.mountInfo, nil, nil + nodeServer.getMounts = func() (map[string]utils.StringSet, error) { + return testCase.mountInfo, nil } nodeServer.bindMount = func(source, stagingTargetPath string, readOnly bool) error { if _, found := testCase.mountInfo[source]; !found { @@ -110,39 +95,29 @@ func TestStageUnstageVolume(t *testing.T) { testDriveName := "test_drive" testVolumeName50MB := "test_volume_50MB" - testObjects := []runtime.Object{ - &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: testDriveName, - Finalizers: []string{ - string(consts.DriveFinalizerDataProtection), - consts.DriveFinalizerPrefix + testVolumeName50MB, - }, - }, - Status: types.DriveStatus{ - NodeName: testNodeName, - Status: directpvtypes.DriveStatusOK, - FreeCapacity: 50 * MiB, - AllocatedCapacity: 50 * MiB, - TotalCapacity: 100 * MiB, - }, - }, - &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: testVolumeName50MB, - Finalizers: []string{ - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: testNodeName, - DriveName: testDriveName, - FSUUID: testDriveName, - TotalCapacity: 20 * MiB, - }, + drive := types.NewDrive( + directpvtypes.DriveID(testDriveName), + types.DriveStatus{ + TotalCapacity: 100 * MiB, + FreeCapacity: 50 * MiB, + AllocatedCapacity: 50 * MiB, + Status: directpvtypes.DriveStatusReady, }, + testNodeName, + "sda", + directpvtypes.AccessTierDefault, + ) + drive.AddVolumeFinalizer(testVolumeName50MB) + testObjects := []runtime.Object{ + drive, + types.NewVolume( + testVolumeName50MB, + testDriveName, + testNodeName, + directpvtypes.DriveID(testDriveName), + directpvtypes.DriveName(testDriveName), + 20*MiB, + ), } stageVolumeRequest := csi.NodeStageVolumeRequest{ @@ -172,8 +147,8 @@ func TestStageUnstageVolume(t *testing.T) { ctx := context.TODO() ns := createFakeServer() dataPath := path.Join(consts.MountRootDir, testDriveName, ".FSUUID."+testDriveName, testVolumeName50MB) - ns.getMounts = func() (map[string][]string, map[string][]string, error) { - return map[string][]string{consts.MountRootDir: {}}, nil, nil + ns.getMounts = func() (map[string]utils.StringSet, error) { + return map[string]utils.StringSet{consts.MountRootDir: nil}, nil } // Stage Volume test diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 2366e60ca..83767bf27 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -21,7 +21,6 @@ import ( "testing" "github.com/minio/directpv/pkg/consts" - "github.com/minio/directpv/pkg/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -32,7 +31,7 @@ func TestWriteObject(t *testing.T) { Name: consts.GroupName, Namespace: consts.GroupName, Annotations: map[string]string{ - string(types.CreatedByLabelKey): "kubectl/directpv", + consts.GroupName + "/created-by": "kubectl/directpv", }, Labels: map[string]string{ "app": consts.GroupName, diff --git a/pkg/volume/event.go b/pkg/volume/event.go index 87a58fb62..b43836ca3 100644 --- a/pkg/volume/event.go +++ b/pkg/volume/event.go @@ -146,7 +146,9 @@ func (handler *volumeEventHandler) delete(ctx context.Context, volume *types.Vol ) if err == nil { - client.Eventf(volume, client.EventTypeNormal, client.EventReasonVolumeReleased, "volume is released") + if len(volume.Finalizers) != 0 { // This should not happen here. + client.Eventf(volume, client.EventTypeNormal, client.EventReasonVolumeReleased, "volume is released") + } } return err diff --git a/pkg/volume/event_test.go b/pkg/volume/event_test.go index 1e68d137f..76a5b8acf 100644 --- a/pkg/volume/event_test.go +++ b/pkg/volume/event_test.go @@ -24,7 +24,6 @@ import ( directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types" "github.com/minio/directpv/pkg/client" clientsetfake "github.com/minio/directpv/pkg/clientset/fake" - "github.com/minio/directpv/pkg/consts" "github.com/minio/directpv/pkg/listener" "github.com/minio/directpv/pkg/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,12 +32,10 @@ import ( const MiB = 1024 * 1024 -func createFakeVolumeEventListener(nodeName string) *volumeEventHandler { +func createFakeVolumeEventListener(nodeID directpvtypes.NodeID) *volumeEventHandler { return &volumeEventHandler{ - nodeID: nodeName, - safeUnmount: func(target string, force, detach, expire bool) error { - return nil - }, + nodeID: nodeID, + unmount: func(target string) error { return nil }, } } @@ -46,57 +43,41 @@ func TestVolumeEventHandlerHandle(t *testing.T) { testDriveName := "test_drive" testVolumeName20MB := "test_volume_20MB" testVolumeName30MB := "test_volume_30MB" - testDriveObject := &types.Drive{ - TypeMeta: types.NewDriveTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: testDriveName, - Finalizers: []string{ - string(consts.DriveFinalizerDataProtection), - consts.DriveFinalizerPrefix + testVolumeName20MB, - consts.DriveFinalizerPrefix + testVolumeName30MB, - }, - }, - Status: types.DriveStatus{ - NodeName: "test-node", - Status: directpvtypes.DriveStatusOK, + testDriveObject := types.NewDrive( + directpvtypes.DriveID(testDriveName), + types.DriveStatus{ + TotalCapacity: 100 * MiB, FreeCapacity: 50 * MiB, AllocatedCapacity: 50 * MiB, - TotalCapacity: 100 * MiB, + Status: directpvtypes.DriveStatusReady, }, - } + "test-node", + "sda", + directpvtypes.AccessTierDefault, + ) + testDriveObject.AddVolumeFinalizer(testVolumeName20MB) + testDriveObject.AddVolumeFinalizer(testVolumeName30MB) + + volume := types.NewVolume( + testVolumeName30MB, + "fsuuid1", + "test-node", + directpvtypes.DriveID(testDriveName), + directpvtypes.DriveName(testDriveName), + 30*MiB, + ) + volume.Status.StagingTargetPath = "/path/staging" + volume.Status.TargetPath = "/path/target" testVolumeObjects := []runtime.Object{ - &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: testVolumeName20MB, - Finalizers: []string{ - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: "test-node", - DataPath: "hostpath", - DriveName: testDriveName, - TotalCapacity: 20 * MiB, - }, - }, - &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: testVolumeName30MB, - Finalizers: []string{ - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: "test-node", - DataPath: "hostpath", - DriveName: testDriveName, - TotalCapacity: 30 * MiB, - StagingTargetPath: "/path/staging", - TargetPath: "/path/target", - }, - }, + types.NewVolume( + testVolumeName20MB, + "fsuuid1", + "test-node", + directpvtypes.DriveID(testDriveName), + directpvtypes.DriveName(testDriveName), + 20*MiB, + ), + volume, } vl := createFakeVolumeEventListener("test-node") @@ -109,7 +90,7 @@ func TestVolumeEventHandlerHandle(t *testing.T) { for _, testObj := range testVolumeObjects { var stagingUmountCalled, targetUmountCalled bool - vl.safeUnmount = func(target string, force, detach, expire bool) error { + vl.unmount = func(target string) error { if testObj.(*types.Volume).Status.StagingTargetPath == "" && testObj.(*types.Volume).Status.TargetPath == "" { return errors.New("umount should never be called for volumes with empty staging and target paths") } @@ -132,6 +113,7 @@ func TestVolumeEventHandlerHandle(t *testing.T) { now := metav1.Now() newObj.DeletionTimestamp = &now + newObj.RemovePVProtection() _, vErr = client.VolumeClient().Update( ctx, newObj, metav1.UpdateOptions{TypeMeta: types.NewVolumeTypeMeta()}, @@ -164,12 +146,11 @@ func TestVolumeEventHandlerHandle(t *testing.T) { t.Fatalf("Error while getting the drive object: %+v", dErr) } - driveFinalizers := driveObj.GetFinalizers() - if len(driveFinalizers) != 1 || driveFinalizers[0] != consts.DriveFinalizerDataProtection { - t.Fatalf("Unexpected drive finalizers set after clean-up: %+v", driveFinalizers) + if driveObj.GetVolumeCount() != 0 { + t.Fatalf("Unexpected drive finalizers set after clean-up: %+v", driveObj.GetFinalizers()) } - if driveObj.Status.Status != directpvtypes.DriveStatusOK { - t.Errorf("Unexpected drive status set. Expected: %s, Got: %s", string(directpvtypes.DriveStatusOK), string(driveObj.Status.Status)) + if driveObj.Status.Status != directpvtypes.DriveStatusReady { + t.Errorf("Unexpected drive status set. Expected: %s, Got: %s", directpvtypes.DriveStatusReady, driveObj.Status.Status) } if driveObj.Status.FreeCapacity != 100*MiB { t.Errorf("Unexpected free capacity set. Expected: %d, Got: %d", 100*MiB, driveObj.Status.FreeCapacity) @@ -180,22 +161,8 @@ func TestVolumeEventHandlerHandle(t *testing.T) { } func TestAbnormalDeleteEventHandle(t *testing.T) { - testVolumeObject := &types.Volume{ - TypeMeta: types.NewVolumeTypeMeta(), - ObjectMeta: metav1.ObjectMeta{ - Name: "test-volume", - Finalizers: []string{ - string(consts.VolumeFinalizerPVProtection), - string(consts.VolumeFinalizerPurgeProtection), - }, - }, - Status: types.VolumeStatus{ - NodeName: "test-node", - DataPath: "hostpath", - DriveName: "test-drive", - TotalCapacity: int64(100), - }, - } + testVolumeObject := types.NewVolume("test-volume", "fsuuid1", "test-node", "test-drive", "test-drive", 100) + testVolumeObject.Status.DataPath = "data/path" vl := createFakeVolumeEventListener("test-node") ctx := context.TODO() diff --git a/pkg/volume/list_test.go b/pkg/volume/list_test.go index 08c55dc36..d5e43c620 100644 --- a/pkg/volume/list_test.go +++ b/pkg/volume/list_test.go @@ -33,7 +33,7 @@ func TestGetVolumeList(t *testing.T) { client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) client.SetVolumeInterface(clientset.DirectpvLatest().DirectPVVolumes()) - volumes, err := GetVolumeList(context.TODO(), nil, nil, nil, nil, nil) + volumes, err := NewLister().Get(context.TODO()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -52,7 +52,7 @@ func TestGetVolumeList(t *testing.T) { client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) client.SetVolumeInterface(clientset.DirectpvLatest().DirectPVVolumes()) - volumes, err = GetVolumeList(context.TODO(), nil, nil, nil, nil, nil) + volumes, err = NewLister().Get(context.TODO()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -66,7 +66,7 @@ func TestGetSortedVolumeList(t *testing.T) { client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) client.SetVolumeInterface(clientset.DirectpvLatest().DirectPVVolumes()) - volumes, err := GetVolumeList(context.TODO(), nil, nil, nil, nil, nil) + volumes, err := NewLister().Get(context.TODO()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -95,7 +95,7 @@ func TestGetSortedVolumeList(t *testing.T) { client.SetDriveInterface(clientset.DirectpvLatest().DirectPVDrives()) client.SetVolumeInterface(clientset.DirectpvLatest().DirectPVVolumes()) - volumes, err = GetVolumeList(context.TODO(), nil, nil, nil, nil, nil) + volumes, err = NewLister().Get(context.TODO()) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/xfs/xfs_test.go b/pkg/xfs/probe_linux_test.go similarity index 93% rename from pkg/xfs/xfs_test.go rename to pkg/xfs/probe_linux_test.go index ed5c58e16..30e6db09c 100644 --- a/pkg/xfs/xfs_test.go +++ b/pkg/xfs/probe_linux_test.go @@ -1,3 +1,5 @@ +//go:build linux + // This file is part of MinIO DirectPV // Copyright (c) 2021, 2022 MinIO, Inc. // @@ -21,7 +23,7 @@ import ( "testing" ) -func TestProbe(t *testing.T) { +func TestReadSuperBlock(t *testing.T) { testCases := []struct { filename string fsuuid string @@ -43,7 +45,7 @@ func TestProbe(t *testing.T) { } defer file.Close() - fsuuid, label, totalCapacity, freeCapacity, err := Probe(file) + fsuuid, label, totalCapacity, freeCapacity, err := readSuperBlock(file) if testCase.expectErr { if err == nil { t.Fatalf("case %v: expected error, but succeeded", i+1)