NPM v2 Linux IPSet Manager #1029

huntergregory · 2021-09-22T00:04:10Z

Reason for Change:
Linux implementation of new NPM with batch ipset-restore calls and error handling. Flushes anything in the dirty cache and adds all current members instead of calculating a diff.

Includes a tool for creating restore files and handling errors (will use for iptables too).

Also implements ApplyAll mode and introduces a reboot function for ipsetmanager.

Issue Fixed:

Requirements:

uses conventional commit messages
includes documentation
adds unit tests

Notes:

huntergregory · 2021-09-22T20:11:24Z

Adding some notes on changes to generic ipsetmanager in this PR

vakalapa

We will need to abstract files part, and also incorporate some diff calculation logic on deleting existing members

npm/pkg/dataplane/ipsets/ipset.go

npm/pkg/dataplane/ipsets/ipsetmanager.go

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go

vakalapa · 2021-09-23T17:45:30Z

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go

+	// DEBUGGING
+	fmt.Printf("DEBUG-ME\nname: %s\nkind: %s\npodip: \n", set.Name, set.Kind)
+	fmt.Println(set.IPPodKey)
+	fmt.Println("members: ")


How are we managing deleting existing members of an ipset ?

in updateMembers(), we flush the set before adding each member

This reverts commit b20c486.

vakalapa

initial CR

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go

…lback

This reverts commit b53af2c.

huntergregory · 2021-10-14T20:12:20Z

Noting that in a second pass, we need to make 2 changes because ipset restore will commit lines before a line that fails.

change update member logic: we should be deleting members instead of flushing and adding all members (there would be a gap between a flush and a retry if an add fails)
change error handling methods to skip the lines before a failure

npm/pkg/dataplane/ipsets/ipsetmanager.go

vakalapa · 2021-10-14T18:58:22Z

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go

+				// TODO add error handler?
+				creator.AddLine(sectionID, nil, util.IpsetAppendFlag, set.HashedName, ip) // add IP
+			}
+		} else {


Check if after flush and error occured while adding, will there be an issue or a time gap between flushed state and actual state

wrote a comment above about how we should address this

huntergregory · 2021-10-15T18:48:44Z

azp run

huntergregory · 2021-10-15T19:27:09Z

/azp run

azure-pipelines · 2021-10-15T19:27:25Z

Azure Pipelines successfully started running 2 pipeline(s).

vakalapa · 2021-10-15T22:37:32Z

npm/pkg/dataplane/ioutil/file-creator.go

+
+	commandString := cmd + " " + strings.Join(args, " ")
+	stdErr := string(stdErrBytes)
+	log.Errorf("on try number %d, failed to run command [%s] with error [%v] and stdErr [%s]. Used file:\n%s", creator.tryCount, commandString, err, stdErr, fileString)


We should be using klogf since thats the norm in other packages.

TODO in another PR

npm/pkg/dataplane/ipsets/ipsetmanager.go

npm/pkg/dataplane/ipsets/ipsetmanager_windows.go

huntergregory · 2021-10-15T23:11:50Z

Noting that in a second pass, we need to make 2 changes because ipset restore will commit lines before a line that fails.

change update member logic: we should be deleting members instead of flushing and adding all members (there would be a gap between a flush and a retry if an add fails)

change error handling methods to skip the lines before a failure

noting our plan for # 1 to use ipset save to get kernel state and delete the diff

vakalapa

🚀 Send it

huntergregory added 3 commits September 21, 2021 17:00

add note in comment

b20c486

fix ipset metrics, add TODO comments, and add delete cache

d10077c

linux ipsetmanager

5e5a71e

huntergregory requested review from JungukCho, matmerr and vakalapa September 22, 2021 00:04

vakalapa requested changes Sep 23, 2021

View reviewed changes

huntergregory added 12 commits September 23, 2021 13:56

Revert "add note in comment"

6bf3970

This reverts commit b20c486.

update comments and add stub for destroyNPMIPsets() func

5262761

move restore file logic to external package

320830a

remove existence check in updateDirtyCache()

7f46887

rearranging stuff and updates for old version of new ipsetmanager

00afe57

update ipsetmanager to version in master (from ipsetmanager-update PR)

42951c6

merge with master

ebce2f5

updates to ipsetmanager linux test

b1362b4

Merge branch 'master' into linux-ipsetmanager

c3c6f45

revamped file creator for retry logic (for ipset and iptables restore)

053cf78

renaming variables and moving code around

02c5d48

completed logic for retrying (generic and for ipset restore)

0054f0e

vakalapa reviewed Oct 7, 2021

View reviewed changes

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go Outdated Show resolved Hide resolved

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go Show resolved Hide resolved

npm/pkg/dataplane/ipsets/ipsetmanager_linux.go Outdated Show resolved Hide resolved

huntergregory added 9 commits October 7, 2021 12:03

addressing sectionID comment and including a call to errorHandler Cal…

60aaf39

…lback

resolve small merge conflict

7fa383a

remove obsolete struct fields

b53af2c

Revert "remove obsolete struct fields"

888e48b

This reverts commit b53af2c.

unit tests and some changes/reordering of code for file-creator

5beda9e

file creator unit tests (forgot to add in last commit)

5899b5c

refactored file creator for external testing of error handling

39700a5

added missing argument to fucntion

aea97a0

first pass at unit testing for ipsetmanager_linux

55122b5

huntergregory added 6 commits October 13, 2021 17:36

full UT coverage for ipsetmanager linux

3d41e88

Merge branch 'master' into linux-ipsetmanager

aae8924

resolvee golint problems with function literals

cb7659d

resolve golint problems with function literals v2

e79d61f

rename variable

84bf1d7

added reboot function to generic ipsetmanager

363cb0f

huntergregory marked this pull request as ready for review October 14, 2021 01:00

huntergregory changed the title ~~Linux IPSet Manager~~ NPM v2 Linux IPSet Manager Oct 14, 2021

huntergregory added 2 commits October 13, 2021 18:17

added caveat about ApplyAll mode

a6601c4

initialize metrics in dataplane constructor

ab64e9d

vakalapa added enhancement npm Related to NPM. labels Oct 14, 2021

huntergregory added 2 commits October 14, 2021 10:43

fix go lints and update error messages in file creator

bf64e8d

add basic integration testing file

c5a779b

vakalapa reviewed Oct 14, 2021

View reviewed changes

huntergregory added 3 commits October 14, 2021 16:25

addressing comments

e0915c8

fix unit tests

7a33814

fix lints in integration test file

096b71f

vakalapa reviewed Oct 15, 2021

View reviewed changes

npm/pkg/dataplane/ipsets/ipsetmanager.go Show resolved Hide resolved

vakalapa reviewed Oct 15, 2021

View reviewed changes

npm/pkg/dataplane/ipsets/ipsetmanager_windows.go Outdated Show resolved Hide resolved

fixed bad function name in windows ipsetmanager

d39c879

vakalapa approved these changes Oct 15, 2021

View reviewed changes

huntergregory merged commit e692542 into master Oct 15, 2021

vakalapa deleted the linux-ipsetmanager branch October 29, 2021 22:03

NPM v2 Linux IPSet Manager #1029

NPM v2 Linux IPSet Manager #1029

Uh oh!

Conversation

huntergregory commented Sep 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huntergregory commented Sep 22, 2021

Uh oh!

vakalapa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vakalapa Sep 23, 2021

Choose a reason for hiding this comment

Uh oh!

huntergregory Sep 23, 2021

Choose a reason for hiding this comment

Uh oh!

vakalapa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

huntergregory commented Oct 14, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vakalapa Oct 14, 2021

Choose a reason for hiding this comment

Uh oh!

huntergregory Oct 14, 2021

Choose a reason for hiding this comment

Uh oh!

huntergregory commented Oct 15, 2021

Uh oh!

huntergregory commented Oct 15, 2021

Uh oh!

azure-pipelines bot commented Oct 15, 2021

Uh oh!

vakalapa Oct 15, 2021

Choose a reason for hiding this comment

Uh oh!

huntergregory Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

huntergregory commented Oct 15, 2021

Uh oh!

vakalapa left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

huntergregory commented Sep 22, 2021 •

edited

Loading

huntergregory Oct 15, 2021 •

edited

Loading