o/snapstate, o/devicestate: remodel with components from the store #14933

andrewphelpsj · 2025-01-15T19:36:58Z

This change enables us to do a remodel that includes components from the store. This is a little incomplete, as I discovered that I need to handle the case where we swap to an already installed kernel (leftover from a previous remodeling) that may have some components already installed. This is a small edge case, and will not change most of the PR.

Based on #14939 for now.

github-actions · 2025-01-15T21:34:05Z

Fri Jan 31 23:04:53 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/13075034459

Failures:

Preparing:

openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64

Executing:

openstack:debian-12-64:tests/upgrade/basic
google-nested:ubuntu-24.04-64:tests/nested/manual/remodel-min-size

Restoring:

openstack:debian-12-64:tests/upgrade/basic
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64
openstack:opensuse-tumbleweed-64

codecov · 2025-01-16T19:24:03Z

Codecov Report

Attention: Patch coverage is 78.99160% with 25 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@81fd929). Learn more about missing BASE report.
Report is 64 commits behind head on master.

Files with missing lines	Patch %	Lines
overlord/devicestate/devicestate.go	78.81%	16 Missing and 9 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #14933   +/-   ##
=========================================
  Coverage          ?   78.22%           
=========================================
  Files             ?     1163           
  Lines             ?   154159           
  Branches          ?        0           
=========================================
  Hits              ?   120597           
  Misses            ?    26132           
  Partials          ?     7430

Flag	Coverage Δ
unittests	`78.22% <78.99%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andrewphelpsj · 2025-01-17T14:56:02Z

overlord/devicestate/devicestate.go

-				snapsupTask = t
-				break
-			}
+		t := ts.MaybeEdge(snapstate.SnapSetupEdge)


@pedronis here is where the new edge is used

pedronis

sorry, only couple of trivials first

pedronis · 2025-01-24T16:52:27Z

tests/nested/manual/remodel-with-components/task.yaml

+summary: create a recovery system with a kernel module component and reboot into it
+
+details: |
+  This test creates a recovery system with a kernel module component and
+  validates that the newly created system can be rebooted into.


summary and description sound off as they don't mention remodel?

pedronis · 2025-01-27T16:47:18Z

overlord/devicestate/devicestate.go

+			return nil, nil, err
+		}
+
+		if !snapsup.ComponentExclusiveSetup {


hasn't this been renamed?

pedronis

did a pass

pedronis · 2025-01-27T17:06:43Z

overlord/devicestate/devicestate.go

 	default:
 		// nothing to do but add the snap to the prereq tracker
 		r.tracker.Add(currentInfo)
 		return remodelNoAction, nil, nil
 	}
 }

-func (r *remodeler) shouldJustSwitch(rt remodelSnapTarget, needsRevisionChange bool) bool {
+func (r *remodeler) shouldSwitchWithoutRefresh(rt remodelSnapTarget, needsRevisionChange bool, needsComponentChanges bool) bool {


I'm not entirely sure if passing needsComponentChanges is more readable than checking it outside of the helper

pedronis · 2025-01-27T17:10:05Z

overlord/devicestate/devicestate.go

+				requiredComponents = append(requiredComponents, c)
+			}
+		}
+	}


maybe the two loops to compute needsComponentChanges and update requiredCompontents should be a helper?

Pushed 9c30418. Name of function might need some work.

pedronis · 2025-01-27T17:10:22Z

overlord/devicestate/devicestate.go

@@ -563,7 +616,18 @@ func (r *remodeler) maybeInstallOrUpdate(ctx context.Context, st *state.State, r
 			return remodelChannelSwitch, []*state.TaskSet{ts}, nil
 		}

-		goal, err := r.updateGoal(st, rt, constraints)
+		// right now, we don't properly handle switching a channel and


do we have a test for that combo though?

Added one in 909780f.

pedronis

thank you

alfonsosanchezbeato

Thanks, some minor things

tests/nested/manual/remodel-with-components/task.yaml

alfonsosanchezbeato · 2025-01-28T20:31:54Z

tests/nested/manual/remodel-with-components/task.yaml

+        exit
+    fi
+
+    snap install test-snapd-swtpm --edge


Isn't this installed by nested_start_core_vm_unit()?

It is, but it won't work there since we've already set up the fake store at that point. I'll add a comment.

tests/nested/manual/remodel-with-components/task.yaml

overlord/devicestate/devicestate.go

alfonsosanchezbeato

LGTM, thanks for the changes

andrewphelpsj · 2025-01-31T14:34:11Z

Only required failure is google-nested:ubuntu-24.04-64:tests/nested/manual/remodel-min-size, which is a known issue and not related to this change.

… and DownloadComponents

andrewphelpsj force-pushed the remodel-components-from-store branch from fb52d67 to 7e4bd1c Compare January 15, 2025 21:28

andrewphelpsj requested review from pedronis and alfonsosanchezbeato January 15, 2025 21:28

andrewphelpsj force-pushed the remodel-components-from-store branch 2 times, most recently from 9d70ca3 to f291bf0 Compare January 16, 2025 19:06

andrewphelpsj commented Jan 17, 2025

View reviewed changes

andrewphelpsj force-pushed the remodel-components-from-store branch from f291bf0 to 8acd511 Compare January 23, 2025 22:02

pedronis reviewed Jan 27, 2025

View reviewed changes

andrewphelpsj force-pushed the remodel-components-from-store branch from 8acd511 to 0dd1396 Compare January 27, 2025 16:56

pedronis self-requested a review January 27, 2025 17:00

pedronis reviewed Jan 27, 2025

View reviewed changes

andrewphelpsj requested a review from pedronis January 27, 2025 18:07

pedronis approved these changes Jan 28, 2025

View reviewed changes

alfonsosanchezbeato reviewed Jan 28, 2025

View reviewed changes

andrewphelpsj requested a review from alfonsosanchezbeato January 29, 2025 18:32

andrewphelpsj added the Run nested The PR also runs tests inluded in nested suite label Jan 29, 2025

alfonsosanchezbeato approved these changes Jan 29, 2025

View reviewed changes

andrewphelpsj force-pushed the remodel-components-from-store branch from e158aaf to fddca0c Compare January 30, 2025 13:55

andrewphelpsj added 2 commits January 31, 2025 09:46

o/devicestate, tests: handle components during a remodel

4905bda

o/snapstate: add SnapSetupEdge on snap setup task created in Download…

44cc1d2

… and DownloadComponents

andrewphelpsj force-pushed the remodel-components-from-store branch from a7744b5 to 44cc1d2 Compare January 31, 2025 14:46

alfonsosanchezbeato merged commit 4be46eb into canonical:master Feb 4, 2025
64 of 67 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

o/snapstate, o/devicestate: remodel with components from the store #14933

o/snapstate, o/devicestate: remodel with components from the store #14933

andrewphelpsj commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

codecov bot commented Jan 16, 2025 •

edited

Loading

andrewphelpsj Jan 17, 2025

pedronis left a comment

pedronis Jan 24, 2025

pedronis Jan 27, 2025

pedronis left a comment

pedronis Jan 27, 2025

pedronis Jan 27, 2025

andrewphelpsj Jan 27, 2025

pedronis Jan 27, 2025

andrewphelpsj Jan 27, 2025

pedronis left a comment

alfonsosanchezbeato left a comment

alfonsosanchezbeato Jan 28, 2025

andrewphelpsj Jan 29, 2025

alfonsosanchezbeato left a comment

andrewphelpsj commented Jan 31, 2025

o/snapstate, o/devicestate: remodel with components from the store #14933

o/snapstate, o/devicestate: remodel with components from the store #14933

Conversation

andrewphelpsj commented Jan 15, 2025 • edited Loading

github-actions bot commented Jan 15, 2025 • edited Loading

Failures:

Preparing:

Executing:

Restoring:

codecov bot commented Jan 16, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

pedronis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedronis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedronis left a comment

Choose a reason for hiding this comment

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

andrewphelpsj commented Jan 31, 2025

andrewphelpsj commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

codecov bot commented Jan 16, 2025 •

edited

Loading