Add back support for kubenet style networking with containerd #10651
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
area/api
area/nodeup
approved
hakman
force-pushed
the
containerd-kubenet-style
branch
3 times, most recently
from
8cff486
to
f9aef8b
Compare
|
Test results are good when forcing kubenet style networking with containerd: |
hakman
force-pushed
the
containerd-kubenet-style
branch
5 times, most recently
from
8b66a03
to
27f81f0
Compare
cmd/kops/create_cluster.go
Outdated
| @@ -489,8 +488,8 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr | |||
| if c.ContainerRuntime != "" { | |||
| cluster.Spec.ContainerRuntime = c.ContainerRuntime | |||
| } | |||
| if c.ContainerRuntime == "containerd" && components.UsesKubenet(cluster.Spec.Networking) { | |||
| return fmt.Errorf("--networking with CNI plugin is required for containerd") | |||
| if c.ContainerRuntime == "containerd" && c.Networking == "" { | |||
There was a problem hiding this comment.
we default to networking = kubenet correct? if the default --networking value now works with containerd then we should just drop this "if" statement altogether rather than require a --networking flag.
There was a problem hiding this comment.
True. I wanted to force people to choose. Removed.
hakman
force-pushed
the
containerd-kubenet-style
branch
from
27f81f0
to
7aeb8c2
Compare
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
Just checked and the kope.io tests are running once again. The cherry-pick should be safe to merge to 1.19: |
| // buildCNIConfigTemplateFile is responsible for creating a special template for setups using Kubenet | ||
| func (b *ContainerdBuilder) buildCNIConfigTemplateFile(c *fi.ModelBuilderContext) { | ||
| contents := `{ | ||
| "cniVersion": "0.4.0", |
There was a problem hiding this comment.
This is a different configuration from the one we reverted - where did you get it from?
(I'm trying to track down the "Kubernetes e2e suite.[sig-network] Services should preserve source pod IP for traffic thru service cluster IP [LinuxOnly]" flakes)
There was a problem hiding this comment.
I proposed taking the upstream configuration in #10759
There was a problem hiding this comment.
I tried this version at some point, but more tests were failing. We can merge #10759 as an experiment as see what happens. PR tests are done with Calico.
The current config is very similar from what was reverted and was taken from :
https://github.com/containerd/containerd/blob/774cb16a446bf7210584a455b47e58d488455aab/script/setup/install-cni#L37-L68
Mostly reverting #10501.