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

Other components support set log level dynamically #64601

Merged
merged 4 commits into from Aug 16, 2018

Conversation

@hzxuzhonghu
Member

hzxuzhonghu commented Jun 1, 2018

What this PR does / why we need it:

#63777 introduced a way to set glog.logging.verbosity dynamically.
We should enable this for all other components, which is specially useful in debugging.

Release note:

Expose `/debug/flags/v` to allow kubelet dynamically set glog logging level.  If want to change glog level to 3, you only have to send a PUT request like `curl -X PUT http://127.0.0.1:8080/debug/flags/v -d "3"`.
@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu
Member

hzxuzhonghu commented Jun 1, 2018

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 5, 2018

Member

/retest

Member

hzxuzhonghu commented Jun 5, 2018

/retest

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu
Member

hzxuzhonghu commented Jun 5, 2018

/assign @thockin

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 5, 2018

Member

/test pull-kubernetes-e2e-gce

Member

hzxuzhonghu commented Jun 5, 2018

/test pull-kubernetes-e2e-gce

@thockin

This comment has been minimized.

Show comment
Hide comment
@thockin

thockin Jun 5, 2018

Member

Shouldn't we have some sort of auth mechanism here? This is a DoS waiting to happen.

Member

thockin commented Jun 5, 2018

Shouldn't we have some sort of auth mechanism here? This is a DoS waiting to happen.

@sttts

This comment has been minimized.

Show comment
Hide comment
@sttts

sttts Jun 5, 2018

Contributor

Shouldn't we have some sort of auth mechanism here? This is a DoS waiting to happen.

This has to be behind an RBAC rule by default only allowing it for the cluster admin, plus an e2e test to verify that. For the scheduler and controller-manager we will add delegated authn/z for 1.12 (WIP PRs are open). Not sure about kubelet and kube-proxy (/cc @liggitt).

Contributor

sttts commented Jun 5, 2018

Shouldn't we have some sort of auth mechanism here? This is a DoS waiting to happen.

This has to be behind an RBAC rule by default only allowing it for the cluster admin, plus an e2e test to verify that. For the scheduler and controller-manager we will add delegated authn/z for 1.12 (WIP PRs are open). Not sure about kubelet and kube-proxy (/cc @liggitt).

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 6, 2018

Member

Not sure about kubelet and kube-proxy

kubelet now startup a secure https schema, and after authn/authz. Not sure about kube-proxy.

Just confirmed kube-proxy is insecure http without authn/authz.

Member

hzxuzhonghu commented Jun 6, 2018

Not sure about kubelet and kube-proxy

kubelet now startup a secure https schema, and after authn/authz. Not sure about kube-proxy.

Just confirmed kube-proxy is insecure http without authn/authz.

@sttts

This comment has been minimized.

Show comment
Hide comment
@sttts
Contributor

sttts commented Jun 6, 2018

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Jun 20, 2018

Member

Not sure about kubelet and kube-proxy

kubelet now startup a secure https schema, and after authn/authz. Not sure about kube-proxy.

Just confirmed kube-proxy is insecure http without authn/authz.

can we enable this on the components that can protect it, and defer on the ones that can't yet?

Member

liggitt commented Jun 20, 2018

Not sure about kubelet and kube-proxy

kubelet now startup a secure https schema, and after authn/authz. Not sure about kube-proxy.

Just confirmed kube-proxy is insecure http without authn/authz.

can we enable this on the components that can protect it, and defer on the ones that can't yet?

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 20, 2018

Member

can we enable this on the components that can protect it, and defer on the ones that can't yet?

Yes, we can.

Member

hzxuzhonghu commented Jun 20, 2018

can we enable this on the components that can protect it, and defer on the ones that can't yet?

Yes, we can.

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 25, 2018

Member

@thockin @liggitt Now this only support kubelet, since others are without authn/authz. ptal

Member

hzxuzhonghu commented Jun 25, 2018

@thockin @liggitt Now this only support kubelet, since others are without authn/authz. ptal

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 25, 2018

Member

/retest

Member

hzxuzhonghu commented Jun 25, 2018

/retest

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Jun 25, 2018

Member

@thockin @liggitt Now this only support kubelet, since others are without authn/authz

and apiserver, right?

Member

liggitt commented Jun 25, 2018

@thockin @liggitt Now this only support kubelet, since others are without authn/authz

and apiserver, right?

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jun 26, 2018

Member

and apiserver, right?

Yes, it is already supported before. In this pr, as I move glog setter to a common place, I just update apiserver to make use of the new package.

Member

hzxuzhonghu commented Jun 26, 2018

and apiserver, right?

Yes, it is already supported before. In this pr, as I move glog setter to a common place, I just update apiserver to make use of the new package.

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu
Member

hzxuzhonghu commented Jul 27, 2018

kindly ping @thockin @liggitt

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Jul 27, 2018

Member

/lgtm

Member

liggitt commented Jul 27, 2018

/lgtm

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Jul 28, 2018

Member

Thanks @liggitt

@thockin can you approve?

Member

hzxuzhonghu commented Jul 28, 2018

Thanks @liggitt

@thockin can you approve?

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Aug 1, 2018

Member

/retest

Member

hzxuzhonghu commented Aug 1, 2018

/retest

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Aug 1, 2018

Member

ping @lavalamp @thockin for approval

Member

hzxuzhonghu commented Aug 1, 2018

ping @lavalamp @thockin for approval

@sttts

This comment has been minimized.

Show comment
Hide comment
@sttts

sttts Aug 6, 2018

Contributor

/approve

for apiextensions-apiserver.

Contributor

sttts commented Aug 6, 2018

/approve

for apiextensions-apiserver.

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu
Member

hzxuzhonghu commented Aug 7, 2018

ping @kubernetes/sig-node-pr-reviews for kubelet part

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu
Member

hzxuzhonghu commented Aug 7, 2018

/assign @tallclair

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu

hzxuzhonghu Aug 11, 2018

Member

ping @tallclair for kubelet part

Member

hzxuzhonghu commented Aug 11, 2018

ping @tallclair for kubelet part

return "successfully set glog.logging.verbosity to " + val, nil
}),
))
routes.DebugFlags{}.Install(s.Handler.NonGoRestfulMux, "v", routes.StringFlagPutHandler(logs.GlogSetter))

This comment has been minimized.

@tallclair

tallclair Aug 14, 2018

Member

Why is this gated on EnableProfiling?

@tallclair

tallclair Aug 14, 2018

Member

Why is this gated on EnableProfiling?

This comment has been minimized.

@hzxuzhonghu
@hzxuzhonghu
Show outdated Hide outdated staging/src/k8s.io/apiserver/pkg/util/logs/logs.go

@k8s-ci-robot k8s-ci-robot removed the lgtm label Aug 15, 2018

@tallclair

This comment has been minimized.

Show comment
Hide comment
@tallclair

tallclair Aug 15, 2018

Member

/lgtm
(for kubelet)

Member

tallclair commented Aug 15, 2018

/lgtm
(for kubelet)

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 15, 2018

@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Aug 15, 2018

Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, liggitt, sttts, tallclair

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Contributor

k8s-ci-robot commented Aug 15, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, liggitt, sttts, tallclair

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Aug 15, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot commented Aug 15, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Aug 16, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot commented Aug 16, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-merge-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-merge-robot

k8s-merge-robot Aug 16, 2018

Contributor

/test all [submit-queue is verifying that this PR is safe to merge]

Contributor

k8s-merge-robot commented Aug 16, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-merge-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-merge-robot

k8s-merge-robot Aug 16, 2018

Contributor

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

Contributor

k8s-merge-robot commented Aug 16, 2018

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit cffa2ae into kubernetes:master Aug 16, 2018

16 of 18 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-kubemark-e2e-gce-big
Details
pull-kubernetes-local-e2e-containerized Job triggered.
Details
cla/linuxfoundation hzxuzhonghu authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@hzxuzhonghu hzxuzhonghu deleted the hzxuzhonghu:cm-dynamic-loglevel-set branch Aug 16, 2018

@tossmilestone

This comment has been minimized.

Show comment
Hide comment
@tossmilestone

tossmilestone Sep 17, 2018

Contributor

Does this feature can be used in kube-scheduler? @hzxuzhonghu

Contributor

tossmilestone commented Sep 17, 2018

Does this feature can be used in kube-scheduler? @hzxuzhonghu

@hzxuzhonghu

This comment has been minimized.

Show comment
Hide comment
@hzxuzhonghu
Member

hzxuzhonghu commented Sep 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment