Skip to content

Commit

Permalink
PWX-39302 Add inspect options to volume inspect API
Browse files Browse the repository at this point in the history
Allow skipping fetch of volume consumers by adding a flag in options

Signed-off-by: Harsh Desai <[email protected]>
  • Loading branch information
harsh-px committed Sep 27, 2024
1 parent 2478442 commit 140bec1
Show file tree
Hide file tree
Showing 30 changed files with 5,807 additions and 5,772 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ test-sdk: install-sdk-test launch-sdk

# TODO: Remove GODEBUG and fix test certs
test: packr
GODEBUG=x509ignoreCN=0 go test -tags "$(TAGS)" $(TESTFLAGS) $(PKGS)
GODEBUG=x509ignoreCN=0 go test -tags "$(TAGS)" -v $(TESTFLAGS) $(PKGS)

docs: $(GOPATH)/bin/gomock $(GOPATH)/bin/swagger $(GOPATH)/bin/mockgen
go generate ./cmd/osd/main.go
Expand Down
11,285 changes: 5,649 additions & 5,636 deletions api/api.pb.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions api/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,10 @@ 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
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(ctx context.Context, volumeIDs []string) ([]*api.Volume, error) {
func (v *volumeClient) Inspect(ctx context.Context, volumeIDs []string, opts *api.VolumeInspectOptions) ([]*api.Volume, error) {
if len(volumeIDs) == 0 {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion api/client/volume/client_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestClientTLS(t *testing.T) {

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

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

require.NoError(t, err)
}
8 changes: 4 additions & 4 deletions api/server/middleware_auth.go
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(correlation.TODO(), []string{volumeID})
v, err := d.Inspect(correlation.TODO(), []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(correlation.TODO(), []string{volumeID})
vols, err := d.Inspect(correlation.TODO(), []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(correlation.TODO(), []string{volumeID})
dk, err := d.Inspect(correlation.TODO(), []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(correlation.TODO(), []string{volumeID})
vols, err := d.Inspect(correlation.TODO(), []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
13 changes: 13 additions & 0 deletions api/server/sdk/api/api.swagger.json

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
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(gomock.Any(), []string{name}).
Inspect(gomock.Any(), []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
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ func (s *VolumeServer) Inspect(
}
v = vols[0]
} else {
vols, err := s.driver(ctx).Inspect(correlation.TODO(), []string{req.GetVolumeId()})
vols, err := s.driver(ctx).Inspect(correlation.TODO(), []string{req.GetVolumeId()}, req.GetOptions())
if err == kvdb.ErrNotFound || (err == nil && len(vols) == 0) {
return nil, status.Errorf(
codes.NotFound,
Expand Down
52 changes: 26 additions & 26 deletions api/server/sdk/volume_ops_test.go
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(gomock.Any(), []string{name}).
Inspect(gomock.Any(), []string{name}, nil).
Return([]*api.Volume{vol}, nil),

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

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

s.MockDriver().
EXPECT().
Inspect(gomock.Any(), []string{name}).
Inspect(gomock.Any(), []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(gomock.Any(), []string{name}).
Do(func(context.Context, []string) {
Inspect(gomock.Any(), []string{name}, nil).
Do(func(context.Context, []string, *api.VolumeInspectOptions) {
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(gomock.Any(), []string{name}).
Inspect(gomock.Any(), []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(gomock.Any(), []string{name}).
Inspect(gomock.Any(), []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(gomock.Any(), []string{name}).
Inspect(gomock.Any(), []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(gomock.Any(), []string{parentid}).
Inspect(gomock.Any(), []string{parentid}, nil).
Return([]*api.Volume{parentVol}, nil).
Times(1),

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Expand Down
10 changes: 5 additions & 5 deletions api/server/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func TestVolumeNoAuth(t *testing.T) {
assert.Nil(t, resp)

// INSPECT
res, err := driverclient.Inspect(context.TODO(), []string{id})
res, err := driverclient.Inspect(context.TODO(), []string{id}, nil)
assert.Nil(t, err)
assert.NotNil(t, res)
assert.NotEmpty(t, res)
Expand Down Expand Up @@ -575,7 +575,7 @@ func TestVolumeInspectSuccess(t *testing.T) {
assert.Nil(t, err)
assert.NotEmpty(t, id)

res, err := driverclient.Inspect(context.TODO(), []string{id})
res, err := driverclient.Inspect(context.TODO(), []string{id}, nil)
assert.Nil(t, err)
assert.NotNil(t, res)
assert.NotEmpty(t, res)
Expand Down Expand Up @@ -632,7 +632,7 @@ func TestVolumeInspectFailed(t *testing.T) {
assert.Nil(t, err)
assert.NotEmpty(t, id)

res, err := driverclient.Inspect(context.TODO(), []string{"myid"})
res, err := driverclient.Inspect(context.TODO(), []string{"myid"}, nil)
assert.Nil(t, err)
assert.Equal(t, len(res), 0)

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

// Confirm that the inspect on secret error returns to the client the correct object,
// which should be an empty list
ret, err := driverclient.Inspect(context.TODO(), []string{id})
ret, err := driverclient.Inspect(context.TODO(), []string{id}, nil)
assert.Nil(t, err)
assert.NotNil(t, ret)
assert.Empty(t, ret)
Expand Down Expand Up @@ -2797,7 +2797,7 @@ func TestStorkVolumeInspect(t *testing.T) {
err = driverclient.Delete(context.TODO(), id)
assert.Nil(t, err)

vols, err := driverclient.Inspect(context.TODO(), []string{id})
vols, err := driverclient.Inspect(context.TODO(), []string{id}, nil)
assert.Equal(t, len(vols), 0)
assert.Nil(t, err)
/*
Expand Down
Loading

0 comments on commit 140bec1

Please sign in to comment.