Skip to content

Add similar ready debug logs as statefulSetReady for job and deployment#30685

Closed
benoittgt wants to merge 2 commits intohelm:mainfrom
benoittgt:add-missing-ready-logs
Closed

Add similar ready debug logs as statefulSetReady for job and deployment#30685
benoittgt wants to merge 2 commits intohelm:mainfrom
benoittgt:add-missing-ready-logs

Conversation

@benoittgt
Copy link
Copy Markdown
Contributor

@benoittgt benoittgt commented Mar 19, 2025 •
edited
Loading

What this PR does / why we need it:

⚠️ Please review first: #30698

This PR is build on top of #30698.

We are consuming debug logs on our project based on the helm SDK. We would like to add even more logs when resources like deployments and jobs are ready. At the moment there is a difference between statefullsets and deployments for example.

Deployment is not ready: l-app-staging/g-station-staging-lfapp-web. 1 out of 2 expected pods are ready
Deployment is not ready: l-app-staging/g-station-staging-lfapp-web. 1 out of 2 expected pods are ready
StatefulSet is ready: l-app-staging/g-station-staging-rediscache-master. 1 out of 1 expected pods are ready
StatefulSet is ready: l-app-staging/g-station-staging-redis-master. 1 out of 1 expected pods are ready

Thanks in advance for the review.

If approved, I can backport it to v3.

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 19, 2025
@benoittgt benoittgt mentioned this pull request Mar 20, 2025
Merged
3 tasks
robertsirc
robertsirc requested changes Mar 20, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@robertsirc robertsirc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello, thank you for your PR. there are a few things that we need to have addressed before merging.

TerryHowe
TerryHowe reviewed Mar 20, 2025
View reviewed changes
@benoittgt
Copy link
Copy Markdown
Contributor Author

benoittgt commented Mar 21, 2025 •
edited
Loading

I will open another PR to migrate to slog this file then re-open this PR.

EDIT: #30698

@benoittgt benoittgt marked this pull request as draft March 21, 2025 10:13
@benoittgt
No longer pass a log to ReadyChecker
36b1dce 
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
@benoittgt benoittgt mentioned this pull request Mar 21, 2025
Closed
3 tasks
@benoittgt benoittgt force-pushed the add-missing-ready-logs branch from aa3efb9 to 6420565 Compare March 21, 2025 12:20
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 21, 2025
@benoittgt
Migrate to slog in ready.go
44faf65 
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
@benoittgt benoittgt force-pushed the add-missing-ready-logs branch from 6420565 to 44faf65 Compare March 21, 2025 12:20
@benoittgt benoittgt mentioned this pull request Mar 25, 2025
Merged
3 tasks
@benoittgt
Copy link
Copy Markdown
Contributor Author

benoittgt commented Mar 25, 2025

Closing for the moment. Wil reopen when work on kube and slog will be done.

@benoittgt benoittgt closed this Mar 25, 2025
@benoittgt benoittgt deleted the add-missing-ready-logs branch March 25, 2025 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TerryHowe TerryHowe TerryHowe left review comments

@robertsirc robertsirc robertsirc requested changes

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

No one assigned

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@benoittgt @TerryHowe @robertsirc