Add minimum age check for pods #86
Conversation
chaoskube/chaoskube.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
Let's keep this function high-level and push most of into the filterBy... functions.
How about changing this to:
pods = filterByMinimumAge(pods, c.MinimumAge) // you can remove the `err`, I accidentally introduced it.and then in filterByMinimumAge
func filterByMinimumAge(pods []v1.Pod, age time.Time) []v1.Pod {
if age == 0 {
return pods // early return unfiltered list
}
for _, pod := range pods {
// ... like you already have
chaoskube/chaoskube.go
Outdated
| @@ -41,6 +41,8 @@ type Chaoskube struct { | |||
| DryRun bool | |||
| // a function to retrieve the current time | |||
| Now func() time.Time | |||
| // minimum age of pods to consider | |||
| MinimumAge time.Duration | |||
There was a problem hiding this comment.
Let's group the fields a little. Can we put this after Timezone and before Logger (you would also change the order of the function args, flags etc.)
chaoskube/chaoskube_test.go
Outdated
| @@ -40,6 +41,7 @@ func (suite *Suite) TestNew() { | |||
| excludedWeekdays = []time.Weekday{time.Friday} | |||
| excludedTimesOfDay = []util.TimePeriod{util.TimePeriod{}} | |||
| excludedDaysOfYear = []time.Time{time.Now()} | |||
| minimumAge = time.Duration(0) | |||
There was a problem hiding this comment.
I like to put a specific value here to ensure it's really tested. I think time.Duration(0) is the default value for this type. How about using time.Duration(42) as the value 😃
chaoskube/chaoskube_test.go
Outdated
| }, | ||
| // minimum age set, but pod is too young | ||
| { | ||
| time.Hour * 12, |
There was a problem hiding this comment.
Thanks for the tests! For the sake of simplicity can we change the minimumAge values to something smaller like 1h or so? I think it will be easier to understand them then.
bakins
force-pushed
the
filter-min-age
branch
2 times, most recently
from
58b4b2a
to
f242fad
Compare
|
@bakins Thanks a lot! |
For #85
Optionally filter pods based on age, excluding those that are not older than the passed minimum age.
I added a test specifically for the age check.