From b36a8a67f9bde45bc395eda554d16cfb42741790 Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 22 Sep 2021 13:44:37 -0700 Subject: [PATCH 1/3] [NPM] Modifying destroy ipsets logic --- npm/ipsm/ipsm.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/npm/ipsm/ipsm.go b/npm/ipsm/ipsm.go index 9b566ecd4f..6b73e65eb7 100644 --- a/npm/ipsm/ipsm.go +++ b/npm/ipsm/ipsm.go @@ -616,22 +616,22 @@ func (ipsMgr *IpsetManager) DestroyNpmIpsets() error { operationFlag: util.IpsetFlushFlag, set: ipsetName, } - _, flushError := ipsMgr.run(flushEntry) + _, err := ipsMgr.run(flushEntry) + if err != nil { + metrics.SendErrorLogAndMetric(util.IpsmID, "{DestroyNpmIpsets} Error: failed to flush ipset %s", ipsetName) + } + } + for _, ipsetName := range ipsetLists { deleteEntry := &ipsEntry{ operationFlag: util.IpsetDestroyFlag, set: ipsetName, } - _, destroyError := ipsMgr.run(deleteEntry) - - if flushError != nil { - metrics.SendErrorLogAndMetric(util.IpsmID, "{DestroyNpmIpsets} Error: failed to flush ipset %s", ipsetName) - } - if destroyError != nil { + _, err := ipsMgr.run(deleteEntry) + if err != nil { destroyFailureCount++ metrics.SendErrorLogAndMetric(util.IpsmID, "{DestroyNpmIpsets} Error: failed to destroy ipset %s", ipsetName) - } - if flushError == nil || destroyError == nil { + } else { metrics.RemoveAllEntriesFromIPSet(ipsetName) } } From aded000a6f08464a1802b5fcb05c2a88e426eae3 Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 22 Sep 2021 13:49:44 -0700 Subject: [PATCH 2/3] [NPM] Modifying destroy ipsets logic --- npm/ipsm/ipsm.go | 2 ++ npm/metrics/ipsets.go | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/npm/ipsm/ipsm.go b/npm/ipsm/ipsm.go index 6b73e65eb7..09878f36d9 100644 --- a/npm/ipsm/ipsm.go +++ b/npm/ipsm/ipsm.go @@ -619,6 +619,8 @@ func (ipsMgr *IpsetManager) DestroyNpmIpsets() error { _, err := ipsMgr.run(flushEntry) if err != nil { metrics.SendErrorLogAndMetric(util.IpsmID, "{DestroyNpmIpsets} Error: failed to flush ipset %s", ipsetName) + } else { + metrics.ResetEntriesInIPSet(ipsetName) } } diff --git a/npm/metrics/ipsets.go b/npm/metrics/ipsets.go index 503388f110..ecfe6f6247 100644 --- a/npm/metrics/ipsets.go +++ b/npm/metrics/ipsets.go @@ -63,7 +63,13 @@ func RemoveEntryFromIPSet(setName string) { } } -// RemoveAllEntriesFromIPSet sets the number of entries for ipset setName to 0. +// ResetEntriesInIPSet sets the number of entries for ipset setName to 0. +// It doesn't ever update the number of IPSets. +func ResetEntriesInIPSet(setName string) { + numIPSetEntries.Add(-getEntryCountForIPSet(setName)) +} + +// RemoveAllEntriesFromIPSet sets the number of entries for ipset setName to 0 and deletes the set // It doesn't ever update the number of IPSets. func RemoveAllEntriesFromIPSet(setName string) { numIPSetEntries.Add(-getEntryCountForIPSet(setName)) From 3733e316aff6c9608a68d64fd8c10d20fddea03f Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 22 Sep 2021 14:25:39 -0700 Subject: [PATCH 3/3] removing the metrics additions and correcting UT fake exec order --- npm/ipsm/ipsm.go | 2 -- npm/ipsm/ipsm_test.go | 2 +- npm/metrics/ipsets.go | 8 +------- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/npm/ipsm/ipsm.go b/npm/ipsm/ipsm.go index 09878f36d9..6b73e65eb7 100644 --- a/npm/ipsm/ipsm.go +++ b/npm/ipsm/ipsm.go @@ -619,8 +619,6 @@ func (ipsMgr *IpsetManager) DestroyNpmIpsets() error { _, err := ipsMgr.run(flushEntry) if err != nil { metrics.SendErrorLogAndMetric(util.IpsmID, "{DestroyNpmIpsets} Error: failed to flush ipset %s", ipsetName) - } else { - metrics.ResetEntriesInIPSet(ipsetName) } } diff --git a/npm/ipsm/ipsm_test.go b/npm/ipsm/ipsm_test.go index 9aeff2c5c3..0788f458af 100644 --- a/npm/ipsm/ipsm_test.go +++ b/npm/ipsm/ipsm_test.go @@ -530,8 +530,8 @@ func TestDestroyNpmIpsets(t *testing.T) { {Cmd: []string{"ipset", "-N", "-exist", util.GetHashedName(testSet2Name), "nethash"}}, {Cmd: []string{"ipset", "list"}, Stdout: ipsetListStdout}, {Cmd: []string{"ipset", "-F", "-exist", testSet1Name}}, - {Cmd: []string{"ipset", "-X", "-exist", testSet1Name}}, {Cmd: []string{"ipset", "-F", "-exist", testSet2Name}}, + {Cmd: []string{"ipset", "-X", "-exist", testSet1Name}}, {Cmd: []string{"ipset", "-X", "-exist", testSet2Name}}, } diff --git a/npm/metrics/ipsets.go b/npm/metrics/ipsets.go index ecfe6f6247..503388f110 100644 --- a/npm/metrics/ipsets.go +++ b/npm/metrics/ipsets.go @@ -63,13 +63,7 @@ func RemoveEntryFromIPSet(setName string) { } } -// ResetEntriesInIPSet sets the number of entries for ipset setName to 0. -// It doesn't ever update the number of IPSets. -func ResetEntriesInIPSet(setName string) { - numIPSetEntries.Add(-getEntryCountForIPSet(setName)) -} - -// RemoveAllEntriesFromIPSet sets the number of entries for ipset setName to 0 and deletes the set +// RemoveAllEntriesFromIPSet sets the number of entries for ipset setName to 0. // It doesn't ever update the number of IPSets. func RemoveAllEntriesFromIPSet(setName string) { numIPSetEntries.Add(-getEntryCountForIPSet(setName))