Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support static addresses in secondary network IPAM #3633

Merged
merged 1 commit into from
Apr 23, 2022

Conversation

jianjuns
Copy link
Contributor

Support configuring static addresses in the CNI IPAM configuration,
besides allocating IPs from IPPools.

Signed-off-by: Jianjun Shen shenj@vmware.com

@jianjuns jianjuns added the area/ipam Issues or PRs related to IP address management (IPAM). label Apr 13, 2022
@jianjuns jianjuns added this to the Antrea v1.7 release milestone Apr 13, 2022
@jianjuns
Copy link
Contributor Author

This is my last planned change for secondary network IPAM.

@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2022

Codecov Report

Merging #3633 (d7482db) into main (c463028) will decrease coverage by 14.37%.
The diff coverage is 57.29%.

❗ Current head d7482db differs from pull request most recent head c70fde8. Consider uploading reports for the commit c70fde8 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3633       +/-   ##
===========================================
- Coverage   64.54%   50.17%   -14.38%     
===========================================
  Files         278      247       -31     
  Lines       39347    35658     -3689     
===========================================
- Hits        25398    17890     -7508     
- Misses      11967    15984     +4017     
+ Partials     1982     1784      -198     
Flag Coverage Δ
e2e-tests 50.17% <57.29%> (?)
kind-e2e-tests ?
unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/openflow/pipeline_other.go 4.00% <0.00%> (+0.07%) ⬆️
pkg/features/antrea_features.go 11.11% <ø> (ø)
pkg/agent/proxy/proxier.go 47.15% <3.70%> (-14.53%) ⬇️
pkg/agent/openflow/pipeline.go 69.35% <56.96%> (-5.55%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go 50.21% <63.63%> (-23.45%) ⬇️
pkg/agent/openflow/pod_connectivity.go 68.31% <75.00%> (+0.27%) ⬆️
pkg/ovs/openflow/ofctrl_action.go 67.29% <100.00%> (-1.58%) ⬇️
pkg/ovs/openflow/ofctrl_builder.go 55.76% <100.00%> (-2.71%) ⬇️
pkg/ipfix/ipfix_intermediate.go 0.00% <0.00%> (-90.91%) ⬇️
pkg/controller/networkpolicy/endpoint_querier.go 4.58% <0.00%> (-88.08%) ⬇️
... and 160 more

@lgtm-com
Copy link

lgtm-com bot commented Apr 13, 2022

This pull request introduces 2 alerts when merging f4687bb into c463028 - view on LGTM.com

new alerts:

  • 2 for Useless assignment to field

@jianjuns jianjuns force-pushed the secnet-ipam branch 3 times, most recently from 26c8f58 to 7071e92 Compare April 13, 2022 20:49
ipConfig, _ := generateIPConfig(ip, int(subnetInfo.PrefixLength), gwIP)
result.IPs = append(result.IPs, ipConfig)
// Add static addresses.
for _, a := range ipamConf.Addresses {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method will not require an IPPool and antrea won't check this IP is allocated by others. Is it correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What behavior we will get if user set both IPPool and IPAddress (in this IPPool or not in this IPPool)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We do not check and users need to make sure no IP conflicts. As described in #3634 , this is just what static IPAM plugin supports (https://www.cni.dev/plugins/current/ipam/static/) and it is also supported by Whereabouts. I added it for compatibility with them.

If both Pools and static addresses are specified, we will allocate IPs from Pools and append static addresses.

Support configuring static addresses in the CNI IPAM configuration,
besides allocating IPs from IPPools.

Signed-off-by: Jianjun Shen <shenj@vmware.com>
Copy link
Contributor

@gran-vmv gran-vmv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jianjuns
Copy link
Contributor Author

/test-all

@jianjuns
Copy link
Contributor Author

/test-integration

@jianjuns jianjuns merged commit 590d036 into antrea-io:main Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipam Issues or PRs related to IP address management (IPAM).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants