NPL code tidy-up for Antrea v2.0

Signed-off-by: Lan Luo <luola@vmware.com>

cmd/antrea-agent/options.go

@@ -469,12 +469,7 @@ func (o *Options) setK8sNodeDefaultOptions() {
 	}
 
 	if o.config.NodePortLocal.Enable {
-		switch {
-		case o.config.NodePortLocal.PortRange != "":
-		case o.config.NPLPortRange != "":
-			klog.InfoS("The nplPortRange option is deprecated, please use nodePortLocal.portRange instead")
-			o.config.NodePortLocal.PortRange = o.config.NPLPortRange
-		default:
+		if o.config.NodePortLocal.PortRange == "" {
 			o.config.NodePortLocal.PortRange = defaultNPLPortRange
 		}
 	}

docs/node-port-local.md

@@ -38,9 +38,10 @@ From Antrea v1.14, NPL is GA.
 In addition to enabling the NodePortLocal feature gate (if needed), you need to
 ensure that the `nodePortLocal.enable` flag is set to true in the Antrea Agent
 configuration. The `nodePortLocal.portRange` parameter can also be set to change
-the range from which Node ports will be allocated. Otherwise, the default range
-of `61000-62000` will be used by default. When using the NodePortLocal feature,
-your `antrea-agent` ConfigMap should look like this:
+the range from which Node ports will be allocated. Otherwise, the range
+of `61000-62000` will be used by default on Linux, and the range `40000-41000` will
+be used on Windows. When using the NodePortLocal feature, your `antrea-agent` ConfigMap
+should look like this:
 
 ```yaml
 kind: ConfigMap
@@ -116,7 +117,7 @@ metadata:
   labels:
     app: nginx
   annotations:
-    nodeportlocal.antrea.io: '[{"podPort":8080,"nodeIP":"10.10.10.10","nodePort":61002,"protocol":"tcp","protocols":["tcp"]}]'
+    nodeportlocal.antrea.io: '[{"podPort":8080,"nodeIP":"10.10.10.10","nodePort":61002,"protocol":"tcp"}]'
 ```
 
 This annotation indicates that port 8080 of the Pod can be reached through port
@@ -132,11 +133,7 @@ NodePortLocal can only be used with Services of type `ClusterIP` or
 Services of type `NodePort` or `ExternalName`. The annotation also has no effect
 for Services with an empty or missing Selector.
 
-Starting from the Antrea v1.7 minor release, the `protocols` field in the
-annotation is deprecated. The array contains a single member, equal to the
-`protocol` field.
-The `protocols` field will be removed from Antrea for minor releases post March 2023,
-as per our deprecation policy.
+Starting from Antrea v2.0, the `protocols` field is removed.
 
 ### Usage pre Antrea v1.7
 
@@ -168,7 +165,7 @@ for a given podPort, and the allocated nodePort may be different for each one.
 Prior to the Antrea v1.4 minor release, the `nodePortLocal` option group in the
 Antrea Agent configuration did not exist. To enable the NodePortLocal feature,
 one simply needed to enable the feature gate, and the port range could be
-configured using the (now deprecated) `nplPortRange` parameter.
+configured using the (now removed) `nplPortRange` parameter.
 
 ### Usage pre Antrea v1.2
 

pkg/agent/nodeportlocal/k8s/npl_controller.go

@@ -512,11 +512,10 @@ func (c *NPLController) handleAddUpdatePod(key string, obj interface{}) error {
 		}
 		if _, ok := nplAnnotationsRequiredMap[portcache.NodePortProtoFormat(nodePort, protocol)]; !ok {
 			nplAnnotationsRequiredMap[portcache.NodePortProtoFormat(nodePort, protocol)] = types.NPLAnnotation{
-				PodPort:   port,
-				NodeIP:    pod.Status.HostIP,
-				NodePort:  nodePort,
-				Protocol:  protocol,
-				Protocols: []string{protocol},
+				PodPort:  port,
+				NodeIP:   pod.Status.HostIP,
+				NodePort: nodePort,
+				Protocol: protocol,
 			}
 		}
 	}
@@ -598,11 +597,10 @@ func (c *NPLController) waitForRulesInitialization() {
 				continue
 			}
 			allNPLPorts = append(allNPLPorts, rules.PodNodePort{
-				NodePort:  npl.NodePort,
-				PodPort:   npl.PodPort,
-				PodIP:     pod.Status.PodIP,
-				Protocol:  npl.Protocol,
-				Protocols: npl.Protocols,
+				NodePort: npl.NodePort,
+				PodPort:  npl.PodPort,
+				PodIP:    pod.Status.PodIP,
+				Protocol: npl.Protocol,
 			})
 		}
 	}

pkg/agent/nodeportlocal/portcache/port_table_others.go

@@ -183,11 +183,10 @@ func (pt *PortTable) syncRules() error {
 			protocols = append(protocols, protocol.Protocol)
 		}
 		nplPorts = append(nplPorts, rules.PodNodePort{
-			NodePort:  npData.NodePort,
-			PodPort:   npData.PodPort,
-			PodIP:     npData.PodIP,
-			Protocol:  protocols[0],
-			Protocols: protocols,
+			NodePort: npData.NodePort,
+			PodPort:  npData.PodPort,
+			PodIP:    npData.PodIP,
+			Protocol: protocols[0],
 		})
 	}
 	if err := pt.PodPortRules.AddAllRules(nplPorts); err != nil {

pkg/agent/nodeportlocal/portcache/port_table_others_test.go

@@ -35,25 +35,22 @@ func TestRestoreRules(t *testing.T) {
 	portTable := newPortTable(mockIPTables, mockPortOpener)
 	allNPLPorts := []rules.PodNodePort{
 		{
-			NodePort:  nodePort1,
-			PodPort:   1001,
-			PodIP:     podIP,
-			Protocol:  "tcp",
-			Protocols: []string{"tcp"},
+			NodePort: nodePort1,
+			PodPort:  1001,
+			PodIP:    podIP,
+			Protocol: "tcp",
 		},
 		{
-			NodePort:  nodePort1,
-			PodPort:   1001,
-			PodIP:     podIP,
-			Protocol:  "udp",
-			Protocols: []string{"udp"},
+			NodePort: nodePort1,
+			PodPort:  1001,
+			PodIP:    podIP,
+			Protocol: "udp",
 		},
 		{
-			NodePort:  nodePort2,
-			PodPort:   1002,
-			PodIP:     podIP,
-			Protocol:  "udp",
-			Protocols: []string{"udp"},
+			NodePort: nodePort2,
+			PodPort:  1002,
+			PodIP:    podIP,
+			Protocol: "udp",
 		},
 	}
 

pkg/agent/nodeportlocal/rules/iptable_rule.go

@@ -103,11 +103,9 @@ func (ipt *iptablesRules) AddAllRules(nplList []PodNodePort) error {
 	writeLine(iptablesData, "*nat")
 	writeLine(iptablesData, iptables.MakeChainLine(NodePortLocalChain))
 	for _, nplData := range nplList {
-		for _, protocol := range nplData.Protocols {
-			destination := nplData.PodIP + ":" + fmt.Sprint(nplData.PodPort)
-			rule := buildRuleForPod(nplData.NodePort, destination, protocol)
-			writeLine(iptablesData, append([]string{"-A", NodePortLocalChain}, rule...)...)
-		}
+		destination := nplData.PodIP + ":" + fmt.Sprint(nplData.PodPort)
+		rule := buildRuleForPod(nplData.NodePort, destination, nplData.Protocol)
+		writeLine(iptablesData, append([]string{"-A", NodePortLocalChain}, rule...)...)
 	}
 	writeLine(iptablesData, "COMMIT")
 	if err := ipt.table.Restore(iptablesData.String(), false, false); err != nil {

pkg/agent/nodeportlocal/rules/iptable_rule_test.go

@@ -114,6 +114,8 @@ func TestAddAndDeleteAllRules(t *testing.T) {
 				mockIPTables.AppendRule(iptables.ProtocolIPv4, iptables.NATTable, iptables.OutputChain, []string{"-p", "all", "-m", "addrtype", "--dst-type", "LOCAL", "-j", NodePortLocalChain})
 				mockIPTables.Restore(`*nat
 :ANTREA-NODE-PORT-LOCAL - [0:0]
+-A ANTREA-NODE-PORT-LOCAL -p udp -m udp --dport 8227 -j DNAT --to-destination 10.10.10.11:7237
+-A ANTREA-NODE-PORT-LOCAL -p tcp -m tcp --dport 8247 -j DNAT --to-destination 10.11.10.12:7257
 COMMIT
 `, false, false)
 				// Set the expectation for ChainExists to return true
@@ -151,6 +153,9 @@ COMMIT
 				mockIPTables.AppendRule(iptables.ProtocolIPv4, iptables.NATTable, iptables.OutputChain, []string{"-p", "all", "-m", "addrtype", "--dst-type", "LOCAL", "-j", NodePortLocalChain})
 				mockIPTables.Restore(`*nat
 :ANTREA-NODE-PORT-LOCAL - [0:0]
+-A ANTREA-NODE-PORT-LOCAL -p tcp -m tcp --dport 7 -j DNAT --to-destination 10.10.10.10:17
+-A ANTREA-NODE-PORT-LOCAL -p udp -m udp --dport 27 -j DNAT --to-destination 10.10.10.11:37
+-A ANTREA-NODE-PORT-LOCAL -p tcp -m tcp --dport 47 -j DNAT --to-destination 10.11.10.12:57
 COMMIT
 `, false, false)
 				// Set the expectation for ChainExists to return true

pkg/agent/nodeportlocal/rules/types.go

@@ -16,9 +16,8 @@ package rules
 
 // PodNodePort contains the Node Port, Pod IP, Pod Port and Protocols for NodePortLocal.
 type PodNodePort struct {
-	NodePort  int
-	PodPort   int
-	PodIP     string
-	Protocol  string
-	Protocols []string
+	NodePort int
+	PodPort  int
+	PodIP    string
+	Protocol string
 }

pkg/agent/nodeportlocal/testing/annotations.go

@@ -47,8 +47,7 @@ func (a *ExpectedNPLAnnotations) find(podPort int, protocol string) *types.NPLAn
 }
 
 func (a *ExpectedNPLAnnotations) Add(nodePort *int, podPort int, protocol string) *ExpectedNPLAnnotations {
-	protocols := []string{protocol}
-	annotation := types.NPLAnnotation{PodPort: podPort, Protocol: protocol, Protocols: protocols}
+	annotation := types.NPLAnnotation{PodPort: podPort, Protocol: protocol}
 	if nodePort != nil {
 		annotation.NodePort = *nodePort
 	}
@@ -66,7 +65,6 @@ func (a *ExpectedNPLAnnotations) Check(t *testing.T, nplValue []types.NPLAnnotat
 		if !assert.NotNilf(t, expectedAnnotation, "Unexpected annotation with PodPort %d", nplAnnotation.PodPort) {
 			continue
 		}
-		assert.ElementsMatch(t, expectedAnnotation.Protocols, nplAnnotation.Protocols, "Protocols mismatch in annotation")
 		if expectedAnnotation.NodeIP != "" {
 			assert.Equal(t, expectedAnnotation.NodeIP, nplAnnotation.NodeIP, "NodeIP mismatch in annotation")
 		}

pkg/agent/nodeportlocal/types/types.go

@@ -21,9 +21,8 @@ const (
 
 // NPLAnnotation is the structure used for setting NodePortLocal annotation on the Pods.
 type NPLAnnotation struct {
-	PodPort   int      `json:"podPort"`
-	NodeIP    string   `json:"nodeIP"`
-	NodePort  int      `json:"nodePort"`
-	Protocol  string   `json:"protocol"`
-	Protocols []string `json:"protocols"` // deprecated, array with a single member which is equal to the Protocol field
+	PodPort  int    `json:"podPort"`
+	NodeIP   string `json:"nodeIP"`
+	NodePort int    `json:"nodePort"`
+	Protocol string `json:"protocol"`
 }

pkg/config/agent/config.go

@@ -141,8 +141,6 @@ type AgentConfig struct {
 	ActiveFlowExportTimeout string `yaml:"activeFlowExportTimeout,omitempty"`
 	// Deprecated. Use the FlowExporter config options instead.
 	IdleFlowExportTimeout string `yaml:"idleFlowExportTimeout,omitempty"`
-	// Deprecated. Use the NodePortLocal config options instead.
-	NPLPortRange string `yaml:"nplPortRange,omitempty"`
 	// NodePortLocal (NPL) configuration options.
 	NodePortLocal NodePortLocalConfig `yaml:"nodePortLocal,omitempty"`
 	// FlowExporter configuration options.

test/e2e/nodeportlocal_test.go

@@ -164,16 +164,14 @@ func checkNPLRules(t *testing.T, data *TestData, r *require.Assertions, nplAnnot
 func checkNPLRulesForPod(t *testing.T, data *TestData, r *require.Assertions, nplAnnotations []types.NPLAnnotation, antreaPod, podIP string, present bool) {
 	var rules []nplRuleData
 	for _, ann := range nplAnnotations {
-		for _, protocol := range ann.Protocols {
-			rule := nplRuleData{
-				nodeIP:   ann.NodeIP,
-				nodePort: ann.NodePort,
-				podIP:    podIP,
-				podPort:  ann.PodPort,
-				protocol: protocol,
-			}
-			rules = append(rules, rule)
+		rule := nplRuleData{
+			nodeIP:   ann.NodeIP,
+			nodePort: ann.NodePort,
+			podIP:    podIP,
+			podPort:  ann.PodPort,
+			protocol: ann.Protocol,
 		}
+		rules = append(rules, rule)
 	}
 	checkForNPLRuleInIPTables(t, data, r, antreaPod, rules, present)
 	checkForNPLListeningSockets(t, data, r, antreaPod, rules, present)
@@ -182,16 +180,14 @@ func checkNPLRulesForPod(t *testing.T, data *TestData, r *require.Assertions, np
 func checkNPLRulesForWindowsPod(t *testing.T, data *TestData, r *require.Assertions, nplAnnotations []types.NPLAnnotation, antreaPod, podIP string, nodeName string, present bool) {
 	var rules []nplRuleData
 	for _, ann := range nplAnnotations {
-		for _, protocol := range ann.Protocols {
-			rule := nplRuleData{
-				nodeIP:   ann.NodeIP,
-				nodePort: ann.NodePort,
-				podIP:    podIP,
-				podPort:  ann.PodPort,
-				protocol: protocol,
-			}
-			rules = append(rules, rule)
+		rule := nplRuleData{
+			nodeIP:   ann.NodeIP,
+			nodePort: ann.NodePort,
+			podIP:    podIP,
+			podPort:  ann.PodPort,
+			protocol: ann.Protocol,
 		}
+		rules = append(rules, rule)
 	}
 	checkForNPLRuleInNetNat(t, data, r, antreaPod, nodeName, rules, present)
 }
@@ -345,10 +341,8 @@ func deleteNPLRuleFromNetNat(t *testing.T, data *TestData, r *require.Assertions
 
 func checkTrafficForNPL(data *TestData, r *require.Assertions, nplAnnotations []types.NPLAnnotation, clientName string) {
 	for i := range nplAnnotations {
-		for j := range nplAnnotations[i].Protocols {
-			err := data.runNetcatCommandFromTestPodWithProtocol(clientName, data.testNamespace, "agnhost", nplAnnotations[i].NodeIP, int32(nplAnnotations[i].NodePort), nplAnnotations[i].Protocols[j])
-			r.NoError(err, "Traffic test failed for NodeIP: %s, NodePort: %d, Protocol: %s", nplAnnotations[i].NodeIP, nplAnnotations[i].NodePort, nplAnnotations[i].Protocols[j])
-		}
+		err := data.runNetcatCommandFromTestPodWithProtocol(clientName, data.testNamespace, "agnhost", nplAnnotations[i].NodeIP, int32(nplAnnotations[i].NodePort), nplAnnotations[i].Protocol)
+		r.NoError(err, "Traffic test failed for NodeIP: %s, NodePort: %d, Protocol: %s", nplAnnotations[i].NodeIP, nplAnnotations[i].NodePort, nplAnnotations[i].Protocol)
 	}
 }
 
@@ -645,7 +639,7 @@ func testNPLMultiplePodsAgentRestart(t *testing.T, data *TestData) {
 
 // testNPLChangePortRangeAgentRestart tests NodePortLocal functionalities after changing port range.
 // - Create multiple Nginx Pods.
-// - Change nplPortRange.
+// - Change the PortRange.
 // - Restart Antrea Agent Pods.
 // - Verify that updated port range is being used for NPL.
 func testNPLChangePortRangeAgentRestart(t *testing.T, data *TestData) {
@@ -677,15 +671,13 @@ func testNPLChangePortRangeAgentRestart(t *testing.T, data *TestData) {
 	for _, testPodName := range testPods {
 		nplAnnotations, testPodIP := getNPLAnnotations(t, data, r, testPodName, nil)
 		for i := range nplAnnotations {
-			for j := range nplAnnotations[i].Protocols {
-				rule := nplRuleData{
-					nodePort: nplAnnotations[i].NodePort,
-					podIP:    testPodIP,
-					podPort:  nplAnnotations[i].PodPort,
-					protocol: nplAnnotations[i].Protocols[j],
-				}
-				rules = append(rules, rule)
+			rule := nplRuleData{
+				nodePort: nplAnnotations[i].NodePort,
+				podIP:    testPodIP,
+				podPort:  nplAnnotations[i].PodPort,
+				protocol: nplAnnotations[i].Protocol,
 			}
+			rules = append(rules, rule)
 		}
 	}
 	configureNPLForAgent(t, data, updatedStartPort, updatedEndPort)