diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index c5470f89c..6e1fd102b 100644 --- a/internal/rbd/rbd_util.go +++ b/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, diff --git a/internal/rbd/rbd_util_test.go b/internal/rbd/rbd_util_test.go index 7481b2b5a..905e97707 100644 --- a/internal/rbd/rbd_util_test.go +++ b/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) - } - }) - } -} diff --git a/internal/util/errors.go b/internal/util/errors.go index 5f757117f..11a3f2964 100644 --- a/internal/util/errors.go +++ b/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)) +} diff --git a/internal/util/errors_test.go b/internal/util/errors_test.go new file mode 100644 index 000000000..cdb106758 --- /dev/null +++ b/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) + } + }) + } +}