PWX-35577: correlation tracing for Snapshot function (#2407)

libopenstorage · Jan 26, 2024 · e65dddd · e65dddd
1 parent 548d38f
commit e65dddd
Show file tree

Hide file tree

Showing 17 changed files with 36 additions and 42 deletions.
diff --git a/api/client/volume/client.go b/api/client/volume/client.go
@@ -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,

diff --git a/api/server/sdk/volume_ops.go b/api/server/sdk/volume_ops.go
@@ -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 {

diff --git a/api/server/sdk/volume_ops_test.go b/api/server/sdk/volume_ops_test.go
@@ -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),
 
@@ -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),
 
@@ -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),
 
@@ -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),
 
@@ -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),
 

diff --git a/api/server/sdk/volume_snapshot.go b/api/server/sdk/volume_snapshot.go
@@ -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)

diff --git a/api/server/sdk/volume_snapshot_test.go b/api/server/sdk/volume_snapshot_test.go
@@ -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).

diff --git a/api/server/volume_test.go b/api/server/volume_test.go
@@ -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{
@@ -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{
@@ -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)
@@ -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
@@ -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

diff --git a/cli/volumes.go b/cli/volumes.go
@@ -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

diff --git a/csi/controller_test.go b/csi/controller_test.go
@@ -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),
 	)
@@ -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().
@@ -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).
@@ -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),
 

diff --git a/pkg/sanity/snapshot.go b/pkg/sanity/snapshot.go
@@ -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()))
 
@@ -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()))
 
@@ -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()))
 

diff --git a/volume/drivers/btrfs/btrfs.go b/volume/drivers/btrfs/btrfs.go
@@ -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
 	}
@@ -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

diff --git a/volume/drivers/buse/buse.go b/volume/drivers/buse/buse.go
@@ -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)

diff --git a/volume/drivers/fake/fake.go b/volume/drivers/fake/fake.go
@@ -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")

diff --git a/volume/drivers/mock/driver.mock.go b/volume/drivers/mock/driver.mock.go
diff --git a/volume/drivers/nfs/nfs.go b/volume/drivers/nfs/nfs.go
@@ -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 {

diff --git a/volume/drivers/test/driver.go b/volume/drivers/test/driver.go
@@ -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})
@@ -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
 }

diff --git a/volume/volume.go b/volume/volume.go
@@ -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

diff --git a/volume/volume_not_supported.go b/volume/volume_not_supported.go
@@ -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
 }