[00:49] PR snapcraft#3371 opened: snap: get legacy branch from local [06:49] morning [06:50] re [07:23] mborzecki: hello [07:23] * zyga installed kubuntu and now wonders how to do stuff in kde :P [07:23] hahah [07:23] zyga: hey [07:24] I'm back on the x240 until the new one arrives, need to do some serial port things and I wanted to avoid doing it from window [07:24] *windows [07:24] how's uc20? [07:40] zyga: it was moving, i was off for 2 days, but there's 2.48 branch, so things are looking good? [07:41] mborzecki: ooh, new house time? [07:42] zyga: nah, i mean uc20 was moving, albeit slowly ;) [07:42] ohhh [07:42] well, :) [07:56] PR snapd#9629 opened: spdx: update to SPDX license list version: 3.10 2020-08-03 [08:03] +1 [08:03] though it remains to be seen if this can be merged [08:03] morning [08:03] hey pstolowski [08:03] hey zyga [08:07] pstolowski: hey [08:07] no mvo today? [08:07] I haven't seen him yet [08:09] hey mvo :) [08:10] mvo: hey, i see 2.48 is branched, yay! [08:10] zyga: good morning [08:10] mborzecki: yes! but one bug is being fixed right now, there is a open PR from ian [08:11] pedronis: hi, i've updated the list of licenses, but iirc you mentioned that some store tooling may be affected, best if you take a look https://github.com/snapcore/snapd/pull/9629 [08:11] PR #9629: spdx: update to SPDX license list version: 3.10 2020-08-03 [08:13] wonder whether we should keep some of the osi approved identifies that are deprecated [08:13] mborzecki: it's timing question, let's chat in the standup about it [08:13] eg the non deprecated ids have GPL-3.0-only instead of GPL-3.0 [08:14] pedronis: sure [08:14] mvo: mborzecki: we have nested failures here: https://github.com/snapcore/snapd/pull/9626 [08:14] PR #9626: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes [08:15] mvo: I think the PR your merged from sergio broke nested tested, that's fixed I think but was a bit annoying [08:15] uh, sorry! [08:15] nested.sh itself uses systemd stuff [08:15] pedronis: did https://github.com/snapcore/snapd/pull/9626/commits/39c2f79922cae157076784705deca45487144d79 fix it? [08:15] seems so, but we have other failures now [08:15] about gadget [08:15] stuff [08:16] that's why somebody should look [08:16] [change 2 "Setup system for run mode" task] failed: cannot use gadget: gadget does not support encrypted data: volume "pc" has no structure with system-save role [08:16] hmm [08:17] mvo: ah, but the fix is not quite right? I honestly didn't look at it [08:17] it was late [08:17] i can take a look [08:17] seems like it's not using the right helper to build the image [08:18] 2020-11-12T04:33:57.7272622Z Nov 12 04:19:39 ubuntu snapd[1240]: taskrunner.go:271: [change 2 "Setup system for run mode" task] failed: cannot use gadget: gadget does not support encrypted data: volume "pc" has no structure with system-save role [08:20] mborzecki: 9628 has the same failure plus one more [08:20] might be order though [08:22] pedronis: mvo: the test is setting NESTED_BUILD_SNAPD_FROM_CURRENT: false, which will not repack the gadget snap with ubuntu-save [08:23] s/with/adding/ [08:23] mborzecki: nice find [08:23] mborzecki: but I suppose it's intentional not to use the current snapd [08:23] so the fix is not just to flip it, no? [08:24] it seems we are mixing different stuff together [08:24] pedronis: i think we can use 20/edge/next for the gaget snap [08:25] or not, there's no 20/beta/next :/ [08:29] mborzecki: let me check [08:30] mborzecki: 20/edge/next [08:32] mvo: so the test wats to switch between 20/beta and 20/edge, but none of those revisions can be used when encryption because it lacks ubuntu-save [08:35] why did it start failing only now though? [08:35] * pedronis is missing something [08:36] are we resuing the wrong vm? [08:36] those tests are not about encryption [08:37] mborzecki: we had passined nested tests yesterday, they started failing only late [08:37] after the systemd merge afaik [08:38] pedronis: fwiw https://github.com/snapcore/snapd/pull/9606 was merged by sergio not me, I did approve it though [08:38] PR #9606: tests: Use systemd-run on tests part2 [08:39] mvo: it has one review fwiw [08:39] pedronis: yes, it was a premature merge :( [08:39] mvo: anyway I'm still quite confused what is going on [08:39] pedronis: same here, just wanted to clarify that I did not hit merge on this one :) [08:40] pedronis: and looking at the diff it seems largely unrelated to nested [08:41] mvo: pedronis: the test would also be skippd if the revisions in beta & edge are the same, maybe something changed there? [08:42] https://github.com/snapcore/snapd/pull/9619 this passed with nested [08:42] PR #9619: cmd/snap-bootstrap,o/devicestate: use a secret to pair data and save [08:45] mborzecki: 20/beta 2020-09-02 and 20/edge 2020-10-21 seem to be not that :/ [08:45] mborzecki: I mean, it looks like they have not recently changed [08:46] anyway I thought we run only a few tests with the tpm ? [08:46] and those are not part of it? or are they? [08:47] mborzecki: one question is what changed between 9169 and those prs [08:47] heh, 9619 [08:55] hm we don't run tests after merging to master, do we? [08:57] not anymore I think [09:25] pedronis: the test also runs with TPM expliciltly enabled, i suppose it is to catch the resealing scenarios too [09:30] mvo: i think we should always run nested tests on 2.48 branch, they are skipped right now [09:32] mborzecki: +1 [09:38] mborzecki: let me know if I can help in any way btw, i.e. if it would help to have a 20/beta/next branch I could push that [09:40] mvo: i'm running this spread test on release/2.48 now, but yeah we could poke xnox to publish a gadget with ubuntu-save to 20/beta/next maybe and then go a fix the test [09:41] mborzecki: I can do that to if it unblocks us [09:43] mborzecki: just say the word :) [09:45] hmm `Nov 12 09:44:48 ubuntu snapd[1258]: taskrunner.go:271: [change 2 "Setup system for run mode" task] failed: cannot use gadget: gadget does not support encrypted data: volume "pc" has no structure with system-save role` on release/2.48 branch [09:48] mborzecki: so it's broken everywhere and we don't know what/when :/ let's fix it :) [09:48] mvo: mhm, poking xnox on mm [09:49] mborzecki: if he does not reply I can also do a upload to pc into 20/beta/next [09:51] mvo: mhm, in the meantime i'll look into having nested tests run on all pushes to release/** branches [09:51] mborzecki: \o/ [10:05] mborzecki: I have a pc gadget based on beta with ubuntu-save that we could use, I'm building it right now but lxd is a bit slow it seems [10:07] mvo: that would work, is it just repacking the snap from 20/beta? [10:08] mborzecki: correct, i took 20/beta and applied the diff from edge -> edge/next (which is just adding ubuntu-save) [10:08] mvo: ok, that should be fine then [10:16] mborzecki: released into 20/beta/next [10:16] mvo: thanks, let me fix the test now [10:17] xnox: I uploaded a new pc gadget into 20/beta/next that contains ubuntu-save, it's otherwise just a repack of 20/beta. this will unbreak our tests and just fyi, it's a branch so noone outside of our tests is affected [10:17] xnox: hope you don't mind :) [10:33] mvo: ok, trying with updated test now, wish we didn't have so much magic hardcoded in NESTED_* env variables [10:35] mborzecki: :( yes [10:51] mvo: that's fine! thanks. [11:02] mvo: omg, heh [11:02] mborzecki: hm? [11:02] mvo: how can i get a revision of a snap from a branch? it's not shown in snap info [11:05] mvo: and not downloading the snap :) [11:06] mborzecki: the revision is 117 [11:06] mvo: i know, the problem is that the tests uses snap info to find the revision of a snap :/ which does not work ofc [11:07] mborzecki: oh, I see, yes, that is unfortunate :/ [11:08] mborzecki: as a workaround you could download the pc snaps, they are small. or we use the store api directly, I don't think we have something currently for this :/ [11:08] mborzecki: shall I have a look at store api? would that help you? [11:09] mvo: hmm i'll come up with something [11:17] mborzecki: this works curl -s -H "Snap-Device-Architecture: amd64" -H "Snap-Device-Series: 16" -X POST -H "Content-Type: application/json" --data '{"context": [], "actions": [{"action": "install", "name": "pc", "channel": "20/beta/next", "instance-key": "1"}]}' https://api.snapcraft.io/v2/snaps/refresh|jq [11:19] mvo: mborzecki: I pushed my tweaks to https://github.com/snapcore/snapd/pull/9628#pullrequestreview-528983238, needs more reviews (but probably after Chris PR), also the state machine diagram is wrong again :/ [11:19] PR #9628: secboot,cmd/snap-bootstrap: fix degraded mode cases with better device handling [11:22] mvo: btw should we squash-merge things for 2.48 ? you added and removed the lable on 9628, same question for #9626 [11:22] PR #9626: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes [11:25] `snap search libreoffice` (executing for a uf support query) gets error (403).. just an FYI https://api.snapcraft.io/v2/snaps/find?architecture=amd64&confinement=strict%2Cclassic&fields=base%2Cconfinement%2Ccontact%2Cdescription%2Cdownload%2Clicense%2Cprices%2Cprivate%2Cpublisher%2Crevision%2Csummary%2Ctitle%2Ctype%2Cversion%2Cwebsite%2Cstore-url%2Cmedia%2Ccommon-ids%2Cchannel&q=libreoffice [11:26] actually the error occurs on lots of queries (error on firefox..) [11:32] guiverc: looks like the store has some trouble right now :/ [11:33] yep, thanks mvo, just saw popey tweet on #ubuntu-desktop [11:37] PR snapd#9630 opened: tests: fix nested tests [11:37] PR snapd#9631 opened: cmd/snap: add snap debug connections/connection commands [11:39] pedronis, mborzecki I just looked at 9626 again - given that a) it got two reviews already b) we know why the nested test fails - I would like to merge it ? release/2.48 has the same test issue as this PR [11:53] pstolowski: reviewed [11:57] zyga: thanks! [12:07] cachio_: hey, can you take another look at #9617? [12:07] PR #9617: tests: compare options of mount units created by snapd and snapd-generator [12:12] pstolowski, left a comment there [12:12] it is quick to fix [12:12] cachio: thx, updated [12:13] mvo: something is not quite right, is it possible to tell which github rev is the snapd in beta? snap info show it's latest/beta: 2.47.1+git1221.ge4f8507 2020-11-11 (10106) [12:14] pstolowski, +1 [12:14] ty [12:14] yaw [12:15] mvo: i suspect it's missing the changes we added for luks/ext4 sizes [12:20] hmmm [12:20] w8, but we moved the keys to a new location right? [12:21] and the kernel was not rebuilt with new initramfs, so there's only theold snap-bootstrap which expects keys at the old location, so it's not possible to get a bootable system with stuff from the store atm [12:21] mvo: ^^ [12:22] so even if i kinda made a part of the test work (it installs atm), the system does not go past initramfs [12:22] mborzecki: about the revision, you can "git show e4f8507 " or checkout etc [12:23] mborzecki: so this is broken until we rebuild the kernel? [12:23] mborzecki: with 2.48? [12:23] mvo: heh, i was searching in git log, but didn't find the rev, maybe had a typo there /o\ [12:23] mborzecki: in this case we need to put it to manual and move on and enable it again [12:23] mvo: yeah, i think so [12:23] mborzecki: the leading "g" needs to be removed [12:23] mborzecki: it's a bit confusing [12:24] mvo: removed the 'g', nvm it's pebkac most likely [12:25] could be a out-of-date local tree or something. anyway, given that we cannot fix this easily right now I will merge the branch from chris [12:27] pedronis: do you want to review 9626 before it lands? ian had some questions. otherwise I land once tests have completed [12:28] i suppose pc-kernel 20/beta is not built with snapd beta right? [12:30] mvo: I worked on it [12:31] mvo: anyway I addressed Ian comments [12:31] in fact [12:31] I mean about 9626 [12:31] mvo: hmmm we may as well disable tpm/secure boot, so no encryption, but the test should work [12:31] guess it's beter than disabling it completely [12:32] mborzecki: yes [12:43] PR snapd#9632 opened: tests/nested/manual/refresh-revert-fundamentals: temporarily disable secure boot [12:53] PR snapd#9633 opened: github: run nested suite when commit is pushed to release branch [13:04] ijohnson: hey, can you take another lokk at #9617? i've added proxy support [13:04] PR #9617: tests: compare options of mount units created by snapd and snapd-generator [13:12] hmm: apparmor="DENIED" operation="exec" profile="snap.test-snapd-after-before-service.before-middle" name="/bin/systemd-notify" pid=342298 comm="start" requested_mask="x" denied_mask="x" fsuid=0 ouid=0 [13:12] ehh, silly me [13:29] pstolowski: yeah sure [13:33] is the "service-control" task being used anywere? pstolowski, ijohnson any chance you remember? [13:35] mvo: yes, in #8960 [13:35] PR #8960: o/snapstate,servicestate: use service-control task for service actions (9/9) <⛔ Blocked> [13:35] ups, mborzecki ^ [13:37] mborzecki: why? [13:40] pstolowski: i'm looking at https://forum.snapcraft.io/t/use-of-before-and-after-keywords-to-define-the-start-order-of-snap-daemons/21082/5 i thought that snap start was fixed to use the right order, but it's not working [13:42] pstolowski: and there's actually 2 places doing something similar, but not quite? one is servicestate.Control which gnerates some commands, and the other is servicemgr.doServiceControl [13:43] mborzecki: are you looking at master? [13:43] or my branch? [13:43] pstolowski: master [13:44] mborzecki: ok. my branch centralizes things around services. would be good to have a spread test with reproducer and then run it against 8960 [13:45] mborzecki: i can look at this after standup [13:45] seems like there is an example already [13:45] on the forum [13:48] pedronis: there, but it shows there's a bug somewhere [13:49] this used to work iirc ;) [13:49] mborzecki: ? [13:59] heh it's not fixed, found an unpushed commit from 2018 which does that [14:03] PR snapd#9634 opened: boot,dirs,c/snap-bootstrap: avoid InstallHost* at the cost of some messiness [14:28] mborzecki: is that old fix for services large? does it have a spraed test already? i wonder if it makes sense to land it or if i should take over it and re-work on top of 8960 [14:29] pstolowski: it's here https://github.com/bboozzoo/snapd/commit/3b5103daf07141d0fefa02f2d774a3f4869aa684 half done and imo it thousl be fixed in servicestate.Control() instead of this [14:32] mborzecki: okay. probably makes sense to tackle it after 8960 as otherwise it would again delay landing of 8960 and possibly lead to annoying and confusing conflicts [14:32] mborzecki: and i can start with a spread test [14:36] pstolowski: hmm looking at 8960 again, this issue should be fixed there [14:37] mborzecki: that would be great. i'm going to create a spread test anyway to verify [14:51] * cachio lunch [14:53] PR snapd#9630 closed: tests: fix nested tests [14:59] PR snapd#9626 closed: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes [15:09] ijohnson: thanks for answering to https://forum.snapcraft.io/t/configure-grub-with-ubuntu-core-20/20835 [15:12] pedronis: yah no problem, there have been a few other folks asking questions on uc20 on the forum I have tried to keep up with and answer [15:17] #9628 needs reviews [15:17] PR #9628: secboot,cmd/snap-bootstrap: fix degraded mode cases with better device handling [15:17] hmm pedronis not sure I agree with 898ade549228a6ae93f117f91c57fe5a1325cca3 there [15:17] let me think about it a bit more, I'm concerned that by saving/observing m.isEncryptedDev we are giving too much meaning to that, when it could be false just by default [15:18] i.e. we don't have a nice way to tell "we definitely saw an unencrypted disk before" and "we haven't yet seen any disk (encrypted or not) yet" [15:18] but let me look a bit more maybe it's fine [15:19] ijohnson: I'm also checking that data is marked found [15:21] it's not that different from how the setUnlockStateWithFallbackKey uses that flag [15:21] in theory we could even push that check down (not sure it woudl be clearer code) [15:21] yes I see that, I'm trying to think about if that's good enough yet [15:21] sadly it's a bit difficult to reason about finding a counter-example where this logic is wrong [15:22] ijohnson: you need to look at all the places we set found for data, all those places make also a definitive decision about whether the device is encrypted [15:23] if we don't find data of course we don't have a decision [15:25] mvo: looks like the unit test job failed in the spread run you mentioned https://github.com/snapcore/snapd/runs/1224993441 ? [15:25] mborzecki: oh, maybe this was just coincidence, hold on [15:26] mborzecki: this one is a better example https://github.com/snapcore/snapd/runs/1386207780 [15:27] hmm `2020-11-11 16:40:13 Cannot allocate google:debian-9-64: google: read JWT from JSON credentials: 'type' field is "authorized_user" (expected "service_account")` [15:28] mvo: aren't we using the same spread binary? [15:30] pedronis: ok yeah I think you're right I can't find a counter-example of where the logic is wrong [15:31] checking FindState is the right thing to do [15:31] ijohnson: tbh though, I don't want to poke at this more right now, but probably IsEncryptedDev should become a tristate Unknown,Encrypted,Unencrypted some checks/things would be clearer [15:32] pedronis: yes that would be a nice thing to have, but agreed for now is fine to leave as-is [15:46] PR snapcraft#3372 opened: tests: make each static test available as a make target [15:47] pedronis: well regardless I looked over all your changes to 9628 and they all lgtm, so if/when that pr is green I think we should land it [15:47] as you say we might need the nested test which disables the tpm/secureboot [16:11] mvo: FYI: groovy's go has a bug in its internal IO loops [16:12] I just spent a better part of the day wondering WTF is going on [16:12] https://go-review.googlesource.com/c/go/+/232862/ is the fixed bug with details [16:12] if you see funny errors in groovy, this _may_ be related [16:12] also if you build with 1.14 [16:14] zyga: oh, fun! thanks for letting me know [16:17] mvo: I poked mwhudson but not sure if there's any priority to backport this [16:17] 1.13 and 1.15 are okay [16:31] PR snapcraft#3373 opened: ci: migrate static checks from travis to github actions [16:39] Hello everyone, i can see there is still issue with snap packages for latested version of manjaro. Does someone know when the issue will be fixed? [16:39] or some workaround known? [16:42] Nemesis: what's the problem? [16:44] zyga: since about 7-10 days all packages instelled on Manjaro (lateset version) does not work (when i start the pakache just showing loading and in 1-2 seconds disapears and nothing else happens). [16:45] i had another issue with 1-2 programs which was able to open (they had squears insted of alphabets).. i read on internet that fonts might be a problem, i changed the font - dint help [16:46] does manjaro use x11 or wayland ? [16:48] Nemesis: can you open any of those apps from command line please [16:48] mvo: ijohnson: I'm going to force merge https://github.com/snapcore/snapd/pull/9632 , it affects only nested tests and those passed (in other tests we have store issues) [16:48] PR #9632: tests/nested/manual/refresh-revert-fundamentals: temporarily disable secure boot [16:48] pedronis: ack sounnds good to me [16:48] [martin@nemesis ~]$ loginctl show-session 2 -p Type [16:48] Type=x11 [16:52] I have most of the packages installed using flatpak/pacman or not installed at all. Give me 1 seconds to go install 2-3 new snap packages [16:54] PR snapd#9632 closed: tests/nested/manual/refresh-revert-fundamentals: temporarily disable secure boot [16:59] cachio: I want to add a uc20 specific nested helper, should I add a function to nested.sh and expose it via the new nested-state binary? [17:00] ijohnson, depends on the funtion [17:00] if it is to manage the service/vm yes [17:00] otherwise it should so directo to nested.sh [17:01] ijohnson: if you can consume it exclusively from nested-state, it'd probably be okay to add to nested-state without adding to nested.sh [17:01] the function is this [17:01] https://www.irccloud.com/pastebin/H0orv42g/ [17:01] zyga: http://ix.io/2DVd - there is one [17:02] it's fairly simple but I need to do it many times in one specific uc20 nested test [17:02] it's kinda managing the VM insofar as it is rebooting the VM and changing the state of it, etc. [17:02] ijohnson, I'd add that to nested.sh [17:03] I'll include that in the following pr to move that to the nested-state tool [17:03] zyga: there is another: http://ix.io/2DVe [17:03] cachio: ok, I will just add it to nested.sh in my PR and use it directly [17:04] there was much more, just i already have them installed using another packages [17:04] ijohnson, nice [17:04] I'll take a look once it is ready [17:09] thanks cachio [17:09] I hope to have it open today [17:34] PR snapd#9634 closed: boot,dirs,c/snap-bootstrap: avoid InstallHost* at the cost of some messiness [17:35] pedronis: thanks for mergning! I will get dinner and then review 9628 again [17:35] (unless someone else beats me to it) [17:35] and then we should be done, yes? [17:37] mvo: yes, I'm merging master into it, and adding a couple not urgent TODOs [17:41] PR snapcraft#3374 opened: [wip] ci: migrate spread tests github actions [17:48] gah [17:49] pedronis: mvo: we need one more fix I just found writing a spread test [17:49] copySafeDefaultData is wrong, it puts the complete file on /run/mnt/host/.../console-conf when it should be putting it on /run/mnt/data/.../console-conf/ [17:50] shall I push the fix to 9628 ? or open a new PR? [17:51] ijohnson: push it to 9628 [17:51] k, one moment [17:51] ijohnson: I just pushed there though, so you need to pull [17:51] right [17:51] I merged master and tweaked a couple of things [17:53] ijohnson: ah, I see, it needs to match the dest part of the if stanze [17:53] stanza [17:53] yes [17:53] boot.InitramfsDataDir [17:53] yep precisely [17:53] so the test is also wrong? [17:53] yeah [17:53] :/ [17:53] this is why spread tests are good though [17:54] yes [17:54] but also why constant for dirs are a mixed bag [17:54] I'm starting to believe that unit test should try to actively spell directories more explictly/differnelty from code [17:55] anyway [17:55] I was wondering if instead of using what you proposed in the SU which would be more work, if we instead had functions from boot that took as an argument the time in which they are being called (or otherwise figured out what time/mode they are being called from directly) [17:55] then you would have for example DataDir(initramfs) or SeedFDEDir(initramfs) and BootFDEDir(install) [17:56] I don't think time is the only relevant variable here [17:56] there's also host vs not [17:56] for recover [17:56] as in your case here [17:57] right but in this case it would be HostDataDir(intitramfs) vs DataDir(initramfs) [17:57] I think the issue here is just that it is confusing that we have both a tpmfs "data" mount and the "host" data mount [17:57] well one of many issues [17:59] ijohnson: it's still an unclear/growing family of functions [18:00] it might be better but is work, if we are going to do work maybe we should do the full work [18:00]