Fix template issues in vsphere clusterclass implementation #3173

tenczar · 2022-08-22T21:06:55Z

What this PR does / why we need it

This PR updates ytt and go text templates for vsphere clusterclass that prevented successful deployment of clusterclass based capv clusters. Changes include:

Fix indentation issue in VsphereMachineTemplate when updating network settings
Fix the kube-vip image and tag declaration to properly access resources from TKR_DATA
Adds a regex matcher for applying node labels. This prevents values from the TKR_DATA being used as node-labels if those values are not valid node-label values
Fix machineHealthCheck definition to only add this block if a cluster definition specifies mhcs.
Fixes credential values
Fixes an issue with incorrectly typed config values being passed to the clusterbootstrap

Which issue(s) this PR fixes

Fixes #3439

Describe testing done for PR

Manually deployed a clusterclass based management cluster using Antrea and validated that the cluster is created successfully.

Release note

Fixes vsphere clusterclass definition to successfully deploy clusterclass capv clusters.

Additional information

Special notes for your reviewer

github-actions · 2022-08-25T20:30:59Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220825203057/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-08-30T20:56:21Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220830205618/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-08-30T21:55:29Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220830215528/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-08-31T00:46:52Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220831004650/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-08-31T19:09:47Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220831190945/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-08-31T20:29:51Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220831202948/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-01T20:01:50Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220901200148/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

vuil · 2022-09-06T16:22:40Z

pkg/v1/providers/config_default.yaml

@@ -174,9 +174,9 @@ NSXT_PASSWORD: ""
 #! NSX-T host
 NSXT_MANAGER_HOST: ""
 #! set this to true if NSX-T uses self-signed cert
-NSXT_ALLOW_UNVERIFIED_SSL: "false"
+NSXT_ALLOW_UNVERIFIED_SSL: false


@tenczar : this change enables legacy config file not specifying this value to work correctly with the vsphere CC, but this changes the behavior when we need to use the equivalent legacy config file in fallback (pure client side ytt) mode.

Either existing logic that assumed string instead of bool will have to be updated (see ytt/02_addons/cpi/cpi_addon_data.lib.yaml and associated vendir-ed code)

or we should consider retaining the definition/semantics of the variables in config_default.yaml (and fixing on the yttcc side to translate the string to bool)

(same for NSXT_REMOTE_AUTH)

Either existing logic that assumed string instead of bool will have to be updated (see ytt/02_addons/cpi/cpi_addon_data.lib.yaml and associated vendir-ed code)

If we change the existing logic, will the upgrade break, since customer's previous value is still string

Thanks for reminding me. We will look for convertion for CC to avoid breaking existing logic.

If we change the existing logic, will the upgrade break, since customer's previous value is still string

@lubronzhan if we elect to change the legacy overlays for handling cpi addons, it will have to be done in a way that accepts both a string and a bool to not break upgrade (or when we need to fallback to legacy creation, for that matter)

@vuil Yeah that's my concern, since we can't break customer's upgrade path.

github-actions · 2022-09-09T01:17:19Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220909011718/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-09T01:20:10Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220909012008/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-09T01:26:02Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220909012600/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-12T19:32:35Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220912193233/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-14T19:18:13Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220914191809/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

DanielXiao · 2022-09-20T08:25:47Z

@tenczar PR to fix CPI/CSI issue is #3320

github-actions · 2022-09-20T18:08:28Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220920180826/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-20T22:44:33Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220920224430/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-09-21T02:03:03Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220921020302/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

codecov · 2022-09-21T02:42:35Z

Codecov Report

Merging #3173 (fbdb2bf) into main (142aa6f) will increase coverage by 0.54%.
The diff coverage is 90.05%.

@@            Coverage Diff             @@
##             main    #3173      +/-   ##
==========================================
+ Coverage   53.12%   53.66%   +0.54%     
==========================================
  Files         103       91      -12     
  Lines       10419    10002     -417     
==========================================
- Hits         5535     5368     -167     
+ Misses       4429     4193     -236     
+ Partials      455      441      -14

Impacted Files	Coverage Δ
...sphere-template-resolver/templateresolver/types.go	`42.10% <42.10%> (ø)`
...ter/vsphere-template-resolver/template/resolver.go	`89.91% <89.91%> (ø)`
...emplate-resolver/templateresolver/resolver_impl.go	`99.02% <99.02%> (ø)`
...re-template-resolver/templateresolver/interface.go	`100.00% <100.00%> (ø)`
...ntroller/tkr-status/clusterstatus/clusterstatus.go	`45.35% <0.00%> (-2.19%)`	⬇️
addons/controllers/clusterbootstrap_controller.go	`62.92% <0.00%> (-1.63%)`	⬇️
capabilities/client/pkg/discovery/cluster_gvr.go
...apabilities/client/pkg/discovery/cluster_object.go
...apabilities/client/pkg/discovery/cluster_schema.go
capabilities/client/pkg/discovery/discovery.go
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

pkg/v1/providers/yttcb/clusterbootstrap.yaml

github-actions · 2022-09-22T01:03:41Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220922010339/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Fix missing config_default values add missing "USE_TOPOLOGY_CATEGORIES" config variable with default value of false. properly convert boolean values to bool in clusterbootstrap.yml Use kube-vip specific imageRepository for deploying kube-vip ccluster gen hack to add resolve-os-image Fix cb secret handling fix indentation in vpshere machine template network defintion updates patches so node-labels avoid adding illegal labels

github-actions · 2022-09-23T18:22:37Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3173/20220923182235/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

packages/tkg-clusterclass-vsphere/bundle/config/upstream/base.yaml

imikushin

Looks good. Thanks @tenczar! (for the change and for your patience)

tenczar requested review from a team as code owners August 22, 2022 21:06

vmwclabot added the cla-not-required label Aug 22, 2022

tenczar force-pushed the topic/ntenczar/vsphere_cc branch 3 times, most recently from 4433ebe to 17f54a2 Compare August 25, 2022 20:20

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from fed3b3c to 276383f Compare August 30, 2022 21:47

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 276383f to 4ccd9a8 Compare August 31, 2022 00:39

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from b509df4 to e194a3e Compare August 31, 2022 20:18

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from e194a3e to d8bde5d Compare September 1, 2022 19:53

tenczar requested a review from a team as a code owner September 1, 2022 19:53

vuil reviewed Sep 6, 2022

View reviewed changes

tenczar force-pushed the topic/ntenczar/vsphere_cc branch 3 times, most recently from e6b15c3 to 2c1c825 Compare September 9, 2022 01:14

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 2c1c825 to 741aaef Compare September 12, 2022 19:24

vuil added the do-not-merge/wip Still work in progress label Sep 13, 2022

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 741aaef to 1de66e6 Compare September 14, 2022 19:06

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 1de66e6 to 4f0c972 Compare September 20, 2022 17:57

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 4f0c972 to 595d6c4 Compare September 20, 2022 22:34

tenczar changed the title ~~WIP vsphere cc changes.~~ Fix template issues in vsphere clusterclass implementation Sep 20, 2022

tenczar removed the do-not-merge/wip Still work in progress label Sep 20, 2022

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 595d6c4 to 2dd7bfb Compare September 21, 2022 01:52

tenczar added the ok-to-merge PRs should be labelled with this before merging label Sep 21, 2022

wjun reviewed Sep 21, 2022

View reviewed changes

pkg/v1/providers/yttcb/clusterbootstrap.yaml Outdated Show resolved Hide resolved

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from 2dd7bfb to d001524 Compare September 22, 2022 00:50

tenczar force-pushed the topic/ntenczar/vsphere_cc branch from d001524 to fbdb2bf Compare September 23, 2022 18:08

ydp reviewed Sep 27, 2022

View reviewed changes

packages/tkg-clusterclass-vsphere/bundle/config/upstream/base.yaml Show resolved Hide resolved

ydp reviewed Sep 27, 2022

View reviewed changes

packages/tkg-clusterclass-vsphere/bundle/config/upstream/base.yaml Show resolved Hide resolved

imikushin approved these changes Sep 27, 2022

View reviewed changes

tenczar merged commit 81af532 into main Sep 28, 2022

tenczar deleted the topic/ntenczar/vsphere_cc branch September 28, 2022 00:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix template issues in vsphere clusterclass implementation #3173

Fix template issues in vsphere clusterclass implementation #3173

tenczar commented Aug 22, 2022 •

edited

Loading

github-actions bot commented Aug 25, 2022

github-actions bot commented Aug 30, 2022

github-actions bot commented Aug 30, 2022

github-actions bot commented Aug 31, 2022

github-actions bot commented Aug 31, 2022

github-actions bot commented Aug 31, 2022

github-actions bot commented Sep 1, 2022

vuil Sep 6, 2022 •

edited

Loading

lubronzhan Sep 7, 2022 •

edited

Loading

DanielXiao Sep 8, 2022

vuil Sep 12, 2022

lubronzhan Sep 13, 2022

github-actions bot commented Sep 9, 2022

github-actions bot commented Sep 9, 2022

github-actions bot commented Sep 9, 2022

github-actions bot commented Sep 12, 2022

github-actions bot commented Sep 14, 2022

DanielXiao commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

github-actions bot commented Sep 21, 2022

codecov bot commented Sep 21, 2022 •

edited

Loading

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 23, 2022

imikushin left a comment

Fix template issues in vsphere clusterclass implementation #3173

Fix template issues in vsphere clusterclass implementation #3173

Conversation

tenczar commented Aug 22, 2022 • edited Loading

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

Additional information

Special notes for your reviewer

github-actions bot commented Aug 25, 2022

github-actions bot commented Aug 30, 2022

github-actions bot commented Aug 30, 2022

github-actions bot commented Aug 31, 2022

github-actions bot commented Aug 31, 2022

github-actions bot commented Aug 31, 2022

github-actions bot commented Sep 1, 2022

vuil Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

lubronzhan Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

DanielXiao Sep 8, 2022

Choose a reason for hiding this comment

vuil Sep 12, 2022

Choose a reason for hiding this comment

lubronzhan Sep 13, 2022

Choose a reason for hiding this comment

github-actions bot commented Sep 9, 2022

github-actions bot commented Sep 9, 2022

github-actions bot commented Sep 9, 2022

github-actions bot commented Sep 12, 2022

github-actions bot commented Sep 14, 2022

DanielXiao commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

github-actions bot commented Sep 21, 2022

codecov bot commented Sep 21, 2022 • edited Loading

Codecov Report

github-actions bot commented Sep 22, 2022

github-actions bot commented Sep 23, 2022

imikushin left a comment

Choose a reason for hiding this comment

tenczar commented Aug 22, 2022 •

edited

Loading

vuil Sep 6, 2022 •

edited

Loading

lubronzhan Sep 7, 2022 •

edited

Loading

codecov bot commented Sep 21, 2022 •

edited

Loading