Fix possible Antrea Agent crash in dualstack mode (#4480)

When a Cluster is dualstack, it assumes that every Node is configured with both IPv4 and IPv6 addresses for Antrea Agent. However, if a Node is only configured with either IPv4 or IPv6 address, Antrea Agent would crash. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
antrea-io · Feb 7, 2023 · 2b29229 · 2b29229
1 parent c75281a
commit 2b29229
Show file tree

Hide file tree

Showing 3 changed files with 153 additions and 10 deletions.
diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go
@@ -1237,8 +1237,15 @@ func (i *Initializer) initNodeLocalConfig() error {
 			return err
 		}
 
-		i.networkConfig.IPv4Enabled = config.IsIPv4Enabled(i.nodeConfig, i.networkConfig.TrafficEncapMode)
-		i.networkConfig.IPv6Enabled = config.IsIPv6Enabled(i.nodeConfig, i.networkConfig.TrafficEncapMode)
+		i.networkConfig.IPv4Enabled, err = config.IsIPv4Enabled(i.nodeConfig, i.networkConfig.TrafficEncapMode)
+		if err != nil {
+			return err
+		}
+		i.networkConfig.IPv6Enabled, err = config.IsIPv6Enabled(i.nodeConfig, i.networkConfig.TrafficEncapMode)
+		if err != nil {
+			return err
+		}
+
 		return nil
 	}
 	if err := i.initVMLocalConfig(nodeName); err != nil {

diff --git a/pkg/agent/config/node_config.go b/pkg/agent/config/node_config.go
@@ -206,16 +206,40 @@ type NetworkConfig struct {
 	IPv6Enabled           bool
 }
 
-// IsIPv4Enabled returns true if the cluster network supports IPv4.
-func IsIPv4Enabled(nodeConfig *NodeConfig, trafficEncapMode TrafficEncapModeType) bool {
-	return nodeConfig.PodIPv4CIDR != nil ||
-		(trafficEncapMode.IsNetworkPolicyOnly() && nodeConfig.NodeIPv4Addr != nil)
+// IsIPv4Enabled returns true if the cluster network supports IPv4. Legal cases are:
+// - NetworkPolicyOnly, NodeIPv4Addr != nil, IPv4 is enabled
+// - NetworkPolicyOnly, NodeIPv4Addr == nil, IPv4 is disabled
+// - Non-NetworkPolicyOnly, PodIPv4CIDR != nil, NodeIPv4Addr != nil, IPv4 is enabled
+// - Non-NetworkPolicyOnly, PodIPv4CIDR == nil, IPv4 is disabled
+func IsIPv4Enabled(nodeConfig *NodeConfig, trafficEncapMode TrafficEncapModeType) (bool, error) {
+	if trafficEncapMode.IsNetworkPolicyOnly() {
+		return nodeConfig.NodeIPv4Addr != nil, nil
+	}
+	if nodeConfig.PodIPv4CIDR != nil {
+		if nodeConfig.NodeIPv4Addr != nil {
+			return true, nil
+		}
+		return false, fmt.Errorf("K8s Node should have an IPv4 address if IPv4 Pod CIDR is defined")
+	}
+	return false, nil
 }
 
-// IsIPv6Enabled returns true if the cluster network supports IPv6.
-func IsIPv6Enabled(nodeConfig *NodeConfig, trafficEncapMode TrafficEncapModeType) bool {
-	return nodeConfig.PodIPv6CIDR != nil ||
-		(trafficEncapMode.IsNetworkPolicyOnly() && nodeConfig.NodeIPv6Addr != nil)
+// IsIPv6Enabled returns true if the cluster network supports IPv6. Legal cases are:
+// - NetworkPolicyOnly, NodeIPv6Addr != nil, IPv6 is enabled
+// - NetworkPolicyOnly, NodeIPv6Addr == nil, IPv6 is disabled
+// - Non-NetworkPolicyOnly, PodIPv6CIDR != nil, NodeIPv6Addr != nil, IPv6 is enabled
+// - Non-NetworkPolicyOnly, PodIPv6CIDR == nil, IPv6 is disabled
+func IsIPv6Enabled(nodeConfig *NodeConfig, trafficEncapMode TrafficEncapModeType) (bool, error) {
+	if trafficEncapMode.IsNetworkPolicyOnly() {
+		return nodeConfig.NodeIPv6Addr != nil, nil
+	}
+	if nodeConfig.PodIPv6CIDR != nil {
+		if nodeConfig.NodeIPv6Addr != nil {
+			return true, nil
+		}
+		return false, fmt.Errorf("K8s Node should have an IPv6 address if IPv6 Pod CIDR is defined")
+	}
+	return false, nil
 }
 
 // NeedsTunnelToPeer returns true if Pod traffic to peer Node needs to be encapsulated by OVS tunneling.

diff --git a/pkg/agent/config/node_config_test.go b/pkg/agent/config/node_config_test.go
@@ -163,3 +163,115 @@ func TestNetworkConfig_NeedsDirectRoutingToPeer(t *testing.T) {
 		})
 	}
 }
+
+func TestIsIPv4Enabled(t *testing.T) {
+	_, podIPv4CIDR, _ := net.ParseCIDR("10.10.0.0/24")
+	_, nodeIPv4Addr, _ := net.ParseCIDR("192.168.77.100/24")
+	tests := []struct {
+		name                string
+		nodeConfig          *NodeConfig
+		trafficEncapMode    TrafficEncapModeType
+		expectedErrString   string
+		expectedWithError   bool
+		expectedIPv4Enabled bool
+	}{
+		{
+			name: "Non-NetworkPolicyOnly, with IPv4PodCIDR, without NodeIPv4Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv4CIDR: podIPv4CIDR,
+			},
+			expectedWithError:   true,
+			expectedErrString:   "K8s Node should have an IPv4 address if IPv4 Pod CIDR is defined",
+			expectedIPv4Enabled: false,
+		},
+		{
+			name: "Non-NetworkPolicyOnly, with IPv4PodCIDR, with NodeIPv4Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv4CIDR:  podIPv4CIDR,
+				NodeIPv4Addr: nodeIPv4Addr,
+			},
+			expectedIPv4Enabled: true,
+		},
+		{
+			name:                "NetworkPolicyOnly, without NodeIPv4Addr",
+			nodeConfig:          &NodeConfig{},
+			trafficEncapMode:    TrafficEncapModeNetworkPolicyOnly,
+			expectedIPv4Enabled: false,
+		},
+		{
+			name: "NetworkPolicyOnly, with NodeIPv4Addr",
+			nodeConfig: &NodeConfig{
+				NodeIPv4Addr: nodeIPv4Addr,
+			},
+			trafficEncapMode:    TrafficEncapModeNetworkPolicyOnly,
+			expectedIPv4Enabled: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ipv4Enabled, err := IsIPv4Enabled(tt.nodeConfig, tt.trafficEncapMode)
+			if tt.expectedWithError {
+				assert.ErrorContains(t, err, tt.expectedErrString)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tt.expectedIPv4Enabled, ipv4Enabled)
+		})
+	}
+}
+
+func TestIsIPv6Enabled(t *testing.T) {
+	_, podIPv6CIDR, _ := net.ParseCIDR("10:10::/64")
+	_, nodeIPv6Addr, _ := net.ParseCIDR("192:168:77::100/80")
+	tests := []struct {
+		name                string
+		nodeConfig          *NodeConfig
+		trafficEncapMode    TrafficEncapModeType
+		expectedWithError   bool
+		expectedErrString   string
+		expectedIPv6Enabled bool
+	}{
+		{
+			name: "Non-NetworkPolicyOnly, with IPv6PodCIDR, without NodeIPv6Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv6CIDR: podIPv6CIDR,
+			},
+			expectedWithError:   true,
+			expectedErrString:   "K8s Node should have an IPv6 address if IPv6 Pod CIDR is defined",
+			expectedIPv6Enabled: false,
+		},
+		{
+			name: "Non-NetworkPolicyOnly, with IPv6PodCIDR, with NodeIPv6Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv6CIDR:  podIPv6CIDR,
+				NodeIPv6Addr: nodeIPv6Addr,
+			},
+			expectedIPv6Enabled: true,
+		},
+		{
+			name:                "NetworkPolicyOnly, without NodeIPv6Addr",
+			nodeConfig:          &NodeConfig{},
+			trafficEncapMode:    TrafficEncapModeNetworkPolicyOnly,
+			expectedIPv6Enabled: false,
+		},
+		{
+			name: "NetworkPolicyOnly, with NodeIPv6Addr",
+			nodeConfig: &NodeConfig{
+				NodeIPv6Addr: nodeIPv6Addr,
+			},
+			trafficEncapMode:    TrafficEncapModeNetworkPolicyOnly,
+			expectedIPv6Enabled: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ipv6Enabled, err := IsIPv6Enabled(tt.nodeConfig, tt.trafficEncapMode)
+			if tt.expectedWithError {
+				assert.ErrorContains(t, err, tt.expectedErrString)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tt.expectedIPv6Enabled, ipv6Enabled)
+		})
+	}
+}