cni-plugin: merge calico-ipam into calico

[liuxu623]

Description

Merge calico-ipam into calico, use function calls instead of binary file calls to improve performance and code robustness.

Related issues/PRs

Todos

• Tests
• Documentation
• Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

• docs-pr-required: This change requires a change to the documentation that has not been completed yet.
• docs-completed: This change has all necessary documentation completed.
• docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

• release-note-required: This PR has user-facing changes. Most PRs should have this label.
• release-note-not-required: This PR has no user-facing changes.

Other optional labels:

• cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
• needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

[caseydavenport]

Thanks for the PR @liuxu623 - just wanted to let you know I haven't missed this one, just haven't had time to take a look yet.

[caseydavenport]

@liuxu623 did you have some way of measuring performance / robustness here? As far as I know, this has never been a problem and I want to make sure we're able to quantify the impact of this change if we're going to make it.

Was there some problem you were seeing that this resolves?

[liuxu623]

@liuxu623 did you have some way of measuring performance / robustness here? As far as I know, this has never been a problem and I want to make sure we're able to quantify the impact of this change if we're going to make it.

Was there some problem you were seeing that this resolves?

@caseydavenport In fact, I want to reduce some api calls in this way, such as list ippools, but this PR is just an attempt, the actual effect is very limited.

I am considering some other ways to optimize API calls, such as caching some data required by CNI plugins through felix, and felix provides an interface for CNI plugins to call instead of directly accessing the apiserver.

What do you think about?

[caseydavenport]

Reducing the number of API calls required to make an IP allocation is something I'd like to do as well! I think it's a fairly complicated task - unfortunately, our IPAM code does require a fair number of API calls as it is currently implemented.

More broadly, I think the way to do this is similar to what you described above - switching to a (e.g., gRPC) daemon backed model, where the IPAM plugin can make requests to the Daemon and get IPs back. The Daemon would be using watches to cache data instead of explicit requests (ideally via Typha to avoid API server load) - but this is a really big change and requires a lot of care to ensure we handle edge cases and race conditions appropriately.

I'd be happy to talk through options for moving that direction - I don't think that removing the exec call to the IPAM plugin is going to move the needle here though, and I'd rather not merge changes without an impact - maybe let's start another issue where we can discuss what the future of Calico's IPAM implementation might look like?

[liuxu623]

Reducing the number of API calls required to make an IP allocation is something I'd like to do as well! I think it's a fairly complicated task - unfortunately, our IPAM code does require a fair number of API calls as it is currently implemented.

More broadly, I think the way to do this is similar to what you described above - switching to a (e.g., gRPC) daemon backed model, where the IPAM plugin can make requests to the Daemon and get IPs back. The Daemon would be using watches to cache data instead of explicit requests (ideally via Typha to avoid API server load) - but this is a really big change and requires a lot of care to ensure we handle edge cases and race conditions appropriately.

I'd be happy to talk through options for moving that direction - I don't think that removing the exec call to the IPAM plugin is going to move the needle here though, and I'd rather not merge changes without an impact - maybe let's start another issue where we can discuss what the future of Calico's IPAM implementation might look like?

ok, close this PR and discuss in another issue.