[06:09] PR snapd#7665 closed: devicestate: add support for gadget->gadget remodel [06:13] morning [06:14] mvo: thanks for merging 7665, do you need help with other remodel PRs? [06:14] mborzecki: hey, good morning! [06:14] mborzecki: maybe, let me see what pawel wrote [06:14] mvo: ok [06:15] mborzecki: 7720 needs a second review, should be super simple [06:15] mvo: ok, on it [06:24] Hello [06:24] Starting soon [06:29] zyga: good morning [06:30] mborzecki: I updated 7715, there is one question about test improvements, I need to think about it but I think it's not a blocker, feel free to ponder over this, I will not attack this today (his question about testing the taskEdges) [06:33] zyga: hey [06:33] mvo: let me check that [06:36] zyga: did you dream about keyctl? :) [06:37] in the office now [06:37] mborzecki: no, I decided to really rest today [06:37] zyga: i went through https://www.kernel.org/doc/html/v4.13/security/keys/core.html and then ecryptfs key usage examples but feel none wiser really [06:38] mborzecki: I'm sure there are keys here [06:38] https://twitter.com/zygoon/status/1191776451995095041 [06:38] but not like you know it ;) [06:38] mborzecki: I think given timing we need to investigate keys next week [06:38] zyga: still, it feels like a replacement for a cookie, so half of the problem could be addressed [06:38] this week I'd like to get the cgroup in place and adjust snapd [06:38] maybe get the UI proposed [07:23] PR snapd#7687 closed: snap-bootstrap: check gadget versus disk partitions [07:34] * zyga break, some back pain, may work from floor today === pstolowski|afk is now known as pstolowski [08:08] morning [08:10] good morning pstolowski [08:15] Breakfast [08:41] Implemented the cgroup idea [08:41] Running tests [08:41] Snapd side changes next [08:48] PR snapd#7721 opened: gadget: add support for hybrid partitioning schemas [08:48] pstolowski: hey [08:50] trivial PR for gadget ^^ [09:06] mborzecki: sure [09:06] pstolowski: thanks! [09:07] I really want snap set system reexec=no [09:12] PR snapd#7720 closed: asserts: add "snapd" type to valid types in the model assertion [09:14] Some progress [09:14] Freezer freezing snap confine is silly [09:16] hahah [09:16] zyga: you mean s-c froze itself? :) [09:23] google:ubuntu-core-18-64:tests/main/remodel-gadget failed on master :/ [09:24] a second review for 7711 would be great, it has one already and is green. hopefully straightforward [09:25] mborzecki: oh no! in what way? [09:25] - Remove data for snap "pc" (36) (failed to remove snap "pc" base directory: remove /var/snap/pc: directory not empty) [09:25] mborzecki: just being silly [09:25] mborzecki: it works now [09:25] hm probably some other test leaving stuff behind [09:25] https://www.irccloud.com/pastebin/DUaJsF5k/ [09:25] mborzecki: no, I put it in the wrong spot [09:25] mborzecki: lxd escapes us though [09:26] mborzecki: I'll send the patch with the function in a moment [09:26] mborzecki: need to clean up the commit tree [09:28] mborzecki: lxd escaping all cgroups https://www.irccloud.com/pastebin/76oMP7KO/ [09:30] mborzecki: the C diff https://paste.ubuntu.com/p/T9NmjppHrY/ [09:45] zyga: FYI, filed https://bugs.launchpad.net/snapd/+bug/1851480 (forgot to hit “submit” yesterday …). Will ping here if I get stuck in my fix attempt [09:45] Bug #1851480: Hooks are not included in slot/plug label expressions [09:51] mborzecki: have a look at https://github.com/snapcore/snapd/pull/7722 [09:51] PR #7722: cmd/snap-confine: add sc_join_sub_group [09:51] dot-tobias: hey [09:51] PR snapd#7722 opened: cmd/snap-confine: add sc_join_sub_group [09:51] dot-tobias: thanks! [09:52] mborzecki: on top I have the move of cgroup handling so that it affects all snap kinds [09:52] zyga: Welcome, hope I got half the snapd lingo right 😄 [09:52] mborzecki: and a one liner than enables this new logic [09:52] mborzecki: my thinking is we can keep the pids thing for a while [09:52] mborzecki: and nuke it later once I'm done with snapd-side change [09:53] dot-tobias: yeah, the bug report looks great [09:54] mborzecki: does this seem sensible? [09:54] zyga: the diff doesn't look too bad [09:55] zyga: it'd be nice to run the idea by jamie and mvo too before we spend too much time digging [09:55] zyga: i mean jdstrand [09:57] uhh, the way we restore the system in tests is broken [09:57] mvo, jdstrand: ^ [10:07] zyga: whats the high level idea? I have the C diff loaded now [10:07] mvo: the high-level idea is that we use a sub-directory of the current unified/v2 cgroup as a tag [10:07] mvo: and move the process there to identify it as a process belonging to a snap [10:08] mvo: I discussed this with xnox yesterday for in the context of being systemd-safe [10:08] mvo: it's safe to move to a deeper hierarchy [10:08] mvo: (moving up is not) [10:08] mvo: it uses v2 in all systems, giving us notification ability [10:08] mvo: the new idea is from maciek really, that cgroups can act as process tags [10:09] mvo: the idea to use v2 is because it is very much like name=snapd in v1 world (no controllers) [10:09] mvo: and in v2 world it doesn't fall apart as we don't have to steal a process entirely [10:09] mvo: and we can move it deper wherever systemd had originally placed it [10:09] mvo: this is the key improvement: it works in v2 in a way that doesn't disable systemd's operation [10:10] zyga: aha, ok [10:10] mvo: it works in lxd as well, I confirmed this with stgraber two days ago [10:10] zyga: that sounds promising! [10:10] mvo: notification is implemented differently [10:10] mvo: and enumeration is implemented differently [10:10] mvo: and both are coming as follow-ups to snapd [10:10] mvo: first enumeration, so that existing code can be moved over (along with all the spread tests for this) [10:11] mvo: notification should let us salvage the UI work that is waiting now [10:11] mvo: so maybe... just maybe... it will really work by Friday [10:11] mvo: no rush to merge it, I'll carry on implementing it, but it would be good to review and discuss with jamie [10:12] mvo: xnox raised a concern about security aspect of keeping an app open [10:12] mvo: because it would prevent refreshes [10:12] mvo: but I think this is really the feature design [10:12] mvo: we are also looking with mborzecki at kernel keyring as a way to prevent spoofing [10:12] mvo: where all our processes could hold something that cannot be forged [10:13] mvo: but all it does is make the feature more security tight, it doesn't change that anyone can run a snap and prevent it from refreshing using "snap run..." [10:13] mvo: so our existing snapd-side safeguards must suffice - that is the window of time after which we refresh anyway [10:14] zyga: right, agreed [10:17] mvo: there's enough priority items not to review it yet [10:17] mvo: but we might try to review all of it by end of week [10:17] mvo: just to be able to say "it's in master" [10:19] is there a way to bypass the restriction on refreshing to a revision? [10:19] some how during recent update we discovered several users skipped a revision [10:20] and during some troubleshooting we really need them to refresh or revert to that revisiom [10:20] revision* [10:20] geekgonecrazy: can you explain how skipping a revision was a problem for your users? [10:21] geekgonecrazy: based on what you said it seems like snap epochs are the solution we had designed and implemented for this problem [10:21] geekgonecrazy: where you can force all users to migrate through a set of revisions [10:21] geekgonecrazy: e.g. everyone will see and run the revision that can migrate your data format from v1 to v2 [10:21] geekgonecrazy: before allowing them to refresh to a revision that only has v2 support [10:22] where is this magic? we thought it was this way out of the box [10:22] geekgonecrazy: how did you think it operats? [10:22] geekgonecrazy: (it doesn't but I'd like to understand) [10:22] geekgonecrazy: let me refer to the docs on this feature, hold on [10:23] geekgonecrazy: https://snapcraft.io/docs/snap-epochs [10:23] it was exactly that. we had to upgrade db and introduced revision and gave it a week thinking all would be updated before introducing the next revision [10:23] geekgonecrazy: you need to use epochs to really make that a guarantee [10:24] geekgonecrazy: as to what to do in an existing situation, I don't know the details of what you did to answer [10:24] geekgonecrazy: perhaps you can refresh everyone to an intermediate revision that still understands old formats [10:24] geekgonecrazy: and publish the one that doesn't in a new epoch [10:24] geekgonecrazy: please read about this feature first [10:26] so for us we were on mongo 3.4 then intermediate revision (the one some have missed) sets compatibility mode which has to be set before it csn go to 3.6. then latest revision is 3.6 [10:26] geekgonecrazy: epochs is meant to be used for that kind of thing [10:27] geekgonecrazy: https://snapcraft.io/docs/snap-epochs [10:27] heh, i see zyga linked there already [10:27] ok [10:27] * zyga hugs Chipaca for implementing this magic [10:29] so in theory we could release another revision requiring both of those and if they already had all would be fine? [10:29] geekgonecrazy: both of which? [10:30] or will we have to hack past and put this revision in another channel and have them refresh to that channel to run that revision and then back to the latest [10:30] Chipaca: both required revisions [10:30] sorry, i don't follow [10:32] ok. so problem is we have that revision some didnt hit. now they cant refresh or revert there. so we need some way for them to get that revision [10:32] we cant release new revision. because would take those that succeeded backwards in database breaking their installs [10:34] so trying to figure out if there is a way we could have them manually refresh to it [10:34] then back to stable [10:36] geekgonecrazy: ah [10:37] geekgonecrazy: can you detect a broken install programmatically? [10:38] Chipaca: yes. But... fixing requires the older database version. [10:38] geekgonecrazy: it does sound like your easiest way forward is to use a branch [10:39] and then in future start using epochs so it doesn't happen again [10:39] really wish i would have known of epochs sooner :) would have been perfect [10:39] geekgonecrazy: a branch is like a temporary track that goes away on ts own [10:40] so you push the snap that would fix it to latest/stable/fix-for-the-thing, and tell affected users to refresh to it [10:41] oh dang.. didnt know you could do that either [10:41] we still are trying to figure [10:41] out the tracks :) [10:41] :) [10:41] branches are hard to explain until you need them [10:42] rather like epochs actually [10:46] thanks Chipaca & zyga ! [10:48] btw Chipaca on thr forum post regarding dig it seems its another dependency and dig just wont run. so will be digging after this fire and getting to bottom of it. [10:51] geekgonecrazy: good luck! [11:12] sil2100: I pushed a tiny PR to ubuntu-image so that we can have /boot on the system-seed partitoin, this should bring us one step closer to a botting image, hopefully a trviail review (cc xnox) [11:13] Chipaca: do you think we should have a short catchup today on recovery.grub? I think we are getting to a point where it will be useful :) [11:14] mvo: I was just trying the image created with your steps from yesterday, and something seems off [11:14] Chipaca: i.e. we can create an image now but right now (due to a bug) it has no /boot but even when it has that we still don't have a recovery grub that will work. [11:14] mvo: but maybe i'm missing something :-) [11:14] so, yeah [11:14] Chipaca: tell me! [11:14] mvo: /systems/2019yadda/ is empty, and instead things are in /snaps/ ? [11:15] Chipaca: its fully empty? [11:15] Chipaca: so "/snaps" has all the snaps that are shared between recoveries that is ok [11:16] mvo: ah, i missed that [11:16] Chipaca: but /systems/20191106/ should not be empty [11:16] mvo: empty of snaps [11:16] Chipaca: thats ok :) assertions and the model should be there [11:16] yep that's there [11:16] Chipaca: there will only be local unasserted snaps iirc [11:16] Chipaca: cool [11:17] Chipaca: once we have https://github.com/CanonicalLtd/ubuntu-image/pull/177 we should be able to get /boot too with grub.cfg and grubenv [11:17] PR CanonicalLtd/ubuntu-image#177: System seed boot dir [11:17] Chipaca: but we also need to tweak boot.MakeBootable to copy the right grub :) [11:17] Chipaca: but I'm excited, we are relatively close I think(?) [11:19] mvo: where's the bit that creates writable? [11:21] Chipaca: we create only "ubuntu-seed" nowdays, the recovery kernel then boots and the recovery initramfs will notice it runs in "install" mode and will run "snap-bootstrap" which will create all the missing partitions [11:22] hmmm, maybe i did something wrong [11:22] mvo: i thought i'd kicked that off, but it hung [11:22] mvo: i'll try again, paying more attention :) [11:22] Chipaca: no worries! you kicked what off? [11:23] mvo: booted the kernel, with the initramfs [11:23] and it got to the mounting step and hung [11:23] Chipaca: oh, woah - how did that happen? [11:23] mvo: the booting? or the hanging? :) [11:23] Chipaca: I mean, how did you manage to get a booting kernel? [11:23] mvo: i can walk you through that, it's not hard [11:24] Chipaca: a pastebin with the commands would be great [11:24] mvo: or i can write a workng grub.cfg for the image as it is right now [11:24] Chipaca: but thats actually much better news than I anticipated [11:24] mvo: but, basically, grub is awesome (as long as you're patient) [11:24] * mvo hugs Chipaca really hard [11:24] Chipaca: s/grub/chipaca/ [11:24] Chipaca: (and strike the patient part) [11:25] nah leave that one on [11:25] Chipaca: thats super exciting, can't wait to learn more [11:27] mvo: "ubuntu-seed"'s label is actually 'Recovery', fwiw [11:27] not sure if that's a bug or not :) [11:29] Chipaca: in the gadget.yaml ? or where is it set to Recovery? [11:29] Chipaca: it sounds like a bug [11:30] mvo: on the image [11:30] haven't looked at the yaml [11:31] quick errand, back in 30 [11:31] Chipaca: ha! you are right [11:31] Chipaca: its in the gadget.yaml, should be trivial to fix [11:32] Chipaca: I have lunch now, let's sync on the grub stuff when I'm back [11:32] hm, again it reached "Running /scripts/local-premount" and gets stuck there [11:32] maybe i'm being impatient? [11:33] * Chipaca waits more [11:34] Chipaca: will probably timeout eventually - alternatively the usual "break=top" or so should work. *maybe* even "systemd.debug-shell=1" [11:34] Chipaca: this is the new stuff from xnox, we may have systemd already in initiramfs :) [11:34] mvo: ah i was about t'ask [11:35] "the required kernel commandline snap_core is not set" [11:35] Chipaca: silly thing :) [11:35] that's not a thing in 20 [11:35] Chipaca: I think we need quite a few updates to our initramfs [11:35] Chipaca, is this conversation related to the removel-gadget test is failing on master? [11:35] Chipaca: and the fact that we can boot a kernel now means we can actually do it (hurazzz!) [11:36] cachio: no :) [11:36] Chipaca, ok, I'll take a look to the test in that case [11:36] thanks!! [11:36] :) [11:36] cachio: AFAICT noone has looked deeper into this one yet :/ any hints appreciated (but maybe mborzecki has looked at the failing remodel-gadget on master, not sure) [11:36] * mvo gets lunch first [11:37] mvo: i'll get an editable workable grub.cfg into a pastebin so more people can play [11:37] Chipaca: \o/ [11:37] Chipaca: or even into github.com:snpcore/pc-gadget in the 20 branch? :) ? [11:38] mvo: ah! Nice catch! I forgot we're skipping that one [11:38] mvo, ok [11:39] mvo: let's wait for at least one test run to finish and I'll merge it [11:46] mvo: something else is is snapcore/pc-gadget a thing? [11:46] er [11:46] mvo: sorry those were two separate things [11:46] mvo: second part first: where is this snapcore/pc-gadget? [11:47] mvo: first part: something else needs doing because it's not picking up grub.cfg from the obvious place [11:47] might be the old fat vs vfat names sillyness [11:47] * Chipaca tries [12:04] hmm [12:04] Chipaca: without reading the backlog, is something failing and you are looking at pc-gadget? [12:13] zyga: not really no [12:27] re [12:28] mborzecki: making progress, some more complexity but nothing breaking [12:31] zyga: sound like you're having fun :) [12:32] mvo: cachio: yes i have, and the way we restore the system state after the test is wrong or just lacking [12:33] mborzecki, what I see is that the test is failing just when other test is executed before [12:33] mvo: cachio: in short what happens is that we have a tar.gz with files that existed *before* the test, and in restore the restore that state, but things that are newly added during the test stay behind, they don't get cleaned up [12:33] mvo: cachio: imo what we need is like rsync --delete or just fix the tests to do the cleanups [12:34] mborzecki, nice, I'll try that [12:34] cachio: i'm fixing the tests for now, but maybe i can find a sensible way of detecting when stuff is left behind [12:35] sensible, as in not invovling gruesome shell one liners === ricab is now known as ricab|lunch [12:41] - Remove data for snap "pc" (36) (failed to remove snap "pc" base directory: remove /var/snap/pc: directory not empty) <- Chipaca <- was that the topic? [12:51] PR snapcraft#2792 closed: pluginhandler: use well-formed build package/snap lists [12:51] zyga: no [12:57] mborzecki: nice, thanks for looking into this [12:57] Chipaca: re gadget> https://github.com/snapcore/pc-amd64-gadget/tree/20 [12:58] Chipaca: that's the current place and we can update the 20 branch [13:00] mvo: where is EFI, on the resulting image? [13:03] Chipaca: a good question, I need to look [13:03] Chipaca: not sure if that needs special handling [13:03] Chipaca: aha, hmmmm, I *think* we did that using the gadget.yaml [13:04] Chipaca: maybe that needs updating [13:04] mvo: I mean, gadget.yaml talks about EFI, but there is no /EFI/ in the recovery partition afaict [13:04] Chipaca: hmm, that might need tweaks to ubuntu-image again [13:05] Chipaca: https://github.com/snapcore/pc-amd64-gadget/blob/20/gadget.yaml#L23 should populate the partition [13:05] Chipaca: but when I look I also see no content there [13:05] mvo: also, also, i'd like to be able to load a grub module :) [13:05] Chipaca: what does that involve? [13:05] is that doable? [13:06] mvo: in grub.cfg it's an insmod, and a file under EFI/ [13:06] not sure how it interferes with secure boot though [13:06] maybe a question for cmatsuoka [13:07] Chipaca: should be ok - we may hardcode it into grub though, i.e. build grub with the module built-in [13:07] mvo: but insmod regexp gives me globs so i can loop over a directory and pick out stuff without having to have env vars for everything [13:07] mvo: so i think we want that :) [13:10] Chipaca: right [13:10] Chipaca: so I think we have two options, we can built it into grub or add the module to the gadget.yaml [13:10] Chipaca: I think the later is quicker for now [13:10] mvo: only if adding it to gadget.yaml actually puts it somewhere :) [13:11] Chipaca: but maybe xnox has an opinion on regexp module vs build-in [13:11] Chipaca: right, that's indeed a bit of a mystery right now [13:12] Chipaca: mvo: loading modules is prohibited under Secureboot [13:12] and i thought regex is a built-in already anyway [13:12] mvo: also i see grub.conf in the pc-gadget, is that a bad symlink that should have been called grub.cnf? [13:13] also generated image has duplicate full copies of snaps itseems, that's quite suboptimal. [13:14] note systemd-seed is vfat thus no hardlinks / symlinks available... [13:14] (unless my world view is wrong about vfat) [13:15] xnox: grub.conf vs grub.cfg is a long story [13:15] mvo: i slowly back away. cool then. [13:15] xnox: I don't see duplicated snaps [13:15] PR snapd#7723 opened: snap-bootstrap: create encrypted partition [13:15] xnox: I just see /snaps and a bunch of them there [13:16] xnox: AFACIT the regexp module is not built-in [13:16] so reading source code i beleve regexp module is built-into our grub efi image at least. On bios it needs to be loaded, which is allowed there. [13:16] Chipaca: hm. [13:16] so maybe this is booting via the bios one? [13:16] Chipaca: how are you booting it? [13:16] seems strange, because it loads the cfg from EFI [13:16] Chipaca: I will wrestle a bit why ubuntu-image does not include the content on the recovery partition unless sil2100 has an idea, I will probably have to do some deep dive [13:17] Chipaca: we are dual-boot/hybrid by default..... [13:17] xnox: you know that email from mvo saying how to get a pc.img ? [13:17] xnox: that [13:17] xnox: how can i know if it's booting in efi or bios? [13:18] we shoudl really get rsync into the core snap [13:18] Chipaca: are you using a VM? and have you provided OVMF firware to do bios boot and get a spash with tiano core? or do you use seabios ie. default. [13:19] xnox: ah, this is a vm but wihtout the fancy ovmf stuff [13:19] mvo: I think I know the reason [13:19] so i guess it's plain ol bios [13:19] Chipaca: there is nothing in the email that tells people how to boot it. qemu binary defaults to bios boot, unless one provides EFI firwmare. I use virt-manager, and use gui to specify ovmf stuff. [13:19] Chipaca: which is not our primary target. =) [13:19] mborzecki: we have a rsync snap, no? [13:19] we are racing to boot unencrypted with UEFI =) [13:19] mvo: my expectation was that what snap prepare-image generates will already have all the necessary EFI/ directories [13:19] xnox: right, i'll get that rig up (not my main rig because it still refuses to upgrade out of 1604) [13:19] mvo: but if that's still required to be done by ubuntu-image, then I can do that [13:20] sil2100: aha, interessting [13:20] har har [13:20] mborzecki: another chunk https://github.com/snapcore/snapd/pull/7724 [13:20] PR #7724: cmd/snap-confine: tracking processes with classic confinement [13:20] sil2100: its a good question, in theory we have all the code to do that I think [13:20] mborzecki: this applies the tracking (still with pids) for classic confiement [13:26] ruh roh, ubuntulog fight! [13:27] mvo: right, but it's a snap, and i need to restore this on a core device [13:29] mborzecki: could we download it to /var/tmp and unpack there? [13:29] mborzecki: or is that too crazy [13:30] mvo: ha, this might even work! :) [13:30] mborzecki: ssh + tar dude [13:30] mvo: ok, let me finish up with this simple fix i have now and i can try that later [13:30] mborzecki: tar cz path/to/tar | ssh 'tar xvz' [13:30] mborzecki: \o/ [13:31] Chipaca: i'd seriously just want rsync -a in prepare, then rsync -a --delete in restore [13:31] mborzecki: or, snap install rsync --devmode [13:33] Chipaca: right, but the cleanup is in restore, where we dump the mount units, the state and so on, that's why unpacking to /tmp/rsync sounds appealing [13:34] unpacking the snap ofc [13:35] mborzecki: should work as long as it's the same core [13:36] mhm [13:39] PR snapd#7711 closed: seed: test and improve Core 20 seed handling errors [13:42] brb, reboot === ricab|lunch is now known as ricab [13:50] PR snapd#7725 opened: tests/lib/state: snapshot and restore /var/snap during the tests [13:51] mvo: cachio: zyga: poor man's fix ^^ [13:52] mborzecki: classic mount namespaces [13:52] mborzecki: is that why you picked /var/snap/* over /var/snap [13:52] ? [14:09] pstolowski: I updated 7715 with most of your comments [14:28] mvo: ok, PR coming to you in a minute, just finishing running the unit tests [14:28] sil2100: \o/ [14:29] ijohnson: do you know if mksquashfs lets you append to an existing squash using a different compression level? [14:30] Chipaca: you can set compression options like block size and dictionary size, but it doesn't significantly improve performance because those things don't 1:1 correspond to compression ratio and you the size differences are minimal between that and the default [14:30] Chipaca: I don't know I haven't looked at that [14:31] Chipaca: my gut guess is that no, you can only use a single compression level for the whole squash image because the compression options are written in exactly one place in the image for the different types (i.e. file data, file metadata, and something) [14:36] ijohnson: yeah, unless an append adds a new superblock, i don't see it happening either [14:40] https://forum.snapcraft.io/t/support-for-man-pages/2299 [14:40] i saw somewhere that we might add man page support to snapd. or did I dream that? [14:40] Chipaca: yeah exactly [14:40] mvo: hopefully this will do it: https://github.com/CanonicalLtd/ubuntu-image/pull/178 [14:40] PR CanonicalLtd/ubuntu-image#178: System seed boot content [14:41] I mean, if I didn't mix it up in my head [14:42] mvo: #7715 +1, with one final remark [14:42]