diff --git a/cmd/cephfs/main.go b/cmd/cephfs/main.go index 3d54b87c4..ece731eac 100644 --- a/cmd/cephfs/main.go +++ b/cmd/cephfs/main.go @@ -27,7 +27,10 @@ import ( ) func init() { - flag.Set("logtostderr", "true") + if err := flag.Set("logtostderr", "true"); err != nil { + glog.Errorf("failed to set logtostderr flag: %v", err) + os.Exit(1) + } } var ( diff --git a/cmd/rbd/main.go b/cmd/rbd/main.go index c89092845..9bcd8a3d8 100644 --- a/cmd/rbd/main.go +++ b/cmd/rbd/main.go @@ -27,7 +27,10 @@ import ( ) func init() { - flag.Set("logtostderr", "true") + if err := flag.Set("logtostderr", "true"); err != nil { + glog.Errorf("failed to set logtostderr flag: %v", err) + os.Exit(1) + } } var ( @@ -64,10 +67,11 @@ func main() { func createPersistentStorage(persistentStoragePath string) error { if _, err := os.Stat(persistentStoragePath); os.IsNotExist(err) { - if err := os.MkdirAll(persistentStoragePath, os.FileMode(0755)); err != nil { + if err = os.MkdirAll(persistentStoragePath, os.FileMode(0755)); err != nil { return err } } else { + return err } return nil } diff --git a/pkg/cephfs/cephconf.go b/pkg/cephfs/cephconf.go index f122332a2..d9a0cebbd 100644 --- a/pkg/cephfs/cephconf.go +++ b/pkg/cephfs/cephconf.go @@ -21,6 +21,8 @@ import ( "os" "path" "text/template" + + "github.com/golang/glog" ) const cephConfig = `[global] @@ -89,7 +91,11 @@ func writeCephTemplate(fileName string, m os.FileMode, t *template.Template, dat return err } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + glog.Errorf("failed to close file %s with error %s", f.Name(), err) + } + }() return t.Execute(f, data) } diff --git a/pkg/cephfs/cephuser.go b/pkg/cephfs/cephuser.go index 48675e054..9536b4483 100644 --- a/pkg/cephfs/cephuser.go +++ b/pkg/cephfs/cephuser.go @@ -21,6 +21,8 @@ import ( "encoding/json" "fmt" "os" + + "github.com/golang/glog" ) const ( @@ -111,12 +113,20 @@ func deleteCephUser(adminCr *credentials, volID volumeID) error { "auth", "rm", cephEntityClientPrefix + userID, } - if err := execCommandAndValidate("ceph", args[:]...); err != nil { + var err error + if err = execCommandAndValidate("ceph", args[:]...); err != nil { return err } - os.Remove(getCephKeyringPath(volID, userID)) - os.Remove(getCephSecretPath(volID, userID)) + keyringPath := getCephKeyringPath(volID, userID) + if err = os.Remove(keyringPath); err != nil { + glog.Errorf("failed to remove keyring file %s with error %s", keyringPath, err) + } + + secretPath := getCephSecretPath(volID, userID) + if err = os.Remove(secretPath); err != nil { + glog.Errorf("failed to remove secret file %s with error %s", secretPath, err) + } return nil } diff --git a/pkg/cephfs/controllerserver.go b/pkg/cephfs/controllerserver.go index f42780114..20b6fe729 100644 --- a/pkg/cephfs/controllerserver.go +++ b/pkg/cephfs/controllerserver.go @@ -132,7 +132,8 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol // mons may have changed since create volume, // retrieve the latest mons and override old mons secret := req.GetSecrets() - if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { + mon := "" + if mon, err = getMonValFromSecret(secret); err == nil && len(mon) > 0 { glog.Infof("override old mons [%q] with [%q]", ce.VolOptions.Monitors, mon) ce.VolOptions.Monitors = mon } diff --git a/pkg/cephfs/nodeserver.go b/pkg/cephfs/nodeserver.go index 2c7c3b658..412196929 100644 --- a/pkg/cephfs/nodeserver.go +++ b/pkg/cephfs/nodeserver.go @@ -197,18 +197,21 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis // NodeUnpublishVolume unmounts the volume from the target path func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { - if err := validateNodeUnpublishVolumeRequest(req); err != nil { + var err error + if err = validateNodeUnpublishVolumeRequest(req); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } targetPath := req.GetTargetPath() // Unmount the bind-mount - if err := unmountVolume(targetPath); err != nil { + if err = unmountVolume(targetPath); err != nil { return nil, status.Error(codes.Internal, err.Error()) } - os.Remove(targetPath) + if err = os.Remove(targetPath); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } glog.Infof("cephfs: successfully unbinded volume %s from %s", req.GetVolumeId(), targetPath) @@ -217,18 +220,21 @@ func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu // NodeUnstageVolume unstages the volume from the staging path func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { - if err := validateNodeUnstageVolumeRequest(req); err != nil { + var err error + if err = validateNodeUnstageVolumeRequest(req); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } stagingTargetPath := req.GetStagingTargetPath() // Unmount the volume - if err := unmountVolume(stagingTargetPath); err != nil { + if err = unmountVolume(stagingTargetPath); err != nil { return nil, status.Error(codes.Internal, err.Error()) } - os.Remove(stagingTargetPath) + if err = os.Remove(stagingTargetPath); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } glog.Infof("cephfs: successfully umounted volume %s from %s", req.GetVolumeId(), stagingTargetPath) diff --git a/pkg/cephfs/volume.go b/pkg/cephfs/volume.go index ce6f01a17..51d290dae 100644 --- a/pkg/cephfs/volume.go +++ b/pkg/cephfs/volume.go @@ -20,6 +20,8 @@ import ( "fmt" "os" "path" + + "github.com/golang/glog" ) const ( @@ -70,8 +72,7 @@ func createVolume(volOptions *volumeOptions, adminCr *credentials, volID volumeI } defer func() { - unmountVolume(cephRoot) - os.Remove(cephRoot) + umountAndRemove(cephRoot) }() volOptions.RootPath = getVolumeRootPathCeph(volID) @@ -123,8 +124,7 @@ func purgeVolume(volID volumeID, adminCr *credentials, volOptions *volumeOptions } defer func() { - unmountVolume(volRoot) - os.Remove(volRoot) + umountAndRemove(volRoot) }() if err := os.Rename(volRoot, volRootDeleting); err != nil { @@ -137,3 +137,14 @@ func purgeVolume(volID volumeID, adminCr *credentials, volOptions *volumeOptions return nil } + +func umountAndRemove(mountPoint string) { + var err error + if err = unmountVolume(mountPoint); err != nil { + glog.Errorf("failed to unmount %s with error %s", mountPoint, err) + } + + if err = os.Remove(mountPoint); err != nil { + glog.Errorf("failed to remove %s with error %s", mountPoint, err) + } +} diff --git a/pkg/cephfs/volumeoptions.go b/pkg/cephfs/volumeoptions.go index 8b7028991..ce59d9c2a 100644 --- a/pkg/cephfs/volumeoptions.go +++ b/pkg/cephfs/volumeoptions.go @@ -102,7 +102,8 @@ func newVolumeOptions(volOptions, secret map[string]string) (*volumeOptions, err // extract mon from secret first if err = extractOption(&opts.MonValueFromSecret, "monValueFromSecret", volOptions); err == nil { - if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { + mon := "" + if mon, err = getMonValFromSecret(secret); err == nil && len(mon) > 0 { opts.Monitors = mon } } @@ -131,6 +132,8 @@ func newVolumeOptions(volOptions, secret map[string]string) (*volumeOptions, err } // This field is optional, don't check for its presence + // nolint: errcheck + // (skip errcheck as this is optional) extractOption(&opts.Mounter, "mounter", volOptions) if err = opts.validate(); err != nil { diff --git a/pkg/rbd/controllerserver.go b/pkg/rbd/controllerserver.go index 7d5fb8205..ab17f4c66 100644 --- a/pkg/rbd/controllerserver.go +++ b/pkg/rbd/controllerserver.go @@ -55,16 +55,19 @@ var ( // info from metadata store func (cs *ControllerServer) LoadExDataFromMetadataStore() error { vol := &rbdVolume{} + // nolint: errcheck cs.MetadataStore.ForAll("csi-rbd-vol-", vol, func(identifier string) error { rbdVolumes[identifier] = vol return nil }) snap := &rbdSnapshot{} + // nolint: errcheck cs.MetadataStore.ForAll("csi-rbd-(.*)-snap-", snap, func(identifier string) error { rbdSnapshots[identifier] = snap return nil }) + glog.Infof("Loaded %d volumes and %d snapshots from metadata store", len(rbdVolumes), len(rbdSnapshots)) return nil } @@ -91,7 +94,11 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, err } volumeNameMutex.LockKey(req.GetName()) - defer volumeNameMutex.UnlockKey(req.GetName()) + defer func() { + if err := volumeNameMutex.UnlockKey(req.GetName()); err != nil { + glog.Warningf("failed to unlock mutex volume:%s %v", req.GetName(), err) + } + }() // Need to check for already existing volume name, and if found // check for the requested capacity and already allocated capacity @@ -208,7 +215,13 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol // For now the image get unconditionally deleted, but here retention policy can be checked volumeID := req.GetVolumeId() volumeIDMutex.LockKey(volumeID) - defer volumeIDMutex.UnlockKey(volumeID) + + defer func() { + if err := volumeIDMutex.UnlockKey(volumeID); err != nil { + glog.Warningf("failed to unlock mutex volume:%s %v", volumeID, err) + } + }() + rbdVol := &rbdVolume{} if err := cs.MetadataStore.Get(volumeID, rbdVol); err != nil { if os.IsNotExist(errors.Cause(err)) { @@ -276,7 +289,12 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS } snapshotNameMutex.LockKey(req.GetName()) - defer snapshotNameMutex.UnlockKey(req.GetName()) + + defer func() { + if err := snapshotNameMutex.UnlockKey(req.GetName()); err != nil { + glog.Warningf("failed to unlock mutex snapshot:%s %v", req.GetName(), err) + } + }() // Need to check for already existing snapshot name, and if found // check for the requested source volume id and already allocated source volume id @@ -397,7 +415,12 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS return nil, status.Error(codes.InvalidArgument, "Snapshot ID cannot be empty") } snapshotIDMutex.LockKey(snapshotID) - defer snapshotIDMutex.UnlockKey(snapshotID) + + defer func() { + if err := snapshotIDMutex.UnlockKey(snapshotID); err != nil { + glog.Warningf("failed to unlock mutex snapshot:%s %v", snapshotID, err) + } + }() rbdSnap := &rbdSnapshot{} if err := cs.MetadataStore.Get(snapshotID, rbdSnap); err != nil { diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index 835728ad7..a54bf7058 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -60,7 +60,12 @@ func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { targetPath := req.GetTargetPath() targetPathMutex.LockKey(targetPath) - defer targetPathMutex.UnlockKey(targetPath) + + defer func() { + if err := targetPathMutex.UnlockKey(targetPath); err != nil { + glog.Warningf("failed to unlock mutex targetpath:%s %v", targetPath, err) + } + }() var volName string isBlock := req.GetVolumeCapability().GetBlock() != nil @@ -84,12 +89,12 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis if os.IsNotExist(err) { if isBlock { // create an empty file - targetPathFile, err := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, 0750) - if err != nil { + targetPathFile, e := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, 0750) + if e != nil { glog.V(4).Infof("Failed to create targetPath:%s with error: %v", targetPath, err) - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.Internal, e.Error()) } - if err := targetPathFile.Close(); err != nil { + if err = targetPathFile.Close(); err != nil { glog.V(4).Infof("Failed to close targetPath:%s with error: %v", targetPath, err) return nil, status.Error(codes.Internal, err.Error()) } @@ -153,7 +158,12 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { targetPath := req.GetTargetPath() targetPathMutex.LockKey(targetPath) - defer targetPathMutex.UnlockKey(targetPath) + + defer func() { + if err := targetPathMutex.UnlockKey(targetPath); err != nil { + glog.Warningf("failed to unlock mutex targetpath:%s %v", targetPath, err) + } + }() notMnt, err := ns.mounter.IsNotMountPoint(targetPath) if err != nil { @@ -177,7 +187,6 @@ func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu // Bind mounted device needs to be resolved by using resolveBindMountedBlockDevice if devicePath == "devtmpfs" { - var err error devicePath, err = resolveBindMountedBlockDevice(targetPath) if err != nil { return nil, status.Error(codes.Internal, err.Error()) @@ -206,13 +215,13 @@ func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu } // Unmapping rbd device - if err := detachRBDDevice(devicePath); err != nil { + if err = detachRBDDevice(devicePath); err != nil { glog.V(3).Infof("failed to unmap rbd device: %s with error: %v", devicePath, err) return nil, err } // Remove targetPath - if err := os.RemoveAll(targetPath); err != nil { + if err = os.RemoveAll(targetPath); err != nil { glog.V(3).Infof("failed to remove targetPath: %s with error: %v", targetPath, err) return nil, err } @@ -240,7 +249,7 @@ func parseFindMntResolveSource(out string) (string, error) { return match[1], nil } // Check if out is a block device - reBlk := regexp.MustCompile("^devtmpfs\\[(/[^/]+(?:/[^/]*)*)\\]$") + reBlk := regexp.MustCompile(`^devtmpfs\\[(/[^/]+(?:/[^/]*)*)\\]$`) if match := reBlk.FindStringSubmatch(out); match != nil { return fmt.Sprintf("/dev%s", match[1]), nil } diff --git a/pkg/rbd/rbd.go b/pkg/rbd/rbd.go index 79c44c846..ba83b325f 100644 --- a/pkg/rbd/rbd.go +++ b/pkg/rbd/rbd.go @@ -112,7 +112,10 @@ func (r *Driver) Run(driverName, nodeID, endpoint string, containerized bool, ca } r.cs = NewControllerServer(r.cd, cachePersister) - r.cs.LoadExDataFromMetadataStore() + + if err = r.cs.LoadExDataFromMetadataStore(); err != nil { + glog.Fatalf("failed to load metadata from store, err %v\n", err) + } s := csicommon.NewNonBlockingGRPCServer() s.Start(endpoint, r.ids, r.cs, r.ns) diff --git a/pkg/rbd/rbd_attach.go b/pkg/rbd/rbd_attach.go index ee7b39a2c..1cca3f8b7 100644 --- a/pkg/rbd/rbd_attach.go +++ b/pkg/rbd/rbd_attach.go @@ -229,7 +229,12 @@ func attachRBDImage(volOptions *rbdVolume, userID string, credentials map[string devicePath, found := waitForPath(volOptions.Pool, image, 1, useNBD) if !found { attachdetachMutex.LockKey(imagePath) - defer attachdetachMutex.UnlockKey(imagePath) + + defer func() { + if err = attachdetachMutex.UnlockKey(imagePath); err != nil { + glog.Warningf("failed to unlock mutex imagepath:%s %v", imagePath, err) + } + }() _, err = execCommand("modprobe", []string{moduleName}) if err != nil { diff --git a/pkg/util/nodecache.go b/pkg/util/nodecache.go index 10cf1dc45..f4a1c566f 100644 --- a/pkg/util/nodecache.go +++ b/pkg/util/nodecache.go @@ -73,7 +73,10 @@ func (nc *NodeCache) ForAll(pattern string, destObj interface{}, f ForAllFunc) e } decoder := json.NewDecoder(fp) if err = decoder.Decode(destObj); err != nil { - fp.Close() + if err = fp.Close(); err != nil { + return errors.Wrapf(err, "failed to close file %s", file.Name()) + + } return errors.Wrapf(err, "node-cache: couldn't decode file %s", file.Name()) } if err := f(strings.TrimSuffix(file.Name(), filepath.Ext(file.Name()))); err != nil { @@ -90,7 +93,13 @@ func (nc *NodeCache) Create(identifier string, data interface{}) error { if err != nil { return errors.Wrapf(err, "node-cache: failed to create metadata storage file %s\n", file) } - defer fp.Close() + + defer func() { + if err = fp.Close(); err != nil { + glog.Warningf("failed to close file:%s %v", fp.Name(), err) + } + }() + encoder := json.NewEncoder(fp) if err = encoder.Encode(data); err != nil { return errors.Wrapf(err, "node-cache: failed to encode metadata for file: %s\n", file) @@ -106,7 +115,12 @@ func (nc *NodeCache) Get(identifier string, data interface{}) error { if err != nil { return errors.Wrapf(err, "node-cache: open error for %s", file) } - defer fp.Close() + + defer func() { + if err = fp.Close(); err != nil { + glog.Warningf("failed to close file:%s %v", fp.Name(), err) + } + }() decoder := json.NewDecoder(fp) if err = decoder.Decode(data); err != nil {