From 1d0e013490f07171b8793b1869e1727b56e3b8df Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 28 Jan 2021 17:15:36 -0800 Subject: [PATCH 1/3] Supporting Namespace label updates --- npm/namespace.go | 59 +++++++++++++++++++++-- npm/namespace_test.go | 108 ++++++++++++++++++++++++++++++++++++++++++ npm/util/util.go | 34 +++++++++++-- 3 files changed, 193 insertions(+), 8 deletions(-) diff --git a/npm/namespace.go b/npm/namespace.go index d95f1530db..54761aeaf5 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -157,12 +157,63 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n oldNsNs, oldNsLabel, newNsNs, newNsLabel, ) - if err = npMgr.DeleteNamespace(oldNsObj); err != nil { - return err + if oldNsNs != newNsNs { + if err = npMgr.DeleteNamespace(oldNsObj); err != nil { + return err + } + + if newNsObj.ObjectMeta.DeletionTimestamp == nil && newNsObj.ObjectMeta.DeletionGracePeriodSeconds == nil { + if err = npMgr.AddNamespace(newNsObj); err != nil { + return err + } + } + + return nil + } + + //if no change in labels then return + if reflect.DeepEqual(oldNsLabel, newNsLabel) { + log.Logf( + "NAMESPACE UPDATING:\n nothing to delete or add. old namespace: [%s/%v]\n new namespace: [%s/%v]", + oldNsNs, oldNsLabel, newNsNs, newNsLabel, + ) + return nil + } + + //If the Namespace is not deleted, delete removed labels and create new labels + toAddNsLabels, toDeleteNsLabels := util.CompareMapDiff(oldNsLabel, newNsLabel) + + // Delete the namespace from its label's ipset list. + ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr + for nsLabelKey, nsLabelVal := range toDeleteNsLabels { + labelKey := "ns-" + nsLabelKey + 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) + return err + } + + label := "ns-" + nsLabelKey + ":" + nsLabelVal + log.Logf("Deleting namespace %s from ipset list %s", oldNsNs, label) + if err = ipsMgr.DeleteFromList(label, oldNsNs); err != nil { + log.Errorf("Error: failed to delete namespace %s from ipset list %s", oldNsNs, label) + return err + } } - if newNsObj.ObjectMeta.DeletionTimestamp == nil && newNsObj.ObjectMeta.DeletionGracePeriodSeconds == nil { - if err = npMgr.AddNamespace(newNsObj); err != nil { + // Add the namespace to its label's ipset list. + for nsLabelKey, nsLabelVal := range toAddNsLabels { + labelKey := "ns-" + nsLabelKey + 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) + return err + } + + label := "ns-" + nsLabelKey + ":" + nsLabelVal + log.Logf("Adding namespace %s to ipset list %s", oldNsNs, label) + if err = ipsMgr.AddToList(label, oldNsNs); err != nil { + log.Errorf("Error: failed to add namespace %s to ipset list %s", oldNsNs, label) return err } } diff --git a/npm/namespace_test.go b/npm/namespace_test.go index 4b947f7768..4f55e09be9 100644 --- a/npm/namespace_test.go +++ b/npm/namespace_test.go @@ -135,6 +135,114 @@ func TestUpdateNamespace(t *testing.T) { npMgr.Unlock() } +func TestAddNamespaceLabel(t *testing.T) { + npMgr := &NetworkPolicyManager{ + nsMap: make(map[string]*namespace), + TelemetryEnabled: false, + } + + allNs, err := newNs(util.KubeAllNamespacesFlag) + if err != nil { + panic(err.Error) + } + npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs + + ipsMgr := ipsm.NewIpsetManager() + if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestAddNamespaceLabel failed @ ipsMgr.Save") + } + + defer func() { + if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestAddNamespaceLabel failed @ ipsMgr.Restore") + } + }() + + oldNsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-test-namespace", + Labels: map[string]string{ + "app": "old-test-namespace", + }, + }, + } + + newNsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-test-namespace", + Labels: map[string]string{ + "app": "old-test-namespace", + "update": "true", + }, + }, + } + + npMgr.Lock() + if err := npMgr.AddNamespace(oldNsObj); err != nil { + t.Errorf("TestAddNamespaceLabel failed @ npMgr.AddNamespace") + } + + if err := npMgr.UpdateNamespace(oldNsObj, newNsObj); err != nil { + t.Errorf("TestAddNamespaceLabel failed @ npMgr.UpdateNamespace") + } + npMgr.Unlock() +} + +func TestDeleteandUpdateNamespaceLabel(t *testing.T) { + npMgr := &NetworkPolicyManager{ + nsMap: make(map[string]*namespace), + TelemetryEnabled: false, + } + + allNs, err := newNs(util.KubeAllNamespacesFlag) + if err != nil { + panic(err.Error) + } + npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs + + ipsMgr := ipsm.NewIpsetManager() + if err := ipsMgr.Save(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ ipsMgr.Save") + } + + defer func() { + if err := ipsMgr.Restore(util.IpsetTestConfigFile); err != nil { + t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ ipsMgr.Restore") + } + }() + + oldNsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-test-namespace", + Labels: map[string]string{ + "app": "old-test-namespace", + "update": "true", + "group": "test", + }, + }, + } + + newNsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-test-namespace", + Labels: map[string]string{ + "app": "old-test-namespace", + "update": "false", + }, + }, + } + + npMgr.Lock() + if err := npMgr.AddNamespace(oldNsObj); err != nil { + t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ npMgr.AddNamespace") + } + + if err := npMgr.UpdateNamespace(oldNsObj, newNsObj); err != nil { + t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ npMgr.UpdateNamespace") + } + npMgr.Unlock() +} + func TestDeleteNamespace(t *testing.T) { npMgr := &NetworkPolicyManager{ nsMap: make(map[string]*namespace), diff --git a/npm/util/util.go b/npm/util/util.go index eb77b951ae..05c1e0cef2 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -7,8 +7,8 @@ import ( "hash/fnv" "os" "regexp" - "strings" "sort" + "strings" "github.com/Masterminds/semver" "k8s.io/apimachinery/pkg/version" @@ -69,6 +69,32 @@ func SortMap(m *map[string]string) ([]string, []string) { return sortedKeys, sortedVals } +// CompareMapDiff will compare two maps string[string] and returns +// missing values in both +func CompareMapDiff(orig map[string]string, new map[string]string) (map[string]string, map[string]string) { + notInOrig := make(map[string]string) + notInNew := make(map[string]string) + + for keyOrig, valOrig := range orig { + if valNew, ok := new[keyOrig]; ok { + if valNew != valOrig { + notInNew[keyOrig] = valOrig + notInOrig[keyOrig] = valNew + } + } else { + notInNew[keyOrig] = valOrig + } + } + + for keyNew, valNew := range new { + if _, ok := orig[keyNew]; !ok { + notInOrig[keyNew] = valNew + } + } + + return notInOrig, notInNew +} + // UniqueStrSlice removes duplicate elements from the input string. func UniqueStrSlice(s []string) []string { m, unique := map[string]bool{}, []string{} @@ -106,7 +132,7 @@ func CompareK8sVer(firstVer *version.Info, secondVer *version.Info) int { if len(v1Minor) < 1 { return -2 } - v1, err := semver.NewVersion(firstVer.Major+"."+v1Minor[0]) + v1, err := semver.NewVersion(firstVer.Major + "." + v1Minor[0]) if err != nil { return -2 } @@ -114,7 +140,7 @@ func CompareK8sVer(firstVer *version.Info, secondVer *version.Info) int { if len(v2Minor) < 1 { return -2 } - v2, err := semver.NewVersion(secondVer.Major+"."+v2Minor[0]) + v2, err := semver.NewVersion(secondVer.Major + "." + v2Minor[0]) if err != nil { return -2 } @@ -201,4 +227,4 @@ func DropEmptyFields(s []string) []string { } return s -} \ No newline at end of file +} From e3b5fbbcfa8c7bd8c8193ab064bc7e97855e865f Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 2 Feb 2021 14:05:52 -0800 Subject: [PATCH 2/3] first pass at saving NS labels in nsmap --- npm/namespace.go | 30 ++++++++++++++++++++++++++---- npm/util/util.go | 10 ++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/npm/namespace.go b/npm/namespace.go index 54761aeaf5..de1a4c58a5 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -17,6 +17,7 @@ import ( type namespace struct { name string + labelsMap map[string]string setMap map[string]string podMap map[types.UID]*corev1.Pod rawNpMap map[string]*networkingv1.NetworkPolicy @@ -29,6 +30,7 @@ type namespace struct { func newNs(name string) (*namespace, error) { ns := &namespace{ name: name, + labelsMap: make(map[string]string), setMap: make(map[string]string), podMap: make(map[types.UID]*corev1.Pod), rawNpMap: make(map[string]*networkingv1.NetworkPolicy), @@ -138,6 +140,9 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { if err != nil { log.Errorf("Error: failed to create namespace %s", nsName) } + + // Append all labels to the cache NS obj + ns.labelsMap = util.AppendMap(ns.labelsMap, nsLabel) npMgr.nsMap[nsName] = ns return nil @@ -171,17 +176,30 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n return nil } + // If orignal AddNamespace failed for some reason, then NS will not be found + // in nsMap, resulting in retry of ADD. + curNsObj, exists := npMgr.nsMap[newNsNs] + if !exists { + if newNsObj.ObjectMeta.DeletionTimestamp == nil && newNsObj.ObjectMeta.DeletionGracePeriodSeconds == nil { + if err = npMgr.AddNamespace(newNsObj); err != nil { + return err + } + } + + return nil + } + //if no change in labels then return - if reflect.DeepEqual(oldNsLabel, newNsLabel) { + if reflect.DeepEqual(curNsObj.labelsMap, newNsLabel) { log.Logf( - "NAMESPACE UPDATING:\n nothing to delete or add. old namespace: [%s/%v]\n new namespace: [%s/%v]", - oldNsNs, oldNsLabel, newNsNs, newNsLabel, + "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, ) return nil } //If the Namespace is not deleted, delete removed labels and create new labels - toAddNsLabels, toDeleteNsLabels := util.CompareMapDiff(oldNsLabel, newNsLabel) + toAddNsLabels, toDeleteNsLabels := util.CompareMapDiff(curNsObj.labelsMap, newNsLabel) // Delete the namespace from its label's ipset list. ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr @@ -218,6 +236,10 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n } } + // Append all labels to the cache NS obj + curNsObj.labelsMap = util.ClearAndAppendMap(curNsObj.labelsMap, newNsLabel) + npMgr.nsMap[newNsNs] = curNsObj + return nil } diff --git a/npm/util/util.go b/npm/util/util.go index 05c1e0cef2..b23b2e9ddd 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -110,6 +110,16 @@ func UniqueStrSlice(s []string) []string { return unique } +// ClearAndAppendMap clears base and appends new to base. +func ClearAndAppendMap(base, new map[string]string) map[string]string { + base = make(map[string]string) + for k, v := range new { + base[k] = v + } + + return base +} + // AppendMap appends new to base. func AppendMap(base, new map[string]string) map[string]string { for k, v := range new { From 606a5e71da7d426bf44dbc1bccec4f75225111de Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 2 Feb 2021 16:58:35 -0800 Subject: [PATCH 3/3] Strengthing testcases --- npm/namespace_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/npm/namespace_test.go b/npm/namespace_test.go index 4f55e09be9..7f1719369a 100644 --- a/npm/namespace_test.go +++ b/npm/namespace_test.go @@ -4,6 +4,7 @@ package npm import ( "os" + "reflect" "testing" "github.com/Azure/azure-container-networking/npm/iptm" @@ -143,7 +144,7 @@ func TestAddNamespaceLabel(t *testing.T) { allNs, err := newNs(util.KubeAllNamespacesFlag) if err != nil { - panic(err.Error) + t.Fatal(err.Error()) } npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs @@ -185,6 +186,11 @@ func TestAddNamespaceLabel(t *testing.T) { if err := npMgr.UpdateNamespace(oldNsObj, newNsObj); err != nil { t.Errorf("TestAddNamespaceLabel failed @ npMgr.UpdateNamespace") } + + if !reflect.DeepEqual(npMgr.nsMap["ns-"+newNsObj.Name].labelsMap, newNsObj.ObjectMeta.Labels) { + t.Errorf("TestAddNamespaceLabel failed @ npMgr.nsMap labelMap check") + } + npMgr.Unlock() } @@ -196,7 +202,7 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) { allNs, err := newNs(util.KubeAllNamespacesFlag) if err != nil { - panic(err.Error) + t.Fatal(err.Error()) } npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs @@ -240,6 +246,10 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) { if err := npMgr.UpdateNamespace(oldNsObj, newNsObj); err != nil { t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ npMgr.UpdateNamespace") } + + if !reflect.DeepEqual(npMgr.nsMap["ns-"+newNsObj.Name].labelsMap, newNsObj.ObjectMeta.Labels) { + t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ npMgr.nsMap labelMap check") + } npMgr.Unlock() } @@ -283,6 +293,10 @@ func TestDeleteNamespace(t *testing.T) { if err := npMgr.DeleteNamespace(nsObj); err != nil { t.Errorf("TestDeleteNamespace @ npMgr.DeleteNamespace") } + + if _, exists := npMgr.nsMap["ns-"+nsObj.Name]; exists { + t.Errorf("TestDeleteNamespace failed @ npMgr.nsMap check") + } npMgr.Unlock() }