Add a RoutingMode option to cniConfig #6766

meyskens · 2023-10-04T08:49:05Z

Description of changes: This adds two routing modes for CIlium to use in the CNI configuration. Tunneled keeps the default geneve tunnel mode.
Direct adds a no tunnel mode with native as routing mode

Documentation added/planned (if applicable): Added a part explaining the option and linking to Cilium documentation to the cni.md file.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This adds two routing modes for CIlium to use in the CNI configuration. Tunneled keeps the default geneve tunnel mode. Direct adds a no tunnel mode with native as routing mode Signed-off-by: Maartje Eyskens <[email protected]>

eks-distro-bot · 2023-10-04T08:49:17Z

Hi @meyskens. Thanks for your PR.

I'm waiting for a aws member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

chrisdoherty4 · 2023-10-04T12:39:19Z

Cc @jaxesn

codecov · 2023-10-04T12:44:55Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (0f10c3c) 75.67% compared to head (c45992c) 71.58%.
Report is 202 commits behind head on main.

Files	Patch %	Lines
pkg/api/v1alpha1/cluster_types.go	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6766      +/-   ##
==========================================
- Coverage   75.67%   71.58%   -4.09%     
==========================================
  Files         475      545      +70     
  Lines       38411    42362    +3951     
==========================================
+ Hits        29066    30324    +1258     
- Misses       7733    10345    +2612     
- Partials     1612     1693      +81

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chrisdoherty4 · 2023-10-04T12:53:10Z

pkg/networking/cilium/templater.go

@@ -237,6 +237,11 @@ func templateValues(spec *cluster.Spec, versionsBundle *cluster.VersionsBundle)
 		val["egressMasqueradeInterfaces"] = spec.Cluster.Spec.ClusterNetwork.CNIConfig.Cilium.EgressMasqueradeInterfaces
 	}

+	if spec.Cluster.Spec.ClusterNetwork.CNIConfig.Cilium.RoutingMode == anywherev1.CiliumRoutingModeDirect {
+		val["tunnel"] = "disabled"
+		val["routingMode"] = "native"


EKS-A uses Cilium v1.12 and will be transitioning to v1.13. Neither of these have the routingMode option so I don't think this will be successful.

Should we also expose ipv4NativeRoutingCIDR from the EKS-A API? Its not totally clear to me what that option does. From the docs:

Allows to explicitly specify the IPv4 CIDR for native routing. When specified, Cilium assumes networking for this CIDR is preconfigured and hands traffic destined for that range to the Linux network stack without applying any SNAT. Generally speaking, specifying a native routing CIDR implies that Cilium can depend on the underlying networking stack to route packets to their destination. To offer a concrete example, if Cilium is configured to use direct routing and the Kubernetes CIDR is included in the native routing CIDR, the user must configure the routes to reach pods, either manually or by setting the auto-direct-node-routes flag.

I'm struggling to make sense of the example.

You're right the flag only got added in 1.14, i had the wrong branch open. I have added the NativeRouting prefix to the config and set the auto direct mode accordingly

chrisdoherty4 · 2023-10-04T12:58:11Z

pkg/networking/cilium/templater.go

@@ -237,6 +237,11 @@ func templateValues(spec *cluster.Spec, versionsBundle *cluster.VersionsBundle)
 		val["egressMasqueradeInterfaces"] = spec.Cluster.Spec.ClusterNetwork.CNIConfig.Cilium.EgressMasqueradeInterfaces
 	}

