-
Notifications
You must be signed in to change notification settings - Fork 28
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
Event recorder #55
Event recorder #55
Conversation
… targeted Kubernetes objects Signed-off-by: Nick Jüttner <[email protected]>
9d2e2c2
to
d95a1f0
Compare
4931d9e
to
803fe7b
Compare
Signed-off-by: Nick Jüttner <[email protected]>
… targeted Kubernetes objects Signed-off-by: Nick Jüttner <[email protected]>
… targeted Kubernetes objects Signed-off-by: Nick Jüttner <[email protected]>
controller/stackset.go
Outdated
@@ -157,7 +163,7 @@ func (c *StackSetController) Run(ctx context.Context) { | |||
continue | |||
} | |||
|
|||
c.logger.Infof("Adding entry for StackSet %s/%s", stackset.Namespace, stackset.Name) | |||
c.recorder.Eventf(e.StackSet, apiv1.EventTypeNormal, "CreateStackSet", "StackSet '%s/%s' added", stackset.Namespace, stackset.Name) |
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 is more internal logging for the controller. I think it's a bit confusing to log as an event as it will happen every time the controller restarts.
controller/stackset.go
Outdated
c.recorder.Eventf(stackset, | ||
apiv1.EventTypeWarning, | ||
"GetStackSet", | ||
"Failed to get StackSet Object") |
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.
All events in add
, update
and delete
are internal information for the controller. I think they are too "spammy" to log as events. They could potentially cause an event every time the controller is restarted.
Signed-off-by: Nick Jüttner <[email protected]>
d2544b9
to
0f4c417
Compare
kubernetes kubernetes.Interface | ||
stackset stackset.Interface | ||
kubernetes.Interface | ||
stackset stackset.Interface |
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.
can we drop stackset
as well?
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.
How do you mean? We can't do:
type x struct {
kubernetes.Interface
stackset.Interface
}
as the two Interface
field names would overlap.
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.
That's what I meant, thanks.
👍 |
1 similar comment
👍 |
Fix for #12