| mborzecki | morning | 05:44 |
|---|---|---|
| mborzecki | mvo: hey | 06:08 |
| mvo | mborzecki: good morning! | 06:08 |
| mborzecki | hmm something for zyga https://paste.ubuntu.com/p/fMmRQNryVs/ | 06:11 |
| zyga | mborzecki, mvo: we must disable parallel instances for classic | 06:21 |
| zyga | mborzecki: that is already excluded, I suspect someone ran a PR from earlier master | 06:22 |
| zyga | althouhh | 06:22 |
| zyga | *although | 06:22 |
| zyga | 2020-04-17T20:30:19.4282018Z + find /run/snapd -user test '!' -path '*/user@12345.service/*' | 06:22 |
| zyga | interesting, so there's another form on 20.04 now | 06:22 |
| zyga | oh well | 06:22 |
| zyga | I'll exclude that one too | 06:22 |
| zyga | thanks | 06:22 |
| mvo | thanks zyga | 06:23 |
| zyga | https://bugs.launchpad.net/snapd/+bug/1873701 | 06:23 |
| mup | Bug #1873701: support for parallel instances on classic breaks all snap remove in lxd <snapd:New> <https://launchpad.net/bugs/1873701> | 06:23 |
| mborzecki | zyga: what's broken with paralle instances? | 06:23 |
| zyga | mvo: I believe this is urgent | 06:23 |
| zyga | mvo: not security related | 06:24 |
| zyga | mvo: it's just a bug that is particularly annoying, you cannot remove snaps, system is in a broken state | 06:24 |
| mvo | zyga: right, when does it happen? it's behind a experimental flag so how easy is it to trigger? | 06:24 |
| zyga | mvo: IIRC the flag is enabled on master now | 06:25 |
| zyga | mvo: basically using edge breaks when you try to remove snaps | 06:25 |
| mborzecki | zyga: iirc it's still experimental on master | 06:25 |
| zyga | mborzecki: hmmm then something is broken | 06:26 |
| zyga | mborzecki: we may not respect the flag then | 06:26 |
| zyga | mborzecki: it definitely takes effect on edge | 06:26 |
| mvo | zyga: also why did we not caught this in tests :/ | 06:26 |
| zyga | mborzecki: try the bug instructions please | 06:26 |
| mvo | zyga: what bugnumber is that again? | 06:26 |
| zyga | mvo: just above | 06:26 |
| zyga | mvo: 1873701 | 06:26 |
| zyga | mvo: in other news snapd is entirely unusable in a 14.04 lxd container | 06:28 |
| zyga | mvo: due to at least three bugs | 06:28 |
| mborzecki | zyga: hm not sure i see it, how's that parallel instances? | 06:28 |
| zyga | mvo: not sure if we want to be explicit about that | 06:28 |
| zyga | mborzecki: it's the /snap mount point | 06:28 |
| zyga | ├─/snap /dev/sda5[/var/snap/lxd/common/lxd/storage-pools/default/containers/sid/rootfs/snap] | 06:29 |
| zyga | we created this for parallel instances | 06:29 |
| zyga | mvo: and there's a typical duplication of entries it causes | 06:29 |
| zyga | er mborzecki ^ | 06:29 |
| mborzecki | zyga: ok, see that now, isn't that the shared code in s-c? | 06:30 |
| zyga | yes | 06:30 |
| mvo | zyga: oh, interessting. we sometimes hat "cannot umount ..." errors, do you think that could explain it? | 06:30 |
| zyga | mborzecki: I think it just runs | 06:30 |
| zyga | mvo: perhaps | 06:30 |
| zyga | mvo: would have to look at a specific instance | 06:30 |
| zyga | mvo: related to your work, another bug: https://bugs.launchpad.net/snapd/+bug/1873703 | 06:31 |
| mup | Bug #1873703: snapd doesn't start in a Ubuntu 14.04 container <snapd:New> <https://launchpad.net/bugs/1873703> | 06:31 |
| zyga | mvo: this one I believe affects 14.04 in general, I cannot explain why it doesn't fail in our tests | 06:31 |
| zyga | mvo: it's also easy to reproduce | 06:31 |
| zyga | mvo: one more https://bugs.launchpad.net/snapd/+bug/1873704 | 06:31 |
| zyga | mvo: I guess we just never tested this case | 06:31 |
| mup | Bug #1873704: snapd cannot be used in a Ubuntu 14.04 container - no squahsfuse <snapd:New> <https://launchpad.net/bugs/1873704> | 06:32 |
| zyga | mvo: there's one more bug that I didn't report, 14.04 containers do not have loopback control device, making it impossible to mount any snap, even with side-loaded snap/squashfuse | 06:32 |
| zyga | but I want to ask stgraber about it first | 06:32 |
| zyga | in other news, good morning | 06:34 |
| mvo | zyga: the lxd one is confusing | 06:34 |
| zyga | sorry to bring bad news up front | 06:34 |
| mvo | zyga: dpkg -L snapd |grep fuse | 06:34 |
| mvo | /usr/bin/snapfuse | 06:34 |
| zyga | mvo: where? it's not in the archive package | 06:34 |
| mvo | zyga: apt list --installed snapd | 06:34 |
| mvo | Listing... Done | 06:34 |
| mvo | snapd/focal,now 2.44.3+20.04 amd64 [installed,automatic] | 06:34 |
| zyga | mvo: perhaps I messed something up but it was definitely not there when I tried | 06:34 |
| mborzecki | zyga: / wasn't shared under lxd, was it? | 06:35 |
| zyga | mvo: that's a focal package :) | 06:35 |
| zyga | mborzecki: correct | 06:35 |
| zyga | mvo: 14.04 package is older and doesn't have it | 06:35 |
| mborzecki | zyga: right, so this is where the bind mount /snap -> /snap comes from | 06:35 |
| mvo | zyga: maybe it's another instance of 14.04 deb needing an update, I pushed something some weeks ago and then it lost with the people who can actually update things in 14.04 :/ | 06:35 |
| zyga | mborzecki: I see | 06:35 |
| mvo | zyga: old pkg> aha, yes, then that's the case | 06:35 |
| zyga | mborzecki: I wonder what's the new thing then, was removing snaps in containers always broken | 06:36 |
| zyga | mborzecki: can you reduce my reproduction case to something smaller | 06:36 |
| zyga | maybe the edge snapd there is not required? | 06:36 |
| zyga | mborzecki: it was kind of late when I reported it | 06:36 |
| mborzecki | zyga: looking at the pr with classic & parallel, the change was to have that mount ns setup when we're running a !classic snap || the experimental flag is set | 06:39 |
| mborzecki | zyga: then we make sure that /snap is shared bind mount if / isn't | 06:39 |
| mborzecki | (so lxd case?) | 06:40 |
| mborzecki | the experimental flag would also trigger /var/snap mount | 06:40 |
| zyga | indeed! | 06:40 |
| zyga | mborzecki: so I guess the interesting combination is the edge refresh | 06:44 |
| zyga | mborzecki: this code used to be absent | 06:44 |
| zyga | mborzecki: right? | 06:44 |
| mborzecki | zyga: hmm i would think a scenario like this, install (mount appears), you run a snap (/snap -> /snap appears), refresh happens (new mount appears), remove only unmounts /snap/foo/{n,n+1}, the original /snap/foo/{n} mount hidden by the /snap -> /snap is still present? | 06:48 |
| zyga | mborzecki: I don't think so, the app snap has only one revision | 06:49 |
| mborzecki | zyga: isnt't that similar to the problem we tried to solve with snap.mount unit that broke things on upgrade? | 06:49 |
| zyga | mborzecki: it's the core snap that gets refreshed | 06:49 |
| mborzecki | zyga: ah core, ok, but the app snap was run, and it triggerd /snap -> /snap bind mount | 06:50 |
| zyga | yes, that part is essential | 06:51 |
| mborzecki | zyga: then the mount /snap/open-watcom that had the /snap->/snap as its parent was cleaned up, but one that was under / was not | 06:53 |
| zyga | that's how I understand it as well | 06:54 |
| mborzecki | hm it'd be intersting to check that on older snapd, but that shared / check was present for a long time | 06:54 |
| zyga | mborzecki: perhaps we should do something else, detach everything and remount / attach | 06:54 |
| mborzecki | wonder if we call it earlier now | 06:55 |
| mup | PR snapd#8522 opened: asserts: introduce ModelGrade.Code <Created by pedronis> <https://github.com/snapcore/snapd/pull/8522> | 07:16 |
| pstolowski | morning | 07:17 |
| mborzecki | pstolowski: good morning! | 07:17 |
| mup | PR snapd#8523 opened: image,seed/seedwriter: support redirect channel aka default tracks <Created by pedronis> <https://github.com/snapcore/snapd/pull/8523> | 07:36 |
| mup | PR snapd#8524 opened: bootloader: use binary.Read/Write <Simple 😃> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/8524> | 07:40 |
| pedronis | mvo: mborzecki: hi, I proposed some small PRs, two are UC20 related, one is the default track fix for prepare-image | 07:44 |
| mvo | pedronis: thank you | 07:57 |
| pedronis | mborzecki: is #8487 ready for reviews? | 07:58 |
| mup | PR #8487: gadget, cmd/snap-bootstrap: MBR schema support <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8487> | 07:58 |
| mborzecki | pedronis: yes, please do | 07:58 |
| zyga | re | 08:52 |
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/8525 | 09:02 |
| mup | PR #8525: tests: ignore user-12345 slice and service <Created by zyga> <https://github.com/snapcore/snapd/pull/8525> | 09:03 |
| mup | PR snapd#8525 opened: tests: ignore user-12345 slice and service <Created by zyga> <https://github.com/snapcore/snapd/pull/8525> | 09:03 |
| zyga | mborzecki: I think that should fix it | 09:03 |
| zyga | I need a review for https://github.com/snapcore/snapd/pull/8525 | 09:13 |
| mup | PR #8525: tests: ignore user-12345 slice and service <Created by zyga> <https://github.com/snapcore/snapd/pull/8525> | 09:13 |
| zyga | er wait | 09:13 |
| zyga | bad paste | 09:13 |
| zyga | https://github.com/snapcore/snapd/pull/8515 | 09:13 |
| mup | PR #8515: testutil: add NewDBusTestConn <Created by zyga> <https://github.com/snapcore/snapd/pull/8515> | 09:13 |
| pstolowski | mvo: hey, found one issue in your initramfs PR; i'm testing it this morning and something is still not quite right, same symptoms as before with cloud-init 🧐 | 09:29 |
| pstolowski | (to be clear, your change takes effect and does the thing) | 09:29 |
| mborzecki | meh everything is red | 09:36 |
| === pedronis_ is now known as pedronis | ||
| mvo | pstolowski: aha, do you have more details? or maye the image so that I can run it myself? | 10:09 |
| pstolowski | mvo: cloud-init is not run, extra-users are not created, don't have any other clues yet. i can share the image, give me a moment. nb ubuntu-image will need a change afaict, atm i'm moving system-defaults to the proper location alongside system-data and user-data manually | 10:16 |
| mvo | pstolowski: yeah, sharing the image would be ideal, if it's not too much effort | 10:21 |
| mvo | pstolowski: that should allow me to peek at things | 10:21 |
| mvo | pstolowski: might be something silly, I really want to write this spread test but didn't had time yet :( | 10:22 |
| pstolowski | mvo: aaah i think i know | 10:26 |
| pstolowski | mvo: i didn't update writable-paths | 10:26 |
| mvo | pstolowski: no update it in what way? | 10:28 |
| pstolowski | mvo: ah nvm, got confused. of course i'm not writing to /etc/systemd anymore, so not an issue. hmmm | 10:28 |
| pstolowski | mvo: ok i uploaded the image | 10:28 |
| mvo | pstolowski: please share the link | 10:29 |
| pstolowski | mvo: sent you gdrive link | 10:30 |
| mvo | pstolowski: checkin | 10:30 |
| mvo | g | 10:30 |
| mvo | pstolowski: downloading now | 10:32 |
| pstolowski | mvo: actually, it seems it didn't copy the rsyslog symlink from system-data-defaults to system-data (but did run, .done is there). it doesn't explain why cloud-init didn't run though | 10:34 |
| mvo | pstolowski: interessting | 10:35 |
| mvo | pstolowski: booting mine now | 10:35 |
| pstolowski | maybe i did a mistake somewhere | 10:35 |
| mvo | pstolowski: inspecting now, looks like something funny/funky with cloud-init | 10:40 |
| mvo | pstolowski: fwiw, I see the /dev/null symlink in /etc/systemd/system/rsyslog.service | 10:41 |
| pstolowski | mvo: yes. but this might have been created by configure hook later | 10:42 |
| mvo | pstolowski: aha, ok | 10:42 |
| mvo | pstolowski: thanks, that's good to know | 10:42 |
| pstolowski | mvo: we should see rsyslog -> null symlink in system-data/etc/... as well, copied from system-data-defaults afaiu? | 10:42 |
| mvo | pstolowski: yes, I see it there | 10:44 |
| mvo | pstolowski: I don't see the var/lib/snapd/features there but that's because snapd cleans this dir on restart | 10:44 |
| pstolowski | mvo: ah jeez, you're right it's there. i must have been blind | 10:44 |
| mvo | pstolowski: no worries, let me look at cloud init some more | 10:45 |
| pstolowski | mvo: interesting (about features) | 10:45 |
| zyga | pstolowski: to remove features future snapd wrote but we don't know abut | 10:45 |
| pstolowski | zyga: yes, makes sense | 10:46 |
| mvo | pstolowski: and I suspect the state has this opiton not set (the experimental robust...) | 10:48 |
| zyga | mvo: note, robust is now default | 10:49 |
| zyga | when uset | 10:49 |
| zyga | unless you explicitly disabled it it should be enabled | 10:49 |
| zyga | unless this snapd is simply older | 10:49 |
| mvo | pstolowski: as for cloud-init, it seems like it cannot found a data source, is there a cloud init no-cloud data file somewhere in this image? | 10:49 |
| mvo | zyga: aha, thanks | 10:50 |
| pstolowski | mvo: ahh, i forgot to copy cloud-data onto the image! | 10:53 |
| mvo | pstolowski: hopefully that is the missing piece of the puzzle! | 10:53 |
| pstolowski | mvo: yep. sorry about that! lots of hops and manual steps | 10:53 |
| mvo | pstolowski: if it looks good after this test I will update my core-build PR and also provide this for core20 | 10:53 |
| mvo | pstolowski: yeah, no worries | 10:53 |
| pstolowski | mvo: note the .done bug | 10:53 |
| mvo | pstolowski: yeah, thanks for spotting this, silly me | 10:54 |
| mvo | pstolowski: that's what I meant with "will update my pr" :) | 10:54 |
| pstolowski | mvo: afaiu we also need to update ubuntu-image | 10:56 |
| mvo | pstolowski: please remind me for what bits? | 10:57 |
| mvo | pstolowski: could config? | 10:57 |
| pedronis | we need to keep it as synced for UC 16/18 because we don't know what people did with hooks | 10:58 |
| pedronis | so u-i should still work | 10:58 |
| pedronis | for them | 10:58 |
| pedronis | we want to clean this up a bit but shouldn't be a blocker, or so I think | 10:58 |
| mvo | yeah, this is why I'm asking, I wonder if there is more | 10:58 |
| pstolowski | mvo, pedronis it doesn't seem to take system-data-defaults directory at the root level (alongside user-data and system-data) | 11:02 |
| pstolowski | unless i messed something | 11:02 |
| pedronis | ah | 11:02 |
| pstolowski | atm i writing them under system-data and move manually before booting for the first time, jsut for the test | 11:03 |
| pedronis | that's different and might need fixing | 11:03 |
| pedronis | it also works differently in UC16/18 vs UC20 | 11:03 |
| pstolowski | i didn't dig into u-i python code yet | 11:03 |
| pedronis | the directory structure is different | 11:03 |
| pstolowski | pedronis: should we put defaults under system-data to simplify? | 11:05 |
| mup | PR snapd#8524 closed: bootloader: use binary.Read/Write <Simple 😃> <UC20> <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/8524> | 11:05 |
| pedronis | pstolowski: maybe, something to discuss with mvo | 11:05 |
| pstolowski | mvo: success! | 11:07 |
| pstolowski | mvo: cloud-init works | 11:07 |
| pstolowski | mvo: ^ so the last bit to consider is what i just discussed above with Samuele | 11:12 |
| zyga | brb, a small break | 11:17 |
| mup | PR snapcraft#3060 closed: V2 npm plugin <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3060> | 11:41 |
| pedronis | mborzecki: I reviewed #8487 | 11:46 |
| mup | PR #8487: gadget, cmd/snap-bootstrap: MBR schema support <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8487> | 11:46 |
| mborzecki | pedronis: thanks | 11:46 |
| mup | PR snapd#8523 closed: image,seed/seedwriter: support redirect channel aka default tracks <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/8523> | 12:01 |
| mup | PR snapd#8526 opened: image: improve/adjust DownloadSnap doc comment <Created by pedronis> <https://github.com/snapcore/snapd/pull/8526> | 12:10 |
| mup | PR snapd#8527 opened: seed: clearer errors for missing essential snapd or snap <Created by pedronis> <https://github.com/snapcore/snapd/pull/8527> | 12:14 |
| mvo | pstolowski: aha, indeed, I think this needs fixing in u-i | 12:14 |
| mvo | pstolowski: let me think about this for a moment - we could use /system-data/.defaults | 12:15 |
| zyga | mvo: FYI: https://bugs.launchpad.net/ubuntu/+source/initramfs-tools-ubuntu-core/+bug/1873797 | 12:32 |
| zyga | mvo: I didn't look deeper | 12:32 |
| mup | Bug #1873797: add_mountroot_fail_hook is either buggy or not used properly, causes boot problems <initramfs-tools-ubuntu-core (Ubuntu):New> <https://launchpad.net/bugs/1873797> | 12:32 |
| mvo | zyga: nice, thanks | 12:34 |
| pstolowski | zyga: should https://bugs.launchpad.net/snapd/+bug/1872115 be re-assigned to core? i don't know what to do about it tbh | 12:37 |
| mup | Bug #1872115: [UC16] log rotation doesn't properly restart rsyslogd <snapd:New for stolowski> <https://launchpad.net/bugs/1872115> | 12:37 |
| zyga | pstolowski: sure, feel free to reassign | 12:37 |
| zyga | pstolowski: I can look after beta | 12:38 |
| zyga | mvo: ^ if you agree | 12:38 |
| mvo | pstolowski: +1 | 12:38 |
| mvo | pstolowski: how do you feel about /writable/system-data/.defaults (cc pedronis) - this avoids the need to modify u-i I think | 12:39 |
| pstolowski | mvo: yes, this works for me, it's more less what i was suggesting | 12:39 |
| pedronis | mvo: any particular reason to make it hidden? | 12:39 |
| mvo | pedronis: no real reason, mostly worried about potential future clashes but probably a bit silly to worry about this | 12:40 |
| mvo | pedronis: if not hidden, maybe /writable/system-data/config-defaults even ? | 12:40 |
| pedronis | I actually think that making it hidden makes clashes a bit more likely | 12:40 |
| pedronis | I have a /.cache which I don't know what makes it | 12:41 |
| mborzecki | pedronis: updated 8487 | 12:41 |
| mvo | pedronis: ok, not-hidden is fine for me | 12:42 |
| pedronis | mvo: pstolowski: let me think a bit about the name | 12:42 |
| mvo | ok | 12:43 |
| pedronis | mvo: all other directories there are meant to be bind mounted, right? | 12:44 |
| mvo | pedronis: correct | 12:44 |
| mvo | zyga: https://bugs.launchpad.net/ubuntu/+source/initramfs-tools-ubuntu-core/+bug/1873797 is about the new initramfs code, right? | 12:48 |
| mup | Bug #1873797: add_mountroot_fail_hook is either buggy or not used properly, causes boot problems <initramfs-tools-ubuntu-core (Ubuntu):New> <https://launchpad.net/bugs/1873797> | 12:48 |
| zyga | mvo: I don't know, it's just something that flashed through my inbox | 12:49 |
| mvo | zyga: it's the old stuff | 12:49 |
| mvo | zyga: meh, thanks! | 12:49 |
| zyga | presseding error on ubuntu 20.04 https://www.irccloud.com/pastebin/GMvrEUtj/ | 13:19 |
| zyga | mvo: ^ the log | 13:19 |
| mup | PR snapd#8525 closed: tests: ignore user-12345 slice and service <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8525> | 13:19 |
| mvo | zyga: yeah, I saw it, I suspect it's because something changed (lxd from using core->core18?) | 13:27 |
| zyga | ah, inded | 13:27 |
| zyga | that did happen just recently | 13:27 |
| zyga | ENOTEA | 13:41 |
| zyga | I'll eat lunch and make some tea | 13:42 |
| zyga | ttyl | 13:42 |
| ijohnson | mborzecki: you said you were going to try and pick up the ubuntu-boot grub.cfg writing from snapd? this is also needed for the pi4 boot, since when we run makeBootable20RunMode during install mode, we need the ubuntu-boot uboot.env to be there during the reboot's initrd, otherwise snap-bootstrap can't find a bootloader | 13:45 |
| ijohnson | mborzecki: what I was going to do in the short term until we rewrite the ubuntu-boot boot assets like this was to just have the pi gadget install it's empty uboot.env to ubuntu-boot, so that the gadget partition creation effectively installs an empty uboot.env there | 13:46 |
| mup | PR snapcraft#3063 opened: cli: update command names to new design <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3063> | 13:59 |
| pedronis | ijohnson: to be clear, hard coding boot config is only for grub for now | 14:20 |
| ijohnson | pedronis: why only grub ? | 14:20 |
| pedronis | ijohnson: because we measure thing only with grub atm | 14:20 |
| ijohnson | pedronis: but we basically have the same need for i.e. u-boot too for uc20? | 14:21 |
| ijohnson | not for measuring | 14:21 |
| ijohnson | but just for booting | 14:21 |
| pedronis | we don't measure | 14:21 |
| pedronis | so there no a deep constraint | 14:21 |
| pedronis | also there's more variability on uboot land | 14:21 |
| pedronis | not sure there will be one uboot.cfg | 14:21 |
| * cachio afk -> bank | 14:45 | |
| ijohnson | mborzecki: did you see my messages above? | 15:32 |
| ijohnson | mborzecki: I am pushing forward with just having the gadget put a uboot.env on ubuntu-boot, and pedronis doesn't think it's necessary so probably for now if you pick up my branch just make non-grub bootloaders no-op on that | 15:33 |
| mup | PR snapd#8528 opened: tests: naive fix for preseeding failure <Created by zyga> <https://github.com/snapcore/snapd/pull/8528> | 15:37 |
| mup | PR pc-amd64-gadget#43 opened: Clean up cmdline, after initrd&core land <Created by xnox> <https://github.com/snapcore/pc-amd64-gadget/pull/43> | 15:38 |
| pedronis | mborzecki: I re-reviewed #8487, couple small comments | 15:41 |
| mup | PR #8487: gadget, cmd/snap-bootstrap: MBR schema support <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8487> | 15:41 |
| ijohnson | pedronis: should I re-review that PR? | 15:41 |
| ijohnson | my original changes in the PR are probably not relevant | 15:41 |
| mborzecki | pedronis: thanks, i'll push it in a minute | 15:41 |
| pedronis | it does need a 2nd review | 15:41 |
| mborzecki | ijohnson: sorry missed that, let me check the backlog | 15:41 |
| ijohnson | I'll try to review it today then, I'll also re-test it with the pi today | 15:41 |
| mborzecki | ijohnson: right, grub.cfg for now | 15:42 |
| ijohnson | mborzecki: yep | 15:42 |
| pstolowski | cachio: ping | 16:03 |
| cachio | pstolowski, hey | 16:05 |
| pstolowski | cachio: do you have a moment for HO? | 16:05 |
| cachio | yes | 16:06 |
| pstolowski | cachio: ok, in standup HO | 16:06 |
| pstolowski | pedronis: having small issue with the gadget + system-files plug + install hook approach as gadget is modified/unasserted, so no autoconected, also connections: in gadget yaml has no effect. not sure if there is any way around this :}. would devmode make sense? | 16:28 |
| pedronis | pstolowski: do you know why connections in gadget don't work? | 16:30 |
| pstolowski | pedronis: i suppose snapid is not avilable | 16:30 |
| pstolowski | at least this is my understanding of the code atm | 16:30 |
| pedronis | ah, yes | 16:31 |
| ijohnson | mborzecki: https://github.com/snapcore/snapd/pull/8487#pullrequestreview-396608940 | 16:31 |
| mup | PR #8487: gadget, cmd/snap-bootstrap: MBR schema support <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8487> | 16:31 |
| pstolowski | pedronis: i suppose i could update seed.yaml with devmode for the gadget (so the plug wouldn't be needed).. but that's slightly terrible | 16:35 |
| pedronis | pstolowski: it's terrible but is the only thing that will work short term I fear, so it's ok, the confinment of the hooks is not really what the test is about | 16:36 |
| pstolowski | mhm | 16:37 |
| pedronis | ? | 16:39 |
| pstolowski | pedronis: no disagreement | 16:40 |
| bdx | @reviewers bump https://forum.snapcraft.io/t/classic-confinement-request-for-nhc-snap | 16:48 |
| pedronis | pstolowski: also preseed tests are failing on 20.04 | 16:56 |
| pedronis | 2020-04-20T16:49:24.6022877Z + CORE_IMAGE= | 16:56 |
| pedronis | 2020-04-20T16:49:24.6023208Z + unsquashfs '' | 16:57 |
| pedronis | 2020-04-20T16:49:24.6023431Z Could not open , because No such file or directory | 16:57 |
| ijohnson | pedronis: seems he quit | 17:03 |
| ijohnson | pedronis: I thought I saw zyga send a PR for that | 17:03 |
| mup | PR snapcraft#3061 closed: plugins: introduce v2.RustPlugin <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3061> | 17:12 |
| mup | PR snapcraft#3064 opened: cmake v2 plugin: rename configflags to cmake-parameters <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3064> | 17:24 |
| pedronis | ijohnson: yes, but it says it actually breaks in a different way | 17:25 |
| ijohnson | ah I didn't actually read the PR, I just saw the notification | 17:25 |
| mborzecki | ijohnson: pushed a fix, thanks for catching this | 17:28 |
| === ijohnson is now known as ijohnson|lunch | ||
| pedronis | so the issue is that cloud images now carry snapd snap, not the core snap | 17:32 |
| pedronis | I disable those preseed tests on 20.04 for now | 17:42 |
| pedronis | mmh, do we even need the hacks anymore | 17:45 |
| === bdx1 is now known as bdx | ||
| mup | PR snapcraft#3065 opened: tests: fix local plugin spread test to be multi-arch aware <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3065> | 18:15 |
| mup | PR snapcraft#3031 closed: build_providers: pass through SNAPCRAFT_{BUILD,IMAGE}_INFO to container or VM <Created by jhenstridge> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3031> | 18:48 |
| mup | PR snapd#8529 opened: cmd/snap-bootstrap/initramfs-mounts: support non-ebl bootloaders <UC20> <⚠ Critical> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8529> | 18:58 |
| === ijohnson|lunch is now known as ijohnson | ||
| ijohnson | pedronis: I opened ^ it would be great if you could review it in your AM and we land it ASAP, I think it's the last bit we need in the initrd specifically to support uc20 on raspi | 19:00 |
| mup | PR snapd#8522 closed: asserts: introduce ModelGrade.Code <UC20> <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/8522> | 19:00 |
| mvo | ijohnson: thank you so much for this | 19:03 |
| ijohnson | yaw mvo | 19:03 |
| ijohnson | I will have 1 maybe 2 other PR's in addition to this to get the pi working, but those are just for snapd proper | 19:04 |
| pedronis | ijohnson: mmh | 19:25 |
| pedronis | ijohnson: I think we should really avoid to touch unrelated tests | 19:26 |
| ijohnson | pedronis: ? | 19:27 |
| pedronis | the Bootenv16 hack | 19:28 |
| ijohnson | pedronis: hmm I guess I can get around that, what do you think the uc20 tests that want to use uc16 style thing should look like? | 19:28 |
| pedronis | there is probably a simple way to do it without having to touch unrelated tests | 19:28 |
| ijohnson | pedronis: how about `boottest.MockUC20LegacyKernelBootenv(bootloadertest.Mock("mock", c.MkDir()))` ? | 19:29 |
| pedronis | also we should try to agree how to call the thing, because legacy is kind of misleading | 19:29 |
| pedronis | we are stuck with this | 19:29 |
| pedronis | and also it's using kernel_status | 19:29 |
| ijohnson | yes I really don't know what to call it | 19:30 |
| ijohnson | what I called it in 8512 is "pure env" | 19:30 |
| ijohnson | because it just strictly uses the "bootenv" | 19:30 |
| pedronis | ijohnson: and sorry I'm coming from this angle, instead of great, yay progress. I suppose the touching of unrelated tests tend to put me in a slightly critical state of mind | 19:30 |
| ijohnson | pedronis: it's okay | 19:31 |
| ijohnson | pedronis: I understand, wdyt about my proposal to use "pure env" | 19:31 |
| ijohnson | ? | 19:31 |
| pedronis | ijohnson: so there are two things to it, right? one is that is using the env and no symlinks, the other is that the extracted kernel is not one image but a bunch of files in dirs? | 19:33 |
| ijohnson | pedronis: yes that is true, but note that there's nothing that specifically says we don't have a single image | 19:34 |
| ijohnson | pedronis: all the code I've written/worked with are just about using env and no symlinks | 19:34 |
| ijohnson | I don't think we have any assumptions that aren't uboot specific about having multiple files | 19:34 |
| ijohnson | but maybe I'm forgetting something | 19:34 |
| pedronis | no, it's likely so | 19:35 |
| pedronis | ijohnson: I'm just trying to find something that is not too far from ExtractedRunKernelImageBootloader | 19:36 |
| ijohnson | pedronis: maybe ExtractedRunKernelImage is wrong | 19:36 |
| ijohnson | pedronis: maybe we should change that to something like ExtractedRunKernelImageSymlink ? | 19:37 |
| ijohnson | or ExtractedKernelSymlink | 19:37 |
| pedronis | ijohnson: no, it's right because Image means one file | 19:37 |
| ijohnson | mmm | 19:37 |
| pedronis | but there are nuances we lose either way | 19:38 |
| pedronis | unless we use unbearably long new names | 19:38 |
| ijohnson | how about we just drop run and have ExtractedKernelImageSymlinkBootloader ? then for this other one it would just be ExtractedKernelEnvBootloader ? | 19:38 |
| ijohnson | I mean I don't think we get to have short names given the subject matter and the specificity we desire | 19:38 |
| pedronis | anyway it also not the time to rename the other one I think | 19:38 |
| ijohnson | okay, how about ExtractedKernelEnvBootloader for this new thing ? | 19:38 |
| ijohnson | it doesn't necessarily need to be an actual bootloader.Bootloader yet | 19:39 |
| ijohnson | but we would just call that kind of thing an "extracted kernel env" thing ? | 19:39 |
| pedronis | but the kernel doesn't need to be extracted, as you said | 19:39 |
| pedronis | it is though in practice | 19:39 |
| ijohnson | well I may have mis-spoke, I think we do require it to be "extracted" in that it can't be in a .snap | 19:40 |
| ijohnson | it needs to be extracted from the snap | 19:40 |
| ijohnson | but not really any more extracted | 19:40 |
| pedronis | EnvRefExtractedKernelBootloader | 19:44 |
| ijohnson | sure that's fine | 19:45 |
| ijohnson | so then for mocking we would have | 19:45 |
| ijohnson | boottest.MockUC20EnvRefExtractedKernelRunBootenv(bootloadertest.Mock("mock", c.MkDir())) | 19:45 |
| pedronis | yes | 19:46 |
| ijohnson | a mouthful, but good enough | 19:46 |
| ijohnson | give me a few minutes to update the PR to not change the other tests | 19:46 |
| ijohnson | and use this new thing | 19:46 |
| pedronis | I mean you can probably cheat and use/change Bootenv16 if useful as long as it's not visible to the test using it | 19:47 |
| ijohnson | yes that's exactly what I've done | 19:47 |
| pedronis | I mean the MockUC16 shouldn't take new params | 19:47 |
| ijohnson | yes | 19:48 |
| ijohnson | but the struct now has an additional unexported member that is what status var | 19:49 |
| ijohnson | to use | 19:49 |
| pedronis | that's fine I suppose | 19:49 |
| pedronis | we don't make it directly | 19:49 |
| ijohnson | pedronis: okay updated back to just 3 files changed | 19:55 |
| ijohnson | I am going to rename the things in#8512 too to match this new name | 19:56 |
| pedronis | ijohnson: why is the first new test separate from the rest? is creating a bit of a confusing diff | 20:09 |
| ijohnson | pedronis: as I explained in the commit msg / PR description ideally we shouldn't test this additional dimension of type of bootloader here in snap-bootstrap, we should test that in the boot pkg when all this logic goes there | 20:09 |
| ijohnson | pedronis: so for now just to get adequate coverage / assurance things are working I just copied all the kernel tests we have and ported them to use that bootloader name we just invented | 20:10 |
| ijohnson | pedronis: I apologize for the diff thing, I don't know why github does that | 20:10 |
| ijohnson | it's more sensible if you look at just the commit | 20:10 |
| ijohnson | i.e. https://github.com/snapcore/snapd/pull/8529/commits/1619e0824b62649e5957050ef7c70570bb0ed008 | 20:11 |
| mup | PR #8529: cmd/snap-bootstrap/initramfs-mounts: support EnvRefExtractedKernelBootloader's <UC20> <⚠ Critical> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8529> | 20:11 |
| pedronis | ijohnson: that's not my question, my questions is why is TestInitramfsMountsRunModeStep2LegacyKernelBootstate all alone | 20:12 |
| pedronis | and all the other Legacy are later | 20:12 |
| ijohnson | oh hmm I dunno | 20:12 |
| ijohnson | I can move it if you like | 20:12 |
| ijohnson | I guess I wrote that one first from scratch then just copy pasted all the other ones and changed the relevant bits | 20:13 |
| * ijohnson has already forgotten | 20:13 | |
| pedronis | ijohnson: heh, it's ok, by yes if you don't have a strong reason to have it where you put it, having it together with the rest is preferable | 20:14 |
| ijohnson | sure I can push another commit moving it by the others | 20:14 |
| ijohnson | one second | 20:14 |
| pedronis | ijohnson: did you rename them away from Legacy ? | 20:14 |
| ijohnson | ahhhhhh I will now :-) | 20:14 |
| pedronis | ijohnson: jusrt s/Legacy/EnvRef/ | 20:15 |
| pedronis | don't think we want super long names | 20:15 |
| * ijohnson glares at TestInitramfsMountsRunModeStep2EnvRefKernelBootstateUntrustedTryKernelSnapFallsBack | 20:16 | |
| pedronis | ijohnson: reviewed | 20:29 |
| ijohnson | ack, thanks for the review | 20:30 |
| pedronis | ijohnson: thanks for working on making the diff a bit more digistible | 20:32 |
| ijohnson | no problem , re the TODO, is the TODO just about not returning a Bootenv16 or is it about not using that at all for the implementation of MockUC20EnvRefExtractedKernelRunBootenv ? | 20:32 |
| pedronis | ijohnson: it's about no returning a Bootenv16, I can see various ways, anyway is not an immediate concern, but is a bit confusing to use a thing called 16 related to 20 | 20:40 |
| ijohnson | pedronis: ack that makes sense | 20:41 |
| pedronis | ijohnson: next I should review #8512 ? | 20:51 |
| mup | PR #8512: boot/bootstate20: add EnvRefExtractedKernelBootloader bootstate20 implementation <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8512> | 20:51 |
| ijohnson | pedronis: yes, I will have the last PR open later tonight | 20:53 |
| ijohnson | but 8512 is the next one, then the one I am yet to open | 20:53 |
| pedronis | ijohnson: ok, I'll look at 8512 in my morning, if you have a moment you should s/Legacy/EnvRef/ in the mostly test names there | 20:58 |
| ijohnson | ah yes, forgot about the tests there too | 20:58 |
| ijohnson | have a good night pedronis | 20:58 |
| pedronis | thanks, have a good end of the day | 20:59 |
| mup | PR snapcraft#3066 opened: autotools v2 plugin: rename configflags to autotools-configure-parame… <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3066> | 21:15 |
| mup | PR snapd#8530 opened: boot: enable makeBootable20RunMode for EnvRefExtractedKernel bootloaders <UC20> <⚠ Critical> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8530> | 21:19 |
| mup | PR snapcraft#3063 closed: cli: update command names to new design <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3063> | 21:36 |
| mup | PR snapcraft#3067 opened: requirements: uprev python-apt <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3067> | 21:48 |
| mup | PR snapd#8531 opened: secboot,cmd/snap-bootstrap: add model to pcr protection profile <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8531> | 22:15 |
| mup | PR snapcraft#3065 closed: tests: fix local plugin spread test to be multi-arch aware <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3065> | 22:58 |
| mup | PR snapcraft#3067 closed: requirements: uprev python-apt <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3067> | 23:37 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!