Use target port of Service in NodePortLocal to configure Pod reachabi…

…lity - addressed review comments - part 4

Signed-off-by: Monotosh Das <[email protected]>

pkg/agent/nodeportlocal/k8s/npl_controller.go

@@ -465,7 +465,8 @@ func (c *NPLController) handleAddUpdatePod(key string, obj interface{}) error {
 
 	// targetPortsInt contains list of all ports that needs to be exposed for the Pod, including container ports
 	// for named ports present in targetPortsStr. If it is empty, then all existing rules and annotations for the
-	// Pod have to be cleaned up.
+	// Pod have to be cleaned up. If a Service uses a named target port that doesn't match any named container port
+	// for the current Pod, no corresponding entry will be added to the targetPortsInt set by the code above.
 	if len(targetPortsInt) == 0 {
 		if err := c.deleteAllPortRulesIfAny(podIP); err != nil {
 			return err

pkg/agent/nodeportlocal/npl_agent_test.go

@@ -424,7 +424,7 @@ func TestPodDelete(t *testing.T) {
 }
 
 // TestPodAddMultiPort creates a Pod with multiple ports and a Service with same ports as target ports.
-// It is verified that the Pod's NPL annotationthe local port table are updated correctly.
+// It verifies that the Pod's NPL annotation and the local port table are updated correctly.
 func TestAddMultiPortPodSvc(t *testing.T) {
 	newPort := 90
 	testSvc := getTestSvc(defaultPort, int32(newPort))
@@ -449,8 +449,8 @@ func TestAddMultiPortPodSvc(t *testing.T) {
 }
 
 // TestPodAddMultiPort creates a Pod with multiple ports and a Service with only one target port.
-// It is verified that the Pod's NPL annotation and the local port table are updated
-// with only one port used as target port of the Service.
+// It verifies that the Pod's NPL annotation and the local port table are updated correctly,
+// with only one port corresponding to the Service's single target port.
 func TestAddMultiPortPodSinglePortSvc(t *testing.T) {
 	testSvc := getTestSvc()
 	testPod := getTestPod()
@@ -497,9 +497,9 @@ func TestPodAddHostPort(t *testing.T) {
 }
 
 // TestPodAddHostPort creates a Pod with multiple host ports having same value but different protocol.
-// It is verified that the Pod's NPL annotation is updated with host port, without allocating extra port
+// It verifies that the Pod's NPL annotation is updated with host port, without allocating extra port
 // from NPL pool. No NPL rule is required for this case.
-func TestPodAddMultiProtocol(t *testing.T) {
+func TestPodAddHostPortMultiProtocol(t *testing.T) {
 	testSvc := getTestSvc()
 	testPod := getTestPod()
 	hostPort := 4001

test/e2e/nodeportlocal_test.go

@@ -185,8 +185,8 @@ func validatePortsInAnnotation(t *testing.T, r *require.Assertions, nplAnnotatio
 }
 
 // createNginxPodWithPort creates an nginx Pod in the test namespace with configurable container port.
-func createNginxPodWithPort(name, nodeName string, port int32) error {
-	return testData.createPodOnNode(name, nodeName, nginxImage, []string{}, nil, nil, []corev1.ContainerPort{
+func (data *TestData) createNginxPodWithPort(name, nodeName string, port int32) error {
+	return data.createPodOnNode(name, nodeName, nginxImage, []string{}, nil, nil, []corev1.ContainerPort{
 		{
 			Name:          "http",
 			ContainerPort: port,
@@ -229,7 +229,7 @@ func NPLTestMultiplePods(t *testing.T) {
 	for i := 0; i < 4; i++ {
 		testPodName := randName("test-pod-")
 		testPods = append(testPods, testPodName)
-		err := createNginxPodWithPort(testPodName, node, 90)
+		err := testData.createNginxPodWithPort(testPodName, node, 90)
 		r.NoError(err, "Error creating test Pod: %v", err)
 	}
 
@@ -331,7 +331,7 @@ func NPLTestLocalAccess(t *testing.T) {
 	node := nodeName(0)
 
 	testPodName := randName("test-pod-")
-	err := createNginxPodWithPort(testPodName, node, 90)
+	err := testData.createNginxPodWithPort(testPodName, node, 90)
 	r.NoError(err, "Error creating test Pod: %v", err)
 
 	clientName := randName("test-client-")
@@ -382,7 +382,7 @@ func TestNPLMultiplePodsAgentRestart(t *testing.T) {
 	for i := 0; i < 4; i++ {
 		testPodName := randName("test-pod-")
 		testPods = append(testPods, testPodName)
-		err = createNginxPodWithPort(testPodName, node, 90)
+		err = testData.createNginxPodWithPort(testPodName, node, 90)
 		r.NoError(err, "Error creating test Pod: %v", err)
 	}
 
@@ -454,7 +454,7 @@ func TestNPLChangePortRangeAgentRestart(t *testing.T) {
 	for i := 0; i < 4; i++ {
 		testPodName := randName("test-pod-")
 		testPods = append(testPods, testPodName)
-		err = createNginxPodWithPort(testPodName, node, 90)
+		err = testData.createNginxPodWithPort(testPodName, node, 90)
 		r.NoError(err, "Error Creating test Pod: %v", err)
 	}