fix: use kube logger with status waiter#31717
Merged
scottrigby merged 2 commits intohelm:mainfrom Jan 23, 2026
Merged
Conversation
2 tasks
Contributor
|
Hello @AustinAbro321 Inspired by my snipet. I tested your code, and it is working as expected: Edit: I am wondering about the duplication in logs of deployment object. Here is during a |
7708d02 to
743f851
Compare
Contributor
Author
|
@benoittgt Yeah the logging strategy can be improved, though I believe it's a different scope than this PR. Right now it's firing a log every time the statusobserver is called. The benefit of this is that a user will always know at least one resource that they're waiting on. The downside is that it fires a ton of logs and the more resources in the chart the more logs it writes. What do you think of a separate goroutine that will log a not ready resource every X seconds and removing the log from the status observer? This would mirror the behavior of the old Helm status logger. I could spin up a separate PR for this |
Contributor
Completely agree.
I think it's a good idea. But maybe we should get other points of view first, to avoid unnecessary work for you. |
da9b162 to
5c50f3e
Compare
gjenkins8
previously approved these changes
Jan 21, 2026
Member
|
@AustinAbro321 -- changes look good, can you please rebase / fix conflicts |
Signed-off-by: Austin Abro <austinabro321@gmail.com>
5b7efdf to
63b40a7
Compare
Contributor
Author
|
@gjenkins8 @TerryHowe Changes are rebased, codeql seems to be failing for unrelated reasons |
Signed-off-by: Austin Abro <austinabro321@gmail.com>
scottrigby
approved these changes
Jan 22, 2026
Member
There was a problem hiding this comment.
nice work, lgtm. though is this a feature or a bug that we didn't catch in the earlier fix: #31411?
Contributor
@scottrigby Well observed, I think the bug was introduced here: #13604 And yes, could have been fixed in #31411 |
TerryHowe
approved these changes
Jan 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Currently unless the global slogger is set, SDK users will not receive any logs at all from the waiter. This is critical as several minutes can go by without any feedback to users. Relates to #31585
If applicable:
docs neededlabel should be applied if so)