v1.10 backports 2022-05-06 #19744

kaworu · 2022-05-06T20:59:32Z

hubble/relay: Make Peer Service a K8s Service #18620 -- hubble/relay: Make Peer Service a K8s Service (@nathanjsweet)
test/upgrade: use the unreleased helm chart of stable branches #19710 -- test/upgrade: use the unreleased helm chart of stable branches (@aanm)
images/cilium: remove cilium group from Dockerfile #19711 -- images/cilium: remove cilium group from Dockerfile (@aanm)
operator: Add cilium node garbage collector #19576 -- operator: Add cilium node garbage collector (@sayboras)

Once this PR is merged, you can update the PR labels via:

$ for pr in 18620 19710 19711 19576; do contrib/backporting/set-labels.py $pr done 1.10; done

or with

$ make add-label BRANCH=v1.10 ISSUES=18620,19710,19711,19576

[ upstream commit 21e6e6a ] Currently Hubble-Relay builds its client list by querying the Peer Service over the local Hubble Unix domain socket. This goes against best security practices (sharing files across pods) and is not allowed on platforms that strictly enforce SELinux policies (e.g. OpenShift). This PR enables, by default, the creation of a Kubernetes Service that proxies the Hubble Peer Service so that Hubble-Relay can use it to build its client list, eliminating the need for a shared Unix domain socket completely. Helm values and configurations have been added to enable the service in a cilium deployment. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

kaworu

@aanm @nathanjsweet @sayboras this backport was complicated due to conflicts and the size of upstream PRs, please review and test carefully 🙏

install/kubernetes/cilium/templates/cilium-operator-clusterrole.yaml

kaworu · 2022-05-06T21:05:00Z

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

@@ -1,4 +1,8 @@
 {{- if .Values.hubble.relay.enabled }}
+{{- $peerSvcPort := .Values.hubble.peerService.servicePort -}}


@nathanjsweet note that the condition above in master guard for .Value.hubble.enabled as well. I don't think that's an issue, but to note.

kaworu · 2022-05-06T21:06:20Z

install/kubernetes/cilium/templates/hubble/peer-service.yaml

@@ -0,0 +1,24 @@
+{{- if and .Values.hubble.enabled .Values.hubble.listenAddress .Values.hubble.peerService.enabled }}


@nathanjsweet I've left this file in templates/hubble like in the upstream commit, although it's at odd with the v1.10 layout.

kaworu · 2022-05-06T21:08:23Z

operator/option/config.go

+	c.NodeGCInterval = viper.GetDuration(NodesGCInterval)
 	c.NodesGCInterval = viper.GetDuration(NodesGCInterval)


@sayboras I've moved this new line right next to the "old" c.NodesGCInterval (notice the Nodes plural in the name) which is deprecated in v1.11 and recycled in v1.12 (the upstream commit). Is it OK to have this behaviour introduced in v1.10 (basically piggy backing on the old CLI flag)?

Also note that c.NodesGCInterval is set twice in this func (second time new line 432), but I don't think it matter.

I think it should be fine for bug fixing 💯

kaworu · 2022-05-06T21:10:14Z

test/helpers/cons.go

-	CiliumStableHelmChartVersion = "1.9-dev"
+	CiliumStableHelmChartVersion = "1.9"


@aanm as suggested

kaworu · 2022-05-09T08:29:57Z

/test-backport-1.10

Job 'Cilium-PR-K8s-1.18-kernel-4.9' failed:

Click to show.

Test Name

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

Failure Output

FAIL: cannot install connectivity-check

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

test/k8sT/Updates.go

kaworu · 2022-05-09T12:34:05Z

/test-backport-1.10

sayboras

Thanks a lot. Looks good for my commit, also I tested the changes here locally 💯

$ ksyslo deployment/cilium-operator --timestamps | grep "subsys=watchers"                                              
2022-05-09T13:31:38.609543593Z level=info msg="Starting to garbage collect stale CiliumNode custom resources" subsys=watchers
2022-05-09T13:31:38.609590071Z level=debug msg="Cilium pod not running for node" nodeName=minikube subsys=watchers
2022-05-09T13:31:38.609597765Z level=debug msg="CiliumNode is valid, no gargage collection required" nodeName=minikube subsys=watchers
...
2022-05-09T13:36:38.610401836Z level=debug msg="CiliumNode is valid, no gargage collection required" nodeName=minikube subsys=watchers
2022-05-09T13:36:38.610488690Z level=info msg="Add CiliumNode to garbage collector candidates" nodeName=dummy-one subsys=watchers
2022-05-09T13:36:44.338787619Z level=debug msg="Node without taint and with CiliumIsUp condition" nodeName=minikube subsys=watchers
...
$ ksyslo deployment/cilium-operator --timestamps | grep "subsys=watchers"                                              
2022-05-09T13:31:38.609543593Z level=info msg="Starting to garbage collect stale CiliumNode custom resources" subsys=watchers
2022-05-09T13:31:38.609590071Z level=debug msg="Cilium pod not running for node" nodeName=minikube subsys=watchers
2022-05-09T13:31:38.609597765Z level=debug msg="CiliumNode is valid, no gargage collection required" nodeName=minikube subsys=watchers
...
2022-05-09T13:36:38.610401836Z level=debug msg="CiliumNode is valid, no gargage collection required" nodeName=minikube subsys=watchers
2022-05-09T13:36:38.610488690Z level=info msg="Add CiliumNode to garbage collector candidates" nodeName=dummy-one subsys=watchers
2022-05-09T13:36:44.338787619Z level=debug msg="Node without taint and with CiliumIsUp condition" nodeName=minikube subsys=watchers
...
2022-05-09T13:41:38.611438448Z level=debug msg="CiliumNode is valid, no gargage collection required" nodeName=minikube subsys=watchers
2022-05-09T13:41:38.611484484Z level=info msg="Perform GC for invalid CiliumNode" nodeName=dummy-one subsys=watchers
2022-05-09T13:41:38.618173682Z level=info msg="CiliumNode is garbage collected successfully" nodeName=dummy-one subsys=watchers
2022-05-09T13:41:50.655388256Z level=debug msg="Node without taint and with CiliumIsUp condition" nodeName=minikube subsys=watchers

nathanjsweet · 2022-05-09T16:01:45Z

To not break CI, we need to kind of backport the following PRs:

ci: Update Uninstall Command For Cilium CLI #19679
ci: update master workflows to cilium-cli v0.11.4 #19665
And wherever this line/commit came from

aanm · 2022-05-09T17:40:15Z

/test-only --focus="K8sUpdates.*Tests upgrade and downgrade from a Cilium stable image to master" --k8s_version=1.16 --kernel_version=net-next

[ upstream commit 88d31cd ] The upgrade tests are using the official helm charts with unreleased Cilium images. This can might cause the upgrade tests to fail in case the changes done in the unreleased Cilium versions require a new helm chart release. To fix this problem the upgrade tests will now use the unreleased helm charts as well. Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit 67f74ff ] Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit e80f60d ] This commit is to perform lift and shift current initialization for k8s node watcher to another function with single scope of work, so that it can be re-used later. There is no change in logic. Signed-off-by: Tam Mach <tam.mach@isovalent.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit 64b37e1 ] This is to make sure that k8s node watcher is only setup at max once. Also, synced channel is added, so that the consumer of this store knows if syncing process is done. Signed-off-by: Tam Mach <tam.mach@isovalent.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit edc1a0a ] In the normal scenario, CiliumNode is created by agent with owner references attached all time in below PR[0]. However, there could be the case that CiliumNode is created by IPAM module[1], which didn't have any ownerReferences at all. For this case, if the corresponding node got terminated and never came back with same name, the CiliumNode resource is still dangling, and needs to be garbage collected. This commit is to add garbage collector for CiliumNode, with below logic: - Gargage collector will run every predefined interval (e.g. specify by flag --nodes-gc-interval) - Each run will check if CiliumNode is having a counterpart k8s node resource. Also, remove this node from GC candidate if required. - If yes, CiliumNode is considered as valid, happy day. - If no, check if ownerReferences are set. - If yes, let k8s perform garbage collection. - If no, mark the node as GC candidate. If in the next run, this node is still in GC candidate, remove it. References: [0]: cilium#17329 [1]: https://github.com/cilium/cilium/blob/master/pkg/ipam/allocator/podcidr/podcidr.go#L258 Signed-off-by: Tam Mach <tam.mach@isovalent.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

[ upstream commit 167fa2b ] This commit is to map operator CLI node GC interval flag in helm value and config map. Signed-off-by: Tam Mach <tam.mach@isovalent.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

aanm · 2022-05-09T19:10:13Z

/test-only --focus="K8sUpdates.*Tests upgrade and downgrade from a Cilium stable image to master" --k8s_version=1.16 --kernel_version=net-next

aanm · 2022-05-09T19:45:20Z

/test-backport-1.10

kaworu · 2022-05-10T07:12:17Z

To not break CI, we need to kind of backport the following PRs:

#19679
#19665
And wherever this line/commit came from

@nathanjsweet for both v1.11 and v1.10 correct?

kaworu requested a review from a team as a code owner May 6, 2022 20:59

kaworu requested review from nathanjsweet, aanm and sayboras May 6, 2022 20:59

maintainer-s-little-helper bot added backport/1.10 kind/backports This PR provides functionality previously merged into master. labels May 6, 2022

aanm approved these changes May 6, 2022

View reviewed changes

kaworu commented May 6, 2022

View reviewed changes

aanm requested changes May 9, 2022

View reviewed changes

test/k8sT/Updates.go Outdated Show resolved Hide resolved

kaworu force-pushed the pr/v1.10-backport-2022-05-06 branch from 2ab7ad2 to d1dcf03 Compare May 9, 2022 12:12

kaworu requested a review from aanm May 9, 2022 12:34

sayboras approved these changes May 9, 2022

View reviewed changes

aanm force-pushed the pr/v1.10-backport-2022-05-06 branch from d1dcf03 to 68690a0 Compare May 9, 2022 17:39

aanm and others added 6 commits May 9, 2022 21:09

images/cilium: remove cilium group from Dockerfile

cc3de93

[ upstream commit 67f74ff ] Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

helm: Add nodes-gc-interval attribute

4e7ca33

[ upstream commit 167fa2b ] This commit is to map operator CLI node GC interval flag in helm value and config map. Signed-off-by: Tam Mach <tam.mach@isovalent.com> Signed-off-by: Alexandre Perrin <alex@kaworu.ch>

aanm force-pushed the pr/v1.10-backport-2022-05-06 branch from 68690a0 to 4e7ca33 Compare May 9, 2022 19:10

aanm merged commit 3016cfb into cilium:v1.10 May 9, 2022

aanm mentioned this pull request May 10, 2022

Prepare for release v1.10.11 #19755

Merged

kaworu deleted the pr/v1.10-backport-2022-05-06 branch May 10, 2022 07:11

rolinh mentioned this pull request May 12, 2022

CI: K8sHubbleTest Hubble Observe with hubble-relay #19790

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v1.10 backports 2022-05-06 #19744

v1.10 backports 2022-05-06 #19744

kaworu commented May 6, 2022

kaworu left a comment

kaworu May 6, 2022

kaworu May 6, 2022

kaworu May 6, 2022

sayboras May 9, 2022

kaworu May 6, 2022

kaworu commented May 9, 2022 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

kaworu commented May 9, 2022

sayboras left a comment

nathanjsweet commented May 9, 2022

aanm commented May 9, 2022

aanm commented May 9, 2022

aanm commented May 9, 2022

kaworu commented May 10, 2022

		@@ -1,4 +1,8 @@
		{{- if .Values.hubble.relay.enabled }}
		{{- $peerSvcPort := .Values.hubble.peerService.servicePort -}}

		@@ -0,0 +1,24 @@
		{{- if and .Values.hubble.enabled .Values.hubble.listenAddress .Values.hubble.peerService.enabled }}

		c.NodeGCInterval = viper.GetDuration(NodesGCInterval)
		c.NodesGCInterval = viper.GetDuration(NodesGCInterval)

		CiliumStableHelmChartVersion = "1.9-dev"
		CiliumStableHelmChartVersion = "1.9"

v1.10 backports 2022-05-06 #19744

v1.10 backports 2022-05-06 #19744

Conversation

kaworu commented May 6, 2022

kaworu left a comment

Choose a reason for hiding this comment

kaworu May 6, 2022

Choose a reason for hiding this comment

kaworu May 6, 2022

Choose a reason for hiding this comment

kaworu May 6, 2022

Choose a reason for hiding this comment

sayboras May 9, 2022

Choose a reason for hiding this comment

kaworu May 6, 2022

Choose a reason for hiding this comment

kaworu commented May 9, 2022 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

kaworu commented May 9, 2022

sayboras left a comment

Choose a reason for hiding this comment

nathanjsweet commented May 9, 2022

aanm commented May 9, 2022

aanm commented May 9, 2022

aanm commented May 9, 2022

kaworu commented May 10, 2022

kaworu commented May 9, 2022 •

edited by maintainer-s-little-helper bot