ipv6: containerd routes support for IPv6 #15594
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XS
|
/retest |
nodeup/pkg/model/containerd.go
Outdated
| @@ -461,6 +461,13 @@ func (b *ContainerdBuilder) buildCNIConfigTemplateFile(c *fi.NodeupModelBuilderC | |||
| ] | |||
| } | |||
| ` | |||
|
|
|||
| routes := `[{ "dst": "0.0.0.0/0" }]"` | |||
There was a problem hiding this comment.
Not blocking, but I dislike putting JSON strings in code. It's more maintainable if you call json.Marshal().
There was a problem hiding this comment.
Yes, I agree and I did debate structifying this, but I don't think it has a well-defined schema, much less a go type we can import - so it would be a big change. Or I could start with routes and we can gradually expand, but then it's weird that we are mixing approaches here.
There was a problem hiding this comment.
You can use ad-hoc types like []map[string]string{{"dst":"0.0.0.0/0"}} or something like that.
justinsb
force-pushed
the
containerd_ipv6
branch
2 times, most recently
from
693e4be
to
f14eee4
Compare
k8s-ci-robot
added
size/S
|
Looks good, just need to update-expected.sh |
justinsb
force-pushed
the
containerd_ipv6
branch
2 times, most recently
from
52b9d35
to
f4ffed6
Compare
If using IPv6 and a kubenet-style CNI (which is more common with IPv6), we need to support an IPv6 route on the pod, or else Pods will be unable to reach other Pods. Co-authored-by: Ciprian Hacman <ciprian@hakman.dev>
k8s-ci-robot
added
size/M
|
We should probably have a containerdbuilder test that exercises the new functionality. |
|
@johngmyers I tried adding a test and sadly we can't get more test coverage on the IPv6 side until we tweak which CNI providers work with IPv6 - right now kubenet is blocked. I'm not sure whether we want to enable IPv6 with kubenet itself or work on a different CNI. My current approach - which I'm whittling down to something more manageable - has kops-controller setting the node networking status condition ready with IPv6, but that relies on the cloudprovider setting the providerID, which relies on the CCM, which on some clouds relies on the node being ready. That might be an issue with the CCM providers (Hetzner has this dependency, GCE does not). We do have coverage of the IPv4 case - you can see that the spacing changed because of json.Marshal |
|
We could probably unblock /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johngmyers 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
the
approved
|
Ah, |
If using IPv6 and a kubenet-style CNI (which is more common with
IPv6), we need to support an IPv6 route on the pod, or else Pods will
be unable to reach other Pods.