From 784ba005fb00b0ff8ee7d8ff93a419de6bfb8afd Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 7 Jul 2021 10:25:09 -0700 Subject: [PATCH 1/3] Fixing the looping issue in backup and reconcile chains --- npm/npm.go | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/npm/npm.go b/npm/npm.go index f4dab0a135..8fb86dee82 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -157,14 +157,22 @@ func (npMgr *NetworkPolicyManager) restore() { } // backup takes snapshots of iptables filter table and saves it periodically. -func (npMgr *NetworkPolicyManager) backup() { - iptMgr := iptm.NewIptablesManager() - var err error - for { - time.Sleep(backupWaitTimeInSeconds * time.Second) +func (npMgr *NetworkPolicyManager) backup(stopCh <-chan struct{}) { + var ( + err error + iptMgr = iptm.NewIptablesManager() + ticker = time.NewTicker(time.Second * time.Duration(backupWaitTimeInSeconds)) + ) + defer ticker.Stop() - if err = iptMgr.Save(util.IptablesConfigFile); err != nil { - metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to back up Azure-NPM states %s", err.Error()) + for { + select { + case <-stopCh: + return + case <-ticker.C: + if err = iptMgr.Save(util.IptablesConfigFile); err != nil { + metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to back up Azure-NPM states %s", err.Error()) + } } } } @@ -194,8 +202,8 @@ func (npMgr *NetworkPolicyManager) Start(stopCh <-chan struct{}) error { go npMgr.podController.Run(threadness, stopCh) go npMgr.nameSpaceController.Run(threadness, stopCh) go npMgr.netPolController.Run(threadness, stopCh) - go npMgr.reconcileChains() - go npMgr.backup() + go npMgr.reconcileChains(stopCh) + go npMgr.backup(stopCh) return nil } @@ -280,13 +288,19 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in } // reconcileChains checks for ordering of AZURE-NPM chain in FORWARD chain periodically. -func (npMgr *NetworkPolicyManager) reconcileChains() error { +func (npMgr *NetworkPolicyManager) reconcileChains(stopCh <-chan struct{}) error { iptMgr := iptm.NewIptablesManager() - select { - case <-time.After(reconcileChainTimeInMinutes * time.Minute): - if err := iptMgr.CheckAndAddForwardChain(); err != nil { - return err + + ticker := time.NewTicker(time.Minute * time.Duration(reconcileChainTimeInMinutes)) + defer ticker.Stop() + for { + select { + case <-stopCh: + return nil + case <-ticker.C: + if err := iptMgr.CheckAndAddForwardChain(); err != nil { + return err + } } } - return nil } From 2af56b2abd2420b52eb269445a6f88126cc52e95 Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 7 Jul 2021 10:32:46 -0700 Subject: [PATCH 2/3] fixing a golint --- npm/npm.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/npm/npm.go b/npm/npm.go index 8fb86dee82..c0d55d0b97 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -288,7 +288,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in } // reconcileChains checks for ordering of AZURE-NPM chain in FORWARD chain periodically. -func (npMgr *NetworkPolicyManager) reconcileChains(stopCh <-chan struct{}) error { +func (npMgr *NetworkPolicyManager) reconcileChains(stopCh <-chan struct{}) { iptMgr := iptm.NewIptablesManager() ticker := time.NewTicker(time.Minute * time.Duration(reconcileChainTimeInMinutes)) @@ -296,10 +296,10 @@ func (npMgr *NetworkPolicyManager) reconcileChains(stopCh <-chan struct{}) error for { select { case <-stopCh: - return nil + return case <-ticker.C: if err := iptMgr.CheckAndAddForwardChain(); err != nil { - return err + metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to reconcileChains Azure-NPM due to %s", err.Error()) } } } From 82e51d626e930b85d0854589d60fda1b661bde7b Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 7 Jul 2021 10:45:30 -0700 Subject: [PATCH 3/3] addressing some comments --- npm/npm.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/npm/npm.go b/npm/npm.go index c0d55d0b97..01d4a9dcc0 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -158,11 +158,8 @@ func (npMgr *NetworkPolicyManager) restore() { // backup takes snapshots of iptables filter table and saves it periodically. func (npMgr *NetworkPolicyManager) backup(stopCh <-chan struct{}) { - var ( - err error - iptMgr = iptm.NewIptablesManager() - ticker = time.NewTicker(time.Second * time.Duration(backupWaitTimeInSeconds)) - ) + iptMgr := iptm.NewIptablesManager() + ticker := time.NewTicker(time.Second * time.Duration(backupWaitTimeInSeconds)) defer ticker.Stop() for { @@ -170,7 +167,7 @@ func (npMgr *NetworkPolicyManager) backup(stopCh <-chan struct{}) { case <-stopCh: return case <-ticker.C: - if err = iptMgr.Save(util.IptablesConfigFile); err != nil { + if err := iptMgr.Save(util.IptablesConfigFile); err != nil { metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to back up Azure-NPM states %s", err.Error()) } } @@ -290,9 +287,9 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in // reconcileChains checks for ordering of AZURE-NPM chain in FORWARD chain periodically. func (npMgr *NetworkPolicyManager) reconcileChains(stopCh <-chan struct{}) { iptMgr := iptm.NewIptablesManager() - ticker := time.NewTicker(time.Minute * time.Duration(reconcileChainTimeInMinutes)) defer ticker.Stop() + for { select { case <-stopCh: