Deprecate Kubelet cAdvisor port #56523

tallclair · 2017-11-29T00:34:37Z

cAdvisor is becoming an implementation detail of Kubernetes, and may even be removed entirely in the future (or pushed down into the CRI layer). The Kubelet's stats API has been available for a long time as the prefered way of gathering stats, and there are a number of solutions (e.g. influxdb+grafana) for cluster-level monitoring.

As such, we should begin the process of deprecating and removing the cAdvisor API surface. As a first step, I propose that we ~~default the cAdvisor port to 0 (disabled), starting with Kubernetes 1.10, and add a deprecation warning to the flag.~~
EDIT: We should add the deprecation warning in 1.10 along with a release note, but not change the default. The notes should warn that the default will change in 1.11. We disable the flag by default in 1.11, and remove it entirely in 1.12 or 1.13.

If you currently depend on the UI or the API, speak up! Going forward, the recommended way of taking advantage of those features will be to run cAdvisor as a DaemonSet.

/cc @kubernetes/sig-node-proposals @philips @dashpole

tianshapjq · 2017-11-30T08:11:38Z

/cc

jberkus · 2018-02-03T01:15:22Z

Are you still planning to make changes as part of 1.10 for this issue?

If so, can you please complete the labels for it? priority/ is required. Thanks.

mtaufen · 2018-02-08T00:37:50Z

This applies to #53833, better get rid of it before it's canonized in beta kubeletconfig (also v1.10).

mtaufen · 2018-02-08T18:06:11Z

#59580

luxas · 2018-02-08T19:35:34Z

I'm in favor of stopping to listen on localhost:4194 by default, we stopped doing that in kubeadm clusters already by v1.7 and nobody has complained so far.

luxas · 2018-02-08T19:35:53Z

We need a BIG release note and announcement though...

See: kubernetes#56523, cAdvisor is becoming an implementation detail of Kubernetes, and we should not canonize its knobs on the KubeletConfiguration.

tallclair · 2018-02-08T22:28:41Z

I think we need to give people a heads up that the default is changing. I modified my proposal to capture that.

tallclair · 2018-02-08T22:33:10Z

FYI, flag deprecation policy: https://kubernetes.io/docs/reference/deprecation-policy/#deprecating-a-flag-or-cli

This falls under the "admin-facing" components, as a GA flag:

Rule #5b: CLI elements of admin-facing components (e.g. kubelet) must function after their announced deprecation for no less than:

GA: 6 months or 1 release (whichever is longer)

@thockin

Automatic merge from submit-queue (batch tested with PRs 59353, 59905, 53833). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Graduate kubeletconfig API group to beta Regarding kubernetes/enhancements#281, this PR moves the kubeletconfig API group to beta. After #53088, the KubeletConfiguration type should not contain any deprecated or experimental fields, and we should not have to remove any more fields from the type before graduating it to beta. We need the community to double check for two things, however: 1. Are there any fields currently in the KubeletConfiguration type that you were going to mark deprecated this quarter, but haven't yet? 2. Are there any fields currently in the KubeletConfiguration type that are experimental or alpha, but were not explicitly denoted as such? Please comment on this PR if you can answer "yes" to either of those two questions. Please cc anyone with a stake in the kubeletconfig API, so we get as much coverage as possible. /cc @thockin @dchen1107 @Random-Liu @yujuhong @dashpole @tallclair @vishh @abw @freehan @dnardo @bowei @MrHohn @luxas @liggitt @ncdc @derekwaynecarr @mikedanese @kubernetes/sig-network-pr-reviews, @kubernetes/sig-node-pr-reviews ```release-note action required: The `kubeletconfig` API group has graduated from alpha to beta, and the name has changed to `kubelet.config.k8s.io`. Please use `kubelet.config.k8s.io/v1beta1`, as `kubeletconfig/v1alpha1` is no longer available. ``` **TODO:** - [x] Move experimental/non-gated-alpha/soon-to-be-deprecated fields to `KubeletFlags` - [x] #53088 - [x] #54154 - [x] #54160 - [x] #55562 - [x] #55983 - [x] #57851 - [x] Lift embedded structure out of strings - [x] #53025 - [x] #54643 - [x] #54823 - [x] #55254 - [x] Resolve relative paths against the location config files are loaded from - [x] #55648 - [x] Rename to `kubelet.config.k8s.io` - [x] Comments - [x] Make sure existing comments at least read sensibly. - [x] Note default values in comments on the versioned struct. - [x] Remove any reference to default values in comments on the internal struct. - [x] Most fields should be `+optional` and `omitempty`. Add where necessary. ~Where omitted, explicitly comment.~ Edit: We should not distinguish between nil and empty, see below items. - [x] Ensure defaults are specified via `pkg/kubelet/apis/kubelet.config.k8s.io/v1beta1/defaults.go`, not `cmd/kubelet/app/options/options.go`. - [x] #57770 - [x] Ensure kubeadm does not persist v1alpha1 KubeletConfiguration objects (or feature-gates this functionality) - [x] Don't make a distinction between empty and nil, because of #43203. - [x] #59515 - [x] #59681 - [x] Take the opportunity to fix insecure Kubelet defaults @tallclair - [x] #59666 - [x] Remove CAdvisorPort from KubeletConfiguration wrt #56523. - [x] #59580 - [x] Hide `ConfigTrialDuration` until we're more sure what to do with it. - [x] #59628 - [x] Fix `// default: x` comments after rebasing on recent changes.

