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

Change port value to string in kube-vip jsonPatch#3804

Merged
ydp merged 1 commit intomainfrom
topic/dingpingy/apiserverPort
Nov 1, 2022
Merged

Change port value to string in kube-vip jsonPatch#3804
ydp merged 1 commit intomainfrom
topic/dingpingy/apiserverPort

Conversation

@ydp
Copy link
Copy Markdown

@ydp ydp commented Nov 1, 2022

What this PR does / why we need it

The port in kube-vip manifest env needs to be string if it is set, otherwise parsing manifest fails.

Which issue(s) this PR fixes

Fixes #3803

Describe testing done for PR

Manually created cluster with customized CLUSTER_API_SERVER_PORT.

Release note

Support CLUSTER_API_SERVER_PORT in classy cluster on vSphere

Additional information

Special notes for your reviewer

@ydp ydp requested a review from a team as a code owner November 1, 2022 10:29
@ydp ydp requested review from a team November 1, 2022 10:29
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 1, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3804/20221101103730/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 DanielXiao added the ok-to-merge PRs should be labelled with this before merging label Nov 1, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 1, 2022

Codecov Report

Merging #3804 (2b144aa) into main (986bff7) will decrease coverage by 0.90%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3804      +/-   ##
==========================================
- Coverage   46.60%   45.69%   -0.91%     
==========================================
  Files         400      425      +25     
  Lines       39726    41282    +1556     
==========================================
+ Hits        18514    18864     +350     
- Misses      19514    20702    +1188     
- Partials     1698     1716      +18
Impacted Files Coverage Δ
...oller-manager/controllers/v3_cascade_controller.go 56.19% <0.00%> (-5.72%) ⬇️
...ons/controllers/packageinstallstatus_controller.go 77.99% <0.00%> (-1.16%) ⬇️
cmd/cli/plugin/cluster/upgrade.go 58.50% <0.00%> (ø)
cmd/cli/plugin/cluster/delete_node_pool.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/credentials_update.go 9.23% <0.00%> (ø)
cmd/cli/plugin/cluster/get.go 6.27% <0.00%> (ø)
...i/plugin/cluster/delete_machinehealthcheck_node.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 9.52% <0.00%> (ø)
cmd/cli/plugin/cluster/available_upgrade.go 16.32% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
... and 18 more

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

@ydp ydp merged commit 0db3776 into main Nov 1, 2022
@ydp ydp deleted the topic/dingpingy/apiserverPort branch November 1, 2022 12:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@DanielXiao DanielXiao DanielXiao approved these changes

Reviewers whose approvals may not affect merge requirements

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.

CLUSTER_API_SERVER_PORT not working in classy cluster with kube-vip

3 participants

@ydp @DanielXiao @vmwclabot