Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add options for configuring IPv4 and IPv6 support with Calico #11688

Merged
merged 1 commit into from
Jun 5, 2021

Conversation

hakman
Copy link
Member

@hakman hakman commented Jun 4, 2021

Sample config to get started with AWS, Calico and IPv6, using a kOps cluster with public topology:

  kubeAPIServer:
    bindAddress: "::"
  nonMasqueradeCIDR: fd00:10:96::/64
  serviceClusterIPRange: fd00:10:96::/108
  kubeControllerManager:
    clusterCIDR: fd00:10:96::/80
    allocateNodeCIDRs: false
  kubeDNS:
    provider: CoreDNS
    upstreamNameservers:
    - 2620:119:35::35
    - 2620:119:53::53
  networking:
    calico:
      ipv4Support: false
      ipv6Support: true
      awsSrcDstCheck: Disable
  subnets:
   - type: Public
     ipv6CIDR: 2a05:d014:...::/64

Ref: #8432

/cc @justinsb @johngmyers @aojea

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/addons area/api labels Jun 4, 2021
@aojea
Copy link
Member

aojea commented Jun 4, 2021

is there ipv6 CI job?

@hakman
Copy link
Member Author

hakman commented Jun 4, 2021

is there ipv6 CI job?

Not yet. There are a few things left on another PR and after that will try to get a CI job to run.

@hakman
Copy link
Member Author

hakman commented Jun 4, 2021

/retest

Copy link
Member

@johngmyers johngmyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, but it would be good if we could automatically set the IPv*Support fields based on a cluster-level setting as to whether the cluster is v4-only, dual-stack, or v6-only.

{{- if (WithDefaultBool .Networking.Calico.IPv6Support false) }}
- name: CALICO_IPV6POOL_CIDR
value: "{{ .KubeControllerManager.ClusterCIDR }}"
- name: CALICO_IPV6POOL_NAT_OUTGOING
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to NAT outgoing IPv6? Could we not assign a routable CIDR to the pool? Or does that require mucking about with ENI mode?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few considerations here:

  • default behavior for Calico is to do outgoing NAT
  • this is a setting that will be applied for the default IPv6 Pool created on first start.
  • I don't see how assigning a routable CIDR would be possible until AWS adds a routable IPv6 CIDR blocks to each node.
    With these in mind, seems a safe default for now. It can easily be changed after initial start and, once there is some way to use routable CIRDS, it can be split into a separate option.

