cleanup: address golangci 'errcheck' issues

Many reports are about closing or removing files. In some cases it is
possible to report an error in the logs, in other cases the error can be
ignored without potential issues.

Test cases have been modified to not remove the temporary files. The
temporary directory that is provided by the testing package, is removed
once the tests are done.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
This commit is contained in:
Niels de Vos
2025-05-02 17:33:29 +02:00
committed by mergify[bot]
parent 0a22e3a186
commit bc8b1e792f
19 changed files with 131 additions and 71 deletions

View File

@ -21,6 +21,7 @@ import (
"fmt"
rbderrors "github.com/ceph/ceph-csi/internal/rbd/errors"
"github.com/ceph/ceph-csi/internal/util/log"
)
// Sparsify checks the size of the objects in the RBD image and calls
@ -28,7 +29,7 @@ import (
// of the image.
// This function will return ErrImageInUse if the image is in use, since
// sparsifying an image on which i/o is in progress is not optimal.
func (ri *rbdImage) Sparsify(_ context.Context) error {
func (ri *rbdImage) Sparsify(ctx context.Context) error {
inUse, err := ri.isInUse()
if err != nil {
return fmt.Errorf("failed to check if image is in use: %w", err)
@ -42,7 +43,12 @@ func (ri *rbdImage) Sparsify(_ context.Context) error {
if err != nil {
return err
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
imageInfo, err := image.Stat()
if err != nil {

View File

@ -24,6 +24,7 @@ import (
rbderrors "github.com/ceph/ceph-csi/internal/rbd/errors"
"github.com/ceph/ceph-csi/internal/rbd/types"
"github.com/ceph/ceph-csi/internal/util/log"
)
// AddToGroup adds the image to the group. This is called from the rbd_group
@ -45,7 +46,12 @@ func (rv *rbdVolume) AddToGroup(ctx context.Context, vg types.VolumeGroup) error
if err != nil {
return fmt.Errorf("failed to open image %q: %w", rv, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
info, err := image.GetGroup()
if err != nil {
@ -88,7 +94,12 @@ func (rv *rbdVolume) GetVolumeGroupID(ctx context.Context, resolver types.Volume
if err != nil {
return "", fmt.Errorf("failed to open image %q: %w", rv, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
info, err := image.GetGroup()
if err != nil {

View File

@ -103,12 +103,17 @@ func (ri *rbdImage) ToMirror() (types.Mirror, error) {
}
// EnableMirroring enables mirroring on an image.
func (rm *rbdMirror) EnableMirroring(_ context.Context, mode librbd.ImageMirrorMode) error {
func (rm *rbdMirror) EnableMirroring(ctx context.Context, mode librbd.ImageMirrorMode) error {
image, err := rm.open()
if err != nil {
return fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
err = image.MirrorEnable(mode)
if err != nil {
@ -119,12 +124,17 @@ func (rm *rbdMirror) EnableMirroring(_ context.Context, mode librbd.ImageMirrorM
}
// DisableMirroring disables mirroring on an image.
func (rm *rbdMirror) DisableMirroring(_ context.Context, force bool) error {
func (rm *rbdMirror) DisableMirroring(ctx context.Context, force bool) error {
image, err := rm.open()
if err != nil {
return fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
err = image.MirrorDisable(force)
if err != nil {
@ -135,12 +145,17 @@ func (rm *rbdMirror) DisableMirroring(_ context.Context, force bool) error {
}
// GetMirroringInfo gets mirroring information of an image.
func (rm *rbdMirror) GetMirroringInfo(_ context.Context) (types.MirrorInfo, error) {
func (rm *rbdMirror) GetMirroringInfo(ctx context.Context) (types.MirrorInfo, error) {
image, err := rm.open()
if err != nil {
return nil, fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
info, err := image.GetMirrorImageInfo()
if err != nil {
@ -151,12 +166,18 @@ func (rm *rbdMirror) GetMirroringInfo(_ context.Context) (types.MirrorInfo, erro
}
// Promote promotes image to primary.
func (rm *rbdMirror) Promote(_ context.Context, force bool) error {
func (rm *rbdMirror) Promote(ctx context.Context, force bool) error {
image, err := rm.open()
if err != nil {
return fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
err = image.MirrorPromote(force)
if err != nil {
return fmt.Errorf("failed to promote image %q with error: %w", rm, err)
@ -196,12 +217,18 @@ func (rm *rbdMirror) ForcePromote(ctx context.Context, cr *util.Credentials) err
}
// Demote demotes image to secondary.
func (rm *rbdMirror) Demote(_ context.Context) error {
func (rm *rbdMirror) Demote(ctx context.Context) error {
image, err := rm.open()
if err != nil {
return fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
err = image.MirrorDemote()
if err != nil {
return fmt.Errorf("failed to demote image %q with error: %w", rm, err)
@ -220,7 +247,13 @@ func (rm *rbdMirror) Resync(ctx context.Context) error {
if err != nil {
return fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
err = image.MirrorResync()
if err != nil {
return fmt.Errorf("failed to resync image %q with error: %w", rm, err)
@ -270,12 +303,18 @@ func (rm *rbdMirror) Resync(ctx context.Context) error {
}
// GetGlobalMirroringStatus get the mirroring status of an image.
func (rm *rbdMirror) GetGlobalMirroringStatus(_ context.Context) (types.GlobalStatus, error) {
func (rm *rbdMirror) GetGlobalMirroringStatus(ctx context.Context) (types.GlobalStatus, error) {
image, err := rm.open()
if err != nil {
return nil, fmt.Errorf("failed to open image %q with error: %w", rm, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
statusInfo, err := image.GetGlobalMirrorStatus()
if err != nil {
return nil, fmt.Errorf("failed to get image mirroring status %q with error: %w", rm, err)

View File

@ -1331,7 +1331,7 @@ func (ns *NodeServer) ext4SupportsPrezeroed() bool {
return false
}
defer os.Remove(tempImgFile.Name())
defer os.Remove(tempImgFile.Name()) //nolint:errcheck // failed to remove temp file :-(
if err = file.CreateSparseFile(tempImgFile, 1); err != nil {
log.WarningLog(ctx, "failed to create sparse file: %v", err)

View File

@ -515,7 +515,7 @@ func (ri *rbdImage) getImageID() error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
id, err := image.GetId()
if err != nil {
@ -563,7 +563,7 @@ func (ri *rbdImage) isInUse() (bool, error) {
// any error should assume something else is using the image
return true, err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
watchers, err := image.ListWatchers()
if err != nil {
@ -933,7 +933,7 @@ func (ri *rbdImage) getParentName() (string, error) {
if err != nil {
return "", err
}
defer rbdImage.Close()
defer rbdImage.Close() //nolint:errcheck // not a critical failure
parentInfo, err := rbdImage.GetParent()
if err != nil {
@ -948,7 +948,7 @@ func (ri *rbdImage) flatten() error {
if err != nil {
return err
}
defer rbdImage.Close()
defer rbdImage.Close() //nolint:errcheck // not a critical failure
err = rbdImage.Flatten()
if err != nil {
@ -1519,7 +1519,12 @@ func (ri *rbdImage) createSnapshot(ctx context.Context, pOpts *rbdSnapshot) erro
if err != nil {
return err
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
_, err = image.CreateSnapshot(pOpts.RbdSnapName)
@ -1532,7 +1537,12 @@ func (ri *rbdImage) deleteSnapshot(ctx context.Context, pOpts *rbdSnapshot) erro
if err != nil {
return err
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
snap := image.GetSnapshot(pOpts.RbdSnapName)
if snap == nil {
@ -1691,7 +1701,7 @@ func (ri *rbdImage) getImageInfo() error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
imageInfo, err := image.Stat()
if err != nil {
@ -1787,7 +1797,7 @@ func (ri *rbdImage) checkSnapExists(rbdSnap *rbdSnapshot) error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
snaps, err := image.GetSnapshotNames()
if err != nil {
@ -1947,7 +1957,7 @@ func (ri *rbdImage) resize(newSize int64) error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
err = image.Resize(uint64(util.RoundOffVolSize(newSize) * helpers.MiB))
if err != nil {
@ -1964,7 +1974,7 @@ func (ri *rbdImage) GetMetadata(key string) (string, error) {
if err != nil {
return "", err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
return image.GetMetadata(key)
}
@ -1974,7 +1984,7 @@ func (ri *rbdImage) SetMetadata(key, value string) error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
return image.SetMetadata(key, value)
}
@ -1985,7 +1995,7 @@ func (ri *rbdImage) RemoveMetadata(key string) error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
return image.RemoveMetadata(key)
}
@ -2050,7 +2060,7 @@ func (ri *rbdImage) DeepCopy(dest *rbdImage) error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
err = image.DeepCopy(dest.ioctx, dest.RbdImageName, opts)
if err != nil {
@ -2067,7 +2077,7 @@ func (ri *rbdImage) DisableDeepFlatten() error {
if err != nil {
return err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
return image.UpdateFeatures(librbd.FeatureDeepFlatten, false)
}
@ -2085,7 +2095,7 @@ func (ri *rbdImage) listSnapAndChildren() (*snapAndChildrenInfo, error) {
if err != nil {
return nil, err
}
defer image.Close()
defer image.Close() //nolint:errcheck // not a critical failure
snaps, err := image.GetSnapshotNames()
if err != nil {

View File

@ -302,7 +302,6 @@ func TestStrategicActionOnLogFile(t *testing.T) {
if _, err = os.Stat(newExt); os.IsNotExist(err) {
t.Errorf("compressed logFile (%s) not found: %v", newExt, err)
}
os.Remove(newExt)
case "remove":
if _, err = os.Stat(tt.args.logFile); !os.IsNotExist(err) {
t.Errorf("logFile (%s) not removed: %v", tt.args.logFile, err)
@ -311,7 +310,6 @@ func TestStrategicActionOnLogFile(t *testing.T) {
if _, err = os.Stat(tt.args.logFile); os.IsNotExist(err) {
t.Errorf("logFile (%s) not preserved: %v", tt.args.logFile, err)
}
os.Remove(tt.args.logFile)
}
})
}

View File

@ -355,7 +355,12 @@ func (rv *rbdVolume) NewSnapshotByID(
if err != nil {
return nil, fmt.Errorf("failed to open snapshot image %q: %w", snap, err)
}
defer image.Close()
defer func() {
cErr := image.Close()
if cErr != nil {
log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr)
}
}()
snapImage, err = image.CreateSnapshot(snap.RbdSnapName)
if err != nil && !errors.Is(err, librbd.ErrExist) {