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

Add documentation for secondary network IPAM #3634

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

jianjuns
Copy link
Contributor

Add a document for secondary network IPAM, and add the Antrea IPAM
option to the Multus cookbook.

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

@jianjuns jianjuns added area/ipam Issues or PRs related to IP address management (IPAM). kind/documentation Categorizes issue or PR as related to a documentation. labels Apr 13, 2022
@jianjuns jianjuns added this to the Antrea v1.7 release milestone Apr 13, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2022

Codecov Report

Merging #3634 (393740c) into main (ce0de59) will decrease coverage by 0.40%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3634      +/-   ##
==========================================
- Coverage   63.94%   63.54%   -0.41%     
==========================================
  Files         278      278              
  Lines       27838    39360   +11522     
==========================================
+ Hits        17802    25010    +7208     
- Misses       8110    12420    +4310     
- Partials     1926     1930       +4     
Flag Coverage Δ
kind-e2e-tests 50.68% <ø> (-0.30%) ⬇️
unit-tests 43.83% <ø> (+0.47%) ⬆️

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

Impacted Files Coverage Δ
pkg/features/antrea_features.go 11.11% <ø> (+2.02%) ⬆️
...g/agent/apiserver/handlers/featuregates/handler.go 4.54% <0.00%> (-77.81%) ⬇️
...kg/apiserver/registry/system/supportbundle/rest.go 22.17% <0.00%> (-52.83%) ⬇️
pkg/support/dump.go 7.90% <0.00%> (-49.47%) ⬇️
pkg/support/dump_others.go 0.00% <0.00%> (-44.00%) ⬇️
...egator/apiserver/handlers/recordmetrics/handler.go 4.00% <0.00%> (-40.45%) ⬇️
pkg/agent/cniserver/pod_configuration_linux.go 26.31% <0.00%> (-40.36%) ⬇️
...gregator/apiserver/handlers/flowrecords/handler.go 1.44% <0.00%> (-38.56%) ⬇️
...g/agent/apiserver/handlers/addressgroup/handler.go 5.00% <0.00%> (-35.00%) ⬇️
...agent/apiserver/handlers/appliedtogroup/handler.go 5.00% <0.00%> (-35.00%) ⬇️
... and 269 more

@jianjuns jianjuns force-pushed the ipam-doc branch 2 times, most recently from 3892d55 to bdcf497 Compare April 13, 2022 17:43
Comment on lines 35 to 36
# bridging mode and allocates IPs to Pods in bridging mode. It is also required by secondary network
# network IPAM.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is also required by secondary network IPAM

If I interpret this correctly, we should stop shipping the Whereabouts binary altogether in the Antrea Docker image, as AntreaIPAM is the only supported option now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current implementation supports only Multus managed network, not Antrea native secondary network. I plan to wait for the SR-IOV CI ready to add Antrea IPAM support for native secondary network.

build/yamls/base/conf/antrea-agent.conf Outdated Show resolved Hide resolved
build/yamls/base/conf/antrea-controller.conf Outdated Show resolved Hide resolved
docs/cookbooks/multus/README.md Outdated Show resolved Hide resolved
docs/cookbooks/multus/README.md Outdated Show resolved Hide resolved
docs/cookbooks/multus/README.md Outdated Show resolved Hide resolved
Comment on lines 3 to 6
With the [AntreaIPM feature](antrea-ipam.md#antrea-flexible-ipam), Antrea can
allocate IPs for Pod secondary networks. At the moment, AntreaIPAM supports
secondary networks managed by [Multus](https://github.com/k8snetworkplumbingwg/multus-cni),
we will add support for secondary networks managed by Antrea in future.
Copy link
Contributor

Choose a reason for hiding this comment

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

I may be missing something, but that seems in direct contradiction with what you added to the config comments:

It is also required by secondary network IPAM

I feel like it is quite confusing to use the "secondary network" terminology in both cases, as this is the name of the Antrea feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is your suggestion then? But at least when we finally support IPAM for Antrea native secondary network, then it will not be problem any more?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess most of my confusion comes from the "It is also required by secondary network IPAM." statement in other places. That doesn't seem to apply to either case:

  1. For the Multus case, any IPAM solution can be used, so AntreaIPAM is not required
  2. For the Antrea SecondaryNetwork feature, only Whereabouts is supported at the moment

I think I see what you are trying to say, but maybe something like this would help clarify (in config comments):

To use Antrea for IPAM when configuring secondary network interfaces with Multus, this feature gate needs to be enabled.

For this documentation, the current version sounds good to me. In the last sentence I would just add a reference to the Antrea feature name (SecondaryNetwork) for clarification.

BTW, I am not sure why we need a new document and cannot place this information in https://github.com/antrea-io/antrea/blob/main/docs/antrea-ipam.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Let me rephrase the comments on feature gate. I can merge the doc to antrea-ipam.md as well. Earlier thought secondary network IPAM is not interested by most people, so created a separate doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@jianjuns jianjuns force-pushed the ipam-doc branch 2 times, most recently from b7aeee5 to f4f6609 Compare April 13, 2022 19:01
Comment on lines 3 to 6
With the [AntreaIPM feature](antrea-ipam.md#antrea-flexible-ipam), Antrea can
allocate IPs for Pod secondary networks. At the moment, AntreaIPAM supports
secondary networks managed by [Multus](https://github.com/k8snetworkplumbingwg/multus-cni),
we will add support for secondary networks managed by Antrea in future.
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess most of my confusion comes from the "It is also required by secondary network IPAM." statement in other places. That doesn't seem to apply to either case:

  1. For the Multus case, any IPAM solution can be used, so AntreaIPAM is not required
  2. For the Antrea SecondaryNetwork feature, only Whereabouts is supported at the moment

I think I see what you are trying to say, but maybe something like this would help clarify (in config comments):

To use Antrea for IPAM when configuring secondary network interfaces with Multus, this feature gate needs to be enabled.

For this documentation, the current version sounds good to me. In the last sentence I would just add a reference to the Antrea feature name (SecondaryNetwork) for clarification.

BTW, I am not sure why we need a new document and cannot place this information in https://github.com/antrea-io/antrea/blob/main/docs/antrea-ipam.md?

docs/secondary-network-ipam.md Outdated Show resolved Hide resolved
@jianjuns jianjuns force-pushed the ipam-doc branch 3 times, most recently from fa31969 to 98f1e98 Compare April 13, 2022 21:33
@@ -32,7 +32,10 @@ featureGates:
# Egress: true

# Enable AntreaIPAM, which can allocate IP addresses from IPPools. AntreaIPAM is required by the
# bridging mode and allocates IPs to Pods in bridging mode.
# bridging mode and allocates IPs to Pods in bridging mode. It is also required to use Antrea for
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think that a new line for each case (bridgingMode and secondaryNetwork) is more clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel one paragraph is ok. I do not like short paragraphs.

# Enable flexible IPAM mode for Antrea. This mode allows to assign IP Ranges to Namespaces,
# Deployments and StatefulSets via IP Pool annotation.
# Enable AntreaIPAM, which can allocate IP addresses from IPPools. AntreaIPAM is required by the
# bridging mode and allocates IPs to Pods in bridging mode. It is also required to use Antrea for
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

some small comments, otherwise LGTM

With the AntreaIPAM feature, Antrea can allocate IPs for Pod secondary networks. At the
moment, AntreaIPAM supports secondary networks managed by [Multus](https://github.com/k8snetworkplumbingwg/multus-cni),
we will add support for [secondary networks managed by Antrea](feature-gates.md#secondarynetwork)
in future.
Copy link
Contributor

Choose a reason for hiding this comment

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

in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


To configure Antrea IPAM, `antrea` should be specified as the IPAM plugin in the
the CNI IPAM configuration, and at least one Antrea IPPool should be specified
in the `ippools` field. IPs will be allcoated from the specified IPPool(s) for
Copy link
Contributor

Choose a reason for hiding this comment

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

s/allcoated/allocated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment on lines 272 to 274
Additionally, Antrea IPAM also supports configuration of static IP addresses,
static routes, and DNS settings, as what are supported by the [static IPAM
plugin](https://www.cni.dev/plugins/current/ipam/static). The following example
Copy link
Contributor

Choose a reason for hiding this comment

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

That sentence is not correct. Maybe you mean:

Additionally, Antrea IPAM also supports the same configuration of static IP addresses, static routes, and DNS settings as what is supported by the ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.


## `IPPool` CRD

Antrea IP pools are defined with `IPPool` CRD. The following two examples define
Copy link
Contributor

Choose a reason for hiding this comment

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

with the IPPool CRD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

by the underlay network. For more information, please refer to the
[Antrea IPAM document](antrea-ipam.md#antrea-flexible-ipam).

This feature gate also needs to be enabled, to use Antrea for IPAM when
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This feature gate also needs to be enabled, to use Antrea for IPAM when
This feature gate also needs to be enabled to use Antrea for IPAM when

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

[Antrea IPAM document](antrea-ipam.md#antrea-flexible-ipam).

This feature gate also needs to be enabled, to use Antrea for IPAM when
configuring secondary network interfaces with Multus, in which Antrea works
Copy link
Contributor

Choose a reason for hiding this comment

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

in which case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

be included in the Namespace annotation. In the future, annotation of up to two pools for
IPv4 and IPv6 respectively will be supported.
The bridging mode works only with `system` OVS datapath type; and `noEncap`,
`noSNAT` traffic mode. At this moment, it supports only IPv4. The IPs in an IP
Copy link
Contributor

Choose a reason for hiding this comment

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

At the moment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Add a document for secondary network IPAM, and add the Antrea IPAM
option to the Multus cookbook.

Signed-off-by: Jianjun Shen <shenj@vmware.com>
@jianjuns jianjuns force-pushed the ipam-doc branch 2 times, most recently from e20b5ef to 393740c Compare April 14, 2022 20:31
Copy link
Contributor

@antoninbas antoninbas 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

/skip-all

@jianjuns jianjuns merged commit 2ab80d0 into antrea-io:main Apr 14, 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). kind/documentation Categorizes issue or PR as related to a documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants