Fix issues found in gometalinter

Signed-off-by: Madhu Rajanna <mrajanna@redhat.com>
This commit is contained in:
Madhu Rajanna 2019-01-28 19:29:16 +05:30
parent 008c82c1e7
commit 7a0c233c27
13 changed files with 136 additions and 38 deletions

View File

@ -27,7 +27,10 @@ import (
) )
func init() { 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 ( var (

View File

@ -27,7 +27,10 @@ import (
) )
func init() { 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 ( var (
@ -64,10 +67,11 @@ func main() {
func createPersistentStorage(persistentStoragePath string) error { func createPersistentStorage(persistentStoragePath string) error {
if _, err := os.Stat(persistentStoragePath); os.IsNotExist(err) { 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 return err
} }
} else { } else {
return err
} }
return nil return nil
} }

View File

@ -21,6 +21,8 @@ import (
"os" "os"
"path" "path"
"text/template" "text/template"
"github.com/golang/glog"
) )
const cephConfig = `[global] const cephConfig = `[global]
@ -89,7 +91,11 @@ func writeCephTemplate(fileName string, m os.FileMode, t *template.Template, dat
return err 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) return t.Execute(f, data)
} }

View File

@ -21,6 +21,8 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"os" "os"
"github.com/golang/glog"
) )
const ( const (
@ -111,12 +113,20 @@ func deleteCephUser(adminCr *credentials, volID volumeID) error {
"auth", "rm", cephEntityClientPrefix + userID, "auth", "rm", cephEntityClientPrefix + userID,
} }
if err := execCommandAndValidate("ceph", args[:]...); err != nil { var err error
if err = execCommandAndValidate("ceph", args[:]...); err != nil {
return err return err
} }
os.Remove(getCephKeyringPath(volID, userID)) keyringPath := getCephKeyringPath(volID, userID)
os.Remove(getCephSecretPath(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 return nil
} }

View File

@ -132,7 +132,8 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
// mons may have changed since create volume, // mons may have changed since create volume,
// retrieve the latest mons and override old mons // retrieve the latest mons and override old mons
secret := req.GetSecrets() 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) glog.Infof("override old mons [%q] with [%q]", ce.VolOptions.Monitors, mon)
ce.VolOptions.Monitors = mon ce.VolOptions.Monitors = mon
} }

View File

@ -197,18 +197,21 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
// NodeUnpublishVolume unmounts the volume from the target path // NodeUnpublishVolume unmounts the volume from the target path
func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { 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()) return nil, status.Error(codes.InvalidArgument, err.Error())
} }
targetPath := req.GetTargetPath() targetPath := req.GetTargetPath()
// Unmount the bind-mount // Unmount the bind-mount
if err := unmountVolume(targetPath); err != nil { if err = unmountVolume(targetPath); err != nil {
return nil, status.Error(codes.Internal, err.Error()) 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) 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 // NodeUnstageVolume unstages the volume from the staging path
func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { 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()) return nil, status.Error(codes.InvalidArgument, err.Error())
} }
stagingTargetPath := req.GetStagingTargetPath() stagingTargetPath := req.GetStagingTargetPath()
// Unmount the volume // Unmount the volume
if err := unmountVolume(stagingTargetPath); err != nil { if err = unmountVolume(stagingTargetPath); err != nil {
return nil, status.Error(codes.Internal, err.Error()) 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) glog.Infof("cephfs: successfully umounted volume %s from %s", req.GetVolumeId(), stagingTargetPath)

View File

@ -20,6 +20,8 @@ import (
"fmt" "fmt"
"os" "os"
"path" "path"
"github.com/golang/glog"
) )
const ( const (
@ -70,8 +72,7 @@ func createVolume(volOptions *volumeOptions, adminCr *credentials, volID volumeI
} }
defer func() { defer func() {
unmountVolume(cephRoot) umountAndRemove(cephRoot)
os.Remove(cephRoot)
}() }()
volOptions.RootPath = getVolumeRootPathCeph(volID) volOptions.RootPath = getVolumeRootPathCeph(volID)
@ -123,8 +124,7 @@ func purgeVolume(volID volumeID, adminCr *credentials, volOptions *volumeOptions
} }
defer func() { defer func() {
unmountVolume(volRoot) umountAndRemove(volRoot)
os.Remove(volRoot)
}() }()
if err := os.Rename(volRoot, volRootDeleting); err != nil { if err := os.Rename(volRoot, volRootDeleting); err != nil {
@ -137,3 +137,14 @@ func purgeVolume(volID volumeID, adminCr *credentials, volOptions *volumeOptions
return nil 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)
}
}

View File

@ -102,7 +102,8 @@ func newVolumeOptions(volOptions, secret map[string]string) (*volumeOptions, err
// extract mon from secret first // extract mon from secret first
if err = extractOption(&opts.MonValueFromSecret, "monValueFromSecret", volOptions); err == nil { 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 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 // This field is optional, don't check for its presence
// nolint: errcheck
// (skip errcheck as this is optional)
extractOption(&opts.Mounter, "mounter", volOptions) extractOption(&opts.Mounter, "mounter", volOptions)
if err = opts.validate(); err != nil { if err = opts.validate(); err != nil {

View File

@ -55,16 +55,19 @@ var (
// info from metadata store // info from metadata store
func (cs *ControllerServer) LoadExDataFromMetadataStore() error { func (cs *ControllerServer) LoadExDataFromMetadataStore() error {
vol := &rbdVolume{} vol := &rbdVolume{}
// nolint: errcheck
cs.MetadataStore.ForAll("csi-rbd-vol-", vol, func(identifier string) error { cs.MetadataStore.ForAll("csi-rbd-vol-", vol, func(identifier string) error {
rbdVolumes[identifier] = vol rbdVolumes[identifier] = vol
return nil return nil
}) })
snap := &rbdSnapshot{} snap := &rbdSnapshot{}
// nolint: errcheck
cs.MetadataStore.ForAll("csi-rbd-(.*)-snap-", snap, func(identifier string) error { cs.MetadataStore.ForAll("csi-rbd-(.*)-snap-", snap, func(identifier string) error {
rbdSnapshots[identifier] = snap rbdSnapshots[identifier] = snap
return nil return nil
}) })
glog.Infof("Loaded %d volumes and %d snapshots from metadata store", len(rbdVolumes), len(rbdSnapshots)) glog.Infof("Loaded %d volumes and %d snapshots from metadata store", len(rbdVolumes), len(rbdSnapshots))
return nil return nil
} }
@ -91,7 +94,11 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
return nil, err return nil, err
} }
volumeNameMutex.LockKey(req.GetName()) 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 // Need to check for already existing volume name, and if found
// check for the requested capacity and already allocated capacity // 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 // For now the image get unconditionally deleted, but here retention policy can be checked
volumeID := req.GetVolumeId() volumeID := req.GetVolumeId()
volumeIDMutex.LockKey(volumeID) 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{} rbdVol := &rbdVolume{}
if err := cs.MetadataStore.Get(volumeID, rbdVol); err != nil { if err := cs.MetadataStore.Get(volumeID, rbdVol); err != nil {
if os.IsNotExist(errors.Cause(err)) { if os.IsNotExist(errors.Cause(err)) {
@ -276,7 +289,12 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
} }
snapshotNameMutex.LockKey(req.GetName()) 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 // Need to check for already existing snapshot name, and if found
// check for the requested source volume id and already allocated source volume id // 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") return nil, status.Error(codes.InvalidArgument, "Snapshot ID cannot be empty")
} }
snapshotIDMutex.LockKey(snapshotID) 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{} rbdSnap := &rbdSnapshot{}
if err := cs.MetadataStore.Get(snapshotID, rbdSnap); err != nil { if err := cs.MetadataStore.Get(snapshotID, rbdSnap); err != nil {

View File

@ -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) { func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
targetPath := req.GetTargetPath() targetPath := req.GetTargetPath()
targetPathMutex.LockKey(targetPath) 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 var volName string
isBlock := req.GetVolumeCapability().GetBlock() != nil isBlock := req.GetVolumeCapability().GetBlock() != nil
@ -84,12 +89,12 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
if os.IsNotExist(err) { if os.IsNotExist(err) {
if isBlock { if isBlock {
// create an empty file // create an empty file
targetPathFile, err := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, 0750) targetPathFile, e := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, 0750)
if err != nil { if e != nil {
glog.V(4).Infof("Failed to create targetPath:%s with error: %v", targetPath, err) 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) glog.V(4).Infof("Failed to close targetPath:%s with error: %v", targetPath, err)
return nil, status.Error(codes.Internal, err.Error()) 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) { func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) {
targetPath := req.GetTargetPath() targetPath := req.GetTargetPath()
targetPathMutex.LockKey(targetPath) 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) notMnt, err := ns.mounter.IsNotMountPoint(targetPath)
if err != nil { 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 // Bind mounted device needs to be resolved by using resolveBindMountedBlockDevice
if devicePath == "devtmpfs" { if devicePath == "devtmpfs" {
var err error
devicePath, err = resolveBindMountedBlockDevice(targetPath) devicePath, err = resolveBindMountedBlockDevice(targetPath)
if err != nil { if err != nil {
return nil, status.Error(codes.Internal, err.Error()) 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 // 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) glog.V(3).Infof("failed to unmap rbd device: %s with error: %v", devicePath, err)
return nil, err return nil, err
} }
// Remove targetPath // 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) glog.V(3).Infof("failed to remove targetPath: %s with error: %v", targetPath, err)
return nil, err return nil, err
} }
@ -240,7 +249,7 @@ func parseFindMntResolveSource(out string) (string, error) {
return match[1], nil return match[1], nil
} }
// Check if out is a block device // Check if out is a block device
reBlk := regexp.MustCompile("^devtmpfs\\[(/[^/]+(?:/[^/]*)*)\\]$") reBlk := regexp.MustCompile(`^devtmpfs\\[(/[^/]+(?:/[^/]*)*)\\]$`)
if match := reBlk.FindStringSubmatch(out); match != nil { if match := reBlk.FindStringSubmatch(out); match != nil {
return fmt.Sprintf("/dev%s", match[1]), nil return fmt.Sprintf("/dev%s", match[1]), nil
} }

View File

@ -112,7 +112,10 @@ func (r *Driver) Run(driverName, nodeID, endpoint string, containerized bool, ca
} }
r.cs = NewControllerServer(r.cd, cachePersister) 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 := csicommon.NewNonBlockingGRPCServer()
s.Start(endpoint, r.ids, r.cs, r.ns) s.Start(endpoint, r.ids, r.cs, r.ns)

View File

@ -229,7 +229,12 @@ func attachRBDImage(volOptions *rbdVolume, userID string, credentials map[string
devicePath, found := waitForPath(volOptions.Pool, image, 1, useNBD) devicePath, found := waitForPath(volOptions.Pool, image, 1, useNBD)
if !found { if !found {
attachdetachMutex.LockKey(imagePath) 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}) _, err = execCommand("modprobe", []string{moduleName})
if err != nil { if err != nil {

View File

@ -73,7 +73,10 @@ func (nc *NodeCache) ForAll(pattern string, destObj interface{}, f ForAllFunc) e
} }
decoder := json.NewDecoder(fp) decoder := json.NewDecoder(fp)
if err = decoder.Decode(destObj); err != nil { 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()) 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 { 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 { if err != nil {
return errors.Wrapf(err, "node-cache: failed to create metadata storage file %s\n", file) 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) encoder := json.NewEncoder(fp)
if err = encoder.Encode(data); err != nil { if err = encoder.Encode(data); err != nil {
return errors.Wrapf(err, "node-cache: failed to encode metadata for file: %s\n", file) 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 { if err != nil {
return errors.Wrapf(err, "node-cache: open error for %s", file) 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) decoder := json.NewDecoder(fp)
if err = decoder.Decode(data); err != nil { if err = decoder.Decode(data); err != nil {