-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Added sub-step logging to adm init step on start #9904
Added sub-step logging to adm init step on start #9904
Conversation
Hi @spowelljr. 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 Once the patch is verified, the new status will be reflected by the 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. |
Can one of the admins verify this patch? |
1f34c5f
to
cf35c15
Compare
cf35c15
to
b33f40d
Compare
7adccf9
to
3bb8810
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add integration test to error_spam_test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ensure to run make generate-docs
after the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let ensrue the sub steps are spining by adding a Spining Option step
f92c383
to
9cf6e82
Compare
06a8f38
to
6f635ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding unit test and great work, I approve this PR after all other reviews are approved.
final approve leaving for @tstromberg or @priyawadhwa
a6509d1
to
d3f93ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with minor nit.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, spowelljr, tstromberg 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 |
d3f93ae
to
5f0144a
Compare
Fixes: #9779
Added StartCmd and WaitCmd to CommandRunner interface to allow real-time streaming of logs.
Used these two new methods to add sub-step logging to adm init on start, these steps also output to JSON.