Fix that Antrea Agent may crash when dualstack

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 will get crashed. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
antrea-io · Dec 14, 2022 · 60826ed · 60826ed
1 parent 630f68f
commit 60826ed
Show file tree

Hide file tree

Showing 3 changed files with 148 additions and 10 deletions.
diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go
@@ -1222,8 +1222,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
@@ -201,16 +201,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 IPv4 address should not be nil")
+	}
+	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 IPv6 address should not be nil")
+	}
+	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
@@ -15,6 +15,7 @@
 package config
 
 import (
+	"fmt"
 	"net"
 	"testing"
 
@@ -163,3 +164,109 @@ 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
+		expectedErr      error
+		expected         bool
+	}{
+		{
+			name: "Non-NetworkPolicyOnly, with IPv4PodCIDR, without NodeIPv4Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv4CIDR: podIPv4CIDR,
+			},
+			expectedErr: fmt.Errorf("k8s Node IPv4 address should not be nil"),
+			expected:    false,
+		},
+		{
+			name: "Non-NetworkPolicyOnly, with IPv4PodCIDR, with NodeIPv4Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv4CIDR:  podIPv4CIDR,
+				NodeIPv4Addr: nodeIPv4Addr,
+			},
+			expectedErr: nil,
+			expected:    true,
+		},
+		{
+			name:             "NetworkPolicyOnly, without NodeIPv4Addr",
+			nodeConfig:       &NodeConfig{},
+			trafficEncapMode: TrafficEncapModeNetworkPolicyOnly,
+			expectedErr:      nil,
+			expected:         false,
+		},
+		{
+			name: "NetworkPolicyOnly, with NodeIPv4Addr",
+			nodeConfig: &NodeConfig{
+				NodeIPv4Addr: nodeIPv4Addr,
+			},
+			trafficEncapMode: TrafficEncapModeNetworkPolicyOnly,
+			expectedErr:      nil,
+			expected:         true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ipv4Enabled, err := IsIPv4Enabled(tt.nodeConfig, tt.trafficEncapMode)
+			assert.Equal(t, tt.expectedErr, err)
+			assert.Equal(t, tt.expected, 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
+		expectedErr      error
+		expected         bool
+	}{
+		{
+			name: "Non-NetworkPolicyOnly, with IPv6PodCIDR, without NodeIPv6Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv6CIDR: podIPv6CIDR,
+			},
+			expectedErr: fmt.Errorf("k8s Node IPv6 address should not be nil"),
+			expected:    false,
+		},
+		{
+			name: "Non-NetworkPolicyOnly, with IPv6PodCIDR, with NodeIPv6Addr",
+			nodeConfig: &NodeConfig{
+				PodIPv6CIDR:  podIPv6CIDR,
+				NodeIPv6Addr: nodeIPv6Addr,
+			},
+			expectedErr: nil,
+			expected:    true,
+		},
+		{
+			name:             "NetworkPolicyOnly, without NodeIPv6Addr",
+			nodeConfig:       &NodeConfig{},
+			trafficEncapMode: TrafficEncapModeNetworkPolicyOnly,
+			expectedErr:      nil,
+			expected:         false,
+		},
+		{
+			name: "NetworkPolicyOnly, with NodeIPv6Addr",
+			nodeConfig: &NodeConfig{
+				NodeIPv6Addr: nodeIPv6Addr,
+			},
+			trafficEncapMode: TrafficEncapModeNetworkPolicyOnly,
+			expectedErr:      nil,
+			expected:         true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ipv6Enabled, err := IsIPv6Enabled(tt.nodeConfig, tt.trafficEncapMode)
+			assert.Equal(t, tt.expectedErr, err)
+			assert.Equal(t, tt.expected, ipv6Enabled)
+		})
+	}
+}