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

add ctx to Set function #2405

Merged
merged 6 commits into from
Jan 24, 2024
Merged
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 .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ services:
- docker
language: go
go:
- 1.18.x
- 1.19.x
install:
- go install github.com/vbatts/[email protected]
- curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/amd64/kubectl
Expand Down
3 changes: 1 addition & 2 deletions api/client/volume/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,7 @@ func (v *volumeClient) Unmount(ctx context.Context, volumeID string, mountPath s
}

// Update volume
func (v *volumeClient) Set(volumeID string, locator *api.VolumeLocator,
spec *api.VolumeSpec) error {
func (v *volumeClient) Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
return v.doVolumeSet(correlation.TODO(),
volumeID,
&api.VolumeSetRequest{
Expand Down
2 changes: 1 addition & 1 deletion api/server/middleware_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (a *authMiddleware) setWithAuth(w http.ResponseWriter, r *http.Request, nex

if req.Spec != nil && req.Spec.Size > 0 {
isOpDone = true
err = d.Set(volumeID, req.Locator, req.Spec)
err = d.Set(ctx, volumeID, req.Locator, req.Spec)
}

for err == nil && req.Action != nil {
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 @@ -729,7 +729,7 @@ func (s *VolumeServer) Update(
maskUnModified(updatedSpec, req.GetSpec())

// Send to driver
if err := s.driver(ctx).Set(req.GetVolumeId(), locator, updatedSpec); err != nil {
if err := s.driver(ctx).Set(ctx, req.GetVolumeId(), locator, updatedSpec); err != nil {
return nil, status.Errorf(codes.Internal, "Failed to update volume: %v", err)
}

Expand Down
12 changes: 6 additions & 6 deletions api/server/sdk/volume_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ func TestSdkVolumeUpdate(t *testing.T) {
AnyTimes()
s.MockDriver().
EXPECT().
Set(id, &api.VolumeLocator{VolumeLabels: newlabels}, &api.VolumeSpec{SnapshotInterval: math.MaxUint32}).
Set(gomock.Any(), id, &api.VolumeLocator{VolumeLabels: newlabels}, &api.VolumeSpec{SnapshotInterval: math.MaxUint32}).
Return(nil).
Times(1)

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

s.MockDriver().
EXPECT().
Set(id, nil, &api.VolumeSpec{Size: 1234, SnapshotInterval: math.MaxUint32}).
Set(gomock.Any(), id, nil, &api.VolumeSpec{Size: 1234, SnapshotInterval: math.MaxUint32}).
Return(nil).
Times(1)
_, err = c.Update(context.Background(), req)
Expand All @@ -687,7 +687,7 @@ func TestSdkVolumeUpdate(t *testing.T) {

s.MockDriver().
EXPECT().
Set(
Set(gomock.Any(),
id,
&api.VolumeLocator{VolumeLabels: newlabels},
&api.VolumeSpec{Size: 1234, SnapshotInterval: math.MaxUint32},
Expand Down Expand Up @@ -1156,7 +1156,7 @@ func TestSdkCloneOwnership(t *testing.T) {
Times(1),
mv.
EXPECT().
Set(id, nil, &api.VolumeSpec{
Set(gomock.Any(), id, nil, &api.VolumeSpec{
Size: 1234,
Ownership: &api.Ownership{
Owner: user2,
Expand Down Expand Up @@ -1283,7 +1283,7 @@ func TestSdkCloneOwnership(t *testing.T) {
Times(1),
mv.
EXPECT().
Set(id, nil, &api.VolumeSpec{
Set(gomock.Any(), id, nil, &api.VolumeSpec{
Size: 1234,
Ownership: &api.Ownership{
Owner: user2,
Expand Down Expand Up @@ -1803,7 +1803,7 @@ func TestSdkVolumeUpdatePolicyOwnership(t *testing.T) {
AnyTimes()
mv.
EXPECT().
Set(id, nil, volPolSpec).
Set(gomock.Any(), id, nil, volPolSpec).
Return(nil).
Times(1)

Expand Down
7 changes: 4 additions & 3 deletions api/server/sdk/volume_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -18,6 +18,7 @@ package sdk

import (
"context"
"github.com/golang/mock/gomock"
"math"
"testing"

Expand Down Expand Up @@ -311,7 +312,7 @@ func TestSdkVolumeSnapshotScheduleUpdate(t *testing.T) {
Times(1)
s.MockDriver().
EXPECT().
Set(volid, nil, &api.VolumeSpec{
Set(gomock.Any(), volid, nil, &api.VolumeSpec{
SnapshotSchedule: "policy=mypolicy",
SnapshotInterval: math.MaxUint32,
}).
Expand Down Expand Up @@ -348,7 +349,7 @@ func TestSdkVolumeSnapshotScheduleUpdateDelete(t *testing.T) {
AnyTimes()
s.MockDriver().
EXPECT().
Set(volid, nil, &api.VolumeSpec{
Set(gomock.Any(), volid, nil, &api.VolumeSpec{
SnapshotSchedule: "",
SnapshotInterval: math.MaxUint32,
}).
Expand Down
22 changes: 10 additions & 12 deletions api/server/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestVolumeNoAuth(t *testing.T) {

newspec := req.GetSpec()
newspec.Size = newsize
resp := driverclient.Set(id, req.GetLocator(), newspec)
resp := driverclient.Set(context.TODO(), id, req.GetLocator(), newspec)
assert.Nil(t, resp)

// INSPECT
Expand Down Expand Up @@ -793,7 +793,7 @@ func TestVolumeSetSuccess(t *testing.T) {
Spec: &api.VolumeSpec{Size: newsize},
}

res := driverclient.Set(id, req.GetLocator(), req2.GetSpec())
res := driverclient.Set(context.TODO(), id, req.GetLocator(), req2.GetSpec())
assert.Nil(t, res)

// Assert volume information is correct
Expand All @@ -808,13 +808,11 @@ func TestVolumeSetSuccess(t *testing.T) {
assert.Equal(t, newsize, r.GetVolume().GetSpec().GetSize())

// Send HA request
res = driverclient.Set(id,
nil,
&api.VolumeSpec{
HaLevel: 2,
ReplicaSet: &api.ReplicaSet{Nodes: []string{}},
SnapshotInterval: math.MaxUint32,
})
res = driverclient.Set(context.TODO(), id, nil, &api.VolumeSpec{
HaLevel: 2,
ReplicaSet: &api.ReplicaSet{Nodes: []string{}},
SnapshotInterval: math.MaxUint32,
})
assert.Nil(t, res, fmt.Sprintf("Error: %v", res))

// Assert volume information is correct
Expand Down Expand Up @@ -884,7 +882,7 @@ func TestVolumeSetFailed(t *testing.T) {
Spec: &api.VolumeSpec{Size: size, HaLevel: halevel},
}
// Cannot get this to fail....
err = driverclient.Set("doesnotexist", req2.GetLocator(), req2.GetSpec())
err = driverclient.Set(context.TODO(), "doesnotexist", req2.GetLocator(), req2.GetSpec())
// assert.NotNil(t, err)

// Assert volume information is correct
Expand Down Expand Up @@ -929,7 +927,7 @@ func TestMiddlewareVolumeSetSizeSuccess(t *testing.T) {

// Not setting mock secrets

err = driverclient.Set(id, nil, req.GetSpec())
err = driverclient.Set(context.TODO(), id, nil, req.GetSpec())
assert.NoError(t, err, "Unexpected error on Set")

// Assert volume information is correct
Expand Down Expand Up @@ -976,7 +974,7 @@ func TestMiddlewareVolumeSetFailure(t *testing.T) {

// Not setting mock secrets

err = driverclient.Set(id, &api.VolumeLocator{Name: "myvol"}, req.GetSpec())
err = driverclient.Set(context.TODO(), id, &api.VolumeLocator{Name: "myvol"}, req.GetSpec())
assert.Error(t, err, "Unexpected error on Set")

}
Expand Down
6 changes: 3 additions & 3 deletions csi/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1955,7 +1955,7 @@ func TestControllerCreateVolumeFromSnapshot(t *testing.T) {

s.MockDriver().
EXPECT().
Set(gomock.Any(), gomock.Any(), gomock.Any()).
Set(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil).
Times(1),

Expand Down Expand Up @@ -2092,7 +2092,7 @@ func TestControllerCreateVolumeSnapshotThroughParameters(t *testing.T) {
Times(1),
s.MockDriver().
EXPECT().
Set(gomock.Any(), gomock.Any(), gomock.Any()).
Set(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil).
Times(1),
// final inspect
Expand Down Expand Up @@ -2750,7 +2750,7 @@ func TestControllerExpandVolume(t *testing.T) {
AnyTimes(),
s.MockDriver().
EXPECT().
Set(gomock.Any(), gomock.Any(), &api.VolumeSpec{
Set(gomock.Any(), gomock.Any(), gomock.Any(), &api.VolumeSpec{
Size: 46 * units.GiB, // Round up from 45.5 to 46
SnapshotInterval: math.MaxUint32,
}).
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ require (
github.com/libopenstorage/gossip v0.0.0-20220309192431-44c895e0923e
github.com/libopenstorage/secrets v0.0.0-20200207034622-cdb443738c67
github.com/libopenstorage/systemutils v0.0.0-20160208220149-44ac83be3ce1
github.com/moby/sys/mount v0.2.0
github.com/moby/sys/mountinfo v0.4.0
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826
github.com/onsi/ginkgo v1.16.5
Expand Down Expand Up @@ -105,6 +104,7 @@ require (
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/mapstructure v1.3.3 // indirect
github.com/moby/locker v1.0.1 // indirect
github.com/moby/sys/mount v0.2.0 // indirect
github.com/moby/sys/symlink v0.1.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
Expand Down
4 changes: 2 additions & 2 deletions pkg/sanity/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ var _ = Describe("Volume [Volume Tests]", func() {
},
}

err = volumedriver.Set(volumeID, set.GetLocator(), set.GetSpec())
err = volumedriver.Set(context.TODO(), volumeID, set.GetLocator(), set.GetSpec())
Expect(err).NotTo(HaveOccurred())

By("Inspecting the volume for new updates")
Expand Down Expand Up @@ -679,7 +679,7 @@ var _ = Describe("Volume [Volume Tests]", func() {
},
}

err = volumedriver.Set(volumeID, set.Locator, set.Spec)
err = volumedriver.Set(context.TODO(), volumeID, set.Locator, set.Spec)
Expect(err).NotTo(HaveOccurred())

By("Inspecting the volume for new updates")
Expand Down
2 changes: 1 addition & 1 deletion volume/drivers/buse/buse.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func (d *driver) SnapshotGroup(groupID string, labels map[string]string, volumeI
return nil, volume.ErrNotSupported
}

func (d *driver) Set(volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
func (d *driver) Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
if spec != nil {
return volume.ErrNotSupported
}
Expand Down
2 changes: 1 addition & 1 deletion volume/drivers/fake/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func (d *driver) CloudMigrateStatus(request *api.CloudMigrateStatusRequest) (*ap
}, nil
}

func (d *driver) Set(volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
func (d *driver) Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
v, err := d.GetVol(volumeID)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion volume/drivers/fake/fake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ func TestFakeSet(t *testing.T) {
assert.NotEmpty(t, volid)

// Set values
err = d.Set(volid, &api.VolumeLocator{
err = d.Set(context.TODO(), volid, &api.VolumeLocator{
Name: "newname",
VolumeLabels: map[string]string{
"hello": "world",
Expand Down
2 changes: 1 addition & 1 deletion volume/drivers/fuse/volume_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (v *volumeDriver) Unmount(ctx context.Context, volumeID string, mountpath s
return v.UpdateVol(volume)
}

func (v *volumeDriver) Set(volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
func (v *volumeDriver) Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
return volume.ErrNotSupported

}
Expand Down
8 changes: 4 additions & 4 deletions volume/drivers/mock/driver.mock.go

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

2 changes: 1 addition & 1 deletion volume/drivers/nfs/nfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ func (d *driver) Detach(ctx context.Context, volumeID string, options map[string
return nil
}

func (d *driver) Set(volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
func (d *driver) Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
if spec != nil {
return volume.ErrNotSupported
}
Expand Down
2 changes: 1 addition & 1 deletion volume/drivers/test/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func set(t *testing.T, ctx *Context) {
require.Equal(t, vols[0].Id, ctx.volID, "Expect volID %v actual %v", ctx.volID, vols[0].Id)

vols[0].Locator.VolumeLabels["UpdateTest"] = "Success"
err = ctx.Set(ctx.volID, vols[0].Locator, nil)
err = ctx.Set(correlation.TODO(), ctx.volID, vols[0].Locator, nil)
if err != volume.ErrNotSupported {
require.NoError(t, err, "Failed in Update")
vols, err = ctx.Inspect(correlation.TODO(), []string{ctx.volID})
Expand Down
2 changes: 1 addition & 1 deletion volume/drivers/vfs/vfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (d *driver) Unmount(ctx context.Context, volumeID string, mountpath string,
return d.UpdateVol(v)
}

func (d *driver) Set(volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
func (d *driver) Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error {
if spec != nil {
return volume.ErrNotSupported
}
Expand Down
2 changes: 1 addition & 1 deletion volume/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ type ProtoDriver interface {
Unmount(ctx context.Context, volumeID string, mountPath string, options map[string]string) error
// Update not all fields of the spec are supported, ErrNotSupported will be thrown for unsupported
// updates.
Set(volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error
Set(ctx context.Context, volumeID string, locator *api.VolumeLocator, spec *api.VolumeSpec) error
// Status returns a set of key-value pairs which give low
// level diagnostic status about this driver.
Status() [][2]string
Expand Down
Loading