MAJOR windows fixes for secretgen and antrea-cleanup by hxietkg · Pull Request #990 · vmware-tanzu/tanzu-framework

hxietkg · 2021-10-27T09:23:37Z

several fixes to windows

regression for prod clusters fixed by skipping multi AZs
fix to antrea start after Windows nodes reboot because OVS bridge and HnsNetwork previously created by Antrea is not cleaned, a cleanup script is provided in the new version of Antrea, this change registers the
script as a callback on system's shutdown, the cleanup script will be invoked before Windows shutdown.
fix to secret-gen does not support Windows cluster, just disable it for Windows

…

multiple-azs is supported by #1108, but the change breaks Windows cluster, and this feature was not estimated for Windows cluster, so just disable it for Windows .

What this PR does / why we need it

Antrea-agent fails to start after reboot, so the a Windows node will be re-created once it reboots, this is not expected, we need Windows node support reboot.

also, we fix the other issues mentioned above here for expedience

Which issue(s) this PR fixes

Fixes #988

Describe testing done for PR

Create a windows cluster, then reboot Windows node, and verify antrea-agent in the node after reboot.

Release note

windows fixes for reboot, secret gen omission, and multi AZ omission.

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

github-actions · 2021-10-27T09:36:17Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211027093614/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 · 2021-10-27T09:41:49Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211027094147/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.

jayunit100 · 2021-10-28T17:23:14Z

(deleting earlier comment about ipconfig /renew
seems to work thanks to @stuartpreston for testing for us on his bare metal hardware as well

github-actions · 2021-10-30T00:58:17Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211030005815/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 · 2021-10-30T03:00:17Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211030030016/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 · 2021-10-30T04:20:09Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211030042008/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 · 2021-10-30T04:27:23Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211030042721/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 · 2021-10-30T05:17:48Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211030051746/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.

jayunit100 · 2021-11-10T01:32:34Z

before merging lets add the one liner to IS_WINDOWS_WORKLOAD_CLUSTER as well to simplify the merge process /review process.

Lets also rename this to "Minor windows fixes for secretgen and antreacleanup"

jayunit100 · 2021-11-10T01:36:18Z

@hxietkg can you rename this to "two minor fixes to windows YTT" and add unit tests to the MR also (if its possible, I think it should be, I think we should be generating the secretgen in the yaml).

github-actions · 2021-11-10T04:51:13Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211110045110/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.

hxietkg · 2021-11-10T13:18:55Z

@hxietkg can you rename this to "two minor fixes to windows YTT" and add unit tests to the MR also (if its possible, I think it should be, I think we should be generating the secretgen in the yaml).

this is not a bug, we may just disable the secret-gen by adding "SECRETGEN_CONTROLLER_ENABLE: false" to config file.

jayunit100 · 2021-11-10T13:25:27Z

thats fair - but its still technically a bug on windows side since that SECRETGEN isnt a default disable

... can you add that as a DEFAULT to the windows ytt then ?

github-actions · 2021-11-10T16:36:22Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211110163620/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.

jayunit100 · 2021-11-11T12:46:50Z

pkg/v1/providers/tests/unit/windows_test.go

 			filepath.Join(yamlRoot, "ytt", "02_addons", "cpi", "cpi_addon_data.lib.yaml"),
 			filepath.Join(yamlRoot, "ytt", "03_customizations", "02_avi", "ako-deployment.lib.yaml"),
 			//filepath.Join(YAML_ROOT, "provider-bundle", "providers", "ytt", "02_addons", "cpi", "cpi_addon_data.lib.yaml"),
+			filepath.Join(yamlRoot, "ytt", "03_customizations", "03_windows"),


ah yes ! thank you for adding this.

jayunit100 · 2021-11-11T14:13:33Z

pkg/v1/providers/ytt/03_customizations/03_windows/prevent_windows_updates.yaml

 #@ load("@ytt:data", "data")
 #@ if data.values.IS_WINDOWS_WORKLOAD_CLUSTER:
-#@overlay/match by=overlay.subset({"kind":"KubeadmConfigTemplate"})
+#@overlay/match by=overlay.subset({"kind":"KubeadmConfigTemplate", "metadata":{"name": str(data.values.CLUSTER_NAME) + "-md-0-windows-containerd"}})


interesting . I assume this works, but am curious what changed that now requires us to start doing this?

OH its because we have multiple kubeadmconfig templates ! duh.

github-actions · 2021-11-15T00:34:06Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211115003404/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.

hxietkg · 2021-11-15T00:37:40Z

rebased on 15/Nov

jayunit100 · 2021-11-15T13:53:51Z

pkg/v1/providers/infrastructure-vsphere/v1.0.1/ytt/base-template.yaml

 apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
 kind: VSphereMachineTemplate
 metadata:
-  name: '${ CLUSTER_NAME }-worker'


well need to potentially disable VSPHERE_AZ_1/2 stuff for now, just to get this merged and working for release

possible solutions:

disable VSPHERE_AZs

enable AZ1/2 in our windows ytt

going w/ one bc...well... time :)

@stuartpreston fYi were disabling this for expedeince

github-actions · 2021-11-15T15:15:06Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211115151504/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 · 2021-11-16T02:22:51Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211116022249/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 · 2021-11-16T02:39:44Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211116023943/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 · 2021-11-16T03:25:30Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211116032528/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 · 2021-11-16T03:40:30Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211116034029/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.

Antrea fails to start after Windows nodes reboot because OVS bridge and HnsNetwork previously created by Antrea is not cleaned, a cleanup script is provided in the new version of Antrea, this change registers the script as a callback on system's shutdown, the cleanup script will be invoked before Windows shutdown. secretgen does not support Windows cluster, just disable it. Disable vsphere-az1/2 on Windows cluster because this is not tested on Windows clusters.

github-actions · 2021-11-17T00:29:11Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/990/20211117002910/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 · 2021-11-17T01:44:44Z

Please add an entry in the release notes section.
Also please update the PR descriptoin since there is more than just introducing the cleanup script in the change now.

jayunit100 · 2021-11-17T01:59:59Z

Lgtm

clustergen tests are correctly reporting removal of the old error

“””Error: : unable to get template: Overlaying (in following order: overlay-windows.yaml, vsphere-overlay.yaml, 01_plans/prod.yaml,”””

jayunit100 · 2021-11-17T02:03:21Z

Hongsheng please add release note

vuil

lgtm, thanks!

hxietkg requested a review from a team as a code owner October 27, 2021 09:23

vmwclabot added the cla-not-required label Oct 27, 2021

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from f2d9cf5 to a9591c5 Compare October 27, 2021 09:29

vuil added the ok-to-merge PRs should be labelled with this before merging label Oct 27, 2021

jayunit100 mentioned this pull request Oct 28, 2021

Ytt integration testing #797

Merged

vuil assigned jayunit100 Oct 28, 2021

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from a9591c5 to 988ea3a Compare October 30, 2021 00:47

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch 2 times, most recently from 0c80479 to 2cdcdc5 Compare October 30, 2021 04:13

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from 2cdcdc5 to 65c8293 Compare October 30, 2021 05:05

jayunit100 mentioned this pull request Nov 10, 2021

Disable Secretgen in windows #1122

Open

9 tasks

hxietkg changed the title ~~Register cleanup for antrea~~ two minor fixes to windows YTT : Register cleanup for antrea & remove secret-gen Nov 10, 2021

hxietkg changed the title ~~two minor fixes to windows YTT : Register cleanup for antrea & remove secret-gen~~ Minor windows fixes for secretgen and antreacleanup Nov 10, 2021

hxietkg changed the title ~~Minor windows fixes for secretgen and antreacleanup~~ Register cleanup for antrea Nov 10, 2021

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from 65c8293 to 44698fa Compare November 10, 2021 04:35

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from 44698fa to 150047a Compare November 10, 2021 16:19

jayunit100 reviewed Nov 11, 2021

View reviewed changes

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from db1e3aa to aef7d20 Compare November 15, 2021 00:17

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from ac0fe9e to ea38e56 Compare November 15, 2021 12:09

jayunit100 changed the title ~~Minor windows fixes for secretgen and antrea-cleanup~~ MAJOR windows fixes for secretgen and antrea-cleanup Nov 15, 2021

jayunit100 reviewed Nov 15, 2021

View reviewed changes

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from ea38e56 to 2f778ba Compare November 15, 2021 14:59

jayunit100 assigned hxietkg Nov 16, 2021

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from 2f778ba to bc186cb Compare November 16, 2021 02:16

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from bc186cb to bf037c2 Compare November 16, 2021 02:34

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from bf037c2 to 5670be3 Compare November 16, 2021 03:19

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from 5670be3 to e249c88 Compare November 16, 2021 03:34

hxietkg force-pushed the topic/hxie/register_antrea_cleanup branch from e249c88 to 15f4cb2 Compare November 17, 2021 00:16

vuil added the kind/bug PR/Issue related to a bug label Nov 17, 2021

vuil unassigned hxietkg Nov 17, 2021

vuil approved these changes Nov 17, 2021

View reviewed changes

vuil merged commit 4e94999 into main Nov 17, 2021

vuil deleted the topic/hxie/register_antrea_cleanup branch November 17, 2021 02:18

Conversation

hxietkg commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

PR Checklist

Additional information

Special notes for your reviewer

Uh oh!

github-actions bot commented Oct 27, 2021

Uh oh!

github-actions bot commented Oct 27, 2021

Uh oh!

jayunit100 commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 30, 2021

Uh oh!

github-actions bot commented Oct 30, 2021

Uh oh!

github-actions bot commented Oct 30, 2021

Uh oh!

github-actions bot commented Oct 30, 2021

Uh oh!

github-actions bot commented Oct 30, 2021

Uh oh!

jayunit100 commented Nov 10, 2021

Uh oh!

jayunit100 commented Nov 10, 2021

Uh oh!

github-actions bot commented Nov 10, 2021

Uh oh!

hxietkg commented Nov 10, 2021

Uh oh!

jayunit100 commented Nov 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 10, 2021

Uh oh!

jayunit100 Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

jayunit100 Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

jayunit100 Nov 11, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 15, 2021

Uh oh!

hxietkg commented Nov 15, 2021

Uh oh!

jayunit100 Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayunit100 Nov 15, 2021

Choose a reason for hiding this comment

Uh oh!

jayunit100 Nov 15, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 15, 2021

Uh oh!

github-actions bot commented Nov 16, 2021

Uh oh!

github-actions bot commented Nov 16, 2021

Uh oh!

github-actions bot commented Nov 16, 2021

Uh oh!

github-actions bot commented Nov 16, 2021

Uh oh!

github-actions bot commented Nov 17, 2021

Uh oh!

vuil commented Nov 17, 2021

Uh oh!

jayunit100 commented Nov 17, 2021

Uh oh!

hxietkg commented Oct 27, 2021 •

edited

Loading

jayunit100 commented Oct 28, 2021 •

edited

Loading

jayunit100 commented Nov 10, 2021 •

edited

Loading

jayunit100 Nov 15, 2021 •

edited

Loading