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

When running from within a k8s pod, use the k8s API to get the cgroup name #5576

Merged
merged 12 commits into from Mar 13, 2019

Conversation

5 participants
@cakrit
Copy link
Contributor

cakrit commented Mar 7, 2019

Summary

Fixes #5387 (just the pod/container monitoring part, presentation will be a different issue)
Fixes #3520 (again, just wrt naming, presentation will be a new issue).

k8s don't necessarily have a docker executable available. Use the k8s pod API to get the pod name.
Used for cgroup naming.
Disable cgroups that are not returned by the k8s API (e.g. pause containers).

@cakrit cakrit requested review from ktsaou and vlvkobal as code owners Mar 7, 2019

@cakrit cakrit requested a review from paulfantom Mar 7, 2019

@@ -116,13 +133,16 @@ for CONFIG in "${NETDATA_USER_CONFIG_DIR}/cgroups-names.conf" "${NETDATA_STOCK_C
fi
done

if [ -z "${NAME}" ] && [ -n "${KUBERNETES_SERVICE_HOST}" ] && [ -n "${KUBERNETES_PORT_443_TCP_PORT}" ]; then

This comment has been minimized.

@paulfantom

paulfantom Mar 7, 2019

Contributor

KUBERNETES_PORT_443_TCP_PORT -> KUBERNETES_API_PORT

No need to copy-paste from k8s docs when it doesn't improve UX.

This comment has been minimized.

@cakrit

cakrit Mar 8, 2019

Author Contributor

I haven't seen KUBERNETES_API_PORT in any official docs, or even mentioned in issues. How can I trust it to be the https port?

This comment has been minimized.

@cakrit

cakrit Mar 13, 2019

Author Contributor

It worked on all tests with this, so leaving it as is.

This comment has been minimized.

@cakrit

cakrit Mar 13, 2019

Author Contributor

It worked on all tests with this, so leaving it as is.

@cakrit cakrit force-pushed the cakrit:k8s-1 branch from e81a2ba to c70214d Mar 8, 2019

@cakrit cakrit added this to In progress in Kubernetes monitoring via automation Mar 8, 2019

@cakrit

This comment has been minimized.

Copy link
Contributor Author

cakrit commented Mar 8, 2019

This won't be considered complete until #3520 is implemented (at least to the point of correctly showing only the relevant cgroups, with appropriate names).

@cakrit cakrit force-pushed the cakrit:k8s-1 branch from ccac26c to 8f98dd5 Mar 9, 2019

@cakrit cakrit changed the title [WIP] When running from within a k8s pod, use the k8s API to get the pod name When running from within a k8s pod, use the k8s API to get the cgroup name Mar 12, 2019

@cakrit cakrit force-pushed the cakrit:k8s-1 branch from 5acaa6e to 351119e Mar 12, 2019

@cakrit cakrit requested a review from mfundul as a code owner Mar 13, 2019

@ktsaou

ktsaou approved these changes Mar 13, 2019

Copy link
Member

ktsaou left a comment

We should probably do something about the jq command to make sure it is installed with netdata.

I mean, to be installed by kickstart.sh and netdata.spec.

@cakrit cakrit merged commit b7998ec into netdata:master Mar 13, 2019

5 of 6 checks passed

Codacy/PR Quality Review Hang in there, Codacy is reviewing your Pull request.
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
license/cla Contributor License Agreement is signed.
Details
netlify/netdata/deploy-preview Deploy preview ready!
Details

Kubernetes monitoring automation moved this from In progress to Done Mar 13, 2019

@cakrit cakrit deleted the cakrit:k8s-1 branch Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.