-
Notifications
You must be signed in to change notification settings - Fork 307
Add zone/region and http proxy support to vsphere CPI and CSI #593
Conversation
83b88e6
to
525e413
Compare
@chenliu1993 could you help review the vsphere region/zone part that you authored ? Thanks. |
The part of region/zone codes maybe needs a fixup, so please do not merge this first. Thanks |
@chenliu1993 what needs fixup? Is there something wrong with this PR or in general zone/region doesnt work ? |
b1f0285
to
2a71aba
Compare
Yes, csi deployment needs two additional args which currently is commented to make zone/region work properly when there is zone/region topology existing. Otherwise, the zone/region tagged PV/PVC cannot work properly. |
secretName: nsxt-tls-certificates | ||
#@overlay/append | ||
- secret: | ||
secretName: nsxt-tls-certificates | ||
name: nsxt-certificates-volume |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably delete the tab for this line as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
5ea12b1
to
3f067d1
Compare
Hi @shyaamsn here is the PR for the fix: https://github.com/vmware-tanzu-private/core/pull/679/files |
3f067d1
to
1c89352
Compare
1c89352
to
353b0b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zone/region part looks good.
@seemiller Could you please take a look at this. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What this PR does / why we need it
Add zone/region and http proxy support to vsphere CPI and CSI
Describe testing done for PR
Tested templates using ytt and deployed package