o/snapstate: allow indefinite all-snaps holds

Extends the 'refresh.hold' "all-snaps" hold mechanism to allow
indefinite holds.

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

overlord/configstate/configcore/refresh.go

@@ -3,7 +3,7 @@
 // +build !nomanagers
 
 /*
- * Copyright (C) 2017-2018 Canonical Ltd
+ * Copyright (C) 2017-2022 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
@@ -69,7 +69,7 @@ func validateRefreshSchedule(tr config.Conf) error {
 	if err != nil {
 		return err
 	}
-	if refreshHoldStr != "" {
+	if refreshHoldStr != "" && refreshHoldStr != "forever" {
 		if _, err := time.Parse(time.RFC3339, refreshHoldStr); err != nil {
 			return fmt.Errorf("refresh.hold cannot be parsed: %v", err)
 		}

overlord/snapstate/autorefresh.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2017-2020 Canonical Ltd
+ * Copyright (C) 2017-2022 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
@@ -60,10 +60,15 @@ var (
 	CanAutoRefresh        func(st *state.State) (bool, error)
 	CanManageRefreshes    func(st *state.State) bool
 	IsOnMeteredConnection func() (bool, error)
-)
 
-// refreshRetryDelay specified the minimum time to retry failed refreshes
-var refreshRetryDelay = 20 * time.Minute
+	// refreshRetryDelay specified the minimum time to retry failed refreshes
+	refreshRetryDelay = 20 * time.Minute
+
+	// used to represent "forever". It's actually 290 years from the current time
+	// because that's the maximum representable duration and a larger time would
+	// underflow when doing infinity.Sub(time.Now())
+	maxDuration = time.Duration(1<<63 - 1)
+)
 
 // refreshCandidate carries information about a single snap to update as part
 // of auto-refresh.
@@ -139,37 +144,25 @@ func (m *autoRefresh) LastRefresh() (time.Time, error) {
 }
 
 // EffectiveRefreshHold returns the time until to which refreshes are
-// held if refresh.hold configuration is set and accounting for the
-// max postponement since the last refresh.
+// held if refresh.hold configuration is set.
 func (m *autoRefresh) EffectiveRefreshHold() (time.Time, error) {
-	var holdTime time.Time
+	var holdValue string
 
 	tr := config.NewTransaction(m.state)
-	err := tr.Get("core", "refresh.hold", &holdTime)
+	err := tr.Get("core", "refresh.hold", &holdValue)
 	if err != nil && !config.IsNoOption(err) {
 		return time.Time{}, err
 	}
 
-	// cannot hold beyond last-refresh + max-postponement
-	lastRefresh, err := m.LastRefresh()
-	if err != nil {
-		return time.Time{}, err
+	if string(holdValue) == "forever" {
+		return timeNow().Add(maxDuration), nil
 	}
-	if lastRefresh.IsZero() {
-		seedTime, err := getTime(m.state, "seed-time")
-		if err != nil {
+
+	var holdTime time.Time
+	if holdValue != "" {
+		if holdTime, err = time.Parse(time.RFC3339, holdValue); err != nil {
 			return time.Time{}, err
 		}
-		if seedTime.IsZero() {
-			// no reference to know whether holding is reasonable
-			return time.Time{}, nil
-		}
-		lastRefresh = seedTime
-	}
-
-	limitTime := lastRefresh.Add(maxPostponement)
-	if holdTime.After(limitTime) {
-		return limitTime, nil
 	}
 
 	return holdTime, nil

overlord/snapstate/autorefresh_test.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2017-2018 Canonical Ltd
+ * Copyright (C) 2017-2022 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
@@ -396,6 +396,37 @@ func (s *autoRefreshTestSuite) TestDefaultScheduleIsRandomized(c *C) {
 	}
 }
 
+func (s *autoRefreshTestSuite) TestRefreshHoldForever(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	lastRefresh := time.Now().Add(-12 * time.Hour)
+	s.state.Set("last-refresh", lastRefresh)
+
+	tr := config.NewTransaction(s.state)
+	tr.Set("core", "refresh.hold", "forever")
+	tr.Commit()
+
+	af := snapstate.NewAutoRefresh(s.state)
+	s.state.Unlock()
+	err := af.Ensure()
+	s.state.Lock()
+	c.Check(err, IsNil)
+
+	// no refresh
+	c.Check(s.store.ops, HasLen, 0)
+
+	var storedRefresh time.Time
+	c.Assert(s.state.Get("last-refresh", &storedRefresh), IsNil)
+	cmt := Commentf("expected %s but got %s instead", lastRefresh.Format(time.RFC3339), storedRefresh.Format(time.RFC3339))
+	c.Check(storedRefresh.Equal(lastRefresh), Equals, true, cmt)
+
+	var holdVal string
+	err = tr.Get("core", "refresh.hold", &holdVal)
+	c.Assert(err, IsNil)
+	c.Check(holdVal, Equals, "forever")
+}
+
 func (s *autoRefreshTestSuite) TestLastRefreshRefreshHold(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
@@ -670,11 +701,7 @@ func (s *autoRefreshTestSuite) TestEffectiveRefreshHold(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
-	// assume no seed-time
-	s.state.Set("seed-time", nil)
-
 	af := snapstate.NewAutoRefresh(s.state)
-
 	t0, err := af.EffectiveRefreshHold()
 	c.Assert(err, IsNil)
 	c.Check(t0.IsZero(), Equals, true)
@@ -684,24 +711,23 @@ func (s *autoRefreshTestSuite) TestEffectiveRefreshHold(c *C) {
 	tr.Set("core", "refresh.hold", holdTime)
 	tr.Commit()
 
-	seedTime := holdTime.Add(-100 * 24 * time.Hour)
-	s.state.Set("seed-time", seedTime)
-
 	t1, err := af.EffectiveRefreshHold()
 	c.Assert(err, IsNil)
-	c.Check(t1.Equal(seedTime.Add(95*24*time.Hour)), Equals, true)
+	c.Check(t1.Equal(holdTime), Equals, true)
 
-	lastRefresh := holdTime.Add(-99 * 24 * time.Hour)
-	s.state.Set("last-refresh", lastRefresh)
+	longTime := time.Now().Add(snapstate.MaxDuration + time.Hour)
+	// truncate time that isn't part of the RFC3339 timestamp
+	longTime = longTime.UTC().Truncate(time.Second)
 
-	t1, err = af.EffectiveRefreshHold()
-	c.Assert(err, IsNil)
-	c.Check(t1.Equal(lastRefresh.Add(95*24*time.Hour)), Equals, true)
+	tr = config.NewTransaction(s.state)
+	tr.Set("core", "refresh.hold", longTime.Format(time.RFC3339))
+	tr.Commit()
 
-	s.state.Set("last-refresh", holdTime.Add(-6*time.Hour))
-	t1, err = af.EffectiveRefreshHold()
+	t2, err := af.EffectiveRefreshHold()
 	c.Assert(err, IsNil)
-	c.Check(t1.Equal(holdTime), Equals, true)
+
+	t2 = t2.UTC().Truncate(time.Second)
+	c.Check(t2.Equal(longTime), Equals, true)
 }
 
 func (s *autoRefreshTestSuite) TestEnsureLastRefreshAnchor(c *C) {

overlord/snapstate/export_test.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2016-2019 Canonical Ltd
+ * Copyright (C) 2016-2022 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
@@ -313,6 +313,7 @@ var (
 var (
 	InhibitRefresh = inhibitRefresh
 	MaxInhibition  = maxInhibition
+	MaxDuration    = maxDuration
 )
 
 type RefreshCandidate = refreshCandidate
@@ -349,7 +350,6 @@ type HoldState = holdState
 var (
 	HoldDurationLeft           = holdDurationLeft
 	LastRefreshed              = lastRefreshed
-	HeldSnaps                  = heldSnaps
 	PruneRefreshCandidates     = pruneRefreshCandidates
 	ResetGatingForRefreshed    = resetGatingForRefreshed
 	PruneGating                = pruneGating

overlord/snapstate/snapmgr.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2016-2017 Canonical Ltd
+ * Copyright (C) 2016-2022 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
@@ -602,8 +602,7 @@ func (m *SnapManager) NextRefresh() time.Time {
 }
 
 // EffectiveRefreshHold returns the time until to which refreshes are
-// held if refresh.hold configuration is set and accounting for the
-// max postponement since the last refresh.
+// held if refresh.hold configuration is set.
 // The caller should be holding the state lock.
 func (m *SnapManager) EffectiveRefreshHold() (time.Time, error) {
 	return m.autoRefresh.EffectiveRefreshHold()