Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Update CSI controller in tanzu-framework to set Zone for Paravirtual CSI #2926

Merged
merged 12 commits into from
Jul 18, 2022

Conversation

ridaz-zz
Copy link
Contributor

@ridaz-zz ridaz-zz commented Jul 13, 2022

What this PR does / why we need it

This PR will bring in the following changes:

  1. Adds Zone field for Paravirtual CSI.
  2. Check if Valid Availability Zones are available.
  3. For stretch supervisor support, if zones (apart from the default availability zone) are available add the given flags in csi-provisioner container of the csi-controller deployment.

Note: If default zone named, vmware-system-legacy is present, the topology flags need not be added.

--feature-gates=Topology=true
--strict-topology

Which issue(s) this PR fixes

Fixes #2925

Describe testing done for PR

  1. Testbed with Multiple Zones
kubectl get az
NAME     AGE
zone-1   109m
zone-2   109m
zone-3   109m

Deployed a TKC, and patched the addons-controller image to the tanzu-addons-controller-manager deployment.

kubectl edit deployments -n vmware-system-csi vsphere-csi-controller
- args:
        - --v=4
        - --timeout=300s
        - --csi-address=$(ADDRESS)
        - --leader-election
        - --default-fstype=ext4
        - --kube-api-qps=100
        - --kube-api-burst=100
        - --feature-gates=Topology=true
        - --strict-topology
        env:
        - name: ADDRESS
          value: /csi/csi.sock
        name: csi-provisioner
  1. Testbed without availability zones/Default availability zone:
kubectl get az
NAME                   AGE
vmware-system-legacy   70m

kubectl edit deployments -n vmware-system-csi vsphere-csi-controller
- args:
        - --v=4
        - --timeout=300s
        - --csi-address=$(ADDRESS)
        - --leader-election
        - --default-fstype=ext4
        - --kube-api-qps=100
        - --kube-api-burst=100
        env:
        - name: ADDRESS
          value: /csi/csi.sock
        name: csi-provisioner
  1. Unit Test PASSED with validating if zone is set to true:
vspherePVCSI:
    cluster_name: test-cluster-pv-csi
    cluster_uid: 57e8428a-fbb4-4fff-b8f5-8f0e58ffd910
    namespace: vmware-system-csi
    supervisor_master_endpoint_hostname: supervisor.default.svc
    supervisor_master_port: 6443
    feature_states:
        state1: value1
        state2: value2
        state3: value3
    zone: true
• [SLOW TEST:72.178 seconds]
VSphereCSIConfig Reconciler
/home/runner/work/tanzu-framework/tanzu-framework/addons/controllers/vspherecsiconfig_controller_test.go:28
  reconcile VSphereCSIConfig manifests in paravirtual mode
/home/runner/work/tanzu-framework/tanzu-framework/addons/controllers/vspherecsiconfig_controller_test.go:276
    Should reconcile VSphereCSIConfig and create data values secret for VSphereCSIConfig on management cluster
/home/runner/work/tanzu-framework/tanzu-framework/addons/controllers/vspherecsiconfig_controller_test.go:294
------------------------------

Release note


PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@ridaz-zz ridaz-zz requested review from a team as code owners July 13, 2022 14:40
@github-actions
Copy link

Hi @ridaz! And thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Tanzu Framework better.

@codecov
Copy link

codecov bot commented Jul 16, 2022

Codecov Report

Merging #2926 (4858145) into main (59b46aa) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2926      +/-   ##
==========================================
+ Coverage   43.99%   44.00%   +0.01%     
==========================================
  Files         416      416              
  Lines       41849    41852       +3     
==========================================
+ Hits        18411    18417       +6     
+ Misses      21716    21714       -2     
+ Partials     1722     1721       -1     
Impacted Files Coverage Δ
addons/controllers/addon_controller.go 63.61% <100.00%> (+0.30%) ⬆️
...ons/controllers/packageinstallstatus_controller.go 80.64% <0.00%> (-2.16%) ⬇️
addons/controllers/clusterbootstrap_controller.go 63.90% <0.00%> (+0.61%) ⬆️
pkg/v1/tkg/tkgpackageclient/package_update.go 85.00% <0.00%> (+1.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59b46aa...4858145. Read the comment docs.

@shyaamsn shyaamsn requested a review from maralavi July 18, 2022 18:32
@maralavi
Copy link
Contributor

The changes look good to me. Thanks for adding the tests!

@maralavi maralavi added the ok-to-merge PRs should be labelled with this before merging label Jul 18, 2022
@maralavi maralavi merged commit e921082 into vmware-tanzu:main Jul 18, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

topology flags/Zones missing for csi-provisioner for PVCSI
6 participants