-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 WatchDeploymentLogsByLabelSelector function #7039
🌱Added WatchDeploymentLogsByLabelSelector function #7039
Conversation
@adilGhaffarDev: This issue is currently awaiting triage. If CAPI contributors determines this is a relevant issue, they will accept it by applying the The 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. |
Hi @adilGhaffarDev. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. |
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.
Looks like a very good idea!
/ok-to-test
8710040
to
34e2b00
Compare
/retest |
34e2b00
to
9c9f05a
Compare
9c9f05a
to
e6abdc7
Compare
8b86052
to
e350817
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.
Some further improvement recommendations:-)
e350817
to
25db926
Compare
Change lgtm to me. |
ab6b959
to
da4158c
Compare
da4158c
to
8e9df77
Compare
Should I create a separate PR for notes or just add it to this PR. |
@adilGhaffarDev Please add them to this PR |
8e9df77
to
881d2df
Compare
/test pull-cluster-api-test-main |
881d2df
to
89b15dd
Compare
Found two other ones. Apart from that it should be really fine now (I also verified the resulting log folder structure and it looks good) |
89b15dd
to
3c636ea
Compare
/retest |
Thank you very much! /lgtm I'll merge through after the tests. Not sure why they weren't triggered. Some GitHub or Prow issue I assume. |
LGTM label has been added. Git tree hash: 2388aa10023fffcacbb32d77a2ee85ecbf6c8859
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
What this PR does / why we need it:
Adds WatchDeploymentLogsByLabelSelector function. Now we can call a function in e2e tests to stream logs of deployments, selected by a label selector, for not requiring to know the exact name of the deployment in the test implementation.
Which issue(s) this PR fixes(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged)*:Fixes #6965