autodetect global addresses on loopback interfaces #95790

aojea · 2020-10-22T11:04:04Z

What type of PR is this?

/kind bug
/kind feature

What this PR does / why we need it:

There are some network scenarios that, in order to have high availability based
on routing protocols, you may have multiple interfaces in the host and you use
a routing protocol to configure the routing.
It is common to not use global addresses on those interfaces because you don't want
them to be reachable, so you assign the global address to the loopback interface.

Loopback interfaces are always up, regardless of the states of physical interfaces.
They are not subject to interface problems, i.e., if the interface is down or flapping
you can not reach the IP despite you have connectivity through another interface.

We should consider global ip addresses on loopback interfaces when:

the host has default routes
there are no global IPs on those interfaces.

There can be more cases in which you have global addresses on the interfaces too,
but that will open an explosion of scenarios hard to support and to "autodetect"
It will be a cluster admin responsibility to configure the network in such
scenarios.

Which issue(s) this PR fixes:

Fixes #95779

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

The apimachinery util/net function used to detect the bind address `ResolveBindAddress()` takes into consideration global IP addresses on loopback interfaces when 1) the host has default routes, or 2) there are no global IPs on those interfaces in order to support more complex network scenarios like BGP Unnumbered RFC 5549

k8s-ci-robot · 2020-10-22T11:04:12Z

@aojea: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

aojea · 2020-10-22T11:05:30Z

/assign @thockin @liggitt

There are some network scenarios that, in order to have high availability based on routing protocols, you may have multiple interfaces in the host and you use a routing protocol to configure the routing in the host. It is common to not use global addresses on those interfaces because you don't want them to be reachable, so you assign the global address to the loopback interface. Loopback interfaces are always up, regardless of the states of physical interfaces. They are not subject to interface problems, i.e., if the interface is down or flapping you can not reach the IP despite you have connectivity through another interface. We should consider global ip addresses on loopback interfaces when: - the host has default routes - there are no global IPs on those interfaces. There can be more cases in which you have global addresses on the interfaces too, but that will open an explosion of scenarios hard to support and to "autodetect" It will be a cluster admin responsability to configure the network in such scenarios.

liggitt · 2020-10-22T14:39:48Z

I don't have the networking context to know if this change makes sense. Can you clarify the following:

would this change behavior for callers that are currently succeeding, or only for callers that currently fail?
does this change the function to make it possible to return loopback interfaces where that was not previously possible? do all callers expect this?

neolit123 · 2020-10-22T14:58:19Z

i think we should reword the release note to exclude code details and only leave the user facing part for affected components e.g.:

kube-apiserver/kubeadm: for detection of bind-addresses take into consideration global IP addresses on loopback interfaces when:
    - the host has default routes
    - there are no global IPs on those interfaces.
in order to support more complex network scenarios like BGP Unnumbered RFC 5549

aojea · 2020-10-22T15:04:42Z

would this change behavior for callers that are currently succeeding, or only for callers that currently fail?

for current callers that are succeeding there is no change, I've also added unit test to verify that we don't break that

does this change the function to make it possible to return loopback interfaces where that was not previously possible? do all callers expect this?

the goal of ResolveBindAddress is to find an address that you can use to bind, and that address has to be reachable, it uses chooseHostInterfaceFromRoute to detect that IP. The heuristic is, if I have a route through one interface and that interface has a global IP address, this is the one that I want to use. This is a typical scenario in a cloud environment, the host is a VM with one interface and one IP assigned to that interface.

In more complex scenarios that does not have the advantage of one IaaS taking care of the underlay network of the host, like in baremetal, you use to have more than one interface connected to different switches, to provide redundancy.

There are different ways to provide redundancy with multiple interfaces, but that opens a lot of scenarios and I don't think that we must support any of them, but from all of those there is one that we can be sure that the the global address in the loopback interface is the right one to return unequivocally:

