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
288 stars 173 forks source link

fix continues update volume metadata calls for TKC volumes #2909

Closed divyenpatel closed 1 month ago

divyenpatel commented 1 month ago

What this PR does / why we need it: We have a bug in the vSphere CSI Driver full sync logic for TKC volumes. During each full sync, the update volume metadata API is called unnecessarily for every TKC volume, even when there is no metadata to update.

The root cause of this issue lies in the isUpdateRequired function, which checks whether the ClusterDistribution is set on the volume. This check was introduced to populate the ClusterDistribution field for vSphere 7.0u2 and above. ClusterDistribution is incorrectly considered unset when the ContainerClusterArray contains two entry, which is consistently the case for TKC volumes. As a result, the UpdateVolume metadata API is invoked for every TKC volume during each full sync cycle.

This PR is fixing full sync in the supervisor cluster for TKC volumes for which continues update volume metadata calls were observed.

Testing done: Unit tests result

%  go test -v -run TestHasClusterDistributionSet              
=== RUN   TestHasClusterDistributionSet
=== RUN   TestHasClusterDistributionSet/Cluster_distribution_matches
=== RUN   TestHasClusterDistributionSet/Cluster_distribution_does_not_set
=== RUN   TestHasClusterDistributionSet/#00
=== RUN   TestHasClusterDistributionSet/No_container_clusters_in_metadata
--- PASS: TestHasClusterDistributionSet (0.00s)
    --- PASS: TestHasClusterDistributionSet/Cluster_distribution_matches (0.00s)
    --- PASS: TestHasClusterDistributionSet/Cluster_distribution_does_not_set (0.00s)
    --- PASS: TestHasClusterDistributionSet/#00 (0.00s)
    --- PASS: TestHasClusterDistributionSet/No_container_clusters_in_metadata (0.00s)
PASS
ok      sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer    1.203s

Verified full log is no longer observing update required for the TKC volumes: b5a149bd-b566-4426-91da-e5ba236b5fbd, 81e34368-bc19-46ac-ad0c-b528c8ffb4a3 and 423f2387-fe31-4503-8c6d-9657bfb2f313

Logs after the fix

2024-05-31T17:56:21.404Z    INFO    syncer/fullsync.go:861  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "b5a149bd-b566-4426-91da-e5ba236b5fbd"  {"TraceId": "2e34706d-8748-48d7-bf6b-a93a421c7201"}
2024-05-31T17:56:21.405Z    INFO    syncer/fullsync.go:861  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "81e34368-bc19-46ac-ad0c-b528c8ffb4a3"  {"TraceId": "2e34706d-8748-48d7-bf6b-a93a421c7201"}
2024-05-31T17:56:21.405Z    INFO    syncer/fullsync.go:861  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "423f2387-fe31-4503-8c6d-9657bfb2f313"  {"TraceId": "2e34706d-8748-48d7-bf6b-a93a421c7201"}
2024-05-31T17:56:21.406Z    INFO    syncer/fullsync.go:861  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "76b98441-192d-4ef8-b376-6100dabacf9c"  {"TraceId": "2e34706d-8748-48d7-bf6b-a93a421c7201"}
2024-05-31T17:56:21.406Z    INFO    syncer/fullsync.go:861  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "235b16cf-d0cd-4534-8580-76067cdb7f61"  {"TraceId": "2e34706d-8748-48d7-bf6b-a93a421c7201"}

Logs before the fix

2024-05-30T21:17:21.462Z    INFO    syncer/fullsync.go:864  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is required for volume: "b5a149bd-b566-4426-91da-e5ba236b5fbd"  {"TraceId": "febe5fd3-f694-4f34-9c37-fcaf515aaecb"}
2024-05-30T21:17:21.478Z    INFO    syncer/fullsync.go:864  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is required for volume: "81e34368-bc19-46ac-ad0c-b528c8ffb4a3"  {"TraceId": "febe5fd3-f694-4f34-9c37-fcaf515aaecb"}
2024-05-30T21:17:21.478Z    INFO    syncer/fullsync.go:864  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is required for volume: "423f2387-fe31-4503-8c6d-9657bfb2f313"  {"TraceId": "febe5fd3-f694-4f34-9c37-fcaf515aaecb"}
2024-05-30T21:17:21.480Z    INFO    syncer/fullsync.go:867  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "76b98441-192d-4ef8-b376-6100dabacf9c"  {"TraceId": "febe5fd3-f694-4f34-9c37-fcaf515aaecb"}
2024-05-30T21:17:21.480Z    INFO    syncer/fullsync.go:867  FullSync for VC sc2-10-186-28-26.eng.vmware.com: update is not required for volume: "235b16cf-d0cd-4534-8580-76067cdb7f61"  {"TraceId": "febe5fd3-f694-4f34-9c37-fcaf515aaecb"}

Special notes for your reviewer:

Release note:

fix continues update volume metadata calls for TKC volumes
k8s-ci-robot commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chethanv28, 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)~~ [chethanv28,divyenpatel] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
chethanv28 commented 1 month ago

/hold

chethanv28 commented 1 month ago

/hold cancel