🌱 Add dualstack e2e test #8517

killianmuldoon · 2023-04-12T14:57:21Z

Adds a dualstack e2e quick start test for CAPI

/area testing

killianmuldoon

Q: should this be implemented with ClusterClass? Should the ipv6 test be on ClusterClass?

killianmuldoon · 2023-04-12T19:40:32Z

test/e2e/data/infrastructure-docker/main/cluster-template-dualstack/md-dualstack.yaml

+      initConfiguration:
+        nodeRegistration:
+          kubeletExtraArgs:
+            node-ip: "0.0.0.0" # Note: The control plane node should use an ipv6 address as an internalIP. The worker node should use an ipv4 address as an internalIP.


Kubernetes (kubeadm) doesn't allow setting this field as unspecified - i.e. 0.0.0.0 / :: - for dualstack. The compromise here is to mix the nodes between ipv4 and ipv6.

Be careful here kubernetes/kubernetes#116305
There is problem with Dual stack node-ip with external cloud-provider, kubernetes/enhancements#3705
It's fixed in k8s 1.27, added here
Before that, node will only show single IP stack IP with external cloud-provider. Please correct me if I'm wrong @christianang

If I understand the issue correctly - setting this config on Kubelet means that the node object will only ever advertise one node address. Is that roughly right @lubronzhan?

I think, if we need to have explicitly dualstack nodes here, I can remove this flag and patch the node in the docker provider similar to what a cloud provider would do.

Hi @killianmuldoon the issue is only with external cloud-provider. if it's baremetal it's fine. Will you set cloud-provider=external on kubelet when using docker provider? If not setting to external it's probably ok

Yeah I think you both have it covered. As Lubron said it probably depends on if you set cloud-provider to external because in the end both affect the node-ips that are set in the node status.

chrischdi

/lgtm

Just wondering if there's something so we could extend the QuickStartSpec to check if the nodes actually have the dualstack / ips (something like via kubectl get nodes (of course via client) or so 🤔

k8s-ci-robot · 2023-04-13T06:36:15Z

LGTM label has been added.

Git tree hash: 1d315f6a3fe0d95b713d536a6a64bbebc0f6df75

killianmuldoon · 2023-04-13T07:31:35Z

Just wondering if there's something so we could extend the QuickStartSpec to check if the nodes actually have the dualstack / ips (something like via kubectl get nodes (of course via client) or so thinking

Probably a good idea - we could us a hook like we have in the upgrade test to run this check. Unfortunately though this test doesn't give us dual-stack node IPs, at least not in its current form. It results in mixed IPs - some 6, some 4.

sbueringer

Overall looks good.

I'm a bit confused why this seems to work while we needed a separate job where we run kind with IPv6 to get the IPv6-only stuff working. But if it just works that's fine, although we should have a follow-up to check if we still need a separate job for IPv6 and if we do figure out why IPv6 only doesn't work while DualStack works.

But maybe it's just that things changed either in kind or in our test framework and the problems we had in the past have been resolved

test/e2e/quick_start_test.go

test/infrastructure/container/docker.go

test/e2e/data/infrastructure-docker/main/cluster-template-dualstack/kcp-dualstack.yaml

test/e2e/data/infrastructure-docker/main/cluster-template-dualstack/cluster-dualstack.yaml

test/e2e/quick_start_test.go

christianang · 2023-04-13T22:28:04Z

You could deploy dual stack with different “primary” ip families, where the CIDRs and listen addresses, node-ip, etc are either IPv6 or IPv4. It could be worth testing both families as the “primary” ip family to get additional coverage on if a field can work with both IPv6 or IPv4.

Related, it is interesting that you configure the control plane as IPv6 family and the worker as IPv4, which I assume is to get similar coverage as what I’m suggesting above. Probably fine to do and probably gets some of the coverage I’m suggesting above.

sbueringer · 2023-05-15T10:16:05Z

@killianmuldoon I did another retry and it's green now. What is the current state of this PR?
(+/- I should do another review)

killianmuldoon · 2023-05-15T10:31:10Z

I've got a couple of fixups pending, should be ready for another round later today.

killianmuldoon · 2023-05-15T12:15:44Z

/retest

test/e2e/config/docker.yaml

test/e2e/data/infrastructure-docker/main/clusterclass-quick-start.yaml

test/e2e/data/kubetest/conformance.yaml

test/e2e/quick_start_test.go

test/infrastructure/docker/internal/docker/machine.go

test/infrastructure/docker/internal/docker/util.go

Signed-off-by: killianmuldoon <[email protected]>

sbueringer · 2023-05-15T16:54:21Z

Thank you!

/lgtm

/assign @fabriziopandini

k8s-ci-robot · 2023-05-15T16:54:27Z

LGTM label has been added.

Git tree hash: 2795c6f201d9f2b5d7b0ad07c3ccadfe8fd2c106

fabriziopandini

great work!
/lgtm
/approve

fabriziopandini · 2023-05-15T09:51:46Z

test/framework/bootstrap/kind_provider.go

@@ -70,7 +70,15 @@ func WithDockerSockMount() KindClusterOption {
 // the new kind cluster.
 func WithIPv6Family() KindClusterOption {
 	return kindClusterOptionAdapter(func(k *KindClusterProvider) {
-		k.ipFamily = clusterv1.IPv6IPFamily
+		k.ipFamily = clusterv1.DualStackIPFamily


Can we add a note why are we setting DuaStack for WithIPv6Family

k8s-ci-robot · 2023-05-16T07:33:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [fabriziopandini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added area/testing Issues or PRs related to testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 12, 2023

k8s-ci-robot requested review from chrischdi and enxebre April 12, 2023 14:57

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 12, 2023

killianmuldoon changed the title ~~🌱 Add dualstack e2e test~~ [WIP] 🌱 Add dualstack e2e test Apr 12, 2023

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 12, 2023

killianmuldoon commented Apr 12, 2023

View reviewed changes

killianmuldoon force-pushed the pr-e2e-dual-stack branch 4 times, most recently from 80385dd to d32c5c8 Compare April 12, 2023 19:39

killianmuldoon changed the title ~~[WIP] 🌱 Add dualstack e2e test~~ 🌱 Add dualstack e2e test Apr 12, 2023

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 12, 2023

killianmuldoon commented Apr 12, 2023

View reviewed changes

chrischdi reviewed Apr 13, 2023

View reviewed changes

k8s-ci-robot assigned chrischdi Apr 13, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 13, 2023

sbueringer reviewed Apr 13, 2023

View reviewed changes

killianmuldoon force-pushed the pr-e2e-dual-stack branch from d32c5c8 to 3e61c27 Compare April 13, 2023 18:11

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 13, 2023

k8s-ci-robot requested a review from chrischdi April 13, 2023 18:11

killianmuldoon force-pushed the pr-e2e-dual-stack branch from 3e61c27 to ee525fa Compare May 4, 2023 19:02

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 4, 2023

killianmuldoon force-pushed the pr-e2e-dual-stack branch 2 times, most recently from 2fb6ede to cdf9a42 Compare May 4, 2023 19:20

killianmuldoon force-pushed the pr-e2e-dual-stack branch 5 times, most recently from 9268327 to 7be3410 Compare May 14, 2023 21:26

killianmuldoon force-pushed the pr-e2e-dual-stack branch 2 times, most recently from b3bc55a to 3325f45 Compare May 15, 2023 12:08

killianmuldoon force-pushed the pr-e2e-dual-stack branch 3 times, most recently from eb9627d to 26a9598 Compare May 15, 2023 12:45

sbueringer reviewed May 15, 2023

View reviewed changes

killianmuldoon mentioned this pull request May 15, 2023

🌱 Use dualstack kind cluster in quick-start and startup script #8661

Merged

killianmuldoon force-pushed the pr-e2e-dual-stack branch from 26a9598 to 4451c3e Compare May 15, 2023 14:39

Add dualstack e2e test

68213cf

Signed-off-by: killianmuldoon <[email protected]>

killianmuldoon force-pushed the pr-e2e-dual-stack branch from 4451c3e to 68213cf Compare May 15, 2023 14:46

killianmuldoon mentioned this pull request May 15, 2023

test/e2e: Consolidate dualstack / IPv6 / ... tests #8578

Closed

k8s-ci-robot assigned fabriziopandini and sbueringer May 15, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2023

killianmuldoon mentioned this pull request May 15, 2023

🌱 Make kind testing cluster dualstack #8663

Closed

fabriziopandini reviewed May 16, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 16, 2023

k8s-ci-robot merged commit ae6832c into kubernetes-sigs:main May 16, 2023

k8s-ci-robot added this to the v1.5 milestone May 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Add dualstack e2e test #8517

🌱 Add dualstack e2e test #8517

killianmuldoon commented Apr 12, 2023 •

edited

Loading

killianmuldoon left a comment

killianmuldoon Apr 12, 2023

lubronzhan Apr 13, 2023 •

edited

Loading

killianmuldoon Apr 13, 2023

lubronzhan Apr 13, 2023

christianang Apr 13, 2023

chrischdi left a comment

k8s-ci-robot commented Apr 13, 2023

killianmuldoon commented Apr 13, 2023

sbueringer left a comment •

edited

Loading

christianang commented Apr 13, 2023

sbueringer commented May 15, 2023 •

edited

Loading

killianmuldoon commented May 15, 2023

killianmuldoon commented May 15, 2023

sbueringer commented May 15, 2023

k8s-ci-robot commented May 15, 2023

fabriziopandini left a comment

fabriziopandini May 15, 2023

k8s-ci-robot commented May 16, 2023

🌱 Add dualstack e2e test #8517

🌱 Add dualstack e2e test #8517

Conversation

killianmuldoon commented Apr 12, 2023 • edited Loading

killianmuldoon left a comment

Choose a reason for hiding this comment

killianmuldoon Apr 12, 2023

Choose a reason for hiding this comment

lubronzhan Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

killianmuldoon Apr 13, 2023

Choose a reason for hiding this comment

lubronzhan Apr 13, 2023

Choose a reason for hiding this comment

christianang Apr 13, 2023

Choose a reason for hiding this comment

chrischdi left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 13, 2023

killianmuldoon commented Apr 13, 2023

sbueringer left a comment • edited Loading

Choose a reason for hiding this comment

christianang commented Apr 13, 2023

sbueringer commented May 15, 2023 • edited Loading

killianmuldoon commented May 15, 2023

killianmuldoon commented May 15, 2023

sbueringer commented May 15, 2023

k8s-ci-robot commented May 15, 2023

fabriziopandini left a comment

Choose a reason for hiding this comment

fabriziopandini May 15, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented May 16, 2023

killianmuldoon commented Apr 12, 2023 •

edited

Loading

lubronzhan Apr 13, 2023 •

edited

Loading

sbueringer left a comment •

edited

Loading

sbueringer commented May 15, 2023 •

edited

Loading