@@ -84,7 +84,7 @@ data:
ttl 30
}
prometheus :9153
forward . /etc/resolv.conf {
forward . {{ or (join KubeDNS.UpstreamNameservers " ") "/etc/resolv.conf" }} {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an unrelated change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, missing feature from Coredns that you need in order to not muck with the node-level DNS? This should perhaps be a separate PR.

Copy link
Member Author

@hakman hakman Jun 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about making this a separate PR, but it was a one liner, related to current work.
As AWS DNS servers don't support IPv6, this was the easiest way to get CoreDNS to work.

@@ -3857,8 +3861,17 @@ spec:
# The default IPv4 pool to create on startup if none exists. Pod IPs will be
# chosen from this range. Changing this value after installation will have
# no effect. This should fall within `--cluster-cidr`.
{{- if (WithDefaultBool .Networking.Calico.IPv6Support false) }}
- name: CALICO_IPV6POOL_CIDR
value: "{{ .KubeControllerManager.ClusterCIDR }}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://kubernetes.io/docs/concepts/services-networking/dual-stack/ the KCM --cluster-cidr is, in dual-stack, an IPv4 CIDR and an IPv6 CIDR, separated by a comma. It doesn't look like calico/node accepts that syntax in either CALICO_IPV6POOL_CIDR or CALICO_IPV4POOL_CIDR.

I think we'd need a template function to parse ClusterCIDR into its two components.

Copy link
Member

@johngmyers johngmyers Jun 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm okay with not supporting dual-stack in the ClusterCIDR. We should probably have API validation around that, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, single stack ClusterCIDR is ok from my point of view too.
Dual-stack is also blocked by kubernetes/cloud-provider-aws#230, as the kubelet expects the cloud provider to return both IPv4 and IPv6 for the node.

@johngmyers
Copy link
Member

I think the only thing blocking is API validation to prohibit the combination of Calico and dual-stack ClusterCIDR.

@johngmyers
Copy link
Member

Okay, existing ClusterCIDR validation prohibits multiple CIDRs.

@johngmyers johngmyers self-requested a review June 5, 2021 22:24
Copy link
Member

@johngmyers johngmyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these comments can be followup work.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2021
@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2021
@k8s-ci-robot k8s-ci-robot merged commit 7001de3 into kubernetes:master Jun 5, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jun 5, 2021
@johngmyers
Copy link
Member

Suggest API validation to prohibit enabling both IPv4 and IPv6 for Calico.

@johngmyers
Copy link
Member

Hrm, perhaps a single, tri-state setting? With values 'ipv4', 'ipv6', 'dual', default 'ipv4'.

@johngmyers
Copy link
Member

Or key off of how ClusterCIDR parses?

@hakman hakman deleted the ipv6-calico branch June 6, 2021 02:53
@hakman
Copy link
Member Author

hakman commented Jun 6, 2021

Or key off of how ClusterCIDR parses?

I think this may be a good way to pull the defaults, but would sill allow overriding for experiments.

@hakman
Copy link
Member Author

hakman commented Jun 6, 2021

In other news, I ran the Conformance tests manually on a fresh cluster and got the following failures:

[Fail] [sig-network] Services [It] should have session affinity work for NodePort service [LinuxOnly] [Conformance]
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/service.go:203

[Fail] [sig-network] Services [It] should be able to switch session affinity for NodePort service [LinuxOnly] [Conformance]
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/service.go:203

[Fail] [sig-network] EndpointSlice [It] should create Endpoints and EndpointSlices for Pods matching a Service [Conformance]
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/endpointslice.go:320

[Fail] [sig-network] HostPort [It] validates that there is no conflict between pods with same hostPort but different hostIP and protocol [LinuxOnly] [Conformance]
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes/runner.go:113

[Fail] [sig-network] Services [It] should have session affinity timeout work for NodePort service [LinuxOnly] [Conformance]
/workspace/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/network/service.go:203

These are happening because the tests try to connect to the IPv4 address of the service exposed via NodePort. If the tests would connect to the IPv6 address instead, they would pass (tested by setting --node-ip=<IPv6-Address> and using a custom cloud provider binary). For some reason, using --node-ip=:: did not help.

The cloud provider also only provides the IPv4 address for the node, so this is an expected failure.

@johngmyers
Copy link
Member

In retrospect, there shouldn't be IPv4Support and IPv6Support fields in the CalicoNetworkingSpec. The templates should key off of something that derives from whether the cluster has an IPv4 or IPv6 (or both) ClusterCIDR.

@hakman
Copy link
Member Author

hakman commented Jun 6, 2021

I agree, but this may allow some use cases like some fake pod dual-stack that works at node level.
Also, those are the default IP pools and what kOps does is just give a simple way to set the defaults correctly. Advanced users will do a lot more magic there.
So, the fields should be ok to exist, as overrides for advanced users, but the settings should default to the IP family of what's set in ClusterCIDR.

@johngmyers
Copy link
Member

The Calico templates don't support dual-stack. For example, CALICO_ROUTER_ID should not be set to "hash" if it's dual-stack.

Advanced configurations will undoubtedly require code to configure all the things needed for the particular use case.

The fields don't make sense to expose. They just clutter up the API.

@johngmyers
Copy link
Member

I think we might need code in pkg/model/kubelet.go to set --node-ip correctly in the IPv6 case. There's that b.UsesSecondaryIP() case there that currently assumes IPv4.

@hakman
Copy link
Member Author

hakman commented Jun 6, 2021

The Calico templates don't support dual-stack. For example, CALICO_ROUTER_ID should not be set to "hash" if it's dual-stack.

This is more of a quote from the docs which are lacking a lot of details when it comes to IPv6. "hash" means a CRC32 of the hostname, which contains the IP. It has nothing to do with how dual-stack is configured in Calico.

Calico allows lots of config using calicoctl, outside of the daemonset template, including creating new IP pools. kOps only configures the default IP pool, which is based on ClusterCIDR.

I think we might need code in pkg/model/kubelet.go to set --node-ip correctly in the IPv6 case. There's that b.UsesSecondaryIP() case there that currently assumes IPv4.

I am aware of that option and will take it into consideration, weather to support it or consider it mutually exclusive with IPv6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/addons area/api cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants