Fixes #1134, fixed support for windowed schedule

cmd/snapctl/task.go

@@ -170,15 +170,15 @@ func (t *task) setWindowedSchedule(start *time.Time, stop *time.Time, duration *
 		// if start is set and stop is not then use duration to create stop
 		if start != nil && stop == nil {
 			newStop := start.Add(*duration)
-			t.Schedule.StartTime = start
-			t.Schedule.StopTime = &newStop
+			t.Schedule.StartTimestamp = start
+			t.Schedule.StopTimestamp = &newStop
 			return nil
 		}
 		// if stop is set and start is not then use duration to create start
 		if stop != nil && start == nil {
 			newStart := stop.Add(*duration * -1)
-			t.Schedule.StartTime = &newStart
-			t.Schedule.StopTime = stop
+			t.Schedule.StartTimestamp = &newStart
+			t.Schedule.StopTimestamp = stop
 			return nil
 		}
 		// otherwise, the start and stop are both undefined but a duration was passed in,
@@ -187,19 +187,19 @@ func (t *task) setWindowedSchedule(start *time.Time, stop *time.Time, duration *
 		// and the duration
 		newStart := time.Now().Add(createTaskNowPad)
 		newStop := newStart.Add(*duration)
-		t.Schedule.StartTime = &newStart
-		t.Schedule.StopTime = &newStop
+		t.Schedule.StartTimestamp = &newStart
+		t.Schedule.StopTimestamp = &newStop
 		return nil
 	}
 	// if a start date/time was specified, we will use it to replace
 	// the current schedule's start date/time
 	if start != nil {
-		t.Schedule.StartTime = start
+		t.Schedule.StartTimestamp = start
 	}
 	// if a stop date/time was specified, we will use it to replace the
 	// current schedule's stop date/time
 	if stop != nil {
-		t.Schedule.StopTime = stop
+		t.Schedule.StopTimestamp = stop
 	}
 	// if we get this far, then just return a nil error (indicating success)
 	return nil

core/schedule.go

@@ -21,21 +21,26 @@ package core
 
 import (
 	"errors"
+	"fmt"
 	"time"
 
 	"github.com/intelsdi-x/snap/pkg/schedule"
 )
 
 type Schedule struct {
-	Type           string `json:"type,omitempty"`
-	Interval       string `json:"interval,omitempty"`
-	StartTimestamp *int64 `json:"start_timestamp,omitempty"`
-	StopTimestamp  *int64 `json:"stop_timestamp,omitempty"`
+	Type           string     `json:"type,omitempty"`
+	Interval       string     `json:"interval,omitempty"`
+	StartTimestamp *time.Time `json:"start_timestamp,omitempty"`
+	StopTimestamp  *time.Time `json:"stop_timestamp,omitempty"`
 }
 
 func makeSchedule(s Schedule) (schedule.Schedule, error) {
 	switch s.Type {
 	case "simple":
+		if s.Interval == "" {
+			return nil, errors.New("missing `interval` in configuration of simple schedule")
+		}
+
 		d, err := time.ParseDuration(s.Interval)
 		if err != nil {
 			return nil, err
@@ -48,24 +53,22 @@ func makeSchedule(s Schedule) (schedule.Schedule, error) {
 		}
 		return sch, nil
 	case "windowed":
+		if s.StartTimestamp == nil || s.StopTimestamp == nil || s.Interval == "" {
+			errmsg := fmt.Sprintf("missing parameter/parameters in configuration of windowed schedule,"+
+				"start_timestamp: %s, stop_timestamp: %s, interval: %s",
+				s.StartTimestamp, s.StopTimestamp, s.Interval)
+			return nil, errors.New(errmsg)
+		}
+
 		d, err := time.ParseDuration(s.Interval)
 		if err != nil {
 			return nil, err
 		}
 
-		var start, stop *time.Time
-		if s.StartTimestamp != nil {
-			t := time.Unix(*s.StartTimestamp, 0)
-			start = &t
-		}
-		if s.StopTimestamp != nil {
-			t := time.Unix(*s.StopTimestamp, 0)
-			stop = &t
-		}
 		sch := schedule.NewWindowedSchedule(
 			d,
-			start,
-			stop,
+			s.StartTimestamp,
+			s.StopTimestamp,
 		)
 
 		err = sch.Validate()
@@ -75,7 +78,7 @@ func makeSchedule(s Schedule) (schedule.Schedule, error) {
 		return sch, nil
 	case "cron":
 		if s.Interval == "" {
-			return nil, errors.New("missing cron entry")
+			return nil, errors.New("missing `interval` in configuration of cron schedule")
 		}
 		sch := schedule.NewCronSchedule(s.Interval)
 

core/schedule_small_test.go

@@ -43,6 +43,14 @@ func TestMakeSchedule(t *testing.T) {
 		So(err.Error(), ShouldEqual, fmt.Sprintf("unknown schedule type %s", DUMMY_TYPE))
 	})
 
+	Convey("Simple schedule with missing interval in configuration", t, func() {
+		sched1 := &Schedule{Type: "simple"}
+		rsched, err := makeSchedule(*sched1)
+		So(rsched, ShouldBeNil)
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldEqual, "missing `interval` in configuration of simple schedule")
+	})
+
 	Convey("Simple schedule with bad duration", t, func() {
 		sched1 := &Schedule{Type: "simple", Interval: "dummy"}
 		rsched, err := makeSchedule(*sched1)
@@ -67,73 +75,87 @@ func TestMakeSchedule(t *testing.T) {
 		So(rsched.GetState(), ShouldEqual, 0)
 	})
 
+	Convey("Windowed schedule with missing interval", t, func() {
+		sched1 := &Schedule{Type: "windowed"}
+		rsched, err := makeSchedule(*sched1)
+		So(rsched, ShouldBeNil)
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldStartWith, "missing parameter/parameters in configuration of windowed schedule")
+	})
+
 	Convey("Windowed schedule with bad duration", t, func() {
-		sched1 := &Schedule{Type: "windowed", Interval: "dummy"}
+		now := time.Now()
+		sched1 := &Schedule{Type: "windowed", Interval: "dummy", StartTimestamp: &now, StopTimestamp: &now}
 		rsched, err := makeSchedule(*sched1)
 		So(rsched, ShouldBeNil)
 		So(err, ShouldNotBeNil)
 		So(err.Error(), ShouldStartWith, "time: invalid duration ")
 	})
 
 	Convey("Windowed schedule with invalid duration", t, func() {
-		sched1 := &Schedule{Type: "windowed", Interval: "-1s"}
+		startTime := time.Now().Add(time.Minute)
+		stopTime := time.Now().Add(2 * time.Minute)
+		sched1 := &Schedule{Type: "windowed", Interval: "-1s", StartTimestamp: &startTime, StopTimestamp: &stopTime}
 		rsched, err := makeSchedule(*sched1)
 		So(rsched, ShouldBeNil)
 		So(err, ShouldNotBeNil)
 		So(err.Error(), ShouldEqual, "Interval must be greater than 0")
 	})
 
-	Convey("Windowed schedule with stop in the past", t, func() {
+	Convey("Windowed schedule with missing start_timestamp", t, func() {
 		now := time.Now()
-		startSecs := now.Unix()
-		stopSecs := startSecs - 3600
-		sched1 := &Schedule{Type: "windowed", Interval: "1s",
-			StartTimestamp: &startSecs, StopTimestamp: &stopSecs}
+		sched1 := &Schedule{Type: "windowed", Interval: "1s", StopTimestamp: &now}
 		rsched, err := makeSchedule(*sched1)
 		So(rsched, ShouldBeNil)
 		So(err, ShouldNotBeNil)
-		So(err.Error(), ShouldEqual, "Stop time is in the past")
+		So(err.Error(), ShouldStartWith, "missing parameter/parameters in configuration of windowed schedule")
 	})
 
-	Convey("Windowed schedule with stop before start", t, func() {
+	Convey("Windowed schedule with missing stop_timestamp", t, func() {
 		now := time.Now()
-		startSecs := now.Unix()
-		stopSecs := startSecs + 600
-		startSecs = stopSecs + 600
+		sched1 := &Schedule{Type: "windowed", Interval: "1s", StartTimestamp: &now}
+		rsched, err := makeSchedule(*sched1)
+		So(rsched, ShouldBeNil)
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldStartWith, "missing parameter/parameters in configuration of windowed schedule")
+	})
+
+	Convey("Windowed schedule with stop in the past", t, func() {
+		startTime := time.Now().Add(time.Minute)
+		stopTime := time.Now().Add(-60 * time.Minute)
 		sched1 := &Schedule{Type: "windowed", Interval: "1s",
-			StartTimestamp: &startSecs, StopTimestamp: &stopSecs}
+			StartTimestamp: &startTime, StopTimestamp: &stopTime}
 		rsched, err := makeSchedule(*sched1)
 		So(rsched, ShouldBeNil)
 		So(err, ShouldNotBeNil)
-		So(err.Error(), ShouldEqual, "Stop time cannot occur before start time")
+		So(err.Error(), ShouldEqual, "Stop time is in the past")
 	})
 
 	Convey("Windowed schedule with stop before start", t, func() {
-		now := time.Now()
-		startSecs := now.Unix()
-		stopSecs := startSecs + 600
+		startTime := time.Now().Add(2 * time.Minute)
+		stopTime := time.Now().Add(1 * time.Minute)
 		sched1 := &Schedule{Type: "windowed", Interval: "1s",
-			StartTimestamp: &startSecs, StopTimestamp: &stopSecs}
+			StartTimestamp: &startTime, StopTimestamp: &stopTime}
 		rsched, err := makeSchedule(*sched1)
-		So(err, ShouldBeNil)
-		So(rsched, ShouldNotBeNil)
-		So(rsched.GetState(), ShouldEqual, 0)
+		So(rsched, ShouldBeNil)
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldEqual, "Stop time cannot occur before start time")
 	})
 
-	Convey("Cron schedule with bad duration", t, func() {
-		sched1 := &Schedule{Type: "cron", Interval: ""}
+	Convey("Cron schedule with missing interval duration", t, func() {
+		sched1 := &Schedule{Type: "cron"}
 		rsched, err := makeSchedule(*sched1)
 		So(rsched, ShouldBeNil)
 		So(err, ShouldNotBeNil)
-		So(err.Error(), ShouldEqual, "missing cron entry")
+		So(err.Error(), ShouldEqual, "missing `interval` in configuration of cron schedule")
 	})
 
 	Convey("Cron schedule with invalid duration", t, func() {
-		sched1 := &Schedule{Type: "windowed", Interval: "-1s"}
+		sched1 := &Schedule{Type: "cron", Interval: "-1 -2 -3 -4 -5 -6"}
 		rsched, err := makeSchedule(*sched1)
 		So(rsched, ShouldBeNil)
 		So(err, ShouldNotBeNil)
-		So(err.Error(), ShouldEqual, "Interval must be greater than 0")
+		So(err.Error(), ShouldStartWith, "Failed to parse int from")
 	})
 
 	Convey("Cron schedule with too few fields entry", t, func() {

docs/TASKS.md

@@ -51,7 +51,28 @@ The header contains a version, used to differentiate between versions of the tas
 
 The schedule describes the schedule type and interval for running the task.  The type of a schedule could be a simple "run forever" schedule, which is what we see above as `"simple"` or something more complex.  Snap is designed in a way where custom schedulers can easily be dropped in.  If a custom schedule is used, it may require more key/value pairs in the schedule section of the manifest.  At the time of this writing, Snap has three schedules:
 - **simple schedule** which is described above,
-- **window schedule** which adds a start and stop time,
+- **window schedule** which adds a start and stop time for the task. The time must be given as a quoted string in [RFC 3339](https://www.ietf.org/rfc/rfc3339.txt) format, for example with specific timezone offset:
+```json
+    "version": 1,
+    "schedule": {
+        "type": "windowed",
+        "interval": "1s",
+        "start_timestamp": "2016-10-27T16:39:57+01:00",
+        "stop_timestamp": "2016-10-28T16:39:57+01:00"
+    },
+    "max-failures": 10,
+```
+or without timezone offset (in that cases uppercase'Z' must be present):
+```json
+    "version": 1,
+    "schedule": {
+        "type": "windowed",
+        "interval": "1s",
+        "start_timestamp": "2016-10-27T16:39:57Z",
+        "stop_timestamp": "2016-10-28T16:39:57Z"
+    },
+    "max-failures": 10,
+```
 - **cron schedule** which supports cron-like entries in ```interval``` field, like in this example (workflow will fire every hour on the half hour):
 ```json
     "version": 1,

mgmt/rest/client/client_func_test.go

@@ -348,6 +348,49 @@ func TestSnapClient(t *testing.T) {
 				So(ttb.Err, ShouldNotBeNil)
 			})
 
+			Convey("Creating tasks with different schedule configuration", func() {
+				Convey("Creating a task with missing parameter (interval) for simple schedule", func() {
+					incorrectSchedule := &Schedule{Type: "simple"}
+					tt := c.CreateTask(incorrectSchedule, wf, "baron", "", true, 0)
+					So(tt.Err, ShouldNotBeNil)
+				})
+
+				Convey("Creating a task with missing parameters (start_timestamp and stop_timestamp) "+
+					"for windowed schedule", func() {
+					incorrectSchedule := &Schedule{Type: "windowed", Interval: "1s"}
+					tt := c.CreateTask(incorrectSchedule, wf, "baron", "", true, 0)
+					So(tt.Err, ShouldNotBeNil)
+				})
+
+				Convey("Creating a task with missing parameter (interval) for cron schedule", func() {
+					incorrectSchedule := &Schedule{Type: "cron"}
+					tt := c.CreateTask(incorrectSchedule, wf, "baron", "", true, 0)
+					So(tt.Err, ShouldNotBeNil)
+				})
+
+				Convey("Creating a task with correct configuration for simple schedule", func() {
+					correctSchedule := &Schedule{Type: "simple", Interval: "1s"}
+					tt := c.CreateTask(correctSchedule, wf, "baron", "", true, 0)
+					So(tt.Err, ShouldBeNil)
+				})
+
+				Convey("Creating a task with correct configuration for windowed schedule", func() {
+					startTime := time.Now().Add(time.Minute)
+					stopTime := time.Now().Add(2 * time.Minute)
+					correctSchedule := &Schedule{Type: "windowed", Interval: "1s",
+						StartTimestamp: &startTime,
+						StopTimestamp:  &stopTime}
+					tt := c.CreateTask(correctSchedule, wf, "baron", "", true, 0)
+					So(tt.Err, ShouldBeNil)
+				})
+
+				Convey("Creating a task with correct configuration for cron schedule", func() {
+					correctSchedule := &Schedule{Type: "cron", Interval: "1 1 1 1 1 1"}
+					tt := c.CreateTask(correctSchedule, wf, "baron", "", true, 0)
+					So(tt.Err, ShouldBeNil)
+				})
+			})
+
 			tf := c.CreateTask(sch, wf, "baron", "", false, 0)
 			Convey("valid task not started on creation", func() {
 				So(tf.Err, ShouldBeNil)

mgmt/rest/client/task.go

@@ -35,13 +35,13 @@ import (
 
 type Schedule struct {
 	// Type specifies the type of the schedule. Currently, the type of "simple", "windowed" and "cron" are supported.
-	Type string
+	Type string `json:"type,omitempty"`
 	// Interval specifies the time duration.
-	Interval string
-	// StartTime specifies the beginning time.
-	StartTime *time.Time
-	// StopTime specifies the end time.
-	StopTime *time.Time
+	Interval string `json:"interval,omitempty"`
+	// StartTimestamp specifies the beginning time.
+	StartTimestamp *time.Time `json:"start_timestamp,omitempty"`
+	// StopTimestamp specifies the end time.
+	StopTimestamp *time.Time `json:"stop_timestamp,omitempty"`
 }
 
 // CreateTask creates a task given the schedule, workflow, task name, and task state.
@@ -51,22 +51,15 @@ type Schedule struct {
 func (c *Client) CreateTask(s *Schedule, wf *wmap.WorkflowMap, name string, deadline string, startTask bool, maxFailures int) *CreateTaskResult {
 	t := core.TaskCreationRequest{
 		Schedule: &core.Schedule{
-			Type:     s.Type,
-			Interval: s.Interval,
+			Type:           s.Type,
+			Interval:       s.Interval,
+			StartTimestamp: s.StartTimestamp,
+			StopTimestamp:  s.StopTimestamp,
 		},
 		Workflow:    wf,
 		Start:       startTask,
 		MaxFailures: maxFailures,
 	}
-	// Add start and/or stop timestamps if they exist
-	if s.StartTime != nil {
-		u := s.StartTime.Unix()
-		t.Schedule.StartTimestamp = &u
-	}
-	if s.StopTime != nil {
-		u := s.StopTime.Unix()
-		t.Schedule.StopTimestamp = &u
-	}
 
 	if name != "" {
 		t.Name = name

mgmt/rest/rbody/task.go

@@ -222,13 +222,11 @@ func assertSchedule(s schedule.Schedule, t *AddScheduledTask) {
 		}
 		return
 	case *schedule.WindowedSchedule:
-		startTime := v.StartTime.Unix()
-		stopTime := v.StopTime.Unix()
 		t.Schedule = &core.Schedule{
 			Type:           "windowed",
 			Interval:       v.Interval.String(),
-			StartTimestamp: &startTime,
-			StopTimestamp:  &stopTime,
+			StartTimestamp: v.StartTime,
+			StopTimestamp:  v.StopTime,
 		}
 		return
 	case *schedule.CronSchedule: