-
Notifications
You must be signed in to change notification settings - Fork 42.4k
Send update events from service-controller. #20607
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
Send update events from service-controller. #20607
Conversation
|
Labelling this PR as size/S |
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.
Reason should be camel case https://github.com/kubernetes/kubernetes/blob/master/docs/devel/api-conventions.md#events
|
These could all have the same reason UpdatingService and describe why in the message |
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.
This could use a better message
9ff93fc to
33654c8
Compare
I tried that initially and though having UpdateService and a message like foo changed from x -> y was redundant. Thoughts? |
|
GCE e2e test build/test passed for commit 9ff93fcd4dca3acf00aae7e266ec806cdfe70da1. |
|
It would make searching/grepping somewhat easier to have a string that is common across all of these events. It's not super important |
|
GCE e2e test build/test passed for commit 33654c8. |
|
@k8s-bot test this issue: #IGNORE Tests have been pending for 24 hours |
|
GCE e2e test build/test passed for commit 33654c8. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e test build/test passed for commit 33654c8. |
|
Automatic merge from submit-queue |
Auto commit by PR queue bot
@maisem since I noticed you took #20547