+	if spec.Cluster.Spec.ClusterNetwork.CNIConfig.Cilium.RoutingMode == anywherev1.CiliumRoutingModeDirect {
+		val["tunnel"] = "disabled"
+		val["routingMode"] = "native"


@g-gaston I'm wondering if we want to expose autoDirectNodeRoutes also which, when all nodes are connected on the same L2, means route propagation is handled by Cilium. More info at https://docs.cilium.io/en/v1.12/concepts/networking/routing/#native-routing.

Yeah, as long as we have a real use case for it and we understand its implications (just so we can supports folks using it in the field), we can expose it
Personally I would prefer to roll these slowly so we can have time to learn them better, but if someone needs this right now, as I said, we can for it
Let's sync offline

Given the time this has been open I think we can add the autoDirectNodeRoutes` later - this still has value.

Signed-off-by: Maartje Eyskens <[email protected]>

chrisdoherty4 · 2023-11-16T21:15:56Z

/lgtm

chrisdoherty4 · 2023-11-16T21:30:33Z

/approve

eks-distro-bot · 2023-11-16T21:30:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrisdoherty4

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:

~~OWNERS~~ [chrisdoherty4]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chrisdoherty4 · 2023-11-18T22:18:54Z

/test eks-anywhere-presubmit

chrisnegus

Just a couple of nits from me.

config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml

chrisdoherty4 · 2023-12-12T16:05:45Z

/lgtm

Add a RoutingMode option to cniConfig

82bbbd0

This adds two routing modes for CIlium to use in the CNI configuration. Tunneled keeps the default geneve tunnel mode. Direct adds a no tunnel mode with native as routing mode Signed-off-by: Maartje Eyskens <[email protected]>

eks-distro-bot added the needs-ok-to-test label Oct 4, 2023

github-actions bot added area/docs Documentation documentation and removed needs-ok-to-test labels Oct 4, 2023

eks-distro-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 4, 2023

chrisdoherty4 requested review from chrisdoherty4 and g-gaston October 4, 2023 12:31

chrisdoherty4 requested a review from csplinter October 4, 2023 12:39

chrisdoherty4 reviewed Oct 4, 2023

View reviewed changes

Documet the routingmode flag for Cilium

72e8589

Signed-off-by: Maartje Eyskens <[email protected]>

meyskens force-pushed the add-ciliun-routing-mode branch from 8749045 to 72e8589 Compare October 9, 2023 11:06

eks-distro-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 9, 2023

chrisdoherty4 self-assigned this Nov 16, 2023

chrisdoherty4 approved these changes Nov 16, 2023

View reviewed changes

eks-distro-bot added the lgtm label Nov 16, 2023

eks-distro-bot added the approved label Nov 16, 2023

chrisnegus reviewed Nov 24, 2023

View reviewed changes

config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml Outdated Show resolved Hide resolved

config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml Outdated Show resolved Hide resolved

config/crd/bases/anywhere.eks.amazonaws.com_clusters.yaml Show resolved Hide resolved

eks-distro-bot removed the lgtm label Dec 12, 2023

chrisdoherty4 force-pushed the add-ciliun-routing-mode branch from 56be799 to bf112fb Compare December 12, 2023 14:00

Update API comments for native routing

c45992c

chrisdoherty4 force-pushed the add-ciliun-routing-mode branch from bf112fb to c45992c Compare December 12, 2023 15:21

eks-distro-bot added the lgtm label Dec 12, 2023

eks-distro-bot merged commit 91693d0 into aws:main Dec 12, 2023
14 checks passed

cxbrowne1207 mentioned this pull request Dec 15, 2023

Fix controller generating cilium manifests with registry mirror #7170

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a RoutingMode option to cniConfig #6766

Add a RoutingMode option to cniConfig #6766

meyskens commented Oct 4, 2023

eks-distro-bot commented Oct 4, 2023

chrisdoherty4 commented Oct 4, 2023 •

edited

Loading

codecov bot commented Oct 4, 2023 •

edited

Loading

chrisdoherty4 Oct 4, 2023 •

edited

Loading

meyskens Oct 9, 2023

chrisdoherty4 Oct 4, 2023

g-gaston Oct 6, 2023

chrisdoherty4 Nov 16, 2023 •

edited

Loading

chrisdoherty4 commented Nov 16, 2023

chrisdoherty4 commented Nov 16, 2023

eks-distro-bot commented Nov 16, 2023

chrisdoherty4 commented Nov 18, 2023

chrisnegus left a comment

chrisdoherty4 commented Dec 12, 2023

Add a RoutingMode option to cniConfig #6766

Add a RoutingMode option to cniConfig #6766

Conversation

meyskens commented Oct 4, 2023

eks-distro-bot commented Oct 4, 2023

chrisdoherty4 commented Oct 4, 2023 • edited Loading

codecov bot commented Oct 4, 2023 • edited Loading

Codecov Report

chrisdoherty4 Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

meyskens Oct 9, 2023

Choose a reason for hiding this comment

chrisdoherty4 Oct 4, 2023

Choose a reason for hiding this comment

g-gaston Oct 6, 2023

Choose a reason for hiding this comment

chrisdoherty4 Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

chrisdoherty4 commented Nov 16, 2023

chrisdoherty4 commented Nov 16, 2023

eks-distro-bot commented Nov 16, 2023

chrisdoherty4 commented Nov 18, 2023

chrisnegus left a comment

Choose a reason for hiding this comment

chrisdoherty4 commented Dec 12, 2023

chrisdoherty4 commented Oct 4, 2023 •

edited

Loading

codecov bot commented Oct 4, 2023 •

edited

Loading

chrisdoherty4 Oct 4, 2023 •

edited

Loading

chrisdoherty4 Nov 16, 2023 •

edited

Loading