Ipsetmanager-update #1034
Ipsetmanager-update #1034
Conversation
…usage of prometheus metrics, and update dataplane API to not use IPSets
…with a special type, and fixed go lints
…nd write TODOs for kernel-based metrics
vakalapa
left a comment
vakalapa
left a comment
There was a problem hiding this comment.
We will need think about bulk inputs, i think bulk inputs will complicate the controllers. We can take it up as a separate work item.
vakalapa
left a comment
vakalapa
left a comment
There was a problem hiding this comment.
we do not have to fix these now, just for note keeping purposes
| iMgr.Lock() | ||
| defer iMgr.Unlock() | ||
| if iMgr.exists(setName) { | ||
| return npmerrors.Errorf(npmerrors.CreateIPSet, false, fmt.Sprintf("ipset %s already exists", setName)) |
There was a problem hiding this comment.
this should be a no-op, return nil. in which case, we can remove error return on this whole function
| iMgr.Lock() | ||
| defer iMgr.Unlock() | ||
| if !iMgr.exists(name) { | ||
| return npmerrors.Errorf(npmerrors.DestroyIPSet, false, fmt.Sprintf("ipset %s does not exist", name)) |
There was a problem hiding this comment.
should be no-op with a warning message,
|
|
||
| set := iMgr.setMap[name] | ||
| if !set.canBeDeleted() { | ||
| return npmerrors.Errorf(npmerrors.DeleteIPSet, false, fmt.Sprintf("ipset %s cannot be deleted", name)) |
There was a problem hiding this comment.
I think it would be better to have some error codes so the caller can decide what to do with this error, may be not in this case, but in some scenarios caller needs to decide if the error is a no-op or legitimate retry mechanism
vakalapa
left a comment
vakalapa
left a comment
There was a problem hiding this comment.
more iterations to come!
Adds logic for adding/removing sets to kernel in ipsetmanger and changes the dataplane API to not use IPSets