diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index a3b6bdec92f..b73290d9d37 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -603,7 +603,8 @@ func newProxyServer(config *kubeproxyconfig.KubeProxyConfiguration, master strin return nil, err } - s.PrimaryIPFamily, s.NodeIPs = detectNodeIPs(s.Client, s.Hostname, config.BindAddress) + rawNodeIPs := getNodeIPs(s.Client, s.Hostname) + s.PrimaryIPFamily, s.NodeIPs = detectNodeIPs(rawNodeIPs, config.BindAddress) s.Broadcaster = events.NewBroadcaster(&events.EventSinkImpl{Interface: s.Client.EventsV1()}) s.Recorder = s.Broadcaster.NewRecorder(proxyconfigscheme.Scheme, "kube-proxy") @@ -955,27 +956,27 @@ func (s *ProxyServer) birthCry() { // // The order of precedence is: // 1. if bindAddress is not 0.0.0.0 or ::, then it is used as the primary IP. -// 2. if the Node object can be fetched, then its address(es) is/are used +// 2. if rawNodeIPs is not empty, then its address(es) is/are used // 3. otherwise the node IPs are 127.0.0.1 and ::1 -func detectNodeIPs(client clientset.Interface, hostname, bindAddress string) (v1.IPFamily, map[v1.IPFamily]net.IP) { +func detectNodeIPs(rawNodeIPs []net.IP, bindAddress string) (v1.IPFamily, map[v1.IPFamily]net.IP) { primaryFamily := v1.IPv4Protocol nodeIPs := map[v1.IPFamily]net.IP{ v1.IPv4Protocol: net.IPv4(127, 0, 0, 1), v1.IPv6Protocol: net.IPv6loopback, } - if ips := getNodeIPs(client, hostname); len(ips) > 0 { - if !netutils.IsIPv4(ips[0]) { + if len(rawNodeIPs) > 0 { + if !netutils.IsIPv4(rawNodeIPs[0]) { primaryFamily = v1.IPv6Protocol } - nodeIPs[primaryFamily] = ips[0] - if len(ips) > 1 { + nodeIPs[primaryFamily] = rawNodeIPs[0] + if len(rawNodeIPs) > 1 { // If more than one address is returned, they are guaranteed to be of different families family := v1.IPv4Protocol - if !netutils.IsIPv4(ips[1]) { + if !netutils.IsIPv4(rawNodeIPs[1]) { family = v1.IPv6Protocol } - nodeIPs[family] = ips[1] + nodeIPs[family] = rawNodeIPs[1] } } diff --git a/cmd/kube-proxy/app/server_test.go b/cmd/kube-proxy/app/server_test.go index e459839f690..5990181693f 100644 --- a/cmd/kube-proxy/app/server_test.go +++ b/cmd/kube-proxy/app/server_test.go @@ -17,9 +17,11 @@ limitations under the License. package app import ( + "context" "errors" "fmt" "io/ioutil" + "net" "path" "testing" "time" @@ -36,6 +38,7 @@ import ( componentbaseconfig "k8s.io/component-base/config" logsapi "k8s.io/component-base/logs/api/v1" kubeproxyconfig "k8s.io/kubernetes/pkg/proxy/apis/config" + netutils "k8s.io/utils/net" "k8s.io/utils/ptr" ) @@ -593,11 +596,7 @@ func TestAddressFromDeprecatedFlags(t *testing.T) { } } -func makeNodeWithAddresses(name, internal, external string) *v1.Node { - if name == "" { - return &v1.Node{} - } - +func makeNodeWithAddress(name, primaryIP string) *v1.Node { node := &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -607,26 +606,76 @@ func makeNodeWithAddresses(name, internal, external string) *v1.Node { }, } - if internal != "" { + if primaryIP != "" { node.Status.Addresses = append(node.Status.Addresses, - v1.NodeAddress{Type: v1.NodeInternalIP, Address: internal}, - ) - } - - if external != "" { - node.Status.Addresses = append(node.Status.Addresses, - v1.NodeAddress{Type: v1.NodeExternalIP, Address: external}, + v1.NodeAddress{Type: v1.NodeInternalIP, Address: primaryIP}, ) } return node } +// Test that getNodeIPs retries on failure +func Test_getNodeIPs(t *testing.T) { + var chans [3]chan error + + client := clientsetfake.NewSimpleClientset( + // node1 initially has no IP address. + makeNodeWithAddress("node1", ""), + + // node2 initially has an invalid IP address. + makeNodeWithAddress("node2", "invalid-ip"), + + // node3 initially does not exist. + ) + + for i := range chans { + chans[i] = make(chan error) + ch := chans[i] + nodeName := fmt.Sprintf("node%d", i+1) + expectIP := fmt.Sprintf("192.168.0.%d", i+1) + go func() { + ips := getNodeIPs(client, nodeName) + if len(ips) == 0 { + ch <- fmt.Errorf("expected IP %s for %s but got nil", expectIP, nodeName) + } else if ips[0].String() != expectIP { + ch <- fmt.Errorf("expected IP %s for %s but got %s", expectIP, nodeName, ips[0].String()) + } else if len(ips) != 1 { + ch <- fmt.Errorf("expected IP %s for %s but got multiple IPs", expectIP, nodeName) + } + close(ch) + }() + } + + // Give the goroutines time to fetch the bad/non-existent nodes, then fix them. + time.Sleep(1200 * time.Millisecond) + + _, _ = client.CoreV1().Nodes().UpdateStatus(context.TODO(), + makeNodeWithAddress("node1", "192.168.0.1"), + metav1.UpdateOptions{}, + ) + _, _ = client.CoreV1().Nodes().UpdateStatus(context.TODO(), + makeNodeWithAddress("node2", "192.168.0.2"), + metav1.UpdateOptions{}, + ) + _, _ = client.CoreV1().Nodes().Create(context.TODO(), + makeNodeWithAddress("node3", "192.168.0.3"), + metav1.CreateOptions{}, + ) + + // Ensure each getNodeIP completed as expected + for i := range chans { + err := <-chans[i] + if err != nil { + t.Error(err.Error()) + } + } +} + func Test_detectNodeIPs(t *testing.T) { cases := []struct { name string - nodeInfo *v1.Node - hostname string + rawNodeIPs []net.IP bindAddress string expectedFamily v1.IPFamily expectedIPv4 string @@ -634,8 +683,7 @@ func Test_detectNodeIPs(t *testing.T) { }{ { name: "Bind address IPv4 unicast address and no Node object", - nodeInfo: makeNodeWithAddresses("fakeHost", "", ""), - hostname: "fakeHost", + rawNodeIPs: nil, bindAddress: "10.0.0.1", expectedFamily: v1.IPv4Protocol, expectedIPv4: "10.0.0.1", @@ -643,38 +691,31 @@ func Test_detectNodeIPs(t *testing.T) { }, { name: "Bind address IPv6 unicast address and no Node object", - nodeInfo: makeNodeWithAddresses("fakeHost", "", ""), - hostname: "fakeHost", + rawNodeIPs: nil, bindAddress: "fd00:4321::2", expectedFamily: v1.IPv6Protocol, expectedIPv4: "127.0.0.1", expectedIPv6: "fd00:4321::2", }, { - name: "No Valid IP found", - nodeInfo: makeNodeWithAddresses("fakeHost", "", ""), - hostname: "fakeHost", + name: "No Valid IP found and no bind address", + rawNodeIPs: nil, bindAddress: "", expectedFamily: v1.IPv4Protocol, expectedIPv4: "127.0.0.1", expectedIPv6: "::1", }, - // Disabled because the GetNodeIP method has a backoff retry mechanism - // and the test takes more than 30 seconds - // ok k8s.io/kubernetes/cmd/kube-proxy/app 34.136s - // { - // name: "No Valid IP found and unspecified bind address", - // nodeInfo: makeNodeWithAddresses("", "", ""), - // hostname: "fakeHost", - // bindAddress: "0.0.0.0", - // expectedFamily: v1.IPv4Protocol, - // expectedIPv4: "127.0.0.1", - // expectedIPv6: "::", - // }, + { + name: "No Valid IP found and unspecified bind address", + rawNodeIPs: nil, + bindAddress: "0.0.0.0", + expectedFamily: v1.IPv4Protocol, + expectedIPv4: "127.0.0.1", + expectedIPv6: "::1", + }, { name: "Bind address 0.0.0.0 and node with IPv4 InternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "192.168.1.1", "90.90.90.90"), - hostname: "fakeHost", + rawNodeIPs: []net.IP{netutils.ParseIPSloppy("192.168.1.1")}, bindAddress: "0.0.0.0", expectedFamily: v1.IPv4Protocol, expectedIPv4: "192.168.1.1", @@ -682,8 +723,7 @@ func Test_detectNodeIPs(t *testing.T) { }, { name: "Bind address :: and node with IPv4 InternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "192.168.1.1", "90.90.90.90"), - hostname: "fakeHost", + rawNodeIPs: []net.IP{netutils.ParseIPSloppy("192.168.1.1")}, bindAddress: "::", expectedFamily: v1.IPv4Protocol, expectedIPv4: "192.168.1.1", @@ -691,8 +731,7 @@ func Test_detectNodeIPs(t *testing.T) { }, { name: "Bind address 0.0.0.0 and node with IPv6 InternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "fd00:1234::1", "2001:db8::2"), - hostname: "fakeHost", + rawNodeIPs: []net.IP{netutils.ParseIPSloppy("fd00:1234::1")}, bindAddress: "0.0.0.0", expectedFamily: v1.IPv6Protocol, expectedIPv4: "127.0.0.1", @@ -700,98 +739,73 @@ func Test_detectNodeIPs(t *testing.T) { }, { name: "Bind address :: and node with IPv6 InternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "fd00:1234::1", "2001:db8::2"), - hostname: "fakeHost", + rawNodeIPs: []net.IP{netutils.ParseIPSloppy("fd00:1234::1")}, bindAddress: "::", expectedFamily: v1.IPv6Protocol, expectedIPv4: "127.0.0.1", expectedIPv6: "fd00:1234::1", }, { - name: "Bind address 0.0.0.0 and node with only IPv4 ExternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "", "90.90.90.90"), - hostname: "fakeHost", - bindAddress: "0.0.0.0", - expectedFamily: v1.IPv4Protocol, - expectedIPv4: "90.90.90.90", - expectedIPv6: "::1", - }, - { - name: "Bind address :: and node with only IPv4 ExternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "", "90.90.90.90"), - hostname: "fakeHost", - bindAddress: "::", - expectedFamily: v1.IPv4Protocol, - expectedIPv4: "90.90.90.90", - expectedIPv6: "::1", - }, - { - name: "Bind address 0.0.0.0 and node with only IPv6 ExternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "", "2001:db8::2"), - hostname: "fakeHost", - bindAddress: "0.0.0.0", - expectedFamily: v1.IPv6Protocol, - expectedIPv4: "127.0.0.1", - expectedIPv6: "2001:db8::2", - }, - { - name: "Bind address :: and node with only IPv6 ExternalIP set", - nodeInfo: makeNodeWithAddresses("fakeHost", "", "2001:db8::2"), - hostname: "fakeHost", - bindAddress: "::", - expectedFamily: v1.IPv6Protocol, - expectedIPv4: "127.0.0.1", - expectedIPv6: "2001:db8::2", - }, - { - name: "Dual stack, primary IPv4", - nodeInfo: makeNodeWithAddresses("fakeHost", "90.90.90.90", "2001:db8::2"), - hostname: "fakeHost", + name: "Dual stack, primary IPv4", + rawNodeIPs: []net.IP{ + netutils.ParseIPSloppy("90.90.90.90"), + netutils.ParseIPSloppy("2001:db8::2"), + }, bindAddress: "::", expectedFamily: v1.IPv4Protocol, expectedIPv4: "90.90.90.90", expectedIPv6: "2001:db8::2", }, { - name: "Dual stack, primary IPv6", - nodeInfo: makeNodeWithAddresses("fakeHost", "2001:db8::2", "90.90.90.90"), - hostname: "fakeHost", + name: "Dual stack, primary IPv6", + rawNodeIPs: []net.IP{ + netutils.ParseIPSloppy("2001:db8::2"), + netutils.ParseIPSloppy("90.90.90.90"), + }, bindAddress: "0.0.0.0", expectedFamily: v1.IPv6Protocol, expectedIPv4: "90.90.90.90", expectedIPv6: "2001:db8::2", }, { - name: "Dual stack, override IPv4", - nodeInfo: makeNodeWithAddresses("fakeHost", "2001:db8::2", "90.90.90.90"), - hostname: "fakeHost", + name: "Dual stack, override IPv4", + rawNodeIPs: []net.IP{ + netutils.ParseIPSloppy("2001:db8::2"), + netutils.ParseIPSloppy("90.90.90.90"), + }, bindAddress: "80.80.80.80", expectedFamily: v1.IPv4Protocol, expectedIPv4: "80.80.80.80", expectedIPv6: "2001:db8::2", }, { - name: "Dual stack, override IPv6", - nodeInfo: makeNodeWithAddresses("fakeHost", "90.90.90.90", "2001:db8::2"), - hostname: "fakeHost", + name: "Dual stack, override IPv6", + rawNodeIPs: []net.IP{ + netutils.ParseIPSloppy("90.90.90.90"), + netutils.ParseIPSloppy("2001:db8::2"), + }, bindAddress: "2001:db8::555", expectedFamily: v1.IPv6Protocol, expectedIPv4: "90.90.90.90", expectedIPv6: "2001:db8::555", }, { - name: "Dual stack, override primary family, IPv4", - nodeInfo: makeNodeWithAddresses("fakeHost", "2001:db8::2", "90.90.90.90"), - hostname: "fakeHost", + name: "Dual stack, override primary family, IPv4", + rawNodeIPs: []net.IP{ + netutils.ParseIPSloppy("2001:db8::2"), + netutils.ParseIPSloppy("90.90.90.90"), + }, bindAddress: "127.0.0.1", expectedFamily: v1.IPv4Protocol, expectedIPv4: "127.0.0.1", expectedIPv6: "2001:db8::2", }, { - name: "Dual stack, override primary family, IPv6", - nodeInfo: makeNodeWithAddresses("fakeHost", "90.90.90.90", "2001:db8::2"), - hostname: "fakeHost", + name: "Dual stack, override primary family, IPv6", + rawNodeIPs: []net.IP{ + netutils.ParseIPSloppy("90.90.90.90"), + netutils.ParseIPSloppy("2001:db8::2"), + }, bindAddress: "::1", expectedFamily: v1.IPv6Protocol, expectedIPv4: "90.90.90.90", @@ -800,8 +814,7 @@ func Test_detectNodeIPs(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - client := clientsetfake.NewSimpleClientset(c.nodeInfo) - primaryFamily, ips := detectNodeIPs(client, c.hostname, c.bindAddress) + primaryFamily, ips := detectNodeIPs(c.rawNodeIPs, c.bindAddress) if primaryFamily != c.expectedFamily { t.Errorf("Expected family %q got %q", c.expectedFamily, primaryFamily) }