Skip to content

Conversation

@ryarnyah
Copy link
Contributor

@ryarnyah ryarnyah commented Sep 25, 2017

I add some improvement to remove old ipset set used by old networkpolicies.

  • Remove ipset dependency to use thread safe ipset
  • Delete unused ipset sets on clean

Feel free to comment or suggest any modification to this PR.

Edit: Seems that travis can't push to docker.io....

@murali-reddy
Copy link
Member

@ryarnyah thanks for your PR. Yes we have pending issue to fix Travis CI. Please ignore that.

We will review, and get back to you.

@murali-reddy
Copy link
Member

@ryarnyah Overall looks good. Somehow cleaning up ipsets slipped all this while, there is no tracking bug as well :) So thanks for fixing it.

I understand its just a wrapper around ipset command, so its okay if it needed to be replaced with what you wrote. But i am not sure how the new implementation is thread-safe?

@ryarnyah
Copy link
Contributor Author

@murali-reddy The main differencies between the go-ipset implementation and this is that "ipsetPath" variable is initialized per IPSet object and not by globally initialized by the first call to New (which in go-ipset always create a new set).

You can call this implementation more "thread-safe" because IPSet object carry is own "ipsetPath" and init it in each IPSet objects and not shared with all threads and not initialized by only one(which can generate an error with call to Swap per example).

Also this implementation support Save and Restore to Sync with real ipset sets.

Is it more clear? (i'm sorry if my english is not as clear as i want...)

@murali-reddy murali-reddy merged commit fc86d2e into cloudnativelabs:master Sep 27, 2017
@ryarnyah ryarnyah deleted the fix/clean-ipset branch September 27, 2017 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants