Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve k8s client heartbeat capability #10673

Merged
merged 6 commits into from
Mar 24, 2020
Merged

Improve k8s client heartbeat capability #10673

merged 6 commits into from
Mar 24, 2020

Conversation

aanm
Copy link
Member

@aanm aanm commented Mar 23, 2020

Switched the heartbeat to be done on /healthz instead of doing a get
on kube-system namespace which has higher overhead than /heatlhz.

If Cilium has interactions with k8s, Cilium does not need to
perform heatbeats to the kube-apiserver. Unless those interactions
result in an error for which we should perform a heartbeat to
kube-apiserver to check if the connection is still alive. In case the
response from kube-apiserver is an error Cilium should then close all
connections with the kube-apiserver.

Read per commit for more details

follow up of #10184

@aanm aanm added pending-review sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. labels Mar 23, 2020
@aanm aanm requested a review from a team March 23, 2020 22:59
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

2 similar comments
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 23, 2020
@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Mar 23, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.2 Mar 23, 2020
@aanm
Copy link
Member Author

aanm commented Mar 23, 2020

test-me-please

@coveralls
Copy link

coveralls commented Mar 23, 2020

Coverage Status

Coverage increased (+0.06%) to 45.591% when pulling ab48bf4 on pr/fix-cilium-heartbeat into d512d92 on master.

pkg/k8s/init.go Outdated Show resolved Hide resolved
pkg/k8s/client.go Outdated Show resolved Hide resolved
@aanm aanm force-pushed the pr/fix-cilium-heartbeat branch 2 times, most recently from 9e6bfed to 1f25d44 Compare March 24, 2020 12:41
@aanm aanm removed the sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. label Mar 24, 2020
@aanm
Copy link
Member Author

aanm commented Mar 24, 2020

test-me-please

@aanm aanm requested a review from tgraf March 24, 2020 12:51
There's no point on doing a get on the kube-system namespace since we
can also perform the same operation in the `/healthz` endpoint which is
also available to check if kube-apiserver is available.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
Making the heartbeat as a function will allow to write unit tests more
easily.

Signed-off-by: André Martins <andre@cilium.io>
If Cilium has interactions with k8s, Cilium does not need to
perform heatbeats to the kube-apiserver. Unless those interactions
result in an error for which we should perform a heartbeat to
kube-apiserver to check if the connection is still alive. In case the
response from kube-apiserver is an error Cilium should then close all
connections with the kube-apiserver.

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
@aanm
Copy link
Member Author

aanm commented Mar 24, 2020

test-me-please

Copy link
Member

@tgraf tgraf left a comment

Choose a reason for hiding this comment

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

Awesome

@aanm
Copy link
Member Author

aanm commented Mar 24, 2020

hit #10118

@aanm aanm merged commit d03d08e into master Mar 24, 2020
1.8.0 automation moved this from In progress to Merged Mar 24, 2020
@aanm aanm deleted the pr/fix-cilium-heartbeat branch March 24, 2020 16:51
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.2 Mar 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.2 Mar 30, 2020
@tklauser tklauser moved this from Backport pending to v1.7 to Backport done to v1.7 in 1.7.2 Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.7.2
Backport done to v1.7
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants