Fix NPM Bugs (#542)

* Remove old npm chains which were causing errors on uninit

* Utilize rawNpMap and refrain from updating policies with no change.

* redacted

* add added policy to processedNpMap

Author: jaer-tsun

npm/iptm/iptm.go

@@ -182,18 +182,11 @@ func (iptMgr *IptablesManager) InitNpmChains() error {
 func (iptMgr *IptablesManager) UninitNpmChains() error {
 	IptablesAzureChainList := []string{
 		util.IptablesAzureChain,
-		util.IptablesAzureKubeSystemChain,
 		util.IptablesAzureIngressPortChain,
 		util.IptablesAzureIngressFromChain,
 		util.IptablesAzureEgressPortChain,
 		util.IptablesAzureEgressToChain,
 		util.IptablesAzureTargetSetsChain,
-		// Below chains exists only for before Azure-NPM:v1.0.27
-		// and should be removed after a baking period.
-		util.IptablesAzureIngressFromNsChain,
-		util.IptablesAzureIngressFromPodChain,
-		util.IptablesAzureEgressToNsChain,
-		util.IptablesAzureEgressToPodChain,
 	}
 
 	// Remove AZURE-NPM chain from FORWARD chain.
@@ -236,12 +229,10 @@ func (iptMgr *IptablesManager) Exists(entry *IptEntry) (bool, error) {
 	iptMgr.OperationFlag = util.IptablesCheckFlag
 	returnCode, err := iptMgr.Run(entry)
 	if err == nil {
-		log.Printf("Rule exists. %+v.", entry)
 		return true, nil
 	}
 
 	if returnCode == iptablesErrDoesNotExist {
-		log.Printf("Rule doesn't exist. %+v.", entry)
 		return false, nil
 	}
 
@@ -348,12 +339,15 @@ func (iptMgr *IptablesManager) Run(entry *IptEntry) (int, error) {
 	}
 
 	cmdArgs := append([]string{util.IptablesWaitFlag, entry.LockWaitTimeInSeconds, iptMgr.OperationFlag, entry.Chain}, entry.Specs...)
-	log.Printf("Executing iptables command %s %v", cmdName, cmdArgs)
-	_, err := exec.Command(cmdName, cmdArgs...).Output()
 
+	if iptMgr.OperationFlag != util.IptablesCheckFlag {
+		log.Printf("Executing iptables command %s %v", cmdName, cmdArgs)
+	}
+
+	_, err := exec.Command(cmdName, cmdArgs...).Output()
 	if msg, failed := err.(*exec.ExitError); failed {
 		errCode := msg.Sys().(syscall.WaitStatus).ExitStatus()
-		if errCode > 0 {
+		if errCode > 0 && iptMgr.OperationFlag != util.IptablesCheckFlag {
 			log.Errorf("Error: There was an error running command: [%s %v] Stderr: [%v, %s]", cmdName, strings.Join(cmdArgs, " "), err, strings.TrimSuffix(string(msg.Stderr), "\n"))
 		}
 

npm/nwpolicy.go

@@ -46,6 +46,8 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
 		return nil
 	}
 
+	ns.rawNpMap[npObj.ObjectMeta.Name] = npObj
+
 	allNs := npMgr.nsMap[util.KubeAllNamespacesFlag]
 
 	if !npMgr.isAzureNpmChainCreated {
@@ -67,13 +69,16 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
 	var addedPolicy *networkingv1.NetworkPolicy
 	addedPolicy = nil
 	if oldPolicy, oldPolicyExists := ns.processedNpMap[hashedSelector]; oldPolicyExists {
+		npMgr.isSafeToCleanUpAzureNpmChain = false
+		npMgr.DeleteNetworkPolicy(oldPolicy)
+		npMgr.isSafeToCleanUpAzureNpmChain = true
+
 		addedPolicy, err = addPolicy(oldPolicy, npObj)
 		if err != nil {
 			log.Printf("Error adding policy %s to %s", npName, oldPolicy.ObjectMeta.Name)
+		} else {
+			ns.processedNpMap[hashedSelector] = addedPolicy
 		}
-		npMgr.isSafeToCleanUpAzureNpmChain = false
-		npMgr.DeleteNetworkPolicy(oldPolicy)
-		npMgr.isSafeToCleanUpAzureNpmChain = true
 	} else {
 		ns.processedNpMap[hashedSelector] = npObj
 	}
@@ -116,6 +121,10 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
 
 // UpdateNetworkPolicy handles updateing network policy in iptables.
 func (npMgr *NetworkPolicyManager) UpdateNetworkPolicy(oldNpObj *networkingv1.NetworkPolicy, newNpObj *networkingv1.NetworkPolicy) error {
+	if isSamePolicy(oldNpObj, newNpObj) {
+		return nil
+	}
+
 	var err error
 
 	log.Printf("NETWORK POLICY UPDATING:\n old policy:[%v]\n new policy:[%v]", oldNpObj, newNpObj)
@@ -164,6 +173,8 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
 		}
 	}
 
+	delete(ns.rawNpMap, npObj.ObjectMeta.Name)
+
 	hashedSelector := HashSelector(&npObj.Spec.PodSelector)
 	if oldPolicy, oldPolicyExists := ns.processedNpMap[hashedSelector]; oldPolicyExists {
 		deductedPolicy, err := deductPolicy(oldPolicy, npObj)
@@ -179,11 +190,11 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
 	}
 
 	if npMgr.canCleanUpNpmChains() {
+		npMgr.isAzureNpmChainCreated = false
 		if err = iptMgr.UninitNpmChains(); err != nil {
 			log.Errorf("Error: failed to uninitialize azure-npm chains.")
 			return err
 		}
-		npMgr.isAzureNpmChainCreated = false
 	}
 
 	return nil