diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 116eb11e347..7a9b0b5df2d 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -355,8 +355,15 @@ func (i *Initializer) Initialize() error { return err } - i.networkConfig.IPv4Enabled = config.IsIPv4Enabled(i.nodeConfig, i.networkConfig.TrafficEncapMode) - i.networkConfig.IPv6Enabled = config.IsIPv6Enabled(i.nodeConfig, i.networkConfig.TrafficEncapMode) + var err error + 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 + } if err := i.prepareHostNetwork(); err != nil { return err @@ -460,14 +467,15 @@ func persistRoundNum(num uint64, bridgeClient ovsconfig.OVSBridgeClient, interva // initOpenFlowPipeline sets up necessary Openflow entries, including pipeline, classifiers, conn_track, and gateway flows // Every time the agent is (re)started, we go through the following sequence: -// 1. agent determines the new round number (this is done by incrementing the round number -// persisted in OVSDB, or if it's not available by picking round 1). -// 2. any existing flow for which the round number matches the round number obtained from step 1 -// is deleted. -// 3. all required flows are installed, using the round number obtained from step 1. -// 4. after convergence, all existing flows for which the round number matches the previous round -// number (i.e. the round number which was persisted in OVSDB, if any) are deleted. -// 5. the new round number obtained from step 1 is persisted to OVSDB. +// 1. agent determines the new round number (this is done by incrementing the round number +// persisted in OVSDB, or if it's not available by picking round 1). +// 2. any existing flow for which the round number matches the round number obtained from step 1 +// is deleted. +// 3. all required flows are installed, using the round number obtained from step 1. +// 4. after convergence, all existing flows for which the round number matches the previous round +// number (i.e. the round number which was persisted in OVSDB, if any) are deleted. +// 5. the new round number obtained from step 1 is persisted to OVSDB. +// // The rationale for not persisting the new round number until after all previous flows have been // deleted is to avoid a situation in which some stale flows are never deleted because of successive // agent restarts (with the agent crashing before step 4 can be completed). With the sequence diff --git a/pkg/agent/config/node_config.go b/pkg/agent/config/node_config.go index c4ee7445140..cb26a5de67a 100644 --- a/pkg/agent/config/node_config.go +++ b/pkg/agent/config/node_config.go @@ -179,16 +179,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 index 9e28ec96325..a290be143da 100644 --- 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) + }) + } +}