Fix flexvolume volumename issue

[fredkan]

Introduction:

Flexvolume GetVolumeName always calls DefaultPlugin.GetVolumeName() and return the default value.
There are huge amounts of unexpected messages in kubelet logs like:

"kubelet: W0802 19:49:27.913542    7873 plugin-defaults.go:32] flexVolume driver alicloud/disk: using default GetVolumeName for volume d-wz9cy4ofbgt2wqm04ix9"

Issues Analysis:

As the below comments, lots of messages are printed as kubelet log, and real volumename is ignored, default volumename always be the response.

// GetVolumeName is part of the volume.VolumePlugin interface.
func (plugin *flexVolumePlugin) GetVolumeName(spec *volume.Spec) (string, error) {
	call := plugin.NewDriverCall(getVolumeNameCmd)
	call.AppendSpec(spec, plugin.host, nil)

	_, err := call.Run()
	if isCmdNotSupportedErr(err) {
		return (*pluginDefaults)(plugin).GetVolumeName(spec)
	} else if err != nil {
		return "", err
	}

	// Although GetVolumeName is supported in plugin, this will always be called.
	// The expect VolumeName is ignored.
	name, err := (*pluginDefaults)(plugin).GetVolumeName(spec)
	if err != nil {
		return "", err
	}

	// Also, this warning message always be printed, which is not expected.
	klog.Warning(logPrefix(plugin), "GetVolumeName is not supported yet. Defaulting to PV or volume name: ", name)

	return name, nil
}

The unnecessary code should be removed.

release-note

None

What type of PR is this?

/kind bug
/sig storage
/priority important-soon

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

Hi @fredkan. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[fredkan]

@van4th @saad-ali Please help to review this issue.

[gnufied]

/ok-to-test
/assign

[gnufied]

/kind bug

[xianlubird]

/ok-to-test

[fredkan]

one testcase failed, but it seems etcd related reason.

[+]log ok
[-]etcd failed: reason withheld
[+]poststarthook/generic-apiserver-start-informers ok
[+]poststarthook/bootstrap-controller ok
[-]poststarthook/rbac/bootstrap-roles failed: reason withheld
[-]poststarthook/scheduling/bootstrap-system-priority-classes failed: reason withheld
[-]poststarthook/ca-registration failed: reason withheld
healthz check failed

@gnufied @saad-ali @ivan4th please help to view.

[gnufied]

/lgtm

[xing-yang]

/test pull-kubernetes-integration

[gnufied]

/assign @chakri-nelluri

[gnufied]

please add a release-note to the issue and also we tend to use rebase for updating a topic branch against master (not merge).

/lgtm cancel

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[fredkan]

History :) The volume name was supposed to be unique which was tough to be enforced on the driver side and there is no clear mapping from driver volume name to PV name. To avoid this confusion we always defaulted to default volume name.

History, we should always use the default volume name, so it is not necessary to call the plugin GetVolumeName interface.

Just remove the plugin call related code.

[chakri-nelluri]

Please keep this code. Some drivers might be assuming this call and use it for periodic health checks etc and they will be broken. Please keep the change to adjust the verbosity of the log. Thanks.

[fredkan]

Got it，Thanks。

[saad-ali]

/approve

Will let @chakri-nelluri LGTM

[saad-ali]

Why not reduce verbosity of this message instead of removing it altogether?

[fredkan]

Why not reduce verbosity of this message instead of removing it altogether?

There is default volumeName log in GetVolumeName:

klog.V(4).Infof(logPrefix((*flexVolumePlugin)(plugin)), "using default GetVolumeName for volume ", spec.Name())

And, the message "GetVolumeName is not supported yet. Defaulting to PV or volume name: " is not suitable here, this log will be printed out when GetVolumeName is supported by plugin.

[chakri-nelluri]

@fredkan GetVolumeName is not supported on the K8S side, not on the plugin side. I would recommend to just adjust the verbosity of the logs, unless there is a big need to.

[fredkan]

Agree. Thanks.

[chakri-nelluri]

/LGTM
/approve

[xianlubird]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chakri-nelluri, fredkan, saad-ali, xianlubird

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/volume/flexvolume/OWNERS [chakri-nelluri,saad-ali]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[fredkan]

/retest