cleanup: move ShouldRetryVolumeGeneration from internal/rbd to intern…

…al/util

Signed-off-by: Praveen M <[email protected]>

internal/rbd/rbd_util.go

@@ -1214,7 +1214,7 @@ func GenVolFromVolID(
 	}
 
 	vol, err = generateVolumeFromVolumeID(ctx, volumeID, vi, cr, secrets)
-	if !shouldRetryVolumeGeneration(err) {
+	if !util.ShouldRetryVolumeGeneration(err) {
 		return vol, err
 	}
 
@@ -1225,7 +1225,7 @@ func GenVolFromVolID(
 	}
 	if mapping != nil {
 		rbdVol, vErr := generateVolumeFromMapping(ctx, mapping, volumeID, vi, cr, secrets)
-		if !shouldRetryVolumeGeneration(vErr) {
+		if !util.ShouldRetryVolumeGeneration(vErr) {
 			return rbdVol, vErr
 		}
 	}
@@ -1278,7 +1278,7 @@ func generateVolumeFromMapping(
 					// Add mapping poolID to Identifier
 					nvi.LocationID = pID
 					vol, err = generateVolumeFromVolumeID(ctx, volumeID, nvi, cr, secrets)
-					if !shouldRetryVolumeGeneration(err) {
+					if !util.ShouldRetryVolumeGeneration(err) {
 						return vol, err
 					}
 				}
@@ -1289,33 +1289,6 @@ func generateVolumeFromMapping(
 	return vol, util.ErrPoolNotFound
 }
 
-// shouldRetryVolumeGeneration determines whether the process of finding or generating
-// volumes should continue based on the type of error encountered.
-//
-// It checks if the given error matches any of the following known errors:
-//   - util.ErrKeyNotFound: The key required to locate the volume is missing in Rados omap.
-//   - util.ErrPoolNotFound: The rbd pool where the volume/omap is expected doesn't exist.
-//   - ErrImageNotFound: The image doesn't exist in the rbd pool.
-//   - rados.ErrPermissionDenied: Permissions to access the pool is denied.
-//
-// If any of these errors are encountered, the function returns `true`, indicating
-// that the volume search should continue because of known error. Otherwise, it
-// returns `false`, meaning the search should stop.
-//
-// This helper function is used in scenarios where multiple attempts may be made
-// to retrieve or generate volume information, and we want to gracefully handle
-// specific failure cases while retrying for others.
-func shouldRetryVolumeGeneration(err error) bool {
-	if err == nil {
-		return false // No error, do not retry
-	}
-	// Continue searching for specific known errors
-	return (errors.Is(err, util.ErrKeyNotFound) ||
-		errors.Is(err, util.ErrPoolNotFound) ||
-		errors.Is(err, util.ErrImageNotFound) ||
-		errors.Is(err, rados.ErrPermissionDenied))
-}
-
 func genVolFromVolumeOptions(
 	ctx context.Context,
 	volOptions map[string]string,

internal/rbd/rbd_util_test.go

@@ -23,11 +23,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/ceph/go-ceph/rados"
 	librbd "github.com/ceph/go-ceph/rbd"
 	"github.com/stretchr/testify/require"
-
-	"github.com/ceph/ceph-csi/internal/util"
 )
 
 func TestHasSnapshotFeature(t *testing.T) {
@@ -390,54 +387,3 @@ func Test_checkValidImageFeatures(t *testing.T) {
 		})
 	}
 }
-
-func Test_shouldRetryVolumeGeneration(t *testing.T) {
-	t.Parallel()
-	type args struct {
-		err error
-	}
-	tests := []struct {
-		name string
-		args args
-		want bool
-	}{
-		{
-			name: "No error (stop searching)",
-			args: args{err: nil},
-			want: false, // No error, stop searching
-		},
-		{
-			name: "ErrKeyNotFound (continue searching)",
-			args: args{err: util.ErrKeyNotFound},
-			want: true, // Known error, continue searching
-		},
-		{
-			name: "ErrPoolNotFound (continue searching)",
-			args: args{err: util.ErrPoolNotFound},
-			want: true, // Known error, continue searching
-		},
-		{
-			name: "ErrImageNotFound (continue searching)",
-			args: args{err: util.ErrImageNotFound},
-			want: true, // Known error, continue searching
-		},
-		{
-			name: "ErrPermissionDenied (continue searching)",
-			args: args{err: rados.ErrPermissionDenied},
-			want: true, // Known error, continue searching
-		},
-		{
-			name: "Different error (stop searching)",
-			args: args{err: errors.New("unknown error")},
-			want: false, // Unknown error, stop searching
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			if got := shouldRetryVolumeGeneration(tt.args.err); got != tt.want {
-				t.Errorf("shouldRetryVolumeGeneration() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}

internal/util/errors.go

@@ -18,6 +18,8 @@ package util
 
 import (
 	"errors"
+
+	"github.com/ceph/go-ceph/rados"
 )
 
 var (
@@ -39,3 +41,30 @@ var (
 	// ErrMissingConfigForMonitor is returned when clusterID is not found for the mon.
 	ErrMissingConfigForMonitor = errors.New("missing configuration of cluster ID for monitor")
 )
+
+// ShouldRetryVolumeGeneration determines whether the process of finding or generating
+// volumes should continue based on the type of error encountered.
+//
+// It checks if the given error matches any of the following known errors:
+//   - util.ErrKeyNotFound: The key required to locate the volume is missing in Rados omap.
+//   - util.ErrPoolNotFound: The rbd pool where the volume/omap is expected doesn't exist.
+//   - ErrImageNotFound: The image doesn't exist in the rbd pool.
+//   - rados.ErrPermissionDenied: Permissions to access the pool is denied.
+//
+// If any of these errors are encountered, the function returns `true`, indicating
+// that the volume search should continue because of known error. Otherwise, it
+// returns `false`, meaning the search should stop.
+//
+// This helper function is used in scenarios where multiple attempts may be made
+// to retrieve or generate volume information, and we want to gracefully handle
+// specific failure cases while retrying for others.
+func ShouldRetryVolumeGeneration(err error) bool {
+	if err == nil {
+		return false // No error, do not retry
+	}
+	// Continue searching for specific known errors
+	return (errors.Is(err, ErrKeyNotFound) ||
+		errors.Is(err, ErrPoolNotFound) ||
+		errors.Is(err, ErrImageNotFound) ||
+		errors.Is(err, rados.ErrPermissionDenied))
+}

internal/util/errors_test.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2025 The Ceph-CSI Authors.
+
+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
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/ceph/go-ceph/rados"
+)
+
+func Test_shouldRetryVolumeGeneration(t *testing.T) {
+	t.Parallel()
+	type args struct {
+		err error
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "No error (stop searching)",
+			args: args{err: nil},
+			want: false, // No error, stop searching
+		},
+		{
+			name: "ErrKeyNotFound (continue searching)",
+			args: args{err: ErrKeyNotFound},
+			want: true, // Known error, continue searching
+		},
+		{
+			name: "ErrPoolNotFound (continue searching)",
+			args: args{err: ErrPoolNotFound},
+			want: true, // Known error, continue searching
+		},
+		{
+			name: "ErrImageNotFound (continue searching)",
+			args: args{err: ErrImageNotFound},
+			want: true, // Known error, continue searching
+		},
+		{
+			name: "ErrPermissionDenied (continue searching)",
+			args: args{err: rados.ErrPermissionDenied},
+			want: true, // Known error, continue searching
+		},
+		{
+			name: "Different error (stop searching)",
+			args: args{err: errors.New("unknown error")},
+			want: false, // Unknown error, stop searching
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := ShouldRetryVolumeGeneration(tt.args.err); got != tt.want {
+				t.Errorf("ShouldRetryVolumeGeneration() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}