many: some more docs and improvements for readability

Author: Meulengracht

boot/assets_piboot.go

@@ -38,8 +38,16 @@ func NewPibootConfigUpdateObserver() gadget.ContentUpdateObserver {
 	return &pibootConfigUpdateObserver{}
 }
 
+func isPibootConfigUpdate(op gadget.ContentOperation, partRole, relativeTarget string) bool {
+	// We only care about updates to config.txt in the system seed,
+	// and only if there is a before value to check for an os_prefix.
+	return op == gadget.ContentUpdate &&
+		(partRole == gadget.SystemSeed || partRole == gadget.SystemSeedNull) &&
+		relativeTarget == "config.txt"
+}
+
 func (o *pibootConfigUpdateObserver) Observe(op gadget.ContentOperation, partRole, root, relativeTarget string, data *gadget.ContentChange) (gadget.ContentChangeAction, error) {
-	if op != gadget.ContentUpdate || (partRole != gadget.SystemSeed && partRole != gadget.SystemSeedNull) || relativeTarget != "config.txt" || data == nil || data.Before == "" {
+	if !isPibootConfigUpdate(op, partRole, relativeTarget) || data == nil || data.Before == "" {
 		return gadget.ChangeApply, nil
 	}
 

gadget/update_observer.go

@@ -26,8 +26,13 @@ type compositeContentUpdateObserver struct {
 var _ ContentUpdateObserver = (*compositeContentUpdateObserver)(nil)
 
 // NewCompositeContentUpdateObserver combines multiple content update observers
-// into one. Nil observers are ignored. When a single non-nil observer is
-// provided, it is returned unchanged.
+// into one. Ignores nil observers. If no observers are provided, returns nil.
+// If a single non-nil observer is provided, it is returned unchanged.
+//
+// The composite Observe method calls observers in order. It returns
+// immediately on the first error or ChangeAbort. ChangeIgnore is sticky but
+// does not stop iteration: remaining observers are still called, and the
+// final result is ChangeIgnore unless a later observer aborts or errors.
 func NewCompositeContentUpdateObserver(observers ...ContentUpdateObserver) ContentUpdateObserver {
 	filtered := make([]ContentUpdateObserver, 0, len(observers))
 	for _, observer := range observers {

gadget/update_test.go

@@ -754,6 +754,35 @@ func (u *updateTestSuite) TestNewCompositeContentUpdateObserver(c *C) {
 	c.Check(obs2.doneCalled, Equals, 1)
 }
 
+func (u *updateTestSuite) TestNewCompositeContentUpdateObserverObserveContinuesAfterIgnore(c *C) {
+	obs1 := &mockPostApplyObserver{observeAction: gadget.ChangeIgnore}
+	obs2 := &mockPostApplyObserver{observeAction: gadget.ChangeApply}
+
+	combined := gadget.NewCompositeContentUpdateObserver(obs1, obs2)
+
+	action, err := combined.Observe(gadget.ContentUpdate, gadget.SystemBoot, "/run/mnt/ubuntu-boot", "config.txt", nil)
+	c.Assert(err, IsNil)
+	c.Check(action, Equals, gadget.ChangeIgnore)
+	c.Check(obs1.observeCalled, Equals, 1)
+	c.Check(obs2.observeCalled, Equals, 1)
+}
+
+func (u *updateTestSuite) TestNewCompositeContentUpdateObserverObserveStopsOnAbort(c *C) {
+	expectedErr := errors.New("boom")
+	obs1 := &mockPostApplyObserver{observeAction: gadget.ChangeApply}
+	obs2 := &mockPostApplyObserver{observeAction: gadget.ChangeAbort, observeErr: expectedErr}
+	obs3 := &mockPostApplyObserver{observeAction: gadget.ChangeApply}
+
+	combined := gadget.NewCompositeContentUpdateObserver(obs1, obs2, obs3)
+
+	action, err := combined.Observe(gadget.ContentUpdate, gadget.SystemBoot, "/run/mnt/ubuntu-boot", "config.txt", nil)
+	c.Assert(err, Equals, expectedErr)
+	c.Check(action, Equals, gadget.ChangeAbort)
+	c.Check(obs1.observeCalled, Equals, 1)
+	c.Check(obs2.observeCalled, Equals, 1)
+	c.Check(obs3.observeCalled, Equals, 0)
+}
+
 func (u *updateTestSuite) TestUpdateApplyHappy(c *C) {
 	oldData, newData, rollbackDir := u.updateDataSet(c)
 	// update two structs

overlord/devicestate/handlers_gadget.go

@@ -199,7 +199,11 @@ func (m *DeviceManager) doUpdateGadgetAssets(t *state.Task, _ *tomb.Tomb) error
 		if err == nil {
 			observers = append(observers, observeTrustedBootAssets)
 		}
-		if snapsup.Type == snap.TypeGadget && currentData.Info != nil && currentData.Info.HasBootloader("piboot") {
+
+		// On raspberry pi devices with piboot, we need to observe updates to config.txt
+		// in the system seed so that we can extract the os_prefix and preserve the value
+		// across gadget updates.
+		if snapsup.Type == snap.TypeGadget && currentData.Info.HasBootloader("piboot") {
 			observers = append(observers, boot.NewPibootConfigUpdateObserver())
 		}