Update to csi-proxy v1 APIs #966

wongma7 · 2021-07-06T22:08:43Z

Is this a bug fix or adding new feature? feature

What is this PR about? / Why do we need it? https://github.com/kubernetes-csi/csi-proxy/releases/tag/client%2Fv1.0.0-rc.1 csi-proxy GA is coming , need to test it

Changes are mostly straightforward renaming/refactoring.

Plus DiskNumber is treated as uint32 now instead of string by csi-proxy, so there is some conversion happening, but it should be safe in any case:

First we call ListDiskIDs which returns map[uint32]*DiskIDs where the uint32 is diskNumber. Then we convert it to a string since findDevicePath ought to return a string. Later we pass this string to FormatAndMount which converts it back to a uint32 in order to pass it to csi-proxy APIs PartitionDisk and ListVolumesOnDisk.

ListDiskIDs for reference: https://github.com/kubernetes-csi/csi-proxy/blob/d2c1b5469ddc8718c8d2ea679af074db620c9b05/client/api/disk/v1/api.pb.go#L441-L448

TODO

update paths in windows daemonset
update example (README)
test it

What testing is done?
I will manually test exactly the same way I tested the beta API implementation: #823:
build and push to my own registry.

$ docker buildx build --platform windows -f Dockerfile.windows . --push -t public.ecr.aws/b5w6x5z2/aws-ebs-csi-driver:windows

create cluster with windows node. edit windows daemonset to say image: public.ecr.aws/b5w6x5z2/aws-ebs-csi-driver:windows

$ k apply -f examples/kubernetes/windows/specs/windows.yaml
$ k logs ebs-csi-node-windows-d2jtg  -n kube-system ebs-plugin
I0708 19:45:33.558151    1764 driver.go:72] Driver: ebs.csi.aws.com Version: v1.0.0
I0708 19:45:33.834126    1764 node.go:85] [Debug] Retrieving node info from metadata service
I0708 19:45:33.834126    1764 metadata.go:101] retrieving instance data from ec2 metadata
I0708 19:45:33.893528    1764 metadata.go:108] ec2 metadata is available
I0708 19:45:34.597172    1764 driver.go:142] Listening for connections on address: &net.UnixAddr{Name:"\\csi\\csi.sock", Net:"unix"}
I0708 19:45:36.670941    1764 node.go:509] NodeGetInfo: called with args {XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:47:01.507998    1764 node.go:493] NodeGetCapabilities: called with args {XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:47:01.521313    1764 node.go:105] NodeStageVolume: called with args {VolumeId:vol-0975d9f262cf3e30c PublishContext:map[devicePath:/dev/xvdba] StagingTargetPath:\var\lib\kubelet\plugins\kubernetes.io\csi\pv\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\globalmount VolumeCapability:mount:<fs_type:"ntfs" > access_mode:<mode:SINGLE_NODE_WRITER >  Secrets:map[] VolumeContext:map[storage.kubernetes.io/csiProvisionerIdentity:1625768702119-8081-ebs.csi.aws.com] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:47:04.927708    1764 node.go:174] NodeStageVolume: find device path /dev/xvdba -> 5
I0708 19:47:05.274980    1764 node.go:208] NodeStageVolume: formatting 5 and mounting at \var\lib\kubelet\plugins\kubernetes.io\csi\pv\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\globalmount with fstype ntfs
I0708 19:47:23.473123    1764 node.go:152] NodeStageVolume: volume="vol-0975d9f262cf3e30c" operation finished
I0708 19:47:23.474855    1764 inflight.go:73] Node Service: volume="vol-0975d9f262cf3e30c" operation finished
I0708 19:47:23.489487    1764 node.go:493] NodeGetCapabilities: called with args {XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:47:23.499828    1764 node.go:349] NodePublishVolume: called with args {VolumeId:vol-0975d9f262cf3e30c PublishContext:map[devicePath:/dev/xvdba] StagingTargetPath:\var\lib\kubelet\plugins\kubernetes.io\csi\pv\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\globalmount TargetPath:c:\var\lib\kubelet\pods\5bea126c-cb4c-4dc0-92d8-934c29546ebf\volumes\kubernetes.io~csi\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\mount VolumeCapability:mount:<fs_type:"ntfs" > access_mode:<mode:SINGLE_NODE_WRITER >  Readonly:false Secrets:map[] VolumeContext:map[storage.kubernetes.io/csiProvisionerIdentity:1625768702119-8081-ebs.csi.aws.com] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:47:23.499828    1764 node_windows.go:73] NodePublishVolume: removing dir c:\var\lib\kubelet\pods\5bea126c-cb4c-4dc0-92d8-934c29546ebf\volumes\kubernetes.io~csi\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\mount
I0708 19:47:23.501413    1764 node.go:622] NodePublishVolume: mounting \var\lib\kubelet\plugins\kubernetes.io\csi\pv\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\globalmount at c:\var\lib\kubelet\pods\5bea126c-cb4c-4dc0-92d8-934c29546ebf\volumes\kubernetes.io~csi\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\mount with option [bind] as fstype ntfs
I0708 19:47:23.502044    1764 node.go:378] NodePublishVolume: volume="vol-0975d9f262cf3e30c" operation finished
I0708 19:47:23.502044    1764 inflight.go:73] Node Service: volume="vol-0975d9f262cf3e30c" operation finished
# exec pod 1
PS C:\> New-Item -Path data -Name "testfile1.txt" -ItemType "file" -Value "Thi
s 
>> is a text string." 


    Directory: C:\data 


Mode                LastWriteTime         Length Name
----                -------------         ------ ----
-a----         7/8/2021   7:48 PM             23 testfile1.txt

# exec pod 2
PS C:\> ls data 


    Directory: C:\data 


Mode                LastWriteTime         Length Name
----                -------------         ------ ----
-a----         7/8/2021   7:48 PM             23 testfile1.txt
$ k delete -f examples/kubernetes/windows/specs/windows.yaml

$ k logs ebs-csi-node-windows-d2jtg  -n kube-system ebs-plugin
...
I0708 19:53:20.502629    1764 node.go:402] NodeUnpublishVolume: called with args {VolumeId:vol-0975d9f262cf3e30c TargetPath:c:\var\lib\kubelet\pods\b1026ec4-14d1-4f9e-96ea-f04e1524e39d\volumes\kubernetes.io~csi\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\mount XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:53:20.502629    1764 node.go:420] NodeUnpublishVolume: unmounting c:\var\lib\kubelet\pods\b1026ec4-14d1-4f9e-96ea-f04e1524e39d\volumes\kubernetes.io~csi\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\mount
I0708 19:53:22.861461    1764 node.go:416] NodeUnPublishVolume: volume="vol-0975d9f262cf3e30c" operation finished
I0708 19:53:22.861461    1764 inflight.go:73] Node Service: volume="vol-0975d9f262cf3e30c" operation finished
I0708 19:53:22.944595    1764 node.go:493] NodeGetCapabilities: called with args {XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:53:22.946714    1764 node.go:231] NodeUnstageVolume: called with args {VolumeId:vol-0975d9f262cf3e30c StagingTargetPath:\var\lib\kubelet\plugins\kubernetes.io\csi\pv\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\globalmount XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:53:23.207954    1764 node.go:271] NodeUnstageVolume: unmounting \var\lib\kubelet\plugins\kubernetes.io\csi\pv\pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7\globalmount
I0708 19:53:25.204790    1764 node.go:246] NodeUnStageVolume: volume="vol-0975d9f262cf3e30c" operation finished
I0708 19:53:25.204790    1764 inflight.go:73] Node Service: volume="vol-0975d9f262cf3e30c" operation finished

