diff --git a/internals/overlord/checkstate/manager_test.go b/internals/overlord/checkstate/manager_test.go index 2e42d9a5d..a455c6a0f 100644 --- a/internals/overlord/checkstate/manager_test.go +++ b/internals/overlord/checkstate/manager_test.go @@ -250,6 +250,7 @@ func (s *ManagerSuite) TestFailures(c *C) { c.Assert(check.ChangeID, Equals, originalChangeID) // Should have called failure handler and be unhealthy after 3 failures (threshold) + c.Assert(changeData(c, s.overlord.State(), check.ChangeID), DeepEquals, map[string]string{"check-name": "chk1"}) check = waitCheck(c, s.manager, "chk1", func(check *checkstate.CheckInfo) bool { return check.Failures == 3 && check.ChangeID != originalChangeID }) @@ -278,6 +279,7 @@ func (s *ManagerSuite) TestFailures(c *C) { c.Assert(check.Threshold, Equals, 3) c.Assert(notifies.Load(), Equals, int32(1)) c.Assert(lastTaskLog(s.overlord.State(), check.ChangeID), Equals, "") + c.Assert(changeData(c, s.overlord.State(), check.ChangeID), DeepEquals, map[string]string{"check-name": "chk1"}) } func (s *ManagerSuite) TestFailuresBelowThreshold(c *C) { @@ -571,3 +573,14 @@ func lastTaskLog(st *state.State, changeID string) string { } return logs[len(logs)-1] } + +func changeData(c *C, st *state.State, changeID string) map[string]string { + st.Lock() + defer st.Unlock() + + chg := st.Change(changeID) + var data map[string]string + err := chg.Get("notice-data", &data) + c.Assert(err, IsNil) + return data +} diff --git a/internals/overlord/checkstate/request.go b/internals/overlord/checkstate/request.go index 7cc99c4a1..b60c12df3 100644 --- a/internals/overlord/checkstate/request.go +++ b/internals/overlord/checkstate/request.go @@ -37,7 +37,9 @@ func performCheckChange(st *state.State, config *plan.Check) (changeID string) { task := st.NewTask(performCheckKind, summary) task.Set(checkDetailsAttr, &checkDetails{Name: config.Name}) - change := st.NewChange(performCheckKind, task.Summary()) + change := st.NewChangeWithNoticeData(performCheckKind, task.Summary(), map[string]string{ + "check-name": config.Name, + }) change.Set(noPruneAttr, true) change.AddTask(task) @@ -55,7 +57,9 @@ func recoverCheckChange(st *state.State, config *plan.Check, failures int) (chan task := st.NewTask(recoverCheckKind, summary) task.Set(checkDetailsAttr, &checkDetails{Name: config.Name, Failures: failures}) - change := st.NewChange(recoverCheckKind, task.Summary()) + change := st.NewChangeWithNoticeData(recoverCheckKind, task.Summary(), map[string]string{ + "check-name": config.Name, + }) change.Set(noPruneAttr, true) change.AddTask(task) diff --git a/internals/overlord/state/change.go b/internals/overlord/state/change.go index 5fae98f9e..e334d989f 100644 --- a/internals/overlord/state/change.go +++ b/internals/overlord/state/change.go @@ -17,6 +17,7 @@ package state import ( "bytes" "encoding/json" + "errors" "fmt" "sort" "strings" @@ -428,7 +429,16 @@ func (c *Change) addNotice() error { opts := &AddNoticeOptions{ Data: map[string]string{"kind": c.Kind()}, } - _, err := c.state.AddNotice(nil, ChangeUpdateNotice, c.id, opts) + var extraData map[string]string + err := c.Get("notice-data", &extraData) + if err == nil { + for k, v := range extraData { + opts.Data[k] = v + } + } else if !errors.Is(err, ErrNoState) { + return fmt.Errorf("cannot get notice data from change %s: %w", c.ID(), err) + } + _, err = c.state.AddNotice(nil, ChangeUpdateNotice, c.id, opts) return err } diff --git a/internals/overlord/state/change_test.go b/internals/overlord/state/change_test.go index 6e1aee810..55339f3b6 100644 --- a/internals/overlord/state/change_test.go +++ b/internals/overlord/state/change_test.go @@ -51,6 +51,19 @@ func (cs *changeSuite) TestNewChange(c *C) { c.Check(n["occurrences"], Equals, 1.0) } +func (cs *changeSuite) TestNewChangeWithExtraNoticeData(c *C) { + st := state.New(nil) + st.Lock() + defer st.Unlock() + + st.NewChangeWithNoticeData("perform-check", "...", map[string]string{"check-name": "c"}) + + notices := st.Notices(nil) + c.Assert(notices, HasLen, 1) + n := noticeToMap(c, notices[0]) + c.Check(n["last-data"], DeepEquals, map[string]any{"kind": "perform-check", "check-name": "c"}) +} + func (cs *changeSuite) TestReadyTime(c *C) { st := state.New(nil) st.Lock() diff --git a/internals/overlord/state/state.go b/internals/overlord/state/state.go index d22e23210..ab51b849e 100644 --- a/internals/overlord/state/state.go +++ b/internals/overlord/state/state.go @@ -333,11 +333,22 @@ func (s *State) Cache(key, value interface{}) { // NewChange adds a new change to the state. func (s *State) NewChange(kind, summary string) *Change { + return s.NewChangeWithNoticeData(kind, summary, nil) +} + +// NewChangeWithNoticeData adds a new change to the state, adding in any provided notice data. +func (s *State) NewChangeWithNoticeData(kind, summary string, noticeData map[string]string) *Change { s.writing() s.lastChangeId++ id := strconv.Itoa(s.lastChangeId) chg := newChange(s, id, kind, summary) s.changes[id] = chg + + // Set this before calling addNotice as that needs to use it. + if len(noticeData) > 0 { + chg.Set("notice-data", noticeData) + } + // Add change-update notice for newly spawned change // NOTE: Implies State.writing() if err := chg.addNotice(); err != nil { diff --git a/internals/overlord/state/state_test.go b/internals/overlord/state/state_test.go index b1cf534c6..b76a9d391 100644 --- a/internals/overlord/state/state_test.go +++ b/internals/overlord/state/state_test.go @@ -354,6 +354,36 @@ func (ss *stateSuite) TestNewChangeAndChanges(c *C) { c.Check(st.Change("no-such-id"), IsNil) } +func (ss *stateSuite) TestNewChangeWithNoticeData(c *C) { + st := state.New(nil) + st.Lock() + defer st.Unlock() + + extraData := map[string]string{"foo": "bar"} + chg := st.NewChangeWithNoticeData("perform-check", "...", extraData) + + chgs := st.Changes() + c.Check(chgs, HasLen, 1) + + var data map[string]string + err := chg.Get("notice-data", &data) + c.Assert(err, IsNil) + c.Check(data, DeepEquals, extraData) +} + +func (ss *stateSuite) TestNewChangeWithNilNoticeData(c *C) { + st := state.New(nil) + st.Lock() + defer st.Unlock() + + st.NewChange("replan", "...") + st.NewChangeWithNoticeData("replan", "...", nil) + + for _, chg := range st.Changes() { + c.Assert(chg.Has("notice-data"), Equals, false) + } +} + func (ss *stateSuite) TestNewChangeAndCheckpoint(c *C) { b := new(fakeStateBackend) st := state.New(b)