Skip to content

Commit

Permalink
PWX-35577: correlation tracing for Snapshot function (#2407)
Browse files Browse the repository at this point in the history
  • Loading branch information
alicelyy authored Jan 26, 2024
1 parent 548d38f commit e65dddd
Show file tree
Hide file tree
Showing 17 changed files with 36 additions and 42 deletions.
6 changes: 1 addition & 5 deletions api/client/volume/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,7 @@ func (v *volumeClient) Delete(ctx context.Context, volumeID string) error {
// Snap specified volume. IO to the underlying volume should be quiesced before
// calling this function.
// Errors ErrEnoEnt may be returned
func (v *volumeClient) Snapshot(volumeID string,
readonly bool,
locator *api.VolumeLocator,
noRetry bool,
) (string, error) {
func (v *volumeClient) Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
response := &api.SnapCreateResponse{}
request := &api.SnapCreateRequest{
Id: volumeID,
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 @@ -171,7 +171,7 @@ func (s *VolumeServer) create(
}

// Create a snapshot from the parent
id, err = s.driver(ctx).Snapshot(parent.GetId(), false, &api.VolumeLocator{
id, err = s.driver(ctx).Snapshot(ctx, parent.GetId(), false, &api.VolumeLocator{
Name: volName,
}, false)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions api/server/sdk/volume_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func TestSdkVolumeClone(t *testing.T) {

s.MockDriver().
EXPECT().
Snapshot(parentid, false, &api.VolumeLocator{Name: name}, false).
Snapshot(gomock.Any(), parentid, false, &api.VolumeLocator{Name: name}, false).
Return(id, nil).
Times(1),

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

mv.
EXPECT().
Snapshot(parentid, false, &api.VolumeLocator{Name: name}, false).
Snapshot(gomock.Any(), parentid, false, &api.VolumeLocator{Name: name}, false).
Return(id, nil).
Times(1),

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

mv.
EXPECT().
Snapshot(parentid, false, &api.VolumeLocator{Name: name}, false).
Snapshot(gomock.Any(), parentid, false, &api.VolumeLocator{Name: name}, false).
Return(id, nil).
Times(1),

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

mv.
EXPECT().
Snapshot(parentid, false, &api.VolumeLocator{Name: name}, false).
Snapshot(gomock.Any(), parentid, false, &api.VolumeLocator{Name: name}, false).
Return(id, nil).
Times(1),

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

mv.
EXPECT().
Snapshot(parentid, false, &api.VolumeLocator{Name: name}, false).
Snapshot(gomock.Any(), parentid, false, &api.VolumeLocator{Name: name}, false).
Return(id, nil).
Times(1),

Expand Down
2 changes: 1 addition & 1 deletion api/server/sdk/volume_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (s *VolumeServer) SnapshotCreate(
}

readonly := true
snapshotID, err := s.driver(ctx).Snapshot(req.GetVolumeId(), readonly, &api.VolumeLocator{
snapshotID, err := s.driver(ctx).Snapshot(ctx, req.GetVolumeId(), readonly, &api.VolumeLocator{
Name: req.GetName(),
VolumeLabels: req.GetLabels(),
}, false)
Expand Down
2 changes: 1 addition & 1 deletion api/server/sdk/volume_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestSdkVolumeSnapshotCreate(t *testing.T) {
Times(1)
s.MockDriver().
EXPECT().
Snapshot(req.GetVolumeId(), true, &api.VolumeLocator{
Snapshot(gomock.Any(), req.GetVolumeId(), true, &api.VolumeLocator{
Name: snapName,
}, false).
Return(snapid, nil).
Expand Down
12 changes: 6 additions & 6 deletions api/server/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ func TestVolumeSnapshotCreateSuccess(t *testing.T) {
Readonly: true,
}

_, err = driverclient.Snapshot(id, req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
_, err = driverclient.Snapshot(context.TODO(), id, req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
assert.Nil(t, err)

_, err = volumes.Delete(ctx, &api.SdkVolumeDeleteRequest{
Expand Down Expand Up @@ -529,7 +529,7 @@ func TestVolumeSnapshotCreateFailed(t *testing.T) {
Readonly: true,
}

res, _ := driverclient.Snapshot("doesnotexist", req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
res, _ := driverclient.Snapshot(context.TODO(), "doesnotexist", req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
assert.Equal(t, "", res)

_, err = volumes.Delete(ctx, &api.SdkVolumeDeleteRequest{
Expand Down Expand Up @@ -696,15 +696,15 @@ func TestVolumeSnapshotList(t *testing.T) {
Readonly: true,
}

_, err = driverclient.Snapshot(id, req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
_, err = driverclient.Snapshot(context.TODO(), id, req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
assert.Nil(t, err)

res, err := driverclient.SnapEnumerate([]string{id}, nil)
assert.Nil(t, err)
assert.NotNil(t, res)
assert.Len(t, res, 1)

_, err = driverclient.Snapshot(id, req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
_, err = driverclient.Snapshot(context.TODO(), id, req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
assert.Nil(t, err)

res, err = driverclient.SnapEnumerate([]string{id}, nil)
Expand Down Expand Up @@ -1651,7 +1651,7 @@ func TestVolumeRestoreSuccess(t *testing.T) {
Readonly: true,
}

res, err := driverclient.Snapshot(req2.GetId(), req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
res, err := driverclient.Snapshot(context.TODO(), req2.GetId(), req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
assert.Nil(t, err)

// create client
Expand Down Expand Up @@ -1712,7 +1712,7 @@ func TestVolumeRestoreFailed(t *testing.T) {
Readonly: true,
}

_, err = driverclient.Snapshot(req2.GetId(), req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
_, err = driverclient.Snapshot(context.TODO(), req2.GetId(), req2.GetReadonly(), req2.GetLocator(), req2.GetNoRetry())
assert.Nil(t, err)

// create client
Expand Down
2 changes: 1 addition & 1 deletion cli/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func (v *volDriver) snapCreate(cliContext *cli.Context) {
}
readonly := cliContext.Bool("readonly")
noRetry := cliContext.Bool("noretry")
id, err := v.volDriver.Snapshot(volumeID, readonly, locator, noRetry)
id, err := v.volDriver.Snapshot(context.TODO(), volumeID, readonly, locator, noRetry)
if err != nil {
cmdError(cliContext, fn, err)
return
Expand Down
8 changes: 4 additions & 4 deletions csi/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,7 @@ func TestControllerCreateVolumeBadSnapshot(t *testing.T) {
// Return an error from snapshot
s.MockDriver().
EXPECT().
Snapshot(parent, false, &api.VolumeLocator{Name: name}, false).
Snapshot(gomock.Any(), parent, false, &api.VolumeLocator{Name: name}, false).
Return("", fmt.Errorf("snapshoterr")).
Times(1),
)
Expand Down Expand Up @@ -1937,7 +1937,7 @@ func TestControllerCreateVolumeFromSnapshot(t *testing.T) {
// create
s.MockDriver().
EXPECT().
Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(snapID, nil).
Times(1),
s.MockDriver().
Expand Down Expand Up @@ -2044,7 +2044,7 @@ func TestControllerCreateVolumeSnapshotThroughParameters(t *testing.T) {
// create snap
s.MockDriver().
EXPECT().
Snapshot(mockParentID, false, &api.VolumeLocator{
Snapshot(gomock.Any(), mockParentID, false, &api.VolumeLocator{
Name: name,
},
false).
Expand Down Expand Up @@ -2933,7 +2933,7 @@ func TestControllerCreateSnapshot(t *testing.T) {
// snapshot
s.MockDriver().
EXPECT().
Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Snapshot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(snapId, nil).
Times(1),

Expand Down
6 changes: 3 additions & 3 deletions pkg/sanity/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ var _ = Describe("Volume [Snapshot Tests]", func() {
Name: "snapshot-of-" + volumeID,
}

snapID, err = volumedriver.Snapshot(volumeID, true, loc, false)
snapID, err = volumedriver.Snapshot(context.TODO(), volumeID, true, loc, false)
Expect(err).NotTo(HaveOccurred())
Expect(snapID).To(Not(BeNil()))

Expand Down Expand Up @@ -204,7 +204,7 @@ var _ = Describe("Volume [Snapshot Tests]", func() {
Name: "snapshot-" + strconv.Itoa(i) + "-of-" + volumeID,
}

snapID, err = volumedriver.Snapshot(volumeID, true, loc, false)
snapID, err = volumedriver.Snapshot(context.TODO(), volumeID, true, loc, false)
Expect(err).NotTo(HaveOccurred())
Expect(snapID).To(Not(BeNil()))

Expand Down Expand Up @@ -298,7 +298,7 @@ var _ = Describe("Volume [Snapshot Tests]", func() {
Name: "snapshot-of-" + volumeID,
}

snapID, err = volumedriver.Snapshot(volumeID, true, loc, false)
snapID, err = volumedriver.Snapshot(context.TODO(), volumeID, true, loc, false)
Expect(err).NotTo(HaveOccurred())
Expect(snapID).To(Not(BeNil()))

Expand Down
4 changes: 2 additions & 2 deletions volume/drivers/btrfs/btrfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,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 All @@ -160,7 +160,7 @@ func (d *driver) Set(volumeID string, locator *api.VolumeLocator, spec *api.Volu
}

// Snapshot create new subvolume from volume
func (d *driver) Snapshot(volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
func (d *driver) Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
vols, err := d.Inspect([]string{volumeID})
if err != nil {
return "", err
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 @@ -324,7 +324,7 @@ func (d *driver) Unmount(ctx context.Context, volumeID string, mountpath string,
return d.UpdateVol(v)
}

func (d *driver) Snapshot(volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
func (d *driver) Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
volIDs := make([]string, 1)
volIDs[0] = volumeID
vols, err := d.Inspect(nil, volIDs)
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 @@ -277,7 +277,7 @@ func (d *driver) Unmount(ctx context.Context, volumeID string, mountpath string,
return d.UpdateVol(v)
}

func (d *driver) Snapshot(volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
func (d *driver) Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {

if len(locator.GetName()) == 0 {
return "", fmt.Errorf("Name for snapshot must be provided")
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 @@ -674,7 +674,7 @@ func (d *driver) clone(newVolumeID, volumeID string) error {
return nil
}

func (d *driver) Snapshot(volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
func (d *driver) Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
volIDs := []string{volumeID}
vols, err := d.Inspect(nil, volIDs)
if err != nil {
Expand Down
6 changes: 2 additions & 4 deletions 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(correlation.TODO(), ctx.volID, vols[0].Locator, nil)
err = ctx.Set(context.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 Expand Up @@ -321,9 +321,7 @@ func snap(t *testing.T, ctx *Context) {
attach(t, ctx)
labels := map[string]string{"oh": "snap"}
require.NotEqual(t, ctx.volID, "", "invalid volume ID")
id, err := ctx.Snapshot(ctx.volID, false,
&api.VolumeLocator{Name: "snappy", VolumeLabels: labels},
false)
id, err := ctx.Snapshot(context.TODO(), ctx.volID, false, &api.VolumeLocator{Name: "snappy", VolumeLabels: labels}, false)
require.NoError(t, err, "Failed in creating a snapshot")
ctx.snapID = id
}
Expand Down
2 changes: 1 addition & 1 deletion volume/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ type IODriver interface {
type SnapshotDriver interface {
// Snapshot create volume snapshot.
// Errors ErrEnoEnt may be returned
Snapshot(volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error)
Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error)
// Restore restores volume to specified snapshot.
Restore(volumeID string, snapshotID string) error
// SnapshotGroup takes a snapshot of a group of volumes that can be specified with either of the following
Expand Down
2 changes: 1 addition & 1 deletion volume/volume_not_supported.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (b *blockNotSupported) Detach(ctx context.Context, volumeID string, options

type snapshotNotSupported struct{}

func (s *snapshotNotSupported) Snapshot(volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
func (s *snapshotNotSupported) Snapshot(ctx context.Context, volumeID string, readonly bool, locator *api.VolumeLocator, noRetry bool) (string, error) {
return "", ErrNotSupported
}

Expand Down

0 comments on commit e65dddd

Please sign in to comment.