WIP: refactor

weaveworks · Oct 22, 2017 · 11792f5 · 11792f5
1 parent 2d89092
commit 11792f5
Showing 1 changed file with 28 additions and 31 deletions.
diff --git a/npc/namespace.go b/npc/namespace.go
@@ -32,7 +32,7 @@ type ns struct {
 	uid     types.UID     // surrogate UID to own allPods selector
 	allPods *selectorSpec // hash:ip ipset of all pod IPs in this namespace
 
-	// stores IP addrs of pods which are not selected by any dst podselector of
+	// stores IP addrs of pods which are not selected by any dst podSelector of
 	// any netpol; used only in non-legacy mode and is used as a dst in
 	// the WEAVE-NPC-DEFAULT iptables chain.
 	defaultAllowIPSet ipset.Name
@@ -129,20 +129,8 @@ func (ns *ns) onDestroyDstPodSelector(selector *selector) error {
 	for _, pod := range ns.pods {
 		if hasIP(pod) {
 			if selector.matches(pod.ObjectMeta.Labels) {
-				// If the dst selector matches the pod and there are no other matching
-				// dst selectors, add the pod to default-allow.
-				// TODO(mp) optimize (avoid iterating over selectors) by ref counting IP addrs
-				found := false
-				for _, s := range ns.podSelectors.entries {
-					if ns.podSelectors.dstSelectorExist(s) && s.matches(pod.ObjectMeta.Labels) {
-						found = true
-						break
-					}
-				}
-				if !found {
-					if err := ns.ips.AddEntryIfNotExist(ns.defaultAllowIPSet, pod.Status.PodIP, podComment(pod)); err != nil {
-						return err
-					}
+				if err := ns.addToDefaultAllowIfNoMatching(pod); err != nil {
+					return err
 				}
 			}
 		}
@@ -151,6 +139,24 @@ func (ns *ns) onDestroyDstPodSelector(selector *selector) error {
 	return nil
 }
 
+// Add pod IP addr to default-allow ipset if there are no matching dst selectors
+func (ns *ns) addToDefaultAllowIfNoMatching(pod *coreapi.Pod) error {
+	found := false
+	// TODO(mp) optimize (avoid iterating over selectors) by ref counting IP addrs.
+	for _, s := range ns.podSelectors.entries {
+		if ns.podSelectors.dstSelectorExist(s) && s.matches(pod.ObjectMeta.Labels) {
+			found = true
+			break
+		}
+	}
+	if !found {
+		if err := ns.ips.AddEntryIfNotExist(ns.defaultAllowIPSet, pod.Status.PodIP, podComment(pod)); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (ns *ns) checkLocalPod(obj *coreapi.Pod) bool {
 	if obj.Spec.NodeName != ns.nodeName {
 		return false
@@ -223,7 +229,6 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 		return nil
 	}
 
-	// Update default-allow ipset
 	if !ns.legacy &&
 		oldObj.Status.PodIP != newObj.Status.PodIP &&
 		ns.ips.Exist(ns.defaultAllowIPSet, oldObj.Status.PodIP) {
@@ -256,22 +261,14 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 				}
 			}
 
-			if !ns.legacy && !oldMatch && newMatch && ns.podSelectors.dstSelectorExist(ps) {
-				if err := ns.ips.DelEntryIfExists(ns.defaultAllowIPSet, oldObj.Status.PodIP); err != nil {
-					return err
-				}
-			}
-
-			if !ns.legacy && oldMatch && !newMatch && ns.podSelectors.dstSelectorExist(ps) {
-				found := false
-				for _, s := range ns.podSelectors.entries {
-					if ns.podSelectors.dstSelectorExist(s) && s.matches(newObj.ObjectMeta.Labels) {
-						found = true
-						break
+			if !ns.legacy && ns.podSelectors.dstSelectorExist(ps) {
+				switch {
+				case !oldMatch && newMatch:
+					if err := ns.ips.DelEntryIfExists(ns.defaultAllowIPSet, oldObj.Status.PodIP); err != nil {
+						return err
 					}
-				}
-				if !found {
-					if err := ns.ips.AddEntryIfNotExist(ns.defaultAllowIPSet, newObj.Status.PodIP, podComment(newObj)); err != nil {
+				case oldMatch && !newMatch:
+					if err := ns.addToDefaultAllowIfNoMatching(newObj); err != nil {
 						return err
 					}
 				}