From b012d7473a91545883c384d13f9eb3e8e5d54ee4 Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 11 Mar 2021 11:46:41 -0800 Subject: [PATCH 1/8] first pass at decoupling resource maps --- npm/util/const.go | 18 +++++++++++++----- npm/util/util.go | 6 ++++++ 2 files changed, 19 insertions(+), 5 deletions(-) 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) +} From 7d7afd4497b58ae4bddd8dd3af520cb9fc979217 Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 11 Mar 2021 13:17:56 -0800 Subject: [PATCH 2/8] First pass on decoupling resource maps --- .pipelines/npm/npm-conformance-tests.yaml | 2 + npm/http/server/server_test.go | 10 +-- npm/namespace.go | 43 ++---------- npm/npm.go | 12 ++-- npm/nwpolicy.go | 83 ++++++++++++++++++----- npm/nwpolicy_test.go | 31 +++++++++ npm/pod.go | 54 +++++++++------ npm/pod_test.go | 42 +++++++++++- 8 files changed, 187 insertions(+), 90 deletions(-) diff --git a/.pipelines/npm/npm-conformance-tests.yaml b/.pipelines/npm/npm-conformance-tests.yaml index cc7963dbda..16659c7c74 100644 --- a/.pipelines/npm/npm-conformance-tests.yaml +++ b/.pipelines/npm/npm-conformance-tests.yaml @@ -179,7 +179,9 @@ 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() - 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..95e2a777a4 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -11,16 +11,12 @@ import ( "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 +25,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 +56,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] @@ -223,8 +194,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 } 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..ceae85fceb 100644 --- a/npm/nwpolicy.go +++ b/npm/nwpolicy.go @@ -13,20 +13,63 @@ 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, "[Util] {GetNetworkPolicyKey} Error: while running MetaNamespaceKeyFunc err: %s", err) + return "" + } + return util.GetNSNameWithPrefix(netpolKey) +} + +// GetProcessedNPKey will return netpolKey +func GetProcessedNPKey(npObj *networkingv1.NetworkPolicy, hashSelector string) string { + netpolKey := npObj.GetNamespace() + "/" + hashSelector + 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 == "" { + + // TODO check this case + 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 ( @@ -49,7 +92,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP npMgr.NsMap[npNs] = ns } - if ns.policyExists(npObj) { + if npMgr.policyExists(npObj) { return nil } @@ -69,6 +112,8 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP var ( hashedSelector = HashSelector(&npObj.Spec.PodSelector) + npKey = GetNetworkPolicyKey(npObj) + npProcessedKey = GetProcessedNPKey(npObj, hashedSelector) addedPolicy *networkingv1.NetworkPolicy sets, namedPorts, lists []string ingressIPCidrs, egressIPCidrs [][]string @@ -77,14 +122,14 @@ 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) @@ -92,12 +137,12 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP } 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 { @@ -148,9 +193,12 @@ 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 + ns *Namespace + 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 @@ -177,19 +225,18 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo 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) } if deductedPolicy == nil { - delete(ns.ProcessedNpMap, hashedSelector) + delete(npMgr.ProcessedNpMap, npProcessedKey) } else { - ns.ProcessedNpMap[hashedSelector] = deductedPolicy + npMgr.ProcessedNpMap[npProcessedKey] = deductedPolicy } } diff --git a/npm/nwpolicy_test.go b/npm/nwpolicy_test.go index d46bf17194..49dd2518dd 100644 --- a/npm/nwpolicy_test.go +++ b/npm/nwpolicy_test.go @@ -368,3 +368,34 @@ 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) + + // 2 characters are / + if len(netpolKey) <= 2 { + 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..c053c7301a 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, "[Util] {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) { @@ -179,6 +191,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { var ( err error + podKey = GetPodKey(podObj) podNs = util.GetNSNameWithPrefix(npmPodObj.Namespace) podUID = npmPodObj.PodUID podName = npmPodObj.Name @@ -229,7 +242,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { } // add the Pod info to the podMap - npMgr.NsMap[podNs].PodMap[podUID] = npmPodObj + npMgr.PodMap[podKey] = npmPodObj return nil } @@ -252,6 +265,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 @@ -272,7 +286,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { } } - 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) @@ -394,7 +408,7 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { } // 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,25 +424,23 @@ 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 - } + 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 { @@ -463,9 +475,7 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { log.Errorf("Error: failed to delete pod from namespace ipset.") } - 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..f75b77d9f2 100644 --- a/npm/pod_test.go +++ b/npm/pod_test.go @@ -152,7 +152,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") } @@ -225,7 +227,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") } @@ -287,7 +291,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() @@ -472,3 +476,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) + } +} From 06cdb70978bf368388d20cfe4395b14e611881bc Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 11 Mar 2021 14:20:41 -0800 Subject: [PATCH 3/8] Adding telemetry capabilities to resource CRUD events --- npm/namespace.go | 27 ++++++++++++++------------- npm/nwpolicy.go | 22 ++++++---------------- npm/pod.go | 46 +++++++++++++++++++++++----------------------- 3 files changed, 43 insertions(+), 52 deletions(-) diff --git a/npm/namespace.go b/npm/namespace.go index 95e2a777a4..0bbcac907d 100644 --- a/npm/namespace.go +++ b/npm/namespace.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/iptm" + "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" @@ -65,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", ns, util.KubeAllNamespacesFlag) return err } } @@ -82,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", ns, util.KubeAllNamespacesFlag) return err } } @@ -100,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.", nsName) 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.", nsName) return err } @@ -115,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", nsName, labelKey) 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", nsName, label) 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", nsName) } setResourceVersion(ns, nsObj.GetObjectMeta().GetResourceVersion()) @@ -209,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", oldNsNs, labelKey) return err } } @@ -219,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", oldNsNs, labelKey) return err } } @@ -252,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", nsName, labelKey) 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", nsName, label) 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", nsName, util.KubeAllNamespacesFlag) 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.", nsName) return err } diff --git a/npm/nwpolicy.go b/npm/nwpolicy.go index ceae85fceb..fc0d56dc66 100644 --- a/npm/nwpolicy.go +++ b/npm/nwpolicy.go @@ -17,7 +17,7 @@ import ( func GetNetworkPolicyKey(npObj *networkingv1.NetworkPolicy) string { netpolKey, err := util.GetObjKeyFunc(npObj) if err != nil { - metrics.SendErrorLogAndMetric(util.NetpolID, "[Util] {GetNetworkPolicyKey} Error: while running MetaNamespaceKeyFunc err: %s", err) + metrics.SendErrorLogAndMetric(util.NetpolID, "[GetNetworkPolicyKey] Error: while running MetaNamespaceKeyFunc err: %s", err) return "" } return util.GetNSNameWithPrefix(netpolKey) @@ -98,12 +98,12 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP 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 } @@ -170,7 +170,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP 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", iptEntry) } } @@ -194,7 +194,6 @@ func (npMgr *NetworkPolicyManager) UpdateNetworkPolicy(oldNpObj *networkingv1.Ne func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.NetworkPolicy) error { var ( err error - ns *Namespace allNs = npMgr.NsMap[util.KubeAllNamespacesFlag] hashedSelector = HashSelector(&npObj.Spec.PodSelector) npKey = GetNetworkPolicyKey(npObj) @@ -204,21 +203,12 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo 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 - } - _, _, _, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(npObj) 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", iptEntry) } } @@ -243,7 +233,7 @@ 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.") + metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: failed to uninitialize azure-npm chains with err: %s.", err) return err } } diff --git a/npm/pod.go b/npm/pod.go index c053c7301a..987836745c 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -158,7 +158,7 @@ func appendNamedPortIpsets(ipsMgr *ipsm.IpsetManager, portList []v1.ContainerPor func GetPodKey(podObj *corev1.Pod) string { podKey, err := util.GetObjKeyFunc(podObj) if err != nil { - metrics.SendErrorLogAndMetric(util.PodID, "[Util] {GetPodKey} Error: while running MetaNamespaceKeyFunc err: %s", err) + metrics.SendErrorLogAndMetric(util.PodID, "[GetPodKey] Error: while running MetaNamespaceKeyFunc err: %s", err) return "" } podKey = podKey + "/" + string(podObj.GetObjectMeta().GetUID()) @@ -185,7 +185,7 @@ 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", podObj.ObjectMeta.Name, podObj) return podErr } @@ -208,7 +208,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { 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", podNs) } log.Logf("Creating set: %v, hashedSet: %v", podNs, util.GetHashedName(podNs)) if err = ipsMgr.CreateSet(podNs, append([]string{util.IpsetNetHashFlag})); err != nil { @@ -219,26 +219,26 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { // 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.") } // 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.") } 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.") } } // 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.") } // add the Pod info to the podMap @@ -278,18 +278,18 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { 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", newPodObjNs) } 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", newPodObjNs) } } 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 } @@ -315,7 +315,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 @@ -338,7 +338,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] Error: 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) @@ -357,11 +357,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.") } // 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.") } } else { //if no change in labels then return @@ -380,7 +380,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.") } } @@ -388,7 +388,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.") } } @@ -399,11 +399,11 @@ 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.") } // 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.") } } @@ -445,7 +445,7 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { // 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] Error: 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) } @@ -453,26 +453,26 @@ 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.") } // 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.") } 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.") } } // 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.") } delete(npMgr.PodMap, podKey) From c938073627d0d87f04e3deb58f0e29a63d732535 Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 11 Mar 2021 14:54:24 -0800 Subject: [PATCH 4/8] Initializing new maps in nprMgr for tests --- npm/namespace_test.go | 16 ++++++++++++++++ npm/nwpolicy_test.go | 9 +++++++++ npm/pod_test.go | 22 ++++++++++++++++++++++ 3 files changed, 47 insertions(+) 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/nwpolicy_test.go b/npm/nwpolicy_test.go index 49dd2518dd..22ca1013d5 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, } diff --git a/npm/pod_test.go b/npm/pod_test.go index f75b77d9f2..9bda8f23d9 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, } @@ -168,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, } @@ -248,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, } @@ -300,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, } @@ -351,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, } @@ -425,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, } From 7999d42e5ceb0b101a6c6c94842ed24f25cd58c9 Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 11 Mar 2021 15:04:25 -0800 Subject: [PATCH 5/8] Initializing new maps in nprMgr for tests --- npm/pod_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pod_test.go b/npm/pod_test.go index 9bda8f23d9..18fdf7e801 100644 --- a/npm/pod_test.go +++ b/npm/pod_test.go @@ -215,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", From a6f8c9c9a5fb66029d3d8ebdf17ad359bc76665e Mon Sep 17 00:00:00 2001 From: vakr Date: Fri, 12 Mar 2021 09:17:23 -0800 Subject: [PATCH 6/8] Adding artifact for Npm logs --- .pipelines/npm/npm-conformance-tests.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pipelines/npm/npm-conformance-tests.yaml b/.pipelines/npm/npm-conformance-tests.yaml index 16659c7c74..9d73bcef73 100644 --- a/.pipelines/npm/npm-conformance-tests.yaml +++ b/.pipelines/npm/npm-conformance-tests.yaml @@ -182,6 +182,7 @@ jobs: condition: always() - publish: $(System.DefaultWorkingDirectory)/npmLogs condition: always() + artifact: NpmLogs - job: Clean_up displayName: "Cleanup" From ca7f1c722e011b50b4059301b90f92b83c339e8f Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 15 Mar 2021 11:20:13 -0700 Subject: [PATCH 7/8] Addressing comments --- npm/namespace.go | 26 ++++++++--------- npm/nwpolicy.go | 76 ++++++++++++++++++++++++++++++------------------ npm/pod.go | 62 ++++++++++++++++++++++++--------------- 3 files changed, 99 insertions(+), 65 deletions(-) diff --git a/npm/namespace.go b/npm/namespace.go index 0bbcac907d..e09fac20d8 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -66,7 +66,7 @@ func (npMgr *NetworkPolicyManager) InitAllNsList() error { } if err := allNs.IpsMgr.AddToList(util.KubeAllNamespacesFlag, ns); err != nil { - metrics.SendErrorLogAndMetric(util.NSID, "[InitAllNsList] 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 } } @@ -83,7 +83,7 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error { } if err := allNs.IpsMgr.DeleteFromList(util.KubeAllNamespacesFlag, ns); err != nil { - metrics.SendErrorLogAndMetric(util.NSID, "[UninitAllNsList] 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 } } @@ -101,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 { - metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] 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 { - metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] 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 } @@ -116,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 { - metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] 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 { - metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] 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 { - metrics.SendErrorLogAndMetric(util.NSID, "[AddNamespace] 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()) @@ -210,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 { - metrics.SendErrorLogAndMetric(util.NSID, "[UpdateNamespace] 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 } } @@ -220,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 { - metrics.SendErrorLogAndMetric(util.NSID, "[UpdateNamespace] 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 } } @@ -253,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 { - metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] 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 { - metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] 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 { - metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] 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 { - metrics.SendErrorLogAndMetric(util.NSID, "[DeleteNamespace] 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/nwpolicy.go b/npm/nwpolicy.go index fc0d56dc66..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" @@ -20,12 +21,19 @@ func GetNetworkPolicyKey(npObj *networkingv1.NetworkPolicy) string { 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 { - netpolKey := npObj.GetNamespace() + "/" + hashSelector + // hashSelector will never be empty + netpolKey := hashSelector + if len(npObj.GetNamespace()) > 0 { + netpolKey = npObj.GetNamespace() + "/" + netpolKey + } return util.GetNSNameWithPrefix(netpolKey) } @@ -44,8 +52,6 @@ func (npMgr *NetworkPolicyManager) canCleanUpNpmChains() bool { func (npMgr *NetworkPolicyManager) policyExists(npObj *networkingv1.NetworkPolicy) bool { npKey := GetNetworkPolicyKey(npObj) if npKey == "" { - - // TODO check this case return false } @@ -73,21 +79,30 @@ func (npMgr *NetworkPolicyManager) policyExists(npObj *networkingv1.NetworkPolic // 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 } @@ -98,12 +113,12 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP if !npMgr.isAzureNpmChainCreated { if err = allNs.IpsMgr.CreateSet(util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil { - metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to initialize kube-system ipset with err %s.", err) + 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 { - metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to initialize azure-npm chains with err %s.", err) + metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] Error: failed to initialize azure-npm chains with err %s", err) return err } @@ -111,9 +126,6 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP } var ( - hashedSelector = HashSelector(&npObj.Spec.PodSelector) - npKey = GetNetworkPolicyKey(npObj) - npProcessedKey = GetProcessedNPKey(npObj, hashedSelector) addedPolicy *networkingv1.NetworkPolicy sets, namedPorts, lists []string ingressIPCidrs, egressIPCidrs [][]string @@ -132,7 +144,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP 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) } } @@ -148,29 +160,29 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP 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 { - metrics.SendErrorLogAndMetric(util.NetpolID, "[AddNetworkPolicy] 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) } } @@ -203,12 +215,18 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo npNs, npName := util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace), npObj.ObjectMeta.Name log.Logf("NETWORK POLICY DELETING: 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 + } + _, _, _, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(npObj) iptMgr := allNs.iptMgr for _, iptEntry := range iptEntries { if err = iptMgr.Delete(iptEntry); err != nil { - metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] 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) } } @@ -220,7 +238,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo 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 { @@ -233,7 +251,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo if npMgr.canCleanUpNpmChains() { npMgr.isAzureNpmChainCreated = false if err = iptMgr.UninitNpmChains(); err != nil { - metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: failed to uninitialize azure-npm chains with err: %s.", err) + metrics.SendErrorLogAndMetric(util.NetpolID, "[DeleteNetworkPolicy] Error: failed to uninitialize azure-npm chains with err: %s", err) return err } } @@ -252,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 @@ -261,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) } } } @@ -281,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/pod.go b/npm/pod.go index 987836745c..e3806cb02e 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -185,7 +185,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { npmPodObj, podErr := newNpmPod(podObj) if podErr != nil { - metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] 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 } @@ -204,41 +204,47 @@ 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[AddPod] 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 @@ -274,15 +280,21 @@ 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error creating ipset %s", newPodObjNs) + metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] Error creating ipset %s with err: %v", newPodObjNs, err) } } @@ -337,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 - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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) @@ -357,11 +368,11 @@ func (npMgr *NetworkPolicyManager) UpdatePod(newPodObj *corev1.Pod) error { newPodObjNs, ) if err = ipsMgr.DeleteFromSet(cachedPodObj.Namespace, cachedPodIP, cachedPodObj.PodUID); err != nil { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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 @@ -380,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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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) } } @@ -388,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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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) } } @@ -399,11 +410,11 @@ 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[UpdatePod] 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) } } @@ -432,6 +443,12 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { podUID = string(podObj.ObjectMeta.UID) ) + 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 @@ -444,8 +461,7 @@ func (npMgr *NetworkPolicyManager) DeletePod(podObj *corev1.Pod) error { // if the podIp exists, it must match the cachedIp if len(podObj.Status.PodIP) > 0 && cachedPodIP != podObj.Status.PodIP { - // TODO Add AI telemetry event - metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] 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) } @@ -453,26 +469,26 @@ 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] 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 { - metrics.SendErrorLogAndMetric(util.PodID, "[DeletePod] 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(npMgr.PodMap, podKey) From f93a3a2de7dcaacb5b395f4adba6eed4fac25156 Mon Sep 17 00:00:00 2001 From: vakr Date: Mon, 15 Mar 2021 11:21:50 -0700 Subject: [PATCH 8/8] Addressing comments --- npm/nwpolicy_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/npm/nwpolicy_test.go b/npm/nwpolicy_test.go index 22ca1013d5..87a229d663 100644 --- a/npm/nwpolicy_test.go +++ b/npm/nwpolicy_test.go @@ -398,8 +398,7 @@ func TestGetNetworkPolicyKey(t *testing.T) { netpolKey := GetNetworkPolicyKey(npObj) - // 2 characters are / - if len(netpolKey) <= 2 { + if netpolKey == "" { t.Errorf("TestGetNetworkPolicyKey failed @ netpolKey length check %s", netpolKey) }