diff --git a/.pipelines/npm/npm-conformance-tests.yaml b/.pipelines/npm/npm-conformance-tests.yaml index cc7963dbda..9d73bcef73 100644 --- a/.pipelines/npm/npm-conformance-tests.yaml +++ b/.pipelines/npm/npm-conformance-tests.yaml @@ -179,7 +179,10 @@ jobs: mkdir -p $(System.DefaultWorkingDirectory)/npmLogs for npm in $npmPodList; do kubectl logs -n kube-system $npm > $(System.DefaultWorkingDirectory)/npmLogs/$npm ;done displayName: "Gather NPM Logs" + condition: always() - publish: $(System.DefaultWorkingDirectory)/npmLogs + condition: always() + artifact: NpmLogs - job: Clean_up displayName: "Cleanup" diff --git a/npm/http/server/server_test.go b/npm/http/server/server_test.go index d1eeee3c93..e6f39165ba 100644 --- a/npm/http/server/server_test.go +++ b/npm/http/server/server_test.go @@ -15,13 +15,9 @@ import ( func TestGetNpmMgrHandler(t *testing.T) { assert := assert.New(t) npMgr := &npm.NetworkPolicyManager{ - NsMap: map[string]*npm.Namespace{ - "test": &npm.Namespace{ - PodMap: map[string]*npm.NpmPod{ - "": &npm.NpmPod{ - Name: "testpod", - }, - }, + PodMap: map[string]*npm.NpmPod{ + "": &npm.NpmPod{ + Name: "testpod", }, }, } diff --git a/npm/namespace.go b/npm/namespace.go index a408046b88..e09fac20d8 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -8,19 +8,16 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/iptm" + "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" ) type Namespace struct { name string LabelsMap map[string]string // NameSpace labels SetMap map[string]string - PodMap map[string]*NpmPod // Key is PodUID - rawNpMap map[string]*networkingv1.NetworkPolicy - ProcessedNpMap map[string]*networkingv1.NetworkPolicy IpsMgr *ipsm.IpsetManager iptMgr *iptm.IptablesManager resourceVersion uint64 // NameSpace ResourceVersion @@ -29,14 +26,11 @@ type Namespace struct { // newNS constructs a new namespace object. func newNs(name string) (*Namespace, error) { ns := &Namespace{ - name: name, - LabelsMap: make(map[string]string), - SetMap: make(map[string]string), - PodMap: make(map[string]*NpmPod), - rawNpMap: make(map[string]*networkingv1.NetworkPolicy), - ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), - IpsMgr: ipsm.NewIpsetManager(), - iptMgr: iptm.NewIptablesManager(), + name: name, + LabelsMap: make(map[string]string), + SetMap: make(map[string]string), + IpsMgr: ipsm.NewIpsetManager(), + iptMgr: iptm.NewIptablesManager(), // resource version is converted to uint64 // so make sure it is initialized to "0" resourceVersion: 0, @@ -63,28 +57,6 @@ func isInvalidNamespaceUpdate(oldNsObj, newNsObj *corev1.Namespace) (isInvalidUp return } -func (ns *Namespace) policyExists(npObj *networkingv1.NetworkPolicy) bool { - np, exists := ns.rawNpMap[npObj.ObjectMeta.Name] - if !exists { - return false - } - - if !util.CompareResourceVersions(np.ObjectMeta.ResourceVersion, npObj.ObjectMeta.ResourceVersion) { - log.Logf("Cached Network Policy has larger ResourceVersion number than new Obj. Name: %s Cached RV: %d New RV: %d\n", - npObj.ObjectMeta.Name, - np.ObjectMeta.ResourceVersion, - npObj.ObjectMeta.ResourceVersion, - ) - return true - } - - if isSamePolicy(np, npObj) { - return true - } - - return false -} - // InitAllNsList syncs all-namespace ipset list. func (npMgr *NetworkPolicyManager) InitAllNsList() error { allNs := npMgr.NsMap[util.KubeAllNamespacesFlag] @@ -94,7 +66,7 @@ func (npMgr *NetworkPolicyManager) InitAllNsList() error { } if err := allNs.IpsMgr.AddToList(util.KubeAllNamespacesFlag, ns); err != nil { - log.Errorf("Error: failed to add namespace set %s to ipset list %s", ns, util.KubeAllNamespacesFlag) + metrics.SendErrorLogAndMetric(util.NSID, "[InitAllNsList] Error: failed to add namespace set %s to ipset list %s with err: %v", ns, util.KubeAllNamespacesFlag, err) return err } } @@ -111,7 +83,7 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error { } if err := allNs.IpsMgr.DeleteFromList(util.KubeAllNamespacesFlag, ns); err != nil { - log.Errorf("Error: failed to delete namespace set %s from list %s", ns, util.KubeAllNamespacesFlag) + metrics.SendErrorLogAndMetric(util.NSID, "[UninitAllNsList] Error: failed to delete namespace set %s from list %s with err: %v", ns, util.KubeAllNamespacesFlag, err) return err } } @@ -129,12 +101,12 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { ipsMgr := npMgr.NsMap[util.KubeAllNamespacesFlag].IpsMgr // Create ipset for the namespace. if err = ipsMgr.CreateSet(nsName, append([]string{util.IpsetNetHashFlag})); err != nil { - log.Errorf("Error: failed to create ipset for namespace %s.", nsName) + metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] Error: failed to create ipset for namespace %s with err: %v", nsName, err) return err } if err = ipsMgr.AddToList(util.KubeAllNamespacesFlag, nsName); err != nil { - log.Errorf("Error: failed to add %s to all-namespace ipset list.", nsName) + metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] Error: failed to add %s to all-namespace ipset list with err: %v", nsName, err) return err } @@ -144,21 +116,21 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { labelKey := util.GetNSNameWithPrefix(nsLabelKey) log.Logf("Adding namespace %s to ipset list %s", nsName, labelKey) if err = ipsMgr.AddToList(labelKey, nsName); err != nil { - log.Errorf("Error: failed to add namespace %s to ipset list %s", nsName, labelKey) + metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] Error: failed to add namespace %s to ipset list %s with err: %v", nsName, labelKey, err) return err } label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal) log.Logf("Adding namespace %s to ipset list %s", nsName, label) if err = ipsMgr.AddToList(label, nsName); err != nil { - log.Errorf("Error: failed to add namespace %s to ipset list %s", nsName, label) + metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] Error: failed to add namespace %s to ipset list %s with err: %v", nsName, label, err) return err } } ns, err := newNs(nsName) if err != nil { - log.Errorf("Error: failed to create namespace %s", nsName) + metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] Error: failed to create namespace %s with err: %v", nsName, err) } setResourceVersion(ns, nsObj.GetObjectMeta().GetResourceVersion()) @@ -223,8 +195,8 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n //if no change in labels then return if reflect.DeepEqual(curNsObj.LabelsMap, newNsLabel) { log.Logf( - "NAMESPACE UPDATING:\n nothing to delete or add. old namespace: [%s/%v]\n cache namespace: [%s/%v] new namespace: [%s/%v]", - oldNsNs, oldNsLabel, curNsObj.name, curNsObj.LabelsMap, newNsNs, newNsLabel, + "NAMESPACE UPDATING: nothing to delete or add. namespace: [%s/%v]", + newNsNs, newNsLabel, ) return nil } @@ -238,7 +210,7 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n labelKey := util.GetNSNameWithPrefix(nsLabelVal) log.Logf("Deleting namespace %s from ipset list %s", oldNsNs, labelKey) if err = ipsMgr.DeleteFromList(labelKey, oldNsNs); err != nil { - log.Errorf("Error: failed to delete namespace %s from ipset list %s", oldNsNs, labelKey) + metrics.SendErrorLogAndMetric(util.NSID, "[UpdateNamespace] Error: failed to delete namespace %s from ipset list %s with err: %v", oldNsNs, labelKey, err) return err } } @@ -248,7 +220,7 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n labelKey := util.GetNSNameWithPrefix(nsLabelVal) log.Logf("Adding namespace %s to ipset list %s", oldNsNs, labelKey) if err = ipsMgr.AddToList(labelKey, oldNsNs); err != nil { - log.Errorf("Error: failed to add namespace %s to ipset list %s", oldNsNs, labelKey) + metrics.SendErrorLogAndMetric(util.NSID, "[UpdateNamespace] Error: failed to add namespace %s to ipset list %s with err: %v", oldNsNs, labelKey, err) return err } } @@ -281,27 +253,27 @@ func (npMgr *NetworkPolicyManager) DeleteNamespace(nsObj *corev1.Namespace) erro labelKey := util.GetNSNameWithPrefix(nsLabelKey) log.Logf("Deleting namespace %s from ipset list %s", nsName, labelKey) if err = ipsMgr.DeleteFromList(labelKey, nsName); err != nil { - log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, labelKey) + metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] Error: failed to delete namespace %s from ipset list %s with err: %v", nsName, labelKey, err) return err } label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal) log.Logf("Deleting namespace %s from ipset list %s", nsName, label) if err = ipsMgr.DeleteFromList(label, nsName); err != nil { - log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, label) + metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] Error: failed to delete namespace %s from ipset list %s with err: %v", nsName, label, err) return err } } // Delete the namespace from all-namespace ipset list. if err = ipsMgr.DeleteFromList(util.KubeAllNamespacesFlag, nsName); err != nil { - log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, util.KubeAllNamespacesFlag) + metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] Error: failed to delete namespace %s from ipset list %s with err: %v", nsName, util.KubeAllNamespacesFlag, err) return err } // Delete ipset for the namespace. if err = ipsMgr.DeleteSet(nsName); err != nil { - log.Errorf("Error: failed to delete ipset for namespace %s.", nsName) + metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] Error: failed to delete ipset for namespace %s with err: %v", nsName, err) return err } diff --git a/npm/namespace_test.go b/npm/namespace_test.go index 123d7b831e..e1d88fab93 100644 --- a/npm/namespace_test.go +++ b/npm/namespace_test.go @@ -13,6 +13,7 @@ import ( "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -48,6 +49,9 @@ func TestAllNsList(t *testing.T) { func TestAddNamespace(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -87,6 +91,9 @@ func TestAddNamespace(t *testing.T) { func TestUpdateNamespace(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -139,6 +146,9 @@ func TestUpdateNamespace(t *testing.T) { func TestAddNamespaceLabel(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -200,6 +210,9 @@ func TestAddNamespaceLabel(t *testing.T) { func TestDeleteandUpdateNamespaceLabel(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -261,6 +274,9 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) { func TestDeleteNamespace(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } diff --git a/npm/npm.go b/npm/npm.go index 7d954430eb..b83ef51dc2 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -50,6 +50,9 @@ type NetworkPolicyManager struct { NodeName string NsMap map[string]*Namespace + PodMap map[string]*NpmPod // Key is ns-// + RawNpMap map[string]*networkingv1.NetworkPolicy // Key is ns-/ + ProcessedNpMap map[string]*networkingv1.NetworkPolicy // Key is ns-/ isAzureNpmChainCreated bool isSafeToCleanUpAzureNpmChain bool @@ -120,10 +123,8 @@ func (npMgr *NetworkPolicyManager) SendClusterMetrics() { podCount.Value = 0 //Reducing one to remove all-namespaces ns obj nsCount.Value = float64(len(npMgr.NsMap) - 1) - for _, ns := range npMgr.NsMap { - nwPolicyCount.Value += float64(len(ns.rawNpMap)) - podCount.Value += float64(len(ns.PodMap)) - } + nwPolicyCount.Value += float64(len(npMgr.RawNpMap)) + podCount.Value += float64(len(npMgr.PodMap)) npMgr.Unlock() metrics.SendMetric(podCount) @@ -232,6 +233,9 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in npInformer: npInformer, NodeName: os.Getenv("HOSTNAME"), NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), isAzureNpmChainCreated: false, isSafeToCleanUpAzureNpmChain: false, clusterState: telemetry.ClusterState{ diff --git a/npm/nwpolicy.go b/npm/nwpolicy.go index 6f1c55f301..bfd564fcf1 100644 --- a/npm/nwpolicy.go +++ b/npm/nwpolicy.go @@ -3,6 +3,7 @@ package npm import ( + "fmt" "strconv" "github.com/Azure/azure-container-networking/log" @@ -13,54 +14,111 @@ import ( networkingv1 "k8s.io/api/networking/v1" ) +// GetNetworkPolicyKey will return netpolKey +func GetNetworkPolicyKey(npObj *networkingv1.NetworkPolicy) string { + netpolKey, err := util.GetObjKeyFunc(npObj) + if err != nil { + metrics.SendErrorLogAndMetric(util.NetpolID, "[GetNetworkPolicyKey] Error: while running MetaNamespaceKeyFunc err: %s", err) + return "" + } + if len(netpolKey) == 0 { + return "" + } + return util.GetNSNameWithPrefix(netpolKey) +} + +// GetProcessedNPKey will return netpolKey +func GetProcessedNPKey(npObj *networkingv1.NetworkPolicy, hashSelector string) string { + // hashSelector will never be empty + netpolKey := hashSelector + if len(npObj.GetNamespace()) > 0 { + netpolKey = npObj.GetNamespace() + "/" + netpolKey + } + return util.GetNSNameWithPrefix(netpolKey) +} + func (npMgr *NetworkPolicyManager) canCleanUpNpmChains() bool { if !npMgr.isSafeToCleanUpAzureNpmChain { return false } - for _, ns := range npMgr.NsMap { - if len(ns.ProcessedNpMap) > 0 { - return false - } + if len(npMgr.ProcessedNpMap) > 0 { + return false } return true } +func (npMgr *NetworkPolicyManager) policyExists(npObj *networkingv1.NetworkPolicy) bool { + npKey := GetNetworkPolicyKey(npObj) + if npKey == "" { + return false + } + + np, exists := npMgr.RawNpMap[npKey] + if !exists { + return false + } + + if !util.CompareResourceVersions(np.ObjectMeta.ResourceVersion, npObj.ObjectMeta.ResourceVersion) { + log.Logf("Cached Network Policy has larger ResourceVersion number than new Obj. Name: %s Cached RV: %d New RV: %d\n", + npObj.ObjectMeta.Name, + np.ObjectMeta.ResourceVersion, + npObj.ObjectMeta.ResourceVersion, + ) + return true + } + + if isSamePolicy(np, npObj) { + return true + } + + return false +} + // AddNetworkPolicy handles adding network policy to iptables. func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkPolicy) error { var ( - err error - ns *Namespace - exists bool - npNs = util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace) - npName = npObj.ObjectMeta.Name - allNs = npMgr.NsMap[util.KubeAllNamespacesFlag] - timer = metrics.StartNewTimer() + err error + ns *Namespace + exists bool + npNs = util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace) + npName = npObj.ObjectMeta.Name + allNs = npMgr.NsMap[util.KubeAllNamespacesFlag] + timer = metrics.StartNewTimer() + hashedSelector = HashSelector(&npObj.Spec.PodSelector) + npKey = GetNetworkPolicyKey(npObj) + npProcessedKey = GetProcessedNPKey(npObj, hashedSelector) ) log.Logf("NETWORK POLICY CREATING: NameSpace%s, Name:%s", npNs, npName) + if npKey == "" { + err = fmt.Errorf("[AddNetworkPolicy] Error: npKey is empty for %s network policy in %s", npName, npNs) + metrics.SendErrorLogAndMetric(util.NetpolID, err.Error()) + return err + } + if ns, exists = npMgr.NsMap[npNs]; !exists { ns, err = newNs(npNs) if err != nil { - log.Logf("Error creating namespace %s\n", npNs) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating namespace %s with err: %v", npNs, err) } npMgr.NsMap[npNs] = ns } - if ns.policyExists(npObj) { + if npMgr.policyExists(npObj) { return nil } if !npMgr.isAzureNpmChainCreated { if err = allNs.IpsMgr.CreateSet(util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil { - log.Errorf("Error: failed to initialize kube-system ipset.") + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to initialize kube-system ipset with err %s", err) return err } if err = allNs.iptMgr.InitNpmChains(); err != nil { - log.Errorf("Error: failed to initialize azure-npm chains.") + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to initialize azure-npm chains with err %s", err) return err } @@ -68,7 +126,6 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP } var ( - hashedSelector = HashSelector(&npObj.Spec.PodSelector) addedPolicy *networkingv1.NetworkPolicy sets, namedPorts, lists []string ingressIPCidrs, egressIPCidrs [][]string @@ -77,55 +134,55 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP ) // Remove the existing policy from processed (merged) network policy map - if oldPolicy, oldPolicyExists := ns.rawNpMap[npObj.ObjectMeta.Name]; oldPolicyExists { + if oldPolicy, oldPolicyExists := npMgr.RawNpMap[npKey]; oldPolicyExists { npMgr.isSafeToCleanUpAzureNpmChain = false npMgr.DeleteNetworkPolicy(oldPolicy) npMgr.isSafeToCleanUpAzureNpmChain = true } // Add (merge) the new policy with others who apply to the same pods - if oldPolicy, oldPolicyExists := ns.ProcessedNpMap[hashedSelector]; oldPolicyExists { + if oldPolicy, oldPolicyExists := npMgr.ProcessedNpMap[npProcessedKey]; oldPolicyExists { addedPolicy, err = addPolicy(oldPolicy, npObj) if err != nil { - log.Logf("Error adding policy %s to %s", npName, oldPolicy.ObjectMeta.Name) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: adding policy %s to %s with err: %v", npName, oldPolicy.ObjectMeta.Name, err) } } if addedPolicy != nil { - ns.ProcessedNpMap[hashedSelector] = addedPolicy + npMgr.ProcessedNpMap[npProcessedKey] = addedPolicy } else { - ns.ProcessedNpMap[hashedSelector] = npObj + npMgr.ProcessedNpMap[npProcessedKey] = npObj } - ns.rawNpMap[npObj.ObjectMeta.Name] = npObj + npMgr.RawNpMap[npKey] = npObj sets, namedPorts, lists, ingressIPCidrs, egressIPCidrs, iptEntries = translatePolicy(npObj) for _, set := range sets { log.Logf("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set)) if err = ipsMgr.CreateSet(set, append([]string{util.IpsetNetHashFlag})); err != nil { - log.Logf("Error creating ipset %s", set) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating ipset %s with err: %v", set, err) } } for _, set := range namedPorts { log.Logf("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set)) if err = ipsMgr.CreateSet(set, append([]string{util.IpsetIPPortHashFlag})); err != nil { - log.Logf("Error creating ipset named port %s", set) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating ipset named port %s with err: %v", set, err) } } for _, list := range lists { if err = ipsMgr.CreateList(list); err != nil { - log.Logf("Error creating ipset list %s", list) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: creating ipset list %s with err: %v", list, err) } } if err = npMgr.InitAllNsList(); err != nil { - log.Logf("Error initializing all-namespace ipset list.") + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: initializing all-namespace ipset list with err: %v", err) } createCidrsRule("in", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, ingressIPCidrs, ipsMgr) createCidrsRule("out", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, egressIPCidrs, ipsMgr) iptMgr := allNs.iptMgr for _, iptEntry := range iptEntries { if err = iptMgr.Add(iptEntry); err != nil { - log.Errorf("Error: failed to apply iptables rule. Rule: %+v", iptEntry) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to apply iptables rule. Rule: %+v with err: %v", iptEntry, err) } } @@ -148,21 +205,20 @@ func (npMgr *NetworkPolicyManager) UpdateNetworkPolicy(oldNpObj *networkingv1.Ne // DeleteNetworkPolicy handles deleting network policy from iptables. func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.NetworkPolicy) error { var ( - err error - ns *Namespace - allNs = npMgr.NsMap[util.KubeAllNamespacesFlag] + err error + allNs = npMgr.NsMap[util.KubeAllNamespacesFlag] + hashedSelector = HashSelector(&npObj.Spec.PodSelector) + npKey = GetNetworkPolicyKey(npObj) + npProcessedKey = GetProcessedNPKey(npObj, hashedSelector) ) npNs, npName := util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace), npObj.ObjectMeta.Name log.Logf("NETWORK POLICY DELETING: Namespace: %s, Name:%s", npNs, npName) - var exists bool - if ns, exists = npMgr.NsMap[npNs]; !exists { - ns, err = newNs(npName) - if err != nil { - log.Logf("Error creating namespace %s", npNs) - } - npMgr.NsMap[npNs] = ns + if npKey == "" { + err = fmt.Errorf("[AddNetworkPolicy] Error: npKey is empty for %s network policy in %s", npName, npNs) + metrics.SendErrorLogAndMetric(util.NetpolID, err.Error()) + return err } _, _, _, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(npObj) @@ -170,33 +226,32 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo iptMgr := allNs.iptMgr for _, iptEntry := range iptEntries { if err = iptMgr.Delete(iptEntry); err != nil { - log.Errorf("Error: failed to apply iptables rule. Rule: %+v", iptEntry) + metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: failed to apply iptables rule. Rule: %+v with err: %v", iptEntry, err) } } removeCidrsRule("in", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, ingressIPCidrs, allNs.IpsMgr) removeCidrsRule("out", npObj.ObjectMeta.Name, npObj.ObjectMeta.Namespace, egressIPCidrs, allNs.IpsMgr) - delete(ns.rawNpMap, npObj.ObjectMeta.Name) + delete(npMgr.RawNpMap, npKey) - hashedSelector := HashSelector(&npObj.Spec.PodSelector) - if oldPolicy, oldPolicyExists := ns.ProcessedNpMap[hashedSelector]; oldPolicyExists { + if oldPolicy, oldPolicyExists := npMgr.ProcessedNpMap[npProcessedKey]; oldPolicyExists { deductedPolicy, err := deductPolicy(oldPolicy, npObj) if err != nil { - log.Logf("Error deducting policy %s from %s", npName, oldPolicy.ObjectMeta.Name) + metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: deducting policy %s from %s with err: %v", npName, oldPolicy.ObjectMeta.Name, err) } if deductedPolicy == nil { - delete(ns.ProcessedNpMap, hashedSelector) + delete(npMgr.ProcessedNpMap, npProcessedKey) } else { - ns.ProcessedNpMap[hashedSelector] = deductedPolicy + npMgr.ProcessedNpMap[npProcessedKey] = deductedPolicy } } if npMgr.canCleanUpNpmChains() { npMgr.isAzureNpmChainCreated = false if err = iptMgr.UninitNpmChains(); err != nil { - log.Errorf("Error: failed to uninitialize azure-npm chains.") + metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: failed to uninitialize azure-npm chains with err: %s", err) return err } } @@ -215,7 +270,7 @@ func createCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]st setName := policyName + "-in-ns-" + ns + "-" + strconv.Itoa(i) + ingressOrEgress log.Logf("Creating set: %v, hashedSet: %v", setName, util.GetHashedName(setName)) if err := ipsMgr.CreateSet(setName, spec); err != nil { - log.Logf("Error creating ipset %s", ipCidrSet) + metrics.SendErrorLogAndMetric(util.NetpolID, "[createCidrsRule] Error: creating ipset %s with err: %v", ipCidrSet, err) } for _, ipCidrEntry := range util.DropEmptyFields(ipCidrSet) { // Ipset doesn't allow 0.0.0.0/0 to be added. A general solution is split 0.0.0.0/1 in half which convert to @@ -224,12 +279,12 @@ func createCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]st splitEntry := [2]string{"1.0.0.0/1", "128.0.0.0/1"} for _, entry := range splitEntry { if err := ipsMgr.AddToSet(setName, entry, util.IpsetNetHashFlag, ""); err != nil { - log.Logf("Error adding ip cidrs %s into ipset %s", entry, ipCidrSet) + metrics.SendErrorLogAndMetric(util.NetpolID, "[createCidrsRule] adding ip cidrs %s into ipset %s with err: %v", entry, ipCidrSet, err) } } } else { if err := ipsMgr.AddToSet(setName, ipCidrEntry, util.IpsetNetHashFlag, ""); err != nil { - log.Logf("Error adding ip cidrs %s into ipset %s", ipCidrEntry, ipCidrSet) + metrics.SendErrorLogAndMetric(util.NetpolID, "[createCidrsRule] adding ip cidrs %s into ipset %s with err: %v", ipCidrEntry, ipCidrSet, err) } } } @@ -244,7 +299,7 @@ func removeCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]st setName := policyName + "-in-ns-" + ns + "-" + strconv.Itoa(i) + ingressOrEgress log.Logf("Delete set: %v, hashedSet: %v", setName, util.GetHashedName(setName)) if err := ipsMgr.DeleteSet(setName); err != nil { - log.Logf("Error deleting ipset %s", ipCidrSet) + metrics.SendErrorLogAndMetric(util.NetpolID, "[removeCidrsRule] deleting ipset %s with err: %v", ipCidrSet, err) } } } diff --git a/npm/nwpolicy_test.go b/npm/nwpolicy_test.go index d46bf17194..87a229d663 100644 --- a/npm/nwpolicy_test.go +++ b/npm/nwpolicy_test.go @@ -20,6 +20,9 @@ import ( func TestAddNetworkPolicy(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -164,6 +167,9 @@ func TestAddNetworkPolicy(t *testing.T) { func TestUpdateNetworkPolicy(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -276,6 +282,9 @@ func TestUpdateNetworkPolicy(t *testing.T) { func TestDeleteNetworkPolicy(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -368,3 +377,33 @@ func TestDeleteNetworkPolicy(t *testing.T) { t.Errorf("Change in policy number didn't register in prometheus") } } +func TestGetNetworkPolicyKey(t *testing.T) { + npObj := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "allow-egress", + Namespace: "test-nwpolicy", + }, + Spec: networkingv1.NetworkPolicySpec{ + Egress: []networkingv1.NetworkPolicyEgressRule{ + networkingv1.NetworkPolicyEgressRule{ + To: []networkingv1.NetworkPolicyPeer{{ + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"ns": "test"}, + }, + }}, + }, + }, + }, + } + + netpolKey := GetNetworkPolicyKey(npObj) + + if netpolKey == "" { + t.Errorf("TestGetNetworkPolicyKey failed @ netpolKey length check %s", netpolKey) + } + + expectedKey := util.GetNSNameWithPrefix("test-nwpolicy/allow-egress") + if netpolKey != expectedKey { + t.Errorf("TestGetNetworkPolicyKey failed @ netpolKey did not match expected value %s", netpolKey) + } +} diff --git a/npm/pod.go b/npm/pod.go index 8e5162f00d..e3806cb02e 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/ipsm" + "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" @@ -153,6 +154,17 @@ func appendNamedPortIpsets(ipsMgr *ipsm.IpsetManager, portList []v1.ContainerPor return nil } +// GetPodKey will return podKey +func GetPodKey(podObj *corev1.Pod) string { + podKey, err := util.GetObjKeyFunc(podObj) + if err != nil { + metrics.SendErrorLogAndMetric(util.PodID, "[GetPodKey] Error: while running MetaNamespaceKeyFunc err: %s", err) + return "" + } + podKey = podKey + "/" + string(podObj.GetObjectMeta().GetUID()) + return util.GetNSNameWithPrefix(podKey) +} + // AddPod handles adding pod ip to its label's ipset. func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { if !isValidPod(podObj) { @@ -173,12 +185,13 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { npmPodObj, podErr := newNpmPod(podObj) if podErr != nil { - log.Errorf("Error: failed to create namespace %s, %+v", podObj.ObjectMeta.Name, podObj) + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to create namespace %s, %+v with err %v", podObj.ObjectMeta.Name, podObj, podErr) return podErr } var ( err error + podKey = GetPodKey(podObj) podNs = util.GetNSNameWithPrefix(npmPodObj.Namespace) podUID = npmPodObj.PodUID podName = npmPodObj.Name @@ -191,45 +204,51 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { log.Logf("POD CREATING: [%s%s/%s/%s%+v%s]", podUID, podNs, podName, podNodeName, podLabels, podIP) + if podKey == "" { + err = fmt.Errorf("[AddPod] Error: podKey is empty for %s pod in %s with UID %s", podName, podNs, podUID) + metrics.SendErrorLogAndMetric(util.PodID, err.Error()) + return err + } + // Add pod namespace if it doesn't exist if _, exists := npMgr.NsMap[podNs]; !exists { npMgr.NsMap[podNs], err = newNs(podNs) if err != nil { - log.Errorf("Error: failed to create namespace %s", podNs) + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to create namespace %s with err: %v", podNs, err) } log.Logf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs)) if err = ipsMgr.CreateSet(podNs, append([]string{util.IpsetNetHashFlag})); err != nil { - log.Logf("Error creating ipset %s", podNs) + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: creating ipset %s with err: %v", podNs, err) } } // Add the pod to its namespace's ipset. log.Logf("Adding pod %s to ipset %s", podIP, podNs) if err = ipsMgr.AddToSet(podNs, podIP, util.IpsetNetHashFlag, podUID); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to namespace ipset with err: %v", err) } // Add the pod to its label's ipset. for podLabelKey, podLabelVal := range podLabels { log.Logf("Adding pod %s to ipset %s", podIP, podLabelKey) if err = ipsMgr.AddToSet(podLabelKey, podIP, util.IpsetNetHashFlag, podUID); err != nil { - log.Errorf("Error: failed to add pod to label ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to label ipset with err: %v", err) } label := podLabelKey + ":" + podLabelVal log.Logf("Adding pod %s to ipset %s", podIP, label) if err = ipsMgr.AddToSet(label, podIP, util.IpsetNetHashFlag, podUID); err != nil { - log.Errorf("Error: failed to add pod to label ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to label ipset with err: %v", err) } } // Add pod's named ports from its ipset. if err = appendNamedPortIpsets(ipsMgr, podContainerPorts, podUID, podIP, false); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] Error: failed to add pod to namespace ipset with err: %v", err) } // add the Pod info to the podMap - npMgr.NsMap[podNs].PodMap[podUID] = npmPodObj + npMgr.PodMap[podKey] = npmPodObj return nil } @@ -252,6 +271,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { var ( err error + podKey = GetPodKey(newPodObj) newPodObjNs = util.GetNSNameWithPrefix(newPodObj.ObjectMeta.Namespace) newPodObjName = newPodObj.ObjectMeta.Name newPodObjLabel = newPodObj.ObjectMeta.Labels @@ -260,22 +280,28 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { ipsMgr = npMgr.NsMap[util.KubeAllNamespacesFlag].IpsMgr ) + if podKey == "" { + err = fmt.Errorf("[UpdatePod] Error: podKey is empty for %s pod in %s with UID %s", newPodObjName, newPodObjNs, string(newPodObj.ObjectMeta.UID)) + metrics.SendErrorLogAndMetric(util.PodID, err.Error()) + return err + } + // Add pod namespace if it doesn't exist if _, exists := npMgr.NsMap[newPodObjNs]; !exists { npMgr.NsMap[newPodObjNs], err = newNs(newPodObjNs) if err != nil { - log.Errorf("Error: failed to create namespace %s", newPodObjNs) + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to create namespace %s with err: %v", newPodObjNs, err) } log.Logf("Creating set: %v, hashedSet: %v", newPodObjNs, util.GetHashedName(newPodObjNs)) if err = ipsMgr.CreateSet(newPodObjNs, append([]string{util.IpsetNetHashFlag})); err != nil { - log.Logf("Error creating ipset %s", newPodObjNs) + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error creating ipset %s with err: %v", newPodObjNs, err) } } - cachedPodObj, exists := npMgr.NsMap[newPodObjNs].PodMap[string(newPodObj.ObjectMeta.UID)] + cachedPodObj, exists := npMgr.PodMap[podKey] if !exists { if addErr := npMgr.AddPod(newPodObj); addErr != nil { - log.Errorf("Error: failed to add pod during update with error %+v", addErr) + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod during update with error %+v", addErr) } return nil } @@ -301,7 +327,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { // We are assuming that FAILED to RUNNING pod will send an update if newPodObj.Status.Phase == v1.PodSucceeded || newPodObj.Status.Phase == v1.PodFailed { if delErr := npMgr.DeletePod(newPodObj); delErr != nil { - log.Errorf("Error: failed to add pod during update with error %+v", delErr) + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod during update with error %+v", delErr) } return nil @@ -323,8 +349,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { // if the podIp exists, it must match the cachedIp if cachedPodIP != newPodObjIP { - // TODO Add AI telemetry event - log.Errorf("Error: Unexpected state. Pod (Namespace:%s, Name:%s, uid:%s, has cachedPodIp:%s which is different from PodIp:%s", + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Info: Unexpected state. Pod (Namespace:%s, Name:%s, uid:%s, has cachedPodIp:%s which is different from PodIp:%s", newPodObjNs, newPodObjName, cachedPodObj.PodUID, cachedPodIP, newPodObjIP) // cached PodIP needs to be cleaned up from all the cached labels deleteFromIPSets = util.GetIPSetListFromLabels(cachedLabels) @@ -343,11 +368,11 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { newPodObjNs, ) if err = ipsMgr.DeleteFromSet(cachedPodObj.Namespace, cachedPodIP, cachedPodObj.PodUID); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to delete pod from namespace ipset with err: %v", err) } // Add the pod to its namespace's ipset. if err = ipsMgr.AddToSet(newPodObjNs, newPodObjIP, util.IpsetNetHashFlag, cachedPodObj.PodUID); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod to namespace ipset with err: %v", err) } } else { //if no change in labels then return @@ -366,7 +391,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { for _, podIPSetName := range deleteFromIPSets { log.Logf("Deleting pod %s from ipset %s", cachedPodIP, podIPSetName) if err = ipsMgr.DeleteFromSet(podIPSetName, cachedPodIP, cachedPodObj.PodUID); err != nil { - log.Errorf("Error: failed to delete pod from label ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to delete pod from label ipset with err: %v", err) } } @@ -374,7 +399,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { for _, addIPSetName := range addToIPSets { log.Logf("Adding pod %s to ipset %s", newPodObjIP, addIPSetName) if err = ipsMgr.AddToSet(addIPSetName, newPodObjIP, util.IpsetNetHashFlag, cachedPodObj.PodUID); err != nil { - log.Errorf("Error: failed to add pod to label ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod to label ipset with err: %v", err) } } @@ -385,16 +410,16 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { if !reflect.DeepEqual(cachedPodObj.ContainerPorts, newPodPorts) { // Delete cached pod's named ports from its ipset. if err = appendNamedPortIpsets(ipsMgr, cachedPodObj.ContainerPorts, cachedPodObj.PodUID, cachedPodIP, true); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to delete pod from namespace ipset with err: %v", err) } // Add new pod's named ports from its ipset. if err = appendNamedPortIpsets(ipsMgr, newPodPorts, cachedPodObj.PodUID, newPodObjIP, false); err != nil { - log.Errorf("Error: failed to add pod to namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error: failed to add pod to namespace ipset with err: %v", err) } } // Updating pod cache with new information - npMgr.NsMap[newPodObjNs].PodMap[cachedPodObj.PodUID], err = newNpmPod(newPodObj) + npMgr.PodMap[podKey], err = newNpmPod(newPodObj) if err != nil { return err } @@ -410,30 +435,33 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { podNs := util.GetNSNameWithPrefix(podObj.Namespace) var ( - err error - podName = podObj.ObjectMeta.Name - podNodeName = podObj.Spec.NodeName - podLabels = podObj.ObjectMeta.Labels - ipsMgr = npMgr.NsMap[util.KubeAllNamespacesFlag].IpsMgr - podUID = string(podObj.ObjectMeta.UID) - cachedPodIP = podObj.Status.PodIP - containerPorts = getContainerPortList(podObj) + err error + podKey = GetPodKey(podObj) + podName = podObj.ObjectMeta.Name + podNodeName = podObj.Spec.NodeName + ipsMgr = npMgr.NsMap[util.KubeAllNamespacesFlag].IpsMgr + podUID = string(podObj.ObjectMeta.UID) ) - _, nsExists := npMgr.NsMap[podNs] - if nsExists { - cachedPodObj, podExists := npMgr.NsMap[podNs].PodMap[string(podObj.ObjectMeta.UID)] - if podExists { - cachedPodIP = cachedPodObj.PodIP - podLabels = cachedPodObj.Labels - containerPorts = cachedPodObj.ContainerPorts - } + if podKey == "" { + err = fmt.Errorf("[DeletePod] Error: podKey is empty for %s pod in %s with UID %s", podName, podNs, podUID) + metrics.SendErrorLogAndMetric(util.PodID, err.Error()) + return err + } + + cachedPodObj, podExists := npMgr.PodMap[podKey] + if !podExists { + return nil } + var ( + cachedPodIP = cachedPodObj.PodIP + podLabels = cachedPodObj.Labels + containerPorts = cachedPodObj.ContainerPorts + ) // if the podIp exists, it must match the cachedIp if len(podObj.Status.PodIP) > 0 && cachedPodIP != podObj.Status.PodIP { - // TODO Add AI telemetry event - log.Errorf("Error: Unexpected state. Pod (Namespace:%s, Name:%s, uid:%s, has cachedPodIp:%s which is different from PodIp:%s", + metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Info: Unexpected state. Pod (Namespace:%s, Name:%s, uid:%s, has cachedPodIp:%s which is different from PodIp:%s", podNs, podName, podUID, cachedPodIP, podObj.Status.PodIP) } @@ -441,31 +469,29 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { // Delete the pod from its namespace's ipset. if err = ipsMgr.DeleteFromSet(podNs, cachedPodIP, podUID); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from namespace ipset with err: %v", err) } // Delete the pod from its label's ipset. for podLabelKey, podLabelVal := range podLabels { log.Logf("Deleting pod %s from ipset %s", cachedPodIP, podLabelKey) if err = ipsMgr.DeleteFromSet(podLabelKey, cachedPodIP, podUID); err != nil { - log.Errorf("Error: failed to delete pod from label ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from label ipset with err: %v", err) } label := podLabelKey + ":" + podLabelVal log.Logf("Deleting pod %s from ipset %s", cachedPodIP, label) if err = ipsMgr.DeleteFromSet(label, cachedPodIP, podUID); err != nil { - log.Errorf("Error: failed to delete pod from label ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from label ipset with err: %v", err) } } // Delete pod's named ports from its ipset. Delete is TRUE if err = appendNamedPortIpsets(ipsMgr, containerPorts, podUID, cachedPodIP, true); err != nil { - log.Errorf("Error: failed to delete pod from namespace ipset.") + metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] Error: failed to delete pod from namespace ipset with err: %v", err) } - if nsExists { - delete(npMgr.NsMap[podNs].PodMap, podUID) - } + delete(npMgr.PodMap, podKey) return nil } diff --git a/npm/pod_test.go b/npm/pod_test.go index d6b95d6891..18fdf7e801 100644 --- a/npm/pod_test.go +++ b/npm/pod_test.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-container-networking/npm/ipsm" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,6 +39,9 @@ func TestIsSystemPod(t *testing.T) { func TestAddPod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -94,6 +98,9 @@ func TestAddPod(t *testing.T) { func TestUpdatePod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -152,7 +159,9 @@ func TestUpdatePod(t *testing.T) { t.Errorf("TestUpdatePod failed @ UpdatePod") } - cachedPodObj, exists := npMgr.NsMap["ns-"+newPodObj.Namespace].PodMap[string(newPodObj.ObjectMeta.UID)] + podKey := GetPodKey(newPodObj) + + cachedPodObj, exists := npMgr.PodMap[podKey] if !exists { t.Errorf("TestUpdatePod failed @ pod exists check") } @@ -166,6 +175,9 @@ func TestUpdatePod(t *testing.T) { func TestOldRVUpdatePod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -203,7 +215,7 @@ func TestOldRVUpdatePod(t *testing.T) { newPodObj := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: "new-test-pod", + Name: "old-test-pod", Namespace: "test-namespace", Labels: map[string]string{ "app": "new-test-pod", @@ -225,7 +237,9 @@ func TestOldRVUpdatePod(t *testing.T) { t.Errorf("TestOldRVUpdatePod failed @ UpdatePod") } - cachedPodObj, exists := npMgr.NsMap["ns-"+newPodObj.Namespace].PodMap[string(newPodObj.ObjectMeta.UID)] + podKey := GetPodKey(newPodObj) + + cachedPodObj, exists := npMgr.PodMap[podKey] if !exists { t.Errorf("TestOldRVUpdatePod failed @ pod exists check") } @@ -244,6 +258,9 @@ func TestOldRVUpdatePod(t *testing.T) { func TestDeletePod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -287,7 +304,7 @@ func TestDeletePod(t *testing.T) { t.Errorf("TestDeletePod failed @ DeletePod") } - if len(npMgr.NsMap["ns-"+podObj.Namespace].PodMap) > 1 { + if len(npMgr.PodMap) > 1 { t.Errorf("TestDeletePod failed @ podMap length check") } npMgr.Unlock() @@ -296,6 +313,9 @@ func TestDeletePod(t *testing.T) { func TestAddHostNetworkPod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -347,6 +367,9 @@ func TestAddHostNetworkPod(t *testing.T) { func TestUpdateHostNetworkPod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -421,6 +444,9 @@ func TestUpdateHostNetworkPod(t *testing.T) { func TestDeleteHostNetworkPod(t *testing.T) { npMgr := &NetworkPolicyManager{ NsMap: make(map[string]*Namespace), + PodMap: make(map[string]*NpmPod), + RawNpMap: make(map[string]*networkingv1.NetworkPolicy), + ProcessedNpMap: make(map[string]*networkingv1.NetworkPolicy), TelemetryEnabled: false, } @@ -472,3 +498,35 @@ func TestDeleteHostNetworkPod(t *testing.T) { } npMgr.Unlock() } + +func TestGetPodKey(t *testing.T) { + podObj := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + Labels: map[string]string{ + "app": "test-pod", + }, + UID: "1234", + }, + Status: corev1.PodStatus{ + Phase: "Running", + PodIP: "1.2.3.4", + }, + Spec: corev1.PodSpec{ + HostNetwork: true, + }, + } + + podKey := GetPodKey(podObj) + + // 2 characters are / + if len(podKey) <= 2 { + t.Errorf("TestGetPodKey failed @ podKey length check %s", podKey) + } + + expectedKey := util.GetNSNameWithPrefix("test-namespace/test-pod/1234") + if podKey != expectedKey { + t.Errorf("TestGetPodKey failed @ podKey did not match expected value %s", podKey) + } +} diff --git a/npm/util/const.go b/npm/util/const.go index 14146f9dfe..092d9dc77a 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -165,13 +165,21 @@ const ( GetEnvRetryWaitTimeInSecs int = 3 AiInitializeRetryCount int = 3 AiInitializeRetryInMin int = 1 - // These ID represents where did the error log generate from. - // It's for better query purpose. - NpmID int = 1 - IpsmID int = 2 - IptmID int = 3 DebugMode bool = true ErrorValue float64 = 1 ) + +// These ID represents where did the error log generate from. +// It's for better query purpose. In Kusto these value are used in +// OperationID column +const ( + NpmID int = iota + 1 + IpsmID + IptmID + NSID + PodID + NetpolID + UtilID +) diff --git a/npm/util/util.go b/npm/util/util.go index 6d7cd2d7e0..9f7d455cf5 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Masterminds/semver" "k8s.io/apimachinery/pkg/version" + "k8s.io/client-go/tools/cache" ) // IsNewNwPolicyVerFlag indicates if the current kubernetes version is newer than 1.11 or not @@ -291,3 +292,8 @@ func ParseResourceVersion(rv string) uint64 { return rvInt } + +// GetObjKeyFunc will return obj's key +func GetObjKeyFunc(obj interface{}) (string, error) { + return cache.MetaNamespaceKeyFunc(obj) +}