Added Warning log callback in client-go loading rules #114793

illume · 2023-01-03T22:33:36Z

Also test that the log is not shown when v=1 is not set. Because a klog warning by default is not allowed in library code.

Thanks to Debanitrkl

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #94428

Special notes for your reviewer:

This finishes review requested changes by yliaog on stale PR #108510

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Also test that the log is not shown when v=1 is not set. Because a klog warning by default is not allowed in library code. Co-Authored-By: Debanitrkl <118bm0622@nitrkl.ac.in>

linux-foundation-easycla · 2023-01-03T22:33:41Z

The committers listed above are authorized under a signed CLA.

✅ login: illume / name: René Dudfield (265bce4)

k8s-ci-robot · 2023-01-03T22:33:44Z

Welcome @illume!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-01-03T22:33:45Z

Hi @illume. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

cici37 · 2023-01-10T21:29:33Z

/assign @yliaog
/triage accepted

yliaog · 2023-01-16T18:56:52Z

/ok-to-test

yliaog · 2023-01-19T03:16:28Z

/lgtm
/approve

k8s-ci-robot · 2023-01-19T03:16:34Z

LGTM label has been added.

Git tree hash: 377f22d4d67b813fb1b8449aba7e30f9110917c3

k8s-ci-robot · 2023-01-19T03:16:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: illume, yliaog

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:

~~staging/src/k8s.io/client-go/OWNERS~~ [yliaog]

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

k8s-triage-robot · 2023-01-26T03:26:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-26T06:14:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-26T09:02:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-26T11:50:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-26T14:30:52Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

dims · 2023-01-26T15:31:34Z

/retest

k8s-triage-robot · 2023-01-26T18:10:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-26T20:50:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-26T23:30:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T02:11:54Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T04:57:54Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T07:45:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T10:33:54Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T13:21:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

pacoxu · 2023-01-27T14:35:22Z

/retest

k8s-triage-robot · 2023-01-27T17:07:54Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T19:51:12Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2023-01-27T22:37:12Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-ci-robot · 2023-01-27T23:11:18Z

@illume: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-unit	`265bce4`	link	true	`/test pull-kubernetes-unit`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

aojea · 2023-01-27T23:42:07Z

/hold

the failure is legit

ardaguclu · 2023-03-29T15:53:54Z

/cc

ardaguclu

@illume this PR looks good, it just requires a simple fix in unit tests to move forward. If you don't have any cycles, I can take over your PR.

ardaguclu · 2023-04-04T10:59:27Z

staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go

+	}
+}
+
+func TestNoWarningMissingFiles(t *testing.T) {


This test is failing because it still uses previous test TestWarningMissingFiles's flags.Set("v", "1"). You can assert this by changing the order of these tests.

I'd suggest adding extra flags.Set("v", "0") in this test.

k8s-ci-robot · 2023-05-25T12:35:37Z

PR needs rebase.

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.

illume · 2023-05-25T13:47:33Z

Closing because @ardaguclu took this into #117233

Added Warning log callback in client-go loading rules

265bce4

Also test that the log is not shown when v=1 is not set. Because a klog warning by default is not allowed in library code. Co-Authored-By: Debanitrkl <118bm0622@nitrkl.ac.in>

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. kind/bug Categorizes issue or PR as related to a bug. labels Jan 3, 2023

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 3, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 3, 2023

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jan 3, 2023

k8s-ci-robot requested review from caesarxuchao and yliaog January 3, 2023 22:34

k8s-ci-robot assigned yliaog Jan 10, 2023

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 Jan 10, 2023

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 16, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 19, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2023

k8s-ci-robot requested a review from ardaguclu March 29, 2023 15:53

ardaguclu reviewed Apr 4, 2023

View reviewed changes

ardaguclu mentioned this pull request Apr 12, 2023

Add warning log callback in client-go loading rules #117233

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 25, 2023

illume closed this May 25, 2023

illume deleted the no-print branch May 25, 2023 13:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Warning log callback in client-go loading rules #114793

Added Warning log callback in client-go loading rules #114793

illume commented Jan 3, 2023

linux-foundation-easycla bot commented Jan 3, 2023 •

edited

k8s-ci-robot commented Jan 3, 2023

k8s-ci-robot commented Jan 3, 2023

cici37 commented Jan 10, 2023

yliaog commented Jan 16, 2023

yliaog commented Jan 19, 2023

k8s-ci-robot commented Jan 19, 2023

k8s-ci-robot commented Jan 19, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

dims commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

pacoxu commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-ci-robot commented Jan 27, 2023

aojea commented Jan 27, 2023

ardaguclu commented Mar 29, 2023

ardaguclu left a comment

ardaguclu Apr 4, 2023

k8s-ci-robot commented May 25, 2023

illume commented May 25, 2023

Added Warning log callback in client-go loading rules #114793

Added Warning log callback in client-go loading rules #114793

Conversation

illume commented Jan 3, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

linux-foundation-easycla bot commented Jan 3, 2023 • edited

k8s-ci-robot commented Jan 3, 2023

k8s-ci-robot commented Jan 3, 2023

cici37 commented Jan 10, 2023

yliaog commented Jan 16, 2023

yliaog commented Jan 19, 2023

k8s-ci-robot commented Jan 19, 2023

k8s-ci-robot commented Jan 19, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

dims commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 26, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

pacoxu commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-triage-robot commented Jan 27, 2023

k8s-ci-robot commented Jan 27, 2023

aojea commented Jan 27, 2023

ardaguclu commented Mar 29, 2023

ardaguclu left a comment

Choose a reason for hiding this comment

ardaguclu Apr 4, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented May 25, 2023

illume commented May 25, 2023

linux-foundation-easycla bot commented Jan 3, 2023 •

edited