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

Fix missing variable count on overlay for Windows #3893

Merged
merged 1 commit into from
Nov 15, 2022
Merged

Fix missing variable count on overlay for Windows #3893

merged 1 commit into from
Nov 15, 2022

Conversation

knabben
Copy link
Member

@knabben knabben commented Nov 10, 2022 •
edited
Loading

What this PR does / why we need it

  • Fix Overlay Md replicas count for Prod, right now prodcc has 3 replicas on region one in the MD, and 1 in the others when using this plan.
~$ k get node -o wide
NAME                                        STATUS   ROLES                  AGE   VERSION            INTERNAL-IP      EXTERNAL-IP      OS-IMAGE                       KERNEL-VERSION    CONTAINER-RUNTIME
tkg-vc-antrea-lvnsn-lvqvv                   Ready    control-plane,master   82m   v1.23.8+vmware.2   VMware Photon OS/Linux         4.19.247-7.ph3    containerd://1.6.6
tkg-vc-antrea-lvnsn-mz9cs                   Ready    control-plane,master   89m   v1.23.8+vmware.2  VMware Photon OS/Linux         4.19.247-7.ph3    containerd://1.6.6
tkg-vc-antrea-lvnsn-nwjtr                   Ready    control-plane,master   84m   v1.23.8+vmware.2   Photon OS/Linux         4.19.247-7.ph3    containerd://1.6.6
tkg-vc-antrea-md-0-j9jdn-68756575c7-ff9t2   Ready    <none>                 83m   v1.23.8+vmware.2 Windows Server 2019 Standard   10.0.17763.2114   containerd://1.6.6
tkg-vc-antrea-md-0-j9jdn-68756575c7-mcwcl   Ready    <none>                 82m   v1.23.8+vmware.2 Windows Server 2019 Standard   10.0.17763.2114   containerd://1.6.6
tkg-vc-antrea-md-0-j9jdn-68756575c7-x2zpl   Ready    <none>                 82m   v1.23.8+vmware.2 Windows Server 2019 Standard   10.0.17763.2114   containerd://1.6.6
tkg-vc-antrea-md-1-2nmm8-5b956668c5-mhrkc   Ready    <none>                 82m   v1.23.8+vmware.2 Windows Server 2019 Standard   10.0.17763.2114   containerd://1.6.6
tkg-vc-antrea-md-2-7wkqp-66f9679dbb-kctmw   Ready    <none>                 82m   v1.23.8+vmware.2 Windows Server 2019 Standard   10.0.17763.2114   containerd://1.6.6
  • Fixing other nits like reading the OS_NAME for the tkr resolution, and giving parity with Linux spec

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Release note

Prodcc machineDeployment replicas=1 for Windows clusters

Additional information

Special notes for your reviewer

@knabben knabben requested review from a team as code owners November 10, 2022 14:38
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3893/20221110144803/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
Copy link

codecov bot commented Nov 10, 2022 •
edited
Loading

Codecov Report

Merging #3893 (a42baed) into main (1ab6021) will decrease coverage by 0.95%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3893      +/-   ##
==========================================
- Coverage   46.97%   46.02%   -0.96%     
==========================================
  Files         402      427      +25     
  Lines       40203    41754    +1551     
==========================================
+ Hits        18887    19218     +331     
- Misses      19569    20767    +1198     
- Partials     1747     1769      +22
Impacted Files Coverage Δ
addons/controllers/machine_controller.go 65.65% <0.00%> (-3.04%) ⬇️
addons/controllers/clusterbootstrap_controller.go 63.36% <0.00%> (-1.50%) ⬇️
...ons/controllers/packageinstallstatus_controller.go 79.15% <0.00%> (ø)
cmd/cli/plugin/cluster/delete_node_pool.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 11.36% <0.00%> (ø)
...md/cli/plugin/cluster/delete_machinehealthcheck.go 19.23% <0.00%> (ø)
.../cli/plugin/cluster/get_machinehealthcheck_node.go 9.30% <0.00%> (ø)
cmd/cli/plugin/cluster/osimage.go 100.00% <0.00%> (ø)
cmd/cli/plugin/cluster/credentials_update.go 9.23% <0.00%> (ø)
cmd/cli/plugin/cluster/delete.go 12.50% <0.00%> (ø)
... and 19 more

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

tenczar
tenczar approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small question, otherwise it looks good to me.

providers/infrastructure-vsphere/v1.4.1/yttcc/overlay.yaml Outdated
#@overlay/match missing_ok=True
metadata:
annotations:
#! VVV TODO(tenczar) os-name handling
run.tanzu.vmware.com/resolve-os-image: image-type=ova,os-name=windows
run.tanzu.vmware.com/resolve-os-image: #@ "image-type=ova,os-name={}".format(data.values.OS_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, what are the possible values of OS_NAME in the windows scenario? This all should be fine as long as we were already using these OS_NAMES in the legacy version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a fair point, we do not use OS_NAME for legacy.
If we start to use the same OS_NAME for cp and Windows WL it will conflict, I will roll back to os-name=windows since this is the default OVF prop from image-builder.

@knabben knabben force-pushed the topic/knabben/windows-fix-missing-count branch from b59ae37 to a42baed Compare November 10, 2022 21:36
@github-actions
Copy link

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

@wjun wjun added the ok-to-merge PRs should be labelled with this before merging label Nov 15, 2022
@knabben knabben merged commit 5cf071f into main Nov 15, 2022
@knabben knabben deleted the topic/knabben/windows-fix-missing-count branch November 15, 2022 10:16
benjaminapetersen pushed a commit that referenced this pull request Nov 16, 2022
@knabben @benjaminapetersen
Fix missing variable count on overlay for Windows (#3893)
3c4ef06
@knabben knabben mentioned this pull request Dec 7, 2022
Closed
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@tenczar tenczar tenczar approved these changes

Assignees
No one assigned
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@knabben @tenczar @wjun @vmwclabot