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

Document that disabling enable-cadvisor-json-endpoints flag does not disable stats summary endpoint #96483

Closed
adammw opened this issue Nov 11, 2020 · 8 comments · Fixed by #96928 or kubernetes/website#25621
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@adammw
Copy link

adammw commented Nov 11, 2020

What would you like to be added:
Update the help text for kubelet's enable-cadvisor-json-endpoints flag to explicitly mention that it has no effect over the /stats/summary endpoint.

Why is this needed:
The help text for kubelet's enable-cadvisor-json-endpoints flag describes it as "Enable cAdvisor json /spec and /stats/* endpoints." which would seem to include /stats/summary, however this is not the case as that endpoint is required by kube-metrics-server and exposes stats from cadvisor or CRI (depending on the stat provider selected internally).

Related Issues:
#68522

/sig node
/sig instrumentation
/kind documentation

@adammw adammw added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 11, 2020
@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. kind/documentation Categorizes issue or PR as related to documentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 11, 2020
@ehashman
Copy link
Member

ehashman commented Dec 2, 2020

/kind documentation
/triage accepted
/help

@k8s-ci-robot
Copy link
Contributor

@ehashman:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/kind documentation
/triage accepted
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 2, 2020
@joadavis
Copy link

joadavis commented Dec 2, 2020

/assign

This should be updated in
https://github.com/kubernetes/website/blob/master/content/en/docs/reference/command-line-tools-reference/kubelet.md
as well as
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubelet/app/options/options.go#L372
(unless there is some automated process that updates the website from the options in code that I'm not aware of).

@joadavis joadavis removed their assignment Dec 2, 2020
@joadavis
Copy link

joadavis commented Dec 2, 2020

To summarize a conversation in slack, it appears there is a documented procedure for updating the website found at https://kubernetes.io/docs/contribute/generate-ref-docs/kubectl/
The procedure is long, so it is tempting to just copy in the small amount of text that was changed from options.go into a website PR rather than pulling down so many moving parts.

In any case, we likely should keep this issue open until both the change to options.go and an associated website change are merged.

@ehashman
Copy link
Member

@joadavis can you link to the Slack convo?

@joadavis
Copy link

Can do - https://kubernetes.slack.com/archives/C20HH14P7/p1606936008188200

The conversation ended with a question whether there is a bug that would prevent an automated update of the documentation or if we need to submit a PR now that the fix has merged. I searched for an issue but didn't find a match. So we can either try to find someone on sig-docs who might know if there is an issue if it should be updated automagically, or just fix the doc if the website didn't get updated (and I just checked the master branch and it has not been updated yet).

joadavis added a commit to joadavis/website that referenced this issue Dec 14, 2020
…ot disable stats summary endpoint

Update the help text for kubelet's enable-cadvisor-json-endpoints flag to explicitly mention that it has no effect over the /stats/summary endpoint.

This matches https://github.com/kubernetes/kubernetes/blob/master/cmd/kubelet/app/options/options.go#L372
created to fix kubernetes/kubernetes#96483 .
@joadavis
Copy link

Just in case that is the right answer, I created the PR.

@ehashman
Copy link
Member

I think the website gets autogenerated upon point releases but that'd be a good thing to clarify with the website team. I'll leave a comment asking as much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
4 participants