Skip to content

Commit

Permalink
Fixes #1926 Pod filtering for Services based on Namespaces in NodePor…
Browse files Browse the repository at this point in the history
…tLocal

Also includes UT to check for namespace based pod filtering via Services.
  • Loading branch information
shubhamavi committed Mar 1, 2021
1 parent cdf357f commit dcbdd5f
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
5 changes: 3 additions & 2 deletions pkg/agent/nodeportlocal/k8s/npl_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (c *NPLController) getPodsFromService(svc *corev1.Service) []string {
return pods
}

podList, err := c.podLister.List(labels.SelectorFromSet(labels.Set(svc.Spec.Selector)))
podList, err := c.podLister.Pods(svc.Namespace).List(labels.SelectorFromSet(labels.Set(svc.Spec.Selector)))
if err != nil {
klog.Errorf("Got error while listing Pods: %v", err)
return pods
Expand All @@ -272,7 +272,8 @@ func (c *NPLController) isNPLEnabledForServiceOfPod(obj interface{}) bool {
svc, isSvc := service.(*corev1.Service)
// Selecting Services NOT of type NodePort, with Service selector matching Pod labels.
if isSvc && svc.Spec.Type != corev1.ServiceTypeNodePort {
if matchSvcSelectorPodLabels(svc.Spec.Selector, pod.GetLabels()) {
if matchSvcSelectorPodLabels(svc.Spec.Selector, pod.GetLabels()) &&
pod.GetNamespace() == svc.GetNamespace() {
return true
}
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/agent/nodeportlocal/npl_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,28 @@ func (t *testData) updatePodOrFail(testPod *corev1.Pod) {
t.Logf("Successfully updated Pod: %s", testPod.Name)
}

// TestSvcNamespaceUpdate creates two Services in different Namespaces default and blue.
// It verifies the NPL annotation in the Pod in the default Namespace. It then deletes the
// Service in default Namespace, and verifies that the NPL annotation is also removed.
func TestSvcNamespaceUpdate(t *testing.T) {
testSvcDefaultNS := getTestSvc()
testPodDefaultNS := getTestPod()
testSvcBlue := getTestSvc()
testSvcBlue.Namespace = "blue"
testData := setUp(t, testSvcDefaultNS, testPodDefaultNS, testSvcBlue)
defer testData.tearDown()

// Remove Service testSvcDefaultNS.
err := testData.k8sClient.CoreV1().Services(defaultNS).Delete(context.TODO(), testSvcDefaultNS.Name, metav1.DeleteOptions{})
require.NoError(t, err, "Service deletion failed")
t.Logf("successfully deleted Service: %s", testSvcDefaultNS.Name)

// Check that annotation and the rule are removed.
_, err = testData.pollForPodAnnotation(testPodDefaultNS.Name, false)
require.NoError(t, err, "Poll for annotation check failed")
assert.False(t, testData.portTable.RuleExists(defaultPodIP, defaultPort))
}

// TestSvcUpdateAnnotation updates the Service spec to disabled NPL. It then verifies that the Pod's
// NPL annotation is removed and that the port table is updated.
func TestSvcUpdateAnnotation(t *testing.T) {
Expand Down

0 comments on commit dcbdd5f

Please sign in to comment.