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

client-go prints a klog warning by default in library code which is not allowed #94428

Closed
smarterclayton opened this issue Sep 2, 2020 · 23 comments · Fixed by #117233
Closed

client-go prints a klog warning by default in library code which is not allowed #94428

smarterclayton opened this issue Sep 2, 2020 · 23 comments · Fixed by #117233
Assignees
@illume @soltysh @serathius @sayanibhattacharjee @Debanitrkl
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@smarterclayton
Copy link
Contributor

smarterclayton commented Sep 2, 2020 •
edited

What happened:

A code change was merged that used klog.Warningf in library code by default (NewDefaultClientLoadingRules changed in a way that impacted all code in the world which uses that default, which is pretty common), which is not allowed by convention in Kube because client libraries may be used in UIs that wish to control output (klog is not consistent with other output). The code change should have had a callback that allowed clients to optionally decide to print, not print to Warningf by default.

#78185 (comment)

We should also document the convention for other reviewers that logging by default in client libraries is disallowed.

For instance, a callback would have allowed kubectl to print a human readable message warning: KUBECONFIG does not exist in general, and in specific cases (like kubectl config or login flows where KUBECONFIG not existing is totally reasonable) not print a message.

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version):
  • Cloud provider or hardware configuration:
  • OS (e.g: cat /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Network plugin and version (if this is a network-related bug):
  • Others:
@smarterclayton smarterclayton added the kind/bug Categorizes issue or PR as related to a bug. label Sep 2, 2020
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 2, 2020
@smarterclayton smarterclayton mentioned this issue Sep 2, 2020
Merged
@smarterclayton
Copy link
Contributor Author

/sig cli

@k8s-ci-robot k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 2, 2020
@soltysh soltysh self-assigned this Sep 2, 2020
@dims
Copy link
Member

dims commented Sep 25, 2020

/good-first-issue

@k8s-ci-robot
Copy link
Contributor

@dims:
This request has been marked as suitable for new contributors.

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-good-first-issue command.

In response to this:

/good-first-issue

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 good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Sep 25, 2020
@kaleocheng
Copy link
Contributor

Hi @soltysh are you working on this issue already? If not I'd like to tackle this issue :)

@mlavacca
Copy link
Member

/assign

@mlavacca mlavacca mentioned this issue Oct 13, 2020
Closed
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 11, 2021
@sayanibhattacharjee
Copy link

/assign

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 11, 2021
@k8s-ci-robot k8s-ci-robot mentioned this issue Feb 12, 2021
Closed
@vorburger
Copy link
Contributor

/assign

@vorburger vorburger mentioned this issue Feb 12, 2021
Merged
@ehashman
Copy link
Member

This is now documented by kubernetes/community#5492

/close

@k8s-ci-robot
Copy link
Contributor

@ehashman: Closing this issue.

In response to this:

This is now documented by kubernetes/community#5492

/close

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.

@vorburger
Copy link
Contributor

/reopen strictly speaking kubernetes/community#5492 only documented the convention in general, but the specific problem raised here isn't addressed yet - #95517 and #99033 are 2 PRs open about it.

@k8s-ci-robot
Copy link
Contributor

@ehashman: Reopened this issue.

In response to this:

/reopen

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 reopened this Mar 15, 2021
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 15, 2021
@ehashman
Copy link
Member

/remove-lifecycle rotten
/assign @serathius

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Mar 15, 2021
@ehashman
Copy link
Member

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 15, 2021
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 13, 2021
@serathius
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 14, 2021
@serathius
Copy link
Contributor

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 14, 2021
@Debanitrkl
Copy link
Member

/assign

@Debanitrkl Debanitrkl mentioned this issue Jun 30, 2021
Closed
@Debanitrkl Debanitrkl mentioned this issue Sep 9, 2021
Closed
@vorburger vorburger removed their assignment Oct 30, 2021
@mlavacca mlavacca removed their assignment Jan 19, 2022
@Debanitrkl Debanitrkl mentioned this issue Mar 4, 2022
Closed
@Aryan-Deshpande
Copy link

Hey is this issue still open to work with ?

@SaarthakMaini
Copy link

Hello Team Kubernetes,

I am a beginner and would like to contribute to this repository.

May I begin with this issue if possible?

Thank You

@illume illume mentioned this issue Jan 3, 2023
Closed
@illume
Copy link

illume commented Jan 3, 2023

/assign

@ardaguclu ardaguclu mentioned this issue Apr 12, 2023
Merged
@liggitt liggitt mentioned this issue May 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@illume illume

@soltysh soltysh

@serathius serathius

@sayanibhattacharjee sayanibhattacharjee

@Debanitrkl Debanitrkl

Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone