-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Allow to suspend chaoskube at certain weekdays #56
Conversation
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.
Your implementation does not appear to take into account the timezone of the personnel it is intended to serve unless they are living in a UTC timezone. So, assuming the servers are all on UTC, the chaos would currently resume for someone in California on Sunday at 4:00 PM (16:00).
@twildeboer I skipped that part because I assumed people would just convert their flags to UTC. However, this doesn't work for weekdays 😂 Thanks! |
chaoskube/chaoskube_test.go
Outdated
} | ||
|
||
validateCandidates(t, chaoskube, []map[string]string{ | ||
{"namespace": "testing", "name": "bar"}, |
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 know which pod will be left if the selection is "random"? Is it because, by testing ahead of time with the same Seed, you know it always terminates the same victim? If so, I get that it works, but doesn't "smell" right. (What if logic in the rand
package changes?) I would consider simply asserting that the number of pods available was reduced by 1 would also be valid.
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.
Good point. I was thinking, that by providing a seed I can control what's the "random" outcome. I used it to test that pods not matching the filters are really ignored (by testing that without the filter and the same seed it gets deleted).
However, for this test it doesn't matter which pod is killed. I'll change it as you suggested.
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.
@twildeboer Where applicable, I removed testing for particular pods in favour of testing just the number of surviving pods. Also removed defining a seed where it doesn't influence the outcome.
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.
Cool.
@twildeboer Thanks a lot for your help getting this in. Your comments are very valuable to me. 😄 |
Glad to help. Thanks for creating chaoskube. |
Allow to suspend chaoskube at certain weekdays
This is an alternative implementation of solely the weekday filter proposed in #54. In my opinion it's simpler to understand and maintain than #54 (It also does less at this point).
It adds a flag to specify a list of weekdays when chaoskube doesn't terminate any pods, e.g. specifying
--excluded-weekdays "sat,sun"
would prevent any chaos on the weekend. I picked theexclude-
-approach so that the default empty--excluded-weekdays=""
would behave as before.@klautcomputing @twildeboer This idea comes directly from your PRs #47 and #54 but I focused on the weekday filter first in order to keep it simpe. I intend to look through your PRs and use them to come up with something similar for the "run-at-this-time-of-day"-flag (i.e. the
--chaos-hrs
). I hope you don't mind.@klautcomputing @twildeboer let me know what you think about this one.