Skip to content

Conversation

jrajahalme
Copy link
Member

Fail helm if kube-proxy-replacement is set or defaults to an invalid value.

kube-proxy-replacement can be defaulted to a deprecated (and since removed) "probe" value. User can also set it into an incorrect value explicitly. It is better to fail on helm than cilium agent failing to start.

@jrajahalme jrajahalme added area/daemon Impacts operation of the Cilium daemon. release-note/misc This PR makes changes that have no direct user impact. area/helm Impacts helm charts and user deployment experience labels Jun 5, 2023
@jrajahalme jrajahalme requested review from a team as code owners June 5, 2023 15:04
@jrajahalme jrajahalme requested review from youngnick, sayboras and brb June 5, 2023 15:04
@jrajahalme jrajahalme force-pushed the helm-validate-kpr-value branch from 57945b2 to 292ed3e Compare June 5, 2023 15:05
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -54,6 +54,9 @@
{{- $bpfCtTcpMax := (coalesce .Values.bpf.ctTcpMax $defaultBpfCtTcpMax) -}}
{{- $bpfCtAnyMax := (coalesce .Values.bpf.ctAnyMax $defaultBpfCtAnyMax) -}}
{{- $kubeProxyReplacement := (coalesce .Values.kubeProxyReplacement $defaultKubeProxyReplacement) -}}
{{- if and (ne $kubeProxyReplacement "disabled") (ne $kubeProxyReplacement "partial") (ne $kubeProxyReplacement "strict") }}
Copy link
Member

Choose a reason for hiding this comment

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

any reason to have this check here instead of validation.yaml? I assume the $defaultKubeProxyReplacement is always having valid value.

Copy link
Member Author

Choose a reason for hiding this comment

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

$defaultKubeProxyReplacement is computed only in this file, so I'm assuming it is not having the same values in validation.yaml, or at least that seems to be the case.

@jrajahalme
Copy link
Member Author

/test

@jrajahalme
Copy link
Member Author

Travis hit a known flake #24678

@jrajahalme
Copy link
Member Author

/ci-l4lb

@jrajahalme
Copy link
Member Author

ci-l4lb fails for a verifier fail #25892, needs a rebase to pick up a fix in main.

Fail helm if kube-proxy-replacement is set or defaults to an invalid value.

kube-proxy-replacement can be defaulted to a deprecated (and since
removed) "probe" value. User can also set it into an incorrect value
explicitly. It is better to fail on helm than cilium agent failing to
start.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the helm-validate-kpr-value branch from 292ed3e to c824261 Compare June 6, 2023 06:33
@jrajahalme
Copy link
Member Author

/test

@maintainer-s-little-helper 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 6, 2023
@jrajahalme jrajahalme merged commit f64e073 into cilium:main Jun 6, 2023
@YutaroHayakawa YutaroHayakawa mentioned this pull request Jun 7, 2023
4 tasks
@YutaroHayakawa YutaroHayakawa mentioned this pull request Jun 8, 2023
3 tasks
@YutaroHayakawa YutaroHayakawa mentioned this pull request Jun 8, 2023
2 tasks
@YutaroHayakawa YutaroHayakawa added backport-pending/1.11 backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed needs-backport/1.11 labels Jun 8, 2023
@YutaroHayakawa YutaroHayakawa added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.11 labels Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. area/helm Impacts helm charts and user deployment experience backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

5 participants