@mtaufen

…_port Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Deprecate kubelet flag for cadvisor port **Which issue(s) this PR fixes**: Issue: kubernetes#56523 TL;DR the Kubelet's `stats/summary` API is the preferred way of monitoring the node. If you need additional metrics from cAdvisor, it can be run as a daemonset. **Release note**: ```release-note Deprecate the kubelet's cadvisor port ``` /assign @mtaufen @tallclair cc @kubernetes/sig-node-pr-reviews

dashpole · 2018-02-17T19:45:53Z

/priority important-longterm

karataliu · 2018-03-05T02:51:27Z

The following E2E case depends it listening on 4194, could consider updating:
'should proxy to cadvisor using proxy subresource'

kubernetes/test/e2e/network/proxy.go

Line 77 in b114a11

    
           It("should proxy to cadvisor using proxy subresource", func() { nodeProxyTest(f, prefix+"/nodes/", ":4194/proxy/containers/") })

astefanutti · 2018-05-02T08:33:53Z

The Kubelet's stats API has been available for a long time as the prefered way of gathering stats

@tallclair I think the Kubelet's stats API issue #56297 should be fixed (PR is available #62544), as the current work-around is to fallback on the cAdvisor port.

luxas · 2018-05-15T17:29:01Z

I now stumbled upon this piece of code.
Can we default the cadvisor port to 0 in v1.11 to gradually phase it out?
And then remove in v1.12, right?

The following E2E case depends it listening on 4194, could consider updating

I guess we should just remove it, as it's functionality that we're removing?

As said above, kubeadm has had this off for a year and nobody has complained. I think defaulting to off in v1.11 would be good, with a big release note of course.

I think that's as smooth deprecation/removal as we can get.
If everybody's okay with that, let's send a PR now as we're getting close to the code freeze

@tallclair

Automatic merge from submit-queue (batch tested with PRs 63881, 64046, 63409, 63402, 63221). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Disable the public cadvisor port by default **What this PR does / why we need it**: Quoting @tallclair in #56523: > We should add the deprecation warning in 1.10 along with a release note, but not change the default. The notes should warn that the default will change in 1.11. We disable the flag by default in 1.11, and remove it entirely in 1.12 or 1.13. > If you currently depend on the UI or the API, speak up! Going forward, the recommended way of taking advantage of those features will be to run cAdvisor as a DaemonSet. Disabling the publicly-available cAdvisor port is beneficial for security, as you might not want to expose the UI with lots of information about what your system is doing. We already did this for all kubeadm deployments in v1.7, and haven't recieved any issues for that. This should be okay to do at this stage, as this flag was deprecated in v1.10. Given we need to support this flag for one more release (v1.11), it makes perfect sense to instead switch it off in preparation for v1.12 when we can delete it (see the [deprecation policy](https://kubernetes.io/docs/reference/deprecation-policy/#deprecating-a-flag-or-cli)) **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Part of #56523 **Special notes for your reviewer**: I removed the e2e test that expects cAdvisor to be running, as we don't expect it to be anymore. **Release note**: ```release-note [action required] The formerly publicly-available cAdvisor web UI that the kubelet ran on port 4194 by default is now turned off by default. The flag configuring what port to run this UI on `--cadvisor-port` was deprecated in v1.10. Now the default is `--cadvisor-port=0`, in other words, to not run the web server. The recommended way to run cAdvisor if you still need it, is via a DaemonSet. The `--cadvisor-port` will be removed in v1.12 ``` cc @kubernetes/sig-cluster-lifecycle-pr-reviews @kubernetes/sig-auth-pr-reviews @kubernetes/sig-node-pr-reviews

luxas · 2018-06-30T09:27:41Z

We should now be able to remove this flag option in v1.12. Anyone up for doing that?

k8s-github-robot · 2018-06-30T09:27:50Z

[MILESTONENOTIFIER] Milestone Issue: Up-to-date for process

@dashpole @tallclair

Issue Labels

sig/node: Issue will be escalated to these SIGs if needed.
priority/important-longterm: Escalate to the issue owners; move out of the milestone after 1 attempt.
kind/cleanup: Adding tests, refactoring, fixing old bugs.

Help

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Remove --cadvisor-port - has been deprecated since v1.10 **What this PR does / why we need it**: **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes #56523 **Special notes for your reviewer**: - Deprecated in #59827 (v1.10) - Disabled in #63881 (v1.11) **Release note**: ```release-note [action required] The formerly publicly-available cAdvisor web UI that the kubelet started using `--cadvisor-port` is now entirely removed in 1.12. The recommended way to run cAdvisor if you still need it, is via a DaemonSet. ```

anandsinghkunwar · 2018-10-17T06:07:56Z

What about /metrics/cadvisor endpoint in kubelet, is that going away as well, since cAdvisor is to be replaced by CRI metrics?

dashpole · 2018-10-17T16:57:37Z

@anandsinghkunwar
That isn't covered by this issue, but probably falls under #68522. That isn't happening for at least another couple releases, but probably longer.

tallclair added the kind/technical-debt label Nov 29, 2017

tallclair added this to the v1.10 milestone Nov 29, 2017

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. kind/design Categorizes issue or PR as related to design. labels Nov 29, 2017

mtaufen mentioned this issue Feb 8, 2018

Graduate kubeletconfig API group to beta #53833

Merged

33 tasks

mtaufen mentioned this issue Feb 8, 2018

remove CAdvisorPort from KubeletConfiguration struct #59580

Merged

mtaufen mentioned this issue Feb 8, 2018

deprecate Kubelet cadvisor housekeeping_interval flag #59445

Closed

mtaufen added a commit to mtaufen/kubernetes that referenced this issue Feb 8, 2018

remove CAdvisorPort from KubeletConfiguration

5ab9ccd

See: kubernetes#56523, cAdvisor is becoming an implementation detail of Kubernetes, and we should not canonize its knobs on the KubeletConfiguration.

tallclair assigned dashpole Feb 8, 2018

dashpole mentioned this issue Feb 13, 2018

Deprecate kubelet flag for cadvisor port #59827

Merged

k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Feb 17, 2018

luxas mentioned this issue May 15, 2018

Disable the public cadvisor port by default #63881

Merged

astefanutti mentioned this issue May 17, 2018

Resources usage metrics unavailable astefanutti/kubebox#11

Closed

k8s-github-robot added milestone/incomplete-labels milestone/removed and removed milestone/incomplete-labels labels Jun 20, 2018

k8s-github-robot removed this from the v1.12 milestone Jun 23, 2018

luxas added status/approved-for-milestone and removed milestone/removed labels Jun 30, 2018

luxas added this to the v1.12 milestone Jun 30, 2018

k8s-github-robot added the milestone/incomplete-labels label Jun 30, 2018

luxas added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed milestone/incomplete-labels labels Jun 30, 2018

luxas mentioned this issue Jul 1, 2018

Clarify desired cAdvisor integration in kubelet #32638

Closed

dims mentioned this issue Jul 2, 2018

Remove --cadvisor-port - has been deprecated since v1.10 #65707

Merged

k8s-github-robot closed this as completed in #65707 Jul 7, 2018

sjenning mentioned this issue Aug 1, 2018

Deprecate cAdvisor port in K8s #53615

Closed

knisbet mentioned this issue Sep 12, 2018

Kevin/cis benchmark gravitational/planet#312

Merged

thxCode mentioned this issue Jan 23, 2019

Remove deprecated kubelet options when Windows deploying rancher/rancher#17534

Closed

inductor mentioned this issue Dec 30, 2021

Update kubeadm integration of kube-prometheus prometheus-operator/kube-prometheus#1569

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate Kubelet cAdvisor port #56523

Deprecate Kubelet cAdvisor port #56523

tallclair commented Nov 29, 2017 •

edited

Loading

tianshapjq commented Nov 30, 2017

jberkus commented Feb 3, 2018

mtaufen commented Feb 8, 2018

mtaufen commented Feb 8, 2018

luxas commented Feb 8, 2018

luxas commented Feb 8, 2018

tallclair commented Feb 8, 2018

tallclair commented Feb 8, 2018

dashpole commented Feb 17, 2018

karataliu commented Mar 5, 2018

astefanutti commented May 2, 2018

luxas commented May 15, 2018 •

edited

Loading

luxas commented Jun 30, 2018

k8s-github-robot commented Jun 30, 2018

anandsinghkunwar commented Oct 17, 2018

dashpole commented Oct 17, 2018

Deprecate Kubelet cAdvisor port #56523

Deprecate Kubelet cAdvisor port #56523

Comments

tallclair commented Nov 29, 2017 • edited Loading

tianshapjq commented Nov 30, 2017

jberkus commented Feb 3, 2018

mtaufen commented Feb 8, 2018

mtaufen commented Feb 8, 2018

luxas commented Feb 8, 2018

luxas commented Feb 8, 2018

tallclair commented Feb 8, 2018

tallclair commented Feb 8, 2018

dashpole commented Feb 17, 2018

karataliu commented Mar 5, 2018

astefanutti commented May 2, 2018

luxas commented May 15, 2018 • edited Loading

luxas commented Jun 30, 2018

k8s-github-robot commented Jun 30, 2018

anandsinghkunwar commented Oct 17, 2018

dashpole commented Oct 17, 2018

tallclair commented Nov 29, 2017 •

edited

Loading

luxas commented May 15, 2018 •

edited

Loading