Skip to content

Revert "[NPM] Caching Pod information to compare against desired state" #810

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 1 commit into from
Mar 2, 2021
Merged

Revert "[NPM] Caching Pod information to compare against desired state" #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
105 changes: 43 additions & 62 deletions npm/namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ import (
"github.com/Azure/azure-container-networking/npm/ipsm"
"github.com/Azure/azure-container-networking/npm/iptm"
"github.com/Azure/azure-container-networking/npm/util"
"k8s.io/apimachinery/pkg/types"

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
name string
labelsMap map[string]string
setMap map[string]string
podMap map[types.UID]*corev1.Pod
rawNpMap map[string]*networkingv1.NetworkPolicy
processedNpMap map[string]*networkingv1.NetworkPolicy
ipsMgr *ipsm.IpsetManager
iptMgr *iptm.IptablesManager
}

// newNS constructs a new namespace object.
Expand All @@ -32,24 +32,16 @@ func newNs(name string) (*namespace, error) {
name: name,
labelsMap: make(map[string]string),
setMap: make(map[string]string),
podMap: make(map[string]*npmPod),
podMap: make(map[types.UID]*corev1.Pod),
rawNpMap: make(map[string]*networkingv1.NetworkPolicy),
processedNpMap: make(map[string]*networkingv1.NetworkPolicy),
ipsMgr: ipsm.NewIpsetManager(),
iptMgr: iptm.NewIptablesManager(),
// resource version is converted to uint64
// so make sure it is initialized to "0"
resourceVersion: 0,
}

return ns, nil
}

// setResourceVersion setter func for RV
func setResourceVersion(nsObj *namespace, rv string) {
nsObj.resourceVersion = util.ParseResourceVersion(rv)
}

func isSystemNs(nsObj *corev1.Namespace) bool {
return nsObj.ObjectMeta.Name == util.KubeSystemFlag
}
Expand All @@ -64,22 +56,10 @@ func isInvalidNamespaceUpdate(oldNsObj, newNsObj *corev1.Namespace) (isInvalidUp
}

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
if np, exists := ns.rawNpMap[npObj.ObjectMeta.Name]; exists {
if isSamePolicy(np, npObj) {
return true
}
}

return false
Expand Down Expand Up @@ -123,7 +103,7 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error {
func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
var err error

nsName, nsLabel := util.GetNSNameWithPrefix(nsObj.ObjectMeta.Name), nsObj.ObjectMeta.Labels
nsName, nsLabel := "ns-"+nsObj.ObjectMeta.Name, nsObj.ObjectMeta.Labels
log.Logf("NAMESPACE CREATING: [%s/%v]", nsName, nsLabel)

ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
Expand All @@ -141,14 +121,14 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
// Add the namespace to its label's ipset list.
nsLabels := nsObj.ObjectMeta.Labels
for nsLabelKey, nsLabelVal := range nsLabels {
labelKey := util.GetNSNameWithPrefix(nsLabelKey)
labelKey := "ns-" + 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)
return err
}

label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal)
label := "ns-" + 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)
Expand All @@ -160,7 +140,6 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
if err != nil {
log.Errorf("Error: failed to create namespace %s", nsName)
}
setResourceVersion(ns, nsObj.GetObjectMeta().GetResourceVersion())

// Append all labels to the cache NS obj
ns.labelsMap = util.AppendMap(ns.labelsMap, nsLabel)
Expand All @@ -176,8 +155,8 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n
}

var err error
oldNsNs, oldNsLabel := util.GetNSNameWithPrefix(oldNsObj.ObjectMeta.Name), oldNsObj.ObjectMeta.Labels
newNsNs, newNsLabel := util.GetNSNameWithPrefix(newNsObj.ObjectMeta.Name), newNsObj.ObjectMeta.Labels
oldNsNs, oldNsLabel := "ns-"+oldNsObj.ObjectMeta.Name, oldNsObj.ObjectMeta.Labels
newNsNs, newNsLabel := "ns-"+newNsObj.ObjectMeta.Name, newNsObj.ObjectMeta.Labels
log.Logf(
"NAMESPACE UPDATING:\n old namespace: [%s/%v]\n new namespace: [%s/%v]",
oldNsNs, oldNsLabel, newNsNs, newNsLabel,
Expand Down Expand Up @@ -210,16 +189,6 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n
return nil
}

newRv := util.ParseResourceVersion(newNsObj.ObjectMeta.ResourceVersion)
if !util.CompareUintResourceVersions(curNsObj.resourceVersion, newRv) {
log.Logf("Cached NameSpace has larger ResourceVersion number than new Obj. NameSpace: %s Cached RV: %d New RV:\n",
oldNsNs,
curNsObj.resourceVersion,
newRv,
)
return nil
}

//if no change in labels then return
if reflect.DeepEqual(curNsObj.labelsMap, newNsLabel) {
log.Logf(
Expand All @@ -230,32 +199,45 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n
}

//If the Namespace is not deleted, delete removed labels and create new labels
addToIPSets, deleteFromIPSets := util.GetIPSetListCompareLabels(curNsObj.labelsMap, newNsLabel)
toAddNsLabels, toDeleteNsLabels := util.CompareMapDiff(curNsObj.labelsMap, newNsLabel)

// Delete the namespace from its label's ipset list.
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
for _, nsLabelVal := range deleteFromIPSets {
labelKey := util.GetNSNameWithPrefix(nsLabelVal)
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
}
}

// Add the namespace to its label's ipset list.
for _, nsLabelVal := range addToIPSets {
labelKey := util.GetNSNameWithPrefix(nsLabelVal)
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)
setResourceVersion(curNsObj, newNsObj.GetObjectMeta().GetResourceVersion())
npMgr.nsMap[newNsNs] = curNsObj

return nil
Expand All @@ -265,27 +247,26 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n
func (npMgr *NetworkPolicyManager) DeleteNamespace(nsObj *corev1.Namespace) error {
var err error

nsName, nsLabel := util.GetNSNameWithPrefix(nsObj.ObjectMeta.Name), nsObj.ObjectMeta.Labels
nsName, nsLabel := "ns-"+nsObj.ObjectMeta.Name, nsObj.ObjectMeta.Labels
log.Logf("NAMESPACE DELETING: [%s/%v]", nsName, nsLabel)

cachedNsObj, exists := npMgr.nsMap[nsName]
_, exists := npMgr.nsMap[nsName]
if !exists {
return nil
}

log.Logf("NAMESPACE DELETING cached labels: [%s/%v]", nsName, cachedNsObj.labelsMap)
// Delete the namespace from its label's ipset list.
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
nsLabels := cachedNsObj.labelsMap
nsLabels := nsObj.ObjectMeta.Labels
for nsLabelKey, nsLabelVal := range nsLabels {
labelKey := util.GetNSNameWithPrefix(nsLabelKey)
labelKey := "ns-" + 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)
return err
}

label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal)
label := "ns-" + 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)
Expand Down
7 changes: 1 addition & 6 deletions npm/namespace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestNewNs(t *testing.T) {
func TestnewNs(t *testing.T) {
if _, err := newNs("test"); err != nil {
t.Errorf("TestnewNs failed @ newNs")
}
Expand Down Expand Up @@ -165,7 +165,6 @@ func TestAddNamespaceLabel(t *testing.T) {
Labels: map[string]string{
"app": "old-test-namespace",
},
ResourceVersion: "0",
},
}

Expand All @@ -176,8 +175,6 @@ func TestAddNamespaceLabel(t *testing.T) {
"app": "old-test-namespace",
"update": "true",
},

ResourceVersion: "1",
},
}

Expand Down Expand Up @@ -228,7 +225,6 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) {
"update": "true",
"group": "test",
},
ResourceVersion: "0",
},
}

Expand All @@ -239,7 +235,6 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) {
"app": "old-test-namespace",
"update": "false",
},
ResourceVersion: "1",
},
}

Expand Down
20 changes: 14 additions & 6 deletions npm/npm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type NetworkPolicyManager struct {

nodeName string
nsMap map[string]*namespace
podMap map[string]string // Key: Pod uuid, Value: PodIp
isAzureNpmChainCreated bool
isSafeToCleanUpAzureNpmChain bool

Expand Down Expand Up @@ -117,13 +118,14 @@ func (npMgr *NetworkPolicyManager) SendClusterMetrics() {
for {
<-heartbeat
npMgr.Lock()
podCount.Value = 0
podCount.Value = float64(len(npMgr.podMap))
//Reducing one to remove all-namespaces ns obj
nsCount.Value = float64(len(npMgr.nsMap) - 1)
nwPolCount := 0
for _, ns := range npMgr.nsMap {
nwPolicyCount.Value += float64(len(ns.rawNpMap))
podCount.Value += float64(len(ns.podMap))
nwPolCount = nwPolCount + len(ns.rawNpMap)
}
nwPolicyCount.Value = float64(nwPolCount)
npMgr.Unlock()

metrics.SendMetric(podCount)
Expand Down Expand Up @@ -232,6 +234,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
npInformer: npInformer,
nodeName: os.Getenv("HOSTNAME"),
nsMap: make(map[string]*namespace),
podMap: make(map[string]string),
isAzureNpmChainCreated: false,
isSafeToCleanUpAzureNpmChain: false,
clusterState: telemetry.ClusterState{
Expand All @@ -248,7 +251,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs

// Create ipset for the namespace.
kubeSystemNs := util.GetNSNameWithPrefix(util.KubeSystemFlag)
kubeSystemNs := "ns-" + util.KubeSystemFlag
if err := allNs.ipsMgr.CreateSet(kubeSystemNs, append([]string{util.IpsetNetHashFlag})); err != nil {
metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to create ipset for namespace %s.", kubeSystemNs)
}
Expand All @@ -266,14 +269,19 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
npMgr.AddPod(podObj)
npMgr.Unlock()
},
UpdateFunc: func(_, new interface{}) {
UpdateFunc: func(old, new interface{}) {
oldPodObj, ok := old.(*corev1.Pod)
if !ok {
metrics.SendErrorLogAndMetric(util.NpmID, "UPDATE Pod: Received unexpected old object type: %v", oldPodObj)
return
}
newPodObj, ok := new.(*corev1.Pod)
if !ok {
metrics.SendErrorLogAndMetric(util.NpmID, "UPDATE Pod: Received unexpected new object type: %v", newPodObj)
return
}
npMgr.Lock()
npMgr.UpdatePod(newPodObj)
npMgr.UpdatePod(oldPodObj, newPodObj)
npMgr.Unlock()
},
DeleteFunc: func(obj interface{}) {
Expand Down
4 changes: 2 additions & 2 deletions npm/nwpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
err error
ns *namespace
exists bool
npNs = util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace)
npNs = "ns-" + npObj.ObjectMeta.Namespace
npName = npObj.ObjectMeta.Name
allNs = npMgr.nsMap[util.KubeAllNamespacesFlag]
timer = metrics.StartNewTimer()
Expand Down Expand Up @@ -153,7 +153,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
allNs = npMgr.nsMap[util.KubeAllNamespacesFlag]
)

npNs, npName := util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace), npObj.ObjectMeta.Name
npNs, npName := "ns-"+npObj.ObjectMeta.Namespace, npObj.ObjectMeta.Name
log.Logf("NETWORK POLICY DELETING: Namespace: %s, Name:%s", npNs, npName)

var exists bool
Expand Down
Loading