From 522c4fc28ca902fa106be731b4d15b83460498f4 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Mon, 5 Feb 2024 13:02:26 +0100 Subject: [PATCH] o/snapstate/autorefresh: do not expect a valid time string in refresh.hold It is possible that refresh.hold has been set to "forever" even before snapd ran for the first time. Reference: https://bugs.launchpad.net/bugs/2051917 Signed-off-by: Maciej Borzecki --- overlord/snapstate/autorefresh.go | 12 ++++--- overlord/snapstate/autorefresh_test.go | 48 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/overlord/snapstate/autorefresh.go b/overlord/snapstate/autorefresh.go index a89f16dbefa..8ccca96d776 100644 --- a/overlord/snapstate/autorefresh.go +++ b/overlord/snapstate/autorefresh.go @@ -225,16 +225,18 @@ func (m *autoRefresh) clearRefreshHold() { func (m *autoRefresh) AtSeed() error { // on classic hold refreshes for 2h after seeding if release.OnClassic { - var t1 time.Time - tr := config.NewTransaction(m.state) - err := tr.Get("core", "refresh.hold", &t1) - if !config.IsNoOption(err) { - // already set or error + holdTime, err := effectiveRefreshHold(m.state) + if err != nil { return err } + if !holdTime.IsZero() { + // already set + return nil + } // TODO: have a policy that if the snapd exe itself // is older than X weeks/months we skip the holding? now := time.Now().UTC() + tr := config.NewTransaction(m.state) tr.Set("core", "refresh.hold", now.Add(2*time.Hour)) tr.Commit() m.nextRefresh = now diff --git a/overlord/snapstate/autorefresh_test.go b/overlord/snapstate/autorefresh_test.go index bacb3252607..078947d34a7 100644 --- a/overlord/snapstate/autorefresh_test.go +++ b/overlord/snapstate/autorefresh_test.go @@ -848,6 +848,54 @@ func (s *autoRefreshTestSuite) TestAtSeedPolicy(c *C) { c.Check(t1.Equal(t2), Equals, true) } +func (s *autoRefreshTestSuite) TestAtSeedRefreshHeld(c *C) { + // it is possible that refresh.hold has already been set to a valid time + // or "forever" even before snapd was started for the first time + r := release.MockOnClassic(true) + defer r() + + s.state.Lock() + defer s.state.Unlock() + + tr := config.NewTransaction(s.state) + c.Assert(tr.Set("core", "refresh.hold", "forever"), IsNil) + tr.Commit() + + af := snapstate.NewAutoRefresh(s.state) + err := af.AtSeed() + c.Assert(err, IsNil) + c.Check(af.NextRefresh().IsZero(), Equals, true) + + // now use a valid timestamp + tr = config.NewTransaction(s.state) + c.Assert(tr.Set("core", "refresh.hold", time.Now().UTC()), IsNil) + tr.Commit() + + af = snapstate.NewAutoRefresh(s.state) + err = af.AtSeed() + c.Assert(err, IsNil) + c.Check(af.NextRefresh().IsZero(), Equals, true) +} + +func (s *autoRefreshTestSuite) TestAtSeedInvalidHold(c *C) { + // it is possible that refresh.hold has already been set to forever even + // before snapd was started for the first time + r := release.MockOnClassic(true) + defer r() + + s.state.Lock() + defer s.state.Unlock() + + tr := config.NewTransaction(s.state) + c.Assert(tr.Set("core", "refresh.hold", "this-is-invalid-time"), IsNil) + tr.Commit() + + af := snapstate.NewAutoRefresh(s.state) + + err := af.AtSeed() + c.Assert(err, ErrorMatches, `parsing time "this-is-invalid-time" .*cannot parse.*`) +} + func (s *autoRefreshTestSuite) TestCanRefreshOnMetered(c *C) { s.state.Lock() defer s.state.Unlock()