Skip to content
This repository was archived by the owner on Oct 3, 2020. It is now read-only.

Adding support for events #89

Closed
wants to merge 7 commits into from

Conversation

gregsidelinger
Copy link
Contributor

Adding basic event support for #80.

Events on ScaleUp and ScaleDown are generated and show up when describing the object. An event is also generated if the time spec value format is bad. This way users are notified on a bad configuration if they do no have access to the controller logs.

ScaleUp, ScaleDown and misconfigured time spec events are currently generated

Signed-off-by: Greg Sidelinger <gate@ilive4code.net>
Signed-off-by: Greg Sidelinger <gate@ilive4code.net>
Signed-off-by: Greg Sidelinger <gate@ilive4code.net>
@coveralls
Copy link

coveralls commented Feb 15, 2020

Coverage Status

Coverage decreased (-0.5%) to 94.539% when pulling 69cacd7 on gregsidelinger:events into 514065c on hjacobs:master.

Signed-off-by: Greg Sidelinger <gate@ilive4code.net>
This should get coverage back to where it used to be
@gregsidelinger
Copy link
Contributor Author

I'm not sure how to get the coverage any higher. Let me know if you have any concerns with the patch.

@gregsidelinger
Copy link
Contributor Author

@hjacobs do you see any issues with this PR? I can not get the coverage percentage higher as it is no longer hitting the old exception block on the one failure test for a bad time format since that triggers an event now.

@hjacobs
Copy link
Owner

hjacobs commented Mar 28, 2020

@gregsidelinger thanks for the PR!

Can you make this behavior optional via a command line option? Not everybody might want to have event support (or grant rights to create events).

@gregsidelinger
Copy link
Contributor Author

@hjacobs I can add a command line switch. Do you want it off or on by default? My vote is on but I like events as its a way for users who do not have access to the logs to get feed back. Especially for misconfigured date/time fields.

@hjacobs
Copy link
Owner

hjacobs commented Mar 28, 2020

@gregsidelinger I also like events, but this might cause a lot of events for situations where kube-downscaler is "fighting" against another controller, i.e. where each downscale is reverted by a different controller. I have observed these situations and that's why I would make events opt-in.
Events should also be aggregated (count incremented).

Signed-off-by: Greg Sidelinger <gate@ilive4code.net>
…nto events

Signed-off-by: Greg Sidelinger <gate@ilive4code.net>
@gregsidelinger
Copy link
Contributor Author

@hjacobs I added an --enable-events flag and merged it with your latest changes.

I do have it looking up existing events and bumping the last seen timestamp and count for repeated events. While testing I could not find any other way to have the count increment. I'm not sure if there is a better way to do this using pykube. I use kube builder for all other controllers I have worked on and they have an event recorder which seems to take care of that for you.

@hjacobs hjacobs mentioned this pull request Oct 2, 2020
@hjacobs
Copy link
Owner

hjacobs commented Oct 2, 2020

Replaced by #127

@hjacobs hjacobs closed this Oct 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants