Skip to content

Commit

Permalink
PWX-39302: Adding Inspect options to volume inspect API ( base releas…
Browse files Browse the repository at this point in the history 
…e-9.9) (#2494)

* Adding Inspect options to volume inspect API

* adding new parameter volumeConsumer

* UT test fixes

Signed-off-by: Harsh Desai <[email protected]>

* VolumeInspectOptions should not be nil while calling pxctl v i

* Fixing UTs by passing arg

Signed-off-by: root <[email protected]>

* Setting VolumeConsume=true in enumerate function

Signed-off-by: root <[email protected]>

* fixed UTs

Signed-off-by: Yashwant Joshi <[email protected]>

---------

Signed-off-by: Harsh Desai <[email protected]>
Signed-off-by: root <[email protected]>
Signed-off-by: Yashwant Joshi <[email protected]>
Co-authored-by: Harsh Desai <[email protected]>
Co-authored-by: root <[email protected]>
  • Loading branch information
@yjoshi-px @harsh-px
3 people authored Oct 5, 2024
1 parent ed814c3 commit d41ec0f
Show file tree
Hide file tree
Showing 33 changed files with 2,220 additions and 2,167 deletions.
3,736 changes: 1,874 additions & 1,862 deletions api/api.pb.go
View file

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions api/api.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,11 @@ message VolumeInspectOptions {
// Deep inspection is used to collect more information about
// the volume. Setting this value may delay the request.
bool deep = 1;

// VolumeConsumers if true will go and attempt to inspect who is using the
// volume being inspected. This can be an expensive call and should be skipped if
// you don't need volume consumers
bool volume_consumers = 2;
}

// Source is a structure that can be given to a volume
Expand Down
2 changes: 1 addition & 1 deletion api/client/volume/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (v *volumeClient) Status() [][2]string {

// Inspect specified volumes.
// Errors ErrEnoEnt may be returned.
func (v *volumeClient) Inspect(ids []string) ([]*api.Volume, error) {
func (v *volumeClient) Inspect(ids []string, opts *api.VolumeInspectOptions) ([]*api.Volume, error) {
if len(ids) == 0 {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion api/client/volume/client_volume_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestClientTLS(t *testing.T) {

clnt.SetTLS(&tls.Config{InsecureSkipVerify: true})

_, err = VolumeDriver(clnt).Inspect([]string{"12345"})
_, err = VolumeDriver(clnt).Inspect([]string{"12345"}, nil)

require.NoError(t, err)
}
8 changes: 4 additions & 4 deletions api/server/middleware_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (a *authMiddleware) setWithAuth(w http.ResponseWriter, r *http.Request, nex
if err != nil {
processErrorForVolSetResponse(req.Action, err, &resp)
} else {
v, err := d.Inspect([]string{volumeID})
v, err := d.Inspect([]string{volumeID},nil)
if err != nil {
processErrorForVolSetResponse(req.Action, err, &resp)
} else if v == nil || len(v) != 1 {
Expand Down Expand Up @@ -279,7 +279,7 @@ func (a *authMiddleware) deleteWithAuth(w http.ResponseWriter, r *http.Request,
return
}

vols, err := d.Inspect([]string{volumeID})
vols, err := d.Inspect([]string{volumeID},nil)
if err != nil || len(vols) == 0 || vols[0] == nil {
json.NewEncoder(w).Encode(volumeResponse)
return
Expand Down Expand Up @@ -338,7 +338,7 @@ func (a *authMiddleware) inspectWithAuth(w http.ResponseWriter, r *http.Request,
return
}

dk, err := d.Inspect([]string{volumeID})
dk, err := d.Inspect([]string{volumeID},nil)
if err != nil {
a.log(volumeID, fn).WithError(err).Error("Failed to inspect volume")
http.Error(w, err.Error(), http.StatusNotFound)
Expand Down Expand Up @@ -368,7 +368,7 @@ func (a *authMiddleware) enumerateWithAuth(w http.ResponseWriter, r *http.Reques
}
volumeID := volIDs[0]

vols, err := d.Inspect([]string{volumeID})
vols, err := d.Inspect([]string{volumeID},nil)
if err != nil || len(vols) == 0 || vols[0] == nil {
a.log(volumeID, fn).WithError(err).Error("Failed to get volume object")
json.NewEncoder(w).Encode(emptyVols)
Expand Down
15 changes: 15 additions & 0 deletions api/server/sdk/api/api.swagger.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/server/sdk/server_interceptors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func TestAuthorizationServerInterceptorCreateVolume(t *testing.T) {
gomock.InOrder(
s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
AnyTimes(),
s.MockDriver().
Expand Down
2 changes: 1 addition & 1 deletion api/server/sdk/volume_ops.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ func (s *VolumeServer) Inspect(
}
v = vols[0]
} else {
vols, err := s.driver(ctx).Inspect([]string{req.GetVolumeId()})
vols, err := s.driver(ctx).Inspect([]string{req.GetVolumeId()}, req.GetOptions())
if err == kvdb.ErrNotFound || (err == nil && len(vols) == 0) {
return nil, status.Errorf(
codes.NotFound,
Expand Down
74 changes: 46 additions & 28 deletions api/server/sdk/volume_ops_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,22 @@ func TestSdkVolumeCreateCheckIdempotencyWaitForRemoved(t *testing.T) {
gomock.InOrder(
s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return([]*api.Volume{vol}, nil),

s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return([]*api.Volume{vol}, nil),

s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return([]*api.Volume{vol}, nil),

s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("MOCK ERROR")),

s.MockDriver().
Expand Down Expand Up @@ -150,8 +150,8 @@ func TestSdkVolumeCreateCheckIdempotencyWaitForReady(t *testing.T) {
// 1 for waiting but getting that the volume is up
s.MockDriver().
EXPECT().
Inspect([]string{name}).
Do(func([]string) {
Inspect([]string{name}, nil).
Do(func([]string, ...interface{}) {
count++
if count == 4 {
vol.Status = api.VolumeStatus_VOLUME_STATUS_UP
Expand Down Expand Up @@ -187,7 +187,7 @@ func TestSdkVolumeCreateCheckIdempotency(t *testing.T) {
id := "myid"
s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return([]*api.Volume{
{
Id: id,
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestSdkVolumeCreate(t *testing.T) {
gomock.InOrder(
s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand Down Expand Up @@ -295,7 +295,7 @@ func TestSdkVolumeClone(t *testing.T) {

s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand All @@ -307,7 +307,7 @@ func TestSdkVolumeClone(t *testing.T) {

s.MockDriver().
EXPECT().
Inspect([]string{parentid}).
Inspect([]string{parentid}, nil).
Return([]*api.Volume{parentVol}, nil).
Times(1),

Expand Down Expand Up @@ -430,6 +430,7 @@ func TestSdkVolumeInspect(t *testing.T) {
id := "myid"
req := &api.SdkVolumeInspectRequest{
VolumeId: id,
Options: nil,
}

s.MockDriver().
Expand All @@ -453,10 +454,16 @@ func TestSdkVolumeInspect(t *testing.T) {
assert.NotNil(t, r.GetVolume())
assert.Equal(t, r.GetVolume().GetId(), id)

req.Options = &api.VolumeInspectOptions{Deep: true}
req.Options = &api.VolumeInspectOptions{
Deep: true,
VolumeConsumers: true,
}
s.MockDriver().
EXPECT().
Inspect([]string{id}).
Inspect([]string{id}, &api.VolumeInspectOptions{
Deep: true,
VolumeConsumers: true,
}).
Return([]*api.Volume{
{
Id: id,
Expand Down Expand Up @@ -505,12 +512,17 @@ func TestSdkVolumeInspectKeyNotFound(t *testing.T) {
// Returns key not found
s.MockDriver().
EXPECT().
Inspect([]string{id}).
Inspect([]string{id}, &api.VolumeInspectOptions{
Deep: true,
VolumeConsumers: true,
}).
Return([]*api.Volume{}, kvdb.ErrNotFound).
Times(1)

// Get info
req.Options = &api.VolumeInspectOptions{Deep: true}
req.Options = &api.VolumeInspectOptions{Deep: true,
VolumeConsumers: true,
}
_, err = c.Inspect(context.Background(), req)
assert.Error(t, err)

Expand All @@ -522,7 +534,10 @@ func TestSdkVolumeInspectKeyNotFound(t *testing.T) {
// Key not found, err is nil but empty list returned
s.MockDriver().
EXPECT().
Inspect([]string{id}).
Inspect([]string{id}, &api.VolumeInspectOptions{
Deep: true,
VolumeConsumers: true,
}).
Return([]*api.Volume{}, nil).
Times(1)

Expand All @@ -539,7 +554,10 @@ func TestSdkVolumeInspectKeyNotFound(t *testing.T) {
expectedErr := fmt.Errorf("WEIRD ERROR")
s.MockDriver().
EXPECT().
Inspect([]string{id}).
Inspect([]string{id}, &api.VolumeInspectOptions{
Deep: true,
VolumeConsumers: true,
}).
Return([]*api.Volume{}, expectedErr).
Times(1)

Expand Down Expand Up @@ -1069,7 +1087,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand All @@ -1081,7 +1099,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{parentid}).
Inspect([]string{parentid}, nil).
Return([]*api.Volume{parentVol}, nil).
Times(1),

Expand Down Expand Up @@ -1116,7 +1134,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand All @@ -1128,7 +1146,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{parentid}).
Inspect([]string{parentid}, nil).
Return([]*api.Volume{parentVol}, nil).
Times(1),

Expand Down Expand Up @@ -1187,7 +1205,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand All @@ -1199,7 +1217,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{parentid}).
Inspect([]string{parentid}, nil).
Return([]*api.Volume{parentVol}, nil).
Times(1),

Expand Down Expand Up @@ -1243,7 +1261,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand All @@ -1255,7 +1273,7 @@ func TestSdkCloneOwnership(t *testing.T) {

mv.
EXPECT().
Inspect([]string{parentid}).
Inspect([]string{parentid}, nil).
Return([]*api.Volume{parentVol}, nil).
Times(1),

Expand Down Expand Up @@ -1386,7 +1404,7 @@ func TestSdkVolumeCreateEnforced(t *testing.T) {
gomock.InOrder(
s.MockDriver().
EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand Down Expand Up @@ -1562,7 +1580,7 @@ func TestSdkVolumeCreateDefaultPolicyOwnership(t *testing.T) {
id := "myid"
gomock.InOrder(
mv.EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand Down Expand Up @@ -1615,7 +1633,7 @@ func TestSdkVolumeCreateDefaultPolicyOwnership(t *testing.T) {
// Create response
gomock.InOrder(
mv.EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand Down Expand Up @@ -1763,7 +1781,7 @@ func TestSdkVolumeUpdatePolicyOwnership(t *testing.T) {
id := "myid"
gomock.InOrder(
mv.EXPECT().
Inspect([]string{name}).
Inspect([]string{name}, nil).
Return(nil, fmt.Errorf("not found")).
Times(1),

Expand Down
3 changes: 2 additions & 1 deletion api/server/volume.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,8 @@ func (vd *volAPI) enumerate(w http.ResponseWriter, r *http.Request) {
resp, err := volumes.Inspect(ctx, &api.SdkVolumeInspectRequest{
VolumeId: string(s),
Options: &api.VolumeInspectOptions{
Deep: true,
Deep: true,
VolumeConsumers: true,
},
})
if err == nil {
Expand Down
Loading

0 comments on commit d41ec0f

Please sign in to comment.