Design review for scheduled snapshot

[johnelse]

See http://xapi-project.github.io/xapi/design/schedule-snapshot.html

[johnelse]

I think the field names would be more readable after s/schedule_snapshot/snapshot_schedule.

That makes more grammatical sense to me e.g.

"I would like to create a snapshot schedule"

or

"What is the snapshot schedule for this VM?"

Thoughts?

[jonludlam]

• The design doc would benefit from a high-level overview - at the moment the introduction is too terse, and some important details (like what intervals are possible) are hidden.

Datapath Design section

• Under 'datapath design', is the flag an other-config or a new field? We should decide now.

Xapi Changes section

• rename 'is-schedule-snapshot-enabled' to be 'enabled', 'schedule-snapshot-type' to be 'type', etc.
• rename 'snapshot-schedule' to be 'schedule'
• It's not clear how the frequency and schedule interact, nor that the types of 'schedule' are right - perhaps they should all just be integers?
• remove 'is-schedule-snapshot-running' and use the task API to create a task, update progress, listen for cancellation etc.
• rename 'schedule-snapshot-last-run-time' to be 'last-run-time'
• rename 'is-snapshot-from-vmss' to be 'is-vmss-snapshot'
• why is there 'alarm config' in here? Wouldn't it be easier to use the existing 'message' APIs?

  New APIs section

APIs will need changing to reflect the above.

• what's the use case for 'vmss_snapshot_now'?
• why does 'vmss_create_alert' exist? Couldn't we use the existing message APIs?

[sharady]

Thanks @jonludlam and @johnelse for the review. I have created a PR #190 to reflect the new changes required.
Here are few answers for the concerns:

1. Under 'datapath design', is the flag an other-config or a new field? We should decide now.
   Discussed with sharath, This field will not be required, As there are messages generated on each VMSS run.
2. It's not clear how the frequency and schedule interact, nor that the types of 'schedule' are right - perhaps they should all just be integers?
   frequency -> Just stats the snapshot should be taken on [hourly, daily, weekly] basis.
   schedule -> Stats the clear configuration.
   If hourly then on which quarter of the hour.
   If daily then on which hour and quarter of the hour.
   If weekly then on which days, hour and quarter of the hour.
3. why is there 'alarm config' in here? Wouldn't it be easier to use the existing 'message' APIs? # New APIs section
   alarm_config is required for the mail configuration, This was part of VMPR which is backported here to send mails if mail configuration is setup for VMSS.
4. what's the use case for 'vmss_snapshot_now'?
   Since the vmss plugin runs every 15mins, There is an additional provision to run it instantly when vmss is created. In this case snapshots will be created instantly for the first time and then later on configured intervals.

[jonludlam]

Thanks @sharady.

I'm still unsure about the type of schedule. It's a bit of a shame we're not using cron syntax directly. In any case, if we really want to be specifying day-of-week using day names, we should use an enum, and perhaps have 3 namespaced fields rather than a string->* map.

[jonludlam]

Also, I still don't see the need to have a second place to specify email configuration. We already have a mechanism for emailing - messages. We should just be using that.

[sharady]

@jonludlam Thanks for the further comments :)
Regarding emailing thing. We can set the pool wide system mails via pool other-config, thats true.
But here I am not sure about the use case:
1st case: If we need multiple mail configuration for different VMSS then we need this configuration on VMSS object.
2nd case: If we need a single mail configuration for all the VMSS then we are good to go with pool other-config mail configuration.
Since in existing VMPR we had mail configuration for each vmpp. I have kept it like that.

[jonludlam]

OK. Let's have the schedule be a string -> string map. valid keys are 'minute', 'hour', 'day-of-week'. In a future (simplified) version, we can maybe add a new enum to the frequency: 'cron'. We can then interpret the fields from the schedule be the relevant fields from a crontab entry: the fieldnames I've suggested come from the crontab man page: http://www.freebsd.org/cgi/man.cgi?crontab(5)

I suggest we stick with numbers for minute, hour, day-of-week for now for simplicity.

[sharady]

Thanks @jonludlam for the review, I have updated the same on PR: #191