cleanup: move ShouldRetryVolumeGeneration from internal/rbd to internal/util

Signed-off-by: Praveen M <m.praveen@ibm.com>
This commit is contained in:
Praveen M 2025-02-06 11:47:09 +05:30 committed by mergify[bot]
parent 6414e94401
commit cbd73f296d
4 changed files with 107 additions and 84 deletions

View File

@ -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,

View File

@ -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)
}
})
}
}

View File

@ -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))
}

View File

@ -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)
}
})
}
}