diff --git a/confdb/confdb.go b/confdb/confdb.go index 9191ddde1bc..65b269419f9 100644 --- a/confdb/confdb.go +++ b/confdb/confdb.go @@ -324,6 +324,11 @@ func (s *Schema) ID() SchemaID { } } +// IsSystem returns true if the schema represents a system builtin confdb-schema. +func (s *Schema) IsSystem() bool { + return s.Account == "system" +} + // GetViewsAffectedByPath returns all the views in the confdb schema that have // visibility into a storage path. func (s *Schema) GetViewsAffectedByPath(path []Accessor) []*View { diff --git a/confdb/confdb_test.go b/confdb/confdb_test.go index f348e5be03f..d93029090cf 100644 --- a/confdb/confdb_test.go +++ b/confdb/confdb_test.go @@ -197,6 +197,24 @@ func (*viewSuite) TestNewConfdb(c *C) { } } +func (*viewSuite) TestSchemaIsSystem(c *C) { + views := map[string]any{ + "foo": map[string]any{ + "rules": []any{ + map[string]any{"storage": "foo"}, + }, + }, + } + + db, err := confdb.NewSchema("system", "foo", views, confdb.NewJSONSchema()) + c.Assert(err, IsNil) + c.Check(db.IsSystem(), Equals, true) + + db, err = confdb.NewSchema("other", "foo", views, confdb.NewJSONSchema()) + c.Assert(err, IsNil) + c.Check(db.IsSystem(), Equals, false) +} + func (s *viewSuite) TestMissingRequestDefaultsToStorage(c *C) { databag := confdb.NewJSONDatabag() views := map[string]any{ diff --git a/interfaces/builtin/confdb.go b/interfaces/builtin/confdb.go index 528aba4659c..5511d8f0d4b 100644 --- a/interfaces/builtin/confdb.go +++ b/interfaces/builtin/confdb.go @@ -76,6 +76,10 @@ func (iface *confdbInterface) BeforePreparePlug(plug *snap.PlugInfo) error { return fmt.Errorf(`optional confdb plug "role" attribute must be "custodian"`) } + if role == "custodian" && account == "system" { + return fmt.Errorf(`snaps cannot be custodians of "system" confdb schemas`) + } + return nil } diff --git a/interfaces/builtin/confdb_test.go b/interfaces/builtin/confdb_test.go index 85acaf0ae7e..b62f1c61d3c 100644 --- a/interfaces/builtin/confdb_test.go +++ b/interfaces/builtin/confdb_test.go @@ -98,6 +98,16 @@ func (s *confdbSuite) TestConfdbSanitizePlug(c *C) { account: "my-acc", view: "network/wifi", }, + { + account: "system", + view: "network/wifi", + }, + { + account: "system", + view: "network/wifi", + role: "custodian", + err: `snaps cannot be custodians of "system" confdb schemas`, + }, { err: `confdb plug must have an "account" attribute`, }, diff --git a/overlord/confdbstate/confdbmgr.go b/overlord/confdbstate/confdbmgr.go index 23a6e4f03b4..bf75fea7eb4 100644 --- a/overlord/confdbstate/confdbmgr.go +++ b/overlord/confdbstate/confdbmgr.go @@ -23,6 +23,7 @@ import ( "fmt" "regexp" "strings" + "time" "github.com/snapcore/snapd/client" "github.com/snapcore/snapd/confdb" @@ -62,8 +63,11 @@ type SystemConfdbHandler interface { Databag(st *state.State) (confdb.JSONDatabag, error) } -// systemHandlers holds handlers for "system" confdb-schemas. -var systemHandlers = map[string]SystemConfdbHandler{} +var ( + // systemHandlers holds handlers for "system" confdb-schemas. + systemHandlers = map[string]SystemConfdbHandler{} + commitTimeout = 2 * time.Second +) // RegisterConfdbHandler registers a handler for a "system" confdb-schema. func RegisterConfdbHandler(c SystemConfdbHandler) { @@ -124,7 +128,7 @@ func (m *ConfdbManager) doCommitTransaction(t *state.Task, _ *tomb.Tomb) (err er st.Lock() defer st.Unlock() - tx, _, _, err := GetStoredTransaction(t) + tx, _, saveTxChanges, err := GetStoredTransaction(t) if err != nil { return err } @@ -153,6 +157,63 @@ func (m *ConfdbManager) doCommitTransaction(t *state.Task, _ *tomb.Tomb) (err er } } + // changes to "system" confdbs are persisted by subsystem handlers, not by + // the usual databag commit path + if tx.ConfdbAccount == "system" { + handler, ok := systemHandlers[tx.ConfdbName] + if !ok { + // shouldn't happen; we check for this early + return fmt.Errorf("internal error: no system handler registered for confdb-schema %q", tx.ConfdbName) + } + + var committed bool + if err := t.Get("scheduled-tasks", &committed); err != nil && !errors.Is(err, state.ErrNoState) { + return err + } + + if committed { + // a previous run of this task scheduled async tasks which have now + // finished (see the async path below) so we're done. Reset the tx which + // re-reads state, so hooks get fully updated state since the subsystem + // handlers may make state changes. + if err := tx.Reset(st); err != nil { + return err + } + saveTxChanges() + return nil + } + + taskSets, err := handler.Commit(st, tx) + if err != nil { + return err + } + + if len(taskSets) == 0 { + // synchronous commit, nothing to wait for. Reset the tx which re-reads + // state, so hooks get fully updated state since the subsystem handlers + // may make state changes. + if err := tx.Reset(st); err != nil { + return err + } + saveTxChanges() + return nil + } + + // this confdb handler needs to run tasks asynchronously, so make this task + // wait for those and rerun when they're done + chg := t.Change() + for _, ts := range taskSets { + chg.AddAll(ts) + t.WaitAll(ts) + } + t.Set("scheduled-tasks", true) + + // ensure the new tasks are picked up by the task runner + st.EnsureBefore(0) + + return &state.Retry{After: commitTimeout} + } + // we error early if a write may affect ephemeral data but no save-view hook // is present. However, a change-view hook may have written to an ephemeral // path after that so we have to check again @@ -587,7 +648,7 @@ func (h *saveViewHandler) Error(origErr error) (ignoreErr bool, err error) { } // clear the transaction changes - err = tx.Clear(st) + err = tx.Reset(st) if err != nil { return false, fmt.Errorf("cannot rollback failed save-view: cannot clear transaction changes: %v", err) } diff --git a/overlord/confdbstate/confdbmgr_test.go b/overlord/confdbstate/confdbmgr_test.go index a81bcba62ab..589d3baf4a3 100644 --- a/overlord/confdbstate/confdbmgr_test.go +++ b/overlord/confdbstate/confdbmgr_test.go @@ -465,7 +465,7 @@ func (s *confdbTestSuite) TestCommitTransaction(c *C) { // clearing would remove non-committed changes, so if we read the set value // it's because it has been successfully committed - err = tx.Clear(s.state) + err = tx.Reset(s.state) c.Assert(err, IsNil) val, err := tx.Get(parsePath(c, "wifi.ssid"), nil) diff --git a/overlord/confdbstate/confdbstate.go b/overlord/confdbstate/confdbstate.go index eaf4b4c0e00..231b897d5f8 100644 --- a/overlord/confdbstate/confdbstate.go +++ b/overlord/confdbstate/confdbstate.go @@ -478,13 +478,22 @@ func WriteConfdbFromSnap(hookCtx *hookstate.Context, view *confdb.View, values m } func createChangeConfdbTasks(st *state.State, tx *Transaction, view *confdb.View, callingSnap string) (ts *state.TaskSet, commitTask, clearTxTask *state.Task, err error) { - custodians, custodianPlugs, err := getCustodianPlugsForView(st, view) - if err != nil { - return nil, nil, nil, err - } + var custodians []string + var custodianPlugs map[string]*snap.PlugInfo + if !view.Schema().IsSystem() { + custodians, custodianPlugs, err = getCustodianPlugsForView(st, view) + if err != nil { + return nil, nil, nil, err + } - if len(custodianPlugs) == 0 { - return nil, nil, nil, fmt.Errorf("cannot write confdb view %s: no custodian snap connected", view.ID()) + if len(custodianPlugs) == 0 { + return nil, nil, nil, fmt.Errorf("cannot write confdb view %s: no custodian snap connected", view.ID()) + } + } else { + // fail early if there is no appropriate subsystem handler + if _, ok := systemHandlers[tx.ConfdbName]; !ok { + return nil, nil, nil, fmt.Errorf("cannot write confdb system/%s: no internal handler", view.Schema().Name) + } } paths := tx.AlteredPaths() @@ -524,7 +533,7 @@ func createChangeConfdbTasks(st *state.State, tx *Transaction, view *confdb.View linkTask(chgViewTask) } - if hookPrefix == "save-view-" && mightAffectEph && !saveViewHookPresent { + if hookPrefix == "save-view-" && mightAffectEph && !saveViewHookPresent && !view.Schema().IsSystem() { return nil, nil, nil, fmt.Errorf("cannot write confdb view %s: write might change ephemeral data but no custodians has a save-view hook", view.ID()) } } @@ -949,13 +958,23 @@ func ReadConfdb(ctx context.Context, st *state.State, view *confdb.View, request // load-view or query-view hooks, nil is returned. If there are hooks to run, // a clear-confdb-tx task is also scheduled to remove the ongoing transaction at the end. func createLoadConfdbTasks(st *state.State, tx *Transaction, view *confdb.View, requests []string, constraints map[string]any) (*state.TaskSet, *state.Task, error) { - custodians, custodianPlugs, err := getCustodianPlugsForView(st, view) - if err != nil { - return nil, nil, err - } + var custodians []string + var custodianPlugs map[string]*snap.PlugInfo + if !view.Schema().IsSystem() { + var err error + custodians, custodianPlugs, err = getCustodianPlugsForView(st, view) + if err != nil { + return nil, nil, err + } - if len(custodians) == 0 { - return nil, nil, fmt.Errorf("cannot read confdb view %s: no custodian snap connected", view.ID()) + if len(custodians) == 0 { + return nil, nil, fmt.Errorf("cannot read confdb view %s: no custodian snap connected", view.ID()) + } + } else { + // fail early if there is no appropriate subsystem handler + if _, ok := systemHandlers[tx.ConfdbName]; !ok { + return nil, nil, fmt.Errorf("cannot read confdb system/%s: no internal handler", view.Schema().Name) + } } ts := state.NewTaskSet() @@ -992,7 +1011,7 @@ func createLoadConfdbTasks(st *state.State, tx *Transaction, view *confdb.View, } // there must be least one load-view hook if we're accessing ephemeral data - if hookPrefix == "load-view-" && mightAffectEph && !loadViewHookPresent { + if hookPrefix == "load-view-" && mightAffectEph && !loadViewHookPresent && !view.Schema().IsSystem() { return nil, nil, fmt.Errorf("cannot schedule tasks to read view %s: read might cover ephemeral data but no custodian has a load-view hook", view.ID()) } } diff --git a/overlord/confdbstate/confdbstate_test.go b/overlord/confdbstate/confdbstate_test.go index 310480e2134..b29be4bc4a7 100644 --- a/overlord/confdbstate/confdbstate_test.go +++ b/overlord/confdbstate/confdbstate_test.go @@ -31,6 +31,7 @@ import ( "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/asserts/assertstest" + "github.com/snapcore/snapd/asserts/sysdb" "github.com/snapcore/snapd/client" "github.com/snapcore/snapd/confdb" "github.com/snapcore/snapd/dirs" @@ -41,6 +42,7 @@ import ( "github.com/snapcore/snapd/overlord" "github.com/snapcore/snapd/overlord/assertstate" "github.com/snapcore/snapd/overlord/assertstate/assertstatetest" + valset_confdb "github.com/snapcore/snapd/overlord/assertstate/confdb" "github.com/snapcore/snapd/overlord/confdbstate" "github.com/snapcore/snapd/overlord/configstate/config" "github.com/snapcore/snapd/overlord/hookstate" @@ -63,6 +65,8 @@ type confdbTestSuite struct { dbSchema *confdb.Schema otherSchema *confdb.Schema devAccID string + + restoreDeviceCtx func() } var _ = Suite(&confdbTestSuite{}) @@ -199,6 +203,13 @@ func (s *confdbTestSuite) SetUpTest(c *C) { confdbstate.ResetBlockingSignals() } +func (s *confdbTestSuite) TearDownTest(c *C) { + if s.restoreDeviceCtx != nil { + s.restoreDeviceCtx() + s.restoreDeviceCtx = nil + } +} + func parsePath(c *C, path string) []confdb.Accessor { opts := confdb.ParseOptions{AllowPlaceholders: true} accs, err := confdb.ParsePathIntoAccessors(path, opts) @@ -1065,7 +1076,7 @@ func (s *confdbTestSuite) TestGetStoredTransaction(c *C) { c.Assert(err, IsNil) c.Assert(val, Equals, "bar") - c.Assert(tx.Clear(s.state), IsNil) + c.Assert(tx.Reset(s.state), IsNil) commitTask.Set("confdb-transaction", tx) } } @@ -1265,7 +1276,7 @@ func (s *confdbTestSuite) checkSetConfdbChange(c *C, chg *state.Change, hooks *[ err = s.state.Get("confdb-tx-commits", &txCommits) c.Assert(err, testutil.ErrorIs, &state.NoStateError{}) - err = tx.Clear(s.state) + err = tx.Reset(s.state) c.Assert(err, IsNil) // was committed (otherwise would've been removed by Clear) @@ -3228,3 +3239,415 @@ func (s *confdbTestSuite) TestAPIBlockingAccessTimedOutRacesWithUnblock(c *C) { c.Fatal("expected access to block but timed out") } } + +func (s *confdbTestSuite) TestReadSystemConfdbValidationSets(c *C) { + s.state.Lock() + defer s.state.Unlock() + + s.setupSystemConfdbValidationSets(c) + + view, err := confdbstate.GetView(s.state, "system", "validation-sets", "state") + c.Assert(err, IsNil) + + chgID, err := confdbstate.ReadConfdb(context.Background(), s.state, view, []string{"my-account.my-set"}, nil, confdb.AdminAccess) + c.Assert(err, IsNil) + + chg := s.state.Change(chgID) + c.Assert(chg, NotNil) + c.Assert(chg.Status(), Equals, state.DoneStatus) + c.Assert(chg.Tasks(), HasLen, 0) + + var apiData map[string]any + err = chg.Get("api-data", &apiData) + c.Assert(err, IsNil) + + vals, ok := apiData["values"] + c.Assert(ok, Equals, true) + c.Assert(vals, DeepEquals, map[string]any{ + "my-account.my-set": map[string]any{ + "mode": "enforce", + "status": "invalid", + "pinned-sequence": float64(5), + "sequence": float64(3), + "revision": float64(1), + "snaps": []any{ + map[string]any{ + "name": "my-snap", + "id": "yOqKhntON3vR7kwEbVPsILm7bUViPDzx", + "presence": "required", + }, + map[string]any{ + "name": "other-snap", + "id": "zOqKhntON3vR7kwEbVPsILm7bUViPDzy", + "presence": "optional", + "revision": float64(7), + }, + }, + }, + }) +} + +func (s *confdbTestSuite) TestWriteSystemConfdbValidationSetsWithObserveViewHook(c *C) { + s.state.Lock() + defer s.state.Unlock() + + s.setupSystemConfdbValidationSets(c) + + // set up a snap connected to system/validation-sets/state with an observe-view hook + repo := interfaces.NewRepository() + ifacerepo.Replace(s.state, repo) + + confdbIface := &ifacetest.TestInterface{InterfaceName: "confdb"} + err := repo.AddInterface(confdbIface) + c.Assert(err, IsNil) + + const coreYaml = `name: core +version: 1 +type: os +slots: + confdb-slot: + interface: confdb +` + coreInfo := mockInstalledSnap(c, s.state, coreYaml, nil) + coreSet, err := interfaces.NewSnapAppSet(coreInfo, nil) + c.Assert(err, IsNil) + err = repo.AddAppSet(coreSet) + c.Assert(err, IsNil) + + const observerYaml = `name: observer-snap +version: 1 +type: app +plugs: + state: + interface: confdb + account: system + view: validation-sets/state +` + hooks := []string{"observe-view-state"} + observerInfo := mockInstalledSnap(c, s.state, observerYaml, hooks) + observerInfo.Hooks["observe-view-state"] = &snap.HookInfo{ + Name: "observe-view-state", + Snap: observerInfo, + } + + observerSet, err := interfaces.NewSnapAppSet(observerInfo, nil) + c.Assert(err, IsNil) + err = repo.AddAppSet(observerSet) + c.Assert(err, IsNil) + + _, err = repo.Connect(&interfaces.ConnRef{ + PlugRef: interfaces.PlugRef{Snap: "observer-snap", Name: "state"}, + SlotRef: interfaces.SlotRef{Snap: "core", Name: "confdb-slot"}, + }, nil, nil, nil, nil, nil) + c.Assert(err, IsNil) + + view, err := confdbstate.GetView(s.state, "system", "validation-sets", "admin") + c.Assert(err, IsNil) + + // make sure the hook observes the right thing + var observeViewCalled bool + restore := hookstate.MockRunHook(func(ctx *hookstate.Context, _ *tomb.Tomb) ([]byte, error) { + if ctx.HookName() != "observe-view-state" { + return nil, nil + } + + ctx.State().Lock() + defer ctx.State().Unlock() + + req := []string{"my-account.my-set"} + tx, err := confdbstate.ReadConfdbFromSnap(ctx, view, req, nil, nil) + if err != nil { + return nil, err + } + + res, err := confdbstate.GetViaView(tx, view, req, nil, confdb.AdminAccess) + if err != nil { + return nil, err + } + c.Check(res, DeepEquals, map[string]any{ + "my-account.my-set": map[string]any{ + "mode": "monitor", + "status": "invalid", + "pinned-sequence": float64(4), + // check that the sequence was updated by the subsystem handler and commit + // then updated the transaction object + "sequence": float64(4), + "revision": float64(1), + "snaps": []any{ + map[string]any{ + "name": "pinned-snap", + "id": "aOqKhntON3vR7kwEbVPsILm7bUViPDzz", + "presence": "required", + }, + }, + }, + }) + + observeViewCalled = true + return nil, nil + }) + defer restore() + + chgID, err := confdbstate.WriteConfdb(nil, s.state, view, map[string]any{ + "my-account.my-set.mode": "monitor", + "my-account.my-set.pinned-sequence": 4, + }) + c.Assert(err, IsNil) + + s.state.Unlock() + err = s.o.Settle(5 * time.Second) + s.state.Lock() + c.Assert(err, IsNil) + + chg := s.state.Change(chgID) + c.Assert(chg.Status(), Equals, state.DoneStatus) + + // change propagated to val set state + var tr assertstate.ValidationSetTracking + err = assertstate.GetValidationSet(s.state, "my-account", "my-set", &tr) + c.Assert(err, IsNil) + c.Check(tr.PinnedAt, Equals, 4) + c.Check(tr.Current, Equals, 4) + c.Check(tr.Mode, Equals, assertstate.Monitor) + + // make sure the hook was called and its assertions ran + c.Assert(observeViewCalled, Equals, true) +} + +// setup an assertion DB with the builtin system/validation-sets confdb-schema +// and a my-account/my-set validation set. Also sets up the val sets confdb +// handler and seeds some initial data. +func (s *confdbTestSuite) setupSystemConfdbValidationSets(c *C) { + storeSigning := assertstest.NewStoreStack("canonical", nil) + db, err := asserts.OpenDatabase(&asserts.DatabaseConfig{ + Backstore: asserts.NewMemoryBackstore(), + Trusted: storeSigning.Trusted, + OtherPredefined: asserts.Builtin(), + }) + c.Assert(err, IsNil) + c.Assert(db.Add(storeSigning.StoreAccountKey("")), IsNil) + + devAcc := assertstest.NewAccount(storeSigning, "my-account-user", map[string]any{ + "account-id": "my-account", + }, "") + c.Assert(db.Add(devAcc), IsNil) + + devPrivKey, _ := assertstest.GenerateKey(752) + devAccKey := assertstest.NewAccountKey(storeSigning, devAcc, nil, devPrivKey.PublicKey(), "") + c.Assert(db.Add(devAccKey), IsNil) + + devSigning := assertstest.NewSigningDB("my-account", devPrivKey) + vs, err := devSigning.Sign(asserts.ValidationSetType, map[string]any{ + "series": "16", + "account-id": "my-account", + "authority-id": "my-account", + "name": "my-set", + "sequence": "3", + "revision": "1", + "timestamp": "2030-11-06T09:16:26Z", + "snaps": []any{ + map[string]any{ + "id": "yOqKhntON3vR7kwEbVPsILm7bUViPDzx", + "name": "my-snap", + "presence": "required", + }, + map[string]any{ + "id": "zOqKhntON3vR7kwEbVPsILm7bUViPDzy", + "name": "other-snap", + "presence": "optional", + "revision": "7", + }, + }, + }, nil, "") + c.Assert(err, IsNil) + c.Assert(db.Add(vs), IsNil) + + vs, err = devSigning.Sign(asserts.ValidationSetType, map[string]any{ + "series": "16", + "account-id": "my-account", + "authority-id": "my-account", + "name": "my-set", + "sequence": "4", + "revision": "1", + "timestamp": "2030-11-06T09:17:26Z", + "snaps": []any{ + map[string]any{ + "id": "aOqKhntON3vR7kwEbVPsILm7bUViPDzz", + "name": "pinned-snap", + "presence": "required", + }, + }, + }, nil, "") + c.Assert(err, IsNil) + c.Assert(db.Add(vs), IsNil) + assertstate.ReplaceDB(s.state, db) + + deviceCtx := &snapstatetest.TrivialDeviceContext{ + DeviceModel: sysdb.GenericClassicModel(), + CtxStore: &assertstatetest.FakeStore{State: s.state, DB: storeSigning}, + } + s.restoreDeviceCtx = snapstatetest.MockDeviceContext(deviceCtx) + s.state.Set("seeded", true) + + confdbstate.RegisterConfdbHandler(&valset_confdb.ValsetsConfdbHandler{}) + + assertstate.UpdateValidationSet(s.state, &assertstate.ValidationSetTracking{ + AccountID: "my-account", + Name: "my-set", + Mode: assertstate.Enforce, + PinnedAt: 5, + Current: 3, + }) +} + +type mockConfdbHandler struct { + c *C + + commitFunc func(st *state.State, tx *confdbstate.Transaction) ([]*state.TaskSet, error) +} + +func (h *mockConfdbHandler) SchemaName() string { + return "validation-sets" +} + +func (h *mockConfdbHandler) Commit(st *state.State, tx *confdbstate.Transaction) ([]*state.TaskSet, error) { + if h.commitFunc != nil { + return h.commitFunc(st, tx) + } + return nil, nil +} + +func (h *mockConfdbHandler) Databag(st *state.State) (confdb.JSONDatabag, error) { + bag := confdb.NewJSONDatabag() + bag.Set(parsePath(h.c, "v1.my-account.my-set.mode"), "enforce") + bag.Set(parsePath(h.c, "v1.my-account.my-set.sequence"), 3) + return bag, nil +} + +func (s *confdbTestSuite) TestSystemConfdbAsyncCommit(c *C) { + s.state.Lock() + defer s.state.Unlock() + + s.setupSystemConfdbValidationSets(c) + ifacerepo.Replace(s.state, interfaces.NewRepository()) + + var asyncTaskRan bool + s.o.TaskRunner().AddHandler("mock-async-work", func(t *state.Task, _ *tomb.Tomb) error { + asyncTaskRan = true + return nil + }, nil) + + handler := &mockConfdbHandler{ + c: c, + commitFunc: func(st *state.State, tx *confdbstate.Transaction) ([]*state.TaskSet, error) { + asyncTask := st.NewTask("mock-async-work", "async work from commit handler") + return []*state.TaskSet{state.NewTaskSet(asyncTask)}, nil + }, + } + + // overwrite validation-sets handler so we can "use" the validation-sets + // confdb-schema and not have to mock another one + confdbstate.RegisterConfdbHandler(handler) + + view, err := confdbstate.GetView(s.state, "system", "validation-sets", "admin") + c.Assert(err, IsNil) + + chgID, err := confdbstate.WriteConfdb(nil, s.state, view, map[string]any{"my-account.my-set.pinned-sequence": 10}) + c.Assert(err, IsNil) + + s.state.Unlock() + err = s.o.Settle(5 * time.Second) + s.state.Lock() + c.Assert(err, IsNil) + + chg := s.state.Change(chgID) + c.Assert(chg, NotNil) + + // check commit scheduled the async tasks + c.Check(asyncTaskRan, Equals, true) + c.Check(chg.Status(), Equals, state.DoneStatus) +} + +func (s *confdbTestSuite) TestSystemConfdbAsyncCommitTaskError(c *C) { + s.state.Lock() + defer s.state.Unlock() + + restore := confdbstate.MockCommitTimeout(time.Millisecond) + defer restore() + + s.setupSystemConfdbValidationSets(c) + ifacerepo.Replace(s.state, interfaces.NewRepository()) + + s.o.TaskRunner().AddHandler("mock-async-fail", func(t *state.Task, _ *tomb.Tomb) error { + return fmt.Errorf("async task failed") + }, nil) + + handler := &mockConfdbHandler{ + c: c, + commitFunc: func(st *state.State, tx *confdbstate.Transaction) ([]*state.TaskSet, error) { + asyncTask := st.NewTask("mock-async-fail", "async work that fails") + return []*state.TaskSet{state.NewTaskSet(asyncTask)}, nil + }, + } + confdbstate.RegisterConfdbHandler(handler) + + view, err := confdbstate.GetView(s.state, "system", "validation-sets", "admin") + c.Assert(err, IsNil) + + chgID, err := confdbstate.WriteConfdb(nil, s.state, view, map[string]any{"my-account.my-set.pinned-sequence": 10}) + c.Assert(err, IsNil) + + s.state.Unlock() + err = s.o.Settle(5 * time.Second) + s.state.Lock() + c.Assert(err, IsNil) + + chg := s.state.Change(chgID) + c.Assert(chg, NotNil) + + c.Check(chg.Status(), Equals, state.ErrorStatus) + var commitTask *state.Task + for _, t := range chg.Tasks() { + if t.Kind() == "commit-confdb-tx" { + commitTask = t + break + } + } + c.Assert(commitTask, NotNil) + c.Assert(commitTask.Status(), Equals, state.HoldStatus) +} + +func (s *confdbTestSuite) TestSystemConfdbSyncCommit(c *C) { + s.state.Lock() + defer s.state.Unlock() + + s.setupSystemConfdbValidationSets(c) + ifacerepo.Replace(s.state, interfaces.NewRepository()) + + var commitCalled bool + handler := &mockConfdbHandler{ + c: c, + commitFunc: func(*state.State, *confdbstate.Transaction) ([]*state.TaskSet, error) { + commitCalled = true + return nil, nil + }, + } + confdbstate.RegisterConfdbHandler(handler) + + view, err := confdbstate.GetView(s.state, "system", "validation-sets", "admin") + c.Assert(err, IsNil) + + chgID, err := confdbstate.WriteConfdb(nil, s.state, view, map[string]any{"my-account.my-set.pinned-sequence": 10}) + c.Assert(err, IsNil) + + s.state.Unlock() + err = s.o.Settle(5 * time.Second) + s.state.Lock() + c.Assert(err, IsNil) + + chg := s.state.Change(chgID) + c.Assert(chg, NotNil) + + c.Check(commitCalled, Equals, true) + c.Check(chg.Status(), Equals, state.DoneStatus) +} diff --git a/overlord/confdbstate/export_test.go b/overlord/confdbstate/export_test.go index d61445b98fd..1e846d3d978 100644 --- a/overlord/confdbstate/export_test.go +++ b/overlord/confdbstate/export_test.go @@ -94,3 +94,7 @@ func GetOngoingTxs(st *state.State, account, schemaName string) (ongoingTxs *con func MockFetchConfdbSchemaAssertion(f func(*state.State, int, string, string) error) func() { return testutil.Mock(&AssertstateFetchConfdbSchemaAssertion, f) } + +func MockCommitTimeout(ct time.Duration) func() { + return testutil.Mock(&commitTimeout, ct) +} diff --git a/overlord/confdbstate/transaction.go b/overlord/confdbstate/transaction.go index 23ce1676599..5dceb9eac8d 100644 --- a/overlord/confdbstate/transaction.go +++ b/overlord/confdbstate/transaction.go @@ -230,7 +230,7 @@ func (t *Transaction) Commit(st *state.State, schema confdb.DatabagSchema) error return nil } -func (t *Transaction) Clear(st *state.State) error { +func (t *Transaction) Reset(st *state.State) error { t.mu.Lock() defer t.mu.Unlock() diff --git a/overlord/confdbstate/transaction_test.go b/overlord/confdbstate/transaction_test.go index 4a7b1230c88..ac7e2224eaa 100644 --- a/overlord/confdbstate/transaction_test.go +++ b/overlord/confdbstate/transaction_test.go @@ -484,7 +484,7 @@ func (s *transactionTestSuite) TestAbortPreventsReadsAndWrites(c *C) { err = tx.Set(parsePath(c, "foo"), "bar") c.Assert(err, ErrorMatches, "cannot write to aborted transaction") - err = tx.Clear(s.state) + err = tx.Reset(s.state) c.Assert(err, ErrorMatches, "cannot write to aborted transaction") err = tx.Unset(parsePath(c, "foo"))