Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ineffassign fixes for pkg/volume #87711

Merged
merged 1 commit into from Feb 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions hack/.staticcheck_failures
Expand Up @@ -20,7 +20,6 @@ pkg/registry/core/service/storage
pkg/util/coverage
pkg/volume
pkg/volume/azure_dd
pkg/volume/azure_file
pkg/volume/csi
pkg/volume/fc
pkg/volume/flexvolume
Expand All @@ -35,7 +34,6 @@ pkg/volume/storageos
pkg/volume/util
pkg/volume/util/fsquota
pkg/volume/util/fsquota/common
pkg/volume/util/operationexecutor
pkg/volume/util/subpath
pkg/volume/vsphere_volume
test/e2e/apps
Expand Down
8 changes: 6 additions & 2 deletions pkg/volume/azure_file/azure_file.go
Expand Up @@ -114,6 +114,11 @@ func (plugin *azureFilePlugin) newMounterInternal(spec *volume.Spec, pod *v1.Pod
return nil, err
}
secretName, secretNamespace, err := getSecretNameAndNamespace(spec, pod.Namespace)
if err != nil {
// Log-and-continue instead of returning an error for now
// due to unspecified backwards compatibility concerns (a subject to revise)
klog.Errorf("get secret name and namespace from pod(%s) return with error: %v", pod.Name, err)
}
return &azureFileMounter{
azureFile: &azureFile{
volName: spec.Name(),
Expand Down Expand Up @@ -257,15 +262,14 @@ func (b *azureFileMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) e
klog.Errorf("azureFile - Unmount directory %s failed with %v", dir, err)
return err
}
notMnt = true
}

var accountKey, accountName string
if accountName, accountKey, err = b.util.GetAzureCredentials(b.plugin.host, b.secretNamespace, b.secretName); err != nil {
return err
}

mountOptions := []string{}
var mountOptions []string
source := ""
osSeparator := string(os.PathSeparator)
source = fmt.Sprintf("%s%s%s.file.%s%s%s", osSeparator, osSeparator, accountName, getStorageEndpointSuffix(b.plugin.host.GetCloudProvider()), osSeparator, b.shareName)
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/iscsi/iscsi_util.go
Expand Up @@ -362,7 +362,7 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) (string, error) {
}

// in case of node failure/restart, explicitly set to manual login so it doesn't hang on boot
out, err = execWithLog(b, "iscsiadm", "-m", "node", "-p", tp, "-T", b.Iqn, "-o", "update", "-n", "node.startup", "-v", "manual")
_, err = execWithLog(b, "iscsiadm", "-m", "node", "-p", tp, "-T", b.Iqn, "-o", "update", "-n", "node.startup", "-v", "manual")
if err != nil {
// don't fail if we can't set startup mode, but log warning so there is a clue
klog.Warningf("Warning: Failed to set iSCSI login mode to manual. Error: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/rbd/rbd_util.go
Expand Up @@ -355,7 +355,7 @@ func (util *RBDUtil) rbdUnlock(b rbdMounter) error {
if len(locker) > 0 {
args := []string{"lock", "remove", b.Image, lock_id, locker, "--pool", b.Pool, "--id", b.Id, "-m", mon}
args = append(args, secret_opt...)
cmd, err = b.exec.Command("rbd", args...).CombinedOutput()
_, err = b.exec.Command("rbd", args...).CombinedOutput()
if err == nil {
klog.V(4).Infof("rbd: successfully remove lock (locker_id: %s) on image: %s/%s with id %s mon %s", lock_id, b.Pool, b.Image, b.Id, mon)
} else {
Expand Down
6 changes: 3 additions & 3 deletions pkg/volume/util/fsquota/project.go
Expand Up @@ -89,7 +89,7 @@ func openAndLockProjectFiles() (*os.File, *os.File, error) {
fProjid, err := os.OpenFile(projidFile, os.O_RDONLY|os.O_CREATE, 0644)
if err == nil {
// Check once more, to ensure nothing got changed out from under us
if err := projFilesAreOK(); err == nil {
if err = projFilesAreOK(); err == nil {
err = lockFile(fProjects)
if err == nil {
err = lockFile(fProjid)
Expand Down Expand Up @@ -324,7 +324,7 @@ func createProjectID(path string, ID common.QuotaID) (common.QuotaID, error) {
if err == nil {
defer closeProjectFiles(fProjects, fProjid)
list := readProjectFiles(fProjects, fProjid)
writeProjid := true
var writeProjid bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think default value of bool is false

Copy link
Contributor Author

@alena1108 alena1108 Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anderseknert that's correct, but the default value doesn't have any affect here as the writeProjid value is being reset on the next line anyway. So if set to true on line 327, it's an ineffectual assignment that is not being leveraged, and reported as error by ineffassign.

ID, writeProjid, err = addDirToProject(path, ID, &list)
if err == nil && ID != common.BadQuotaID {
if err = writeProjectFiles(fProjects, fProjid, writeProjid, list); err == nil {
Expand All @@ -345,7 +345,7 @@ func removeProjectID(path string, ID common.QuotaID) error {
if err == nil {
defer closeProjectFiles(fProjects, fProjid)
list := readProjectFiles(fProjects, fProjid)
writeProjid := true
var writeProjid bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think default value of bool is false

Copy link
Contributor Author

@alena1108 alena1108 Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anderseknert same as the above. The default value doesn't have any affect here as the writeProjid value is being reset on the next line anyway. So if set to true on line 327, it's an ineffectual assignment that is not being leveraged, and reported as error by ineffassign.

writeProjid, err = removeDirFromProject(path, ID, &list)
if err == nil {
if err = writeProjectFiles(fProjects, fProjid, writeProjid, list); err == nil {
Expand Down
11 changes: 10 additions & 1 deletion pkg/volume/util/fsquota/quota_linux.go
Expand Up @@ -226,6 +226,11 @@ func clearQuotaOnDir(m mount.Interface, path string) error {
// we explicitly have to check in this case.
klog.V(4).Infof("clearQuotaOnDir %s", path)
supportsQuotas, err := SupportsQuotas(m, path)
if err != nil {
// Log-and-continue instead of returning an error for now
// due to unspecified backwards compatibility concerns (a subject to revise)
klog.V(3).Infof("Attempt to check for quota support failed: %v", err)
}
if !supportsQuotas {
return nil
}
Expand Down Expand Up @@ -409,8 +414,12 @@ func ClearQuota(m mount.Interface, path string) error {
if !ok {
return fmt.Errorf("ClearQuota: No quota available for %s", path)
}
var err error
projid, err := getQuotaOnDir(m, path)
if err != nil {
// Log-and-continue instead of returning an error for now
// due to unspecified backwards compatibility concerns (a subject to revise)
klog.V(3).Infof("Attempt to check quota ID %v on dir %s failed: %v", dirQuotaMap[path], path, err)
}
if projid != dirQuotaMap[path] {
return fmt.Errorf("Expected quota ID %v on dir %s does not match actual %v", dirQuotaMap[path], path, projid)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/util/operationexecutor/operation_generator.go
Expand Up @@ -653,7 +653,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
// - Volume does not support DeviceMounter interface.
// - In case of CSI the volume does not have node stage_unstage capability.
if !resizeDone {
resizeDone, resizeError = og.nodeExpandVolume(volumeToMount, resizeOptions)
_, resizeError = og.nodeExpandVolume(volumeToMount, resizeOptions)
if resizeError != nil {
klog.Errorf("MountVolume.NodeExpandVolume failed with %v", resizeError)
return volumeToMount.GenerateError("MountVolume.Setup failed while expanding volume", resizeError)
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/util/subpath/subpath_windows.go
Expand Up @@ -267,7 +267,7 @@ func findExistingPrefix(base, pathname string) (string, []string, error) {

dirs := strings.Split(rel, string(filepath.Separator))

parent := base
var parent string
currentPath := base
for i, dir := range dirs {
parent = currentPath
Expand Down