add CNI bridge network provider #4352
Conversation
tonistiigi
force-pushed
the
cni-bridge
branch
2 times, most recently
from
16c853b to
e895f73
Compare
cmd/buildkitd/config/config.go
Outdated
| @@ -71,6 +71,8 @@ type NetworkConfig struct { | |||
| CNIConfigPath string `toml:"cniConfigPath"` | |||
| CNIBinaryPath string `toml:"cniBinaryPath"` | |||
| CNIPoolSize int `toml:"cniPoolSize"` | |||
| BridgeName string `toml:"bridgeName"` | |||
| // BridgeSubnet string `toml:"bridgeSubnet"` | |||
There was a problem hiding this comment.
@AkihiroSuda @jedevc If we add #4099 then I think we need to add some reserved IP ranges that the tunnels created via LLB should use. If this BridgeSubnet collides with such ranges we could give an error and buildkitd would not start. Otherwise, if user would use an IP that collides with the configured bridge then it would not be possible to create such tunnels. Does this make sense?
| ] | ||
| } | ||
| } | ||
| ] |
There was a problem hiding this comment.
Can we have ingressPolicy": "same-bridge" ?
https://www.cni.dev/plugins/current/meta/firewall/#ingress-policy
There was a problem hiding this comment.
So this would be another plugin we would need to include?
util/network/cniprovider/bridge.go
Outdated
| "type": "%s", | ||
| "ranges": [ | ||
| [ | ||
| { "subnet": "10.10.0.0/16" } |
There was a problem hiding this comment.
This should be customizable
There was a problem hiding this comment.
Yes, see #4352 (comment)
|
@gabriel-samfira What approach we should take for wcow with this? Just disallow for now? I see there is also https://github.com/containernetworking/plugins/tree/main/plugins/main/windows/win-bridge but I don't know much about it. |
tonistiigi
force-pushed
the
cni-bridge
branch
2 times, most recently
from
ee0c9fd to
feaf181
Compare
Sorry for the late reply. Windows container networking lives here: https://github.com/microsoft/windows-container-networking The But if the goal is to permit basic network connectivity, I'd recommend using the |
|
@gabriel-samfira I think we can cover these in a follow-up if necessary as looks like completely separate code would be needed anyway. You can take a look at #4099 as well to see what the approach for wcow could be for that. |
| nc.BridgeName = "buildkit0" | ||
| } | ||
| if nc.BridgeSubnet == "" { | ||
| nc.BridgeSubnet = "10.10.0.0/16" |
There was a problem hiding this comment.
In a follow up PR, could you add a documentation to explain this default CIDR and other configuration?
Dockerfile
Outdated
| COPY --link --from=cni-plugins /opt/cni/bin/bridge /buildkit-cni-bridge | ||
| COPY --link --from=cni-plugins /opt/cni/bin/loopback /buildkit-cni-loopback | ||
| COPY --link --from=cni-plugins /opt/cni/bin/host-local /buildkit-cni-host-local | ||
| COPY --link --from=cni-plugins /opt/cni/bin/firewall /buildkit-firewall |
There was a problem hiding this comment.
| COPY --link --from=cni-plugins /opt/cni/bin/firewall /buildkit-firewall | |
| COPY --link --from=cni-plugins /opt/cni/bin/firewall /buildkit-cni-firewall |
for consistency
This adds a new network configuration based on CNI bridge. Unlike the existing CNI provider this does not require user to provide CNI configuration and plugins externally. The minimal set of plugins required to use the network mode is provided together with buildkit. Currently, bridge mode is opt-in but intention is to make it default after a testing period of one release cycle. Signed-off-by: Tonis Tiigi <[email protected]>
tonistiigi
force-pushed
the
cni-bridge
branch
from
feaf181 to
d4cd019
Compare
fixes #3864
This adds a new network configuration based on CNI bridge. Unlike the existing CNI provider this does not require the user to provide CNI configuration and plugins externally.
The minimal set of plugins required to use the network mode is provided together with buildkit. Currently, bridge mode is opt-in, but the intention is to make it default after a testing period of one release cycle.
Unlike the CNI provider, if the bridge interface was created by buildkitd, it is cleaned up automatically when buildkitd is shut down.