Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ipv6 dual stack (Phase 1 - ALPHA) #73977

Merged
merged 10 commits into from Jul 3, 2019

Conversation

@khenidak
Copy link
Contributor

commented Feb 12, 2019

What type of PR is this?
/kind api-change

/kind feature

Implements: kubernetes/enhancements#808
Included:

  • Types modification.
  • Conversions.
  • new conversion unit tests.
  • (updated existing unit tests to match the type changes).
  • node ipam controller that supports multi cidrs.
  • unit tests: node ipam mutli cidr.
  • azure cloud provider support for mutli routes per node.
  • unit tests: azure cloud provider support for mutli routes per node.
  • kubenet support for dual-stack.
  • revise host networking for dual-stack (v1.16).
  • unit tests: kubenet support for dual-stack.
  • e2e tests (as a stand alone PR referenced below).
To configure controller manager to use ipv6dual stack:
use --cluster-cidr="<cidr1>,<cidr2>".

Notes:
 
1. Only the first two cidrs are used (soft limits for Alpha, might be lifted later on). 
2. Only the "RangeAllocator" (default) is allowed as a value for --cidr-allocator-type . Cloud allocators are not compatible with ipv6dualstack 

CC @lachie83 @thockin

What is in the box? (alpha status)

  • Node ipam controller now supports multi-cidr per node (node.Spec.PodCIDRs).
  • Pod.PodStatus.PodIPs supports multiple IPs.
  • Route Controller now supports creating routes per node's cidr.
  • kubenet now supports dualstack.
  • Azure support Route() interface implementation for dualstack.
  • Known issue: kubenet will always force reporting ipv4, ipv6 (irrespective of podCIDRs order).
  • updates to CRI to support reporting multiple IPs per PodSandBox.

Known Issues

  • Cluster ipv6 CIDRS mask bigger than 24 will fail
  • ipv6 cidr assignment is using the default ipv4 cidr /24 (Future: add controls over v6 cidr size)
  • kubenet forces v4,v6 reporting of IPs, users who mustv6,v4 as --cluster-cidr
  • Masquerading is not done by kubenet. Users will have to use ip-masq-agent to perform masquerading correctly for ipv6. A standing PR has been created to support this feature kubernetes-incubator/ip-masq-agent#45
@chrisohaver

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

@khenidak khenidak force-pushed the khenidak:ipv6dualstack branch from 217ab59 to 11ac486 Jul 2, 2019

khenidak added some commits Jun 21, 2019

@khenidak khenidak force-pushed the khenidak:ipv6dualstack branch from 11ac486 to 3357371 Jul 2, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

@khenidak: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross 0af3d67 link /test pull-kubernetes-cross

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

khenidak added some commits Jul 2, 2019

@khenidak khenidak force-pushed the khenidak:ipv6dualstack branch from 3357371 to af552ab Jul 2, 2019

@dims

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

/test pull-kubernetes-local-e2e

@thockin

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 2, 2019

@thockin thockin added the approved label Jul 2, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: dcbw, feiskyer, khenidak, thockin

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:

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

1 similar comment
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: dcbw, feiskyer, khenidak, thockin

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:

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 k8s-ci-robot merged commit a33840e into kubernetes:master Jul 3, 2019

23 checks passed

cla/linuxfoundation khenidak authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Job succeeded.
Details
tide In merge pool.
Details
return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily", len(clusterCIDRs))
}

// failure: more than cidrs is not allowed even with dual stack

This comment has been minimized.

Copy link
@tedyu

tedyu Jul 3, 2019

Contributor

more than cidrs is -> more than two cidrs are

return nil, false, err
}

// failure: more than one cidr and dual stack is not enabled

This comment has been minimized.

Copy link
@tedyu

tedyu Jul 3, 2019

Contributor

It seems the following code is almost the same as the check in core.go

Consider consolidating the code into a helper.

@saschagrunert

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

@khenidak Do you think that we can change the release notes for this PR?

It would be awesome if we could state all user-facing changes within the release-notes block, like the dual stack support of kubenet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.