-
Notifications
You must be signed in to change notification settings - Fork 255
Extend mtbf time range #168
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
base: master
Are you sure you want to change the base?
Conversation
0c87b72 to
175ec88
Compare
|
Look good |
| _, err := time.ParseDuration(mtbf) | ||
| if err != nil { | ||
| return -1, err | ||
| return "", fmt.Errorf("error parsing mtbf %s: %v", mtbf, err) |
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 makes it a breaking change. It would be good if we could still use values without a unit which would default to using days.
| **`kube-monkey/enabled`**: Set to **`"enabled"`** to opt-in to kube-monkey | ||
| **`kube-monkey/mtbf`**: Mean time between failure (in days). For example, if set to **`"3"`**, the k8s app can expect to have a Pod | ||
| killed approximately every third weekday. | ||
| **`kube-monkey/mtbf`**: Mean time between failure (in hours/minutes etc. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".). For example, if set to **`"3h"`**, the k8s app can expect to have a Pod |
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.
Using nano seconds for example would not be a valid use case. I think we should restrict to minutes/hours/days.
| secOffset := r.Intn(60) | ||
| return time.Now().In(loc).Add(time.Duration(secOffset) * time.Second) | ||
| } | ||
| return calendar.RandomTimeInRange(config.StartHour(), config.EndHour(), loc) |
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.
Is RandomTimeInRange still used?
| return rangeStart.Add(offsetDuration) | ||
| } | ||
|
|
||
| func CustzRandomTimeInRange(mtbf string, startHour, endHour int, loc *time.Location) time.Time { |
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.
Would you mind using a different name? Not entirely sure what custz means.
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.
I'm a bit confused by the way this method is used.
If I set a mtbf of 1h I would expect my victim to be attacked roughly every hour, or 24 times during a single day.
However looking at the code, this will only schedule a single attack for that day.
Can you please clarify if that's the intention?
|
Hello, im experiencing the same issue/limitation. |
This PR change the defination of "kube-monkey/mtbf".
When we use kube-monkey to kill pod in k8s cluster, we find it too slow to generate a failure, because the label 'kube-monkey/mtbf' only support to generate one failure in days.
So I extend the time range of this label, to support failure in minutes, hours, and calculate mean time of the specified hour/minute.
available usage as follows:
kube-monkey/mtbf: "4h"
kube-monkey/mtbf: "30m"