controller/reclaimspacejob: skip making node req if nodeID is empty

Node reclaimspace request should not be made
when nodeID is empty indicating the pvc is
not mounted to any pod and no volumeattachment
object for that PVC was found.

Signed-off-by: Rakshith R <rar@redhat.com>

controllers/reclaimspacejob_controller.go

@@ -173,6 +173,13 @@ type targetDetails struct {
 	nodeID     string
 }
 
+// canNodeReclaimSpace returns true if nodeID is not empty,
+// indicating volume is mounted to a pod and node reclaimspace
+// request can be sent.
+func (td *targetDetails) canNodeReclaimSpace() bool {
+	return td.nodeID != ""
+}
+
 // reconcile performs time based validation, fetches required details and makes
 // grpc request for controller and node reclaim space operation.
 func (r *ReclaimSpaceJobReconciler) reconcile(
@@ -211,15 +218,26 @@ func (r *ReclaimSpaceJobReconciler) reconcile(
 		return err
 	}
 
-	nodeFound, nodeReclaimedSpace, err := r.nodeReclaimSpace(ctx, logger, target)
-	if err != nil {
-		logger.Error(err, "Failed to make node request")
-		setFailedCondition(
-			&rsJob.Status.Conditions,
-			fmt.Sprintf("Failed to make node request: %v", util.GetErrorMessage(err)),
-			rsJob.Generation)
-
-		return err
+	var (
+		nodeFound          = false
+		nodeReclaimedSpace *int64
+	)
+	// PVC may not be mounted to a pod and volumeattachment for PVC
+	// may not be found due to which nodeID will be empty.
+	// Make node reclaimspace request only if
+	// nodeID is not empty.
+	if target.canNodeReclaimSpace() {
+		nodeFound = true
+		nodeReclaimedSpace, err = r.nodeReclaimSpace(ctx, logger, target)
+		if err != nil {
+			logger.Error(err, "Failed to make node request")
+			setFailedCondition(
+				&rsJob.Status.Conditions,
+				fmt.Sprintf("Failed to make node request: %v", util.GetErrorMessage(err)),
+				rsJob.Generation)
+
+			return err
+		}
 	}
 
 	controllerFound, controllerReclaimedSpace, err := r.controllerReclaimSpace(ctx, logger, target)
@@ -369,20 +387,18 @@ func (r *ReclaimSpaceJobReconciler) controllerReclaimSpace(
 // controllerReclaimSpace makes node reclaim space request if node client is found
 // and returns amount of reclaimed space.
 // This function returns
-// - boolean to indicate client was found or not
 // - pointer to int64 indicating amount of reclaimed space, it is nil if not available
 // - error
 func (r *ReclaimSpaceJobReconciler) nodeReclaimSpace(
 	ctx context.Context,
 	logger *logr.Logger,
-	target *targetDetails) (bool, *int64, error) {
+	target *targetDetails) (*int64, error) {
 	clientName, nodeClient := r.getRSClientWithCap(
 		target.driverName,
 		target.nodeID,
 		identity.Capability_ReclaimSpace_ONLINE)
 	if nodeClient == nil {
-		logger.Info("Node Client not found")
-		return false, nil, nil
+		return nil, errors.New("Node Client not found")
 	}
 	*logger = logger.WithValues("nodeClient", clientName)
 
@@ -394,10 +410,10 @@ func (r *ReclaimSpaceJobReconciler) nodeReclaimSpace(
 	defer cancel()
 	resp, err := nodeClient.NodeReclaimSpace(newCtx, req)
 	if err != nil {
-		return true, nil, err
+		return nil, err
 	}
 
-	return true, calculateReclaimedSpace(resp.PreUsage, resp.PostUsage), nil
+	return calculateReclaimedSpace(resp.PreUsage, resp.PostUsage), nil
 }
 
 // calculateReclaimedSpace returns amount of reclaimed space.

controllers/reclaimspacejob_controller_test.go

@@ -176,3 +176,46 @@ func TestCalculateReclaimedSpace(t *testing.T) {
 		})
 	}
 }
+
+func TestCanNodeReclaimSpace(t *testing.T) {
+	type fields struct {
+		driverName string
+		pvName     string
+		nodeID     string
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   bool
+	}{
+		{
+			name: "empty nodeID",
+			fields: fields{
+				driverName: "example.csi.com",
+				pvName:     "pvc-a8a5c531-9f88-4fc8-b35d-564585fb42a8",
+				nodeID:     "",
+			},
+			want: false,
+		},
+		{
+			name: "non-empty nodeID",
+			fields: fields{
+				driverName: "example.csi.com",
+				pvName:     "pvc-a8a5c531-9f88-4fc8-b35d-564585fb42a8",
+				nodeID:     "worker-1",
+			},
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			td := &targetDetails{
+				driverName: tt.fields.driverName,
+				pvName:     tt.fields.pvName,
+				nodeID:     tt.fields.nodeID,
+			}
+			got := td.canNodeReclaimSpace()
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}