Implement network plugin #7947
Implement network plugin #7947
Conversation
|
Hi @henry118. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
henry118
force-pushed
the
ns3
branch
2 times, most recently
from
a4d65a3
to
04ce65c
Compare
henry118
force-pushed
the
ns3
branch
13 times, most recently
from
99b6d71
to
de53df8
Compare
henry118
force-pushed
the
ns3
branch
2 times, most recently
from
8ebc6ca
to
55917ce
Compare
Signed-off-by: Henry Wang <henwang@amazon.com>
|
@dmcgowan @mikebrow @MikeZappa87 PTAL, thanks! |
|
@henry118 excited to give this a review! This is blog worthy |
| pluginName = "cni" | ||
| ) | ||
|
|
||
| if os.Getenv("ENABLE_NETWORK_SRV") == "" { |
There was a problem hiding this comment.
I believe that this is inspired by ENABLE_CRI_SANDBOXES. But do you think we cannot make this stable enough before releasing 1.7.0? My worry is that nobody use this if that is opt-in.
| } | ||
|
|
||
| // newCNINetConfSyncer creates cni network conf syncer. | ||
| func newCNINetConfSyncer(confDir string, netPlugin cni.CNI, loadOpts []cni.Opt) (*cniNetConfSyncer, error) { | ||
| func newCNINetConfSyncer(confDir string, netPlugin compat.CNI, loadOpts []compat.LoadOpt) (*cniNetConfSyncer, error) { |
There was a problem hiding this comment.
Would we eventually use github.com/containerd/containerd/pkg/net instead of github.com/containerd/containerd/pkg/net/compat?
compat.CNI is fine, but types like compat.LoadOpt, compat.API don't tell the intention much. So it may be better to rename the package or merge that into net.
|
|
||
| // cniAdaptor is created to adapt the APIs of network plugin to their | ||
| // counterparts in go-cni. | ||
| type cniAdaptor struct { |
There was a problem hiding this comment.
The adapter is for both g and c. Isn't cni.CNI and compat.CNI mostly compatible?
|
|
||
| var dopts []cni.Opt | ||
| for _, o := range opts { | ||
| name := getFunctionName(o) |
There was a problem hiding this comment.
I'd like to avoid reflection if possible. Why do we need to know the name?
| "manager": a.Manager(), | ||
| "network": a.Network(), | ||
| "id": a.ID(), | ||
| }).Debugf("remove") |
| if ifname, ok := n.Labels()["ifname"]; ok { | ||
| dopts = append(dopts, net.WithIFName(ifname)) | ||
| } | ||
| go c.asynchAttach(ctx, i, n, &wg, rc, dopts...) |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Implements network service plugin
This is patch that implements the network service plugin proposed by #7751
The main logic is implemented in
pkg/net. In order to ease the integration with the existing CRI code,pkg/net/compatwas written to provide similar APIs asgo-cnipackage.netapi_adaptor.gowas created in CRI to wrap the underlying implementations.This implementation does not change
go-cni, and therefore limits its current scope to CRI code base. But the downside is that it contains similar codes that handles network configuration and results as ingo-cni.Signed-off-by: Henry Wang henwang@amazon.com