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.

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-39302 Add inspect options to volume inspect API #2490

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading