=== ijohnson|lunch is now known as ijohnson [02:02] PR snapd#9720 closed: many: rename disks.FindMatching... to FindMatching...WithFsLabel and err type [06:09] PR snapd#9719 closed: boot: set kernel command line in modeenv during install [06:14] PR snapd#9727 closed: seed: cleanup/drop some no longer valid TODOS, clarify some other points [06:53] morning [07:59] PR snapd#9725 closed: bootloader: remove installableBootloader interface and methods [08:05] pstolowski: hey [08:06] heya [08:09] PR snapd#9713 closed: tests: sign new nested-18|20* models to allow for generic serials [08:16] hey guys [08:26] zyga: hey [08:27] updated the spread test in #9724 and current_kernel_command_lines is not populated after reboot, hmm hmm [08:27] PR #9724: boot: observe successful command line update, provide a default [08:34] good morning pstolowski and mborzecki [08:35] mvo: hey [08:45] hello [08:45] pstolowski's #9409 needs 2nd reviews [08:45] PR #9409: cmd/snap: implement 'snap validate' command <⛔ Blocked> [08:46] pedronis: good morning, let me have a look [08:46] mvo: are you blocked on it for something atm? #9709 seems to need a couple of typo fixes but then could land [08:46] ty [08:46] PR #9709: snapshotstate: improve handling of multiple errors [08:47] pstolowski: 9409 is still marked as blocked, is that still accurate [08:47] pedronis: review for 9705 would be great [08:47] pedronis: otherwise not really blocked [08:47] mvo: was it updated? I missed that [08:47] mvo: kind of, i think we may want to land it close to the followups otherwise it won't do anything [08:48] pedronis: yeah, last evening [08:48] hmmmm so spread test failed, but it works in local testing in a vm [08:48] pstolowski: ok, I have a look [08:48] mvo: ah, I looked at it but never did anything active so I'm not getting emails about it, sorry [08:48] I need a second review for 9718 [08:48] pedronis: no worries [08:50] mborzecki: you have a bunch of small comments on #9724, but you likely saw that [08:50] PR #9724: boot: observe successful command line update, provide a default [08:50] pedronis: yup, trying to figure out the mystery of spread test first [08:51] mvo: at some point last nigh GH actions were failing internally? is that fixed now [08:51] *night [08:51] pedronis: I noticed this too, I restarted some PRs early in my morning, need to double check [08:52] pedronis: hm, I'm pretty sure I restarted the macos test in 9731 but it appears to be still failing for unknown/internal reasons [08:52] * mvo tries again [08:53] it has one passing and one failed [08:53] yes, same here [08:53] it's even stranger [08:53] indeed [08:53] now it's running and looking okay but still strange that we have two [08:54] mvo: do you want me to pick up the small fixes to #9709 ? [08:54] PR #9709: snapshotstate: improve handling of multiple errors [08:54] pedronis: sure, if you have time/want to I would be happy [08:54] pedronis: but only if it's no burden [08:59] PR snapd#9732 opened: asserts: snapasserts method to validate installed snaps against validation sets [09:15] pedronis: ^ [09:15] pstolowski: I saw it thx [09:16] heh, so a race between the test checking mdoeenv and snapd updating it === pedronis_ is now known as pedronis [09:23] mborzecki: this is mark successful? [09:23] pedronis: yes [09:29] mborzecki: I don't think we have a nice way to know ran Ensure at least once [09:30] pedronis: yeah, for now i'm using nested_retry_until_successful, which should be good enough [09:33] mvo: tweaks ended up slightly larger than I expected, please double check quickly: https://github.com/snapcore/snapd/pull/9709/commits/6d07df7ce8a261b032b0a963cd40881bd0134bdc [09:33] PR #9709: snapshotstate: improve handling of multiple errors [09:35] pedronis: sure thing, let me look [09:36] pedronis: nice! yeah, I like that [09:36] pedronis: thank you! [09:54] mvo hey [09:54] one job running on my infra, I'll shut down half when it finishes [09:56] pstolowski: I reviewed 9409 (probably also worth for pedronis to quickly look as I had one output question) [09:56] zyga: great, thanks [09:58] mvo: I looked at #9705, some meta questions there [09:58] PR #9705: devicestate: add runFDESetupHook() helper [09:58] mvo: thank you! [09:58] mvo: most of the output comes from the spec [09:58] but I will double check [09:59] mvo: let me know if we need to chat about 9705 [10:00] pedronis: cool, let me look at 9705, I will let you know [10:05] pstolowski: having a short break and then I will look at the validate PRs, command one, and the new one [10:07] pedronis: thanks [10:40] PR snapd#9709 closed: snapshotstate: improve handling of multiple errors [10:48] #9513 needs a master merge now [10:48] PR #9513: snapshotstate: detect duplicated snapshot imports [10:58] pedronis: thanks, will do (just looking to tweak some things in 9705) [11:44] hmm ... i'm getting popup notifications about snapd not being able to update the snap-store app ... [11:45] ogra: that's because it is in use right now [11:45] the snap-store keeps its backend permanently running even if you close the app [11:45] and there is no way for users to stop it [11:45] it isnt in use ... thats the point 🙂 [11:45] ogra: yeah that's a problem, I guess we need help from the desktop team to make this possible [11:46] the only wy to stop it is to fish the process out of the process list and kill it manually ... even if the UI is closed [11:46] pedronis, mvo i've pushed tweaks to #9409, most notably added helpers and reused regexps that already existed in asserts, and made 'snap validate' hidden so we can land it [11:46] PR #9409: cmd/snap: implement 'snap validate' command <⛔ Blocked> [11:46] mvo, but gerat to see the notification work ! [11:46] *great even [11:54] pstolowski: nice, thanks so much! [11:56] there is just one remaining comment re "invalid ..." [11:56] let me know if my suggestion makes sense [12:00] PR snapd#9731 closed: daemon: split out snapctl support and snap configuration support to their own api_*.go files [12:04] pstolowski: using %w is what we want but it makes 1.9 unhappy, you need to use fmt := on a separate line, maybe const works also [12:04] pstolowski: there are examples already [12:04] pedronis: ah, ok [12:05] right, i can see it, thanks [12:05] pstolowski, hi [12:05] when you have time, could you please take a look to #9479 [12:05] PR #9479: tests: replace pkgdb.sh (library) with tests.pkgs (program) [12:07] hi cachio, sure [12:10] * pstolowski lunch [12:12] pstolowski: thanks for the update to the PR! [12:20] pstolowski: did another pass, sorry a bit of nitpicking about names, https://github.com/snapcore/snapd/pull/9409#pullrequestreview-542769180 [12:20] PR #9409: cmd/snap: implement 'snap validate' command <⛔ Blocked> [12:24] * cachio afk [12:25] pstolowski: also I wonder if we really use name as you say here: https://github.com/snapcore/snapd/pull/9409/files#r534061494, but I commented asking about that [12:25] PR #9409: cmd/snap: implement 'snap validate' command <⛔ Blocked> [12:31] pedronis: I did the master merge for 9513 now and also updated 9705, let me know if that makes sense, if not we should proably have a quick chat before the standup ho [12:32] pstolowski: do you think you could have a look at 9718? should be pretty straightforward hopefully [12:41] mvo: 9705 looks good, but 9718 should probably land first because fdesetup gets moved [12:41] and it will be a bit messy [12:44] mvo: commented on #9705 [12:44] PR #9705: devicestate: add runFDESetupHook() helper [13:05] mvo: yes, i can take a look (after standup most likely) [13:12] pstolowski: left some initial high-level comments in #9732 , let me know if you have questions [13:12] PR #9732: asserts: snapasserts method to validate installed snaps against validation sets [13:13] pedronis: thanks, will check shortly [13:38] morning folks [13:51] PR snapd#9733 opened: tests: New queries for the os tools [14:42] #9721 is pretty simple and could use some reviews :-D [14:42] PR #9721: osutil/disks: add FindMatchingPartitionUUIDWithPartLabel to Disk iface [14:47] ijohnson: looking [14:49] thanks! [14:51] PR snapd#9734 opened: Add support additional camera/capture devices [15:15] mborzecki: #9721 maybe is something you could look at as you reviewed the other code there? [15:15] PR #9721: osutil/disks: add FindMatchingPartitionUUIDWithPartLabel to Disk iface === King_InuYasha is now known as Conan_Kudo === Conan_Kudo is now known as King_InuYasha [15:24] mborzecki: do you have a link to that forum post where you investigated why systems with many snaps are slow to boot because the mounting of the squashfs files conflicts with each other and they end up trying to get mounted in many different places? I think it would be good to at least explain to this person why snaps make their boot slow: [15:24] https://forum.snapcraft.io/t/how-do-i-stop-snaps-mounting-during-boot-on-ubuntu-18-04/21434 [15:26] ijohnson: i looked at some things here: https://forum.snapcraft.io/t/snapd-causing-lengthy-boot-time/10466/6?u=mborzecki [15:28] mborzecki: ah yes that was exactly what I was thinking about [15:28] thanks! === King_InuYasha is now known as Conan_Kudo [15:31] degville: Controlling service startup order with before/after keywords is documented for “the snap format”, but it's missing both from “apps and services metadata” and “Snapcraft.yaml format” pages. Is this intentional? If not, I'll suggest a change on the respective forum threads. === Conan_Kudo is now known as King_InuYasha [15:33] dot-tobias: no, it's not intentional. It's definitely something we should add - thanks for flagging it. [15:37] mvo: pedronis: could one of you sudo merge 9721, it has 2 +1s, but it failed due to 429s from the store [15:37] ijohnson: sure, let me do that [15:37] mvo: thanks! [15:37] btw should we be retrying 429's ? [15:37] seems like yes ? [15:37] but on a exponential backoff or something since 429 is too many requests [15:38] a second review for 9718 would be great [15:38] like this seems wrong: [15:38] retry.go:184: DEBUG: Not retrying: &errors.errorString{s:"too many requests"} [15:38] mvo: sure I will add it to my queue [15:38] ta [15:41] PR snapd#9721 closed: osutil/disks: add FindMatchingPartitionUUIDWithPartLabel to Disk iface [15:44] ijohnson: we already have logic that try to spread out catalog refresh requests but it might be a bit too naive on restarts [15:44] pedronis: well I didn't read the code, just that reading the debug log messages is what confused me. are you saying that we do catalog refreshes at a higher level than the http retry itself? [15:45] ijohnson: yes, retrying with backoff on 429 is probably not useful [15:45] because the backoff needed would be larger than anything practical [15:45] mmm fair enough [15:45] for most operations [15:46] yeah that makes sense [15:46] or at least for the sitatuons were it matters atm [15:46] this might just be hopeless to be clear atm === King_InuYasha is now known as Conan_Kudo === Conan_Kudo is now known as King_InuYasha [15:51] degville: re before/after docs https://forum.snapcraft.io/t/snapcraft-app-and-service-metadata/8335/8?u=tobias and https://forum.snapcraft.io/t/snapcraft-yaml-reference/4276/43?u=tobias [15:52] dot-tobias: thank you! I'll merge those in asap. [15:52] welcome, glad to contribute to docs as a way to say thanks 😊 [15:57] mvo: pedronis: could you also sudo merge #9729, it is very red on 429's though there is one interesting lxd failure that I have saved the logs for and is not at all related the lkenv changes there [15:57] PR #9729: bootloader/lkenv: specify backup file as arg to NewEnv(), use "" as path+"bak" [15:58] ijohnson: sure [15:58] ijohnson: done [15:58] then I can update the full pr with everything [15:58] mvo: done already [15:58] \o/ [15:58] thanks! [15:59] mvo: 2 questions under 5918, +1 [16:01] PR snapd#9729 closed: bootloader/lkenv: specify backup file as arg to NewEnv(), use "" as path+"bak" [16:02] pstolowski: thank you! [16:05] of course it was 9718, not sure how I came up with that other number ; [16:05] ;) [16:13] pstolowski: I wondered :) [16:13] heh [17:50] hi folks, question out of left field. In early boot on azure vms. I run into the following error trying to install a snap: Failed running command '/usr/bin/snap install canonical-livepatch' [exit(1)]. Message: error: cannot perform the following tasks: Run configure hook of "canonical-livepatch" snap if present (run hook "configure": cannot perform operation: mount --bind /snap/core/current/etc/apparmor [17:50] /tmp/snap.rootfs_F0mewd/etc/apparmor: Permission denied) [17:52] might this be related to a condition installing additional snaps too soon after waiting on snapd.seeded? We run ' /usr/bin/snap wait system seed.loaded' before trying to install any snaps. But I'm getting failures like this: https://bugs.launchpad.net/snapd/+bug/1674193 [17:52] Bug #1674193: core snap's configuration hangs on debian | openSUSE | mainline kernel [17:53] I am able to snap install canonical-livepatch shortly after initial boot, so this is a racy/early-boot problem [18:02] core 16-2.48 10444 latest/stable canonical✓ core (bionic, kernel:5.4.0-1031-azure) for reference [18:03] will dig to get more details now. was just wondering about known apparmor type concerns in early boot === ijohnson is now known as ijohnson|lunch [18:11] blackboxsw: tbh that looks like a bug in the snap itself, but I would need to look more when I get back from lunch in the meantime have you asked the livepatch folks about the snap? [18:53] * cachio afk [19:57] PR snapd#9735 opened: gadget: disable ubuntu-boot role validation check [20:02] PR snapd#9733 closed: tests: New queries for the os tools === ijohnson|lunch is now known as ijohnson [20:47] PR snapd#9736 opened: o/devicestate: save model with serial in the device save db [21:02] ijohnson: thanks. I pinging in livepatch and they hadn't seen that issue previously either. It could be a case of slow platform response for snap installs and setup. I'll try adding retries on the command to see if that avoids a racy [21:02] condition [21:03] as it is we retry snap install canonical-livepatch 3 times anyway [21:03] with a backoff 0.5 1 and 5 seconds [21:06] blackboxsw: ok, do you have more logs in that bug? or better yet do you have a shell on such an affected machine? [21:07] ijohnson: I'll try reproducing on an azure PRO vm and see if I can cause it repeatedly. If I can, I'll present an IP here of the affected vm for inspection [21:07] thanks BTw [21:08] PR snapd#9737 opened: tests: use os.query tool instead of comparing the system var [21:08] great, thank you [21:23] axino: /names [22:03] PR snapd#9735 closed: gadget: disable ubuntu-boot role validation check <⚠ Critical> [23:24] ijohnson: couldn't reproduce snap install canonical-livepatch failures on multiple launches. Will bring it up again if I can trigger the failure mode [23:54] blackboxsw: ok, if you are able to reproduce please get a system journal with `journalctl --no-pager`? [23:55] And file a bug with that journal [23:56] ijohnson: will do. I note that I may have interrupted the system's initial boot on this machine in my integration test that exhibited the issue, so I could have possibly disrupted initial snap core setup. [23:56] will grab logs if it happens again [23:57] Great thank you