the host has a default route (presumably, it means it is not an isolated host and has connectivity)
the interfaces does not have global addresses and the loopback has (that means that it has to be configured that way by someone, loopback interfaces by default don't have global addresses)

aojea · 2020-10-22T15:15:01Z

staging/src/k8s.io/apimachinery/pkg/util/net/interface_test.go

+		{"interface and loopback with global, IPv6", routeV6, globalsNetworkInterface{}, preferIPv6, net.ParseIP("fd00::200")},
+		{"interface and loopback with global, dual stack prefer IPv4", bothRoutes, globalsNetworkInterface{}, preferIPv4, net.ParseIP("192.168.1.1")},
+		{"interface and loopback with global, dual stack prefer IPv6", bothRoutes, globalsNetworkInterface{}, preferIPv6, net.ParseIP("fd00::200")},
+		{"interface and loopback with global, no routes", noRoutes, globalsNetworkInterface{}, preferIPv6, nil},


@liggitt these are the tests that verify that we don't modify previous behaviour, 192.168.1.1 and fd00::200 are the ones assigned to the interfaces and we check that take precedence over the ones assigned to the loopback interfaces

fedebongio · 2020-10-22T20:10:39Z

/sig network
/cc @bowei @deads2k

aojea · 2020-10-22T20:33:15Z

/retest

andrewsykim · 2020-10-26T01:48:29Z

staging/src/k8s.io/apimachinery/pkg/util/net/interface.go

+			// In case of network setups where default routes are present, but network
+			// interfaces use only link-local addresses (e.g. as described in RFC5549).
+			// the global IP is assigned to the loopback interface
+			loopbackIP, err := getIPFromInterface(LoopbackInterfaceName, family, nw)


Do we need to make a similar check for loopback addresses in chooseIPFromHostInterfaces? Currently it also skips the loopback interface:

kubernetes/staging/src/k8s.io/apimachinery/pkg/util/net/interface.go

Lines 296 to 299 in 078d2e6

if isLoopbackOrPointToPoint(&intf) {

klog.V(4).Infof("Skipping: LB or P2P interface %q", intf.Name)

continue

}

Ah I see now -- this is only relevant when there's a default route and chooseIPFromHostInterfaces is not called unless there are no default routes anyways.

roycaihw · 2020-10-29T20:05:37Z

SIG apimachinery passed this again and still wait for SIG network feedback.

aojea · 2020-10-29T20:29:57Z

/retest
/assign @thockin

staging/src/k8s.io/apimachinery/pkg/util/net/interface.go

aojea · 2020-11-14T22:01:24Z

/retest

thockin · 2020-11-16T17:08:09Z

Thanks!

/lgtm
/approve

k8s-ci-robot · 2020-11-16T17:08:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, 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:

~~staging/src/k8s.io/apimachinery/pkg/OWNERS~~ [thockin]

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

aojea · 2020-11-17T05:48:06Z

do we want this in 1.20 @thockin ?

leilajal · 2020-11-17T21:05:35Z

/remove-sig api-machinery

aojea · 2020-11-22T10:49:25Z

Let's address next version, too late in this cycle for this change IMHO
/milestone 1.21

k8s-ci-robot · 2020-11-22T10:49:27Z

@aojea: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your and have them propose you as an additional delegate for this responsibility.

In response to this:

Let's address next version, too late in this cycle for this change IMHO
/milestone 1.21

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.

andrewsykim · 2020-11-22T15:21:11Z

/milestone 1.21

k8s-ci-robot · 2020-11-22T15:21:13Z

@andrewsykim: The provided milestone is not valid for this repository. Milestones in this repository: [next-candidate, v1.16, v1.17, v1.18, v1.19, v1.20, v1.21, v1.22]

Use /milestone clear to clear the milestone.

In response to this:

/milestone 1.21

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.

andrewsykim · 2020-11-22T15:21:23Z

/milestone v1.21

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 22, 2020

k8s-ci-robot requested review from gmarek and wojtek-t October 22, 2020 11:04

k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 22, 2020

k8s-ci-robot assigned liggitt and thockin Oct 22, 2020

aojea force-pushed the loopnet branch from 2777625 to 078d2e6 Compare October 22, 2020 11:31

aojea commented Oct 22, 2020

View reviewed changes

k8s-ci-robot requested review from bowei and deads2k October 22, 2020 20:10

k8s-ci-robot added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Oct 22, 2020

andrewsykim reviewed Oct 26, 2020

View reviewed changes

thockin reviewed Nov 13, 2020

View reviewed changes

staging/src/k8s.io/apimachinery/pkg/util/net/interface.go Outdated Show resolved Hide resolved

get ips from loopback interfaces

432e74d

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 14, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2020

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

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Nov 17, 2020

k8s-ci-robot added this to the v1.21 milestone Nov 22, 2020

k8s-ci-robot merged commit 403d0cd into kubernetes:master Dec 9, 2020

github-actions bot mentioned this pull request Dec 16, 2020

Week Ending December 13, 2020 dev-obs/actus#308

Open

aojea mentioned this pull request Jun 10, 2021

Add blog post about avoiding the default address when using kubeadm kubernetes/website#28331

Closed

This was referenced Jan 12, 2022

limit the scope of 169.254.169.252/32 to host #107502

Merged

dnat to 169.254.169.252 for metadata server traffic #100639

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

autodetect global addresses on loopback interfaces #95790

autodetect global addresses on loopback interfaces #95790

aojea commented Oct 22, 2020 •

edited by liggitt

k8s-ci-robot commented Oct 22, 2020

aojea commented Oct 22, 2020

liggitt commented Oct 22, 2020

neolit123 commented Oct 22, 2020

aojea commented Oct 22, 2020 •

edited

aojea Oct 22, 2020

fedebongio commented Oct 22, 2020

aojea commented Oct 22, 2020

andrewsykim Oct 26, 2020

andrewsykim Oct 26, 2020

roycaihw commented Oct 29, 2020

aojea commented Oct 29, 2020

aojea commented Nov 14, 2020

thockin commented Nov 16, 2020

k8s-ci-robot commented Nov 16, 2020

aojea commented Nov 17, 2020

leilajal commented Nov 17, 2020

aojea commented Nov 22, 2020

k8s-ci-robot commented Nov 22, 2020

andrewsykim commented Nov 22, 2020

k8s-ci-robot commented Nov 22, 2020

andrewsykim commented Nov 22, 2020

	if isLoopbackOrPointToPoint(&intf) {
	klog.V(4).Infof("Skipping: LB or P2P interface %q", intf.Name)
	continue
	}

autodetect global addresses on loopback interfaces #95790

autodetect global addresses on loopback interfaces #95790

Conversation

aojea commented Oct 22, 2020 • edited by liggitt

k8s-ci-robot commented Oct 22, 2020

aojea commented Oct 22, 2020

liggitt commented Oct 22, 2020

neolit123 commented Oct 22, 2020

aojea commented Oct 22, 2020 • edited

aojea Oct 22, 2020

Choose a reason for hiding this comment

fedebongio commented Oct 22, 2020

aojea commented Oct 22, 2020

andrewsykim Oct 26, 2020

Choose a reason for hiding this comment

andrewsykim Oct 26, 2020

Choose a reason for hiding this comment

roycaihw commented Oct 29, 2020

aojea commented Oct 29, 2020

aojea commented Nov 14, 2020

thockin commented Nov 16, 2020

k8s-ci-robot commented Nov 16, 2020

aojea commented Nov 17, 2020

leilajal commented Nov 17, 2020

aojea commented Nov 22, 2020

k8s-ci-robot commented Nov 22, 2020

andrewsykim commented Nov 22, 2020

k8s-ci-robot commented Nov 22, 2020

andrewsykim commented Nov 22, 2020

aojea commented Oct 22, 2020 •

edited by liggitt

aojea commented Oct 22, 2020 •

edited