Skip to content

[NPM] Supporting Namespace label updates #773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 5, 2021
Merged

[NPM] Supporting Namespace label updates #773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d0e013
Supporting Namespace label updates
vakalapa Jan 29, 2021
e3b5fbb
first pass at saving NS labels in nsmap
vakalapa Feb 2, 2021
606a5e7
Strengthing testcases
vakalapa Feb 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 77 additions & 4 deletions npm/namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand Down Expand Up @@ -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
Expand All @@ -157,16 +162,84 @@ 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 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(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,
)
return nil
}

//If the Namespace is not deleted, delete removed labels and create new labels
toAddNsLabels, toDeleteNsLabels := util.CompareMapDiff(curNsObj.labelsMap, 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
}
}

// Append all labels to the cache NS obj
curNsObj.labelsMap = util.ClearAndAppendMap(curNsObj.labelsMap, newNsLabel)
npMgr.nsMap[newNsNs] = curNsObj

return nil
}

Expand Down
122 changes: 122 additions & 0 deletions npm/namespace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package npm

import (
"os"
"reflect"
"testing"

"github.com/Azure/azure-container-networking/npm/iptm"
Expand Down Expand Up @@ -135,6 +136,123 @@ 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 {
t.Fatal(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")
Copy link
Contributor

@csfmomo csfmomo Jan 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good to me. For these tests, is there anyway that we check the new namespace label has been added or delete besides no error happens?

Copy link
Contributor Author

@vakalapa vakalapa Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this.

}

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()
}

func TestDeleteandUpdateNamespaceLabel(t *testing.T) {
npMgr := &NetworkPolicyManager{
nsMap: make(map[string]*namespace),
TelemetryEnabled: false,
}

allNs, err := newNs(util.KubeAllNamespacesFlag)
if err != nil {
t.Fatal(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")
}

if !reflect.DeepEqual(npMgr.nsMap["ns-"+newNsObj.Name].labelsMap, newNsObj.ObjectMeta.Labels) {
t.Errorf("TestDeleteandUpdateNamespaceLabel failed @ npMgr.nsMap labelMap check")
}
npMgr.Unlock()
}

func TestDeleteNamespace(t *testing.T) {
npMgr := &NetworkPolicyManager{
nsMap: make(map[string]*namespace),
Expand Down Expand Up @@ -175,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()
}

Expand Down
44 changes: 40 additions & 4 deletions npm/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"hash/fnv"
"os"
"regexp"
"strings"
"sort"
"strings"

"github.com/Masterminds/semver"
"k8s.io/apimachinery/pkg/version"
Expand Down Expand Up @@ -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{}
Expand All @@ -84,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 {
Expand All @@ -106,15 +142,15 @@ 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
}
v2Minor := re.FindAllString(secondVer.Minor, -1)
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
}
Expand Down Expand Up @@ -201,4 +237,4 @@ func DropEmptyFields(s []string) []string {
}

return s
}
}