From 40383deb0c5ac0c04ad77812bcaf0edc3ceb9a1c Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Mon, 3 Aug 2026 15:18:56 +1000 Subject: [PATCH 1/9] chore: port timeutil updates from snapd --- internals/timeutil/export_test.go | 4 +- internals/timeutil/human.go | 2 +- internals/timeutil/human_test.go | 2 +- internals/timeutil/schedule.go | 221 ++++++++++++++++---- internals/timeutil/schedule_test.go | 314 ++++++++++++++++++++++++---- 5 files changed, 460 insertions(+), 83 deletions(-) diff --git a/internals/timeutil/export_test.go b/internals/timeutil/export_test.go index 4453d9019..7ea1fcdb5 100644 --- a/internals/timeutil/export_test.go +++ b/internals/timeutil/export_test.go @@ -20,9 +20,11 @@ var ( ParseClockSpan = parseClockSpan ParseWeekSpan = parseWeekSpan HumanTimeSince = humanTimeSince + MonthNext = monthNext ) -func MockTimeNow(f func() time.Time) (restorer func()) { +// FakeTimeNow mocks the time.Now() calls used in the timeutil package. +func FakeTimeNow(f func() time.Time) (restorer func()) { origTimeNow := timeNow timeNow = f return func() { timeNow = origTimeNow } diff --git a/internals/timeutil/human.go b/internals/timeutil/human.go index 6e9c92b05..2c4a61ba1 100644 --- a/internals/timeutil/human.go +++ b/internals/timeutil/human.go @@ -29,7 +29,7 @@ func sod(t time.Time) time.Time { // consumption. // Human(t) --> "today at 07:47" func Human(then time.Time) string { - return humanTimeSince(then.Local(), time.Now().Local(), 60) + return humanTimeSince(then.Local(), timeNow().Local(), 60) } func delta(then, now time.Time) int { diff --git a/internals/timeutil/human_test.go b/internals/timeutil/human_test.go index 84837e4f8..586b445b2 100644 --- a/internals/timeutil/human_test.go +++ b/internals/timeutil/human_test.go @@ -40,7 +40,7 @@ func (s *humanSuite) SetUpSuite(c *check.C) { s.beforeDSTends = time.Date(2017, 10, 29, 0, 59, 0, 0, loc).Add(60 * time.Minute) s.afterDSTends = time.Date(2017, 10, 29, 1, 1, 0, 0, loc) - // sanity check + // validity check c.Check(s.beforeDSTbegins.Format("MST"), check.Equals, s.afterDSTends.Format("MST")) c.Check(s.beforeDSTbegins.Format("MST"), check.Equals, "GMT") c.Check(s.afterDSTbegins.Format("MST"), check.Equals, s.beforeDSTends.Format("MST")) diff --git a/internals/timeutil/schedule.go b/internals/timeutil/schedule.go index 65c3b2cad..7a25b5fdc 100644 --- a/internals/timeutil/schedule.go +++ b/internals/timeutil/schedule.go @@ -115,8 +115,11 @@ func (w Week) String() string { return day + strconv.Itoa(int(w.Pos)) } -// WeekSpan represents a span of weekdays between Start and End days. WeekSpan -// may wrap around the week, eg. fri-mon is a span from Friday to Monday +// WeekSpan represents a span of weekdays between Start and End days, which may +// be a single day. WeekSpan may wrap around the week, eg. fri-mon is a span +// from Friday to Monday, mon1-fri is a span from the first Monday to the +// following Friday, while mon1 (internally, an equal start and end range) +// represents the 1st Monday of a month. type WeekSpan struct { Start Week End Week @@ -129,6 +132,7 @@ func (ws WeekSpan) String() string { return ws.Start.String() } +// findNthWeekDay finds the nth occurrence of a given weekday in the month of t func findNthWeekDay(t time.Time, weekday time.Weekday, nthInMonth uint) time.Time { // move to the beginning of the month t = t.AddDate(0, 0, -t.Day()+1) @@ -146,34 +150,159 @@ func findNthWeekDay(t time.Time, weekday time.Weekday, nthInMonth uint) time.Tim return t } +// findLastWeekDay finds the last occurrence of a given weekday in the month of t +func findLastWeekDay(t time.Time, weekday time.Weekday) time.Time { + n := monthNext(t).Add(-24 * time.Hour) + for n.Weekday() != weekday { + n = n.Add(-24 * time.Hour) + } + return n +} + +// matchingWeekdaysInMonth returns the number of occurrences of the weekday of t since +// the start of the month until t event +func matchingWeekdaysInMonth(t time.Time) int { + month := t.Month() + nth := 0 + for n := t; n.Month() == month; n = n.Add(-7 * 24 * time.Hour) { + nth++ + } + return nth +} + // Match checks if t is within the day span represented by ws. func (ws WeekSpan) Match(t time.Time) bool { start, end := ws.Start, ws.End wdStart, wdEnd := start.Weekday, end.Weekday - if start.Pos != EveryWeek { - if start.Pos == LastWeek { - // last week of the month - if !isLastWeekdayInMonth(t) { + weekdayMatch := func(t time.Time) bool { + if wdStart <= wdEnd { + // single day (mon) or start < end (eg. mon-fri) + return t.Weekday() >= wdStart && t.Weekday() <= wdEnd + } + // wraps around the week end, eg. fri-mon + return t.Weekday() >= wdStart || t.Weekday() <= wdEnd + } + + if start.Pos == EveryWeek && end.Pos == EveryWeek { + // generic weekday match, eg. mon-fri + return weekdayMatch(t) + } + + // things that use a numbered weekday + + // fun cases, eg (consider the calendar below): + // + // - mon1-fri, week span, start anchored at 1st Monday 06.08, matches: + // 06.08-10.08 + // - mon-fri2, week span, end anchored at 2nd Friday 10.08, matches: + // 06.08-10.08 + // - fri1-mon, week span, start anchored at 1st Friday 3.08, matches + // 03.08-06.08 + // - mon-fri1, week span, end anchored at 1st Friday 3.08, matches + // 30.07-03.08, (crossing the month boundary) + // - fri4-thu, week span, end anchored at 4th Friday 27.07, matches + // 27.07-02.08, (crossing the month boundary), but also 24.08-30.08, + // which is within a single month + // + // July 2018 August 2018 + // Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa + // 1 2 3 4 5 6 7 1 2 3 4 + // 8 9 10 11 12 13 14 5 6 7 8 9 10 11 + // 15 16 17 18 19 20 21 12 13 14 15 16 17 18 + // 22 23 24 25 26 27 28 19 20 21 22 23 24 25 + // 29 30 31 26 27 28 29 30 31 + + // find out the range of week span, anchor sharing the same month as t + startDay, endDay := ws.dateRangeAnchoredAt(t) + anchoredAtStart := ws.AnchoredAtStart() + + if t.After(endDay) || t.Before(startDay) { + // outside of dates range of the week span, consider edge cases: + // - next month if the span is anchored at the end (eg. mon-fri1 30.07-03.08, t=31.07) + // - previous month if the span is anchored at the start (eg. fri4-thu 27.07-02.08, t=01.08) + + if anchoredAtStart { + // eg. fri4-thu, range anchored at previous month + if matchingWeekdaysInMonth(t) != 1 { + // no match if t is not within the first week return false } + prevMonth := monthPrev(t) + startDay, endDay = ws.dateRangeAnchoredAt(prevMonth) } else { - startDay := findNthWeekDay(t, start.Weekday, start.Pos) - endDay := findNthWeekDay(t, end.Weekday, end.Pos) - - if t.Day() < startDay.Day() || t.Day() > endDay.Day() { + // eg. mon-fri1, range anchored at the next month + if !isLastWeekdayInMonth(t) { + // no match if t is not within the last week return false } - return true + nextMonth := monthNext(t) + startDay, endDay = ws.dateRangeAnchoredAt(nextMonth) } + // at this point we will check whether t matches the range that + // spills from the previous month or from the next month + } + outside := t.Before(startDay) || t.After(endDay) + return !outside +} + +// monthNext returns the first day of the next month relative to t +func monthNext(t time.Time) time.Time { + n := t + // advance by 28 days at most, so that we don't skip a 28 day February + n = n.AddDate(0, 0, 28) + for n.Month() == t.Month() { + n = n.Add(24 * time.Hour) + } + if n.Day() != 1 { + // backtrack if we didn't land on the first day yet + n = n.AddDate(0, 0, -n.Day()+1) + } + return n +} + +// monthPrev returns the last day of previous month relative to t +func monthPrev(t time.Time) time.Time { + return t.AddDate(0, 0, -1*(t.Day()+1)) +} + +// AnchoredAtStart returns true when the week span is anchored at the starting +// point, or false otherwise +func (ws WeekSpan) AnchoredAtStart() bool { + return ws.Start.Pos != EveryWeek +} + +// dateRangeAnchoredAt returns the range of dates that match the week span, with the +// anchor sharing the same month as t +func (ws WeekSpan) dateRangeAnchoredAt(t time.Time) (start, end time.Time) { + weekPos := ws.End.Pos + anchoredAtStart := ws.AnchoredAtStart() + if anchoredAtStart { + weekPos = ws.Start.Pos + } + // first check the start/end dates in the same month as t + if weekPos != LastWeek { + start = findNthWeekDay(t, ws.Start.Weekday, weekPos) + end = findNthWeekDay(t, ws.End.Weekday, weekPos) + } else { + start = findLastWeekDay(t, ws.Start.Weekday) + end = findLastWeekDay(t, ws.End.Weekday) } - if wdStart <= wdEnd { - // single day (mon) or start < end (eg. mon-fri) - return t.Weekday() >= wdStart && t.Weekday() <= wdEnd + // eg. mon1-mon span falls under the Equal && !singleDay case + if start.After(end) || (start.Equal(end) && !ws.IsSingleDay()) { + if anchoredAtStart { + end = end.Add(7 * 24 * time.Hour) + } else { + start = start.Add(-7 * 24 * time.Hour) + } } - // wraps around the week end, eg. fri-mon - return t.Weekday() >= wdStart || t.Weekday() <= wdEnd + return start, end +} + +// IsSingleDay returns true when the week span represents a single day +func (ws WeekSpan) IsSingleDay() bool { + return ws.Start == ws.End } // ClockSpan represents a time span within 24h, potentially crossing days. For @@ -460,14 +589,14 @@ func parseClockRange(s string) (start, end Clock, err error) { // ParseLegacySchedule takes an obsolete schedule string in the form of: // -// 9:00-15:00 (every day between 9am and 3pm) -// 9:00-15:00/21:00-22:00 (every day between 9am,5pm and 9pm,10pm) +// 9:00-15:00 (every day between 9am and 3pm) +// 9:00-15:00/21:00-22:00 (every day between 9am,5pm and 9pm,10pm) // // and returns a list of Schedule types or an error func ParseLegacySchedule(scheduleSpec string) ([]*Schedule, error) { var schedule []*Schedule - for s := range strings.SplitSeq(scheduleSpec, "/") { + for _, s := range strings.Split(scheduleSpec, "/") { start, end, err := parseClockRange(s) if err != nil { return nil, err @@ -490,9 +619,10 @@ func ParseLegacySchedule(scheduleSpec string) ([]*Schedule, error) { // eventset = wdaylist / timelist / wdaylist "," timelist // // wdaylist = wdayset *( "," wdayset ) -// wdayset = wday / wdayspan -// wday = ( "sun" / "mon" / "tue" / "wed" / "thu" / "fri" / "sat" ) [ DIGIT ] -// wdayspan = wday "-" wday +// wdayset = wday / wdaynumber / wdayspan +// wday = ( "sun" / "mon" / "tue" / "wed" / "thu" / "fri" / "sat" ) +// wdaynumber = ( "sun" / "mon" / "tue" / "wed" / "thu" / "fri" / "sat" ) DIGIT +// wdayspan = wday "-" wday / wdaynumber "-" wday / wday "-" wdaynumber // // timelist = timeset *( "," timeset ) // timeset = time / timespan @@ -501,21 +631,24 @@ func ParseLegacySchedule(scheduleSpec string) ([]*Schedule, error) { // count = 1*DIGIT // // Examples: -// -// mon,10:00,,fri,15:00 (Monday at 10:00, Friday at 15:00) -// mon,fri,10:00,15:00 (Monday at 10:00 and 15:00, Friday at 10:00 and 15:00) -// mon-wed,fri,9:00-11:00/2 (Monday to Wednesday and on Friday, twice between -// 9:00 and 11:00) -// mon,9:00~11:00,,wed,22:00~23:00 (Monday, sometime between 9:00 and 11:00, and -// on Wednesday, sometime between 22:00 and 23:00) -// mon,wed (Monday and on Wednesday) -// mon,,wed (same as above) +// mon,10:00,,fri,15:00 (Monday at 10:00, Friday at 15:00) +// mon,fri,10:00,15:00 (Monday at 10:00 and 15:00, Friday at 10:00 and 15:00) +// mon-wed,fri,9:00-11:00/2 (Monday to Wednesday and on Friday, twice between +// 9:00 and 11:00) +// mon,9:00~11:00,,wed,22:00~23:00 (Monday, sometime between 9:00 and 11:00, +// and on Wednesday, sometime between 22:00 and 23:00) +// mon,wed (Monday and on Wednesday) +// mon,,wed (same as above) +// mon1-wed (1st Monday of the month to the following Wednesday) +// mon-wed1 (from the 1st Wednesday of the month to the prior Monday) +// mon1 (1st Monday of the month) +// mon1-mon (from the 1st Monday of the month to the following Monday) // // Returns a slice of schedules or an error if parsing failed func ParseSchedule(scheduleSpec string) ([]*Schedule, error) { var schedule []*Schedule - for s := range strings.SplitSeq(scheduleSpec, ",,") { + for _, s := range strings.Split(scheduleSpec, ",,") { // cut the schedule in event sets // eventlist = eventset *( ",," eventset ) sched, err := parseEventSet(s) @@ -550,13 +683,23 @@ func parseWeekSpan(s string) (span WeekSpan, err error) { parsed.End = parsed.Start } - if parsed.End.Pos < parsed.Start.Pos { - // eg. mon4-mon1 - return span, fmt.Errorf("cannot parse %q: unsupported schedule", s) - } + if (parsed.Start.Pos != EveryWeek) && (parsed.End.Pos != EveryWeek) { + // both ends have a week position set - if (parsed.Start.Pos != EveryWeek) != (parsed.End.Pos != EveryWeek) { - return span, fmt.Errorf("cannot parse %q: week number must be present for both weekdays or neither", s) + if parsed.End.Pos < parsed.Start.Pos { + // eg. mon4-mon1 + return span, fmt.Errorf("cannot parse %q: unsupported schedule", s) + } + + if !parsed.IsSingleDay() { + // ambiguous case that produces different schedules depending on + // the calendar, to avoid the ambiguity, anchor the schedule at + // the start of the week span, eg. mon1-tue2 -> mon1-tue + // + // TODO: error out instead of degrading when a + // deprecated span is used under the new rules + parsed.End.Pos = EveryWeek + } } return parsed, nil @@ -608,7 +751,7 @@ func parseWeekday(s string) (week Week, err error) { return week, fmt.Errorf("cannot parse %q: invalid format", s) } - day := s + var day = s var pos uint if l == 4 { day = s[0:3] diff --git a/internals/timeutil/schedule_test.go b/internals/timeutil/schedule_test.go index bbb153f0a..4983d7608 100644 --- a/internals/timeutil/schedule_test.go +++ b/internals/timeutil/schedule_test.go @@ -296,7 +296,7 @@ func (ts *timeutilSuite) TestLegacyScheduleNext(c *C) { fakeNow, err := time.ParseInLocation(shortForm, t.now, time.Local) c.Assert(err, IsNil) - restorer := timeutil.MockTimeNow(func() time.Time { + restorer := timeutil.FakeTimeNow(func() time.Time { return fakeNow }) defer restorer() @@ -334,7 +334,6 @@ func (ts *timeutilSuite) TestParseSchedule(c *C) { {"mon9,9:00", nil, `cannot parse "mon9": "mon9" is not a valid weekday`}, {"mon0,9:00", nil, `cannot parse "mon0": "mon0" is not a valid weekday`}, {"mon5-mon1,9:00", nil, `cannot parse "mon5-mon1": unsupported schedule`}, - {"mon-mon2,9:00", nil, `cannot parse "mon-mon2": week number must be present for both weekdays or neither`}, {"mon%,9:00", nil, `cannot parse "mon%": "mon%" is not a valid weekday`}, {"foo2,9:00", nil, `cannot parse "foo2": "foo2" is not a valid weekday`}, {"9:00---11:00", nil, `cannot parse "9:00---11:00": not a valid time`}, @@ -483,6 +482,14 @@ func (ts *timeutilSuite) TestParseSchedule(c *C) { WeekSpans: []timeutil.WeekSpan{ {Start: timeutil.Week{Weekday: time.Friday}, End: timeutil.Week{Weekday: time.Monday}}}, }}, + }, { + in: "mon-mon2,9:00", + expected: []*timeutil.Schedule{{ + ClockSpans: []timeutil.ClockSpan{ + {Start: timeutil.Clock{Hour: 9}, End: timeutil.Clock{Hour: 9}}}, + WeekSpans: []timeutil.WeekSpan{ + {Start: timeutil.Week{Weekday: time.Monday}, End: timeutil.Week{Weekday: time.Monday, Pos: 2}}}, + }}, }, } { c.Logf("trying %+v", t) @@ -567,6 +574,7 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { // from now next: "503h-503h", }, { + // (deprecated syntax, interpreted as mon1-tue) // from the first Monday of the month to the second Tuesday of // the month, at 10:00 schedule: "mon1-tue2,10:00", @@ -574,21 +582,21 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { last: "2017-02-06 10:00", // Tuesday, the day after the first Monday of the month now: "2017-02-07 11:00", - // expecting to run the next day at 10:00 - next: "23h-23h", + // expecting to run on 03.06.2017 + next: "647h-647h", }, { - // from the first Monday of the month to the second Tuesday of + // from the first Monday of the month to the following Tuesday of // the month, at 10:00 - schedule: "mon1-tue2,10:00", + schedule: "mon1-tue,10:00", last: "2017-02-01 10:00", // Sunday, 10:00 now: "2017-02-05 10:00", // expecting to run the next day at 10:00 next: "24h-24h", }, { - // from the first Monday of the month to the second Tuesday of + // from the first Monday of the month to the following Tuesday of // the month, at 10:00 - schedule: "mon1-tue2,10:00", + schedule: "mon1-tue,10:00", // Tuesday, 10:00 last: "2017-02-14 22:00", // Thursday, 10:00 @@ -596,9 +604,9 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { // expecting to run in 18 days next: "432h-432h", }, { - // from the first Monday of the month to the second Tuesday of + // from the first Monday of the month to the following Tuesday of // the month, at 10:00 - schedule: "mon1-tue2,10:00", + schedule: "mon1-tue,10:00", // Sunday, 22:00 last: "2017-02-05 22:00", // first Monday of the month @@ -606,9 +614,9 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { // expecting to run the next day at 10:00 next: "23h-23h", }, { - // from the first Monday of the month to the second Tuesday of + // from the first Monday of the month to the following Tuesday of // the month, at 10:00 - schedule: "mon1-tue2,10:00-12:00", + schedule: "mon1-tue,10:00-12:00", // Sunday, 22:00 last: "2017-02-05 22:00", // first Monday of the month, within the update window @@ -616,9 +624,9 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { // expecting to run now next: "0h-0h", }, { - // from the first Monday of the month to the second Tuesday of + // from the first Monday of the month to the following Tuesday of // the month, at 10:00 - schedule: "mon1-tue2,10:00~12:00", + schedule: "mon1-tue,10:00~12:00", // Sunday, 22:00 last: "2017-02-05 22:00", // first Monday of the month, within the update window @@ -756,6 +764,38 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { last: "2018-07-29 13:00", // next one on 2018-08-01 13:00 next: "52h-52h", + }, { + // October 2019 + // Su Mo Tu We Th Fr Sa + // 29 30| 1 2 3 4 5 + // 6 7 8 9 10 11 12 + // 13 14 15 16 17 18 19 + // 20 21 22 23 24 25 26 + // 27 28 29 30 31 + + // first Monday to the following Wednesday of the month, in Oct + // 2019, matches 07.10-09.10 + schedule: "mon1-wed,9:00-13:00", + now: "2019-09-30 9:00", + // yesterday + last: "2019-09-30 9:00", + // next one on 2019-10-07 9:00 + next: "168h-168h", + }, { + // first Monday to the following Wednesday of the month, in Oct + // 2019, matches 30.09-04.10 + schedule: "mon-fri1,9:00-13:00", + now: "2019-09-29 9:00", + last: "2019-09-29 9:00", + // next one on 2019-09-30 9:00 + next: "24h-24h", + }, { + // most trivial case + schedule: "21:00-22:00", + now: "2019-09-29 8:00", + last: "2019-09-28 21:05", + // next one on 2019-09-29 at 21:00 + next: "13h-13h", }, } { c.Logf("trying %+v", t) @@ -765,7 +805,7 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { fakeNow, err := time.ParseInLocation(shortForm, t.now, time.Local) c.Assert(err, IsNil) - restorer := timeutil.MockTimeNow(func() time.Time { + restorer := timeutil.FakeTimeNow(func() time.Time { return fakeNow }) defer restorer() @@ -778,7 +818,7 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { previous := time.Duration(0) calls := 2 - for range calls { + for i := 0; i < calls; i++ { next := timeutil.Next(sched, last, maxDuration) if t.randomized { c.Check(next, Not(Equals), previous) @@ -792,7 +832,7 @@ func (ts *timeutilSuite) TestScheduleNext(c *C) { c.Check(next >= minDist && next <= maxDist, Equals, true, - Commentf("invalid distance for schedule %q with last refresh %q, now %q, expected %v, got %v, date %s", + Commentf("invalid distance for schedule %q with last refresh %q, now %q, expected %v, got %v, date %s", t.schedule, t.last, t.now, t.next, next, fakeNow.Add(next))) previous = next } @@ -838,70 +878,80 @@ func (ts *timeutilSuite) TestScheduleIncludes(c *C) { now: "2017-02-27 10:59:20", expecting: true, }, { + // (deprecated syntax) // from first Monday of the month to the second Tuesday of // the month, at 10:00 to 12:00 schedule: "mon1-tue2,10:00-12:00", // Thursday, 11:10 now: "2017-02-09 11:10:00", - expecting: true, + expecting: false, }, { - // from first Monday of the month to the second Tuesday of + // from first Monday of the month to the following Tuesday of // the month, at 10:00 to 12:00 - schedule: "mon1-tue2,10:00~12:00", + schedule: "mon1-tue,10:00~12:00", // Thursday, 11:10 now: "2017-02-02 11:10:00", expecting: false, }, { - // from first Monday of the month to the second Tuesday of + // from first Monday of the month to the following Tuesday of // the month, at 10:00 to 12:00 - schedule: "mon1-tue2,10:00~12:00", + schedule: "mon1-tue,10:00~12:00", // Monday, 11:10 now: "2017-02-06 11:10:00", expecting: true, }, { - // from first Monday of the month to the second Tuesday of + // from first Monday of the month to the following Tuesday of // the month, at 10:00 to 12:00 - schedule: "mon1-tue2,10:00~12:00", + schedule: "mon1-tue,10:00~12:00", // Thursday, 11:10 now: "2017-02-16 11:10:00", expecting: false, }, { - // from first Monday of the month to the second Tuesday of + // from first Monday of the month to the following Tuesday of // the month, at 10:00 to 12:00 - schedule: "mon1-tue2,10:00~12:00", + schedule: "mon1-tue,10:00~12:00", // Thursday, 11:10 - now: "2017-02-16 11:10:00", - expecting: false, + now: "2017-03-06 11:10:00", + expecting: true, }, { - // from first Monday of the month to the second Tuesday of + // from first Monday of the month to the following Tuesday of // the month, at 10:00 to 12:00 - schedule: "mon1-tue2,10:00~12:00", + schedule: "mon1-tue,10:00~12:00", // Thursday, 11:10 now: "2017-02-09 11:10:00", - expecting: true, + expecting: false, }, { - // from first Tuesday of the month to the second Monday of + // from first Tuesday of the month to the following Monday of // the month, at 10:00 to 12:00 - schedule: "tue1-mon2,10:00~12:00", + schedule: "tue1-mon,10:00~12:00", // Thursday, 11:10 now: "2017-02-09 11:10:00", expecting: true, }, { - // from 4th Monday of the month to the last Wednesday of + // (deprecated syntax) + // from 4th Monday of the month to the following Wednesday of // the month, at 10:00 to 12:00 schedule: "mon4-wed5,10:00~12:00", - // Schedule ends up being Feb 20 - Feb 22 2017 - now: "2017-03-01 11:10:00", + // Schedule ends up being Feb 27 - Mar 01 2017 + now: "2017-03-02 11:10:00", expecting: false, }, { - // from 4th Monday of the month to the last Wednesday of + // from last Monday of the month to the following Wednesday of // the month, at 10:00 to 12:00 - schedule: "mon4-wed5,10:00~12:00", - // Schedule ends up being Feb 20 - Feb 22 2017 - now: "2017-02-23 11:10:00", + schedule: "mon5-wed,10:00~12:00", + // Schedule ends up being Feb 27 - Mar 01 2017 + now: "2017-03-01 11:10:00", + expecting: true, + }, { + // from last Monday of the month to the following Wednesday of + // the month, at 10:00 to 12:00 + schedule: "mon5-wed,10:00~12:00", + // Schedule ends up being Feb 27 - Mar 01 2017 + now: "2017-03-02 11:10:00", expecting: false, }, { - // from last Monday of the month to the second Tuesday of + // (deprecated syntax) + // from last Monday of the month to the following Tuesday of // the month, at 10:00 schedule: "mon1-tue2,10:00~12:00", // Sunday, 11:10 @@ -927,6 +977,16 @@ func (ts *timeutilSuite) TestScheduleIncludes(c *C) { // sometime between 10am and 11am now: "2017-02-06 9:30:00", expecting: true, + }, { + schedule: "mon1-wed,9:00-10:00", + // Tue, 9:30 + now: "2019-10-08 9:30:00", + expecting: true, + }, { + schedule: "tue1,9:00-10:00", + // Tue, 9:30 + now: "2019-10-01 9:30:00", + expecting: true, }, } { c.Logf("trying %+v", t) @@ -1027,6 +1087,128 @@ func (ts *timeutilSuite) TestWeekSpans(c *C) { week: "thu5", when: "2018-07-26", match: true, + }, { + // using deprecated syntax + // first Monday (06.08) to first Friday (03.08), see August calendar above + // includes: 01.08-03.08 and 06.08-07.08 + week: "mon1-fri1", + // Wednesday + when: "2018-08-01", + match: false, + }, { + // using deprecated syntax + // first Monday (06.08) to first Friday (03.08), see August calendar above + week: "mon1-fri", + // Tuesday + when: "2018-08-07", + match: true, + }, { + // first Monday (06.08) to first Friday (03.08), see August calendar above + week: "mon1-fri", + // Thursday + when: "2018-08-08", + match: true, + }, { + // second Monday (13.08) to second Friday (10.08), see August calendar above + // includes: 13.08-14.08 and 08.08-10.08 + week: "mon2-fri", + // Thursday + when: "2018-08-13", + match: true, + }, { + // second Monday (13.08) to second Friday (10.08), see August calendar above + week: "mon2-fri", + // Thursday + when: "2018-08-13", + match: true, + }, { + // first Friday (03.08) to the following Monday (06.08), see August calendar above + // includes: 03.08-06.08 + week: "fri1-mon", + // Saturday + when: "2018-08-04", + match: true, + }, { + // first Friday (06.07) to the following Monday (09.07), see July calendar above + // includes: 03.07-09.07 + week: "fri1-mon", + // Sunday + when: "2018-07-08", + match: true, + }, { + // first Friday (03.08) to the preceding Monday (30.07), see July. August calendar above + // includes: 30.07-03.08 + week: "mon-fri1", + // Saturday + when: "2018-08-01", + match: true, + }, { + // first Friday (03.08) to the preceding Monday (30.07), see July. August calendar above + // includes: 30.07-03.08 + week: "mon-fri1", + // Saturday + when: "2018-07-30", + match: true, + }, { + // 4th Friday (27.08) to the following Monday (02.08), see July. August calendar above + // includes: 27.07-02.08 + week: "fri4-thu", + // Saturday + when: "2018-08-01", + match: true, + }, { + // using deprecated syntax + // first Friday (06.07) to the following Monday (09.07), see July calendar above + // includes: 03.07-09.07 + week: "fri1-mon1", + // Sunday + when: "2018-07-08", + match: true, + }, { + // first Friday (06.07) to the following Monday (09.07), see July calendar above + // includes: 06.07-09.07 + week: "fri1-mon", + // Sunday + when: "2018-07-15", + match: false, + }, { + // last Monday (30.07) to the following Friday (03.07), see July calendar above + // includes: 03.07-03.08 + week: "mon5-fri", + // Sunday + when: "2018-07-31", + match: true, + }, { + // last Friday (27.07) to the preceding Monday (23.07), see July calendar above + // includes: 23.07-27.07 + week: "mon-fri5", + // Sunday + when: "2018-07-28", + match: false, + }, { + // last Friday (27.07) to the preceding Monday (23.07), see July calendar above + // includes: 23.07-27.07 + week: "mon-fri5", + // Sunday + when: "2018-07-25", + match: true, + }, { + // first Monday (2.07) to the following Monday (9.07), see July calendar above + // includes: 2.07-9.07 + week: "mon1-mon", + // Tuesday + when: "2018-07-03", + match: true, + }, { + week: "mon1-mon", + // Monday (the farther edge of the span) + when: "2018-07-09", + match: true, + }, { + week: "mon1-mon", + // Tuesday + when: "2018-07-10", + match: false, }, } { c.Logf("trying %+v", t) @@ -1040,3 +1222,53 @@ func (ts *timeutilSuite) TestWeekSpans(c *C) { c.Check(ws.Match(when), Equals, t.match) } } + +func (ts *timeutilSuite) TestTimeZero(c *C) { + // test with a zero time stamp to make sure that code does not do + // anything silly + + // zero time is: time is: 0001-01-01 00:00:00 +0000 UTC and ... Monday + zero := time.Time{} + c.Logf("time is: %v weekday: %v", zero, zero.Weekday()) + + for _, schedule := range []string{ + "mon-tue,0:00-12:00", + "mon1-tue,0:00-12:00", + "mon-tue1,0:00-12:00", + } { + c.Logf("trying: %v", schedule) + sch, err := timeutil.ParseSchedule(schedule) + c.Assert(err, IsNil) + + c.Check(timeutil.Includes(sch, zero), Equals, true) + c.Check(timeutil.Includes(sch, zero.Add(5*time.Hour)), Equals, true) + // wednesday + c.Check(timeutil.Includes(sch, zero.Add(2*24*time.Hour)), Equals, false) + } +} + +func (ts *timeutilSuite) TestMonthNext(c *C) { + const shortForm = "2006-01-02" + for _, t := range []struct { + when, next string + }{ + {"2018-07-01", "2018-08-01"}, + {"2018-07-31", "2018-08-01"}, + {"2018-07-20", "2018-08-01"}, + {"2018-02-01", "2018-03-01"}, + {"2018-02-28", "2018-03-01"}, + {"2018-01-31", "2018-02-01"}, + // in 2020 Feb is 29 days + {"2020-01-31", "2020-02-01"}, + {"2020-02-01", "2020-03-01"}, + {"2020-02-14", "2020-03-01"}, + } { + when, err := time.ParseInLocation(shortForm, t.when, time.Local) + c.Assert(err, IsNil) + c.Logf("when: %v expecting: %v", when, t.next) + + next := timeutil.MonthNext(when) + c.Check(next.Format(shortForm), Equals, t.next) + } + +} From 4197513c043fc31b9919c0b26cc080181c00c98f Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Mon, 3 Aug 2026 15:34:11 +1000 Subject: [PATCH 2/9] feat: add schedule to service --- internals/plan/plan.go | 13 ++++++++++++ internals/plan/plan_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/internals/plan/plan.go b/internals/plan/plan.go index 62be686ab..3dd328d6c 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -32,6 +32,7 @@ import ( "github.com/canonical/pebble/internals/logger" "github.com/canonical/pebble/internals/osutil" + "github.com/canonical/pebble/internals/timeutil" ) // SectionExtension allows the plan layer schema to be extended without @@ -218,6 +219,7 @@ type Service struct { Startup ServiceStartup `yaml:"startup,omitempty"` Override Override `yaml:"override,omitempty"` Command string `yaml:"command,omitempty"` + Schedule string `yaml:"schedule,omitempty"` // Service dependencies After []string `yaml:"after,omitempty"` @@ -274,6 +276,9 @@ func (s *Service) Merge(other *Service) { if other.Command != "" { s.Command = other.Command } + if other.Schedule != "" { + s.Schedule = other.Schedule + } if other.KillDelay.IsSet { s.KillDelay = other.KillDelay } @@ -906,6 +911,14 @@ func (layer *Layer) Validate() error { Message: fmt.Sprintf("plan service %q backoff-factor must be 1.0 or greater, not %g", name, service.BackoffFactor.Value), } } + if service.Schedule != "" { + _, err := timeutil.ParseSchedule(service.Schedule) + if err != nil { + return &FormatError{ + Message: fmt.Sprintf("plan service %q schedule %q invalid: %v", name, service.Schedule, err), + } + } + } } for name, check := range layer.Checks { diff --git a/internals/plan/plan_test.go b/internals/plan/plan_test.go index eed4d5462..eecd5b612 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -124,6 +124,7 @@ var planTests = []planTest{{ srv6: override: replace command: cmd6a + schedule: 9:00 `, ` summary: Simple override layer. description: The second layer. @@ -203,6 +204,7 @@ var planTests = []planTest{{ Override: "replace", Command: "cmd6a", Startup: plan.StartupUnknown, + Schedule: "9:00", }, }, Checks: map[string]*plan.Check{}, @@ -324,6 +326,7 @@ var planTests = []planTest{{ Name: "srv6", Override: "replace", Command: "cmd6b", + Schedule: "9:00", Environment: map[string]string{ "foo": "bar", "baz": "buz", @@ -581,6 +584,15 @@ var planTests = []planTest{{ override: replace command: cmd -v [ foo [ --bar ] ] `}, +}, { + summary: `Invalid service schedule: cannot nest [ ... ] groups`, + error: `plan service \"svc1\" schedule \"fry\" invalid: cannot parse \"fry\": \"fry\" is not a valid weekday`, + input: []string{` + services: + "svc1": + override: replace + schedule: fry + `}, }, { summary: "Checks fields parse correctly and defaults are correct", input: []string{` @@ -1032,6 +1044,36 @@ var planTests = []planTest{{ }, Sections: map[string]plan.Section{}, }, +}, { + summary: "Merging service schedule across layers", + input: []string{` + services: + svc1: + command: foo + override: replace + schedule: 9:00-11:00 + `, ` + services: + svc1: + override: merge + schedule: 13:00-15:00 + `}, + result: &plan.Layer{ + Services: map[string]*plan.Service{ + "svc1": { + Name: "svc1", + Command: "foo", + Override: plan.ReplaceOverride, + Schedule: "13:00-15:00", + BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, + BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, + BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, + }, + }, + Checks: map[string]*plan.Check{}, + LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, + }, }, { summary: "Overriding log targets", input: []string{` From 704eee422d8552710af250cda458e44f01c81d26 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Tue, 4 Aug 2026 11:18:59 +1000 Subject: [PATCH 3/9] feat: schedule service start via changes --- internals/overlord/servstate/export_test.go | 10 + internals/overlord/servstate/manager.go | 12 +- internals/overlord/servstate/schedule.go | 311 +++++++++++++++ internals/overlord/servstate/schedule_test.go | 358 ++++++++++++++++++ 4 files changed, 689 insertions(+), 2 deletions(-) create mode 100644 internals/overlord/servstate/schedule.go create mode 100644 internals/overlord/servstate/schedule_test.go diff --git a/internals/overlord/servstate/export_test.go b/internals/overlord/servstate/export_test.go index 7a6a3052a..16f54216e 100644 --- a/internals/overlord/servstate/export_test.go +++ b/internals/overlord/servstate/export_test.go @@ -22,6 +22,16 @@ import ( "github.com/canonical/pebble/internals/plan" ) +const ( + ServiceScheduleKind = serviceScheduleKind + ScheduleDetailsAttr = scheduleDetailsAttr +) + +var ( + ScheduleShouldRunNow = scheduleShouldRunNow + NextScheduleTime = nextScheduleTime +) + var CalculateNextBackoff = calculateNextBackoff var GetAction = getAction diff --git a/internals/overlord/servstate/manager.go b/internals/overlord/servstate/manager.go index 5662ef015..dfa867607 100644 --- a/internals/overlord/servstate/manager.go +++ b/internals/overlord/servstate/manager.go @@ -60,14 +60,22 @@ func NewManager(s *state.State, runner *state.TaskRunner, serviceOutput io.Write runner.AddHandler("start", manager.doStart, nil) runner.AddHandler("stop", manager.doStop, nil) + // Schedule changes persist for as long as a service has a schedule + // configured. This ensures they don't get pruned. + s.RegisterPendingChangeByAttr(scheduleNoPruneAttr, func(*state.Change) bool { + return true + }) + return manager, nil } // PlanChanged informs the service manager that the plan has been updated. func (m *ServiceManager) PlanChanged(plan *plan.Plan) { m.planLock.Lock() - defer m.planLock.Unlock() m.plan = plan + m.planLock.Unlock() + + m.scheduleChanged(plan) } // getPlan returns the current plan pointer in a concurrency-safe way. The @@ -88,7 +96,7 @@ func (m *ServiceManager) getPlan() *plan.Plan { // Ensure implements StateManager.Ensure. func (m *ServiceManager) Ensure() error { - return nil + return m.ensureSchedules() } type ServiceInfo struct { diff --git a/internals/overlord/servstate/schedule.go b/internals/overlord/servstate/schedule.go new file mode 100644 index 000000000..d27d40465 --- /dev/null +++ b/internals/overlord/servstate/schedule.go @@ -0,0 +1,311 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package servstate + +import ( + "fmt" + "math" + "sort" + "time" + + "github.com/canonical/pebble/internals/logger" + "github.com/canonical/pebble/internals/overlord/state" + "github.com/canonical/pebble/internals/plan" + "github.com/canonical/pebble/internals/timeutil" +) + +const ( + // serviceScheduleKind is the kind used for both the change and the task + // that tracks a service's next scheduled start time. There is at most one + // such change per service at any given time. + serviceScheduleKind = "service-schedule" + + // scheduleDetailsAttr is the task attribute holding scheduleDetails. + scheduleDetailsAttr = "service-schedule-details" + + // scheduleNoPruneAttr marks the schedule change as one that must never + // be pruned while it's still tracking a service's schedule. + scheduleNoPruneAttr = "service-schedule-no-prune" + + // maxScheduleLookahead bounds how far in the future we search for the next + // scheduled start time. + maxScheduleLookahead = 366 * 24 * time.Hour + + // scheduleMissThreshold is how overdue a scheduled start has to be before + // we call it out explicitly as "missed" in the task log. + scheduleMissThreshold = 5 * time.Second +) + +// scheduleDetails is persisted on the service-schedule task, and records the +// schedule string, with the next time the service should be started. +type scheduleDetails struct { + ServiceName string `json:"service-name"` + Schedule string `json:"schedule"` + Next time.Time `json:"next"` +} + +// nextScheduleTime parses the schedule, returning the next time that it fires, +// according to the current time. +func nextScheduleTime(scheduleStr string, last time.Time) (time.Time, error) { + schedules, err := timeutil.ParseSchedule(scheduleStr) + if err != nil { + return time.Time{}, err + } + d := timeutil.Next(schedules, last, maxScheduleLookahead) + return timeNow().Add(d), nil +} + +// scheduleShouldRunNow decides whether a scheduled start that was missed should +// still be acted on now, or skipped in favour of waiting for the following +// occurrence. +// +// The decision is based on which of the two candidate times is closer to now. +// If the missed time is closer (or equidistant), we start the service now; if +// the following occurrence is closer, we wait for it instead. +func scheduleShouldRunNow(now, missed, following time.Time) bool { + if following.IsZero() { + return now.After(missed) + } + missedDelta := max(now.Sub(missed), 0) + followingDelta := max(following.Sub(now), 0) + return missedDelta <= followingDelta +} + +// serviceScheduleChange creates the change/task pair used to track name's next +// scheduled start time, and returns the change ID. +// The caller must hold the state lock. +func serviceScheduleChange(st *state.State, name, scheduleStr string, next time.Time) string { + summary := fmt.Sprintf("Wait for scheduled start of service %q", name) + task := st.NewTask(serviceScheduleKind, summary) + task.Set(scheduleDetailsAttr, &scheduleDetails{ + ServiceName: name, + Schedule: scheduleStr, + Next: next, + }) + // This task is never picked up by a TaskRunner handler (none is + // registered for this kind); it's driven entirely by + // ServiceManager.Ensure. Mark it Doing so it's clear from "pebble + // changes"/"pebble tasks" that it's ongoing. + task.SetStatus(state.DoingStatus) + + change := st.NewChange(serviceScheduleKind, summary) + change.Set(scheduleNoPruneAttr, true) + change.AddTask(task) + return change.ID() +} + +// scheduleChanged is called from PlanChanged to create, update, or retire +// the per-service schedule changes/tasks to match the new plan. +func (m *ServiceManager) scheduleChanged(newPlan *plan.Plan) { + m.state.Lock() + defer m.state.Unlock() + + shouldEnsure := false + existing := make(map[string]bool) + + for _, change := range m.state.Changes() { + if change.Kind() != serviceScheduleKind || change.IsReady() { + continue + } + task := change.Tasks()[0] + if !task.Has(scheduleDetailsAttr) { + continue + } + var details scheduleDetails + err := task.Get(scheduleDetailsAttr, &details) + if err != nil { + logger.Noticef("Cannot get %s change %s schedule details: %v", change.Kind(), change.ID(), err) + task.Errorf("Cannot get %s change %s schedule details: %v", change.Kind(), change.ID(), err) + continue + } + existing[details.ServiceName] = true + + config, inPlan := newPlan.Services[details.ServiceName] + if !inPlan || config.Schedule == "" { + // Service removed from the plan, or no longer has a schedule: + // retire this change. We can't use change.Abort() here because + // there's no TaskRunner handler registered for this task kind to + // process the resulting Abort/Undo status. + task.Logf("Service %q no longer has a schedule configured; no more scheduled starts.", details.ServiceName) + task.SetStatus(state.HoldStatus) + shouldEnsure = true + continue + } + + if config.Schedule == details.Schedule { + // Schedule hasn't changed. + continue + } + + // Schedule string changed: recompute the next scheduled start time, but + // reuse the existing change/task rather than creating a new one. + next, err := nextScheduleTime(config.Schedule, details.Next) + if err != nil { + logger.Noticef("Cannot parse schedule %q for service %q: %v", config.Schedule, details.ServiceName, err) + continue + } + task.Logf("Schedule for service %q changed from %q to %q; next scheduled start at %s.", + details.ServiceName, details.Schedule, config.Schedule, next.Format(time.RFC3339)) + task.Set(scheduleDetailsAttr, &scheduleDetails{ + ServiceName: details.ServiceName, + Schedule: config.Schedule, + Next: next, + }) + shouldEnsure = true + } + + // Start tracking schedules for services that are newly configured with + // one (and don't already have a change tracking them). + names := make([]string, 0, len(newPlan.Services)) + for name := range newPlan.Services { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + config := newPlan.Services[name] + if existing[name] || config.Schedule == "" { + continue + } + // Use "yesterday" as the reference point so that a schedule window + // that's already open today is picked up immediately, without + // having timeutil step forward one day at a time from long ago. + next, err := nextScheduleTime(config.Schedule, timeNow().Add(-24*time.Hour)) + if err != nil { + logger.Noticef("Cannot parse schedule %q for service %q: %v", config.Schedule, name, err) + continue + } + serviceScheduleChange(m.state, name, config.Schedule, next) + shouldEnsure = true + } + + if shouldEnsure { + m.state.EnsureBefore(0) + } +} + +// serviceIsActive reports whether the named service is currently started, +// starting, or otherwise considered "running" for scheduling purposes. +func (m *ServiceManager) serviceIsActive(name string) bool { + m.servicesLock.Lock() + defer m.servicesLock.Unlock() + + s := m.services[name] + if s == nil { + return false + } + switch s.state { + case stateInitial, stateStarting, stateRunning: + return true + default: + return false + } +} + +// startServiceOnSchedule creates an independent "start" change for name (the +// same machinery used by the API's start/replan actions), and returns its +// change ID. The caller must hold the state lock. +func (m *ServiceManager) startServiceOnSchedule(name string) (changeID string, err error) { + lanes, err := m.StartOrder([]string{name}) + if err != nil { + return "", err + } + taskSet, err := Start(m.state, lanes) + if err != nil { + return "", err + } + change := m.state.NewChange("start", fmt.Sprintf("Start service %q on schedule", name)) + change.AddAll(taskSet) + m.state.EnsureBefore(0) + return change.ID(), nil +} + +// ensureSchedules starts any services whose scheduled timer has elapsed, and to +// reschedule the next start. +func (m *ServiceManager) ensureSchedules() error { + m.state.Lock() + defer m.state.Unlock() + + var ( + now time.Time = timeNow() + before time.Duration = math.MaxInt64 + ) + + for _, change := range m.state.Changes() { + if change.Kind() != serviceScheduleKind || change.IsReady() { + continue + } + task := change.Tasks()[0] + if !task.Has(scheduleDetailsAttr) { + continue + } + + var details scheduleDetails + err := task.Get(scheduleDetailsAttr, &details) + if err != nil { + return fmt.Errorf("cannot get service-schedule-details from task: %w", err) + } + + if details.Next.IsZero() { + continue + } + + if now.Before(details.Next) { + before = min(before, details.Next.Sub(now)) + continue + } + + missed := details.Next + following, err := nextScheduleTime(details.Schedule, missed) + if err != nil { + logger.Noticef("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) + task.Errorf("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) + } + + followingMsg := "not scheduled again" + if !following.IsZero() { + followingMsg = fmt.Sprintf("next scheduled start at %s", following.Format(time.RFC3339)) + } + if scheduleShouldRunNow(now, missed, following) { + if now.Sub(missed) > scheduleMissThreshold { + task.Logf("Missed scheduled start at %s for service %q; starting it now.", + missed.Format(time.RFC3339), details.ServiceName) + } + if m.serviceIsActive(details.ServiceName) { + task.Logf("Service %q is already running; %s.", details.ServiceName, followingMsg) + } else { + startedChangeID, err := m.startServiceOnSchedule(details.ServiceName) + if err != nil { + task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) + } else { + task.Logf("Started service %q on schedule (change %q); %s.", + details.ServiceName, startedChangeID, followingMsg) + } + } + } else { + task.Logf("Skipped scheduled start at %s for service %q (missed by too long); %s.", + missed.Format(time.RFC3339), details.ServiceName, followingMsg) + } + + details.Next = following + task.Set(scheduleDetailsAttr, &details) + before = min(before, following.Sub(now)) + } + + if before < math.MaxInt64 { + m.state.EnsureBefore(before) + } + + return nil +} diff --git a/internals/overlord/servstate/schedule_test.go b/internals/overlord/servstate/schedule_test.go new file mode 100644 index 000000000..e4b9e2e4e --- /dev/null +++ b/internals/overlord/servstate/schedule_test.go @@ -0,0 +1,358 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package servstate_test + +import ( + "strings" + "time" + + . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/servstate" + "github.com/canonical/pebble/internals/overlord/state" +) + +// scheduleDetails mirrors the JSON shape of servstate's internal +// scheduleDetails type, so tests can inspect/mutate it via the task's +// generic Get/Set without needing access to the unexported type itself. +type scheduleDetails struct { + ServiceName string `json:"service-name"` + Schedule string `json:"schedule"` + Next time.Time `json:"next"` +} + +const scheduleTestLayer = ` +services: + sched1: + override: replace + command: /bin/sh -c "sleep 10" + schedule: 9:00-11:00 +` + +// scheduleChange returns the (single, expected) service-schedule change, or +// nil if none exists. +func (s *S) scheduleChange(c *C) *state.Change { + s.st.Lock() + defer s.st.Unlock() + for _, chg := range s.st.Changes() { + if chg.Kind() == servstate.ServiceScheduleKind { + return chg + } + } + return nil +} + +func (s *S) scheduleDetails(c *C, chg *state.Change) scheduleDetails { + s.st.Lock() + defer s.st.Unlock() + tasks := chg.Tasks() + c.Assert(tasks, HasLen, 1) + var details scheduleDetails + err := tasks[0].Get(servstate.ScheduleDetailsAttr, &details) + c.Assert(err, IsNil) + return details +} + +func (s *S) setScheduleNext(c *C, chg *state.Change, next time.Time) { + s.st.Lock() + defer s.st.Unlock() + tasks := chg.Tasks() + c.Assert(tasks, HasLen, 1) + var details scheduleDetails + err := tasks[0].Get(servstate.ScheduleDetailsAttr, &details) + c.Assert(err, IsNil) + details.Next = next + tasks[0].Set(servstate.ScheduleDetailsAttr, &details) +} + +func (s *S) scheduleTaskLog(c *C, chg *state.Change) []string { + s.st.Lock() + defer s.st.Unlock() + tasks := chg.Tasks() + c.Assert(tasks, HasLen, 1) + return tasks[0].Log() +} + +func (s *S) countChangesOfKind(c *C, kind string) int { + s.st.Lock() + defer s.st.Unlock() + n := 0 + for _, chg := range s.st.Changes() { + if chg.Kind() == kind { + n++ + } + } + return n +} + +func logContains(logs []string, substr string) bool { + for _, l := range logs { + if strings.Contains(l, substr) { + return true + } + } + return false +} + +// -- Pure decision function -- + +func (s *S) TestScheduleShouldRunNow(c *C) { + now := time.Now() + + // Missed by a minute, next occurrence an hour away: closer to the + // missed time, so run now. + c.Check(servstate.ScheduleShouldRunNow(now, now.Add(-time.Minute), now.Add(time.Hour)), Equals, true) + + // Missed by 10 days, next occurrence an hour away: closer to the next + // occurrence, so skip. + c.Check(servstate.ScheduleShouldRunNow(now, now.Add(-10*24*time.Hour), now.Add(time.Hour)), Equals, false) + + // Equidistant: favour running now. + c.Check(servstate.ScheduleShouldRunNow(now, now.Add(-time.Hour), now.Add(time.Hour)), Equals, true) +} + +func (s *S) TestNextScheduleTimeInvalid(c *C) { + _, err := servstate.NextScheduleTime("not-a-schedule", time.Now()) + c.Assert(err, NotNil) +} + +// -- PlanChanged behaviour -- + +func (s *S) TestScheduleCreatedOnPlanChanged(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + + details := s.scheduleDetails(c, chg) + c.Check(details.ServiceName, Equals, "sched1") + c.Check(details.Schedule, Equals, "9:00-11:00") + c.Check(details.Next.IsZero(), Equals, false) + // The schedule fires daily, so the next occurrence should always be + // within a day of now. + c.Check(details.Next.Before(time.Now().Add(25*time.Hour)), Equals, true) +} + +func (s *S) TestScheduleNotCreatedWithoutSchedule(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, ` +services: + plain1: + override: replace + command: /bin/sh -c "sleep 10" +`) + s.planChanged(c) + + c.Check(s.scheduleChange(c), IsNil) +} + +func (s *S) TestScheduleUnchangedKeepsNext(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + details1 := s.scheduleDetails(c, chg) + + // Re-applying the same plan shouldn't touch the next scheduled time, + // nor create a second change. + s.planChanged(c) + + c.Check(s.countChangesOfKind(c, servstate.ServiceScheduleKind), Equals, 1) + chg2 := s.scheduleChange(c) + c.Assert(chg2.ID(), Equals, chg.ID()) + details2 := s.scheduleDetails(c, chg2) + c.Check(details2.Next.Equal(details1.Next), Equals, true) +} + +func (s *S) TestScheduleChangedUpdatesNextAndReusesChange(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + + s.planAddLayer(c, ` +services: + sched1: + override: merge + schedule: 13:00-15:00 +`) + s.planChanged(c) + + // Same change/task should have been reused, not a new one. + c.Check(s.countChangesOfKind(c, servstate.ServiceScheduleKind), Equals, 1) + chg2 := s.scheduleChange(c) + c.Assert(chg2.ID(), Equals, chg.ID()) + + details2 := s.scheduleDetails(c, chg2) + c.Check(details2.Schedule, Equals, "13:00-15:00") + + logs := s.scheduleTaskLog(c, chg2) + c.Check(logContains(logs, "Schedule for service"), Equals, true) +} + +func (s *S) TestScheduleRemovedRetiresChange(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + + // Replace the service definition with one that has no schedule. + s.planAddLayer(c, ` +services: + sched1: + override: replace + command: /bin/sh -c "sleep 10" +`) + s.planChanged(c) + + s.st.Lock() + ready := chg.IsReady() + status := chg.Status() + s.st.Unlock() + c.Check(ready, Equals, true) + c.Check(status, Equals, state.HoldStatus) + + // No new schedule change should have been created for the service. + c.Check(s.countChangesOfKind(c, servstate.ServiceScheduleKind), Equals, 1) +} + +// -- Ensure behaviour -- + +func (s *S) TestEnsureStartsServiceOnSchedule(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + + // Force the schedule to be due right now. + s.setScheduleNext(c, chg, time.Now().Add(-time.Second)) + + err := s.manager.Ensure() + c.Assert(err, IsNil) + + startChg := s.findChangeOfKind(c, "start") + c.Assert(startChg, NotNil) + waitChangeReady(c, s.runner, startChg, "service to start on schedule") + + s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { + return svc.Current == servstate.StatusActive + }) + + logs := s.scheduleTaskLog(c, chg) + c.Check(logContains(logs, "Started service"), Equals, true) +} + +func (s *S) TestEnsureLogsMissedScheduleButStillRuns(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + + // Missed by 30 seconds (well over the "missed" logging threshold), but + // still much closer to now than the next (daily) occurrence, so it + // should run anyway. + s.setScheduleNext(c, chg, time.Now().Add(-30*time.Second)) + + err := s.manager.Ensure() + c.Assert(err, IsNil) + + startChg := s.findChangeOfKind(c, "start") + c.Assert(startChg, NotNil) + waitChangeReady(c, s.runner, startChg, "service to start on schedule") + + s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { + return svc.Current == servstate.StatusActive + }) + + logs := s.scheduleTaskLog(c, chg) + c.Check(logContains(logs, "Missed scheduled start"), Equals, true) +} + +func (s *S) TestEnsureSkipsStartWhenAlreadyRunning(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + // Start the service manually first. + s.startServices(c, [][]string{{"sched1"}}) + s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { + return svc.Current == servstate.StatusActive + }) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + s.setScheduleNext(c, chg, time.Now().Add(-time.Second)) + + err := s.manager.Ensure() + c.Assert(err, IsNil) + + // No "start" change should have been created by the schedule. + c.Check(s.countChangesOfKind(c, "start"), Equals, 0) + + logs := s.scheduleTaskLog(c, chg) + c.Check(logContains(logs, "already running"), Equals, true) +} + +func (s *S) TestEnsureSkipsFarMissedSchedule(c *C) { + s.newServiceManager(c) + s.planAddLayer(c, scheduleTestLayer) + s.planChanged(c) + + chg := s.scheduleChange(c) + c.Assert(chg, NotNil) + + longAgo := time.Now().Add(-240 * time.Hour) // 10 days ago + s.setScheduleNext(c, chg, longAgo) + + err := s.manager.Ensure() + c.Assert(err, IsNil) + + // No service should have been started because of this. + c.Check(s.countChangesOfKind(c, "start"), Equals, 0) + + details := s.scheduleDetails(c, chg) + // Rescheduled well into the future relative to the missed time (the + // schedule fires daily, so the new Next should be close to now, not + // close to the 10-day-old missed time). + c.Check(details.Next.After(longAgo.Add(48*time.Hour)), Equals, true) + + logs := s.scheduleTaskLog(c, chg) + c.Check(logContains(logs, "Skipped scheduled start"), Equals, true) +} + +// findChangeOfKind returns the first change of the given kind not equal to +// any service-schedule change, or nil if none is found. +func (s *S) findChangeOfKind(c *C, kind string) *state.Change { + s.st.Lock() + defer s.st.Unlock() + for _, chg := range s.st.Changes() { + if chg.Kind() == kind { + return chg + } + } + return nil +} From be5f20297744c8853e3579cea61cf06b26044d0c Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Wed, 5 Aug 2026 17:06:46 +1000 Subject: [PATCH 4/9] fix: keep service schedule waiting in a single change --- internals/overlord/servstate/export_test.go | 5 +- internals/overlord/servstate/manager.go | 13 ++ internals/overlord/servstate/schedule.go | 197 +++++++++++++++--- internals/overlord/servstate/schedule_test.go | 99 ++++++--- 4 files changed, 246 insertions(+), 68 deletions(-) diff --git a/internals/overlord/servstate/export_test.go b/internals/overlord/servstate/export_test.go index 16f54216e..8c4190c37 100644 --- a/internals/overlord/servstate/export_test.go +++ b/internals/overlord/servstate/export_test.go @@ -28,8 +28,9 @@ const ( ) var ( - ScheduleShouldRunNow = scheduleShouldRunNow - NextScheduleTime = nextScheduleTime + ScheduleShouldRunNow = scheduleShouldRunNow + NextScheduleTime = nextScheduleTime + NextScheduleTimeAfter = nextScheduleTimeAfter ) var CalculateNextBackoff = calculateNextBackoff diff --git a/internals/overlord/servstate/manager.go b/internals/overlord/servstate/manager.go index dfa867607..d5d1709f9 100644 --- a/internals/overlord/servstate/manager.go +++ b/internals/overlord/servstate/manager.go @@ -59,6 +59,12 @@ func NewManager(s *state.State, runner *state.TaskRunner, serviceOutput io.Write runner.AddHandler("start", manager.doStart, nil) runner.AddHandler("stop", manager.doStop, nil) + // doServiceSchedule doesn't actually do anything but park itself: it + // exists so that service-schedule tasks (which are driven by + // ServiceManager.Ensure, not a task runner handler) aren't picked up by + // the task runner's generic handling for tasks with no registered + // handler, which would otherwise mark them Done immediately. + runner.AddHandler(serviceScheduleKind, manager.doServiceSchedule, nil) // Schedule changes persist for as long as a service has a schedule // configured. This ensures they don't get pruned. @@ -66,6 +72,13 @@ func NewManager(s *state.State, runner *state.TaskRunner, serviceOutput io.Write return true }) + // Chain service-schedule changes: once one becomes ready (its scheduled + // start fired and any resulting start task(s) finished), create a new + // one to track the service's next scheduled occurrence. + s.Lock() + s.AddChangeStatusChangedHandler(manager.scheduleChangeReady) + s.Unlock() + return manager, nil } diff --git a/internals/overlord/servstate/schedule.go b/internals/overlord/servstate/schedule.go index d27d40465..e07afdcf6 100644 --- a/internals/overlord/servstate/schedule.go +++ b/internals/overlord/servstate/schedule.go @@ -20,6 +20,8 @@ import ( "sort" "time" + "gopkg.in/tomb.v2" + "github.com/canonical/pebble/internals/logger" "github.com/canonical/pebble/internals/overlord/state" "github.com/canonical/pebble/internals/plan" @@ -27,9 +29,17 @@ import ( ) const ( - // serviceScheduleKind is the kind used for both the change and the task - // that tracks a service's next scheduled start time. There is at most one - // such change per service at any given time. + // serviceScheduleKind is the kind used for both the change and the first + // task in it, which tracks a service's next scheduled start time. There + // is at most one such change per service at any given time. + // + // The change starts out with just this one task, in DoingStatus, while + // waiting for the scheduled time. Once that time arrives and the service + // is actually started, a "start" task (or task set, if the service has + // dependencies) is added to the same change and the service-schedule + // task is marked Done. Once the start task(s) finish, the whole change + // becomes ready, and scheduleChangeReady creates a new service-schedule + // change to track the next occurrence. serviceScheduleKind = "service-schedule" // scheduleDetailsAttr is the task attribute holding scheduleDetails. @@ -46,6 +56,10 @@ const ( // scheduleMissThreshold is how overdue a scheduled start has to be before // we call it out explicitly as "missed" in the task log. scheduleMissThreshold = 5 * time.Second + + // scheduleTaskPollInterval is how often doServiceSchedule checks whether + // its task has been finished externally. + scheduleTaskPollInterval = time.Second ) // scheduleDetails is persisted on the service-schedule task, and records the @@ -67,6 +81,35 @@ func nextScheduleTime(scheduleStr string, last time.Time) (time.Time, error) { return timeNow().Add(d), nil } +// nextScheduleTimeAfter is like nextScheduleTime, but guarantees that the +// result (if non-zero) is strictly after the current time. +// +// This matters because timeutil.Next can return a time that isn't after now +// even when "last" is itself a past occurrence: for a schedule with a spread +// (randomised) window, re-evaluating from a previous occurrence can land back +// inside the same still-open window with a new random offset. Without this +// guard, that can result in a scheduled start's "next" time never actually +// advancing into the future, causing it to be treated as immediately due +// again and again. +// +// Re-deriving from the current time instead forces progress, since "now" is +// always considered part of whatever window contains it, so that window gets +// skipped in favour of a later one. +func nextScheduleTimeAfter(scheduleStr string, last time.Time) (time.Time, error) { + next, err := nextScheduleTime(scheduleStr, last) + if err != nil { + return time.Time{}, err + } + now := timeNow() + if !next.IsZero() && !next.After(now) { + next, err = nextScheduleTime(scheduleStr, now) + if err != nil { + return time.Time{}, err + } + } + return next, nil +} + // scheduleShouldRunNow decides whether a scheduled start that was missed should // still be acted on now, or skipped in favour of waiting for the following // occurrence. @@ -94,10 +137,10 @@ func serviceScheduleChange(st *state.State, name, scheduleStr string, next time. Schedule: scheduleStr, Next: next, }) - // This task is never picked up by a TaskRunner handler (none is - // registered for this kind); it's driven entirely by - // ServiceManager.Ensure. Mark it Doing so it's clear from "pebble - // changes"/"pebble tasks" that it's ongoing. + // doServiceSchedule (registered as this kind's TaskRunner handler) parks + // until the status below is changed by ensureSchedules/scheduleChanged; + // it's what drives this task, not the handler itself. Mark it Doing so + // it's clear from "pebble changes"/"pebble tasks" that it's ongoing. task.SetStatus(state.DoingStatus) change := st.NewChange(serviceScheduleKind, summary) @@ -106,6 +149,33 @@ func serviceScheduleChange(st *state.State, name, scheduleStr string, next time. return change.ID() } +// doServiceSchedule is the TaskRunner handler for serviceScheduleKind tasks. +// +// The task's outcome is driven entirely by ServiceManager.Ensure (see +// ensureSchedules) and scheduleChanged, which change its status away from +// DoingStatus once the scheduled time arrives (or the schedule is retired). +// This handler's only job is to occupy the task runner's "do" slot for this +// task kind, so that it isn't picked up by the runner's generic handling for +// tasks with no registered handler (which would otherwise mark it Done +// immediately, defeating the entire point of it representing "still +// waiting"). It just blocks, polling for that external status change, until +// it happens or the runner asks it to stop. +func (m *ServiceManager) doServiceSchedule(task *state.Task, tomb *tomb.Tomb) error { + for { + select { + case <-tomb.Dying(): + return tomb.Err() + case <-time.After(scheduleTaskPollInterval): + } + m.state.Lock() + stillWaiting := task.Status() == state.DoingStatus + m.state.Unlock() + if !stillWaiting { + return nil + } + } +} + // scheduleChanged is called from PlanChanged to create, update, or retire // the per-service schedule changes/tasks to match the new plan. func (m *ServiceManager) scheduleChanged(newPlan *plan.Plan) { @@ -119,6 +189,10 @@ func (m *ServiceManager) scheduleChanged(newPlan *plan.Plan) { if change.Kind() != serviceScheduleKind || change.IsReady() { continue } + // The first task is always the service-schedule task. It may already + // be Done at this point (with a "start" task set alongside it in the + // same change, still running) if the scheduled time has already + // passed; that's fine, we still want to update/retire it below. task := change.Tasks()[0] if !task.Has(scheduleDetailsAttr) { continue @@ -135,9 +209,9 @@ func (m *ServiceManager) scheduleChanged(newPlan *plan.Plan) { config, inPlan := newPlan.Services[details.ServiceName] if !inPlan || config.Schedule == "" { // Service removed from the plan, or no longer has a schedule: - // retire this change. We can't use change.Abort() here because - // there's no TaskRunner handler registered for this task kind to - // process the resulting Abort/Undo status. + // retire this change directly (rather than via change.Abort(), + // which would go through the Abort/Undo dance) since there's + // nothing to undo here. task.Logf("Service %q no longer has a schedule configured; no more scheduled starts.", details.ServiceName) task.SetStatus(state.HoldStatus) shouldEnsure = true @@ -151,7 +225,7 @@ func (m *ServiceManager) scheduleChanged(newPlan *plan.Plan) { // Schedule string changed: recompute the next scheduled start time, but // reuse the existing change/task rather than creating a new one. - next, err := nextScheduleTime(config.Schedule, details.Next) + next, err := nextScheduleTimeAfter(config.Schedule, details.Next) if err != nil { logger.Noticef("Cannot parse schedule %q for service %q: %v", config.Schedule, details.ServiceName, err) continue @@ -213,24 +287,6 @@ func (m *ServiceManager) serviceIsActive(name string) bool { } } -// startServiceOnSchedule creates an independent "start" change for name (the -// same machinery used by the API's start/replan actions), and returns its -// change ID. The caller must hold the state lock. -func (m *ServiceManager) startServiceOnSchedule(name string) (changeID string, err error) { - lanes, err := m.StartOrder([]string{name}) - if err != nil { - return "", err - } - taskSet, err := Start(m.state, lanes) - if err != nil { - return "", err - } - change := m.state.NewChange("start", fmt.Sprintf("Start service %q on schedule", name)) - change.AddAll(taskSet) - m.state.EnsureBefore(0) - return change.ID(), nil -} - // ensureSchedules starts any services whose scheduled timer has elapsed, and to // reschedule the next start. func (m *ServiceManager) ensureSchedules() error { @@ -250,6 +306,12 @@ func (m *ServiceManager) ensureSchedules() error { if !task.Has(scheduleDetailsAttr) { continue } + if task.Status() != state.DoingStatus { + // Already fired: the start task(s) added alongside it are still + // running. Nothing more to do here until the whole change becomes + // ready (see scheduleChangeReady). + continue + } var details scheduleDetails err := task.Get(scheduleDetailsAttr, &details) @@ -267,7 +329,7 @@ func (m *ServiceManager) ensureSchedules() error { } missed := details.Next - following, err := nextScheduleTime(details.Schedule, missed) + following, err := nextScheduleTimeAfter(details.Schedule, missed) if err != nil { logger.Noticef("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) task.Errorf("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) @@ -277,20 +339,32 @@ func (m *ServiceManager) ensureSchedules() error { if !following.IsZero() { followingMsg = fmt.Sprintf("next scheduled start at %s", following.Format(time.RFC3339)) } + + fired := false if scheduleShouldRunNow(now, missed, following) { if now.Sub(missed) > scheduleMissThreshold { task.Logf("Missed scheduled start at %s for service %q; starting it now.", missed.Format(time.RFC3339), details.ServiceName) } if m.serviceIsActive(details.ServiceName) { + // Stay on this change/task: just log and move on to the next + // occurrence. task.Logf("Service %q is already running; %s.", details.ServiceName, followingMsg) } else { - startedChangeID, err := m.startServiceOnSchedule(details.ServiceName) + lanes, err := m.StartOrder([]string{details.ServiceName}) if err != nil { task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) } else { - task.Logf("Started service %q on schedule (change %q); %s.", - details.ServiceName, startedChangeID, followingMsg) + taskSet, err := Start(m.state, lanes) + if err != nil { + task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) + } else { + // Add the start task(s) to this same change, rather + // than creating an independent one. + change.AddAll(taskSet) + task.Logf("Started service %q on schedule; %s.", details.ServiceName, followingMsg) + fired = true + } } } } else { @@ -298,9 +372,22 @@ func (m *ServiceManager) ensureSchedules() error { missed.Format(time.RFC3339), details.ServiceName, followingMsg) } + // Record the following occurrence, whether we fired or not: if we + // fired, scheduleChangeReady reads this back once the change becomes + // ready, to create the follow-up change. details.Next = following task.Set(scheduleDetailsAttr, &details) - before = min(before, following.Sub(now)) + + if fired { + // Mark the service-schedule task done now that the start task(s) + // have been added to the change (must happen after AddAll/Set + // above, so the change's ready channel doesn't close + // prematurely). The change becomes ready once the start task(s) + // finish, at which point scheduleChangeReady takes over. + task.SetStatus(state.DoneStatus) + } else { + before = min(before, following.Sub(now)) + } } if before < math.MaxInt64 { @@ -309,3 +396,45 @@ func (m *ServiceManager) ensureSchedules() error { return nil } + +// scheduleChangeReady is called whenever a change's status changes; it looks +// for service-schedule changes that have just become ready (i.e. their +// service-schedule task fired and any start task(s) added alongside it have +// now finished), and creates a new service-schedule change to track the +// service's next scheduled occurrence. +// +// The caller must hold the state lock; this is guaranteed by the state +// package for change-status-changed handlers. +func (m *ServiceManager) scheduleChangeReady(chg *state.Change, old, new state.Status) { + if chg.Kind() != serviceScheduleKind || old.Ready() || !new.Ready() { + return + } + + tasks := chg.Tasks() + if len(tasks) == 0 { + return + } + // The first task is always the service-schedule task. + task := tasks[0] + if task.Status() != state.DoneStatus { + // The change became ready for some other reason (for example, it was + // retired via HoldStatus because the service or its schedule was + // removed from the plan): don't schedule a follow-up. + return + } + + var details scheduleDetails + err := task.Get(scheduleDetailsAttr, &details) + if err != nil { + logger.Noticef("Cannot get %s change %s schedule details: %v", chg.Kind(), chg.ID(), err) + return + } + if details.Next.IsZero() { + // No further occurrence (schedule string yielded nothing within the + // lookahead window, or failed to parse). + return + } + + serviceScheduleChange(m.state, details.ServiceName, details.Schedule, details.Next) + m.state.EnsureBefore(0) +} diff --git a/internals/overlord/servstate/schedule_test.go b/internals/overlord/servstate/schedule_test.go index e4b9e2e4e..d5106f0d5 100644 --- a/internals/overlord/servstate/schedule_test.go +++ b/internals/overlord/servstate/schedule_test.go @@ -15,6 +15,7 @@ package servstate_test import ( + "fmt" "strings" "time" @@ -41,48 +42,57 @@ services: schedule: 9:00-11:00 ` -// scheduleChange returns the (single, expected) service-schedule change, or -// nil if none exists. +// scheduleChange returns the pending (not yet ready) service-schedule +// change, or nil if none exists. There should be at most one such change per +// service at any given time. func (s *S) scheduleChange(c *C) *state.Change { s.st.Lock() defer s.st.Unlock() for _, chg := range s.st.Changes() { - if chg.Kind() == servstate.ServiceScheduleKind { + if chg.Kind() == servstate.ServiceScheduleKind && !chg.IsReady() { return chg } } return nil } -func (s *S) scheduleDetails(c *C, chg *state.Change) scheduleDetails { +// scheduleTask returns the first task of chg, which is always the +// service-schedule task (a "start" task set may follow it once the schedule +// has fired). +func (s *S) scheduleTask(c *C, chg *state.Change) *state.Task { s.st.Lock() defer s.st.Unlock() tasks := chg.Tasks() - c.Assert(tasks, HasLen, 1) + c.Assert(len(tasks) >= 1, Equals, true) + return tasks[0] +} + +func (s *S) scheduleDetails(c *C, chg *state.Change) scheduleDetails { + task := s.scheduleTask(c, chg) + s.st.Lock() + defer s.st.Unlock() var details scheduleDetails - err := tasks[0].Get(servstate.ScheduleDetailsAttr, &details) + err := task.Get(servstate.ScheduleDetailsAttr, &details) c.Assert(err, IsNil) return details } func (s *S) setScheduleNext(c *C, chg *state.Change, next time.Time) { + task := s.scheduleTask(c, chg) s.st.Lock() defer s.st.Unlock() - tasks := chg.Tasks() - c.Assert(tasks, HasLen, 1) var details scheduleDetails - err := tasks[0].Get(servstate.ScheduleDetailsAttr, &details) + err := task.Get(servstate.ScheduleDetailsAttr, &details) c.Assert(err, IsNil) details.Next = next - tasks[0].Set(servstate.ScheduleDetailsAttr, &details) + task.Set(servstate.ScheduleDetailsAttr, &details) } func (s *S) scheduleTaskLog(c *C, chg *state.Change) []string { + task := s.scheduleTask(c, chg) s.st.Lock() defer s.st.Unlock() - tasks := chg.Tasks() - c.Assert(tasks, HasLen, 1) - return tasks[0].Log() + return task.Log() } func (s *S) countChangesOfKind(c *C, kind string) int { @@ -128,6 +138,31 @@ func (s *S) TestNextScheduleTimeInvalid(c *C) { c.Assert(err, NotNil) } +// TestNextScheduleTimeAfterAlwaysAdvances guards against a schedule +// computation getting "stuck": for a schedule with a spread (randomised) +// window that's currently open, re-deriving the next occurrence from a +// previous occurrence can otherwise land back inside the same still-open +// window with a new random offset, so it never actually advances into the +// future. That would cause a scheduled start's "next" time to be treated as +// immediately due again and again, spawning an unbounded chain of +// service-schedule changes. +func (s *S) TestNextScheduleTimeAfterAlwaysAdvances(c *C) { + now := time.Now() + // A schedule with a spread window that's open right now: re-evaluating + // from a previous occurrence can otherwise land back inside the same + // still-open window with a new random offset. + sched := fmt.Sprintf("%02d:%02d-%02d:%02d/2", now.Hour(), now.Minute(), (now.Hour()+1)%24, now.Minute()) + + last := now.Add(-24 * time.Hour) + for i := 0; i < 50; i++ { + next, err := servstate.NextScheduleTimeAfter(sched, last) + c.Assert(err, IsNil) + c.Assert(next.After(time.Now()), Equals, true, + Commentf("iteration %d: next=%v is not after now", i, next)) + last = next + } +} + // -- PlanChanged behaviour -- func (s *S) TestScheduleCreatedOnPlanChanged(c *C) { @@ -252,9 +287,11 @@ func (s *S) TestEnsureStartsServiceOnSchedule(c *C) { err := s.manager.Ensure() c.Assert(err, IsNil) - startChg := s.findChangeOfKind(c, "start") - c.Assert(startChg, NotNil) - waitChangeReady(c, s.runner, startChg, "service to start on schedule") + // No independent "start" change should have been created; the start + // task is added to the same schedule change. + c.Check(s.countChangesOfKind(c, "start"), Equals, 0) + + waitChangeReady(c, s.runner, chg, "scheduled service start to complete") s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive @@ -262,6 +299,17 @@ func (s *S) TestEnsureStartsServiceOnSchedule(c *C) { logs := s.scheduleTaskLog(c, chg) c.Check(logContains(logs, "Started service"), Equals, true) + + // A new pending change should have been created to track the next + // scheduled occurrence, distinct from the now-finished one, and there + // should only be one pending service-schedule change. + newChg := s.scheduleChange(c) + c.Assert(newChg, NotNil) + c.Check(newChg.ID() != chg.ID(), Equals, true) + s.st.Lock() + ready := chg.IsReady() + s.st.Unlock() + c.Check(ready, Equals, true) } func (s *S) TestEnsureLogsMissedScheduleButStillRuns(c *C) { @@ -280,9 +328,9 @@ func (s *S) TestEnsureLogsMissedScheduleButStillRuns(c *C) { err := s.manager.Ensure() c.Assert(err, IsNil) - startChg := s.findChangeOfKind(c, "start") - c.Assert(startChg, NotNil) - waitChangeReady(c, s.runner, startChg, "service to start on schedule") + c.Check(s.countChangesOfKind(c, "start"), Equals, 0) + + waitChangeReady(c, s.runner, chg, "scheduled service start to complete") s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive @@ -343,16 +391,3 @@ func (s *S) TestEnsureSkipsFarMissedSchedule(c *C) { logs := s.scheduleTaskLog(c, chg) c.Check(logContains(logs, "Skipped scheduled start"), Equals, true) } - -// findChangeOfKind returns the first change of the given kind not equal to -// any service-schedule change, or nil if none is found. -func (s *S) findChangeOfKind(c *C, kind string) *state.Change { - s.st.Lock() - defer s.st.Unlock() - for _, chg := range s.st.Changes() { - if chg.Kind() == kind { - return chg - } - } - return nil -} From f275b0d650c8abe1959632528360f4672b4fb2c4 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Thu, 6 Aug 2026 10:19:11 +1000 Subject: [PATCH 5/9] feat: schedule start via deferred task --- internals/overlord/servstate/manager.go | 9 +- internals/overlord/servstate/schedule.go | 218 ++++++------------ internals/overlord/servstate/schedule_test.go | 53 +++-- 3 files changed, 113 insertions(+), 167 deletions(-) diff --git a/internals/overlord/servstate/manager.go b/internals/overlord/servstate/manager.go index d5d1709f9..4c6b317e0 100644 --- a/internals/overlord/servstate/manager.go +++ b/internals/overlord/servstate/manager.go @@ -59,11 +59,8 @@ func NewManager(s *state.State, runner *state.TaskRunner, serviceOutput io.Write runner.AddHandler("start", manager.doStart, nil) runner.AddHandler("stop", manager.doStop, nil) - // doServiceSchedule doesn't actually do anything but park itself: it - // exists so that service-schedule tasks (which are driven by - // ServiceManager.Ensure, not a task runner handler) aren't picked up by - // the task runner's generic handling for tasks with no registered - // handler, which would otherwise mark them Done immediately. + // doServiceSchedule decides whether to start a service once its scheduled + // time arrives. runner.AddHandler(serviceScheduleKind, manager.doServiceSchedule, nil) // Schedule changes persist for as long as a service has a schedule @@ -109,7 +106,7 @@ func (m *ServiceManager) getPlan() *plan.Plan { // Ensure implements StateManager.Ensure. func (m *ServiceManager) Ensure() error { - return m.ensureSchedules() + return nil } type ServiceInfo struct { diff --git a/internals/overlord/servstate/schedule.go b/internals/overlord/servstate/schedule.go index e07afdcf6..1b9916a83 100644 --- a/internals/overlord/servstate/schedule.go +++ b/internals/overlord/servstate/schedule.go @@ -16,7 +16,6 @@ package servstate import ( "fmt" - "math" "sort" "time" @@ -56,10 +55,6 @@ const ( // scheduleMissThreshold is how overdue a scheduled start has to be before // we call it out explicitly as "missed" in the task log. scheduleMissThreshold = 5 * time.Second - - // scheduleTaskPollInterval is how often doServiceSchedule checks whether - // its task has been finished externally. - scheduleTaskPollInterval = time.Second ) // scheduleDetails is persisted on the service-schedule task, and records the @@ -130,50 +125,97 @@ func scheduleShouldRunNow(now, missed, following time.Time) bool { // scheduled start time, and returns the change ID. // The caller must hold the state lock. func serviceScheduleChange(st *state.State, name, scheduleStr string, next time.Time) string { - summary := fmt.Sprintf("Wait for scheduled start of service %q", name) - task := st.NewTask(serviceScheduleKind, summary) + taskSummary := fmt.Sprintf("Wait for scheduled start of service %q", name) + task := st.NewTask(serviceScheduleKind, taskSummary) task.Set(scheduleDetailsAttr, &scheduleDetails{ ServiceName: name, Schedule: scheduleStr, Next: next, }) - // doServiceSchedule (registered as this kind's TaskRunner handler) parks - // until the status below is changed by ensureSchedules/scheduleChanged; - // it's what drives this task, not the handler itself. Mark it Doing so - // it's clear from "pebble changes"/"pebble tasks" that it's ongoing. task.SetStatus(state.DoingStatus) + task.At(next) - change := st.NewChange(serviceScheduleKind, summary) + changeSummary := fmt.Sprintf("Scheduled start of service %q", name) + change := st.NewChange(serviceScheduleKind, changeSummary) change.Set(scheduleNoPruneAttr, true) change.AddTask(task) return change.ID() } // doServiceSchedule is the TaskRunner handler for serviceScheduleKind tasks. +// The task runner invokes it once the task's scheduled time arrives (set via +// task.At, see serviceScheduleChange and scheduleChanged). // -// The task's outcome is driven entirely by ServiceManager.Ensure (see -// ensureSchedules) and scheduleChanged, which change its status away from -// DoingStatus once the scheduled time arrives (or the schedule is retired). -// This handler's only job is to occupy the task runner's "do" slot for this -// task kind, so that it isn't picked up by the runner's generic handling for -// tasks with no registered handler (which would otherwise mark it Done -// immediately, defeating the entire point of it representing "still -// waiting"). It just blocks, polling for that external status change, until -// it happens or the runner asks it to stop. -func (m *ServiceManager) doServiceSchedule(task *state.Task, tomb *tomb.Tomb) error { - for { - select { - case <-tomb.Dying(): - return tomb.Err() - case <-time.After(scheduleTaskPollInterval): +// It decides whether to start the service now (adding a "start" task set to +// the same change, after which the task runner marks this task Done once we +// return) or to keep waiting for the next occurrence (rescheduling ourselves +// and return Retry, so the task runner invokes us again then). +func (m *ServiceManager) doServiceSchedule(task *state.Task, _ *tomb.Tomb) error { + m.state.Lock() + defer m.state.Unlock() + + var details scheduleDetails + err := task.Get(scheduleDetailsAttr, &details) + if err != nil { + return fmt.Errorf("cannot get service-schedule-details from task: %w", err) + } + + now := timeNow() + if now.Before(details.Next) { + // Woken up early, go back to sleep. + task.At(details.Next) + return &state.Retry{} + } + + missed := details.Next + following, err := nextScheduleTimeAfter(details.Schedule, missed) + if err != nil { + logger.Noticef("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) + task.Errorf("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) + } + + followingMsg := "not scheduled again" + if !following.IsZero() { + followingMsg = fmt.Sprintf("next scheduled start at %s", following.Format(time.RFC3339)) + } + + fired := false + if !scheduleShouldRunNow(now, missed, following) { + task.Logf("Skipped scheduled start at %s for service %q (missed by too long); %s.", + missed.Format(time.RFC3339), details.ServiceName, followingMsg) + } else { + if now.Sub(missed) > scheduleMissThreshold { + task.Logf("Missed scheduled start at %s for service %q; starting it now.", + missed.Format(time.RFC3339), details.ServiceName) } - m.state.Lock() - stillWaiting := task.Status() == state.DoingStatus - m.state.Unlock() - if !stillWaiting { - return nil + if m.serviceIsActive(details.ServiceName) { + task.Logf("Service %q is already running; %s.", details.ServiceName, followingMsg) + } else if lanes, err := m.StartOrder([]string{details.ServiceName}); err != nil { + task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) + } else if taskSet, err := Start(m.state, lanes); err != nil { + task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) + } else { + task.Change().AddAll(taskSet) + task.Logf("Started service %q on schedule; %s.", details.ServiceName, followingMsg) + fired = true } } + + // Record the following occurrence, whether we fired or not: if we fired, + // scheduleChangeReady reads this back once the change becomes ready, to + // create the following change. + details.Next = following + task.Set(scheduleDetailsAttr, &details) + + if fired { + m.state.EnsureBefore(0) + return nil + } + + // Reschedule ourselves for the next occurrence and ask the task runner to + // start this task again then. + task.At(following) + return &state.Retry{} } // scheduleChanged is called from PlanChanged to create, update, or retire @@ -237,6 +279,8 @@ func (m *ServiceManager) scheduleChanged(newPlan *plan.Plan) { Schedule: config.Schedule, Next: next, }) + // If the task hasn't fired yet, reschedule it. + task.At(next) shouldEnsure = true } @@ -287,116 +331,6 @@ func (m *ServiceManager) serviceIsActive(name string) bool { } } -// ensureSchedules starts any services whose scheduled timer has elapsed, and to -// reschedule the next start. -func (m *ServiceManager) ensureSchedules() error { - m.state.Lock() - defer m.state.Unlock() - - var ( - now time.Time = timeNow() - before time.Duration = math.MaxInt64 - ) - - for _, change := range m.state.Changes() { - if change.Kind() != serviceScheduleKind || change.IsReady() { - continue - } - task := change.Tasks()[0] - if !task.Has(scheduleDetailsAttr) { - continue - } - if task.Status() != state.DoingStatus { - // Already fired: the start task(s) added alongside it are still - // running. Nothing more to do here until the whole change becomes - // ready (see scheduleChangeReady). - continue - } - - var details scheduleDetails - err := task.Get(scheduleDetailsAttr, &details) - if err != nil { - return fmt.Errorf("cannot get service-schedule-details from task: %w", err) - } - - if details.Next.IsZero() { - continue - } - - if now.Before(details.Next) { - before = min(before, details.Next.Sub(now)) - continue - } - - missed := details.Next - following, err := nextScheduleTimeAfter(details.Schedule, missed) - if err != nil { - logger.Noticef("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) - task.Errorf("Cannot compute next scheduled start for service %q: %v", details.ServiceName, err) - } - - followingMsg := "not scheduled again" - if !following.IsZero() { - followingMsg = fmt.Sprintf("next scheduled start at %s", following.Format(time.RFC3339)) - } - - fired := false - if scheduleShouldRunNow(now, missed, following) { - if now.Sub(missed) > scheduleMissThreshold { - task.Logf("Missed scheduled start at %s for service %q; starting it now.", - missed.Format(time.RFC3339), details.ServiceName) - } - if m.serviceIsActive(details.ServiceName) { - // Stay on this change/task: just log and move on to the next - // occurrence. - task.Logf("Service %q is already running; %s.", details.ServiceName, followingMsg) - } else { - lanes, err := m.StartOrder([]string{details.ServiceName}) - if err != nil { - task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) - } else { - taskSet, err := Start(m.state, lanes) - if err != nil { - task.Errorf("Cannot start service %q on schedule: %v", details.ServiceName, err) - } else { - // Add the start task(s) to this same change, rather - // than creating an independent one. - change.AddAll(taskSet) - task.Logf("Started service %q on schedule; %s.", details.ServiceName, followingMsg) - fired = true - } - } - } - } else { - task.Logf("Skipped scheduled start at %s for service %q (missed by too long); %s.", - missed.Format(time.RFC3339), details.ServiceName, followingMsg) - } - - // Record the following occurrence, whether we fired or not: if we - // fired, scheduleChangeReady reads this back once the change becomes - // ready, to create the follow-up change. - details.Next = following - task.Set(scheduleDetailsAttr, &details) - - if fired { - // Mark the service-schedule task done now that the start task(s) - // have been added to the change (must happen after AddAll/Set - // above, so the change's ready channel doesn't close - // prematurely). The change becomes ready once the start task(s) - // finish, at which point scheduleChangeReady takes over. - task.SetStatus(state.DoneStatus) - } else { - before = min(before, following.Sub(now)) - } - } - - if before < math.MaxInt64 { - m.state.EnsureBefore(before) - } - - return nil -} - // scheduleChangeReady is called whenever a change's status changes; it looks // for service-schedule changes that have just become ready (i.e. their // service-schedule task fired and any start task(s) added alongside it have diff --git a/internals/overlord/servstate/schedule_test.go b/internals/overlord/servstate/schedule_test.go index d5106f0d5..3834ee404 100644 --- a/internals/overlord/servstate/schedule_test.go +++ b/internals/overlord/servstate/schedule_test.go @@ -86,6 +86,28 @@ func (s *S) setScheduleNext(c *C, chg *state.Change, next time.Time) { c.Assert(err, IsNil) details.Next = next task.Set(servstate.ScheduleDetailsAttr, &details) + task.At(next) +} + +// waitTaskLogContains runs the task runner until the task's log contains the +// sub-string, or fails the test after a timeout. +func waitTaskLogContains(c *C, runner *state.TaskRunner, st *state.State, task *state.Task, substr string) { + timeout := time.After(10 * time.Second) + for { + runner.Ensure() + st.Lock() + found := logContains(task.Log(), substr) + st.Unlock() + if found { + return + } + select { + case <-timeout: + c.Fatalf("timeout waiting for task log to contain %q", substr) + default: + time.Sleep(time.Millisecond) + } + } } func (s *S) scheduleTaskLog(c *C, chg *state.Change) []string { @@ -284,15 +306,12 @@ func (s *S) TestEnsureStartsServiceOnSchedule(c *C) { // Force the schedule to be due right now. s.setScheduleNext(c, chg, time.Now().Add(-time.Second)) - err := s.manager.Ensure() - c.Assert(err, IsNil) + waitChangeReady(c, s.runner, chg, "scheduled service start to complete") // No independent "start" change should have been created; the start // task is added to the same schedule change. c.Check(s.countChangesOfKind(c, "start"), Equals, 0) - waitChangeReady(c, s.runner, chg, "scheduled service start to complete") - s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -325,13 +344,10 @@ func (s *S) TestEnsureLogsMissedScheduleButStillRuns(c *C) { // should run anyway. s.setScheduleNext(c, chg, time.Now().Add(-30*time.Second)) - err := s.manager.Ensure() - c.Assert(err, IsNil) + waitChangeReady(c, s.runner, chg, "scheduled service start to complete") c.Check(s.countChangesOfKind(c, "start"), Equals, 0) - waitChangeReady(c, s.runner, chg, "scheduled service start to complete") - s.waitUntilService(c, "sched1", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -355,14 +371,16 @@ func (s *S) TestEnsureSkipsStartWhenAlreadyRunning(c *C) { c.Assert(chg, NotNil) s.setScheduleNext(c, chg, time.Now().Add(-time.Second)) - err := s.manager.Ensure() - c.Assert(err, IsNil) + task := s.scheduleTask(c, chg) + waitTaskLogContains(c, s.runner, s.st, task, "already running") - // No "start" change should have been created by the schedule. + // No "start" change should have been created by the schedule, and the + // schedule change should still be pending (not finished). c.Check(s.countChangesOfKind(c, "start"), Equals, 0) - - logs := s.scheduleTaskLog(c, chg) - c.Check(logContains(logs, "already running"), Equals, true) + s.st.Lock() + ready := chg.IsReady() + s.st.Unlock() + c.Check(ready, Equals, false) } func (s *S) TestEnsureSkipsFarMissedSchedule(c *C) { @@ -376,8 +394,8 @@ func (s *S) TestEnsureSkipsFarMissedSchedule(c *C) { longAgo := time.Now().Add(-240 * time.Hour) // 10 days ago s.setScheduleNext(c, chg, longAgo) - err := s.manager.Ensure() - c.Assert(err, IsNil) + task := s.scheduleTask(c, chg) + waitTaskLogContains(c, s.runner, s.st, task, "Skipped scheduled start") // No service should have been started because of this. c.Check(s.countChangesOfKind(c, "start"), Equals, 0) @@ -387,7 +405,4 @@ func (s *S) TestEnsureSkipsFarMissedSchedule(c *C) { // schedule fires daily, so the new Next should be close to now, not // close to the 10-day-old missed time). c.Check(details.Next.After(longAgo.Add(48*time.Hour)), Equals, true) - - logs := s.scheduleTaskLog(c, chg) - c.Check(logContains(logs, "Skipped scheduled start"), Equals, true) } From bb788cd23ec34c84fc82f0cbdceb56683ad4fe2d Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Thu, 6 Aug 2026 11:54:32 +1000 Subject: [PATCH 6/9] feat: api access to service scheduled start --- client/services.go | 1 + internals/daemon/api_services.go | 4 ++++ internals/overlord/servstate/manager.go | 7 +++++++ internals/overlord/servstate/schedule.go | 25 ++++++++++++++++++++++++ 4 files changed, 37 insertions(+) diff --git a/client/services.go b/client/services.go index c19c58db1..7f134447e 100644 --- a/client/services.go +++ b/client/services.go @@ -100,6 +100,7 @@ type ServiceInfo struct { Startup ServiceStartup `json:"startup" yaml:"startup"` Current ServiceStatus `json:"current" yaml:"current"` CurrentSince time.Time `json:"current-since,omitzero" yaml:"current-since,omitempty"` + Scheduled time.Time `json:"scheduled,omitzero" yaml:"scheduled,omitempty"` } // ServiceStartup defines the different startup modes for a service. diff --git a/internals/daemon/api_services.go b/internals/daemon/api_services.go index a0933b754..31bf087be 100644 --- a/internals/daemon/api_services.go +++ b/internals/daemon/api_services.go @@ -33,6 +33,7 @@ type serviceInfo struct { Startup string `json:"startup"` Current string `json:"current"` CurrentSince *time.Time `json:"current-since,omitempty"` // pointer as omitempty doesn't work with time.Time directly + Scheduled *time.Time `json:"scheduled,omitempty"` // pointer as omitempty doesn't work with time.Time directly } func v1GetServices(c *Command, r *http.Request, _ *UserState) Response { @@ -54,6 +55,9 @@ func v1GetServices(c *Command, r *http.Request, _ *UserState) Response { if !svc.CurrentSince.IsZero() { info.CurrentSince = &svc.CurrentSince } + if !svc.Scheduled.IsZero() { + info.Scheduled = &svc.Scheduled + } infos = append(infos, info) } return SyncResponse(infos) diff --git a/internals/overlord/servstate/manager.go b/internals/overlord/servstate/manager.go index 4c6b317e0..1a7240e4b 100644 --- a/internals/overlord/servstate/manager.go +++ b/internals/overlord/servstate/manager.go @@ -114,6 +114,7 @@ type ServiceInfo struct { Startup ServiceStartup Current ServiceStatus CurrentSince time.Time + Scheduled time.Time } type ServiceStartup string @@ -136,6 +137,11 @@ const ( // by service name. Filter by the specified service names if provided. func (m *ServiceManager) Services(names []string) ([]*ServiceInfo, error) { currentPlan := m.getPlan() + + m.state.Lock() + scheduled := m.scheduledStartTimes() + m.state.Unlock() + m.servicesLock.Lock() defer m.servicesLock.Unlock() @@ -162,6 +168,7 @@ func (m *ServiceManager) Services(names []string) ([]*ServiceInfo, error) { info.Current = stateToStatus(s.state) info.CurrentSince = s.currentSince } + info.Scheduled = scheduled[name] services = append(services, info) } sort.Slice(services, func(i, j int) bool { diff --git a/internals/overlord/servstate/schedule.go b/internals/overlord/servstate/schedule.go index 1b9916a83..ac9bac0cb 100644 --- a/internals/overlord/servstate/schedule.go +++ b/internals/overlord/servstate/schedule.go @@ -331,6 +331,31 @@ func (m *ServiceManager) serviceIsActive(name string) bool { } } +// scheduledStartTimes returns the next scheduled start time for every +// service that currently has a service-schedule task in the Doing status, +// i.e. is waiting for its next scheduled start. +// +// The caller must hold the state lock. +func (m *ServiceManager) scheduledStartTimes() map[string]time.Time { + scheduled := make(map[string]time.Time) + for _, change := range m.state.Changes() { + if change.Kind() != serviceScheduleKind { + continue + } + for _, task := range change.Tasks() { + if task.Kind() != serviceScheduleKind || task.Status() != state.DoingStatus { + continue + } + var details scheduleDetails + if err := task.Get(scheduleDetailsAttr, &details); err != nil { + continue + } + scheduled[details.ServiceName] = details.Next + } + } + return scheduled +} + // scheduleChangeReady is called whenever a change's status changes; it looks // for service-schedule changes that have just become ready (i.e. their // service-schedule task fired and any start task(s) added alongside it have From e69bf833029b5a209b67280482cf8893a72e016e Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Thu, 6 Aug 2026 12:30:39 +1000 Subject: [PATCH 7/9] feat: scheduled start time in services cmd text output --- internals/cli/cmd_enter_test.go | 6 +++--- internals/cli/cmd_services.go | 8 ++++++-- internals/cli/cmd_services_test.go | 25 +++++++++++++------------ 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/internals/cli/cmd_enter_test.go b/internals/cli/cmd_enter_test.go index 60cce36af..1332bab9c 100644 --- a/internals/cli/cmd_enter_test.go +++ b/internals/cli/cmd_enter_test.go @@ -116,9 +116,9 @@ func (s *PebbleSuite) TestEnterUnknownCommand(c *C) { func (s *PebbleSuite) TestEnterServicesStatus(c *C) { expectedOutput := dumbDedent(` - Service Startup Current Since - write-message-01 enabled inactive - - write-message-02 disabled inactive - + Service Startup Scheduled Current Since + write-message-01 enabled - inactive - + write-message-02 disabled - inactive - `) writeMessageServices(s) diff --git a/internals/cli/cmd_services.go b/internals/cli/cmd_services.go index 18b10c4b8..82cff440f 100644 --- a/internals/cli/cmd_services.go +++ b/internals/cli/cmd_services.go @@ -86,14 +86,18 @@ func (cmd *cmdServices) writeText(services []*client.ServiceInfo) error { w := tabWriter() defer w.Flush() - fmt.Fprintln(w, "Service\tStartup\tCurrent\tSince") + fmt.Fprintln(w, "Service\tStartup\tScheduled\tCurrent\tSince") for _, svc := range services { + scheduled := "-" + if !svc.Scheduled.IsZero() { + scheduled = cmd.fmtTime(svc.Scheduled) + } since := "-" if !svc.CurrentSince.IsZero() { since = cmd.fmtTime(svc.CurrentSince) } - fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", svc.Name, svc.Startup, svc.Current, since) + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", svc.Name, svc.Startup, scheduled, svc.Current, since) } return nil } diff --git a/internals/cli/cmd_services_test.go b/internals/cli/cmd_services_test.go index 587780384..a3e488e72 100644 --- a/internals/cli/cmd_services_test.go +++ b/internals/cli/cmd_services_test.go @@ -34,7 +34,7 @@ func (s *PebbleSuite) TestServices(c *check.C) { "status-code": 200, "result": [ {"name": "svc1", "current": "inactive", "startup": "enabled", "current-since": "2022-04-28T17:05:23+12:00"}, - {"name": "svc2", "current": "inactive", "startup": "enabled"}, + {"name": "svc2", "current": "inactive", "startup": "enabled", "scheduled": "2030-04-28T17:05:23+12:00"}, {"name": "svc3", "current": "backoff", "startup": "enabled"} ] }`) @@ -43,10 +43,10 @@ func (s *PebbleSuite) TestServices(c *check.C) { c.Assert(err, check.IsNil) c.Assert(rest, check.HasLen, 0) c.Check(s.Stdout(), check.Equals, ` -Service Startup Current Since -svc1 enabled inactive 2022-04-28 -svc2 enabled inactive - -svc3 enabled backoff - +Service Startup Scheduled Current Since +svc1 enabled - inactive 2022-04-28 +svc2 enabled 2030-04-28 inactive - +svc3 enabled - backoff - `[1:]) c.Check(s.Stderr(), check.Equals, "") } @@ -133,7 +133,7 @@ func (s *PebbleSuite) TestServicesNames(c *check.C) { "status-code": 200, "result": [ {"name": "bar", "current": "active", "startup": "disabled", "current-since": "2022-04-28T17:05:23+12:00"}, - {"name": "foo", "current": "inactive", "startup": "enabled"} + {"name": "foo", "current": "inactive", "startup": "enabled", "scheduled": "2030-04-28T17:05:23+12:00"} ] }`) }) @@ -141,9 +141,9 @@ func (s *PebbleSuite) TestServicesNames(c *check.C) { c.Assert(err, check.IsNil) c.Assert(rest, check.HasLen, 0) c.Check(s.Stdout(), check.Equals, ` -Service Startup Current Since -bar disabled active 2022-04-28T17:05:23+12:00 -foo enabled inactive - +Service Startup Scheduled Current Since +bar disabled - active 2022-04-28T17:05:23+12:00 +foo enabled 2030-04-28T17:05:23+12:00 inactive - `[1:]) c.Check(s.Stderr(), check.Equals, "") } @@ -158,7 +158,7 @@ func (s *PebbleSuite) TestServicesJSON(c *check.C) { "status-code": 200, "result": [ {"name": "svc1", "current": "inactive", "startup": "enabled", "current-since": "2022-04-28T17:05:23+12:00"}, - {"name": "svc2", "current": "inactive", "startup": "enabled"}, + {"name": "svc2", "current": "inactive", "startup": "enabled", "scheduled": "2030-04-28T17:05:23+12:00"}, {"name": "svc3", "current": "backoff", "startup": "enabled"} ] }`) @@ -166,7 +166,7 @@ func (s *PebbleSuite) TestServicesJSON(c *check.C) { rest, err := cli.ParserForTest().ParseArgs([]string{"services", "--format", "json"}) c.Assert(err, check.IsNil) c.Assert(rest, check.HasLen, 0) - c.Check(s.Stdout(), check.Equals, `{"services":{"svc1":{"name":"svc1","startup":"enabled","current":"inactive","current-since":"2022-04-28T17:05:23+12:00"},"svc2":{"name":"svc2","startup":"enabled","current":"inactive"},"svc3":{"name":"svc3","startup":"enabled","current":"backoff"}}}`+"\n") + c.Check(s.Stdout(), check.Equals, `{"services":{"svc1":{"name":"svc1","startup":"enabled","current":"inactive","current-since":"2022-04-28T17:05:23+12:00"},"svc2":{"name":"svc2","startup":"enabled","current":"inactive","scheduled":"2030-04-28T17:05:23+12:00"},"svc3":{"name":"svc3","startup":"enabled","current":"backoff"}}}`+"\n") c.Check(s.Stderr(), check.Equals, "") } @@ -180,7 +180,7 @@ func (s *PebbleSuite) TestServicesYAML(c *check.C) { "status-code": 200, "result": [ {"name": "svc1", "current": "inactive", "startup": "enabled", "current-since": "2022-04-28T17:05:23+12:00"}, - {"name": "svc2", "current": "inactive", "startup": "enabled"}, + {"name": "svc2", "current": "inactive", "startup": "enabled", "scheduled": "2030-04-28T17:05:23+12:00"}, {"name": "svc3", "current": "backoff", "startup": "enabled"} ] }`) @@ -199,6 +199,7 @@ services: name: svc2 startup: enabled current: inactive + scheduled: 2030-04-28T17:05:23+12:00 svc3: name: svc3 startup: enabled From 62b0d00f2421d4279ce592811d4281e3a983828a Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Thu, 6 Aug 2026 14:02:22 +1000 Subject: [PATCH 8/9] test: add scheduled to integration test --- tests/services_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/services_test.go b/tests/services_test.go index efc5921c6..129be1d2a 100644 --- a/tests/services_test.go +++ b/tests/services_test.go @@ -72,8 +72,8 @@ waitExit: output := runPebbleCommand(t, pebbleDir, "services") expected := ` -Service Startup Current Since -svc1 disabled inactive - +Service Startup Scheduled Current Since +svc1 disabled - inactive - `[1:] if output != expected { t.Fatalf("unexpected services output\nWant:\n%s\nGot:\n%s", expected, output) From 26ccd4defb64c101b9c24391b0bafa5b8e8d43df Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Thu, 6 Aug 2026 19:34:46 +1000 Subject: [PATCH 9/9] docs: service schedule and timer string format --- docs/how-to/index.md | 1 + docs/how-to/schedule-services.md | 60 ++++++++++ docs/reference/layer-specification.md | 16 +++ docs/reference/timer-string-format.md | 153 ++++++++++++++++++++++++++ 4 files changed, 230 insertions(+) create mode 100644 docs/how-to/schedule-services.md create mode 100644 docs/reference/timer-string-format.md diff --git a/docs/how-to/index.md b/docs/how-to/index.md index c680e8cfa..03ca28276 100644 --- a/docs/how-to/index.md +++ b/docs/how-to/index.md @@ -23,6 +23,7 @@ As your needs grow, you may want to use advanced Pebble features to run services :maxdepth: 1 Run services reliably +Run services on a schedule Manage service dependencies Use layers ``` diff --git a/docs/how-to/schedule-services.md b/docs/how-to/schedule-services.md new file mode 100644 index 000000000..8a2d5829b --- /dev/null +++ b/docs/how-to/schedule-services.md @@ -0,0 +1,60 @@ +# How to run services on a schedule + +It can be useful to start a service periodically, either to ensure that a service +is no longer stopped or to perform some action. + +For example, if you have a service that handles software updates, an administrator +might manually stop this service while they perform maintainence on a system. If +the administrator forgets to start the service again, important software updates +might be missed. By setting a schedule to start the service, we can ensure it +is running again. + +Another example that is often seen with systems using TLS certificates, is that +the certificates are issued with a short validity period requiring renewal before +they expire. Using a tool like Certbot, we can fetch new certificates, but for +this to help, we need Certbot to run before the certificate expires. + +(run-a-service-on-a-schedule)= +## Run a service on a schedule + +To ensure a service is started, a schedule can be used to start the service. +This is useful to either delay the start of a service or to ensure the service +is started if it was manually stopped. + +This is an example configuration for a scheduled start service that ensures the +service is running once per day: + +```yaml +services: + svc1: + override: replace + command: daily.sh + startup: disabled + schedule: 0:00 # Once per day at 0:00. +``` + +(run-a-service-as-a-cron-job)= +## Run a service as a cron job + +To run a service as a scheduled repeating one-shot service, it is required to +set the `on-success` field to ignore when the service process exits. +Since the schedule will eventually start the service again, it may be useful to +set the `on-failure` field to ignore the failure. + +This is an example configuration for a cron job service: + +```yaml +services: + svc1: + override: replace + command: ping -c 3 localhost + startup: disabled + schedule: 0:00-24:00/1440 # Once per minute. + on-success: ignore + on-failure: ignore +``` + +## See more + +- [Layer specification](../reference/layer-specification) +- [Timer string format](../reference/timer-string-format) diff --git a/docs/reference/layer-specification.md b/docs/reference/layer-specification.md index f0cec4bdf..e5f81f3c5 100644 --- a/docs/reference/layer-specification.md +++ b/docs/reference/layer-specification.md @@ -41,6 +41,22 @@ services: # Pebble starts or performs a 'replan' operation. Default is "disabled". startup: enabled | disabled + # (Optional) A schedule of when to start the service automatically. + # + # By default, a service has no schedule by which it is started. + # + # See https://ubuntu.com/docs/pebble/reference/timer-string-format/ for + # the syntax of the timer string format, including more examples of its + # use. + # + # Examples: + # - `00:00-24:00/24` - Every hour on the hour + # - `00:00-24:00/48` - Every 30 minutes + # - `00:00-24:00/96` - Every 15 minutes + # - `12:00-13:00/12` - Every 5 minutes from 12:00 to 13:10 + # - `23:00` - Every day at 23:00 + schedule: + # (Optional) A list of other services in the plan that this service # should start after. after: diff --git a/docs/reference/timer-string-format.md b/docs/reference/timer-string-format.md new file mode 100644 index 000000000..635c3b3c3 --- /dev/null +++ b/docs/reference/timer-string-format.md @@ -0,0 +1,153 @@ +# Timer string format + +Timer strings are used for configuring service `schedule`s. + +See this [discourse thread](https://forum.snapcraft.io/t/refresh-scheduling-on-specific-days-of-the-month/1239/6) for details on how the syntax was conceived and evolved over time. + +## Syntax + +``` +eventlist = eventset *( ",," eventset ) +eventset = wdaylist / timelist / wdaylist "," timelist + +wdaylist = wdayset *( "," wdayset ) +wdayset = wday / wdaynumber / wdayspan +wday = ( "mon" / "tue" / "wed" / "thu" / "fri" / "sat" / "sun" ) +wdaynumber = ( "sun" / "mon" / "tue" / "wed" / "thu" / "fri" / "sat" ) DIGIT +wdayspan = wday "-" wday / wdaynumber "-" wday / wday "-" wdaynumber +wspec = ( "1" / "2" / "3" / "4" / "5" ) + +timelist = timeset *( "," timeset ) +timeset = time / timespan +time = 2DIGIT ":" 2DIGIT +timespan = time ( "-" / "~" ) time [ "/" count ] +count = n*DIGIT +``` +Clock times are always specified in 24H format. + +## Examples + +* `00:00-24:00/24`
+ Every hour on the hour + +* `00:00-24:00/48`
+ Every 30 minutes + +* `00:00-24:00/96`
+ Every 15 minutes + +* `12:00-13:00/12`
+ Every 5 minutes from 12:00 to 13:10 + +* `23:00`
+ Every day at 23:00 + +More specific timer examples: + +* `mon,10:00,,fri,15:00`
+ Mondays at 10:00, Fridays at 15:10 + +* `mon,fri,10:00,15:00`
+ Mondays at 10:00 and 15:00, Fridays at 10:00 and 15:00 + +* `mon-wed,fri,9:00-11:00/2`
+ Monday to Wednesday and on Friday, twice between 9:00 and 11:00 + +* `mon,9:00~11:00,,wed,22:00~23:00`
+ Mondays, some time between 9:00 and 11:00, and on Wednesdays, some time between 22:00 and 23:00 + +* `mon,wed`
+ Monday and on Wednesday, at 0:00 + +* `mon2-wed,23:00-24:00`
+ 2nd Monday of the month through the following Wednesday, between 23:00 and 24:00 + +* `fri5,23:00-01:00`
+ Last Friday of the month, from 23:00 to 1:00 the next day. Even in months with 4 Fridays, this schedule will still trigger on the last Friday. + +## Semantics + +A timer string is composed of one or more event sets, which are combined by using commas (`,,`) as separators. + +Each event set defines the weekdays and the time windows in which events may occur. The next event will be scheduled inside the soonest opportunity that matches both one of the provided weekdays and one of the provided time windows. If no weekdays are provided, the default is every day. If no time windows are provided, the default is an arbitrary time in the day. + +For example, consider the timer: + + mon,fri,10:00,15:00 + +Assuming today is Sunday, the next 5 events are, in order: + + Monday 10:00 + Monday 15:00 + Friday 10:00 + Friday 15:00 + Monday 10:00 + +Consider the following timer: + + mon,10:00,,fri,15:00 + +The next 3 events in this case are: + + Monday 10am + Friday 15pm + Monday 10am + +All of these examples work on a weekly basis, but certain events are better scheduled on a monthly basis. To support that, weekdays may be suffixed by `wspec` entry that defines the week number inside the month. As an example, the following timer defines two events every month, on the first and third Mondays at 15:00: + + mon1,mon3,15:00 + +As a special case, the 5th week is considered the last one to hold the given day, so that specifying an event on the last Friday of the month, for instance, is done simply as: + + fri5 + +In addition to specifying precise weekdays, an interval may be used to define a larger span: + + mon-fri,15:00 + +This represents an event per day at 15:00, Monday through Friday, every week. + +The same interval syntax also works to define time spans, but the meaning is slightly different. For instance, consider this time span: + + mon,14:00-16:00 + +It defines an event every Monday that will take place at the earliest chance between 14:100 and 16:00. + +Weekday spans define an event **every day** within the span. In contrast, time spans define a **single event** inside the defined span. + +That latter aspect may be changed via an explicit divisor, which may be specified as a `count`. For instance, consider this time span: + + 8:00-16:00/2 + +This represents two events every day, one in the morning between 8:00 and 12:00, and another one between 12:00 and 16:00. + +While the following represents an hourly event, every day of the week: + + 0:00-24:00/24 + +All of the time spans defined so far work similarly in the sense that the start of the span defines the earliest chance in which the event may start, and the end of the span defines the latest chance for the event to have started. For various reasons, though, it’s often useful to introduce some level of randomization inside the time span so that events won’t all start at exactly the same time. This may be achieved by replacing the time span dash character (`-`) by a tilde (`~`). Consider the following time span: + + 0:00~24:00/4 + +It represents 4 events that will take place at a random time inside time windows of 6 hours each. + +Week spans that need to start or end during a specific week in the month can be defined by appending the week number to either the start or end of a week span. Consider the following schedules: + + mon1-fri + mon-fri1 + +The first example describes a week span that starts on the first Monday of the month and ends on the following Friday, while the second example defines a week span that starts on the Monday _before_ the first Friday of the month, which is when the span ends. + +Consider the following calendar months of July and August 2019: + +``` + July August +Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa + 1 2 3 4 5 6 1 2 3 + 7 8 9 10 11 12 13 4 5 6 7 8 9 10 +14 15 16 17 18 19 20 11 12 13 14 15 16 17 +21 22 23 24 25 26 27 18 19 20 21 22 23 24 +28 29 30 31 25 26 27 28 29 30 31 +``` + +In the above context, `mon1-fri` corresponds describes a span from 5th of August to the 9th of August, while `mon-fri1` covers 29th of July until 2nd of August.