From 9eacb5d00fe870a4fca710975a33778273f41a33 Mon Sep 17 00:00:00 2001 From: Stephen Mwangi Date: Wed, 15 Jul 2026 14:06:30 +0300 Subject: [PATCH] o/devicemgmtstate: remove sequence entry after unsequenced message is processed --- overlord/devicemgmtstate/devicemgmtmgr.go | 12 +++- .../devicemgmtstate/devicemgmtmgr_test.go | 68 ++++++++++++++----- 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/overlord/devicemgmtstate/devicemgmtmgr.go b/overlord/devicemgmtstate/devicemgmtmgr.go index b75198b54a..3e8b04da46 100644 --- a/overlord/devicemgmtstate/devicemgmtmgr.go +++ b/overlord/devicemgmtstate/devicemgmtmgr.go @@ -183,9 +183,9 @@ func (ms *deviceMgmtState) getRequestMessage(id string) (*RequestMessage, error) return nil, fmt.Errorf("cannot find message %q", id) } -// removeRequestMessage removes a processed request message from its sequence, -// leaving the sequence entry in place so its Applied progress is preserved for -// later messages in the same sequence. +// removeRequestMessage removes a processed request message from its sequence. +// For a sequenced message, the sequence entry is left in place so its Applied +// progress is preserved for later messages in the same sequence. func (ms *deviceMgmtState) removeRequestMessage(msg *RequestMessage) { seq := ms.Sequences[msg.BaseID] if seq == nil { @@ -195,6 +195,12 @@ func (ms *deviceMgmtState) removeRequestMessage(msg *RequestMessage) { for i, m := range seq.Messages { if m.SeqNum == msg.SeqNum { seq.Messages = append(seq.Messages[:i], seq.Messages[i+1:]...) + + // Unsequenced messages have no Applied progress to carry forward. + if msg.SeqNum == 0 && len(seq.Messages) == 0 { + delete(ms.Sequences, msg.BaseID) + } + return } } diff --git a/overlord/devicemgmtstate/devicemgmtmgr_test.go b/overlord/devicemgmtstate/devicemgmtmgr_test.go index 2512d35c28..162f88fd8d 100644 --- a/overlord/devicemgmtstate/devicemgmtmgr_test.go +++ b/overlord/devicemgmtstate/devicemgmtmgr_test.go @@ -165,7 +165,7 @@ func (s *deviceMgmtMgrSuite) SetUpTest(c *C) { return chg.ID(), nil }, resultFromChange: func(*state.Change) (map[string]any, error) { - return map[string]any{"key": "value"}, nil + return map[string]any{"values": "ok"}, nil }, }) } @@ -1412,7 +1412,7 @@ func (s *deviceMgmtMgrSuite) TestDoApplyMessageMessageNotFound(c *C) { c.Assert(err, ErrorMatches, `cannot find message "seqA-2"`) } -func (s *deviceMgmtMgrSuite) TestDoQueueResponseOK(c *C) { +func (s *deviceMgmtMgrSuite) TestDoQueueResponseSequencedOK(c *C) { s.st.Lock() defer s.st.Unlock() @@ -1424,18 +1424,6 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseOK(c *C) { }, nil }) - s.mgr.RegisterHandler("test-kind", &mockMessageHandler{ - validate: func(*state.State, *devicemgmtstate.RequestMessage) error { return nil }, - apply: func(st *state.State, msg *devicemgmtstate.RequestMessage) (string, error) { - chg := st.NewChange("subsystem", "apply payload") - devicemgmtstate.MarkChangeForMessage(chg, msg) - return chg.ID(), nil - }, - resultFromChange: func(*state.Change) (map[string]any, error) { - return map[string]any{"values": "ok"}, nil - }, - }) - s.mgr.MockSigner(&mockSigner{ sign: func(accountID, messageID string, status asserts.MessageStatus, body []byte) (*asserts.ResponseMessage, error) { c.Check(accountID, Equals, "my-brand") @@ -1459,10 +1447,49 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseOK(c *C) { ms, err := s.mgr.GetState() c.Assert(err, IsNil) + // The sequence entry is kept because Applied must be preserved. c.Check(ms.Sequences["mesg"].Messages, HasLen, 0) + c.Check(ms.Sequences["mesg"].Applied, Equals, 1) + c.Assert(ms.ReadyResponses, HasLen, 1) c.Check(ms.ReadyResponses["mesg-1"].Format, Equals, "assertion") - c.Check(ms.Sequences["mesg"].Applied, Equals, 1) +} + +func (s *deviceMgmtMgrSuite) TestDoQueueResponseUnsequencedOK(c *C) { + s.st.Lock() + defer s.st.Unlock() + + s.mockStore(func(_ context.Context, _ *store.MessageExchangeRequest) (*store.MessageExchangeResponse, error) { + return &store.MessageExchangeResponse{ + Messages: []store.MessageWithToken{ + s.makeStoreRequestMessage(c, "mesg", "test-kind", "token-1"), + }, + }, nil + }) + + s.mgr.MockSigner(&mockSigner{ + sign: func(accountID, messageID string, status asserts.MessageStatus, body []byte) (*asserts.ResponseMessage, error) { + return assertstest.FakeAssertionWithBody(body, map[string]any{ + "type": "response-message", + "account-id": accountID, + "message-id": messageID, + "device": "serial-1.my-model.my-brand", + "status": string(status), + "body-length": strconv.Itoa(len(body)), + }).(*asserts.ResponseMessage), nil + }, + }) + + s.settle(c) + + ms, err := s.mgr.GetState() + c.Assert(err, IsNil) + + // Unsequenced messages have no Applied progress to carry forward. + c.Check(ms.Sequences, HasLen, 0) + + c.Assert(ms.ReadyResponses, HasLen, 1) + c.Check(ms.ReadyResponses["mesg"].Format, Equals, "assertion") } func (s *deviceMgmtMgrSuite) TestDoQueueResponseStatusAlreadyKnown(c *C) { @@ -1529,8 +1556,9 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseStatusAlreadyKnown(c *C) { c.Assert(err, IsNil) c.Check(ms.Sequences["mesg"].Messages, HasLen, 0) - c.Assert(ms.ReadyResponses, HasLen, 1) c.Check(ms.Sequences["mesg"].Applied, Equals, 0) + + c.Assert(ms.ReadyResponses, HasLen, 1) } func (s *deviceMgmtMgrSuite) TestDoQueueResponseIdempotent(c *C) { @@ -1642,9 +1670,10 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseResultFromChangeError(c *C) { c.Assert(err, IsNil) c.Check(ms.Sequences["mesg"].Messages, HasLen, 0) + c.Check(ms.Sequences["mesg"].Applied, Equals, 0) + c.Assert(ms.ReadyResponses, HasLen, 1) c.Check(ms.ReadyResponses["mesg-1"].Format, Equals, "assertion") - c.Check(ms.Sequences["mesg"].Applied, Equals, 0) } func (s *deviceMgmtMgrSuite) TestDoQueueResponseSubsystemChangeNotFound(c *C) { @@ -1748,7 +1777,8 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseNoHandlerForMessageKind(c *C) { ms, err = s.mgr.GetState() c.Assert(err, IsNil) - c.Check(ms.Sequences["msg1"].Messages, HasLen, 0) + c.Check(ms.Sequences, HasLen, 0) + c.Assert(ms.ReadyResponses, HasLen, 1) c.Check(ms.ReadyResponses["msg1"].Format, Equals, "assertion") } @@ -1831,6 +1861,7 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseSubsystemChangeNotReady(c *C) { ms, err = s.mgr.GetState() c.Assert(err, IsNil) + c.Check(ms.ReadyResponses, HasLen, 1) c.Check(ms.ReadyResponses["msg1"].Format, Equals, "assertion") } @@ -1878,6 +1909,7 @@ func (s *deviceMgmtMgrSuite) TestDoQueueResponseSigningError(c *C) { ms, err := s.mgr.GetState() c.Assert(err, IsNil) + c.Check(ms.Sequences["mesg"].Applied, Equals, 0) }