diff --git a/npm/ipsm/ipsm.go b/npm/ipsm/ipsm.go index da7412fee5..97d5d97aea 100644 --- a/npm/ipsm/ipsm.go +++ b/npm/ipsm/ipsm.go @@ -349,7 +349,11 @@ func (ipsMgr *IpsetManager) AddToSet(setName, ip, spec, podKey string) error { spec: resultSpec, } - // todo: check err handling besides error code, corrupt state possible here + // TODO(jungukcho): revisit all error code handling codes in ipsm.go and iptm.go. + // For example, in below code, ipsm caches the item + // even though invalid IPv4 or IPv6 address are inserted if returned error code is 1. + // Further it causes controllers to process the rest of operations. + // May use error handler code in npm/util/errors/errors.go when revisiting error handling codes. if errCode, err := ipsMgr.Run(entry); err != nil && errCode != 1 { metrics.SendErrorLogAndMetric(util.IpsmID, "Error: failed to create ipset rules. %+v", entry) return err diff --git a/npm/podController.go b/npm/podController.go index f931c0ac8f..5b3831c475 100644 --- a/npm/podController.go +++ b/npm/podController.go @@ -4,7 +4,9 @@ package npm import ( "fmt" + "net" "reflect" + "strings" "time" "github.com/Azure/azure-container-networking/npm/ipsm" @@ -599,8 +601,18 @@ func isCompletePod(podObj *corev1.Pod) bool { return false } +// hasValidPodIP returns true when pod has a valid IPv4 form since currently NPM does not support IPv6. +// (TODO): will need to update this function when NPM wants to support IPV6 and dual stack configuration in K8s func hasValidPodIP(podObj *corev1.Pod) bool { - return len(podObj.Status.PodIP) > 0 + // First filter IPv6 address. It also filters IPv4-mapped IPv6 ("::ffff:192.0.2.1") form, but not sure this form can happen in k8s + if strings.Contains(podObj.Status.PodIP, ":") { + utilruntime.HandleError(fmt.Errorf("IPv6 %s may be assigned to Pod, but NPM does not support IPv6 yet", podObj.Status.PodIP)) + return false + } + + // Check a correct IPv4 form. + ip := net.ParseIP(podObj.Status.PodIP) + return ip != nil } func isHostNetworkPod(podObj *corev1.Pod) bool { diff --git a/npm/podController_test.go b/npm/podController_test.go index 8543959896..9171789083 100644 --- a/npm/podController_test.go +++ b/npm/podController_test.go @@ -269,12 +269,31 @@ func TestAddHostNetworkPod(t *testing.T) { {0, 1, 0}, } checkPodTestResult("TestAddHostNetworkPod", f, testCases) - if _, exists := f.npMgr.PodMap[podKey]; exists { t.Error("TestAddHostNetworkPod failed @ cached pod obj exists check") } } +func TestAddPodWithIPv6(t *testing.T) { + labels := map[string]string{ + "app": "test-pod", + } + + podObj := createPod("test-pod", "test-namespace", "0", "fc00:f853:ccd:e793::6", labels, NonHostNetwork, corev1.PodRunning) + f := newFixture(t) + f.podLister = append(f.podLister, podObj) + f.kubeobjects = append(f.kubeobjects, podObj) + stopCh := make(chan struct{}) + defer close(stopCh) + f.newPodController(stopCh) + + addPod(t, f, podObj) + testCases := []expectedValues{ + {0, 1, 0}, + } + checkPodTestResult("TestAddPodWithIPv6", f, testCases) +} + func TestDeletePod(t *testing.T) { labels := map[string]string{ "app": "test-pod", @@ -456,13 +475,23 @@ func TestPodStatusUpdatePod(t *testing.T) { } func TestHasValidPodIP(t *testing.T) { - podObj := &corev1.Pod{ - Status: corev1.PodStatus{ - Phase: "Running", - PodIP: "1.2.3.4", - }, - } - if ok := hasValidPodIP(podObj); !ok { - t.Errorf("TestisValidPod failed @ isValidPod") + testCases := map[string]bool{ + "1.2.3.4": true, + "172.19.0.6": true, + "": false, + "1234": false, + "256.1.2.3": false, + "fc00:f853:ccd:e793::6": false, + "::ffff:192.0.2.1": false, + ":::::": false, + } + + podObj := &corev1.Pod{} + for ip, want := range testCases { + podObj.Status.PodIP = ip + + if got := hasValidPodIP(podObj); got != want { + t.Errorf("TestHasValidPodIP failed @ isValidPod got %v, want %v", got, want) + } } }