Merge pull request #12035 from MiguelPires/refresh-hold-overlord

o/snapstate: extend support for holding refreshes

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/configstate/configcore/refresh_test.go

@@ -132,6 +132,16 @@ func (s *refreshSuite) TestConfigureRefreshHoldHappy(c *C) {
 	c.Assert(err, IsNil)
 }
 
+func (s *refreshSuite) TestConfigureRefreshHoldForeverHappy(c *C) {
+	err := configcore.Run(classicDev, &mockConf{
+		state: s.state,
+		conf: map[string]interface{}{
+			"refresh.hold": "forever",
+		},
+	})
+	c.Assert(err, IsNil)
+}
+
 func (s *refreshSuite) TestConfigureRefreshHoldInvalid(c *C) {
 	err := configcore.Run(classicDev, &mockConf{
 		state: s.state,

overlord/hookstate/hooks.go

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 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
@@ -151,7 +151,7 @@ func (h *gateAutoRefreshHookHandler) Done() (err error) {
 				return fmt.Errorf("cannot unlock inhibit lock for snap %s: %v", snapName, err)
 			}
 		}
-		return snapstate.ProceedWithRefresh(st, snapName)
+		return snapstate.ProceedWithRefresh(st, snapName, nil)
 	} else {
 		var ok bool
 		action, ok = a.(snapstate.GateAutoRefreshAction)
@@ -172,7 +172,7 @@ func (h *gateAutoRefreshHookHandler) Done() (err error) {
 	case snapstate.GateAutoRefreshProceed:
 		// for action=proceed the ctlcmd doesn't call ProceedWithRefresh
 		// immediately, do it here.
-		if err := snapstate.ProceedWithRefresh(st, snapName); err != nil {
+		if err := snapstate.ProceedWithRefresh(st, snapName, nil); err != nil {
 			return err
 		}
 		if h.refreshAppAwareness {

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
@@ -55,6 +55,9 @@ const maxPostponementBuffer = 5 * 24 * time.Hour
 // to "13 days" left.
 const maxInhibition = 14*24*time.Hour - time.Second
 
+// maxDuration is used to represent "forever" internally (it's 290 years).
+const maxDuration = time.Duration(1<<63 - 1)
+
 // hooks setup by devicestate
 var (
 	CanAutoRefresh        func(st *state.State) (bool, error)
@@ -147,37 +150,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 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_gating.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2021 Canonical Ltd
+ * Copyright (C) 2021-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
@@ -35,6 +35,7 @@ import (
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/release"
 	"github.com/snapcore/snapd/snap"
+	"github.com/snapcore/snapd/strutil"
 )
 
 var gateAutoRefreshHookName = "gate-auto-refresh"
@@ -143,6 +144,36 @@ func holdDurationLeft(now time.Time, lastRefresh, firstHeld time.Time, maxDurati
 	return d2
 }
 
+// HoldRefreshesBySystem is used to hold snaps by the sys admin (denoted by the
+// "system" holding snap). HoldTime can be "forever" to denote an indefinite hold
+// or any RFC3339 timestamp.
+func HoldRefreshesBySystem(st *state.State, holdTime string, holdSnaps []string) error {
+	snaps, err := All(st)
+	if err != nil {
+		return err
+	}
+
+	for _, holdSnap := range holdSnaps {
+		if _, ok := snaps[holdSnap]; !ok {
+			return snap.NotInstalledError{Snap: holdSnap}
+		}
+	}
+
+	// zero value durations denote max allowed time in HoldRefresh
+	var holdDuration time.Duration
+	if holdTime != "forever" {
+		holdTime, err := time.Parse(time.RFC3339, holdTime)
+		if err != nil {
+			return err
+		}
+
+		holdDuration = holdTime.Sub(timeNow())
+	}
+
+	_, err = HoldRefresh(st, "system", holdDuration, holdSnaps...)
+	return err
+}
+
 // HoldRefresh marks affectingSnaps as held for refresh for up to holdTime.
 // HoldTime of zero denotes maximum allowed hold time.
 // Holding fails if not all snaps can be held, in that case HoldError is returned
@@ -162,56 +193,69 @@ func HoldRefresh(st *state.State, gatingSnap string, holdDuration time.Duration,
 
 	now := timeNow()
 	for _, heldSnap := range affectingSnaps {
+		var left time.Duration
+
 		hold, ok := gating[heldSnap][gatingSnap]
 		if !ok {
 			hold = &holdState{
 				FirstHeld: now,
 			}
 		}
 
-		lastRefreshTime, err := lastRefreshed(st, heldSnap)
-		if err != nil {
-			return 0, err
-		}
-
-		mp := maxPostponement - maxPostponementBuffer
-		maxDur := maxAllowedPostponement(gatingSnap, heldSnap, mp)
-
-		// calculate max hold duration that's left considering previous hold
-		// requests of this snap and last refresh time.
-		left := holdDurationLeft(now, lastRefreshTime, hold.FirstHeld, maxDur, mp)
-		if left <= 0 {
-			herr.SnapsInError[heldSnap] = HoldDurationError{
-				Err: fmt.Errorf("snap %q cannot hold snap %q anymore, maximum refresh postponement exceeded", gatingSnap, heldSnap),
+		if gatingSnap == "system" {
+			if holdDuration == 0 {
+				holdDuration = maxDuration
 			}
-			continue
-		}
 
-		dur := holdDuration
-		if dur == 0 {
-			// duration not specified, using a default one (maximum) or what's
-			// left of it.
-			dur = left
+			// if snap is being gated by "system" (it was set by the system admin), it
+			// can be held by any amount of time and no checks are required
+			hold.HoldUntil = now.Add(holdDuration)
+			left = holdDuration
 		} else {
-			// explicit hold duration requested
-			if dur > maxDur {
+			lastRefreshTime, err := lastRefreshed(st, heldSnap)
+			if err != nil {
+				return 0, err
+			}
+
+			mp := maxPostponement - maxPostponementBuffer
+			maxDur := maxAllowedPostponement(gatingSnap, heldSnap, mp)
+
+			// calculate max hold duration that's left considering previous hold
+			// requests of this snap and last refresh time.
+			left = holdDurationLeft(now, lastRefreshTime, hold.FirstHeld, maxDur, mp)
+			if left <= 0 {
 				herr.SnapsInError[heldSnap] = HoldDurationError{
-					Err:          fmt.Errorf("requested holding duration for snap %q of %s by snap %q exceeds maximum holding time", heldSnap, holdDuration, gatingSnap),
-					DurationLeft: left,
+					Err: fmt.Errorf("snap %q cannot hold snap %q anymore, maximum refresh postponement exceeded", gatingSnap, heldSnap),
 				}
 				continue
 			}
-		}
 
-		newHold := now.Add(dur)
-		cutOff := lastRefreshTime.Add(maxPostponement - maxPostponementBuffer)
+			dur := holdDuration
+			if dur == 0 {
+				// duration not specified, using a default one (maximum) or what's
+				// left of it.
+				dur = left
+			} else {
+				// explicit hold duration requested
+				if dur > maxDur {
+					herr.SnapsInError[heldSnap] = HoldDurationError{
+						Err:          fmt.Errorf("requested holding duration for snap %q of %s by snap %q exceeds maximum holding time", heldSnap, holdDuration, gatingSnap),
+						DurationLeft: left,
+					}
+					continue
+				}
+			}
+
+			newHold := now.Add(dur)
+			cutOff := lastRefreshTime.Add(maxPostponement - maxPostponementBuffer)
 
-		// consider last refresh time and adjust hold duration if needed so it's
-		// not exceeded.
-		if newHold.Before(cutOff) {
-			hold.HoldUntil = newHold
-		} else {
-			hold.HoldUntil = cutOff
+			// consider last refresh time and adjust hold duration if needed so it's
+			// not exceeded.
+			if newHold.Before(cutOff) {
+				hold.HoldUntil = newHold
+			} else {
+				hold.HoldUntil = cutOff
+			}
 		}
 
 		// finally store/update gating hold data
@@ -244,19 +288,21 @@ func HoldRefresh(st *state.State, gatingSnap string, holdDuration time.Duration,
 	return durationMin, nil
 }
 
-// ProceedWithRefresh unblocks all snaps held by gatingSnap for refresh. This
+// ProceedWithRefresh unblocks a set of snaps held by gatingSnap for refresh.
+// If no snaps are specified, all snaps held by gatingSnap are unblocked. This
 // should be called for --proceed on the gatingSnap.
-func ProceedWithRefresh(st *state.State, gatingSnap string) error {
+func ProceedWithRefresh(st *state.State, gatingSnap string, unholdSnaps []string) error {
 	gating, err := refreshGating(st)
 	if err != nil {
 		return err
 	}
-	if len(gating) == 0 {
-		return nil
-	}
 
 	var changed bool
 	for heldSnap, gatingSnaps := range gating {
+		if len(unholdSnaps) != 0 && !strutil.ListContains(unholdSnaps, heldSnap) {
+			continue
+		}
+
 		if _, ok := gatingSnaps[gatingSnap]; ok {
 			delete(gatingSnaps, gatingSnap)
 			changed = true
@@ -269,6 +315,7 @@ func ProceedWithRefresh(st *state.State, gatingSnap string) error {
 	if changed {
 		st.Set("snaps-hold", gating)
 	}
+
 	return nil
 }
 
@@ -312,7 +359,8 @@ func resetGatingForRefreshed(st *state.State, refreshedSnaps ...string) error {
 
 	var changed bool
 	for _, snapName := range refreshedSnaps {
-		if _, ok := gating[snapName]; ok {
+		// holds placed by the user remain after a refresh
+		if _, ok := gating[snapName]; ok && snapName != "system" {
 			delete(gating, snapName)
 			changed = true
 		}
@@ -360,8 +408,8 @@ func pruneSnapsHold(st *state.State, snapName string) error {
 	return nil
 }
 
-// heldSnaps returns all snaps that are gated and shouldn't be refreshed.
-func heldSnaps(st *state.State) (map[string]bool, error) {
+// HeldSnaps returns all snaps that are gated and shouldn't be refreshed.
+func HeldSnaps(st *state.State) (map[string]bool, error) {
 	gating, err := refreshGating(st)
 	if err != nil {
 		return nil, err
@@ -374,16 +422,18 @@ func heldSnaps(st *state.State) (map[string]bool, error) {
 
 	held := make(map[string]bool)
 Loop:
-	for heldSnap, holdingSnaps := range gating {
-		refreshed, err := lastRefreshed(st, heldSnap)
+	for heldSnap, holds := range gating {
+		lastRefresh, err := lastRefreshed(st, heldSnap)
 		if err != nil {
 			return nil, err
 		}
-		// make sure we don't hold any snap for more than maxPostponement
-		if refreshed.Add(maxPostponement).Before(now) {
-			continue
-		}
-		for _, hold := range holdingSnaps {
+
+		for holdingSnap, hold := range holds {
+			// enforce the maxPostponement limit on a hold, unless it's held by the user
+			if holdingSnap != "system" && lastRefresh.Add(maxPostponement).Before(now) {
+				continue
+			}
+
 			if hold.HoldUntil.Before(now) {
 				continue
 			}
@@ -628,7 +678,7 @@ var snapsToRefresh = func(gatingTask *state.Task) ([]*refreshCandidate, error) {
 		return nil, err
 	}
 
-	held, err := heldSnaps(gatingTask.State())
+	held, err := HeldSnaps(gatingTask.State())
 	if err != nil {
 		return nil, err
 	}