-
Notifications
You must be signed in to change notification settings - Fork 386
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
[doc][flexible-ipam] Add Flexible IPAM design section #5339
Conversation
1cd19a9
to
07898b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
docs/antrea-ipam.md
Outdated
@@ -218,6 +218,34 @@ metadata: | |||
A StatefulSet Pod's IP will be kept after Pod restarts, when the IP is allocated from the | |||
annotated IPPool. | |||
|
|||
### IPAM behaviors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the main purpose and who are the target readers of this section? It looks like design and internal implementation? If so, I would suggest to move it to the last sub-section and name it IPAM design
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to explain how antrea manages IP when FlexibleIPAM feature is enabled.
Moved it to the last sub-section and name it Flexible IPAM design
.
docs/antrea-ipam.md
Outdated
#### On Pod create | ||
|
||
`antrea-agent` will receive a CNI add request, it will check annotations and allocate an IP from | ||
an IPPool. The IP is a pre-allocated StatefulSet IP, a specified IP or the next available IP in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the corresponding IPPool"
Or, probably we should remove "from an IPPool", given you have another sentence to explain the IP, which seems can be pre-allocated too.
Consider "and allocate an IP for the Pod, which can be a pre-allocated IP StatefulSet IP, a user-specified IP, or the next available IP in the specified IPPool."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IP can be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored this section to
`antrea-agent` will receive a CNI add request, and it will check the Antrea IPAM annotations and
allocate an IP for the Pod, which can be a pre-allocated IP StatefulSet IP, a user-specified IP,
or the next available IP in the specified IPPool.
docs/antrea-ipam.md
Outdated
#### On Pod delete | ||
|
||
`antrea-agent` will receive a CNI del request and release the IP allocation from the IPPool. | ||
The pre-allocation status won't be changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not understand this sentence. You probably should add more information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to:
If the IP is a pre-allocated StatefulSet IP, it will remain pre-allocated status thus the Pod will
get same IP after recreated.
07898b9
to
e43199b
Compare
e43199b
to
0ef53fb
Compare
docs/antrea-ipam.md
Outdated
### Flexible IPAM design | ||
|
||
When the `AntreaIPAM` feature gate is enabled, `antrea-controller` will watch IPPool CRs and | ||
StatefulSets from kube-apiserver, and `antrea-agent` will register the IPAM driver to CNI server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrapp kube-apiserver with ''.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider: "activate the Antrea IPAM driver". "register to CNI server" is too much internal details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added `` for kube-apiserver.
Removed
and `antrea-agent` will register the IPAM driver to CNI server
docs/antrea-ipam.md
Outdated
|
||
#### On Pod create | ||
|
||
`antrea-agent` will receive a CNI add request, and it will check the Antrea IPAM annotations and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider: "it will" -> "it will then"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
docs/antrea-ipam.md
Outdated
#### On Pod delete | ||
|
||
`antrea-agent` will receive a CNI del request and release the IP allocation from the IPPool. | ||
If the IP is a pre-allocated StatefulSet IP, it will remain pre-allocated status thus the Pod will |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"it will stay in the pre-allocated
status"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thanks.
Co-authored-by: Lan <luola@vmware.com> Signed-off-by: gran <gran@vmware.com>
0ef53fb
to
cec56df
Compare
/skip-all |
No description provided.