Skip to content

Commit 1e26d07

Browse files
vakalapavakalapa
authored
Revert "[NPM] Caching Pod information to compare against desired state (#780)" (#810)
This reverts commit b93e4cc.
1 parent cab6615 commit 1e26d07

File tree

9 files changed

+198
-609
lines changed

9 files changed

+198
-609
lines changed

‎npm/namespace.go‎

Lines changed: 43 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,21 @@ import (
99
"github.com/Azure/azure-container-networking/npm/ipsm"
1010
"github.com/Azure/azure-container-networking/npm/iptm"
1111
"github.com/Azure/azure-container-networking/npm/util"
12+
"k8s.io/apimachinery/pkg/types"
1213

1314
corev1 "k8s.io/api/core/v1"
1415
networkingv1 "k8s.io/api/networking/v1"
1516
)
1617

1718
type namespace struct {
18-
name string
19-
labelsMap map[string]string // NameSpace labels
20-
setMap map[string]string
21-
podMap map[string]*npmPod // Key is PodUID
22-
rawNpMap map[string]*networkingv1.NetworkPolicy
23-
processedNpMap map[string]*networkingv1.NetworkPolicy
24-
ipsMgr *ipsm.IpsetManager
25-
iptMgr *iptm.IptablesManager
26-
resourceVersion uint64 // NameSpace ResourceVersion
19+
name string
20+
labelsMap map[string]string
21+
setMap map[string]string
22+
podMap map[types.UID]*corev1.Pod
23+
rawNpMap map[string]*networkingv1.NetworkPolicy
24+
processedNpMap map[string]*networkingv1.NetworkPolicy
25+
ipsMgr *ipsm.IpsetManager
26+
iptMgr *iptm.IptablesManager
2727
}
2828

2929
// newNS constructs a new namespace object.
@@ -32,24 +32,16 @@ func newNs(name string) (*namespace, error) {
3232
name: name,
3333
labelsMap: make(map[string]string),
3434
setMap: make(map[string]string),
35-
podMap: make(map[string]*npmPod),
35+
podMap: make(map[types.UID]*corev1.Pod),
3636
rawNpMap: make(map[string]*networkingv1.NetworkPolicy),
3737
processedNpMap: make(map[string]*networkingv1.NetworkPolicy),
3838
ipsMgr: ipsm.NewIpsetManager(),
3939
iptMgr: iptm.NewIptablesManager(),
40-
// resource version is converted to uint64
41-
// so make sure it is initialized to "0"
42-
resourceVersion: 0,
4340
}
4441

4542
return ns, nil
4643
}
4744

48-
// setResourceVersion setter func for RV
49-
func setResourceVersion(nsObj *namespace, rv string) {
50-
nsObj.resourceVersion = util.ParseResourceVersion(rv)
51-
}
52-
5345
func isSystemNs(nsObj *corev1.Namespace) bool {
5446
return nsObj.ObjectMeta.Name == util.KubeSystemFlag
5547
}
@@ -64,22 +56,10 @@ func isInvalidNamespaceUpdate(oldNsObj, newNsObj *corev1.Namespace) (isInvalidUp
6456
}
6557

6658
func (ns *namespace) policyExists(npObj *networkingv1.NetworkPolicy) bool {
67-
np, exists := ns.rawNpMap[npObj.ObjectMeta.Name]
68-
if !exists {
69-
return false
70-
}
71-
72-
if !util.CompareResourceVersions(np.ObjectMeta.ResourceVersion, npObj.ObjectMeta.ResourceVersion) {
73-
log.Logf("Cached Network Policy has larger ResourceVersion number than new Obj. Name: %s Cached RV: %d New RV: %d\n",
74-
npObj.ObjectMeta.Name,
75-
np.ObjectMeta.ResourceVersion,
76-
npObj.ObjectMeta.ResourceVersion,
77-
)
78-
return true
79-
}
80-
81-
if isSamePolicy(np, npObj) {
82-
return true
59+
if np, exists := ns.rawNpMap[npObj.ObjectMeta.Name]; exists {
60+
if isSamePolicy(np, npObj) {
61+
return true
62+
}
8363
}
8464

8565
return false
@@ -123,7 +103,7 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error {
123103
func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
124104
var err error
125105

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

129109
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
@@ -141,14 +121,14 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
141121
// Add the namespace to its label's ipset list.
142122
nsLabels := nsObj.ObjectMeta.Labels
143123
for nsLabelKey, nsLabelVal := range nsLabels {
144-
labelKey := util.GetNSNameWithPrefix(nsLabelKey)
124+
labelKey := "ns-" + nsLabelKey
145125
log.Logf("Adding namespace %s to ipset list %s", nsName, labelKey)
146126
if err = ipsMgr.AddToList(labelKey, nsName); err != nil {
147127
log.Errorf("Error: failed to add namespace %s to ipset list %s", nsName, labelKey)
148128
return err
149129
}
150130

151-
label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal)
131+
label := "ns-" + nsLabelKey + ":" + nsLabelVal
152132
log.Logf("Adding namespace %s to ipset list %s", nsName, label)
153133
if err = ipsMgr.AddToList(label, nsName); err != nil {
154134
log.Errorf("Error: failed to add namespace %s to ipset list %s", nsName, label)
@@ -160,7 +140,6 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error {
160140
if err != nil {
161141
log.Errorf("Error: failed to create namespace %s", nsName)
162142
}
163-
setResourceVersion(ns, nsObj.GetObjectMeta().GetResourceVersion())
164143

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

178157
var err error
179-
oldNsNs, oldNsLabel := util.GetNSNameWithPrefix(oldNsObj.ObjectMeta.Name), oldNsObj.ObjectMeta.Labels
180-
newNsNs, newNsLabel := util.GetNSNameWithPrefix(newNsObj.ObjectMeta.Name), newNsObj.ObjectMeta.Labels
158+
oldNsNs, oldNsLabel := "ns-"+oldNsObj.ObjectMeta.Name, oldNsObj.ObjectMeta.Labels
159+
newNsNs, newNsLabel := "ns-"+newNsObj.ObjectMeta.Name, newNsObj.ObjectMeta.Labels
181160
log.Logf(
182161
"NAMESPACE UPDATING:\n old namespace: [%s/%v]\n new namespace: [%s/%v]",
183162
oldNsNs, oldNsLabel, newNsNs, newNsLabel,
@@ -210,16 +189,6 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n
210189
return nil
211190
}
212191

213-
newRv := util.ParseResourceVersion(newNsObj.ObjectMeta.ResourceVersion)
214-
if !util.CompareUintResourceVersions(curNsObj.resourceVersion, newRv) {
215-
log.Logf("Cached NameSpace has larger ResourceVersion number than new Obj. NameSpace: %s Cached RV: %d New RV:\n",
216-
oldNsNs,
217-
curNsObj.resourceVersion,
218-
newRv,
219-
)
220-
return nil
221-
}
222-
223192
//if no change in labels then return
224193
if reflect.DeepEqual(curNsObj.labelsMap, newNsLabel) {
225194
log.Logf(
@@ -230,32 +199,45 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n
230199
}
231200

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

235204
// Delete the namespace from its label's ipset list.
236205
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
237-
for _, nsLabelVal := range deleteFromIPSets {
238-
labelKey := util.GetNSNameWithPrefix(nsLabelVal)
206+
for nsLabelKey, nsLabelVal := range toDeleteNsLabels {
207+
labelKey := "ns-" + nsLabelKey
239208
log.Logf("Deleting namespace %s from ipset list %s", oldNsNs, labelKey)
240209
if err = ipsMgr.DeleteFromList(labelKey, oldNsNs); err != nil {
241210
log.Errorf("Error: failed to delete namespace %s from ipset list %s", oldNsNs, labelKey)
242211
return err
243212
}
213+
214+
label := "ns-" + nsLabelKey + ":" + nsLabelVal
215+
log.Logf("Deleting namespace %s from ipset list %s", oldNsNs, label)
216+
if err = ipsMgr.DeleteFromList(label, oldNsNs); err != nil {
217+
log.Errorf("Error: failed to delete namespace %s from ipset list %s", oldNsNs, label)
218+
return err
219+
}
244220
}
245221

246222
// Add the namespace to its label's ipset list.
247-
for _, nsLabelVal := range addToIPSets {
248-
labelKey := util.GetNSNameWithPrefix(nsLabelVal)
223+
for nsLabelKey, nsLabelVal := range toAddNsLabels {
224+
labelKey := "ns-" + nsLabelKey
249225
log.Logf("Adding namespace %s to ipset list %s", oldNsNs, labelKey)
250226
if err = ipsMgr.AddToList(labelKey, oldNsNs); err != nil {
251227
log.Errorf("Error: failed to add namespace %s to ipset list %s", oldNsNs, labelKey)
252228
return err
253229
}
230+
231+
label := "ns-" + nsLabelKey + ":" + nsLabelVal
232+
log.Logf("Adding namespace %s to ipset list %s", oldNsNs, label)
233+
if err = ipsMgr.AddToList(label, oldNsNs); err != nil {
234+
log.Errorf("Error: failed to add namespace %s to ipset list %s", oldNsNs, label)
235+
return err
236+
}
254237
}
255238

256239
// Append all labels to the cache NS obj
257240
curNsObj.labelsMap = util.ClearAndAppendMap(curNsObj.labelsMap, newNsLabel)
258-
setResourceVersion(curNsObj, newNsObj.GetObjectMeta().GetResourceVersion())
259241
npMgr.nsMap[newNsNs] = curNsObj
260242

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

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

271-
cachedNsObj, exists := npMgr.nsMap[nsName]
253+
_, exists := npMgr.nsMap[nsName]
272254
if !exists {
273255
return nil
274256
}
275257

276-
log.Logf("NAMESPACE DELETING cached labels: [%s/%v]", nsName, cachedNsObj.labelsMap)
277258
// Delete the namespace from its label's ipset list.
278259
ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr
279-
nsLabels := cachedNsObj.labelsMap
260+
nsLabels := nsObj.ObjectMeta.Labels
280261
for nsLabelKey, nsLabelVal := range nsLabels {
281-
labelKey := util.GetNSNameWithPrefix(nsLabelKey)
262+
labelKey := "ns-" + nsLabelKey
282263
log.Logf("Deleting namespace %s from ipset list %s", nsName, labelKey)
283264
if err = ipsMgr.DeleteFromList(labelKey, nsName); err != nil {
284265
log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, labelKey)
285266
return err
286267
}
287268

288-
label := util.GetNSNameWithPrefix(nsLabelKey + ":" + nsLabelVal)
269+
label := "ns-" + nsLabelKey + ":" + nsLabelVal
289270
log.Logf("Deleting namespace %s from ipset list %s", nsName, label)
290271
if err = ipsMgr.DeleteFromList(label, nsName); err != nil {
291272
log.Errorf("Error: failed to delete namespace %s from ipset list %s", nsName, label)

‎npm/namespace_test.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1717
)
1818

19-
func TestNewNs(t *testing.T) {
19+
func TestnewNs(t *testing.T) {
2020
if _, err := newNs("test"); err != nil {
2121
t.Errorf("TestnewNs failed @ newNs")
2222
}
@@ -165,7 +165,6 @@ func TestAddNamespaceLabel(t *testing.T) {
165165
Labels: map[string]string{
166166
"app": "old-test-namespace",
167167
},
168-
ResourceVersion: "0",
169168
},
170169
}
171170

@@ -176,8 +175,6 @@ func TestAddNamespaceLabel(t *testing.T) {
176175
"app": "old-test-namespace",
177176
"update": "true",
178177
},
179-
180-
ResourceVersion: "1",
181178
},
182179
}
183180

@@ -228,7 +225,6 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) {
228225
"update": "true",
229226
"group": "test",
230227
},
231-
ResourceVersion: "0",
232228
},
233229
}
234230

@@ -239,7 +235,6 @@ func TestDeleteandUpdateNamespaceLabel(t *testing.T) {
239235
"app": "old-test-namespace",
240236
"update": "false",
241237
},
242-
ResourceVersion: "1",
243238
},
244239
}
245240

‎npm/npm.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type NetworkPolicyManager struct {
5050

5151
nodeName string
5252
nsMap map[string]*namespace
53+
podMap map[string]string // Key: Pod uuid, Value: PodIp
5354
isAzureNpmChainCreated bool
5455
isSafeToCleanUpAzureNpmChain bool
5556

@@ -117,13 +118,14 @@ func (npMgr *NetworkPolicyManager) SendClusterMetrics() {
117118
for {
118119
<-heartbeat
119120
npMgr.Lock()
120-
podCount.Value = 0
121+
podCount.Value = float64(len(npMgr.podMap))
121122
//Reducing one to remove all-namespaces ns obj
122123
nsCount.Value = float64(len(npMgr.nsMap) - 1)
124+
nwPolCount := 0
123125
for _, ns := range npMgr.nsMap {
124-
nwPolicyCount.Value += float64(len(ns.rawNpMap))
125-
podCount.Value += float64(len(ns.podMap))
126+
nwPolCount = nwPolCount + len(ns.rawNpMap)
126127
}
128+
nwPolicyCount.Value = float64(nwPolCount)
127129
npMgr.Unlock()
128130

129131
metrics.SendMetric(podCount)
@@ -232,6 +234,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
232234
npInformer: npInformer,
233235
nodeName: os.Getenv("HOSTNAME"),
234236
nsMap: make(map[string]*namespace),
237+
podMap: make(map[string]string),
235238
isAzureNpmChainCreated: false,
236239
isSafeToCleanUpAzureNpmChain: false,
237240
clusterState: telemetry.ClusterState{
@@ -248,7 +251,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
248251
npMgr.nsMap[util.KubeAllNamespacesFlag] = allNs
249252

250253
// Create ipset for the namespace.
251-
kubeSystemNs := util.GetNSNameWithPrefix(util.KubeSystemFlag)
254+
kubeSystemNs := "ns-" + util.KubeSystemFlag
252255
if err := allNs.ipsMgr.CreateSet(kubeSystemNs, append([]string{util.IpsetNetHashFlag})); err != nil {
253256
metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to create ipset for namespace %s.", kubeSystemNs)
254257
}
@@ -266,14 +269,19 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in
266269
npMgr.AddPod(podObj)
267270
npMgr.Unlock()
268271
},
269-
UpdateFunc: func(_, new interface{}) {
272+
UpdateFunc: func(old, new interface{}) {
273+
oldPodObj, ok := old.(*corev1.Pod)
274+
if !ok {
275+
metrics.SendErrorLogAndMetric(util.NpmID, "UPDATE Pod: Received unexpected old object type: %v", oldPodObj)
276+
return
277+
}
270278
newPodObj, ok := new.(*corev1.Pod)
271279
if !ok {
272280
metrics.SendErrorLogAndMetric(util.NpmID, "UPDATE Pod: Received unexpected new object type: %v", newPodObj)
273281
return
274282
}
275283
npMgr.Lock()
276-
npMgr.UpdatePod(newPodObj)
284+
npMgr.UpdatePod(oldPodObj, newPodObj)
277285
npMgr.Unlock()
278286
},
279287
DeleteFunc: func(obj interface{}) {

‎npm/nwpolicy.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP
3333
err error
3434
ns *namespace
3535
exists bool
36-
npNs = util.GetNSNameWithPrefix(npObj.ObjectMeta.Namespace)
36+
npNs = "ns-" + npObj.ObjectMeta.Namespace
3737
npName = npObj.ObjectMeta.Name
3838
allNs = npMgr.nsMap[util.KubeAllNamespacesFlag]
3939
timer = metrics.StartNewTimer()
@@ -153,7 +153,7 @@ func (npMgr *NetworkPolicyManager) DeleteNetworkPolicy(npObj *networkingv1.Netwo
153153
allNs = npMgr.nsMap[util.KubeAllNamespacesFlag]
154154
)
155155

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

159159
var exists bool

0 commit comments

Comments
 (0)