From 66dfd51ded32fa3716ca488c97814cfaf8e59e8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Sto=C5=82owski?= Date: Wed, 14 Jul 2021 17:02:32 +0200 Subject: [PATCH] Allow snapctl refresh --proceed from snaps. --- overlord/hookstate/ctlcmd/export_test.go | 8 +++ overlord/hookstate/ctlcmd/refresh.go | 31 ++++++++++-- overlord/hookstate/ctlcmd/refresh_test.go | 62 ++++++++++++++++++++--- 3 files changed, 90 insertions(+), 11 deletions(-) diff --git a/overlord/hookstate/ctlcmd/export_test.go b/overlord/hookstate/ctlcmd/export_test.go index 012cac309fd2..668ef5f7de81 100644 --- a/overlord/hookstate/ctlcmd/export_test.go +++ b/overlord/hookstate/ctlcmd/export_test.go @@ -108,3 +108,11 @@ func MockCgroupSnapNameFromPid(f func(int) (string, error)) (restore func()) { cgroupSnapNameFromPid = old } } + +func MockAutoRefreshForGatingSnap(f func(st *state.State, gatingSnap string) error) (restore func()) { + old := autoRefreshForGatingSnap + autoRefreshForGatingSnap = f + return func() { + autoRefreshForGatingSnap = old + } +} diff --git a/overlord/hookstate/ctlcmd/refresh.go b/overlord/hookstate/ctlcmd/refresh.go index fb85c3604c6e..cc6458d3421b 100644 --- a/overlord/hookstate/ctlcmd/refresh.go +++ b/overlord/hookstate/ctlcmd/refresh.go @@ -25,12 +25,16 @@ import ( "gopkg.in/yaml.v2" + "github.com/snapcore/snapd/features" "github.com/snapcore/snapd/i18n" + "github.com/snapcore/snapd/overlord/configstate/config" "github.com/snapcore/snapd/overlord/hookstate" "github.com/snapcore/snapd/overlord/snapstate" "github.com/snapcore/snapd/snap" ) +var autoRefreshForGatingSnap = snapstate.AutoRefreshForGatingSnap + type refreshCommand struct { baseCommand @@ -89,12 +93,8 @@ func (c *refreshCommand) Execute(args []string) error { if context == nil { return fmt.Errorf("cannot run without a context") } - if context.IsEphemeral() { - // TODO: handle this - return fmt.Errorf("cannot run outside of gate-auto-refresh hook") - } - if context.HookName() != "gate-auto-refresh" { + if !context.IsEphemeral() && context.HookName() != "gate-auto-refresh" { return fmt.Errorf("can only be used from gate-auto-refresh hook") } @@ -214,6 +214,9 @@ func (c *refreshCommand) printPendingInfo() error { func (c *refreshCommand) hold() error { ctx := c.context() + if ctx.IsEphemeral() { + return fmt.Errorf("cannot hold outside of gate-auto-refresh hook") + } ctx.Lock() defer ctx.Unlock() st := ctx.State() @@ -241,6 +244,24 @@ func (c *refreshCommand) proceed() error { ctx.Lock() defer ctx.Unlock() + // running outside of hook + if ctx.IsEphemeral() { + st := ctx.State() + // we need to check if GateAutoRefreshHook feature is enabled when + // running by the snap (we don't need to do this when running from the + // hook because in that case hook task won't be created if not enabled). + tr := config.NewTransaction(st) + gateAutoRefreshHook, err := features.Flag(tr, features.GateAutoRefreshHook) + if err != nil && !config.IsNoOption(err) { + return err + } + if !gateAutoRefreshHook { + return fmt.Errorf("cannot proceed without experimental.gate-auto-refresh feature enabled") + } + + return autoRefreshForGatingSnap(st, ctx.InstanceName()) + } + // cache the action, hook handler will trigger proceed logic; we cannot // call snapstate.ProceedWithRefresh() immediately as this would reset // holdState, allowing the snap to --hold with fresh duration limit. diff --git a/overlord/hookstate/ctlcmd/refresh_test.go b/overlord/hookstate/ctlcmd/refresh_test.go index cf7879ac9e78..d43430b2bdbc 100644 --- a/overlord/hookstate/ctlcmd/refresh_test.go +++ b/overlord/hookstate/ctlcmd/refresh_test.go @@ -20,11 +20,13 @@ package ctlcmd_test import ( + "fmt" "time" . "gopkg.in/check.v1" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/overlord/configstate/config" "github.com/snapcore/snapd/overlord/hookstate" "github.com/snapcore/snapd/overlord/hookstate/ctlcmd" "github.com/snapcore/snapd/overlord/hookstate/hooktest" @@ -239,17 +241,65 @@ func (s *refreshSuite) TestRefreshFromUnsupportedHook(c *C) { c.Check(err, ErrorMatches, `can only be used from gate-auto-refresh hook`) } -// TODO: support this case -func (s *refreshSuite) TestRefreshFromApp(c *C) { +func (s *refreshSuite) TestRefreshProceedFromSnap(c *C) { + var called bool + restore := ctlcmd.MockAutoRefreshForGatingSnap(func(st *state.State, gatingSnap string) error { + called = true + c.Check(gatingSnap, Equals, "foo") + return nil + }) + defer restore() + s.st.Lock() + defer s.st.Unlock() + mockInstalledSnap(c, s.st, `name: foo +version: 1 +`) - setup := &hookstate.HookSetup{Snap: "snap", Revision: snap.R(1)} - mockContext, err := hookstate.NewContext(nil, s.st, setup, s.mockHandler, "") + // enable gate-auto-refresh-hook feature + tr := config.NewTransaction(s.st) + tr.Set("core", "experimental.gate-auto-refresh-hook", true) + tr.Commit() + + // foo is the snap that is going to call --proceed. + setup := &hookstate.HookSetup{Snap: "foo", Revision: snap.R(1)} + mockContext, err := hookstate.NewContext(nil, s.st, setup, nil, "") c.Check(err, IsNil) s.st.Unlock() + defer s.st.Lock() - _, _, err = ctlcmd.Run(mockContext, []string{"refresh"}, 0) - c.Check(err, ErrorMatches, `cannot run outside of gate-auto-refresh hook`) + _, _, err = ctlcmd.Run(mockContext, []string{"refresh", "--proceed"}, 0) + c.Assert(err, IsNil) + c.Check(called, Equals, true) +} + +func (s *refreshSuite) TestRefreshProceedFromSnapError(c *C) { + restore := ctlcmd.MockAutoRefreshForGatingSnap(func(st *state.State, gatingSnap string) error { + c.Check(gatingSnap, Equals, "foo") + return fmt.Errorf("boom") + }) + defer restore() + + s.st.Lock() + defer s.st.Unlock() + mockInstalledSnap(c, s.st, `name: foo +version: 1 +`) + + // enable gate-auto-refresh-hook feature + tr := config.NewTransaction(s.st) + tr.Set("core", "experimental.gate-auto-refresh-hook", true) + tr.Commit() + + // foo is the snap that is going to call --proceed. + setup := &hookstate.HookSetup{Snap: "foo", Revision: snap.R(1)} + mockContext, err := hookstate.NewContext(nil, s.st, setup, nil, "") + c.Check(err, IsNil) + s.st.Unlock() + defer s.st.Lock() + + _, _, err = ctlcmd.Run(mockContext, []string{"refresh", "--proceed"}, 0) + c.Assert(err, ErrorMatches, "boom") } func (s *refreshSuite) TestRefreshRegularUserForbidden(c *C) {