From 13d281d0e54daf6ac27e74cc209efefd5e756d4e Mon Sep 17 00:00:00 2001 From: Adam Krpan Date: Tue, 20 Aug 2024 00:25:58 +0000 Subject: [PATCH] PWX-38585: Removes extra pod name label Signed-off-by: Adam Krpan --- api/server/docker.go | 1 + api/server/sdk/volume_ops.go | 14 +-- csi/controller.go | 10 +- csi/controller_test.go | 237 ++++++++++++++++++++++++++++++++++- 4 files changed, 242 insertions(+), 20 deletions(-) diff --git a/api/server/docker.go b/api/server/docker.go index f5f0493be..2073cffa6 100644 --- a/api/server/docker.go +++ b/api/server/docker.go @@ -377,6 +377,7 @@ func (d *driver) create(w http.ResponseWriter, r *http.Request) { _, err = volumes.Clone(ctx, &api.SdkVolumeCloneRequest{ Name: name, ParentId: source.Parent, + AdditionalLabels: locator.GetVolumeLabels(), }) } else { // create diff --git a/api/server/sdk/volume_ops.go b/api/server/sdk/volume_ops.go index 49d44f820..35ad75223 100644 --- a/api/server/sdk/volume_ops.go +++ b/api/server/sdk/volume_ops.go @@ -36,9 +36,6 @@ import ( "google.golang.org/grpc/status" ) -// FADAPodLabelKey is a label added to volume locators in the case of FADA volume clone/snap restore -const FADAPodLabelKey = "pure-pod-name" // Used to plumb in the pod name for volume cloning - // When create is called for an existing volume, this function is called to make sure // the SDK only returns that the volume is ready when the status is UP func (s *VolumeServer) waitForVolumeReady(ctx context.Context, id string) (*api.Volume, error) { @@ -173,18 +170,9 @@ func (s *VolumeServer) create( } // Create a snapshot from the parent - // Only include the FADA pod label - var labels map[string]string = nil - if locator.GetVolumeLabels() != nil { - if pod, ok := locator.GetVolumeLabels()[FADAPodLabelKey]; ok { - labels = map[string]string{ - FADAPodLabelKey: pod, - } - } - } id, err = s.driver(ctx).Snapshot(ctx, parent.GetId(), false, &api.VolumeLocator{ Name: volName, - VolumeLabels: labels, + VolumeLabels: locator.GetVolumeLabels(), }, false) if err != nil { if err == kvdb.ErrNotFound { diff --git a/csi/controller.go b/csi/controller.go index a01079a92..06ac108c3 100644 --- a/csi/controller.go +++ b/csi/controller.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "github.com/libopenstorage/openstorage/api" - "github.com/libopenstorage/openstorage/api/server/sdk" "github.com/libopenstorage/openstorage/pkg/grpcutil" "github.com/libopenstorage/openstorage/pkg/units" "github.com/libopenstorage/openstorage/pkg/util" @@ -614,14 +613,17 @@ func (s *OsdCsiServer) CreateVolume( } newVolumeId = createResp.VolumeId } else { - clonedMetadata := getClonedPVCMetadata(locator) + labels := locator.GetVolumeLabels() if spec.GetFADAPodName() != "" { - clonedMetadata[sdk.FADAPodLabelKey] = spec.GetFADAPodName() + labels[api.SpecPurePodName] = spec.GetFADAPodName() } + delete(labels, intreePvcNameKey) + delete(labels, intreePvcNamespaceKey) + delete(labels, api.SpecParent) cloneResp, err := volumes.Clone(ctx, &api.SdkVolumeCloneRequest{ Name: req.GetName(), ParentId: source.Parent, - AdditionalLabels: clonedMetadata, + AdditionalLabels: labels, }) if err != nil { return nil, err diff --git a/csi/controller_test.go b/csi/controller_test.go index 89031f2ba..e7c9d7713 100644 --- a/csi/controller_test.go +++ b/csi/controller_test.go @@ -33,7 +33,6 @@ import ( "github.com/golang/protobuf/ptypes/timestamp" "github.com/libopenstorage/openstorage/api" "github.com/libopenstorage/openstorage/api/mock" - "github.com/libopenstorage/openstorage/api/server/sdk" "github.com/libopenstorage/openstorage/api/spec" authsecrets "github.com/libopenstorage/openstorage/pkg/auth/secrets" mockLoadBalancer "github.com/libopenstorage/openstorage/pkg/loadbalancer/mock" @@ -1334,7 +1333,7 @@ func TestControllerCreateVolumeBadSnapshot(t *testing.T) { // Return an error from snapshot s.MockDriver(). EXPECT(). - Snapshot(gomock.Any(), parent, false, &api.VolumeLocator{Name: name}, false). + Snapshot(gomock.Any(), parent, false, &api.VolumeLocator{Name: name, VolumeLabels: nil}, false). Return("", fmt.Errorf("snapshoterr")). Times(1), ) @@ -2056,7 +2055,7 @@ func TestControllerCreateVolumeFromSnapshotFADAPod(t *testing.T) { // create s.MockDriver(). EXPECT(). - Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), &api.VolumeLocator{Name: name, VolumeLabels: map[string]string{sdk.FADAPodLabelKey: pod}}, gomock.Any()). + Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), &api.VolumeLocator{Name: name, VolumeLabels: map[string]string{api.SpecPurePodName: pod}}, gomock.Any()). Return(snapID, nil). Times(1), s.MockDriver(). @@ -2246,6 +2245,238 @@ func TestControllerCreateVolumeSnapshotThroughParameters(t *testing.T) { assert.Equal(t, mockParentID, volumeInfo.GetVolumeContext()[api.SpecParent]) } +func TestControllerCreateVolumeFromSource(t *testing.T) { + // Create server and client connection + s := newTestServer(t) + defer s.Stop() + c := csi.NewControllerClient(s.Conn()) + s.mockClusterEnumerateNode(t, "node-1") + // Setup request + mockParentID := "parendId" + name := "myvol" + size := int64(1234) + req := &csi.CreateVolumeRequest{ + Name: name, + VolumeCapabilities: []*csi.VolumeCapability{ + {}, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: size, + }, + VolumeContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Volume{ + Volume: &csi.VolumeContentSource_VolumeSource{ + VolumeId: mockParentID, + }, + }, + }, + Parameters: map[string]string{ + "testkey": "testval", + }, + Secrets: map[string]string{authsecrets.SecretTokenKey: systemUserToken}, + } + + // Setup mock functions + id := "myid" + snapID := id + "-snap" + gomock.InOrder( + + // First check on parent + s.MockDriver(). + EXPECT(). + Enumerate(&api.VolumeLocator{ + VolumeIds: []string{mockParentID}, + }, nil). + Return([]*api.Volume{{Id: mockParentID}}, nil). + Times(1), + + // VolFromName (name) + s.MockDriver(). + EXPECT(). + Inspect(gomock.Any(), []string{name}). + Return(nil, fmt.Errorf("not found")). + Times(1), + + s.MockDriver(). + EXPECT(). + Enumerate(gomock.Any(), nil). + Return(nil, fmt.Errorf("not found")). + Times(1), + + //VolFromName parent + s.MockDriver(). + EXPECT(). + Inspect(gomock.Any(), gomock.Any()). + Return( + []*api.Volume{{ + Id: mockParentID, + }}, nil). + Times(1), + + // create + s.MockDriver(). + EXPECT(). + Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), &api.VolumeLocator{Name: name, VolumeLabels: map[string]string{"testkey": "testval"}}, gomock.Any()). + Return(snapID, nil). + Times(1), + s.MockDriver(). + EXPECT(). + Enumerate(&api.VolumeLocator{ + VolumeIds: []string{snapID}, + }, nil). + Return([]*api.Volume{ + { + Id: id, + Source: &api.Source{Parent: mockParentID}, + }, + }, nil). + Times(2), + + s.MockDriver(). + EXPECT(). + Set(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1), + + s.MockDriver(). + EXPECT(). + Enumerate(gomock.Any(), nil). + Return([]*api.Volume{ + { + Id: id, + Source: &api.Source{Parent: mockParentID}, + }, + }, nil). + Times(1), + ) + + r, err := c.CreateVolume(context.Background(), req) + assert.Nil(t, err) + assert.NotNil(t, r) + volumeInfo := r.GetVolume() + + assert.Equal(t, id, volumeInfo.GetVolumeId()) + assert.NotEqual(t, "true", volumeInfo.GetVolumeContext()[api.SpecSharedv4]) + assert.Equal(t, mockParentID, volumeInfo.GetVolumeContext()[api.SpecParent]) +} + +func TestControllerCreateVolumeFromSourceFADAPod(t *testing.T) { + // Create server and client connection + s := newTestServer(t) + defer s.Stop() + c := csi.NewControllerClient(s.Conn()) + s.mockClusterEnumerateNode(t, "node-1") + // Setup request + mockParentID := "parendId" + name := "myvol" + pod := "mypod" + size := int64(1234) + req := &csi.CreateVolumeRequest{ + Name: name, + VolumeCapabilities: []*csi.VolumeCapability{ + {}, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: size, + }, + VolumeContentSource: &csi.VolumeContentSource{ + Type: &csi.VolumeContentSource_Volume{ + Volume: &csi.VolumeContentSource_VolumeSource{ + VolumeId: mockParentID, + }, + }, + }, + Parameters: map[string]string{ + "testkey": "testval", + api.SpecPurePodName: pod, + }, + Secrets: map[string]string{authsecrets.SecretTokenKey: systemUserToken}, + } + + // Setup mock functions + id := "myid" + snapID := id + "-snap" + gomock.InOrder( + + // First check on parent + s.MockDriver(). + EXPECT(). + Enumerate(&api.VolumeLocator{ + VolumeIds: []string{mockParentID}, + }, nil). + Return([]*api.Volume{{Id: mockParentID}}, nil). + Times(1), + + // VolFromName (name) + s.MockDriver(). + EXPECT(). + Inspect(gomock.Any(), []string{name}). + Return(nil, fmt.Errorf("not found")). + Times(1), + + s.MockDriver(). + EXPECT(). + Enumerate(gomock.Any(), nil). + Return(nil, fmt.Errorf("not found")). + Times(1), + + //VolFromName parent + s.MockDriver(). + EXPECT(). + Inspect(gomock.Any(), gomock.Any()). + Return( + []*api.Volume{{ + Id: mockParentID, + }}, nil). + Times(1), + + // create + s.MockDriver(). + EXPECT(). + Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), &api.VolumeLocator{Name: name, VolumeLabels: map[string]string{api.SpecPurePodName: pod, "testkey": "testval"}}, gomock.Any()). + Return(snapID, nil). + Times(1), + s.MockDriver(). + EXPECT(). + Enumerate(&api.VolumeLocator{ + VolumeIds: []string{snapID}, + }, nil). + Return([]*api.Volume{ + { + Id: id, + Source: &api.Source{Parent: mockParentID}, + }, + }, nil). + Times(2), + + s.MockDriver(). + EXPECT(). + Set(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1), + + s.MockDriver(). + EXPECT(). + Enumerate(gomock.Any(), nil). + Return([]*api.Volume{ + { + Id: id, + Source: &api.Source{Parent: mockParentID}, + }, + }, nil). + Times(1), + ) + + r, err := c.CreateVolume(context.Background(), req) + assert.Nil(t, err) + assert.NotNil(t, r) + volumeInfo := r.GetVolume() + + assert.Equal(t, id, volumeInfo.GetVolumeId()) + assert.NotEqual(t, "true", volumeInfo.GetVolumeContext()[api.SpecSharedv4]) + assert.Equal(t, mockParentID, volumeInfo.GetVolumeContext()[api.SpecParent]) +} + func TestControllerCreateVolumeBlock(t *testing.T) { // Create server and client connection s := newTestServer(t)