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

autodetect advertises address with IPv6 #1195

Closed
wants to merge 1 commit into from

Conversation

aojea
Copy link
Contributor

@aojea aojea commented Dec 20, 2019

apiserver and kubelet advertise ipv6 endpoints when binding to ipv6 since 1.17

This removes the need to hardcode the kubelet and kube-apiserver IPs when using IPv6

Fixes : #1154

Unblocks kubernetes/kubernetes#86480

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 20, 2019
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 20, 2019
@liggitt
Copy link
Contributor

liggitt commented Dec 20, 2019

/hold

it's unclear kubernetes/kubernetes#86480 is a compatible change... we should probably wait for a decision there before merging this

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 20, 2019
@aojea
Copy link
Contributor Author

aojea commented Dec 20, 2019

/hold

it's unclear kubernetes/kubernetes#86480 is a compatible change... we should probably wait for a decision there before merging this

yeah, thanks for the clarification, I've also added the WIP to this PR,
I'm finding IP families inconsistencies here too :/

@aojea
Copy link
Contributor Author

aojea commented Dec 20, 2019

/retest

@aojea
Copy link
Contributor Author

aojea commented Dec 20, 2019

2/2 failure of the same 2 tests, will investigate later
[sig-api-machinery] ResourceQuota should verify ResourceQuota with best effort scope. [Conformance]

[sig-api-machinery] AdmissionWebhook [Privileged:ClusterAdmin] should not be able to mutate or prevent deletion of webhook configuration objects [Conformance] expand_less

https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_kind/1195/pull-kind-conformance-parallel-ipv6/1208054793159839744/

/retest

@aojea
Copy link
Contributor Author

aojea commented Dec 20, 2019

It's failing because the kube-scheduler and the kube-controller-manager are being restarted constantly because they have the wrong http probes configured kubernetes/kubernetes#86493

@aojea
Copy link
Contributor Author

aojea commented Jan 4, 2020

🤔

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_kind/1195/pull-kind-conformance-parallel-ipv6/1213414923934109697/artifacts/logs/kind-control-plane/containers/kube-apiserver-kind-control-plane_kube-system_kube-apiserver-b5b1764ec4d32744472affd4653f94243e70947b6b5b888eb9a3b837e9035147.log

2020-01-04T11:13:47.951628264Z stderr F I0104 11:13:47.951353 1 server.go:596] external host was not specified, using 172.17.0.4

2020-01-04T11:13:48.948109526Z stderr F I0104 11:13:48.947888 1 rest.go:113] the default service ipfamily for this cluster is: IPv6

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 4, 2020
@aojea aojea changed the title WIP apiserver ipv6 bind to :: autodetect apiserver advertise address with IPv6 Jan 4, 2020
@k8s-ci-robot 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 Jan 4, 2020
@aojea
Copy link
Contributor Author

aojea commented Jan 4, 2020

/hold cancel
all the changes needed were merged and we can benefit from the apiserver advertise address autodetection.
just only remains the kubelet to autodetect the node-ip family and we can get rid of hardcoding the node-address
kubernetes/kubernetes#85850
/assign @BenTheElder

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2020
@BenTheElder
Copy link
Member

Is this going to break with older versions of kubernetes then...?

@aojea
Copy link
Contributor Author

aojea commented Jan 7, 2020

Is this going to break with older versions of kubernetes then...?

it will break only with kubeadm.k8s.io/v1beta2 , the commit didn't modify previous kubeadm templates so you can still deploy IPv6 clusters with previous kubernetes versions with kubeadm.k8s.io/v1beta1

Also, I'm not sure about this, but it may be possible to patch the preconfigured templates with a config file, no?

@aojea
Copy link
Contributor Author

aojea commented Jan 7, 2020

/hold
let's wait for
kubernetes/kubernetes#85850

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 7, 2020
@aojea aojea changed the title autodetect apiserver advertise address with IPv6 autodetect advertises address with IPv6 Jan 7, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aojea
To complete the pull request process, please assign bentheelder
You can assign the PR to them by writing /assign @bentheelder in a comment when ready.

The full list of commands accepted by this bot can be found 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 added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 7, 2020
@BenTheElder
Copy link
Member

That's not good enough, we shouldn't break any released k8s.

This needs to be version gated.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 14, 2020
@aojea aojea force-pushed the listenv6 branch 3 times, most recently from 569ee72 to 4f5a341 Compare January 14, 2020 16:02
use `::` as argument to autodetect the IPv6 adderss
bind apiserver to ipv6 address
kubelet autodetect node ip
@k8s-ci-robot
Copy link
Contributor

@aojea: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kind-conformance-parallel-ipv6 4452848 link /test pull-kind-conformance-parallel-ipv6

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.

@aojea
Copy link
Contributor Author

aojea commented Feb 16, 2020

this is no autodetecting the node-ip using :: as parameter of kubelet because the node name doesn't resolve the IPv6 address, using the following code:

package main

import (
        "fmt"
        "net"
        "os"
        "runtime"
)

func main() {
        fmt.Println("GOOS:", runtime.GOOS)
        // Resolver
        addrs, _ := net.LookupIP(os.Args[1])
        fmt.Println("net.LookupIP addrs:", addrs)

}

we can confirm it

 /lookup kind-control-plane
GOOS: linux
net.LookupIP addrs: [172.17.0.2]

@aojea
Copy link
Contributor Author

aojea commented Feb 16, 2020

it was fixed in docker recently by this moby/libnetwork@e3bacd6

@aojea
Copy link
Contributor Author

aojea commented Feb 16, 2020

/hold
it seems we should wait for a new release of docker with the patch mentioned before included
If I understood docker releases correctly, it should be v19.03.6

@BenTheElder
Copy link
Member

BenTheElder commented Feb 18, 2020 via email

@aojea
Copy link
Contributor Author

aojea commented Feb 18, 2020

you are absolutely right ... I was so focused in this PR and chasing this odd behavior that forget these things 😅
/close

@k8s-ci-robot
Copy link
Contributor

@aojea: Closed this PR.

In response to this:

you are absolutely right ... I was so focused in this PR and chasing this odd behavior that forget these things 😅
/close

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.

@BenTheElder
Copy link
Member

fixed in #1521

@aojea aojea deleted the listenv6 branch August 9, 2020 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new k8s IPv6 configuration improvements to KIND
4 participants