Skip to content

secboot: complete the decision for auto-repair/reprovision - #17386

Open
valentindavid wants to merge 4 commits into
canonical:masterfrom
valentindavid:valentindavid/status-api-reprovision
Open

secboot: complete the decision for auto-repair/reprovision#17386
valentindavid wants to merge 4 commits into
canonical:masterfrom
valentindavid:valentindavid/status-api-reprovision

Conversation

@valentindavid

Copy link
Copy Markdown
Member

No description provided.

@valentindavid valentindavid added the Run nested The PR also runs tests inluded in nested suite label Jul 20, 2026
@github-actions github-actions Bot added the Run only one system Only runs spread tests on one system label Jul 20, 2026
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Fri Jul 24 17:04:39 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/30100630514

Failures:

Preparing:

  • openstack-ext:ubuntu-18.04-64:tests/nested/classic/snapshots-with-core-refresh-revert
  • openstack-ext:ubuntu-18.04-64:tests/nested/classic/hotplug
  • openstack-ext:ubuntu-20.04-64:tests/nested/core/interfaces-custom-devices
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/snapd-removes-vulnerable-snap-confine-revs:snapd
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/snapd-removes-vulnerable-snap-confine-revs:core
  • openstack-arm-ext:ubuntu-24.04-arm-64:tests/nested/manual/optee-fde:hooks
  • openstack-arm-ext:ubuntu-24.04-arm-64:tests/nested/manual/optee-fde:optee
  • openstack-arm-ext:ubuntu-24.04-arm-64:tests/nested/core/core20-basic
  • openstack:ubuntu-20.04-64:tests/main/nss-modules:nis

Executing:

  • openstack:debian-12-64:tests/main/cgroup-devices-v2
  • openstack-ext:ubuntu-20.04-64:tests/nested/core/core20-tpm
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/recovery-system-reboot:install
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/core-factory-reset-new-secboot:tpm
  • openstack-ext:ubuntu-22.04-64:tests/nested/core/core20-fault-inject-on-refresh:gadget_reboot_link_snap
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/base-kernel-refresh-rollback
  • openstack-ext:ubuntu-24.04-64:tests/nested/manual/core-recover-from-recovery:break_dbx
  • openstack-ext:ubuntu-24.04-64:tests/nested/manual/recovery-system-reboot:install
  • openstack-ext:ubuntu-24.04-64:tests/nested/manual/core-recover-from-recovery:clear_tpm
  • openstack-ext:ubuntu-26.04-64:tests/nested/core/core20-factory-reset:fde
  • openstack-ext:ubuntu-26.04-64:tests/nested/core/core20-gadget-reseal
  • openstack-ext:ubuntu-26.04-64:tests/nested/core/core20-kernel-reseal
  • openstack:ubuntu-core-18-64:tests/main/interfaces-snapd-control-with-manage
  • openstack:ubuntu-core-20-64:tests/core/snapd-maintenance-msg
  • openstack:ubuntu-26.04-64:tests/main/auto-refresh-pre-download:restart

Restoring:

  • openstack:ubuntu-core-20-64:tests/core/snapd-maintenance-msg
  • openstack:ubuntu-core-20-64:tests/core/
  • openstack:ubuntu-core-20-64:

Skipped tests from snapd-testing-skip

If you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list of the below tests you wish to run (unskip plus test list must be valid yaml)

  • openstack:ubuntu-24.04-64:tests/main/i18n
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-flag-restart
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-prompt-restoration
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-snapd-startup
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-support
  • openstack:ubuntu-26.04-64:tests/main/i18n
  • openstack:ubuntu-26.04-64:tests/main/interfaces-requests-activates-handlers

@valentindavid
valentindavid force-pushed the valentindavid/status-api-reprovision branch from 760871f to a8ad444 Compare July 21, 2026 13:29
@github-actions github-actions Bot added the Needs Documentation -auto- Label automatically added which indicates the change needs documentation label Jul 21, 2026
@valentindavid
valentindavid force-pushed the valentindavid/status-api-reprovision branch 3 times, most recently from 2a42006 to ab5c05f Compare July 24, 2026 06:42
@github-actions github-actions Bot removed the Needs Documentation -auto- Label automatically added which indicates the change needs documentation label Jul 24, 2026
@valentindavid
valentindavid requested a review from pedronis July 24, 2026 06:42
@valentindavid
valentindavid marked this pull request as ready for review July 24, 2026 06:42
@github-actions github-actions Bot removed the Run only one system Only runs spread tests on one system label Jul 24, 2026
@valentindavid
valentindavid force-pushed the valentindavid/status-api-reprovision branch from ab5c05f to a1e889f Compare July 24, 2026 07:21
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.20000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.85%. Comparing base (f88962f) to head (08945cd).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
secboot/secboot_sb.go 88.04% 10 Missing and 1 partial ⚠️
overlord/fdestate/autorepair.go 88.88% 2 Missing and 1 partial ⚠️
secboot/secboot_nosb.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #17386      +/-   ##
==========================================
+ Coverage   78.84%   78.85%   +0.01%     
==========================================
  Files        1411     1407       -4     
  Lines      196727   196843     +116     
  Branches     2462     2462              
==========================================
+ Hits       155111   155225     +114     
- Misses      32280    32283       +3     
+ Partials     9336     9335       -1     
Flag Coverage Δ
unittests 78.85% <87.20%> (+0.01%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@valentindavid
valentindavid force-pushed the valentindavid/status-api-reprovision branch from a1e889f to baf70d4 Compare July 24, 2026 08:00

@pedronis pedronis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small initial comment

Comment thread secboot/secboot.go
DiskModel() string
}

type RemedialActions struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can give a doc comment to this? and also to the fields?

@ZeyadYasser ZeyadYasser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first pass

Comment thread secboot/secboot.go Outdated
DiskModel() string
}

// RemedialActions is a set of action recommended to repair detected

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// RemedialActions is a set of action recommended to repair detected
// RemedialActions is a set of actions recommended to repair detected

Comment thread secboot/secboot.go Outdated
// If the attempt fails, then it means a reprovision is required.
AttemptRepair bool
// RequireReprovision tells whether issues require a reprovision
// and it was detected that auto repair would not be enough.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// and it was detected that auto repair would not be enough.
// and it was detected that auto-repair would not be enough.

type repairState struct {
Result AutoRepairResult `json:"result"`
Result AutoRepairResult `json:"result"`
Recommendations []RecommendedRemedialAction `json:"recommendations,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense for future proofing to make it map[RecommendedRemedialAction]any? so that if we need to augment an action with args we could because we will be stuck with that API unless we deprecate the field.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to add "input" to the action? We can add extra fields for them.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would we add extra fields if it's a list of strings?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean something like: recommendation-args?

@ZeyadYasser ZeyadYasser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you! only a remark on recommendation args

type repairState struct {
Result AutoRepairResult `json:"result"`
Result AutoRepairResult `json:"result"`
Recommendations []RecommendedRemedialAction `json:"recommendations,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would we add extra fields if it's a list of strings?

type repairState struct {
Result AutoRepairResult `json:"result"`
Result AutoRepairResult `json:"result"`
Recommendations []RecommendedRemedialAction `json:"recommendations,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean something like: recommendation-args?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Run nested The PR also runs tests inluded in nested suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants