-
Notifications
You must be signed in to change notification settings - Fork 373
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 cookbook on how to use Antrea with Multus #1223
Add cookbook on how to use Antrea with Multus #1223
Conversation
Thanks for your PR. The following commands are available:
|
Codecov Report
@@ Coverage Diff @@
## master #1223 +/- ##
==========================================
- Coverage 54.99% 54.96% -0.04%
==========================================
Files 110 110
Lines 10573 10573
==========================================
- Hits 5815 5811 -4
- Misses 4185 4187 +2
- Partials 573 575 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
500ed5c
to
fa00318
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.
Took a quick review. Overall looks good to me. Feel a little uncertain regarding to the DHCP daemon Docker image.
docs/cookbooks/multus/README.md
Outdated
All the required software will be deployed using YAML manifests, and the | ||
corresponding container images will be downloaded from public registries. | ||
|
||
macvlan requires the network to be able to handle "promiscuous mode", as the |
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.
I feel this description is not accurate, for promisc mode is configured on the NIC device, but not on the network, and if the underlay network supports MAC learning promisc mode NICs should work fine.
But I know some hypervisor networking requires promisc mode settings on the vSwitch port, and other documentation describes it the same way, so probably not a big deal.
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.
And this requirement should be for macvlan bridge mode only?
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.
I clarified by mentioning that this is for macvlan "bridge" mode.
Regarding your first comment, I don't think the following is too misleading:
macvlan requires the network to be able to handle "promiscuous mode"
A lot of pieces can be involved:
- hypervisor
- physical NIC
- underlying network: won't work on public cloud (as you said it needs to support MAC learning)
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.
Not disagree what you said.
docs/cookbooks/multus/README.md
Outdated
### Step 1: Deploying Antrea | ||
|
||
```bash | ||
kubectl apply -f https://github.com/vmware-tanzu/antrea/releases/download/v0.9.2/antrea.yml |
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.
Would you consider ToT of the master?
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.
Done
|
||
```bash | ||
docker build -t antrea/cni-dhcp-daemon:latest . | ||
docker push antrea/cni-dhcp-daemon:latest |
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.
Not sure if it is too much for us to maintain a DHCP daemon image to demonstrate Multus.
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.
I don't believe this will be a maintenance burden. I can always move the image to my personal Github / Dockerhub accounts, but I don't think this is a better solution. I couldn't find a good existing image anywhere either.
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.
Ok. No strong opinion on this.
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.
A few extra comments.
docs/cookbooks/multus/README.md
Outdated
* when using VMware Fusion, enable "promiscuous mode" in the guest (Node) for | ||
the appropriate interface (e.g. using `ifconfig`); this may prompt for your | ||
password on the host unless you uncheck `Require authentication to enter | ||
promiscuous mode` in the Network Preferences |
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.
Network Preferences -> Network Preferences
?
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.
Replaced with the actual menu names: Preferences ... > Network
docs/cookbooks/multus/README.md
Outdated
This manifest will create a DaemonSet that will run a bash script once on every | ||
Node. It will: | ||
* Enable promiscuous mode on the parent interface using `ifconfig`; if using a | ||
virtual network for the Nodes, this does not replace enabling promiscuous |
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.
I am not sure I understood this sentence. Would you consider rephrasing?
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.
Hopefully it's clearer now, especially with the pointer to the Prerequisites section
@@ -0,0 +1,1931 @@ | |||
<?xml version="1.0" encoding="UTF-8" standalone="no"?> |
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 you think better to add "br-int" to the OVS bridge box?
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.
Done, thanks for pointing this out, I don't know how I missed adding this the first time
We add documentation to show how Antrea can be used with Multus: Antrea is used as the default CNI plugin and an "arbitrary" plugin (in our case, macvlan) can be used to attach additional interfaces to designated Pods. Nothing is required on the Antrea side to make it work, so this is just to show how it can be used in practice. Fixes antrea-io#368
3fa7a6f
to
e5d58ff
Compare
e5d58ff
to
299985a
Compare
/skip-all |
We add documentation to show how Antrea can be used with Multus: Antrea
is used as the default CNI plugin and an "arbitrary" plugin (in our
case, macvlan) can be used to attach additional interfaces to designated
Pods. Nothing is required on the Antrea side to make it work, so this is
just to show how it can be used in practice.
Fixes #368