Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PWX-38585: Removes extra pod name label #2464

Merged
merged 2 commits into from
Aug 21, 2024
Merged

PWX-38585: Removes extra pod name label #2464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a54ebad
PWX-38585: Removes extra pod name label
Pure-AdamuKaapan Aug 20, 2024
f678c15
PWX-38585: PR feedback
Pure-AdamuKaapan Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/server/docker.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading