Re: [kubernetes/kubernetes] Fix local volume directory can't be deleted issue (#65310)

[k8s-ci-robot]

@wenlxie: Reiterating the mentions to trigger a notification:
@kubernetes/sig-storage-pr-reviews

In response to this:

@kubernetes/sig-storage-pr-reviews

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.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[Michelle Au]

@msau42 commented on this pull request.

---

In pkg/volume/local/local.go:

> @@ -169,6 +169,7 @@ func (plugin *localVolumePlugin) NewBlockVolumeUnmapper(volName string,
 
 // TODO: check if no path and no topology constraints are ok
 func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
+	fs := v1.PersistentVolumeFilesystem

I think CSI volume driver also has the same issue. Could you update the csi plugin too?

@saad-ali this is another instance of #52820

[wenlxie]

@wenlxie commented on this pull request.

---

In pkg/volume/local/local.go:

> @@ -169,6 +169,7 @@ func (plugin *localVolumePlugin) NewBlockVolumeUnmapper(volName string,
 
 // TODO: check if no path and no topology constraints are ok
 func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
+	fs := v1.PersistentVolumeFilesystem

@msau42 Thanks for review this.
CSI plugin also updated: #65456

[Michelle Au]

@msau42 commented on this pull request.

---

In pkg/volume/local/local.go:

> @@ -169,6 +169,7 @@ func (plugin *localVolumePlugin) NewBlockVolumeUnmapper(volName string,
 
 // TODO: check if no path and no topology constraints are ok
 func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
+	fs := v1.PersistentVolumeFilesystem

Can you also add a unit test for this?

[wenlxie]

@wenlxie commented on this pull request.

---

In pkg/volume/local/local.go:

> @@ -169,6 +169,7 @@ func (plugin *localVolumePlugin) NewBlockVolumeUnmapper(volName string,
 
 // TODO: check if no path and no topology constraints are ok
 func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
+	fs := v1.PersistentVolumeFilesystem

@msau42 unit test added.

[Michelle Au]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, wenlxie

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/local/OWNERS [msau42]

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

[Kubernetes Submit Queue]

Automatic merge from submit-queue (batch tested with PRs 65582, 65480, 65310, 65644, 65645). If you want to cherry-pick this change to another branch, please follow the instructions here.

[Kubernetes Submit Queue]

Merged #65310.