# controller logs showing create, attach, detach, delete.
$ k logs ebs-csi-controller-758d86d7c7-kfzgj -n kube-system ebs-plugin
I0708 19:46:48.657529       1 controller.go:101] CreateVolume: called with args {Name:pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7 CapacityRange:required_bytes:4294967296  VolumeCapabilities:[mount:<fs_type:"ntfs" > access_mode:<mode:SINGLE_NODE_WRITER > ] Parameters:map[csi.storage.k8s.io/pv/name:pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7 csi.storage.k8s.io/pvc/name:ebs-claim csi.storage.k8s.io/pvc/namespace:default fstype:ntfs] Secrets:map[] VolumeContentSource:<nil> AccessibilityRequirements:requisite:<segments:<key:"topology.ebs.csi.aws.com/zone" value:"us-west-2a" > > requisite:<segments:<key:"topology.ebs.csi.aws.com/zone" value:"us-west-2b" > > preferred:<segments:<key:"topology.ebs.csi.aws.com/zone" value:"us-west-2a" > > preferred:<segments:<key:"topology.ebs.csi.aws.com/zone" value:"us-west-2b" > >  XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
W0708 19:46:49.685586       1 controller.go:148] "fstype" is deprecated, please use "csi.storage.k8s.io/fstype" instead
I0708 19:46:53.089717       1 inflight.go:73] Node Service: volume="pvc-c8b0e0c2-1b5d-4e55-90d3-1716cd239ef7" operation finished
I0708 19:46:53.779271       1 controller.go:309] ControllerPublishVolume: called with args {VolumeId:vol-0975d9f262cf3e30c NodeId:i-05af83d6f8fe847b7 VolumeCapability:mount:<fs_type:"ntfs" > access_mode:<mode:SINGLE_NODE_WRITER >  Readonly:false Secrets:map[] VolumeContext:map[storage.kubernetes.io/csiProvisionerIdentity:1625768702119-8081-ebs.csi.aws.com] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:46:54.475382       1 cloud.go:421] [Debug] AttachVolume volume="vol-0975d9f262cf3e30c" instance="i-05af83d6f8fe847b7" request returned {
  AttachTime: 2021-07-08 19:46:54.461 +0000 UTC,
  Device: "/dev/xvdba",
  InstanceId: "i-05af83d6f8fe847b7",
  State: "attaching",
  VolumeId: "vol-0975d9f262cf3e30c"
}
I0708 19:46:54.598678       1 cloud.go:594] Waiting for volume "vol-0975d9f262cf3e30c" state: actual=attaching, desired=attached
I0708 19:46:55.736791       1 manager.go:197] [Debug] Releasing in-process attachment entry: /dev/xvdba -> volume vol-0975d9f262cf3e30c
I0708 19:46:55.736817       1 controller.go:337] [Debug] ControllerPublishVolume: volume vol-0975d9f262cf3e30c attached to node i-05af83d6f8fe847b7 through device /dev/xvdba
I0708 19:53:30.158534       1 controller.go:368] ControllerUnpublishVolume: called with args {VolumeId:vol-0975d9f262cf3e30c NodeId:i-05af83d6f8fe847b7 Secrets:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:53:30.657387       1 cloud.go:594] Waiting for volume "vol-0975d9f262cf3e30c" state: actual=detaching, desired=detached
I0708 19:53:31.754543       1 cloud.go:594] Waiting for volume "vol-0975d9f262cf3e30c" state: actual=detaching, desired=detached
I0708 19:53:33.643252       1 cloud.go:594] Waiting for volume "vol-0975d9f262cf3e30c" state: actual=detaching, desired=detached
I0708 19:53:36.952000       1 controller.go:382] [Debug] ControllerUnpublishVolume: volume vol-0975d9f262cf3e30c detached from node i-05af83d6f8fe847b7
I0708 19:53:37.457560       1 controller.go:276] DeleteVolume: called with args: {VolumeId:vol-0975d9f262cf3e30c Secrets:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}
I0708 19:53:37.570950       1 inflight.go:73] Node Service: volume="vol-0975d9f262cf3e30c" operation finished

k8s-ci-robot · 2021-07-06T22:08:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wongma7

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:

~~OWNERS~~ [wongma7]

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

deploy/kubernetes/base/controller.yaml

wongma7 · 2021-07-09T00:24:09Z

/hold

it's ready for review but #965 should merge first and I'll rebase.

…csi-proxy/driver

…group

wongma7 · 2021-07-13T18:14:13Z

/hold cancel

nckturner · 2021-07-13T20:12:37Z

