search

kubernetes-sigs / vsphere-csi-driver

vSphere storage Container Storage Interface (CSI) plugin
https://docs.vmware.com/en/VMware-vSphere-Container-Storage-Plug-in/index.html
Apache License 2.0
293 stars 177 forks source link

Change VM Op VM Update call to Patch #2833

Closed akutz closed 5 months ago

akutz commented 5 months ago

What this PR does / why we need it:

This patch updates the controller that publishes volumes to VM Operator VMs to use a Patch API instead of the Kube Update API. This ensures fewer resource conflicts and also prevents potential issues that occur when additive changes are made within the same API version.

cc @bryanv

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes NA

Testing done:

$ go test -v -count 1 ./pkg/csi/service/wcpguest/
=== RUN   TestGuestClusterControllerFlow
{"level":"error","time":"2024-03-20T12:00:08.031235-05:00","caller":"config/config.go:648","msg":"no Supervisor Cluster endpoint defined in Guest Cluster config","stacktrace":"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config.validateGCConfig\n\t/Users/akutz/Projects/vsphere-csi-driver/pkg/common/config/config.go:648\nsigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config.FromEnvToGC\n\t/Users/akutz/Projects/vsphere-csi-driver/pkg/common/config/config.go:588\nsigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/wcpguest.configFromEnvOrSim\n\t/Users/akutz/Projects/vsphere-csi-driver/pkg/csi/service/wcpguest/controller_test.go:72\nsigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/wcpguest.getControllerTest.func1\n\t/Users/akutz/Projects/vsphere-csi-driver/pkg/csi/service/wcpguest/controller_test.go:88\nsync.(*Once).doSlow\n\t/Users/akutz/.go/active/src/sync/once.go:74\nsync.(*Once).Do\n\t/Users/akutz/.go/active/src/sync/once.go:65\nsigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/wcpguest.getControllerTest\n\t/Users/akutz/Projects/vsphere-csi-driver/pkg/csi/service/wcpguest/controller_test.go:85\nsigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/wcpguest.TestGuestClusterControllerFlow\n\t/Users/akutz/Projects/vsphere-csi-driver/pkg/csi/service/wcpguest/controller_test.go:121\ntesting.tRunner\n\t/Users/akutz/.go/active/src/testing/testing.go:1595"}
{"level":"info","time":"2024-03-20T12:00:08.032101-05:00","caller":"wcpguest/controller.go:249","msg":"CreateVolume: called with args {Name:pvc-12345 CapacityRange:required_bytes:1073741824  VolumeCapabilities:[access_mode:<mode:SINGLE_NODE_WRITER > ] Parameters:map[svstorageclass:test-storageclass] Secrets:map[] VolumeContentSource:<nil> AccessibilityRequirements:<nil> MutableParameters:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}","TraceId":"4b7fbc65-37c5-48b8-b525-7772becdc199"}
{"level":"info","time":"2024-03-20T12:00:08.032467-05:00","caller":"wcpguest/controller_helper.go:309","msg":"Waiting up to 240 seconds for PersistentVolumeClaim -12345 in namespace test-namespace to have phase Bound","TraceId":"4b7fbc65-37c5-48b8-b525-7772becdc199"}
{"level":"info","time":"2024-03-20T12:00:09.032835-05:00","caller":"wcpguest/controller_helper.go:333","msg":"PersistentVolumeClaim -12345 in namespace test-namespace is in state Bound","TraceId":"4b7fbc65-37c5-48b8-b525-7772becdc199"}
{"level":"info","time":"2024-03-20T12:00:09.032921-05:00","caller":"wcpguest/controller.go:442","msg":"Volume created successfully. Volume Handle: \"-12345\", PV Name: \"pvc-12345\"","TraceId":"4b7fbc65-37c5-48b8-b525-7772becdc199"}
{"level":"info","time":"2024-03-20T12:00:09.033155-05:00","caller":"wcpguest/controller.go:461","msg":"DeleteVolume: called with args: {VolumeId:-12345 Secrets:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}","TraceId":"79224ffa-c4ee-473c-a714-c2d2c882b514"}
{"level":"info","time":"2024-03-20T12:00:09.033241-05:00","caller":"wcpguest/controller.go:506","msg":"DeleteVolume: Volume deleted successfully. VolumeID: \"-12345\"","TraceId":"79224ffa-c4ee-473c-a714-c2d2c882b514"}
{"level":"info","time":"2024-03-20T12:00:09.033448-05:00","caller":"wcpguest/controller.go:521","msg":"Volume \"-12345\" deleted successfully.","TraceId":"79224ffa-c4ee-473c-a714-c2d2c882b514"}
{"level":"info","time":"2024-03-20T12:00:10.037029-05:00","caller":"wcpguest/controller.go:249","msg":"CreateVolume: called with args {Name:pvc-12345 CapacityRange:required_bytes:1073741824  VolumeCapabilities:[access_mode:<mode:MULTI_NODE_MULTI_WRITER > ] Parameters:map[svstorageclass:test-storageclass] Secrets:map[] VolumeContentSource:<nil> AccessibilityRequirements:<nil> MutableParameters:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}","TraceId":"49453cba-a45b-4707-9ca2-3a88cdde4ada"}
{"level":"info","time":"2024-03-20T12:00:10.037331-05:00","caller":"wcpguest/controller_helper.go:309","msg":"Waiting up to 240 seconds for PersistentVolumeClaim -12345 in namespace test-namespace to have phase Bound","TraceId":"49453cba-a45b-4707-9ca2-3a88cdde4ada"}
{"level":"info","time":"2024-03-20T12:00:11.034688-05:00","caller":"wcpguest/controller_helper.go:333","msg":"PersistentVolumeClaim -12345 in namespace test-namespace is in state Bound","TraceId":"49453cba-a45b-4707-9ca2-3a88cdde4ada"}
{"level":"info","time":"2024-03-20T12:00:11.034888-05:00","caller":"wcpguest/controller.go:442","msg":"Volume created successfully. Volume Handle: \"-12345\", PV Name: \"pvc-12345\"","TraceId":"49453cba-a45b-4707-9ca2-3a88cdde4ada"}
{"level":"info","time":"2024-03-20T12:00:11.035201-05:00","caller":"wcpguest/controller.go:461","msg":"DeleteVolume: called with args: {VolumeId:-12345 Secrets:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}","TraceId":"a0f98918-e3fa-4fdd-aae2-661277f51043"}
{"level":"info","time":"2024-03-20T12:00:11.035252-05:00","caller":"wcpguest/controller.go:506","msg":"DeleteVolume: Volume deleted successfully. VolumeID: \"-12345\"","TraceId":"a0f98918-e3fa-4fdd-aae2-661277f51043"}
{"level":"info","time":"2024-03-20T12:00:11.035272-05:00","caller":"wcpguest/controller.go:521","msg":"Volume \"-12345\" deleted successfully.","TraceId":"a0f98918-e3fa-4fdd-aae2-661277f51043"}
--- PASS: TestGuestClusterControllerFlow (4.01s)
=== RUN   TestGuestClusterControllerFlowForTkgsHA
{"level":"info","time":"2024-03-20T12:00:12.037833-05:00","caller":"wcpguest/controller.go:249","msg":"CreateVolume: called with args {Name:pvc-12345 CapacityRange:required_bytes:1073741824  VolumeCapabilities:[access_mode:<mode:SINGLE_NODE_WRITER > ] Parameters:map[svstorageclass:test-storageclass] Secrets:map[] VolumeContentSource:<nil> AccessibilityRequirements:requisite:<segments:<key:\"R1\" value:\"Zone1\" > > preferred:<segments:<key:\"R1\" value:\"Zone1\" > >  MutableParameters:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}","TraceId":"17ec9b2f-c6be-46f5-99ac-f6f41e278a0f"}
{"level":"info","time":"2024-03-20T12:00:12.03824-05:00","caller":"wcpguest/controller_helper.go:309","msg":"Waiting up to 240 seconds for PersistentVolumeClaim -12345 in namespace test-namespace to have phase Bound","TraceId":"17ec9b2f-c6be-46f5-99ac-f6f41e278a0f"}
{"level":"info","time":"2024-03-20T12:00:13.038436-05:00","caller":"wcpguest/controller_helper.go:333","msg":"PersistentVolumeClaim -12345 in namespace test-namespace is in state Bound","TraceId":"17ec9b2f-c6be-46f5-99ac-f6f41e278a0f"}
{"level":"info","time":"2024-03-20T12:00:13.03863-05:00","caller":"wcpguest/controller.go:417","msg":"Volume \"-12345\" created is accessible from zones: [map[R1:Zone1]]","TraceId":"17ec9b2f-c6be-46f5-99ac-f6f41e278a0f"}
{"level":"info","time":"2024-03-20T12:00:13.038664-05:00","caller":"wcpguest/controller.go:442","msg":"Volume created successfully. Volume Handle: \"-12345\", PV Name: \"pvc-12345\"","TraceId":"17ec9b2f-c6be-46f5-99ac-f6f41e278a0f"}
    controller_test.go:291: AccessibleTopology was correctly set in create volume response
{"level":"info","time":"2024-03-20T12:00:13.039245-05:00","caller":"wcpguest/controller.go:461","msg":"DeleteVolume: called with args: {VolumeId:-12345 Secrets:map[] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}","TraceId":"8e8ac5fd-df17-4c73-bc50-26c6a890ea04"}
{"level":"info","time":"2024-03-20T12:00:13.039461-05:00","caller":"wcpguest/controller.go:506","msg":"DeleteVolume: Volume deleted successfully. VolumeID: \"-12345\"","TraceId":"8e8ac5fd-df17-4c73-bc50-26c6a890ea04"}
{"level":"info","time":"2024-03-20T12:00:13.039479-05:00","caller":"wcpguest/controller.go:521","msg":"Volume \"-12345\" deleted successfully.","TraceId":"8e8ac5fd-df17-4c73-bc50-26c6a890ea04"}
--- PASS: TestGuestClusterControllerFlowForTkgsHA (2.00s)
=== RUN   TestGenerateVolumeAccessibleTopologyFromPVCAnnotation
    controller_test.go:347: Calling generateVolumeAccessibleTopologyFromPVCAnnotation with csi.vsphere.volume-accessible-topology annotation value "[{\"topology.kubernetes.io/zone\":\"zone1\"},{\"topology.kubernetes.io/zone\":\"zone2\"}]"
    controller_test.go:365: accessibleTopologies [map[topology.kubernetes.io/zone:zone1] map[topology.kubernetes.io/zone:zone2]] match with expectedAccessibleTopologies: [map[topology.kubernetes.io/zone:zone1] map[topology.kubernetes.io/zone:zone2]]
--- PASS: TestGenerateVolumeAccessibleTopologyFromPVCAnnotation (0.00s)
=== RUN   TestGenerateVolumeAccessibleTopologyFromInvalidPVCAnnotation
    controller_test.go:382: Calling generateVolumeAccessibleTopologyFromPVCAnnotation with csi.vsphere.volume-accessible-topology annotation value "[{\"topology.kubernetes.io/zone\":\"zone1\",{\"topology.kubernetes.io/zone\":\"zone2\"}]"
    controller_test.go:391: expected error: failed to parse annotation: "csi.vsphere.volume-accessible-topology" value [{"topology.kubernetes.io/zone":"zone1",{"topology.kubernetes.io/zone":"zone2"}] from the claim: "name", namespace: "ns". err: invalid character '{' looking for beginning of object key string
--- PASS: TestGenerateVolumeAccessibleTopologyFromInvalidPVCAnnotation (0.00s)
=== RUN   TestGenerateGuestClusterRequestedTopologyJSON
    controller_test.go:404: Calling generateGuestClusterRequestedTopologyJSON with topologies: [segments:<key:"topology.kubernetes.io/zone" value:"zone1" >  segments:<key:"topology.kubernetes.io/zone" value:"zone2" > ]
    controller_test.go:416: volumeAccessibleTopologyJSON [{"topology.kubernetes.io/zone":"zone1"},{"topology.kubernetes.io/zone":"zone2"}] match with expectedVolumeAccessibleTopologyJSON: [{"topology.kubernetes.io/zone":"zone1"},{"topology.kubernetes.io/zone":"zone2"}]
--- PASS: TestGenerateGuestClusterRequestedTopologyJSON (0.00s)
PASS
ok      sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/wcpguest  6.694s

Special notes for your reviewer:

Release note:

Updates the wcpguest controller to use Patch semantics when publishing volume updates to a VM
divyenpatel commented 5 months ago

/ok-to-test

svcbot-qecnsdp commented 5 months ago

Started GC block pre-checkin pipeline... Build Number: 908

svcbot-qecnsdp commented 5 months ago 
Build ID: 908
GC build status: FAILURE 
Stage before exit: checkout
svcbot-qecnsdp commented 5 months ago

Started GC block pre-checkin pipeline... Build Number: 909

svcbot-qecnsdp commented 5 months ago 
Build ID: 909
GC build status: FAILURE 
Stage before exit: checkout
k8s-ci-robot commented 5 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akutz, divyenpatel

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](https://github.com/kubernetes-sigs/vsphere-csi-driver/blob/master/OWNERS)~~ [divyenpatel] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment