Skip to content

Commit

Permalink
PWX-38585: Removes extra pod name label (#2464)
Browse files Browse the repository at this point in the history 
* PWX-38585: Removes extra pod name label

Signed-off-by: Adam Krpan <[email protected]>

* PWX-38585: PR feedback

Signed-off-by: Adam Krpan <[email protected]>

---------

Signed-off-by: Adam Krpan <[email protected]>
  • Loading branch information
@Pure-AdamuKaapan
Pure-AdamuKaapan authored Aug 21, 2024
1 parent e514fde commit e8c9967
Show file tree
Hide file tree
Showing 4 changed files with 241 additions and 20 deletions.
1 change: 1 addition & 0 deletions api/server/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 1 addition & 13 deletions api/server/sdk/volume_ops.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions csi/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,14 +614,14 @@ 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()
}
cloneResp, err := volumes.Clone(ctx, &api.SdkVolumeCloneRequest{
Name: req.GetName(),
ParentId: source.Parent,
AdditionalLabels: clonedMetadata,
AdditionalLabels: labels,
})
if err != nil {
return nil, err
Expand Down
240 changes: 236 additions & 4 deletions csi/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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: map[string]string{api.SpecParent: parent}}, false).
Return("", fmt.Errorf("snapshoterr")).
Times(1),
)
Expand Down Expand Up @@ -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().
Expand Down Expand Up @@ -2164,7 +2163,8 @@ func TestControllerCreateVolumeSnapshotThroughParameters(t *testing.T) {
s.MockDriver().
EXPECT().
Snapshot(gomock.Any(), mockParentID, false, &api.VolumeLocator{
Name: name,
Name: name,
VolumeLabels: map[string]string{api.SpecParent: mockParentID},
},
false).
Return(id, nil).
Expand Down Expand Up @@ -2246,6 +2246,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)
Expand Down

0 comments on commit e8c9967

Please sign in to comment.