Sensible defaults for Cilium Envoy Config #25901

jrajahalme · 2023-06-05T12:25:58Z

CiliumEnvoyConfig needs BPF NodePort to function properly in most cases. Enable this by default if CEC is enabled, and kubeproxy replacement is not explicitly disabled.

For this to work, helm chart is made to default to KPR=false starting on 1.14. Validation will now fail only if KPR=disabled is explicitly configured or if KPR option is not given and upgradeCompatibility is < 1.14, when KPR will default to a disabled or probe.

BPF NodePort is now enabled by default if CiliumEnvoyConfig is configured.

install/kubernetes/cilium/values.yaml

meyskens · 2023-06-05T12:43:02Z

daemon/cmd/kube_proxy_replacement.go

@@ -62,6 +62,14 @@ func initKubeProxyReplacementOptions() error {
 		return nil
 	}

+	if option.Config.KubeProxyReplacement == option.KubeProxyReplacementPartial {


can this be merged into 1 if statement? instead of a nested if?

It can, but IMO this reads better in the context, which is formed with similar if statements.

gentoo-root · 2023-06-05T12:59:16Z

install/kubernetes/cilium/templates/validate.yaml

  {{- if hasKey .Values "kubeProxyReplacement" }}
-    {{- if and (ne .Values.kubeProxyReplacement "partial") (ne .Values.kubeProxyReplacement "strict") }}
+    {{- if (eq .Values.kubeProxyReplacement "disabled") }}
      {{ fail "Ingress/Gateway API controller requires .Values.kubeProxyReplacement to be set to either 'partial' or 'strict'" }}


The error messages should also mention envoy now, shouldn't they?

Yes, changed messages to this form:

Ingress/Gateway API controller and EnvoyConfig require ...

gentoo-root · 2023-06-05T13:01:12Z

install/kubernetes/cilium/templates/cilium-configmap.yaml

@@ -50,6 +50,14 @@
  {{- $defaultKubeProxyReplacement = "disabled" -}}
 {{- end -}}

+{{- /* Default values when 1.14 was initially deployed */ -}}
+{{- if semverCompare ">=1.14" (default "1.14" .Values.upgradeCompatibility) -}}
+  {{- if or .Values.envoyConfig.enabled .Values.ingressController.enabled .Values.gatewayAPI.enabled }}


Ingress controller and gateway API now also set KPR=partial automatically after this change, right? Why are these features not mentioned in the commit message?

Fixed, thanks!

PhilipSchmid · 2023-06-05T17:10:21Z

Awesome, thank you so much for all your work!
One question: Do you think it's possible to backport this to 1.13? I guess this question mainly depends on daemon/cmd/kube_proxy_replacement.go as everything else isn't really changing existing behavior, so it should be quite safe to do so IMO.

jrajahalme · 2023-06-06T05:57:49Z

Updated docs for the new helm values.

jrajahalme · 2023-06-06T05:58:35Z

/test

jrajahalme · 2023-06-06T06:37:56Z

Added envoyConfig to spelling_wordlist.txt.

jrajahalme · 2023-06-06T06:38:25Z

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.25-kernel-4.19/436/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-4.19 so I can create one.

Then please upload the Jenkins artifacts to that issue.

jrajahalme · 2023-06-15T07:57:24Z

Rebased for merged #26005

joestringer · 2023-06-26T22:19:24Z

@jrajahalme the dependency PR is now merged for this, can you rebase?

jrajahalme · 2023-06-28T15:08:22Z

/test

jrajahalme · 2023-06-28T17:44:09Z

/test

jrajahalme · 2023-06-28T23:09:47Z

/test

kubeProxyReplacement default changes to "false" for new installs and for explicit installs with upgradeCompatibility >= 1.14. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

CiliumEnvoyConfig (also used by Ingress and GatewayAPI controllers) needs BPF NodePort to function properly in most cases. Enable BPF NodePort by default if Envoy config is enabled, and kube proxy replacement is not explicitly disabled. For this to work, helm chart is made to default to KPR=false starting on 1.14. Validation will now fail only if KPR=disabled is explicitly configured or if KPR option is not given and upgradeCompatibility is < 1.14, when KPR will default to a disabled or probe. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

jrajahalme · 2023-06-29T00:22:06Z

Had to add a check or IPSec before enabling BPF node port. This can be lifted when BPF node port works with IPSec.

jrajahalme · 2023-06-29T00:22:14Z

/test

jrajahalme requested review from a team as code owners June 5, 2023 12:25

jrajahalme requested review from youngnick, sayboras and gentoo-root June 5, 2023 12:25

sayboras reviewed Jun 5, 2023

View reviewed changes

install/kubernetes/cilium/values.yaml Outdated Show resolved Hide resolved

meyskens reviewed Jun 5, 2023

View reviewed changes

jrajahalme force-pushed the envoy-config-enable-options branch from 44d5838 to 7d775d5 Compare June 5, 2023 13:00

jrajahalme requested review from a team as code owners June 5, 2023 13:00

jrajahalme requested a review from squeed June 5, 2023 13:00

gentoo-root reviewed Jun 5, 2023

View reviewed changes

jrajahalme force-pushed the envoy-config-enable-options branch from 7d775d5 to 34ac2c6 Compare June 5, 2023 13:45

gentoo-root approved these changes Jun 5, 2023

View reviewed changes

jrajahalme force-pushed the envoy-config-enable-options branch from 34ac2c6 to b7dcaa4 Compare June 6, 2023 05:57

jrajahalme added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Jun 6, 2023

maintainer-s-little-helper bot added this to Needs backport from main in 1.13.4 Jun 6, 2023

jrajahalme force-pushed the envoy-config-enable-options branch from b7dcaa4 to 8c96db5 Compare June 6, 2023 06:37

jrajahalme requested a review from a team as a code owner June 6, 2023 06:37

jrajahalme requested a review from qmonnet June 6, 2023 06:37

joestringer added dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. and removed dont-merge/blocked Another PR must be merged before this one. labels Jun 26, 2023

jrajahalme force-pushed the envoy-config-enable-options branch from 12a3ed5 to 259964b Compare June 27, 2023 06:40

jrajahalme added the dont-merge/blocked Another PR must be merged before this one. label Jun 27, 2023

jrajahalme force-pushed the envoy-config-enable-options branch from 259964b to 192cd55 Compare June 27, 2023 06:43

jrajahalme marked this pull request as draft June 27, 2023 06:54

jrajahalme marked this pull request as ready for review June 28, 2023 15:07

jrajahalme removed the dont-merge/blocked Another PR must be merged before this one. label Jun 28, 2023

jrajahalme force-pushed the envoy-config-enable-options branch from 192cd55 to 4cae1b4 Compare June 28, 2023 17:43

jrajahalme force-pushed the envoy-config-enable-options branch from 4cae1b4 to 427c07f Compare June 28, 2023 20:20

jrajahalme added 2 commits June 29, 2023 03:18

helm: Add configmap default values for Cilium 1.14

3dc92d6

kubeProxyReplacement default changes to "false" for new installs and for explicit installs with upgradeCompatibility >= 1.14. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

jrajahalme force-pushed the envoy-config-enable-options branch from 427c07f to 24b1b0b Compare June 29, 2023 00:20

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 29, 2023

jrajahalme merged commit 7be6bc2 into cilium:main Jun 29, 2023
64 of 65 checks passed

borkmann added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Jun 29, 2023

jrajahalme mentioned this pull request Jun 30, 2023

Revert "Revert agent/helm: Deprecate --kpr=partial|strict|disabled and use --kpr=true|false instead" #26496

Merged

joamaki mentioned this pull request Jul 5, 2023

v1.14 Backports 2023-07-05 #26636

Merged

23 tasks

joamaki added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 5, 2023

jibi added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jul 10, 2023

joestringer mentioned this pull request Jul 17, 2023

Prepare for release v1.14.0-rc.1 #26854

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sensible defaults for Cilium Envoy Config #25901

Sensible defaults for Cilium Envoy Config #25901

jrajahalme commented Jun 5, 2023 •

edited

meyskens Jun 5, 2023

jrajahalme Jun 5, 2023

gentoo-root Jun 5, 2023

jrajahalme Jun 5, 2023

gentoo-root Jun 5, 2023

jrajahalme Jun 5, 2023

PhilipSchmid commented Jun 5, 2023

jrajahalme commented Jun 6, 2023

jrajahalme commented Jun 6, 2023

jrajahalme commented Jun 6, 2023

jrajahalme commented Jun 6, 2023 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

jrajahalme commented Jun 15, 2023

joestringer commented Jun 26, 2023

jrajahalme commented Jun 28, 2023

jrajahalme commented Jun 28, 2023

jrajahalme commented Jun 28, 2023

jrajahalme commented Jun 29, 2023

jrajahalme commented Jun 29, 2023

Sensible defaults for Cilium Envoy Config #25901

Sensible defaults for Cilium Envoy Config #25901

Conversation

jrajahalme commented Jun 5, 2023 • edited

meyskens Jun 5, 2023

Choose a reason for hiding this comment

jrajahalme Jun 5, 2023

Choose a reason for hiding this comment

gentoo-root Jun 5, 2023

Choose a reason for hiding this comment

jrajahalme Jun 5, 2023

Choose a reason for hiding this comment

gentoo-root Jun 5, 2023

Choose a reason for hiding this comment

jrajahalme Jun 5, 2023

Choose a reason for hiding this comment

PhilipSchmid commented Jun 5, 2023

jrajahalme commented Jun 6, 2023

jrajahalme commented Jun 6, 2023

jrajahalme commented Jun 6, 2023

jrajahalme commented Jun 6, 2023 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

jrajahalme commented Jun 15, 2023

joestringer commented Jun 26, 2023

jrajahalme commented Jun 28, 2023

jrajahalme commented Jun 28, 2023

jrajahalme commented Jun 28, 2023

jrajahalme commented Jun 29, 2023

jrajahalme commented Jun 29, 2023

jrajahalme commented Jun 5, 2023 •

edited

jrajahalme commented Jun 6, 2023 •

edited by maintainer-s-little-helper bot