kube-proxy avoid race condition using LocalModeNodeCIDR

Since kube-proxy in LocalModeNodeCIDR needs to obtain the PodCIDR
assigned to the node it watches for the Node object.

However, kube-proxy startup process requires to have these watches in
different places, that opens the possibility of having a race condition
if the same node is recreated and a different PodCIDR is assigned.

Initializing the second watch with the value obtained in the first one
allows us to detect this situation.

Change-Id: I6adeedb6914ad2afd3e0694dcab619c2a66135f8
Signed-off-by: Antonio Ojea <aojea@google.com>

cmd/kube-proxy/app/server.go

@@ -522,6 +522,8 @@ type ProxyServer struct {
 	Hostname      string
 	NodeIP        net.IP
 
+	podCIDRs []string // only used for LocalModeNodeCIDR
+
 	Proxier proxy.Provider
 }
 
@@ -754,7 +756,7 @@ func (s *ProxyServer) Run() error {
 	nodeConfig := config.NewNodeConfig(currentNodeInformerFactory.Core().V1().Nodes(), s.Config.ConfigSyncPeriod.Duration)
 	// https://issues.k8s.io/111321
 	if s.Config.DetectLocalMode == kubeproxyconfig.LocalModeNodeCIDR {
-		nodeConfig.RegisterEventHandler(&proxy.NodePodCIDRHandler{})
+		nodeConfig.RegisterEventHandler(proxy.NewNodePodCIDRHandler(s.podCIDRs))
 	}
 	nodeConfig.RegisterEventHandler(s.Proxier)
 

cmd/kube-proxy/app/server_others.go

@@ -88,6 +88,7 @@ func (s *ProxyServer) createProxier(config *proxyconfigapi.KubeProxyConfiguratio
 		if err != nil {
 			return nil, err
 		}
+		s.podCIDRs = nodeInfo.Spec.PodCIDRs
 		klog.InfoS("NodeInfo", "podCIDR", nodeInfo.Spec.PodCIDR, "podCIDRs", nodeInfo.Spec.PodCIDRs)
 	}
 

cmd/kube-proxy/app/server_others_test.go

@@ -34,16 +34,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
-	netutils "k8s.io/utils/net"
-	"k8s.io/utils/pointer"
-
 	clientsetfake "k8s.io/client-go/kubernetes/fake"
 	clientgotesting "k8s.io/client-go/testing"
-
 	proxyconfigapi "k8s.io/kubernetes/pkg/proxy/apis/config"
 	proxyutiliptables "k8s.io/kubernetes/pkg/proxy/util/iptables"
 	utiliptables "k8s.io/kubernetes/pkg/util/iptables"
 	utiliptablestest "k8s.io/kubernetes/pkg/util/iptables/testing"
+	netutils "k8s.io/utils/net"
+	"k8s.io/utils/pointer"
 )
 
 func Test_platformApplyDefaults(t *testing.T) {
@@ -781,3 +779,51 @@ func TestGetConntrackMax(t *testing.T) {
 		}
 	}
 }
+
+func TestProxyServer_createProxier(t *testing.T) {
+	tests := []struct {
+		name         string
+		node         *v1.Node
+		config       *proxyconfigapi.KubeProxyConfiguration
+		wantPodCIDRs []string
+	}{
+		{
+			name:         "LocalModeNodeCIDR store the node PodCIDRs obtained",
+			node:         makeNodeWithPodCIDRs("10.0.0.0/24"),
+			config:       &proxyconfigapi.KubeProxyConfiguration{DetectLocalMode: proxyconfigapi.LocalModeNodeCIDR},
+			wantPodCIDRs: []string{"10.0.0.0/24"},
+		},
+		{
+			name:         "LocalModeNodeCIDR store the node PodCIDRs obtained dual stack",
+			node:         makeNodeWithPodCIDRs("10.0.0.0/24", "2001:db2:1/64"),
+			config:       &proxyconfigapi.KubeProxyConfiguration{DetectLocalMode: proxyconfigapi.LocalModeNodeCIDR},
+			wantPodCIDRs: []string{"10.0.0.0/24", "2001:db2:1/64"},
+		},
+		{
+			name:   "LocalModeClusterCIDR does not get the node PodCIDRs",
+			node:   makeNodeWithPodCIDRs("10.0.0.0/24", "2001:db2:1/64"),
+			config: &proxyconfigapi.KubeProxyConfiguration{DetectLocalMode: proxyconfigapi.LocalModeClusterCIDR},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := clientsetfake.NewSimpleClientset(tt.node)
+			s := &ProxyServer{
+				Config:   tt.config,
+				Client:   client,
+				Hostname: "nodename",
+				NodeIP:   netutils.ParseIPSloppy("127.0.0.1"),
+			}
+			_, err := s.createProxier(tt.config)
+			// TODO: mock the exec.Interface to not fail probing iptables
+			if (err != nil) && !strings.Contains(err.Error(), "iptables is not supported for primary IP family") {
+				t.Errorf("ProxyServer.createProxier() error = %v", err)
+				return
+			}
+			if !reflect.DeepEqual(s.podCIDRs, tt.wantPodCIDRs) {
+				t.Errorf("Expected PodCIDRs %v got %v", tt.wantPodCIDRs, s.podCIDRs)
+			}
+
+		})
+	}
+}

pkg/proxy/node.go

@@ -33,6 +33,12 @@ type NodePodCIDRHandler struct {
 	podCIDRs []string
 }
 
+func NewNodePodCIDRHandler(podCIDRs []string) *NodePodCIDRHandler {
+	return &NodePodCIDRHandler{
+		podCIDRs: podCIDRs,
+	}
+}
+
 var _ config.NodeHandler = &NodePodCIDRHandler{}
 
 // OnNodeAdd is a handler for Node creates.

pkg/proxy/node_test.go

@@ -37,6 +37,11 @@ func TestNodePodCIDRHandlerAdd(t *testing.T) {
 			name:            "initialized correctly",
 			newNodePodCIDRs: []string{"192.168.1.0/24", "fd00:1:2:3::/64"},
 		},
+		{
+			name:            "already initialized and same node",
+			oldNodePodCIDRs: []string{"10.0.0.0/24", "fd00:3:2:1::/64"},
+			newNodePodCIDRs: []string{"10.0.0.0/24", "fd00:3:2:1::/64"},
+		},
 		{
 			name:            "already initialized and different node",
 			oldNodePodCIDRs: []string{"192.168.1.0/24", "fd00:1:2:3::/64"},