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

remove ipv6 workaround from post kubeadm#3765

Merged
christianang merged 1 commit intovmware-tanzu:mainfrom
adobley:topic/adobley/remove-ipv6-hack
Nov 3, 2022
Merged

remove ipv6 workaround from post kubeadm#3765
christianang merged 1 commit intovmware-tanzu:mainfrom
adobley:topic/adobley/remove-ipv6-hack

Conversation

@adobley
Copy link
Copy Markdown
Contributor

@adobley adobley commented Oct 26, 2022

What this PR does / why we need it

all current versions of tanzu are using a new-enough version of containerd (v1.5.0 or greater) that we can remove this workaround

the issue this addressed was fixed in
containerd/containerd#5145

Describe testing done for PR

Created a cluster to verify it comes up without this workaround

Release note

N/A

Additional information

Special notes for your reviewer

@adobley adobley requested a review from a team as a code owner October 26, 2022 21:46
@adobley adobley requested review from a team October 26, 2022 21:46
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 26, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3765/20221026215640/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
Copy Markdown

codecov Bot commented Oct 26, 2022 •
edited
Loading

Codecov Report

Merging #3765 (80ab92b) into main (0db3776) will decrease coverage by 0.89%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3765      +/-   ##
==========================================
- Coverage   46.60%   45.71%   -0.90%     
==========================================
  Files         400      425      +25     
  Lines       39726    41282    +1556     
==========================================
+ Hits        18516    18873     +357     
- Misses      19513    20695    +1182     
- Partials     1697     1714      +17
Impacted Files Coverage Δ
cmd/cli/plugin/cluster/set_node_pool.go 14.63% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 9.52% <0.00%> (ø)
cmd/cli/plugin/cluster/credentials_update.go 9.23% <0.00%> (ø)
.../cli/plugin/cluster/set_machinehealthcheck_node.go 23.33% <0.00%> (ø)
cmd/cli/plugin/cluster/kubeconfig_get.go 8.82% <0.00%> (ø)
cmd/cli/plugin/cluster/get.go 6.27% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
cmd/cli/plugin/cluster/available_upgrade.go 16.32% <0.00%> (ø)
cmd/cli/plugin/cluster/machinehealthcheck.go 100.00% <0.00%> (ø)
...in/cluster/set_machinehealthcheck_control_plane.go 21.21% <0.00%> (ø)
... and 15 more

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

@adobley adobley added the ok-to-merge PRs should be labelled with this before merging label Oct 26, 2022
@imikushin
Copy link
Copy Markdown
Contributor

imikushin commented Oct 27, 2022

question: Does this only apply to vsphere?

@christianang
Copy link
Copy Markdown
Contributor

christianang commented Oct 27, 2022

Yes, we only support ipv6 on vsphere at the moment.

tenczar
tenczar approved these changes Oct 31, 2022
View reviewed changes
Copy link
Copy Markdown
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.

This will need to be rebased because of the providers bump, but looks good so far. Do we want to remove this hack from the legacy overlays as well?

lubronzhan
lubronzhan approved these changes Oct 31, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@lubronzhan lubronzhan left a comment

Choose a reason for hiding this comment

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

LGTM

DanielXiao
DanielXiao reviewed Nov 1, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@DanielXiao DanielXiao left a comment

Choose a reason for hiding this comment

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

Can you rebase this PR? There is a folder name change in #3772

@christianang @tylerschultz
remove ipv6 workaround from post kubeadm
80ab92b 
all current versions of tanzu are using a new-enough version of
containerd (v1.5.0 or greater) that we can remove this workaround

the issue this addressed was fixed in
containerd/containerd#5145

Signed-off-by: Aidan Obley <[email protected]>
Co-authored-by: Christian Ang <[email protected]>
@adobley adobley force-pushed the topic/adobley/remove-ipv6-hack branch from 5ab49cb to 80ab92b Compare November 1, 2022 17:46
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 1, 2022

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

@christianang christianang merged commit 5a9fd3d into vmware-tanzu:main Nov 3, 2022
@christianang christianang deleted the topic/adobley/remove-ipv6-hack branch November 3, 2022 16:32
@adobley adobley mentioned this pull request Nov 9, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers

@DanielXiao DanielXiao DanielXiao left review comments

@christianang christianang christianang approved these changes

@tenczar tenczar tenczar approved these changes

@lubronzhan lubronzhan lubronzhan 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.

7 participants

@adobley @imikushin @christianang @DanielXiao @tenczar @lubronzhan @vmwclabot