Skip to content

Commit

Permalink
Make bootstrap IP discovery backwards compatible with previous assump…
Browse files Browse the repository at this point in the history
…tions

Before this patch, the new bootstrap IP discovery mechanism would fail bootstrapping
if no IP could be intelligently discovered. A side-effect of that is effectively
validating the machine network CIDR by asserting the bootstrap IPs ability to be
discovered within it. Because there may still be edge cases where we fail to detect
but where the old assumption to choose the "first IP" would still work, we could
introduce an undue burden to fix all existing uses of machine network CIDR even
when our fallback could continue to work in those cases.

This patch adds a fallback behavior so that when intelligent discovery fails, the
first listed IP is selected with a warning, preserving the original discovery
behavior.

This does effectively mean that clusters can still fail to bootstrap if even the
first IP assumption is wrong, but we can presumably use those failures to further
improve detection.

A worthwhile future improvement would be to find a way to more loudly and clearly
surface to the user when we're blindly guessing about the IP, as the resulting
downstream failure may obfuscate the source of failure if bootkube logs are lost.
  • Loading branch information
ironcladlou committed Jul 15, 2020
1 parent dbd49cc commit a79e547
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 15 deletions.
42 changes: 27 additions & 15 deletions pkg/cmd/render/bootstrap_ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ func NetlinkBootstrapIPLocator() *bootstrapIPLocator {
}
}

// BootstrapIPLocator tries to find the bootstrap IP for the machine. It should
// go through the effort of identifying the IP based on its inclusion in the machine
// network CIDR, routability, etc. and fall back to using the first listed IP
// as a last resort (and for compatibility with old behavior).
type BootstrapIPLocator interface {
getBootstrapIP(ipv6 bool, machineCIDR string, excludedIPs []string) (net.IP, error)
}
Expand Down Expand Up @@ -49,32 +53,40 @@ func (l *bootstrapIPLocator) getBootstrapIP(ipv6 bool, machineCIDR string, exclu
ContainedByCIDR(machineCIDR),
AddressNotIn(excludedIPs...),
)
addresses, err := routableAddresses(addrMap, routeMap, ips, addressFilter, NonDefaultRoute)
discoveredAddresses, err := routableAddresses(addrMap, routeMap, ips, addressFilter, NonDefaultRoute)
if err != nil {
return nil, err
}
if len(addresses) == 0 {
return nil, fmt.Errorf("couldn't find any bootstrap node IPs")
if len(discoveredAddresses) > 1 {
klog.Warningf("found multiple candidate bootstrap IPs; only the first one will be considered: %+v", discoveredAddresses)
}
if len(addresses) > 1 {
klog.Warningf("found multiple candidate bootstrap IPs; using only the first one: %+v", addresses)

findIP := func(addresses []net.IP) (net.IP, bool) {
for _, ip := range addresses {
// IPv6
if ipv6 && ip.To4() == nil {
return ip, true
}
// IPv4
if !ipv6 && ip.To4() != nil {
return ip, true
}
}
return nil, false
}

var bootstrapIP net.IP
for _, ip := range addresses {
// IPv6
if ipv6 && ip.To4() == nil {
if ip, found := findIP(discoveredAddresses); found {
bootstrapIP = ip
} else {
klog.Warningf("couldn't detect the bootstrap IP automatically, falling back to the first listed address")
if ip, found := findIP(ips); found {
bootstrapIP = ip
break
}
// IPv4
if !ipv6 && ip.To4() != nil {
bootstrapIP = ip
break
}
}

if bootstrapIP == nil {
return nil, fmt.Errorf("couldn't find a suitable bootstrap node IP from candidates: %+v", addresses)
return nil, fmt.Errorf("couldn't find a suitable bootstrap node IP from candidates\nall: %+v\ndiscovered: %+v", ips, discoveredAddresses)
}

return bootstrapIP, nil
Expand Down
15 changes: 15 additions & 0 deletions pkg/cmd/render/bootstrap_ip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,21 @@ func TestBootstrapIPLocator(t *testing.T) {
exclude: []string{"192.168.125.5"},
expect: net.ParseIP("192.168.125.112"),
},
{
name: "fallback to first IP",
machineCIDR: "192.168.125.0/24",
ips: newIPs(
"10.88.0.1",
"172.17.0.1",
),
addrMap: newAddrMap(
withDevice(0, "10.88.0.1"),
withDevice(1, "172.17.0.1"),
),
routeMap: map[int][]netlink.Route{},
exclude: []string{},
expect: net.ParseIP("10.88.0.1"),
},
}

for _, scenario := range scenarios {
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/render/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ func (t *TemplateData) setBootstrapIP(machineCIDR string, ipv6 bool, excludedIPs
if err != nil {
return err
}
klog.Infof("using bootstrap IP %s", ip.String())
t.BootstrapIP = ip.String()
return nil
}
Expand Down

0 comments on commit a79e547

Please sign in to comment.