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

Azure support for dualstack LB services #80485

Merged
merged 1 commit into from Aug 29, 2019

Conversation

@khenidak
Copy link
Contributor

commented Jul 23, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
Add azure support for dual stack services. IPv4 and IPv6 ingress. CANNOT be merged UNTIL #79386 (dual stack phase 2: services endpoints) and #79576 (dual stack phase 2: kube-proxy)

Which issue(s) this PR fixes:

Special notes for your reviewer:
Please review, but don't approve until dep PRs are merged. only last (Azure) commit needs a review. the rest of commits are part of the dep PR.

Does this PR introduce a user-facing change?:

Azure supports IPv6 only on ELB not ILB. The cloud provider will return an error if the service is internal and is IPv6.

Notes on LB name:
to ensure backword and forward compat:
- SingleStack -v4 (pre v1.16) => BackendPool name == clusterName
- SingleStack -v6 => BackendPool name == clusterName (all cluster bootstrap uses this name)
DualStack:
=> IPv4 BackendPool name == clusterName
=> IPv6 BackendPool name == <clusterName>-IPv6
This result into:
- clusters moving from IPv4 to duakstack will require no changes
- clusters moving from IPv6 (while not seen in the wild, we can not rule out thier existance) to dualstack will require deleting backend pools (the reconciler will take care of creating correct backendpools)

@feiskyer @kubernetes/sig-azure

@roycaihw

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

@k8s-ci-robot k8s-ci-robot requested a review from thockin Jul 25, 2019
@khenidak khenidak force-pushed the khenidak:lb-dualstack-support branch from 346f7ba to 2277c37 Jul 29, 2019
@khenidak

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

@feiskyer et all, this PR is based on #79386. all tests will fail until the dependent PR merges. I hat to drop phase 2 commits from here to avoid duplicate work during rebasing.

@khenidak

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

/lgtm
/hold (merge queue sequencing)

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

@khenidak: you cannot LGTM your own PR.

In response to this:

/lgtm
/hold (merge queue sequencing)

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.

@feiskyer

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 28, 2019
@khenidak

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

/test pull-kubernetes-conformance-kind-ipv6

@feiskyer

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

/approve

@thockin Could you help to approve this changes?

@khenidak

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

/approve

@thockin Could you help to approve this changes?

@feiskyer i think once #79386 merges we will only need azure approval (this pr is layered on top of it), worst case scenario i will rebase in the morning and get @justaugustus to approve.

Thank you

@khenidak khenidak force-pushed the khenidak:lb-dualstack-support branch from 7795bd2 to bd9108b Aug 29, 2019
@khenidak

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

/test pull-kubernetes-bazel-build

@andyzhangx

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

/priority important-soon
/lgtm

Copy link
Member

left a comment

Cool.

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@justaugustus

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

/test pull-kubernetes-integration

@fejta-bot

This comment has been minimized.

Copy link

commented Aug 29, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@lachie83

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

/test pull-kubernetes-bazel-test

@lachie83

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

/test pull-kubernetes-bazel-build

@khenidak

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

/test pull-kubernetes-e2e-gce

@k8s-ci-robot k8s-ci-robot merged commit 3e7e12d into kubernetes:master Aug 29, 2019
24 checks passed
24 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-conformance-kind-ipv6 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 Skipped.
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 Skipped.
tide In merge pool.
Details
// azure does not support ILB for IPv6 yet.
// TODO: remove this check when ILB supports IPv6 *and* the SDK
// have been rev'ed to 2019* version
if utilnet.IsIPv6String(service.Spec.ClusterIP) {

This comment has been minimized.

Copy link
@andrewsykim

andrewsykim Sep 3, 2019

Member

This change isn't feature gated, is that okay because we already assume ClusterIP can never by v6 for Azure internal LB?

This comment has been minimized.

Copy link
@khenidak

khenidak Sep 5, 2019

Author Contributor

This change is not gated, because we want to be able to run IPv6 only clusters on Azure. Also, ILB on Azure does not support IPv6 on ILB (it is in the release notes of this PR)

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.