pkg/driver/node_windows.go

 		cleanVolumeID := strings.ReplaceAll(volumeID, "-", "")
 		if strings.Contains(serialNumber, cleanVolumeID) {
-			foundDiskNumber = diskNumber
+			foundDiskNumber = strconv.Itoa(int(diskNumber))


Was the prior behavior broken?

previously the map was map[string], now it's map[uint32]

nckturner · 2021-07-13T20:13:56Z

pkg/mounter/safe_mounter_windows.go

-		Path:    normalizeWindowsPath(path),
-		Context: fs.PathContext_POD,
-		Force:   true,
+		Path:  normalizeWindowsPath(path),


What's the reason the Context field is no longer set?

it doesn't exist in API anymore, there is just one path versus having pod/plugin path being exactly the same kubernetes-csi/csi-proxy#149

nckturner · 2021-07-13T20:15:15Z

pkg/mounter/safe_mounter_windows.go

@@ -238,26 +237,31 @@ func (mounter *CSIProxyMounter) FindDiskByLun(lun string) (diskNum string, err e
 	// If match is found then return back the disk number.
 	for diskID, location := range findDiskByLunResponse.DiskLocations {
 		if strings.EqualFold(location.LUNID, lun) {
-			return diskID, nil
+			return strconv.Itoa(int(diskID)), nil


nit: small utility func for the conversion.

which part, the for loop?

btw this FindDiskByLun function is copy/pasted to satisfy mounter interface , we don't use it anywhere.

nckturner · 2021-07-13T20:28:46Z

charts/aws-ebs-csi-driver/templates/node-windows.yaml

          volumeMounts:
            - name: kubelet-dir
              mountPath: C:\var\lib\kubelet
              mountPropagation: "None"
            - name: plugin-dir
              mountPath: C:\csi
            - name: csi-proxy-disk-pipe
-              mountPath: \\.\pipe\csi-proxy-disk-v1beta2
+              mountPath: \\.\pipe\csi-proxy-disk-v1


Why does the mount path have a version in it?

that is the way csi-proxy works, it drops a socket at a path for each api it serves

nckturner · 2021-07-13T21:47:30Z

/lgtm

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 6, 2021

k8s-ci-robot requested review from ayberk and bertinatto July 6, 2021 22:08

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 6, 2021

wongma7 commented Jul 8, 2021

View reviewed changes

deploy/kubernetes/base/controller.yaml Outdated Show resolved Hide resolved

wongma7 force-pushed the csiproxyv1 branch from d4478a6 to 83abc35 Compare July 8, 2021 20:00

wongma7 changed the title ~~WIP: Update to csi-proxy v1 APIs~~ Update to csi-proxy v1 APIs Jul 8, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2021

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 9, 2021

wongma7 added 6 commits July 13, 2021 11:04

Update to csi-proxy v1.0.0-rc.1, run go mod tidy && go mod vendor

6d89302

Update to csi-proxy v1 APIs

7d9d6c7

Update windows daemonset to csiproxy v1 paths

a2261de

Update windows example prerequisites to latest (planned) versions of …

4aef531

…csi-proxy/driver

Add WINDOWS bool to hack/e2e/run.sh for creating eksctl windows node …

d1416bc

…group

Update windows daemonset to better match helm chart v2 linux daemonset

9c98a43

wongma7 force-pushed the csiproxyv1 branch from 83abc35 to 9c98a43 Compare July 13, 2021 18:10

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2021

nckturner reviewed Jul 13, 2021

View reviewed changes

k8s-ci-robot assigned nckturner Jul 13, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 13, 2021

k8s-ci-robot merged commit 6b60177 into kubernetes-sigs:master Jul 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to csi-proxy v1 APIs #966

Update to csi-proxy v1 APIs #966

wongma7 commented Jul 6, 2021 •

edited

Loading

k8s-ci-robot commented Jul 6, 2021

wongma7 commented Jul 9, 2021

wongma7 commented Jul 13, 2021

nckturner Jul 13, 2021

wongma7 Jul 13, 2021

wongma7 Jul 13, 2021

nckturner Jul 13, 2021

wongma7 Jul 13, 2021

nckturner Jul 13, 2021

wongma7 Jul 13, 2021

wongma7 Jul 13, 2021

nckturner Jul 13, 2021

wongma7 Jul 13, 2021

nckturner commented Jul 13, 2021

Update to csi-proxy v1 APIs #966

Update to csi-proxy v1 APIs #966

Conversation

wongma7 commented Jul 6, 2021 • edited Loading

k8s-ci-robot commented Jul 6, 2021

wongma7 commented Jul 9, 2021

wongma7 commented Jul 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nckturner commented Jul 13, 2021

wongma7 commented Jul 6, 2021 •

edited

Loading