Repository: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver
Pull request #638 (2020-10-21 20:28:18)

Use existing csi proxy client in Statter
Instead of creating new csi proxy client, this PR uses existing one
created in safe mounter.

<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug
> /kind cleanup
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
> /kind flake

**What this PR does / why we need it**:

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note

```


k8s-ci-robot: @jingxu97: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our [release note process](https://git.k8s.io/community/contributors/guide/release-notes.md) to remove it.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/pull/638#" title="Author self-approved">jingxu97</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fgcp-compute-persistent-disk-csi-driver).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/blob/master/OWNERS)~~ [jingxu97]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @jingxu97: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-gcp-compute-persistent-disk-csi-driver-unit | bed05a18ca481746103b84339e24278fcb7a5bd8 | [link](https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_gcp-compute-persistent-disk-csi-driver/638/pull-gcp-compute-persistent-disk-csi-driver-unit/1319012665179246592/) | `/test pull-gcp-compute-persistent-disk-csi-driver-unit`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=gcp-compute-persistent-disk-csi-driver&pr=638). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Ajingxu97). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @jingxu97: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-gcp-compute-persistent-disk-csi-driver-unit | bed05a18ca481746103b84339e24278fcb7a5bd8 | [link](https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_gcp-compute-persistent-disk-csi-driver/638/pull-gcp-compute-persistent-disk-csi-driver-unit/1319012665179246592/) | `/test pull-gcp-compute-persistent-disk-csi-driver-unit`
pull-gcp-compute-persistent-disk-csi-driver-verify | bed05a18ca481746103b84339e24278fcb7a5bd8 | [link](https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_gcp-compute-persistent-disk-csi-driver/638/pull-gcp-compute-persistent-disk-csi-driver-verify/1319012665200218112/) | `/test pull-gcp-compute-persistent-disk-csi-driver-verify`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=gcp-compute-persistent-disk-csi-driver&pr=638). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Ajingxu97). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go
index 5ca73564..a99e405f 100644
--- a/cmd/gce-pd-csi-driver/main.go
+++ b/cmd/gce-pd-csi-driver/main.go
@@ -96,10 +96,7 @@ func handle() {
 			klog.Fatalf("Failed to get safe mounter: %v", err)
 		}
 		deviceUtils := mountmanager.NewDeviceUtils()
-		statter, err := mountmanager.NewStatter()
-		if err != nil {
-			klog.Fatalf("Failed to set up Statter: %v", err)
-		}
+		statter := mountmanager.NewStatter(mounter)
 		meta, err := metadataservice.NewMetadataService()
 		if err != nil {
 			klog.Fatalf("Failed to set up metadata service: %v", err)
diff --git a/pkg/mount-manager/statter_linux.go b/pkg/mount-manager/statter_linux.go
index 3de01f6e..149f5ecc 100644
--- a/pkg/mount-manager/statter_linux.go
+++ b/pkg/mount-manager/statter_linux.go
@@ -18,19 +18,20 @@ import (
 	"fmt"
 
 	"golang.org/x/sys/unix"
+	"k8s.io/utils/mount"
 )
 
-var _ Statter = realStatter{}
+var _ Statter = &realStatter{}
 
 type realStatter struct {
 }
 
-func NewStatter() (realStatter, error) {
-	return realStatter{}, nil
+func NewStatter(mounter *mount.SafeFormatAndMount) *realStatter {
+	return &realStatter{}
 }
 
 // IsBlock checks if the given path is a block device
-func (realStatter) IsBlockDevice(fullPath string) (bool, error) {
+func (r *realStatter) IsBlockDevice(fullPath string) (bool, error) {
 	var st unix.Stat_t
 	err := unix.Stat(fullPath, &st)
 	if err != nil {
@@ -40,7 +41,7 @@ func (realStatter) IsBlockDevice(fullPath string) (bool, error) {
 	return (st.Mode & unix.S_IFMT) == unix.S_IFBLK, nil
 }
 
-func (realStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
+func (*realStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
 	statfs := &unix.Statfs_t{}
 	err = unix.Statfs(path, statfs)
 	if err != nil {
@@ -62,15 +63,15 @@ func (realStatter) StatFS(path string) (available, capacity, used, inodesFree, i
 
 type fakeStatter struct{}
 
-func NewFakeStatter() fakeStatter {
-	return fakeStatter{}
+func NewFakeStatter(mounter *mount.SafeFormatAndMount) *fakeStatter {
+	return &fakeStatter{}
 }
 
-func (fakeStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
+func (*fakeStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
 	// Assume the file exists and give some dummy values back
 	return 1, 1, 1, 1, 1, 1, nil
 }
 
-func (fakeStatter) IsBlockDevice(fullPath string) (bool, error) {
+func (*fakeStatter) IsBlockDevice(fullPath string) (bool, error) {
 	return false, nil
 }
diff --git a/pkg/mount-manager/statter_windows.go b/pkg/mount-manager/statter_windows.go
index 6674fb36..258a17aa 100644
--- a/pkg/mount-manager/statter_windows.go
+++ b/pkg/mount-manager/statter_windows.go
@@ -17,40 +17,40 @@ package mountmanager
 
 import (
 	"context"
+	"fmt"
 
 	volumeapi "github.com/kubernetes-csi/csi-proxy/client/api/volume/v1beta1"
-	volumeclient "github.com/kubernetes-csi/csi-proxy/client/groups/volume/v1beta1"
+	"k8s.io/utils/mount"
 )
 
-var _ Statter = realStatter{}
+var _ Statter = &realStatter{}
 
 type realStatter struct {
-	VolumeClient *volumeclient.Client
+	mounter *mount.SafeFormatAndMount
 }
 
-func NewStatter() (realStatter, error) {
-	volumeClient, err := volumeclient.NewClient()
-	if err != nil {
-		return realStatter{}, err
-	}
-	return realStatter{
-		VolumeClient: volumeClient,
-	}, nil
+func NewStatter(mounter *mount.SafeFormatAndMount) *realStatter {
+	return &realStatter{mounter: mounter}
 }
 
 // IsBlock checks if the given path is a block device
-func (realStatter) IsBlockDevice(fullPath string) (bool, error) {
+func (r *realStatter) IsBlockDevice(fullPath string) (bool, error) {
 	return false, nil
 }
 
 // StatFS returns volume usage information
-func (r realStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
+func (r *realStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
 	zero := int64(0)
 
+	proxy, ok := r.mounter.Interface.(*CSIProxyMounter)
+	if !ok {
+		return zero, zero, zero, zero, zero, zero, fmt.Errorf("could not cast to csi proxy class")
+	}
+
 	idRequest := &volumeapi.VolumeIDFromMountRequest{
 		Mount: path,
 	}
-	idResponse, err := r.VolumeClient.GetVolumeIDFromMount(context.Background(), idRequest)
+	idResponse, err := proxy.VolumeClient.GetVolumeIDFromMount(context.Background(), idRequest)
 	if err != nil {
 		return zero, zero, zero, zero, zero, zero, err
 	}
@@ -59,7 +59,7 @@ func (r realStatter) StatFS(path string) (available, capacity, used, inodesFree,
 	request := &volumeapi.VolumeStatsRequest{
 		VolumeId: volumeId,
 	}
-	response, err := r.VolumeClient.VolumeStats(context.Background(), request)
+	response, err := proxy.VolumeClient.VolumeStats(context.Background(), request)
 	if err != nil {
 		return zero, zero, zero, zero, zero, zero, err
 	}
@@ -71,15 +71,15 @@ func (r realStatter) StatFS(path string) (available, capacity, used, inodesFree,
 
 type fakeStatter struct{}
 
-func NewFakeStatter() fakeStatter {
-	return fakeStatter{}
+func NewFakeStatter(mounter *mount.SafeFormatAndMount) *fakeStatter {
+	return &fakeStatter{}
 }
 
-func (fakeStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
+func (*fakeStatter) StatFS(path string) (available, capacity, used, inodesFree, inodes, inodesUsed int64, err error) {
 	// Assume the file exists and give some dummy values back
 	return 1, 1, 1, 1, 1, 1, nil
 }
 
-func (fakeStatter) IsBlockDevice(fullPath string) (bool, error) {
+func (*fakeStatter) IsBlockDevice(fullPath string) (bool, error) {
 	return false, nil
 }


Test patch:
diff --git a/pkg/gce-pd-csi-driver/node_test.go b/pkg/gce-pd-csi-driver/node_test.go
index 8ff391b2..221ab04d 100644
--- a/pkg/gce-pd-csi-driver/node_test.go
+++ b/pkg/gce-pd-csi-driver/node_test.go
@@ -42,7 +42,7 @@ func getTestGCEDriverWithCustomMounter(t *testing.T, mounter *mount.SafeFormatAn
 
 func getCustomTestGCEDriver(t *testing.T, mounter *mount.SafeFormatAndMount, deviceUtils mountmanager.DeviceUtils, metaService metadataservice.MetadataService) *GCEDriver {
 	gceDriver := GetGCEDriver()
-	nodeServer := NewNodeServer(gceDriver, mounter, deviceUtils, metaService, mountmanager.NewFakeStatter())
+	nodeServer := NewNodeServer(gceDriver, mounter, deviceUtils, metaService, mountmanager.NewFakeStatter(mounter))
 	err := gceDriver.SetupGCEDriver(driver, "test-vendor", nil, nil, nodeServer)
 	if err != nil {
 		t.Fatalf("Failed to setup GCE Driver: %v", err)
@@ -52,7 +52,8 @@ func getCustomTestGCEDriver(t *testing.T, mounter *mount.SafeFormatAndMount, dev
 
 func getTestBlockingGCEDriver(t *testing.T, readyToExecute chan chan struct{}) *GCEDriver {
 	gceDriver := GetGCEDriver()
-	nodeServer := NewNodeServer(gceDriver, mountmanager.NewFakeSafeBlockingMounter(readyToExecute), mountmanager.NewFakeDeviceUtils(), metadataservice.NewFakeService(), mountmanager.NewFakeStatter())
+	mounter := mountmanager.NewFakeSafeBlockingMounter(readyToExecute)
+	nodeServer := NewNodeServer(gceDriver, mounter, mountmanager.NewFakeDeviceUtils(), metadataservice.NewFakeService(), mountmanager.NewFakeStatter(mounter))
 	err := gceDriver.SetupGCEDriver(driver, "test-vendor", nil, nil, nodeServer)
 	if err != nil {
 		t.Fatalf("Failed to setup GCE Driver: %v", err)
diff --git a/test/sanity/sanity_test.go b/test/sanity/sanity_test.go
index f326d85b..a5d09a35 100644
--- a/test/sanity/sanity_test.go
+++ b/test/sanity/sanity_test.go
@@ -56,7 +56,7 @@ func TestSanity(t *testing.T) {
 	//Initialize GCE Driver
 	identityServer := driver.NewIdentityServer(gceDriver)
 	controllerServer := driver.NewControllerServer(gceDriver, cloudProvider)
-	nodeServer := driver.NewNodeServer(gceDriver, mounter, deviceUtils, metadataservice.NewFakeService(), mountmanager.NewFakeStatter())
+	nodeServer := driver.NewNodeServer(gceDriver, mounter, deviceUtils, metadataservice.NewFakeService(), mountmanager.NewFakeStatter(mounter))
 	err = gceDriver.SetupGCEDriver(driverName, vendorVersion, identityServer, controllerServer, nodeServer)
 	if err != nil {
 		t.Fatalf("Failed to initialize GCE CSI Driver: %v", err)