o/ifacestate: record warning if prompting errors during startup #14838
Conversation
If an error occurs during interfaces requests manager initialization, record a warning containing the error and indicating that prompting will remain inactive until snapd is restarted. Even when an error occurs, the `"apparmor-prompting"` feature flag is left enabled, as the user deliberately turned this flag on, and we don't want to change feature flags without user interaction. Signed-off-by: Oliver Calder <[email protected]>
olivercalder
added
Simple 😃
|
@pedronis please advise on the warning text. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14838 +/- ##
==========================================
+ Coverage 78.20% 78.25% +0.04%
==========================================
Files 1151 1155 +4
Lines 151396 152038 +642
==========================================
+ Hits 118402 118975 +573
- Misses 25662 25707 +45
- Partials 7332 7356 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
LGTM!
Regarding mutable warning, I don't think so. The only thing that could change is the error message, and I suppose different error messages would indicate different problem so mutable warnings would mask them, no? unless of course those errors bubbling up are dynamically built, in this case maybe.
| c.Check(mgr.AppArmorPromptingRunning(), Equals, true) | ||
| c.Check(mgr.InterfacesRequestsManager(), Equals, fakeManager) | ||
|
|
||
| func() { |
There was a problem hiding this comment.
is the function for the defer?
overlord/ifacestate/ifacemgr.go
Outdated
| // warning so the user knows prompting is not current running. | ||
| m.state.Lock() | ||
| defer m.state.Unlock() | ||
| m.state.AddWarning(fmt.Sprintf("failed to start prompting backend: %v\nprompting will be inactive until snapd is restarted", err), nil) |
There was a problem hiding this comment.
Thanks, I'll make the change. Just to confirm, is it alright for the warning to be multi-line (the bit about how prompting will be inactive until snapd restart on the next line, after the error), or should I shorten the warning and put it all in one line, with semicolon after the error message?
| c.Check(mgr.AppArmorPromptingRunning(), Equals, true) | ||
| c.Check(mgr.InterfacesRequestsManager(), Equals, fakeManager) | ||
|
|
||
| func() { |
Signed-off-by: Oliver Calder <[email protected]>
olivercalder
force-pushed
the
prompting-warning-on-startup-error
branch
from
9543eab to
3b7e272
Compare
|
Test failures:
|
If an error occurs during interfaces requests manager initialization, record a warning containing the error and indicating that prompting will remain inactive until snapd is restarted.
Even when an error occurs, the
"apparmor-prompting"feature flag is left enabled, as the user deliberately turned this flag on, and we don't want to change feature flags without user interaction.This work is tracked internally by: https://warthogs.atlassian.net/browse/SNAPDENG-32064