Use ipset-go instead of calling the ipset binary

[skitt]

This reduces our requirements on the environment in which Submariner components run.

The ipset binaries are kept in the container images, for debugging purposes and because subctl gather relies on ipset in the route agent.

[submariner-bot]

🤖 Created branch: z_pr2447/skitt/ipset-library
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

[skitt]

The ipset-go library hasn’t produced any releases (yet), but it’s small, has tests, and responds to pull requests.

[dfarrell07]

The ipset-go library hasn’t produced any releases (yet), but it’s small, has tests, and responds to pull requests.

Not necessary voting -1, but it also hasn't had any contributors other than you, doesn't have a great git history, and doesn't seem to be used by any consumers. 😅

Still looking..

[skitt]

There’s also https://github.com/nadoo/ipset, which has no dependencies and has produced releases, but it’s GPL-3-licensed (which I don’t mind, but probably won’t fly with the CNCF), and it has no tests.

[dfarrell07]

I was able to ask Stephen a bunch of questions about this on a recent call. Overall I think we don't have a great path either way and there isn't one choice that seems obviously best/worst.

• Moving to only-nftables likely isn't going to happen as OpenShiftSDN is moving to focus on OVN.
• The current solution adds a few os.exec calls that this would avoid.
• It's not clear if this path would reduce the likelihood of concurrency issues we see with the current wrapped binary, as those seem to come from timing between userspace and the kernel (which both solutions could have).
• The current library we're using was abandoned by Tencent and forked in to K8s. We're not using the latest version of that, so if we stick with the current design we should likely look into keeping that up-to-date.
• We may want to move subctl gather to use this lib too, to drop the binary there, but that would be a more major change.

If we look through the code of this lib and feel safe about it, I think it's fine to move to it.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nyechiel]

@skitt this one needs a rebase

[nyechiel]

@skitt this one needs a rebase

ping :)

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nyechiel]

bump

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tpantelis]

Do we still need or want to do this?

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further
activity occurs. Thank you for your contributions.

[submariner-bot]

🤖 Closed branches: [z_pr2447/skitt/ipset-library]