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.

Sign up for GitHub

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

Jump to bottom

Restore cAdvisor prometheus metrics to the main port #49079

Merged
merged 1 commit into from
Jul 19, 2017
Merged

Restore cAdvisor prometheus metrics to the main port #49079

merged 1 commit into from
Jul 19, 2017

Conversation

smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Jul 18, 2017 •
edited by luxas
Loading

But under a new path - /metrics/cadvisor. This ensures a secure port still exists for metrics while getting the benefit of separating out container metrics from the kubelet's metrics as recommended in the linked issue.

Fixes #48483

Restored cAdvisor prometheus metrics to the main port -- a regression that existed in v1.7.0-v1.7.2
cAdvisor metrics can now be scraped from `/metrics/cadvisor` on the kubelet ports.
Note that you have to update your scraping jobs to get kubelet-only metrics from `/metrics` and `container_*` metrics from `/metrics/cadvisor`

@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jul 18, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 18, 2017
@smarterclayton
Copy link
Contributor Author

@kubernetes/sig-instrumentation-pr-reviews

@k8s-ci-robot k8s-ci-robot added the sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. label Jul 18, 2017
@brancz
Copy link
Member

brancz commented Jul 18, 2017

While I do agree with the general idea that these two sets of metrics should be separate, this does not restore the previous state. Is that acceptable or considered a breaking change?

@smarterclayton
Copy link
Contributor Author

We don't guarantee backward compatibility for metrics, although we strive. With this change to gather metrics from a new cluster is just a new scrape job, which while annoying... is not the end of the world. An admin who hasn't upgraded to 1.7 could reasonably anticipate and set a scrape job ahead of time.

@smarterclayton
Restore cAdvisor prometheus metrics to the main port
f5f9893 
But under a new path - `/metrics/cadvisor`. This ensures a secure port
still exists for metrics while getting the benefit of separating out
container metrics from the kubelet's metrics.
@fgrzadkowski
Copy link
Contributor

@kubernetes/sig-instrumentation-bugs @piosz

@smarterclayton
Copy link
Contributor Author

/retest

@brancz
Copy link
Member

brancz commented Jul 19, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 19, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, smarterclayton

Associated issue: 48483

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jul 19, 2017
@luxas luxas added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. cherrypick-candidate and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 19, 2017
@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@luxas luxas added this to the v1.7 milestone Jul 19, 2017
@luxas
Copy link
Member

luxas commented Jul 19, 2017

This is a cherrypick candidate for v1.7.3 -- v1.7.2 is already being built today.
@smarterclayton I added a release note that could be used, feel free to modify the wording as you like later today. Added the release note so that this PR could get merged ASAP now that we have approval from sig-instrumentation.

cc @wojtek-t

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit c326cb1 into kubernetes:master Jul 19, 2017
@unixwitch unixwitch mentioned this pull request Jul 19, 2017
Merged
unixwitch added a commit to unixwitch/prometheus that referenced this pull request Jul 19, 2017
@unixwitch
documentation: update Kubernetes example for 1.7
94103aa 
Kubernetes 1.7+ no longer exposes cAdvisor metrics on the Kubelet
metrics endpoint.  Update the example configuration to scrape cAdvisor
in addition to Kubelet.  The provided configuration works for 1.7.3+
and commented notes are given for 1.7.2 and earlier versions.

Also remove the comment about node (Kubelet) CA not matching the master
CA.  Since the example no longer connects directly to the nodes, it
doesn't matter what CA they're using.

References:

- kubernetes/kubernetes#48483
- kubernetes/kubernetes#49079
@squat squat mentioned this pull request Jul 19, 2017
Closed
@feiskyer feiskyer mentioned this pull request Jul 20, 2017
Merged
@danp danp mentioned this pull request Jul 20, 2017
Merged
@smarterclayton smarterclayton mentioned this pull request Jul 21, 2017
Merged
juliusv pushed a commit to prometheus/prometheus that referenced this pull request Jul 21, 2017
@unixwitch @juliusv
documentation: update Kubernetes example for 1.7 (#2918)
f30b102 
Kubernetes 1.7+ no longer exposes cAdvisor metrics on the Kubelet
metrics endpoint.  Update the example configuration to scrape cAdvisor
in addition to Kubelet.  The provided configuration works for 1.7.3+
and commented notes are given for 1.7.2 and earlier versions.

Also remove the comment about node (Kubelet) CA not matching the master
CA.  Since the example no longer connects directly to the nodes, it
doesn't matter what CA they're using.

References:

- kubernetes/kubernetes#48483
- kubernetes/kubernetes#49079
@wojtek-t wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 24, 2017
k8s-github-robot pushed a commit that referenced this pull request Jul 24, 2017
Merge pull request #49254 from feiskyer/49079
ce6d925 
Automatic merge from submit-queue

Automated cherry pick of #49079 upstream release 1.7

Cherry pick of #49079 on release-1.7.

#49079: Restore cAdvisor prometheus metrics to the main port
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

@brancz
Copy link
Member

brancz commented Jul 25, 2017

@wojtek-t @luxas @smarterclayton ^ is this being taken care of?

@wojtek-t
Copy link
Member

@brancz I don't understand your question. What do you mean? If you're asking about removing "cherrypick-candiate" label, this is no longer needed, because this has already been cherrypicked to 1.7 rbanch and will be part of 1.7.3 patch release.

@brancz
Copy link
Member

brancz commented Jul 25, 2017

Sorry about that, I misread it. All good.

edrex added a commit to PlainsightAI/prometheus-operator that referenced this pull request Oct 4, 2017
@edrex
scrape from the new (k8s 1.7.3+) cadvisor metrics path
9bd80b4 
See:

kubernetes/kubernetes#49079
prometheus-operator#633
edrex added a commit to PlainsightAI/prometheus-operator that referenced this pull request Oct 4, 2017
@edrex
scrape from the new (k8s 1.7.3+) cadvisor metrics path
a287e21 
See:

kubernetes/kubernetes#49079
prometheus-operator#633
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@feiskyer feiskyer

@resouer resouer

@brancz brancz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
10 participants
@smarterclayton @brancz @fgrzadkowski @k8s-github-robot @k8s-cherrypick-bot @luxas @wojtek-t @feiskyer @resouer @k8s-ci-robot