From e18958ee952b66c304da9328ddbc57fa0eb57f95 Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Tue, 17 Dec 2019 19:48:36 +0000 Subject: [PATCH 1/3] Adding namespace to npMgr if it doesn't exist. This fixes reboot bug where entries don't get added after failing to append non-existing namespace to all-namespaces. --- npm/namespace.go | 13 ++++++++----- npm/namespace_test.go | 6 +++--- npm/npm.go | 2 +- npm/nwpolicy.go | 19 +++++++++++++++---- npm/nwpolicy_test.go | 6 +++--- npm/pod.go | 22 +++++++++++++++------- 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/npm/namespace.go b/npm/namespace.go index 58f2ccb32b..74127f0ce0 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -86,10 +86,13 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error { return nil } -// AddNamespace handles adding namespace to ipset. -func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { - npMgr.Lock() - defer npMgr.Unlock() +// AddNamespace handles adding namespace to ipset; if reEntrant is set to true, this func will +// not acquire npMgr lock. +func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace, reEntrant bool) error { + if !reEntrant { + npMgr.Lock() + defer npMgr.Unlock() + } var err error @@ -151,7 +154,7 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n } if newNsObj.ObjectMeta.DeletionTimestamp == nil && newNsObj.ObjectMeta.DeletionGracePeriodSeconds == nil { - if err = npMgr.AddNamespace(newNsObj); err != nil { + if err = npMgr.AddNamespace(newNsObj, false); err != nil { return err } } diff --git a/npm/namespace_test.go b/npm/namespace_test.go index 74814fc9e3..be0b29fe62 100644 --- a/npm/namespace_test.go +++ b/npm/namespace_test.go @@ -80,7 +80,7 @@ func TestAddNamespace(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj); err != nil { + if err := npMgr.AddNamespace(nsObj, false); err != nil { t.Errorf("TestAddNamespace @ npMgr.AddNamespace") } } @@ -130,7 +130,7 @@ func TestUpdateNamespace(t *testing.T) { }, } - if err := npMgr.AddNamespace(oldNsObj); err != nil { + if err := npMgr.AddNamespace(oldNsObj, false); err != nil { t.Errorf("TestUpdateNamespace failed @ npMgr.AddNamespace") } @@ -175,7 +175,7 @@ func TestDeleteNamespace(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj); err != nil { + if err := npMgr.AddNamespace(nsObj, false); err != nil { t.Errorf("TestDeleteNamespace @ npMgr.AddNamespace") } diff --git a/npm/npm.go b/npm/npm.go index 128235f3e2..aa79d2b730 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -207,7 +207,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in // Namespace event handlers cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - npMgr.AddNamespace(obj.(*corev1.Namespace)) + npMgr.AddNamespace(obj.(*corev1.Namespace), false) }, UpdateFunc: func(old, new interface{}) { npMgr.UpdateNamespace(old.(*corev1.Namespace), new.(*corev1.Namespace)) diff --git a/npm/nwpolicy.go b/npm/nwpolicy.go index ae3f3bf630..d58fff339c 100644 --- a/npm/nwpolicy.go +++ b/npm/nwpolicy.go @@ -6,7 +6,10 @@ import ( "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/iptm" "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" ) func (npMgr *NetworkPolicyManager) canCleanUpNpmChains() bool { @@ -36,13 +39,21 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP npNs, npName := "ns-"+npObj.ObjectMeta.Namespace, npObj.ObjectMeta.Name log.Printf("NETWORK POLICY CREATING: %v", npObj) + // Add policy namespace if it doesn't exist var exists bool if ns, exists = npMgr.nsMap[npNs]; !exists { - ns, err = newNs(npNs) - if err != nil { - log.Printf("Error creating namespace %s\n", npNs) + nsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: npObj.ObjectMeta.Namespace, + Labels: make(map[string]string), + }, } - npMgr.nsMap[npNs] = ns + + if err = npMgr.AddNamespace(nsObj, true); err != nil { + return err + } + + ns = npMgr.nsMap[npNs] } if ns.policyExists(npObj) { diff --git a/npm/nwpolicy_test.go b/npm/nwpolicy_test.go index 352cd5b5b6..e1e6e7918c 100644 --- a/npm/nwpolicy_test.go +++ b/npm/nwpolicy_test.go @@ -66,7 +66,7 @@ func TestAddNetworkPolicy(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj); err != nil { + if err := npMgr.AddNamespace(nsObj, false); err != nil { t.Errorf("TestAddNetworkPolicy @ npMgr.AddNamespace") } @@ -177,7 +177,7 @@ func TestUpdateNetworkPolicy(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj); err != nil { + if err := npMgr.AddNamespace(nsObj, false); err != nil { t.Errorf("TestUpdateNetworkPolicy @ npMgr.AddNamespace") } @@ -289,7 +289,7 @@ func TestDeleteNetworkPolicy(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj); err != nil { + if err := npMgr.AddNamespace(nsObj, false); err != nil { t.Errorf("TestDeleteNetworkPolicy @ npMgr.AddNamespace") } diff --git a/npm/pod.go b/npm/pod.go index 5e30c34abc..22b55b8d5b 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -7,6 +7,7 @@ import ( "github.com/Azure/azure-container-networking/npm/util" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func isValidPod(podObj *corev1.Pod) bool { @@ -35,6 +36,20 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { podIP := podObj.Status.PodIP log.Printf("POD CREATING: [%s/%s/%s%+v%s]", podNs, podName, podNodeName, podLabels, podIP) + // Add pod namespace if it doesn't exist + if _, exists := npMgr.nsMap[podNs]; !exists { + nsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: podObj.ObjectMeta.Namespace, + Labels: make(map[string]string), + }, + } + + if err = npMgr.AddNamespace(nsObj, true); err != nil { + return err + } + } + // Add the pod to ipset ipsMgr := npMgr.nsMap[util.KubeAllNamespacesFlag].ipsMgr // Add the pod to its namespace's ipset. @@ -60,13 +75,6 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { } } - ns, err := newNs(podNs) - if err != nil { - log.Errorf("Error: failed to create namespace %s", podNs) - return err - } - npMgr.nsMap[podNs] = ns - return nil } From 63dd9d5784f75c11d09d573d5274d90a773bd646 Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Wed, 18 Dec 2019 01:17:31 +0000 Subject: [PATCH 2/3] poll api-server version for a minute before panicking --- npm/npm.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/npm/npm.go b/npm/npm.go index aa79d2b730..6c7e53fa7b 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -136,11 +136,21 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in iptMgr := iptm.NewIptablesManager() iptMgr.UninitNpmChains() - podInformer := informerFactory.Core().V1().Pods() - nsInformer := informerFactory.Core().V1().Namespaces() - npInformer := informerFactory.Networking().V1().NetworkPolicies() + var ( + podInformer = informerFactory.Core().V1().Pods() + nsInformer = informerFactory.Core().V1().Namespaces() + npInformer = informerFactory.Networking().V1().NetworkPolicies() + serverVersion *version.Info + err error + ) - serverVersion, err := clientset.ServerVersion() + for ticker, start := time.NewTicker(1 * time.Second).C, time.Now(); time.Since(start) < time.Minute * 1; { + <-ticker + serverVersion, err = clientset.ServerVersion() + if err == nil { + break + } + } if err != nil { log.Logf("Error: failed to retrieving kubernetes version") panic(err.Error) From 82784c28a503a29641856db00b87195308f1dd79 Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Wed, 18 Dec 2019 23:39:58 +0000 Subject: [PATCH 3/3] addressing Mat's comments --- npm/namespace.go | 20 +++++++++++--------- npm/namespace_test.go | 6 +++--- npm/npm.go | 2 +- npm/nwpolicy.go | 2 +- npm/nwpolicy_test.go | 6 +++--- npm/pod.go | 2 +- 6 files changed, 20 insertions(+), 18 deletions(-) diff --git a/npm/namespace.go b/npm/namespace.go index 74127f0ce0..68b20c1be1 100644 --- a/npm/namespace.go +++ b/npm/namespace.go @@ -86,14 +86,8 @@ func (npMgr *NetworkPolicyManager) UninitAllNsList() error { return nil } -// AddNamespace handles adding namespace to ipset; if reEntrant is set to true, this func will -// not acquire npMgr lock. -func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace, reEntrant bool) error { - if !reEntrant { - npMgr.Lock() - defer npMgr.Unlock() - } - +// AddNamespace handles adding namespace to ipset +func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace) error { var err error nsName, nsLabel := "ns-" + nsObj.ObjectMeta.Name, nsObj.ObjectMeta.Labels @@ -138,6 +132,14 @@ func (npMgr *NetworkPolicyManager) AddNamespace(nsObj *corev1.Namespace, reEntra return nil } +// AddNamespaceWithLock acquires NetworkPolicyManager lock before adding namespace to ipset +func (npMgr *NetworkPolicyManager) AddNamespaceWithLock(nsObj *corev1.Namespace) error { + npMgr.Lock() + defer npMgr.Unlock() + + return npMgr.AddNamespace(nsObj) +} + // UpdateNamespace handles updating namespace in ipset. func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, newNsObj *corev1.Namespace) error { var err error @@ -154,7 +156,7 @@ func (npMgr *NetworkPolicyManager) UpdateNamespace(oldNsObj *corev1.Namespace, n } if newNsObj.ObjectMeta.DeletionTimestamp == nil && newNsObj.ObjectMeta.DeletionGracePeriodSeconds == nil { - if err = npMgr.AddNamespace(newNsObj, false); err != nil { + if err = npMgr.AddNamespaceWithLock(newNsObj); err != nil { return err } } diff --git a/npm/namespace_test.go b/npm/namespace_test.go index be0b29fe62..482fc5d226 100644 --- a/npm/namespace_test.go +++ b/npm/namespace_test.go @@ -80,7 +80,7 @@ func TestAddNamespace(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj, false); err != nil { + if err := npMgr.AddNamespaceWithLock(nsObj); err != nil { t.Errorf("TestAddNamespace @ npMgr.AddNamespace") } } @@ -130,7 +130,7 @@ func TestUpdateNamespace(t *testing.T) { }, } - if err := npMgr.AddNamespace(oldNsObj, false); err != nil { + if err := npMgr.AddNamespaceWithLock(oldNsObj); err != nil { t.Errorf("TestUpdateNamespace failed @ npMgr.AddNamespace") } @@ -175,7 +175,7 @@ func TestDeleteNamespace(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj, false); err != nil { + if err := npMgr.AddNamespaceWithLock(nsObj); err != nil { t.Errorf("TestDeleteNamespace @ npMgr.AddNamespace") } diff --git a/npm/npm.go b/npm/npm.go index 6c7e53fa7b..5365dcf94e 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -217,7 +217,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in // Namespace event handlers cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - npMgr.AddNamespace(obj.(*corev1.Namespace), false) + npMgr.AddNamespaceWithLock(obj.(*corev1.Namespace)) }, UpdateFunc: func(old, new interface{}) { npMgr.UpdateNamespace(old.(*corev1.Namespace), new.(*corev1.Namespace)) diff --git a/npm/nwpolicy.go b/npm/nwpolicy.go index d58fff339c..71efebc5d7 100644 --- a/npm/nwpolicy.go +++ b/npm/nwpolicy.go @@ -49,7 +49,7 @@ func (npMgr *NetworkPolicyManager) AddNetworkPolicy(npObj *networkingv1.NetworkP }, } - if err = npMgr.AddNamespace(nsObj, true); err != nil { + if err = npMgr.AddNamespace(nsObj); err != nil { return err } diff --git a/npm/nwpolicy_test.go b/npm/nwpolicy_test.go index e1e6e7918c..1e51fe6497 100644 --- a/npm/nwpolicy_test.go +++ b/npm/nwpolicy_test.go @@ -66,7 +66,7 @@ func TestAddNetworkPolicy(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj, false); err != nil { + if err := npMgr.AddNamespaceWithLock(nsObj); err != nil { t.Errorf("TestAddNetworkPolicy @ npMgr.AddNamespace") } @@ -177,7 +177,7 @@ func TestUpdateNetworkPolicy(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj, false); err != nil { + if err := npMgr.AddNamespaceWithLock(nsObj); err != nil { t.Errorf("TestUpdateNetworkPolicy @ npMgr.AddNamespace") } @@ -289,7 +289,7 @@ func TestDeleteNetworkPolicy(t *testing.T) { }, } - if err := npMgr.AddNamespace(nsObj, false); err != nil { + if err := npMgr.AddNamespaceWithLock(nsObj); err != nil { t.Errorf("TestDeleteNetworkPolicy @ npMgr.AddNamespace") } diff --git a/npm/pod.go b/npm/pod.go index 22b55b8d5b..926ecccac5 100644 --- a/npm/pod.go +++ b/npm/pod.go @@ -45,7 +45,7 @@ func (npMgr *NetworkPolicyManager) AddPod(podObj *corev1.Pod) error { }, } - if err = npMgr.AddNamespace(nsObj, true); err != nil { + if err = npMgr.AddNamespace(nsObj); err != nil { return err } }