Skip to content
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

Repeat strategy limitation #8

Open
AI-Joe opened this issue Aug 4, 2020 · 8 comments
Open

Repeat strategy limitation #8

AI-Joe opened this issue Aug 4, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@AI-Joe
Copy link

AI-Joe commented Aug 4, 2020

Problem

The current definitions of start and end time boxes the repeat strategy in. Meaning that you could not repeat indefinitely or even for multiple days between certain hours. The use case is to execute experiments on a schedule within business hours/days for an extended or indefinite duration of time.

Current

repeat:
      startTime: "2020-08-03T13:00:00Z"
      endTime: "2020-08-03T21:00:00Z"
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"

This will start at 1pm and end at 8:30pm UTC on the day defined. I'm not really sure where the included days come in to play here. The cron expression created seems like the actual day of the repeat times would not affect the creation of the engines. However, when the engines are actually created it will end the schedule once it is after the user specified end time.

repeat:
      startTime: "2020-08-03T13:00:00Z"
      endTime: "2021-08-03T21:00:00Z"
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"

This example, without temporarily halting the schedule, will execute every 30 minutes from the start to the end time excluding weekends.

Future

repeat:
      startTime: "2020-08-03T13:00:00Z"
      endTime: "2021-08-03T21:00:00Z"
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"
      indefinite: true (default false)

By adding this field the user can opt out of the after end time check, creating an indefinite repeat schedule. Then in the creation of the cron expression you could have something like return fmt.Sprintf("*/%d %d-%d * * %s", interval, startHour, endHour, includedDays), time.Minute * time.Duration(interval), nil

@ksatchit
Copy link
Member

ksatchit commented Aug 4, 2020

Thank you @AI-Joe !! This makes sense, the schema definitely needs more refining. cc: @Sanjay1611

@ksatchit ksatchit added the bug Something isn't working label Aug 4, 2020
@Sanjay1611
Copy link
Contributor

Thank you @AI-Joe !! Its a good catch, We definitely want to improve the schema over time

@Sanjay1611
Copy link
Contributor

An alternate approach can be -
We can allow endTime and startTime to be nil. Then whenever the startTime is nil we will take it as creationTime of the CR and whenever the endTime is nil we will take the schedule to repeated infinitely. For e.g.

repeat:
      startTime: "2020-08-03T13:00:00Z"
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"

This example is not having any endTime. So the schedule will be repeated from the startTime infinitely

repeat:
      endTime: "2021-08-03T21:00:00Z"
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"

This example is not having any startTime. So the schedule will be repeated between creationTime and endTime

repeat:
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"

This example is missing both the timestamps. So the schedule will be repeated from creationTime infinitely

@Sanjay1611
Copy link
Contributor

What are your views on this @AI-Joe

@AI-Joe
Copy link
Author

AI-Joe commented Aug 5, 2020

@Sanjay1611 I think that allowing endTime/startTime to be nil is a better approach!

I only briefly mentioned this in the problem, but allowing an infinite repeat feels incomplete to me without run hour restrictions.

repeat:
      minChaosInterval: "30m"
      instanceCount: "1"
      includedDays: "Mon,Tue,Wed,Thu,Fri"
      includedHours: "13-21" #any cron accepted time annotation in UTC

This example is an infinite repeat that doesn't run after business hours. By not including the includedHours field the user is selecting to run every hour.

Edit -- I forgot that minChaosInterval also supports hours.. so by adding this includedHours field, minChaosInterval could just support the minutes.

@Sanjay1611
Copy link
Contributor

@AI-Joe I think includedHours field would be really helpful. We should have it in our schema.
But minChaosInterval will have to support the hours too. So that we can have minimum number of hours between 2 chaos executions.

@calvinbui
Copy link

+1 to includedHours when we keep it within business hours

+1 to empty startTimes and endTimes

@Sanjay1611
Copy link
Contributor

Will be changing repeat structure to

repeat:
  timeRange:
    startTime:
    endTime:
  properties:
    minChaosINterval:
    instanceCount:
    random:
  workHours:
    includedHours:
  workDays:
    includedDays:

timeRange, workHours and workDays will be optional fields

cc @AI-Joe @calvinbui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants