ipFamilyPolicy:PreferDualStack for kube-dns and metrics-server #8394
Conversation
manuelbuil
force-pushed
the
preferDS
branch
from
01f2afd
to
163607d
Compare
|
Should we do this for traefik as well? https://github.com/k3s-io/k3s-charts/tree/main-source/packages/traefik |
manuelbuil
force-pushed
the
preferDS
branch
from
163607d
to
2084b3d
Compare
traefik is already doing it correctly: https://github.com/k3s-io/k3s/blob/master/manifests/traefik.yaml#L40-L41 |
|
Ah great, I forgot we were injecting that here instead of in the chart patches |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #8394 +/- ##
==========================================
+ Coverage 47.43% 51.47% +4.04%
==========================================
Files 143 143
Lines 14741 14826 +85
==========================================
+ Hits 6992 7632 +640
+ Misses 6643 5970 -673
- Partials 1106 1224 +118
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
So will kube-dns get a static IPv6 address (like |
I don't believe that's true. It accepts a list, just like the cluster CIDR and service CIDR flags. |
Correct. This PR only changes the behavior in dual-stack mode. In that mode, the service will have two IPs (ipv4 and ipv6) |
Yes you are right. The flag usage led me to think it could only receive one IPv4 address. However it seems impossible to reserve an IPv6 address via something like Lines 204 to 217 in 6330a5b |
The service manifest has two fields: ClusterIP (string) and ClusterIPs ([]string). |
|
Yeah, we probably also need to modify the coredns manifest, and how we template in the cluster DNS address, to work properly with dualstack. Just setting the ipfamilypolicy won't work, because we're hardcoding the clusterIP to a single address family. |
|
There should also be a default IPv6 Cluster IP for kube-dns, right? According to the ServiceSpec, the Feels like there should be a default for it. |
|
base plus 10 is probably also a good default for IPv6. |
Thanks for the comments, let me try to reply to you both :). It is important to note that, at this point in k3s, when deploying on dualStack mode, IPv4 is always prioritized over IPv6. (This will change soon but in another PR). Even if you provide a dual-stack type of input to Before this PR, this is what we had with kube-dns service: IPv4-onlyDual-StackIPv6-onlyBoth When this PR is merged, this is what we will have: IPv4-onlyDual-StackIPv6-onlyI can confirm that all the 6 examples work. I can lookup hostnames on the provided clusterIPs. Up to here were facts :). Now on the Now, from a user perspective view, I'd need more information. I wonder if perhaps it is useful to know that in k3s the DNS service is always |
We need to know it ahead of time, and pass it to the kubelet, if we want the kubelet to configure dual-stack resolvers in resolv.conf. If we don't, it will only use the primary address for cluster dns. It doesn't actually look at the Kubernetes service to figure out what its addresses are. From the kubelet docs:
Maybe this is fine and we don't care about providing clients with dual-stack resolvers on dual-stack clusters, and are OK with forcing them to use just the primary ip family. |
Considering how And thanks for the detailed explanation @manuelbuil! My point of view is that for special services like kube-dns, they should have special IP addresses whenever possible (better if they are also easy to remember). Like the default kubernetes service always gets It's strange for kube-dns to get a random IP address, since it will most likely only be accessed via the bare IP address. Say someone wants to lookup hostnames on the IPv6 Cluster IP, Regardless of whether there will be a dual-stack default for the kube-dns in a dual-stack cluster, I think k3s should support customizing the kube-dns' |
|
Yeah, even if we only set the kubelet's --cluster-dns address to the primary address, I think we should still default to allocating base+10 to kube-dns for both address families. |
manuelbuil
force-pushed
the
preferDS
branch
from
2084b3d
to
4984ea2
Compare
|
Added the code pinning coredns IP address for both ip families in dualStack: Both Single stack keeps working: |
| return errors.Wrap(err, "problems calculating extraDNS") | ||
| } | ||
| serverConfig.ControlConfig.ClusterDNSs = append(serverConfig.ControlConfig.ClusterDNSs, extraDNS) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
What happens when I explicitly set --service-cidr 10.43.0.0/16,2001:cafe:42:1::/112 --cluster-dns 10.43.0.10?
Providing clusterIPs: [10.43.0.10] for the first time is ok, it will be assigned a random IPv6 address. However, could there be an error when updating the kube-dns service in the future?
From the ServiceSpec:
- clusterIPs ([]string)
...
This field may not be changed through updates unless the type field is also being changed to ExternalName (which requires this field to be empty) or the type field is being changed from ExternalName (in which case this field may optionally be specified, as describe above).
...
There was a problem hiding this comment.
This should be covered by upgrade testing, but it is good to think about.
If the apply controller doesn't handle it properly, we may have to add some additional logic to delete and re-create the service so that we can change the addresses. I don't think this should be a problem, as I believe should be allowed to change clusterIPs when switching from single-stack to dual-stack? There may be issues with existing dual-stack IPv6-primary clusters though, where we were previously using the IPv4 address as the primary, and the new code will use the IPv6 address instead.
There was a problem hiding this comment.
I think that updating important config parameters in a running cluster is something that we currently don't support in k3s. For example: cluster-dns,service-cidr, cluster-cidr or flannel-backend. If there is enough interest, we could try to design that feature and prototype it. In any case, I'd say this is out of the scope of this PR
There was a problem hiding this comment.
Ah, sorry, I think you two got me wrong.
Let's say I started a cluster with k3s server --service-cidr 10.43.0.0/16,2001:cafe:42:1::/112 --cluster-dns 10.43.0.10. Based on the current PR, the kube-dns service in the coredns.yaml will be rendered as:
clusterIP: 10.43.0.10
clusterIPs: [10.43.0.10]
ipFamilyPolicy: PreferDualStackWhich will result a Service resource similar to this:
apiVersion: v1
kind: Service
metadata:
name: kube-dns
namespace: kube-system
spec:
clusterIP: 10.43.0.10
clusterIPs:
- 10.43.0.10
- 2001:cafe:42:1::62c1 # A random IPv6 address
ipFamilyPolicy: PreferDualStackSo far so good.
Then we manually kubectl apply -f coredns.yaml again without any changes (this will happen every time k3s restarts, right?). I think we'll get this error:
The Service "kube-dns" is invalid: spec.ipFamilyPolicy: Invalid value: "PreferDualStack": must be 'SingleStack' to release the secondary cluster IP
This is because the clusterIPs field cannot be changed once set.
I know that we didn't make any changes to the coredns.yaml itself, but we were actually trying to release the secondary cluster IP (that random IPv6 address) of the kube-dns service without knowing it. Because the rendered field is always clusterIPs: [10.43.0.10].
There was a problem hiding this comment.
Thanks @aofei for the explanation, indeed I was not understanding what you meant. You have a good point. To be safe, we probably should use SingleStack for kube-dns service if cluster-dns flag is set with a singlestack value. It should be easy to code.
Moreover, if afterwards the user realizes a mistake was done and wanted dual-stack kube-dns, the user could change the value of cluster-dns adding the extra IP and restart k3s. Even if it means changing an important config on a running cluster, I think we could support this case because the main DNS IP would not change and we would just be adding a new extra IP
manuelbuil
force-pushed
the
preferDS
branch
from
4984ea2
to
215f1f6
Compare
| } | ||
|
|
||
| // Set ClusterDNS to the first IPv4 address, for legacy clients | ||
| // unless only IPv6 range given | ||
| clusterDNS, _, _, err := util.GetFirstIP(serverConfig.ControlConfig.ClusterDNSs) |
There was a problem hiding this comment.
Is this correct? I think Kubernetes might insist that the primary clusterIP for the service respect the ip family order for the cluster.
There was a problem hiding this comment.
As of today, all our code is handling this situation prioritizing ipv4 regardless of the order. I am preparing another PR to change this. In any case, it is not clear that Kubernetes mandates anything in this respect, in fact, when they describe dual-stack, IPv4 is always the first in all their examples. We can discuss further in my other PR
There was a problem hiding this comment.
BTW, I am creating a different PR because verifying the order to decide what IPFamily gets prioritized requires changes in several parts of the code
There was a problem hiding this comment.
Will this break if I set --service-cidr 2001:cafe:42:1::/112,10.43.0.0/16 --cluster-dns 2001:cafe:42:1::a,10.43.0.10?
If the kube-dns service in the coredns.yaml will be rendered as:
clusterIP: 10.43.0.10
clusterIPs: [2001:cafe:42:1::a, 10.43.0.10]
ipFamilyPolicy: PreferDualStackThen I think things will go wrong, as the ServiceSpec says:
- clusterIPs ([]string)
...
If this field is not specified, it will be initialized from the clusterIP field. If this field is specified, clients must ensure that clusterIPs[0] and clusterIP have the same value.
...
clusterIP and clusterIPs[0] must always be identical.
There was a problem hiding this comment.
As of today, all our code is expecting to define dual-stack as stated in the Kubernetes documentation:
https://kubernetes.io/docs/concepts/services-networking/dual-stack/#configure-ipv4-ipv6-dual-stack
, i.e. it expects the order to be <IPv4 CIDR>,<IPv6 CIDR>. K3s is currently not ready for configs like --service-cidr 2001:cafe:42:1::/112,10.43.0.0/16. You are right, your config will not work, but there are other things that will not work either without some workarounds: e.g. kube-api or netpol controller. I am working on another PR to provide support for <IPv6 CIDR>,<IPv4 CIDR> type of configs. I'll add you to that PR as reviewer and we can address this problem over there
There was a problem hiding this comment.
It actually has nothing to do with the order of --service-cidr. --service-cidr 10.43.0.0/16,2001:cafe:42:1::/112 --cluster-dns 2001:cafe:42:1::a,10.43.0.10 will lead to the same result.
Kind of feels like there should be a check here to make sure that --cluster-dns always matches --service-cidr (within the ranges, having the same families and family order no matter which family takes precedence). I think k3s should throw a fatal to prevent the above case from happening.
There was a problem hiding this comment.
Correction: cmds.ServerConfig.ClusterDNS should always matches cmds.ServerConfig.ServiceCIDR when len(cmds.ServerConfig.ClusterDNS) == len(cmds.ServerConfig.ServiceCIDR).
The case of --service-cidr 10.43.0.0/16,2001:cafe:42:1::/112 --cluster-dns 10.43.0.10 should still be allowed.
There was a problem hiding this comment.
I agree with all of the above. If there are places where we are forcing or expecting IPv4 as the primary AF on dual-stack clusters, we should work towards eliminating those.
On the other hand, if someone changes the cluster-dns setting after the cluster has already been started, I'm not sure how hard we need to go our of our way to handle changes not allowed by Kubernetes itself.
There was a problem hiding this comment.
In my opinion:
--service-cidr 10.43.0.0/16,2001:cafe:42:1::/112 --cluster-dns 2001:cafe:42:1::a,10.43.0.10
is currently not acceptable because cluster-dns is using the non-supported order: <IPv6><IPv4>.
Let me finish up the other PR and we can discuss <IPv6><IPv4> over there. I think I can have it in a few hours
manuelbuil
force-pushed
the
preferDS
branch
from
215f1f6
to
6795775
Compare
Signed-off-by: Manuel Buil <mbuil@suse.com>
manuelbuil
force-pushed
the
preferDS
branch
from
6795775
to
669f655
Compare
|
merged as part of #8460 |
Proposed Changes
Use
ipFamilyPolicy: PreferDualStackas default for kube-dns and metrics services. That way, those services will have one IP when using ipv4-only and ipv6-only but will pick 2 addresses (ipv4 and ipv6) when k3s is deployed in dualStack modeTypes of Changes
Bug fix
Verification
Deploy k3s in dual-stack mode and verify that the service gets two IP addresses (ipv4 and ipv6)
Testing
Linked Issues
#8393
User-Facing Change
Further Comments