Set Cilium as default CNI #12752

embik · 2023-10-19T06:25:22Z

What this PR does / why we need it:

This PR sets Cilium as the default CNI when creating a Cluster object and passing it through the webhook for defaulting.

Which issue(s) this PR fixes:

Fixes #

What type of PR is this?
/kind feature

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Set Cilium as default CNI for user clusters

Documentation:

NONE

embik · 2023-10-19T07:17:09Z

/retest

wozniakjan · 2023-10-19T11:51:10Z

@embik I will try some debugging here on your PR, putting it on hold so it's not accidentally merged

/hold

wozniakjan · 2023-10-19T13:23:42Z

/test pre-kubermatic-e2e-aws-ubuntu-1.28

wozniakjan · 2023-10-19T14:55:23Z

/unhold

wozniakjan · 2023-10-19T14:58:15Z

cmd/conformance-tester/pkg/runner/waiters.go

@@ -122,7 +122,7 @@ func waitUntilAllPodsAreReady(ctx context.Context, log *zap.SugaredLogger, opts
 		unready := sets.New[string]()
 		for _, pod := range podList.Items {
 			// Ignore pods failing kubelet admission (KKP #6185)
-			if !util.PodIsReady(&pod) && !podFailedKubeletAdmissionDueToNodeAffinityPredicate(&pod, log) {
+			if !util.PodIsReady(&pod) && !podFailedKubeletAdmissionDueToNodeAffinityPredicate(&pod, log) && !util.PodIsCompleted(&pod) {


here the tests expected all user cluster pods to be in a state Ready, however, hubble adds a pod that ultimately succeeds and goes to Completed state. Added a small commit 33e8f48 permitting that.

{ context deadline exceeded; last error was: not all Pods are ready: [hubble-generate-certs-lcb7z]}

kube-system hubble-generate-certs-l6x9h 0/1 Completed 0 10m

wozniakjan · 2023-10-20T06:29:17Z

now there are some conformance tests failing, I will investigate further

wozniakjan · 2023-10-20T08:20:20Z

conformance test failures:

hostPort - known issue SchedulerPredicates: pods with same hostPort but different hostIP and protocol cilium/cilium#14287
clusterIP session affinity - apparently fixable CI: session affinity failing with kube-proxy-replacement=disabled cilium/cilium#16163

our test failures:

seccomp:

kubermatic/cmd/conformance-tester/pkg/tests/usercluster_controller.go

Line 97 in 31f9faf

    
           func TestUserClusterSeccompProfiles(ctx context.Context, log *zap.SugaredLogger, opts *ctypes.Options, cluster *kubermaticv1.Cluster, userClusterClient ctrlruntimeclient.Client) error {

wozniakjan · 2023-10-20T08:22:56Z

/test pre-kubermatic-e2e-aws-ubuntu-1.28

embik · 2023-10-20T09:04:11Z

seccomp

If we can, we should configure seccomp profiles in the default Helm values we pass. Not sure if the chart allows that.

wozniakjan · 2023-10-20T09:34:30Z

pkg/cni/cilium/cilium.go

@@ -194,6 +194,11 @@ func GetAppInstallOverrideValues(cluster *kubermaticv1.Cluster, overwriteRegistr
 			// we run Cilium as non-exclusive CNI to allow for Multus use-cases
 			"exclusive": false,
 		},
+		"podSecurityContext": map[string]any{
+			"seccompProfile": map[string]any{
+				"type": "RuntimeDefault",


kkp tests expect pod seccomp profile to be set by default

kubermatic/cmd/conformance-tester/pkg/tests/usercluster_controller.go

Line 149 in 31f9faf

if pod.Spec.SecurityContext.SeccompProfile.Type == corev1.SeccompProfileTypeUnconfined {

If we can, we should configure seccomp profiles in the default Helm values we pass. Not sure if the chart allows that.

upstream chart seems to allows that, followup to #8326

and it's actually needed on all cilium pods
cilium/cilium@5aa1c23

{ function did not succeed after 3 attempts: expected seccomp profile on Pod kube-system/cilium-operator-85b8f66cd-hgkhq, got none expected seccomp profile on Pod kube-system/hubble-generate-certs-g5pl8, got none expected seccomp profile on Pod kube-system/hubble-relay-5ccf97b646-f6gnt, got none expected seccomp profile on Pod kube-system/hubble-ui-869b75b895-trq2t, got none}

configured properly in 15cd1ef

ok, hubble-ui and hubble-generate-certs doesn't have the option supported natively, with @embik we decided to skip these two as short-term remediation in
718db73 and ideally we can try to contribute upstream.

Signed-off-by: Marvin Beckers <marvin@kubermatic.com>

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

https://docs.cilium.io/en/latest/installation/cni-chaining-portmap/#portmap-hostport Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

wozniakjan · 2023-10-20T14:08:20Z

pkg/cni/cilium/cilium.go

@@ -218,6 +244,8 @@ func GetAppInstallOverrideValues(cluster *kubermaticv1.Cluster, overwriteRegistr
 		}
 	} else {
 		values["kubeProxyReplacement"] = "disabled"
+		values["sessionAffinity"] = true
+		valuesCni["chainingMode"] = "portmap"


by default, KKP runs cilium with kube-proxy, this needs portmap plugin and session affinity enabled to pass conformance tests (otherwise, HostPort is not functional)

https://docs.cilium.io/en/latest/installation/cni-chaining-portmap/#portmap-hostport

SchedulerPredicates: pods with same hostPort but different hostIP and protocol cilium/cilium#14287 (comment)

configured in 8ac82a3

wozniakjan · 2023-10-20T14:46:20Z

same case regarding integration tests as #12760 (comment)

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

wozniakjan · 2023-10-21T13:27:09Z

/unhold
/retest

kubermatic-bot · 2023-10-21T13:42:22Z

@embik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pre-kubermatic-e2e-aws-ubuntu-1.25	`2ce611d`	link	true	`/test pre-kubermatic-e2e-aws-ubuntu-1.25`

Full PR test history

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. I understand the commands that are listed here.

cnvergence · 2023-10-24T07:59:03Z

/retest

cnvergence

/approve

kubermatic-bot · 2023-10-24T08:58:53Z

LGTM label has been added.

Git tree hash: 608bee0f61d70fa962a7da7633bdfe61624d8bba

kubermatic-bot · 2023-10-24T08:58:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cnvergence

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~~ [cnvergence]
~~pkg/cni/OWNERS~~ [cnvergence]

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

embik self-assigned this Oct 19, 2023

kubermatic-bot added the sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. label Oct 19, 2023

wozniakjan force-pushed the cilium-default branch from 9e2d699 to f62ce4c Compare October 19, 2023 11:48

kubermatic-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2023

wozniakjan force-pushed the cilium-default branch from f62ce4c to aa6bcca Compare October 19, 2023 11:52

wozniakjan force-pushed the cilium-default branch 3 times, most recently from 986a3bd to 33e8f48 Compare October 19, 2023 14:45

kubermatic-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2023

wozniakjan reviewed Oct 19, 2023

View reviewed changes

kubermatic-bot added the sig/networking Denotes a PR or issue as being assigned to SIG Networking. label Oct 20, 2023

wozniakjan reviewed Oct 20, 2023

View reviewed changes

wozniakjan force-pushed the cilium-default branch from 2ce611d to 6b9d055 Compare October 20, 2023 12:01

kubermatic-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 20, 2023

wozniakjan force-pushed the cilium-default branch from 6b9d055 to c038c60 Compare October 20, 2023 12:12

wozniakjan force-pushed the cilium-default branch from c038c60 to 211fe34 Compare October 20, 2023 12:18

embik and others added 5 commits October 20, 2023 16:05

Default cluster CNI to Cilium

b34cd08

Signed-off-by: Marvin Beckers <marvin@kubermatic.com>

Update unit tests for CNI mutation

8433e97

Signed-off-by: Marvin Beckers <marvin@kubermatic.com>

conformance-tester: allow completed pods

7120015

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

cilium: set pod seccomp profile

15cd1ef

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

cilium: chain CNI hostport plugin when kubeproxy still enabled

8ac82a3

https://docs.cilium.io/en/latest/installation/cni-chaining-portmap/#portmap-hostport Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

wozniakjan force-pushed the cilium-default branch from 211fe34 to 8ac82a3 Compare October 20, 2023 14:06

kubermatic-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 20, 2023

wozniakjan reviewed Oct 20, 2023

View reviewed changes

conformance-tester: skip seccomp for hubble-ui and hubble-generate-certs

9a64627

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

wozniakjan force-pushed the cilium-default branch from 718db73 to 9a64627 Compare October 20, 2023 15:29

cnvergence approved these changes Oct 24, 2023

View reviewed changes

kubermatic-bot assigned cnvergence Oct 24, 2023

kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2023

kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2023

wozniakjan added this to the KKP 2.24 milestone Oct 24, 2023

embik added the code-freeze-approved Indicates a PR has been approved by release managers during code freeze. label Oct 24, 2023

kubermatic-bot removed the do-not-merge/code-freeze Indicates that a PR should not merge because it has not been approved for code freeze yet. label Oct 24, 2023

kubermatic-bot merged commit 0135ed4 into kubermatic:main Oct 24, 2023
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set Cilium as default CNI #12752

Set Cilium as default CNI #12752

embik commented Oct 19, 2023

embik commented Oct 19, 2023

wozniakjan commented Oct 19, 2023

wozniakjan commented Oct 19, 2023

wozniakjan commented Oct 19, 2023

wozniakjan Oct 19, 2023

wozniakjan commented Oct 20, 2023

wozniakjan commented Oct 20, 2023

wozniakjan commented Oct 20, 2023

embik commented Oct 20, 2023

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023

wozniakjan commented Oct 20, 2023

wozniakjan commented Oct 21, 2023

kubermatic-bot commented Oct 21, 2023 •

edited

cnvergence commented Oct 24, 2023

cnvergence left a comment

kubermatic-bot commented Oct 24, 2023

kubermatic-bot commented Oct 24, 2023

Set Cilium as default CNI #12752

Set Cilium as default CNI #12752

Conversation

embik commented Oct 19, 2023

embik commented Oct 19, 2023

wozniakjan commented Oct 19, 2023

wozniakjan commented Oct 19, 2023

wozniakjan commented Oct 19, 2023

wozniakjan Oct 19, 2023

Choose a reason for hiding this comment

wozniakjan commented Oct 20, 2023

wozniakjan commented Oct 20, 2023

wozniakjan commented Oct 20, 2023

embik commented Oct 20, 2023

wozniakjan Oct 20, 2023 • edited

Choose a reason for hiding this comment

wozniakjan Oct 20, 2023 • edited

Choose a reason for hiding this comment

wozniakjan Oct 20, 2023 • edited

Choose a reason for hiding this comment

wozniakjan Oct 20, 2023 • edited

Choose a reason for hiding this comment

wozniakjan Oct 20, 2023 • edited

Choose a reason for hiding this comment

wozniakjan Oct 20, 2023

Choose a reason for hiding this comment

wozniakjan commented Oct 20, 2023

wozniakjan commented Oct 21, 2023

kubermatic-bot commented Oct 21, 2023 • edited

cnvergence commented Oct 24, 2023

cnvergence left a comment

Choose a reason for hiding this comment

kubermatic-bot commented Oct 24, 2023

kubermatic-bot commented Oct 24, 2023

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

wozniakjan Oct 20, 2023 •

edited

kubermatic-bot commented Oct 21, 2023 •

edited