cmd/snap: snap refresh --time with new and legacy schedules

client/client.go

@@ -395,8 +395,12 @@ type OSRelease struct {
 	VersionID string `json:"version-id,omitempty"`
 }
 
+// RefreshInfo contains information about refreshes.
 type RefreshInfo struct {
-	Schedule string `json:"schedule"`
+	// Timer contains the refresh.timer setting.
+	Timer string `json:"timer,omitempty"`
+	// Schedule contains the legacy refresh.schedule setting.
+	Schedule string `json:"schedule,omitempty"`
 	Last     string `json:"last,omitempty"`
 	Next     string `json:"next,omitempty"`
 }

cmd/snap/cmd_snap_op.go

@@ -375,7 +375,7 @@ func showDone(names []string, op string) error {
 			// TRANSLATORS: first %s is a snap name, second %s is a revision
 			fmt.Fprintf(Stdout, i18n.G("%s reverted to %s\n"), snap.Name, snap.Version)
 		default:
-			fmt.Fprintf(Stdout, "internal error, unknown op %q", op)
+			fmt.Fprintf(Stdout, "internal error: unknown op %q", op)
 		}
 		if snap.TrackingChannel != snap.Channel {
 			// TRANSLATORS: first %s is a snap name, following %s is a channel name
@@ -666,7 +666,13 @@ func (x *cmdRefresh) showRefreshTimes() error {
 		return err
 	}
 
-	fmt.Fprintf(Stdout, "schedule: %s\n", sysinfo.Refresh.Schedule)
+	if sysinfo.Refresh.Timer != "" {
+		fmt.Fprintf(Stdout, "timer: %s\n", sysinfo.Refresh.Timer)
+	} else if sysinfo.Refresh.Schedule != "" {
+		fmt.Fprintf(Stdout, "schedule: %s\n", sysinfo.Refresh.Schedule)
+	} else {
+		return errors.New("internal error: both refresh.timer and refresh.schedule are empty")
+	}
 	if sysinfo.Refresh.Last != "" {
 		fmt.Fprintf(Stdout, "last: %s\n", sysinfo.Refresh.Last)
 	} else {
@@ -718,7 +724,6 @@ func (x *cmdRefresh) Execute([]string) error {
 		if x.asksForMode() || x.asksForChannel() {
 			return errors.New(i18n.G("--time does not take mode nor channel flags"))
 		}
-
 		return x.showRefreshTimes()
 	}
 

cmd/snap/cmd_snap_op_test.go

@@ -541,7 +541,7 @@ foo +4.2update1 +17 +bar +-.*
 	c.Check(n, check.Equals, 1)
 }
 
-func (s *SnapSuite) TestRefreshTime(c *check.C) {
+func (s *SnapSuite) TestRefreshLegacyTime(c *check.C) {
 	n := 0
 	s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
 		switch n {
@@ -567,6 +567,42 @@ next: 2017-04-26T00:58:00+0200
 	c.Check(n, check.Equals, 1)
 }
 
+func (s *SnapSuite) TestRefreshTimer(c *check.C) {
+	n := 0
+	s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
+		switch n {
+		case 0:
+			c.Check(r.Method, check.Equals, "GET")
+			c.Check(r.URL.Path, check.Equals, "/v2/system-info")
+			fmt.Fprintln(w, `{"type": "sync", "status-code": 200, "result": {"refresh": {"timer": "0:00-24:00/4", "last": "2017-04-25T17:35:00+0200", "next": "2017-04-26T00:58:00+0200"}}}`)
+		default:
+			c.Fatalf("expected to get 1 requests, now on %d", n+1)
+		}
+
+		n++
+	})
+	rest, err := snap.Parser().ParseArgs([]string{"refresh", "--time"})
+	c.Assert(err, check.IsNil)
+	c.Assert(rest, check.DeepEquals, []string{})
+	c.Check(s.Stdout(), check.Equals, `timer: 0:00-24:00/4
+last: 2017-04-25T17:35:00+0200
+next: 2017-04-26T00:58:00+0200
+`)
+	c.Check(s.Stderr(), check.Equals, "")
+	// ensure that the fake server api was actually hit
+	c.Check(n, check.Equals, 1)
+}
+
+func (s *SnapSuite) TestRefreshNoTimerNoSchedule(c *check.C) {
+	s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
+		c.Check(r.Method, check.Equals, "GET")
+		c.Check(r.URL.Path, check.Equals, "/v2/system-info")
+		fmt.Fprintln(w, `{"type": "sync", "status-code": 200, "result": {"refresh": {"last": "2017-04-25T17:35:00+0200", "next": "2017-04-26T00:58:00+0200"}}}`)
+	})
+	_, err := snap.Parser().ParseArgs([]string{"refresh", "--time"})
+	c.Assert(err, check.ErrorMatches, `internal error: both refresh.timer and refresh.schedule are empty`)
+}
+
 func (s *SnapSuite) TestRefreshListErr(c *check.C) {
 	s.RedirectClientToTestServer(nil)
 	_, err := snap.Parser().ParseArgs([]string{"refresh", "--list", "--beta"})

daemon/api.go

@@ -270,7 +270,7 @@ func sysInfo(c *Command, r *http.Request, user *auth.UserState) Response {
 	defer st.Unlock()
 	nextRefresh := snapMgr.NextRefresh()
 	lastRefresh, _ := snapMgr.LastRefresh()
-	refreshScheduleStr, err := snapMgr.RefreshSchedule()
+	refreshScheduleStr, legacySchedule, err := snapMgr.RefreshSchedule()
 	if err != nil {
 		return InternalError("cannot get refresh schedule: %s", err)
 	}
@@ -279,6 +279,16 @@ func sysInfo(c *Command, r *http.Request, user *auth.UserState) Response {
 		return InternalError("cannot get user auth data: %s", err)
 	}
 
+	refreshInfo := client.RefreshInfo{
+		Last: formatRefreshTime(lastRefresh),
+		Next: formatRefreshTime(nextRefresh),
+	}
+	if !legacySchedule {
+		refreshInfo.Timer = refreshScheduleStr
+	} else {
+		refreshInfo.Schedule = refreshScheduleStr
+	}
+
 	m := map[string]interface{}{
 		"series":         release.Series,
 		"version":        c.d.Version,
@@ -290,11 +300,7 @@ func sysInfo(c *Command, r *http.Request, user *auth.UserState) Response {
 			"snap-mount-dir": dirs.SnapMountDir,
 			"snap-bin-dir":   dirs.SnapBinariesDir,
 		},
-		"refresh": client.RefreshInfo{
-			Schedule: refreshScheduleStr,
-			Last:     formatRefreshTime(lastRefresh),
-			Next:     formatRefreshTime(nextRefresh),
-		},
+		"refresh": refreshInfo,
 	}
 	// NOTE: Right now we don't have a good way to differentiate if we
 	// only have partial confinement (ala AppArmor disabled and Seccomp

daemon/api_test.go

@@ -772,7 +772,17 @@ func (s *apiSuite) TestSysInfo(c *check.C) {
 	rec := httptest.NewRecorder()
 	c.Check(sysInfoCmd.Path, check.Equals, "/v2/system-info")
 
-	s.daemon(c).Version = "42b1"
+	d := s.daemon(c)
+	d.Version = "42b1"
+
+	// set both legacy and new refresh schedules. new one takes priority
+	st := d.overlord.State()
+	st.Lock()
+	tr := config.NewTransaction(st)
+	tr.Set("core", "refresh.schedule", "00:00-9:00/12:00-13:00")
+	tr.Set("core", "refresh.timer", "8:00~9:00/2")
+	tr.Commit()
+	st.Unlock()
 
 	restore := release.MockReleaseInfo(&release.OS{ID: "distro-id", VersionID: "1.2"})
 	defer restore()
@@ -799,7 +809,8 @@ func (s *apiSuite) TestSysInfo(c *check.C) {
 			"snap-bin-dir":   dirs.SnapBinariesDir,
 		},
 		"refresh": map[string]interface{}{
-			"schedule": "00:00-24:00/4",
+			// only the "timer" field
+			"timer": "8:00~9:00/2",
 		},
 		"confinement": "partial",
 	}
@@ -814,6 +825,60 @@ func (s *apiSuite) TestSysInfo(c *check.C) {
 	c.Check(rsp.Result, check.DeepEquals, expected)
 }
 
+func (s *apiSuite) TestSysInfoLegacyRefresh(c *check.C) {
+	rec := httptest.NewRecorder()
+
+	d := s.daemon(c)
+	d.Version = "42b1"
+
+	restore := release.MockReleaseInfo(&release.OS{ID: "distro-id", VersionID: "1.2"})
+	defer restore()
+	restore = release.MockOnClassic(true)
+	defer restore()
+	restore = release.MockForcedDevmode(true)
+	defer restore()
+
+	// set the legacy refresh schedule
+	st := d.overlord.State()
+	st.Lock()
+	tr := config.NewTransaction(st)
+	tr.Set("core", "refresh.schedule", "00:00-9:00/12:00-13:00")
+	tr.Set("core", "refresh.timer", "")
+	tr.Commit()
+	st.Unlock()
+
+	sysInfoCmd.GET(sysInfoCmd, nil, nil).ServeHTTP(rec, nil)
+	c.Check(rec.Code, check.Equals, 200)
+	c.Check(rec.HeaderMap.Get("Content-Type"), check.Equals, "application/json")
+
+	expected := map[string]interface{}{
+		"series":  "16",
+		"version": "42b1",
+		"os-release": map[string]interface{}{
+			"id":         "distro-id",
+			"version-id": "1.2",
+		},
+		"on-classic": true,
+		"managed":    false,
+		"locations": map[string]interface{}{
+			"snap-mount-dir": dirs.SnapMountDir,
+			"snap-bin-dir":   dirs.SnapBinariesDir,
+		},
+		"refresh": map[string]interface{}{
+			// only the "schedule" field
+			"schedule": "00:00-9:00/12:00-13:00",
+		},
+		"confinement": "partial",
+	}
+	var rsp resp
+	c.Assert(json.Unmarshal(rec.Body.Bytes(), &rsp), check.IsNil)
+	c.Check(rsp.Status, check.Equals, 200)
+	c.Check(rsp.Type, check.Equals, ResponseTypeSync)
+	const kernelVersionKey = "kernel-version"
+	delete(rsp.Result.(map[string]interface{}), kernelVersionKey)
+	c.Check(rsp.Result, check.DeepEquals, expected)
+}
+
 func (s *apiSuite) makeDeveloperAPIServer(statusCode int, data string) *httptest.Server {
 	mockDeveloperAPIServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(statusCode)

overlord/snapstate/autorefresh.go

@@ -59,11 +59,12 @@ func newAutoRefresh(st *state.State) *autoRefresh {
 	}
 }
 
-// RefreshSchedule will return a user visible string with the current
-// schedule for the automatic refreshes.
-func (m *autoRefresh) RefreshSchedule() (string, error) {
-	_, scheduleStr, err := m.refreshScheduleWithDefaultsFallback()
-	return scheduleStr, err
+// RefreshSchedule will return a user visible string with the current schedule
+// for the automatic refreshes and a flag indicating whether the schedule is a
+// legacy one.
+func (m *autoRefresh) RefreshSchedule() (schedule string, legacy bool, err error) {
+	_, schedule, legacy, err = m.refreshScheduleWithDefaultsFallback()
+	return schedule, legacy, err
 }
 
 // NextRefresh returns when the next automatic refresh will happen.
@@ -107,7 +108,7 @@ func (m *autoRefresh) Ensure() error {
 		return err
 	}
 
-	refreshSchedule, refreshScheduleStr, err := m.refreshScheduleWithDefaultsFallback()
+	refreshSchedule, refreshScheduleStr, _, err := m.refreshScheduleWithDefaultsFallback()
 	if err != nil {
 		return err
 	}
@@ -163,44 +164,44 @@ func (m *autoRefresh) Ensure() error {
 //
 // TODO: we can remove the refreshSchedule reset because we have validation
 //       of the schedule now.
-func (m *autoRefresh) refreshScheduleWithDefaultsFallback() (ts []*timeutil.Schedule, scheduleAsStr string, err error) {
+func (m *autoRefresh) refreshScheduleWithDefaultsFallback() (ts []*timeutil.Schedule, scheduleAsStr string, legacy bool, err error) {
 	if refreshScheduleManaged(m.state) {
 		if m.lastRefreshSchedule != "managed" {
 			logger.Noticef("refresh.schedule is managed via the snapd-control interface")
 			m.lastRefreshSchedule = "managed"
 		}
-		return nil, "managed", nil
+		return nil, "managed", true, nil
 	}
 
 	tr := config.NewTransaction(m.state)
 
 	// try the new refresh.timer config option first
 	err = tr.Get("core", "refresh.timer", &scheduleAsStr)
 	if err != nil && !config.IsNoOption(err) {
-		return nil, "", err
+		return nil, "", false, err
 	}
 	if scheduleAsStr != "" {
 		ts, err = timeutil.ParseSchedule(scheduleAsStr)
 		if err != nil {
 			logger.Noticef("cannot use refresh.timer configuration: %s", err)
 			return refreshScheduleDefault()
 		}
-		return ts, scheduleAsStr, nil
+		return ts, scheduleAsStr, false, nil
 	}
 
 	// fallback to legacy refresh.schedule setting when the new
 	// config option is not set
 	err = tr.Get("core", "refresh.schedule", &scheduleAsStr)
 	if err != nil && !config.IsNoOption(err) {
-		return nil, "", err
+		return nil, "", false, err
 	}
 	if scheduleAsStr != "" {
 		ts, err = timeutil.ParseLegacySchedule(scheduleAsStr)
 		if err != nil {
 			logger.Noticef("cannot use refresh.schedule configuration: %s", err)
 			return refreshScheduleDefault()
 		}
-		return ts, scheduleAsStr, nil
+		return ts, scheduleAsStr, true, nil
 	}
 
 	return refreshScheduleDefault()
@@ -244,13 +245,13 @@ func (m *autoRefresh) launchAutoRefresh() error {
 	return nil
 }
 
-func refreshScheduleDefault() (ts []*timeutil.Schedule, scheduleStr string, err error) {
+func refreshScheduleDefault() (ts []*timeutil.Schedule, scheduleStr string, legacy bool, err error) {
 	refreshSchedule, err := timeutil.ParseSchedule(defaultRefreshSchedule)
 	if err != nil {
 		panic(fmt.Sprintf("defaultRefreshSchedule cannot be parsed: %s", err))
 	}
 
-	return refreshSchedule, defaultRefreshSchedule, nil
+	return refreshSchedule, defaultRefreshSchedule, false, nil
 }
 
 func autoRefreshInFlight(st *state.State) bool {

overlord/snapstate/autorefresh_test.go

@@ -118,8 +118,9 @@ func (s *autoRefreshTestSuite) TestLastRefreshRefreshManaged(c *C) {
 	c.Check(err, IsNil)
 	c.Check(s.store.ops, HasLen, 0)
 
-	refreshScheduleStr, err := af.RefreshSchedule()
+	refreshScheduleStr, legacy, err := af.RefreshSchedule()
 	c.Check(refreshScheduleStr, Equals, "managed")
+	c.Check(legacy, Equals, true)
 	c.Check(err, IsNil)
 
 	c.Check(af.NextRefresh(), DeepEquals, time.Time{})

overlord/snapstate/snapmgr.go

@@ -366,10 +366,10 @@ func (m *SnapManager) LastRefresh() (time.Time, error) {
 	return m.autoRefresh.LastRefresh()
 }
 
-// RefreshSchedule returns the current refresh schedule as a string
-// suitable to display to a user.
+// RefreshSchedule returns the current refresh schedule as a string suitable for
+// display to a user and a flag indicating whether the schedule is a legacy one.
 // The caller should be holding the state lock.
-func (m *SnapManager) RefreshSchedule() (string, error) {
+func (m *SnapManager) RefreshSchedule() (string, bool, error) {
 	return m.autoRefresh.RefreshSchedule()
 }