Repository: kubernetes-csi/external-snapshotter
Pull request #1193 (2024-11-11 14:15:34)

Pass context on in GroupSnapshot sync functions
**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug

kind cleanup

> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
> /kind flake

**What this PR does / why we need it**:
Address a TODO comment.

**Which issue(s) this PR fixes**:
Updates #1171

**Special notes for your reviewer**:
TODO note added by @leonardoce.

The sync operations are triggered from a work queue, so there are no real contexts related to the original request. A single context is now created and passed on from the work queue function.

**Does this PR introduce a user-facing change?**:
```release-note
NONE
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-csi/external-snapshotter/pull/1193#" title="Author self-approved">nixpanic</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [msau42](https://github.com/msau42) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-csi%2Fexternal-snapshotter).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-csi/external-snapshotter/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["msau42"]} -->
leonardoce: /assign

Patch:
diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go
index de92d9509..08b0f3a44 100644
--- a/pkg/common-controller/groupsnapshot_controller_helper.go
+++ b/pkg/common-controller/groupsnapshot_controller_helper.go
@@ -234,7 +234,7 @@ func (ctrl *csiSnapshotCommonController) getClaimsFromVolumeGroupSnapshot(groupS
 
 // updateGroupSnapshot runs in worker thread and handles "groupsnapshot added",
 // "groupsnapshot updated" and "periodic sync" events.
-func (ctrl *csiSnapshotCommonController) updateGroupSnapshot(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
+func (ctrl *csiSnapshotCommonController) updateGroupSnapshot(ctx context.Context, groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
 	// Store the new group snapshot version in the cache and do not process it
 	// if this is an old version.
 	klog.V(5).Infof("updateGroupSnapshot %q", utils.GroupSnapshotKey(groupSnapshot))
@@ -246,7 +246,7 @@ func (ctrl *csiSnapshotCommonController) updateGroupSnapshot(groupSnapshot *crdv
 		return nil
 	}
 
-	err = ctrl.syncGroupSnapshot(groupSnapshot)
+	err = ctrl.syncGroupSnapshot(ctx, groupSnapshot)
 	if err != nil {
 		if errors.IsConflict(err) {
 			// Version conflict error happens quite often and the controller
@@ -294,7 +294,7 @@ func (ctrl *csiSnapshotCommonController) deleteGroupSnapshot(groupSnapshot *crdv
 // a group snapshot is created, updated or periodically synced. We do not
 // differentiate between these events.
 // For easier readability, it is split into syncUnreadyGroupSnapshot and syncReadyGroupSnapshot
-func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
+func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(ctx context.Context, groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
 	klog.V(5).Infof("synchronizing VolumeGroupSnapshot[%s]", utils.GroupSnapshotKey(groupSnapshot))
 
 	klog.V(5).Infof("syncGroupSnapshot [%s]: check if we should remove finalizer on group snapshot PVC source and remove it if we can", utils.GroupSnapshotKey(groupSnapshot))
@@ -326,7 +326,7 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(groupSnapshot *crdv1a
 	// 3) groupSnapshot.Status.IsBoundVolumeGroupSnapshotContentNameSet is not set
 	// 4) groupSnapshot.Status.IsVolumeSnapshotRefListSet is not set
 	if !utils.IsGroupSnapshotReady(groupSnapshot) || !utils.IsBoundVolumeGroupSnapshotContentNameSet(groupSnapshot) || !utils.IsPVCVolumeSnapshotRefListSet(groupSnapshot) {
-		return ctrl.syncUnreadyGroupSnapshot(groupSnapshot)
+		return ctrl.syncUnreadyGroupSnapshot(ctx, groupSnapshot)
 	}
 	return ctrl.syncReadyGroupSnapshot(groupSnapshot)
 }
@@ -383,7 +383,7 @@ func (ctrl *csiSnapshotCommonController) getGroupSnapshotContentFromStore(conten
 
 // syncUnreadyGroupSnapshot is the main controller method to decide what to do
 // with a group snapshot which is not set to ready.
-func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
+func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(ctx context.Context, groupSnapshot *crdv1alpha1.VolumeGroupSnapshot) error {
 	uniqueGroupSnapshotName := utils.GroupSnapshotKey(groupSnapshot)
 	klog.V(5).Infof("syncUnreadyGroupSnapshot %s", uniqueGroupSnapshotName)
 	driverName, err := ctrl.getGroupSnapshotDriverName(groupSnapshot)
@@ -460,8 +460,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(groupSnapshot
 			return fmt.Errorf("VolumeGroupSnapshotHandle should not be set in the group snapshot content for dynamic provisioning for group snapshot %s", uniqueGroupSnapshotName)
 		}
 
-		// TODO(leonardoce): introduce a current context in this function
-		newGroupSnapshotContentObj, err := ctrl.createSnapshotsForGroupSnapshotContent(context.TODO(), contentObj, groupSnapshot)
+		newGroupSnapshotContentObj, err := ctrl.createSnapshotsForGroupSnapshotContent(ctx, contentObj, groupSnapshot)
 		if err != nil {
 			klog.V(4).Infof("createSnapshotsForGroupSnapshotContent[%s]: failed to create snapshots and snapshotcontents for group snapshot %v: %v",
 				contentObj.Name, groupSnapshot.Name, err.Error())
diff --git a/pkg/common-controller/snapshot_controller_base.go b/pkg/common-controller/snapshot_controller_base.go
index 28ee64836..009cb8fe6 100644
--- a/pkg/common-controller/snapshot_controller_base.go
+++ b/pkg/common-controller/snapshot_controller_base.go
@@ -17,6 +17,7 @@ limitations under the License.
 package common_controller
 
 import (
+	"context"
 	"fmt"
 	"time"
 
@@ -671,7 +672,7 @@ func (ctrl *csiSnapshotCommonController) groupSnapshotWorker() {
 	}
 	defer ctrl.groupSnapshotQueue.Done(keyObj)
 
-	if err := ctrl.syncGroupSnapshotByKey(keyObj.(string)); err != nil {
+	if err := ctrl.syncGroupSnapshotByKey(context.Background(), keyObj.(string)); err != nil {
 		// Rather than wait for a full resync, re-add the key to the
 		// queue to be processed.
 		ctrl.groupSnapshotQueue.AddRateLimited(keyObj)
@@ -704,7 +705,7 @@ func (ctrl *csiSnapshotCommonController) groupSnapshotContentWorker() {
 }
 
 // syncGroupSnapshotByKey processes a VolumeGroupSnapshot request.
-func (ctrl *csiSnapshotCommonController) syncGroupSnapshotByKey(key string) error {
+func (ctrl *csiSnapshotCommonController) syncGroupSnapshotByKey(ctx context.Context, key string) error {
 	klog.V(5).Infof("syncGroupSnapshotByKey[%s]", key)
 
 	namespace, name, err := cache.SplitMetaNamespaceKey(key)
@@ -726,7 +727,7 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshotByKey(key string) erro
 				klog.V(5).Infof("GroupSnapshot %q is being deleted. GroupSnapshotClass has already been removed", key)
 			}
 			klog.V(5).Infof("Updating group snapshot %q", key)
-			return ctrl.updateGroupSnapshot(newGroupSnapshot)
+			return ctrl.updateGroupSnapshot(ctx, newGroupSnapshot)
 		}
 		return err
 	}


Test patch:
diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go
index 9627a3fb1..63f337c0a 100644
--- a/pkg/common-controller/framework_test.go
+++ b/pkg/common-controller/framework_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package common_controller
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -1751,7 +1752,7 @@ func testSyncSnapshot(ctrl *csiSnapshotCommonController, reactor *snapshotReacto
 }
 
 func testSyncGroupSnapshot(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {
-	return ctrl.syncGroupSnapshot(test.initialGroupSnapshots[0])
+	return ctrl.syncGroupSnapshot(context.TODO(), test.initialGroupSnapshots[0])
 }
 
 func testSyncSnapshotError(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {