| mup | PR snapd#7580 opened: spread.yaml: exclude vendor dir <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7580> | 02:15 |
|---|---|---|
| mup | PR snapd#7581 opened: tests: add netplan test on ubuntu core <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7581> | 02:29 |
| mup | PR snapcraft#2748 opened: meta: warn about command mangling <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2748> | 02:36 |
| mborzecki | morning | 05:17 |
| mup | PR snapd#7564 closed: interfaces: add cgroup-version to system-key <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7564> | 05:20 |
| mborzecki | school run, back in 30 or so | 05:20 |
| mborzecki | re | 06:02 |
| mborzecki | mvo: morning | 06:02 |
| mup | PR snapd#7568 closed: snap: when running `snap repair` without arguments, show hint <Simple 😃> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7568> | 06:05 |
| mup | PR snapd#7567 closed: snap-repair: error if run as non-root <Simple 😃> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7567> | 06:06 |
| mvo | hey mborzecki | 06:07 |
| mvo | mborzecki: good morning | 06:07 |
| mborzecki | mvo: can you take a look at https://github.com/snapcore/snapd/pull/7571 ? | 06:08 |
| mup | PR #7571: sandbox/cgroup: refactor process cgroup helper to support v2 and named hierarchies <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7571> | 06:08 |
| mvo | mborzecki: uh, I think I wanted to do this yesterday already :/ | 06:09 |
| mvo | mborzecki: let me do this *now* | 06:09 |
| zyga | sorry, had super stubborn kids | 06:16 |
| zyga | one is still at home | 06:16 |
| zyga | ugh :| | 06:16 |
| zyga | ok, let's try to focus... | 06:17 |
| mborzecki | mvo: thanks! | 06:19 |
| zyga | mvo: https://twitter.com/samerhaj/status/1182121464813756419 | 06:19 |
| zyga | and check out the next tweet there, vmware working on this? | 06:20 |
| zyga | esxi for raspi anyone? | 06:20 |
| mborzecki | zyga: wow, nice | 06:20 |
| zyga | I, for one, would not mind a raspberry pi with a proper boot stack | 06:21 |
| mborzecki | zyga: any clue how does uefi/acpi combo could handle hats? | 06:21 |
| zyga | no idea | 06:21 |
| mvo | mborzecki: added a comment, please let me know if it makes sense or if its too terse. happy to talk here as well if you want :) | 06:26 |
| mborzecki | mvo: looking | 06:26 |
| mborzecki | mvo: i see, it's a bit yagni right now, once we land the named hierarchy, the string will no longer the be controller name, but insteadf a named hierarchy name ('snapd' in this case) | 06:28 |
| mvo | mborzecki: right, landing it is fine | 06:29 |
| mvo | mborzecki: I was mostly wondering if consumers of the ProcGroup() need to know what matcher to create | 06:29 |
| mvo | mborzecki: or if we could simply keep using a string here | 06:30 |
| mvo | mborzecki: and the string in the future might be "snapd" | 06:30 |
| mvo | mborzecki: or is this not how it will work :) ? | 06:30 |
| * mvo obviously has less context | 06:30 | |
| mborzecki | mvo: hm you'd have to pass name=snapd to be explicit, or "" when you mean a unified hierarchy, feels a bit too magical | 06:33 |
| mvo | mborzecki: hm, but we know internally in the cgroup module if we are unified or mixed so could we leverage this knowledge? | 06:35 |
| zyga | mvo: we use cgroup v1 even in unified mode | 06:35 |
| zyga | so perhaps it's all going to be simplified eventually | 06:35 |
| zyga | s/use/going to use/ | 06:35 |
| mvo | ok, I don't want to rathole this too much, sry! it just seems to me right now right now we only have one user of "cgroup.ProcGroup" and we always use freezer here so I wonder if we could encapsulate the knowledge that its freezer/something-else inside the cgroup package itself maybe? | 06:37 |
| zyga | we could but the cgroup package doesn't know that, it's the overlord that knows this; I think it's fine as is, it will change shortly, perhaps before the next release | 06:37 |
| mborzecki | mvo: zyga: hm so perhaps we could have a separate call cgroup.SnapProcGroup which internally knows whether to ask for freezer or a named group? | 06:38 |
| zyga | it is my understanding that by the time we are done it will use a different selector or a hard-coded string for the new location | 06:38 |
| zyga | I really don't think we need to do more now | 06:38 |
| zyga | just land and iterate | 06:38 |
| zyga | meanwhile, I'm back to https://github.com/snapcore/snapd/pull/7576 | 06:38 |
| mup | PR #7576: spread.yaml,.gitignore: sync ignored files <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/7576> | 06:38 |
| zyga | why does something so simple have to be so hard :D | 06:39 |
| mvo | zyga: userd is currently using it and it does not know about freezer either, does it? its just snap-confine that knows about freezer (or am I missing something?) | 06:39 |
| zyga | userd is using cgroups? | 06:40 |
| mvo | zyga: I did a "git grep ProcGroup" and a match there | 06:40 |
| mborzecki | zyga: yeah, it is, that's the next thing, we'll probably need some api that encodes information about how snap cgrups are made | 06:40 |
| mvo | zyga: fwiw, I disagree with "why does something so simple have to be so hard" | 06:41 |
| mborzecki | zyga: it's for checking whcih snap the talking to userd over dbus comes from | 06:41 |
| zyga | oh, I didn't notice that | 06:41 |
| zyga | I just read that code | 06:41 |
| mvo | mborzecki: sry again for lack of context - but do we need (external) api for how cgroups are made? mostly wondering if we can keep most of this knowledge internal to the cgroup package | 06:41 |
| zyga | mvo: (I was referring to the gitignore PR) | 06:42 |
| mvo | zyga: oh, sorry | 06:42 |
| mvo | zyga: then I misunderstood :) | 06:42 |
| zyga | mvo: ignoring in spread and git consistently is haaard :) | 06:42 |
| mvo | zyga: I thought it was about my bikesheding api here :) | 06:42 |
| zyga | and it should not | 06:42 |
| zyga | no no, sorry | 06:42 |
| mvo | zyga: git hard> makes sense ;) | 06:42 |
| zyga | mborzecki: I wonder if the pid usage in userd could go away | 06:42 |
| zyga | and use /proc/pid/root instead | 06:42 |
| zyga | but that's a separate conversation | 06:43 |
| zyga | we discussed this with james, as long term API for discovering snap processes | 06:43 |
| zyga | brb, need tea, coffee | 06:43 |
| zyga | I'll catch up on the thread here | 06:43 |
| mborzecki | mvo: we don't, that's why the cleanups are being done, afaik we have 3 places rightr now that made up the grup names themseleves, userd, snapstate/refresh and s-u-n | 06:43 |
| zyga | but mostly want to unbreak that gitignore branch | 06:43 |
| * zyga loves the acronym s-u-n | 06:43 | |
| mborzecki | mvo: 'groups are made' as in how the path under the relevant group mount point is constructured for a snap :) | 06:44 |
| mvo | mborzecki: oh, this is spread out right now? yeah, a good reason to consolidate it :) | 06:44 |
| mvo | mborzecki: anyway, my concern with the PR as it is is just that the current code in userd has cgroup.ProcGroup(pid, "freezer"). that seems to require knowledge in userd about freezer that should probably not be needed there. the new code now also adds the fact that its a v1controller there. so it seems like it makes this slightly worse. I think we can land but we should reverse the direction in the next prs to require less knowledge about how exa | 06:47 |
| mvo | ctly we do cgroups outside of the cgroup package. does that make sense? | 06:47 |
| zyga | back with coffee | 06:49 |
| zyga | mvo: I agree that eventually we should instead have an API that refers to app / hook tracking | 06:50 |
| zyga | and similar pid discovery | 06:50 |
| zyga | and encapsulate the cgroup logic there | 06:50 |
| zyga | I still would merge this as-is and iterate because velocity and not-worse-than-before | 06:50 |
| zyga | mborzecki: last night I needed to sober up after looking at 22MB binaries | 06:51 |
| zyga | so I started making those few-kb, built-with-C, hello-world programs that don't need libc | 06:51 |
| zyga | that was refreshing | 06:51 |
| zyga | offtopic: I found a great use for raspi zero -- it's the cheapest reliable serial logging device I've had | 06:52 |
| zyga | just plug a wire and log away | 06:53 |
| mborzecki | mvo: right, i mean it makes sense, we should be able to figure everything out in cgrup package given the security tag | 06:53 |
| mborzecki | (hopefully) | 06:53 |
| zyga | mborzecki: I would not even use cgroup as the wrapper | 06:53 |
| zyga | we may end up not using cgroups eventually | 06:53 |
| zyga | the package should be conceptual - pid tracking and discovery, it may use cgroups internally | 06:54 |
| mborzecki | zyga: snaptrack? :P | 06:54 |
| zyga | but what it should really have is functions like PidsOfSnap and SnapOfPid or similar | 06:54 |
| zyga | pidof ;-) | 06:54 |
| mborzecki | mvo: since you're doing reviews already, can you take a look at this one too? https://github.com/snapcore/snapd/pull/7578 again it's moving some pieces scattered around into the cgroup pacakge | 06:54 |
| mup | PR #7578: sandbox/cgroup, overlord/snapstate: move helper for listing pids in group to the cgroup package <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7578> | 06:54 |
| zyga | ok | 06:55 |
| zyga | back to gitignore | 06:55 |
| mvo | mborzecki: 7578> sure - the title is promising for sure! | 06:58 |
| mborzecki | mvo: hope the content doesn't disappoint :P | 06:58 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | morning | 06:59 |
| mborzecki | pstolowski: hey | 06:59 |
| zyga | hey pawel :) | 06:59 |
| * zyga reads how git does .gitignore | 07:04 | |
| zyga | /o\ | 07:06 |
| zyga | git has a screen full of global variables | 07:06 |
| mvo | mborzecki: love it | 07:15 |
| mvo | mborzecki: I would even go a bit further and hide pidsCgroupDir from the outside | 07:15 |
| mvo | mborzecki: but fine as is | 07:16 |
| mup | PR snapd#7571 closed: sandbox/cgroup: refactor process cgroup helper to support v2 and named hierarchies <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7571> | 07:21 |
| zyga | mborzecki: so I had a look at those exclude patterns | 07:47 |
| zyga | and I have the following compromise | 07:47 |
| zyga | mborzecki: git needs /foo to exclude a file ./foo won't cut it | 07:47 |
| zyga | mborzecki: spread's use of tar is exactly the opposite | 07:47 |
| mborzecki | hahah | 07:47 |
| zyga | mborzecki: it requires ./foo and /foo won't cut it | 07:48 |
| zyga | I think it's fine if I document it | 07:48 |
| zyga | it's better then sending garbage | 07:48 |
| zyga | and better than not working | 07:48 |
| zyga | I don't see any way out of this for now | 07:48 |
| * mvo has some eoan problems today and regrets upgrading to it | 08:22 | |
| zyga | mvo: what happens? | 08:23 |
| zyga | mvo: I'm running on a similar-ish thinkpad and apart from suspend resume killing input (ha ha) it works okay (sob) | 08:23 |
| Chipaca | fitting that you regret upgrading to eoan in the morning | 08:25 |
| mvo | zyga: its on my desktop, sound issues, stuttering and it always select the wrong output on login so I need to go to gnome-settings etc | 08:26 |
| zyga | mborzecki: oh god | 08:26 |
| zyga | mborzecki: tar has --exclude-vcs-ignores | 08:26 |
| zyga | mborzecki: it reads .gitignore directly | 08:26 |
| zyga | can I just path spread instead? | 08:26 |
| zyga | mvo: man, that sucks :/ | 08:27 |
| zyga | mvo: it's the future though, better bite the bullet and report them | 08:27 |
| mvo | zyga: doing that but its a bit of a time sink this is why I regret it | 08:28 |
| zyga | indeed | 08:29 |
| mvo | Chipaca: hahaha | 08:29 |
| mvo | Chipaca: took a while until the penny dropped | 08:29 |
| Chipaca | mvo: no regrets | 08:32 |
| mborzecki | zyga: hm can we teach spread about --exclude-vcs-ignores then? | 08:32 |
| zyga | I just did locally | 08:32 |
| mborzecki | it'll take a while to land it though | 08:32 |
| zyga | it doesn't seem to work | 08:33 |
| mborzecki | hahahah | 08:33 |
| zyga | running a debug session to see if the files really got copied | 08:33 |
| zyga | it's driving me insane | 08:33 |
| zyga | I hate doing this, it breaks all C testing I work on | 08:33 |
| mborzecki | well, i'm trying to teach postrm and snap-mgmt about the snap.mount unit :/ so i feel you | 08:33 |
| * zyga hugs mborzecki :) | 08:34 | |
| mborzecki | btw. noticed something intersting in the tests, we've supposedly stopped lxd service (and i guess any containers it ran), but then removing /var/snap/lxd/comon/ns/mntns fails with EBUSY | 08:35 |
| zyga | are you sure it stopped containers? | 08:35 |
| mborzecki | well, i guess i hope it did | 08:36 |
| zyga | --debug doesn't work if project prepare fails | 08:40 |
| zyga | WAAAAAAAAAAT | 08:40 |
| zyga | ah, no, spread is picky where --debug is | 08:40 |
| zyga | man | 08:40 |
| zyga | mborzecki: the plot thickens, the ignore patterns work | 08:43 |
| zyga | but it's removal that matters now :D | 08:43 |
| zyga | man, dpkg, you sure are picky | 08:43 |
| Chipaca | zyga: what're you fighting against? I think I missed that bit | 08:46 |
| zyga | Chipaca: being able to spread a tree with built files while also passing debian build package thing that hates them | 08:47 |
| zyga | Chipaca: almost there though | 08:47 |
| zyga | now that I see what the error message is | 08:47 |
| Chipaca | zyga: ah | 08:48 |
| Chipaca | ondra: if/when you have more time to poke at that slow boot, given your last pastebin, the next step would be to do a profiling of the slow step | 08:48 |
| ondra | Chipaca I did some progress with more logs | 08:49 |
| ondra | Chipaca in the meeting now, but will report back | 08:49 |
| Chipaca | k | 08:49 |
| mborzecki | zyga: there was a snap.mount unit before? | 08:57 |
| zyga | mborzecki: I got it :) | 08:57 |
| zyga | mborzecki: so... back to your question | 08:58 |
| zyga | there used to be but we nuked it and got the snap.mount.service | 08:58 |
| zyga | AFAIR | 08:58 |
| zyga | mborzecki: why? | 08:59 |
| zyga | mborzecki: but I don't recall exactly why | 08:59 |
| zyga | mborzecki: gobs of complexity | 08:59 |
| zyga | mborzecki: note there's also a snapd-generator | 08:59 |
| zyga | mborzecki: look at cmd/snapd-generator/ | 08:59 |
| zyga | it creates a mount unit AFAIR | 08:59 |
| mborzecki | looking at some postinst helpers, and there's a comment about snap.mount in 2.31 | 08:59 |
| zyga | I think we used to have snap.mount | 08:59 |
| mborzecki | deb postinst | 08:59 |
| zyga | but it was too "strong" | 09:00 |
| zyga | so we moved it to the generator conditionally | 09:00 |
| zyga | or something like that | 09:00 |
| zyga | and the snap.mount.service is a separate beast for $REASONS | 09:00 |
| zyga | I honestly don't recall, I'd have to dig | 09:00 |
| * Chipaca steps away for a bit (washing machine beeping) | 09:00 | |
| mborzecki | zyga: no need, just curious | 09:01 |
| zyga | one more place and I think this is good | 09:04 |
| zyga | mborzecki: heh, now the delta code sends the crap again | 09:09 |
| zyga | or is it... | 09:09 |
| zyga | I don't see it in the tree anymore | 09:10 |
| zyga | but I do see it in the tarball we make early on | 09:10 |
| mborzecki | ehh debhelper | 09:34 |
| Chipaca | https://bugs.launchpad.net/snapd/+bugs?field.status%3Alist=NEW is a thing of beauty | 09:37 |
| seb128 | Chipaca, try that on the ubuntu package :) | 09:38 |
| * Chipaca kickbans seb128 | 09:38 | |
| seb128 | sorry *g* | 09:38 |
| zyga | mborzecki: it's fun, disabled the repack helper for a second and I see tar is still ask to send the stuff over | 09:38 |
| Chipaca | :*) | 09:38 |
| zyga | mborzecki: maybe it's a bug in tar | 09:38 |
| zyga | checking | 09:38 |
| zyga | yeah | 09:42 |
| zyga | tar is not respecting those | 09:42 |
| zyga | looking deeper | 09:42 |
| zyga | man this used to be easier | 09:42 |
| mvo | Chipaca: no open bugs | 09:44 |
| mvo | Chipaca: \o/ | 09:45 |
| zyga | mvo: no open bugs? | 09:46 |
| zyga | did we just close them for 5 minutes to feel better ;) ? | 09:46 |
| mvo | zyga: thats what LP told me | 09:46 |
| mvo | " | 09:46 |
| mvo | There are currently no open bugs. | 09:46 |
| mvo | " | 09:46 |
| mvo | zyga: in https://bugs.launchpad.net/snapd/+bugs?field.status%3Alist=NEW | 09:46 |
| mvo | zyga: its a lie of course, it should be "no open bugs in this filtered view" | 09:46 |
| zyga | :D | 09:47 |
| zyga | I see it now | 09:47 |
| mvo | but I like the other string better | 09:47 |
| mvo | :) | 09:47 |
| zyga | 369 open bugs make a man feel honest about himself and that to err is human | 09:47 |
| mvo | being a software developer humbles me every single day^Wminute | 09:47 |
| Chipaca | zyga: #1761621 sounds like fun for you | 09:48 |
| zyga | mmm | 09:49 |
| Chipaca | zyga: it's private because of a stacktrace, i presume, let me know if you can't see it | 09:49 |
| zyga | I can | 09:49 |
| zyga | I'll look later todaty | 09:50 |
| Chipaca | ah i see you in the list, even | 09:50 |
| Chipaca | anybody with better stacktrace-reading chops than myself (basically all of y'all), #1762686 is a puzzle | 09:57 |
| Chipaca | hah! I'd forgotten the "Ubuntu was created for the red-eyed onanists engaged in anal balancing act" | 10:01 |
| Chipaca | I wonder if there's a non-escalating way of telling them that was unappropriate | 10:08 |
| Chipaca | inappropriate, even | 10:08 |
| cjwatson | That isn't just obvious ban-fodder? | 10:13 |
| Chipaca | mvo (and maybe cachio when he's around): anything we should do about #1734845 ? | 10:13 |
| mup | Bug #1734845: hook (core) configure → exit status 1 cannot create lock directory /run/snapd/lock → Permission denied <snapd (Ubuntu):New> <https://launchpad.net/bugs/1734845> | 10:13 |
| Chipaca | cjwatson: I don't know the policy, nor the tools available to enforce it; maybe i should, if i'm going to start calling people out on this kind of stuff | 10:14 |
| Chipaca | and I am going to call people out on it | 10:14 |
| cjwatson | Was this on Launchpad? | 10:14 |
| Chipaca | yes | 10:14 |
| cjwatson | I failed to find it | 10:14 |
| Chipaca | cjwatson: #1818584 | 10:15 |
| mup | Bug #1818584: snaps applications can't open files on an USB key <amd64> <apport-bug> <disco> <snapd (Ubuntu):Invalid> <https://launchpad.net/bugs/1818584> | 10:15 |
| cjwatson | OK, your response is probably appropriate for what seems like a first offence from a frustrated person. If they escalate let me know and we'll take administrative action | 10:16 |
| Chipaca | cjwatson: will do | 10:17 |
| Chipaca | mvo: your input (or your redirect to somebody else) on #1747735 would be good | 10:26 |
| mup | Bug #1747735: command-completion for 'snap' on 14.04 does not recommend snapd <trusty> <command-not-found (Ubuntu):Won't Fix> <snapd (Ubuntu):Invalid> <https://launchpad.net/bugs/1747735> | 10:26 |
| mvo | Chipaca: in a meeting, will get back to you | 10:27 |
| Chipaca | a bug for snapd/ubuntu: gnome display manager it's slow with devuan | 10:31 |
| * Chipaca is nearly WATed out | 10:32 | |
| zyga | mborzecki: debugged it | 10:32 |
| zyga | maaaan | 10:32 |
| mborzecki | zyga: ok, i think i have worked around dh_systemd_start, but heh it's ugly | 10:32 |
| ogra | Chipaca, stop complaining, just fix it ! | 10:33 |
| ogra | (its just a quick port to sysvinit to fix that devuan problem) | 10:34 |
| Chipaca | ogra: in the _description_ of the bug they say "it was only solved by installing the Nvidia drivers." | 10:34 |
| Chipaca | ogra: so the bug is... probably? that nouveau drivers are slow? | 10:35 |
| ogra | ah, then it is easy, just make the nvidia drivers a hard dep of snapd | 10:35 |
| Chipaca | chocolate por la noticia | 10:35 |
| ogra | (how would it work on devuan *at all* ? there is no systemd, logind or any other of the lennartisms in it) | 10:36 |
| mup | PR snapd#7578 closed: sandbox/cgroup, overlord/snapstate: move helper for listing pids in group to the cgroup package <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7578> | 10:40 |
| mup | PR snapcraft#2743 closed: meta: warn about command mangling <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/2743> | 10:49 |
| mup | PR core18#140 closed: run-snapd-from-snap: check for snapd.service existing too <Created by anonymouse64> <Merged by sil2100> <https://github.com/snapcore/core18/pull/140> | 10:52 |
| ondra | Chipaca hi | 10:53 |
| ondra | Chipaca I added more logs and essentially this is all down to sha384 :( | 10:54 |
| Chipaca | ondra: I feel like you owe me a beer or something for not believing me | 10:54 |
| ondra | Chipaca as when we preparing seed, we calculate sha384 while we are parsing seed.yaml | 10:54 |
| Chipaca | ondra: would've saved us both a few hours of work :) | 10:55 |
| ondra | Chipaca I'm happy to get you beer :) | 10:55 |
| ondra | Chipaca but it's not key generation, this is snap assertion validation | 10:55 |
| Chipaca | ondra: strange that you don't see the io or cpu usage of it though, maybe things aren't lining up properly in the bootchart? | 10:55 |
| ondra | Chipaca also interesting one is, if I calculate it from shell, I get 100% cpu load and I get it also 2 as fast for all snapc | 10:56 |
| Chipaca | ondra: _before_ even looking at key gen etc i said it was probably still the sha3 | 10:56 |
| Chipaca | :-/ | 10:56 |
| ondra | Chipaca I loop through all snaps, and it's not taking me as much time | 10:56 |
| ondra | Chipaca ah, sorry for that, I did miss sha3 message from you | 10:57 |
| ondra | Chipaca I did not know we calculate sha3 before we start seeding, I though we do that at time of actual snap seeding | 10:57 |
| * pstolowski walk, bbiab | 10:57 | |
| ondra | Chipaca but cpu load is indeed something puzzling | 10:57 |
| mup | PR snapd#7580 closed: spread.yaml: exclude vendor dir <Simple 😃> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7580> | 10:58 |
| Chipaca | ondra: look for an email with subject 'booting the pi', 26 Jan 2017 | 10:58 |
| mvo | Chipaca: snap on trusty - oh well | 10:58 |
| Chipaca | mvo: ikr | 11:00 |
| ondra | Chipaca ahh nice, found it | 11:00 |
| === asymptotically is now known as Guest37136 | ||
| === asymptotically2 is now known as asymptotically | ||
| mvo | sil2100: thanks for merging the latest core18 PR from ijohnson ! our of curiosity, when do we plan the next stable core18 snap release? | 11:03 |
| mvo | sil2100: we have this fix and the netplan update in edge, would be great if those would make it to stable soon(ish) | 11:04 |
| Chipaca | ondra: wrt looping through all the snaps: did you unmount them and drop caches first | 11:04 |
| Chipaca | ondra: it's possible a large chunk of it is i/o | 11:04 |
| Chipaca | depends on how slow the sd card is maybe | 11:04 |
| ondra | Chipaca no I did not umout them | 11:04 |
| Chipaca | ondra: i'm assuming having them mounted keeps them in the cache somehow but could be wrong | 11:05 |
| Chipaca | ondra: also, what are you using to hash them from shell? | 11:05 |
| sil2100 | mvo: yw! In theory today's a Thursday, so if the timing is right it should get picked up by automation into beta and then to certification testing - we might be able to get it to stable around Monday | 11:05 |
| mvo | sil2100: amazing, thank you | 11:05 |
| ondra | Chipaca may be some, but I doubt entire snap and all, that would be half of all mem available | 11:05 |
| Chipaca | ondra: because sha3-384 is not sha384 | 11:06 |
| ondra | Chipaca I just called sha384sum | 11:06 |
| Chipaca | ondra: yeah, that's the wrong hash | 11:06 |
| ondra | Chipaca ah | 11:06 |
| Chipaca | that's an easy hash :) | 11:06 |
| ondra | Chipaca which one to use then? | 11:06 |
| Chipaca | ondra: snap install sha3384 ? | 11:06 |
| mvo | pedronis: 7556 has two +1, my two nitpicks can easily be done in a followup, I would love to see this landing so that the next pr is smaller, wdyt? | 11:07 |
| diddledan | re: https://discourse.ubuntu.com/t/testing-default-snap-apps-against-equivalent-deb-apps-slow-start-times/12875/17 the desktop launcher script runs gdk-pixbuf-query-loaders and gtk-update-icon-cache and gio-querymodules.. could we utilise the $SNAP_DATA or $SNAP_COMMON directories for these caches and generate them in an install and/or refresh hook? | 11:09 |
| mup | PR snapcraft#2746 closed: elf: handle missing dependencies not found on system <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2746> | 11:10 |
| ondra | Chipaca yep. now time alines more | 11:15 |
| ondra | Chipaca damn it, no cheap wins :( | 11:15 |
| Chipaca | diddledan: is there a way to do it incrementally so a user can also have local stuff? | 11:16 |
| diddledan | those caches only parse files that are distributed within the snap or runtime-snap so the user won't have their own local stuff, AFAIK | 11:17 |
| Chipaca | then why is it done at install at all? | 11:18 |
| Chipaca | why isn't it in the snap itself? | 11:18 |
| Chipaca | anyway, ⇝ lunch | 11:18 |
| diddledan | it is in the snap currently as part of desktop-launch script which wraps the actual command | 11:18 |
| diddledan | I was thinking we could extract it from running every launch to only running once when the snap is installed or refreshed | 11:19 |
| diddledan | at the moment it conditionally runs on refresh at launch time. so we can move it to refresh-time so that the initial launch after a refresh is perceviably faster | 11:20 |
| diddledan | i.e. in the launch script it has equivalent of `if refreshed, rebuild caches, then run the app` | 11:22 |
| mvo | mborzecki: did you had a chance to look at the error log in 7572? | 11:23 |
| mup | PR snapd#7572 closed: gadget: rename "boot{select,img}" -> system-boot-{select,image} <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7572> | 11:24 |
| mborzecki | yeah, but it dodn't make any sense though, if it keeps repeating we'll probably need to take a closer look at either glibc or go in arch | 11:30 |
| mborzecki | mvo: ^^ | 11:30 |
| mvo | mborzecki: ok, thats fine. lets assume its just a glitch | 11:32 |
| mborzecki | mvo: might be serious though, afaict it was related to thread local storage | 11:33 |
| mvo | mborzecki: hm, ok | 11:33 |
| mborzecki | zyga: tests/main/lxd is leaking this: https://paste.ubuntu.com/p/zkY96DMsT6/ | 11:37 |
| mborzecki | zyga: or, iow lxd is leakingthis | 11:38 |
| mborzecki | i suppose the debug output should include findmnt | 11:38 |
| zyga | that's curious | 11:38 |
| zyga | the common/ns/mntns thing is a bind mounted mount namespace | 11:38 |
| zyga | I really really wish lxd removal would clean up | 11:38 |
| zyga | mborzecki: can we paper over this with lxd-tool patch? | 11:39 |
| zyga | mborzecki: tests/lib/bin/lxd-tool | 11:39 |
| zyga | though it's more serious as it seems to break actual "snap remove", right? | 11:39 |
| mborzecki | zyga: we can, stil, removing lxd snap does not clean this up | 11:39 |
| mborzecki | yup | 11:39 |
| mup | PR snapd#7582 opened: spread: include mounts list in task debug output <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7582> | 11:42 |
| mborzecki | zyga: ^^ | 11:42 |
| zyga | mborzecki: +1 but would you mind changing that to co via prepare-restore.sh --debug-each and move the code there. This way more shellcheck love is applied. | 11:49 |
| mborzecki | zyga: this is covered by spread-shellcheck already | 11:50 |
| zyga | debug: | ? | 11:50 |
| mborzecki | zyga: i mean spread.yaml | 11:50 |
| mborzecki | hmm let me double check | 11:50 |
| zyga | mborzecki: https://github.com/snapcore/spread/pull/89 | 11:52 |
| mup | PR spread#89: runner: pass --exclude-vcs{,-ignores} to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89> | 11:52 |
| mborzecki | zyga: yup, debug && debug-each are covered by spread-shellcheck | 11:52 |
| zyga | mborzecki: fair enough, +1 | 11:52 |
| zyga | though I'd still prefer to have less things in the main yaml | 11:52 |
| zyga | but that's not a problem for this | 11:52 |
| zyga | mborzecki: I think we should re-check the leak checker | 11:53 |
| zyga | mborzecki: and see if we can land it for classic | 11:53 |
| zyga | mborzecki: even if core is still leaking other stuff I didn't fix | 11:53 |
| mborzecki | Chipaca: mvo: https://github.com/snapcore/spread/pull/89 might interest you as well | 11:53 |
| mup | PR spread#89: runner: pass --exclude-vcs{,-ignores} to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89> | 11:53 |
| zyga | mborzecki: we'd know earlier | 11:53 |
| mborzecki | zyga: wodner if that's something that snapd could police as well | 11:56 |
| zyga | mborzecki: that's a whole new world there | 11:56 |
| mborzecki | otoh lxd is a special snowflake | 11:56 |
| === ricab is now known as ricab|lunch | ||
| mup | PR snapd#7583 opened: usersession: drive by fixes for things flagged by unused or gosimple <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7583> | 12:07 |
| mborzecki | really simple PR ^^ | 12:07 |
| Chipaca | I wish I knew what these stacktraces were about | 12:19 |
| Chipaca | they're just … noise | 12:19 |
| mup | PR snapd#7576 closed: spread.yaml,.gitignore: sync ignored files <Simple 😃> <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/7576> | 12:20 |
| zyga | brb | 12:25 |
| mborzecki | Chipaca: which ones? | 12:42 |
| Chipaca | mborzecki: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1841384 or https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1762686 for ex | 12:42 |
| mup | Bug #1841384: snapd crashed with SIGSEGV <amd64> <apport-crash> <eoan> <snapd (Ubuntu):New> <https://launchpad.net/bugs/1841384> | 12:42 |
| mborzecki | Chipaca: interesting, in https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1762686 there's an awful lot of threads stuck in receiving from a client, probably via the snapd socket | 12:47 |
| mup | Bug #1811534 changed: snap - install skype warning - main.go: should be wrapped in (i18n) <i18n> <snapd:Incomplete> <snapd (Ubuntu):Incomplete> <https://launchpad.net/bugs/1811534> | 12:49 |
| mup | PR snapcraft#2748 closed: meta: warn about command mangling <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2748> | 12:49 |
| mborzecki | Chipaca: then in the other one, there's only a single thread that's not stuck in something sleep related, and it's calling runtime.scanobject (b=824633781392, gcw=0xc000044170) at /usr/lib/go-1.11/src/runtime/mgcmark.go:1140 | 12:50 |
| mborzecki | Chipaca: hm memory corruption perhaps? | 12:50 |
| zyga | man, it's raining like hell again | 12:53 |
| * mborzecki was hoping for a bike ride today | 12:53 | |
| zyga | yeah | 12:54 |
| zyga | me too | 12:54 |
| zyga | it's looking like Saturday will be nice | 12:54 |
| zyga | 20C and sun, wow | 12:54 |
| mup | PR snapd#7584 opened: .gitignore: pair of trivial changes <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/7584> | 13:07 |
| zyga | mborzecki: ^ | 13:10 |
| mup | PR snapd#7585 opened: spread.yaml: drop exclude list, use .gitignore <Created by zyga> <https://github.com/snapcore/snapd/pull/7585> | 13:12 |
| zyga | pstolowski: it's a one liner to measure the time it really took to run configure | 13:18 |
| zyga | pstolowski: I can show you where to patch | 13:18 |
| zyga | pstolowski: it's useful to do this if you have the rest of the setup | 13:18 |
| pstolowski | zyga: sure | 13:19 |
| zyga | pstolowski: on an os/exec.Cmd you can get .ProcessState which has .SystemTime and .UserTime | 13:20 |
| zyga | just log them after the process terminates | 13:20 |
| zyga | it's that simple | 13:20 |
| pstolowski | zyga: ah, that, right, i think we discussed that at some point some time ago | 13:21 |
| zyga | yeah, JFinallyDI | 13:21 |
| === ricab|lunch is now known as ricab | ||
| zyga | mvo: if you can review the spread change it woulod help me a little | 13:38 |
| mvo | zyga: sure thing | 13:39 |
| mvo | zyga: you may need to remind me toromorrow though | 13:39 |
| mvo | zyga: today is super busy :/ | 13:39 |
| mvo | (for me) | 13:39 |
| zyga | mvo: it's a one liner | 13:39 |
| zyga | https://github.com/snapcore/spread/pull/89 :) | 13:40 |
| mup | PR spread#89: runner: pass --exclude-vcs-ignores to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89> | 13:40 |
| cmatsuoka | cachio: I got this error in the test log, do you know how can I get more information on what failed? https://api.travis-ci.org/v3/job/596058356/log.txt | 13:51 |
| cmatsuoka | cachio: look for "build failed" | 13:51 |
| cmatsuoka | cachio: ah ok, just found an error messages several lines above that | 13:52 |
| cmatsuoka | cachio: but that line number doesn't make any sense :\ | 13:52 |
| ijohnson | ondra, Chipaca if you want faster SHA3 on arm, please +1 this issue https://github.com/golang/go/issues/28148 | 13:58 |
| zyga | cachio: perhaps you could look at https://github.com/snapcore/spread/pull/89 | 14:07 |
| mup | PR spread#89: runner: pass --exclude-vcs-ignores to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89> | 14:07 |
| zyga | pstolowski: can you please look at https://github.com/snapcore/snapd/pull/7584 -- 3 lines | 14:10 |
| mup | PR #7584: .gitignore: pair of trivial changes <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/7584> | 14:10 |
| zyga | gitignore | 14:10 |
| roadmr | does snapd identify if a snap requires a specific content snap and install the content snap automagically? | 14:18 |
| roadmr | (i.e. snapian) | 14:18 |
| mborzecki | roadmr: afaik snap.yaml must name a default-provider for a given content plug | 14:21 |
| roadmr | mborzecki: and the named default-provider snap gets auto-installed/ | 14:21 |
| roadmr | ? | 14:22 |
| mborzecki | roadmr: yes | 14:30 |
| roadmr | thanks mborzecki :) | 14:31 |
| Chipaca | ijohnson: out of curiosity, how hard would it be to pull that in to our snapd build for armhf? | 15:17 |
| roadmr | you're pulling my armhf | 15:17 |
| ijohnson | not too hard, it would be a little awkward IIRC because the hashers in crypto aren't used directly they're registered with the hash package and so we'd have to go use the hasher from sha384 directly rather than using crypto | 15:18 |
| ijohnson | Chipaca: ^ | 15:18 |
| ijohnson | the big thing why I don't think we should do that is because now we have to maintain that SHA3 crypto code | 15:18 |
| Chipaca | yeah i get that | 15:18 |
| Chipaca | but https://go-review.googlesource.com/q/hashtag:%22crypto-assembly%22+(status:open) | 15:19 |
| Chipaca | does not fill me with hope | 15:19 |
| ijohnson | Chipaca: yeah from talking some of the other go devs, the main person who does crypto + assembly review for Go is very very busy and has had a lot of other $GOOGLE things to do in recent years | 15:21 |
| Chipaca | brb, rebooting | 15:33 |
| mup | PR snapd#7583 closed: usersession: drive by fixes for things flagged by unused or gosimple <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7583> | 15:49 |
| mvo | pedronis: do you mind if I merge 7462 (the grade support for models in uc20?) | 15:52 |
| pedronis | mvo: I can do it, one sec | 15:53 |
| mup | PR snapd#7462 closed: asserts: introduce explicit support for grade for Core 20 models <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7462> | 15:54 |
| mvo | pedronis: thanks | 15:54 |
| mvo | pedronis: 7556 can probably also be merged, the suggestions could be done in a follouwp, this would unblock the next review I think(?) | 15:55 |
| pedronis | mvo: it's about documenting, maybe renaming TargetFunc, right ? | 15:56 |
| ondra | Chipaca I managed to shave 40s from that boot now :) | 15:57 |
| pedronis | mvo: and a typo | 15:57 |
| pedronis | mvo: I can merge and then I would apply those in the follow up | 15:57 |
| pedronis | if that's ok | 15:57 |
| ondra | Chipaca it will need reviewing from someone who can understand more implications of what I changed | 15:57 |
| pstolowski | mborzecki: some questions/comments to schedule fix PR | 15:57 |
| ondra | Chipaca but thats ~80s to ~40s | 15:58 |
| mvo | pedronis: yeah, lets do it this way (merge+followup) | 15:58 |
| ondra | Chipaca see bootchart https://private-fileshare.canonical.com/~okubik/bootchart-20191010-1545.svg | 15:58 |
| mvo | pedronis: thank you! | 15:59 |
| ondra | ogra https://private-fileshare.canonical.com/~okubik/bootchart-20191010-1545.svg | 15:59 |
| mup | PR snapd#7556 closed: image,seed/seedwriter: switch image to use seedwriter.Writer <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7556> | 15:59 |
| ondra | mvo pedronis this could be something we might consider for landing once polished, see the difference in cpu from 60 to ~100 second | 16:00 |
| ondra | https://private-fileshare.canonical.com/~okubik/bootchart-20191010-1545.svg and https://private-fileshare.canonical.com/~okubik/bootchart-20191008-1710.svg | 16:00 |
| Chipaca | ondra: what is the change? | 16:00 |
| mvo | ondra: what exactly? | 16:00 |
| mvo | ondra: do you have a diff or something? | 16:00 |
| ondra | Chipaca mvo diff soon | 16:01 |
| ondra | Chipaca mvo but I'm calculating sha3384 in parallel | 16:01 |
| ondra | so this will be even faster on multicore devices | 16:01 |
| ondra | on 4cores this cut hashing to half, and it's mostly because one snap is 3x bigger then next biggest | 16:02 |
| ondra | mvo Chipaca one of you just need to review if I'm not messing up with snap seeding order | 16:04 |
| pedronis | we are touching that code quite a bit atm | 16:04 |
| pedronis | so it's probably easiest to just see the diff and measure it and do it again if it's useful | 16:05 |
| ondra | pedronis are any of your changes parallelising hash calculation? | 16:06 |
| pedronis | no, but we intergrating UC20 code | 16:06 |
| pedronis | which does still differently | 16:06 |
| ondra | pedronis right | 16:06 |
| ondra | I will prepare clean diff | 16:06 |
| pedronis | is this a change in seed16.go ? | 16:06 |
| pedronis | or the older code? | 16:06 |
| ondra | I still think this is worth to consider, as we sit there for 2 minutes with one core maxed and rest of the system is idling | 16:07 |
| ondra | pedronis in seed16.go | 16:07 |
| pedronis | ok | 16:07 |
| pedronis | that shouldn't change a lot anymore | 16:07 |
| pedronis | but we'll see | 16:07 |
| pedronis | clean diff is a good start | 16:07 |
| ondra | pedronis +1 | 16:08 |
| ondra | Chipaca mvo it influenced order how snaps are seeded | 16:14 |
| ondra | so it will still need some tweaking there | 16:14 |
| pedronis | ondra: that doesn't sound right | 16:19 |
| pedronis | to be clear | 16:19 |
| ondra | pedronis well this is first shot and I'm just checking how to address this | 16:19 |
| pedronis | #7558 is ready for review | 16:22 |
| mup | PR #7558: boot,dirs,image: various refinements in the prepare-image code switched to seedwriter <Created by pedronis> <https://github.com/snapcore/snapd/pull/7558> | 16:22 |
| === pstolowski is now known as pstolowski|afk | ||
| pedronis | Chipaca: there are also bits in boot there ^ | 16:22 |
| Chipaca | k | 16:23 |
| Chipaca | i don't think i'll get to that today | 16:23 |
| pedronis | np | 16:23 |
| pedronis | just fyi | 16:23 |
| mup | PR snapd#7584 closed: .gitignore: pair of trivial changes <Simple 😃> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7584> | 16:43 |
| === genii_ is now known as genii | ||
| zyga | pedronis: you have two entries in the meeting notes for Thursday | 16:56 |
| zyga | is one of them for Friday or should they be merged? | 16:56 |
| Chipaca | zyga: one is pedronis' clone | 17:00 |
| zyga | that explains it :) | 17:00 |
| Chipaca | anyhoo, i'm outa here | 17:01 |
| Chipaca | see y'all tomorrow | 17:01 |
| zyga | good call :) | 17:01 |
| Chipaca | 👋 | 17:01 |
| * zyga hugs Chipaca | 17:01 | |
| zyga | bye :) | 17:01 |
| * Chipaca hugs back | 17:01 | |
| zyga | niemeyer: hello, low priority but also low effort request to look at a small change to spread: https://github.com/snapcore/spread/pull/89 | 17:18 |
| mup | PR spread#89: runner: pass --exclude-vcs-ignores to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89> | 17:19 |
| * cachio afk | 17:40 | |
| mup | PR snapcraft#2749 opened: storeapi: add StoreErrorList to handle store errors <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2749> | 20:34 |
| joedborg | hey zyga | 21:01 |
| zyga | hey | 21:01 |
| zyga | it's a little late so I may not respond instantly | 21:02 |
| zyga | but please leave me a message and I'll get to it as soon as I can | 21:02 |
| joedborg | zyga: ah apologies, i keep missing you | 21:02 |
| joedborg | just wondering if you can help with what exception i need to add to apparmor for | 21:03 |
| joedborg | `[285928.025967] audit: type=1400 audit(1570741250.597:1384598): apparmor="DENIED" operation="signal" profile="docker-default" pid=703 comm="kubelet" requested_mask="receive" denied_mask="receive" signal=kill peer="snap.kubernetes-worker.kubelet"` | 21:03 |
| zyga | no need to apologize, it's not our fault :) | 21:03 |
| zyga | sure | 21:03 |
| zyga | this means that the profile called 'docker-default' constrains a process with pid 703 so that it cannot receive the kill signal from a process labelled snap.kubernetes-worker.kubelet' | 21:04 |
| zyga | if you can craft the docker-default profile then it's a one liner to change | 21:04 |
| zyga | AFAIR, without checking something like: signal (receive) peer=snap.kubernetes-worker.kubelet, | 21:05 |
| zyga | note that the permission applies on both sides | 21:05 |
| zyga | so the snap.kubernetes-worker.kubelet needs a send permission with the appropriate peer | 21:06 |
| zyga | joedborg: does this make sense? | 21:06 |
| zyga | joedborg: you can constrain the signal further with set=(...) | 21:07 |
| joedborg | zyga: is there a quick line i can get round it with? | 21:07 |
| joedborg | * that i can add to app armor | 21:07 |
| zyga | signal (receive) set=(kill) peer=snap.kubernetes-worker.kubelet, | 21:07 |
| zyga | though note that the peer here specifies the snap name | 21:07 |
| zyga | so only that snap (and only that snap) would work | 21:07 |
| zyga | it may require some more ooomph | 21:08 |
| zyga | perhaps the signal could be accepted from any snap that can send it | 21:08 |
| zyga | so you could just say: | 21:08 |
| zyga | signal (receive) set=(kill) peer=snap.* | 21:08 |
| zyga | (make sure to add the trailing comma) | 21:08 |
| joedborg | zyga: thanks, i'll try that now | 21:09 |
| joedborg | it's just a further PoC, so I can arrest it if and when it works :) | 21:09 |
| zyga | joedborg: cool :) | 21:11 |
| zyga | joedborg: yeah, try that, let me know if it works or not | 21:11 |
| joedborg | zyga: sadly not, I added that to the profile `/var/lib/snapd/apparmor/profiles/snap.kubernetes-worker.kubelet¬ | 21:12 |
| joedborg | * `/var/lib/snapd/apparmor/profiles/snap.kubernetes-worker.kubelet` | 21:12 |
| zyga | that's not the profile | 21:12 |
| zyga | look at what I said above | 21:12 |
| zyga | this is confining the process using the docker-default profile | 21:12 |
| zyga | whatever owns and writes *that* profile must change | 21:12 |
| zyga | when you have two processes with apparmor profiles | 21:12 |
| zyga | and they signal each other | 21:12 |
| zyga | the sender needs the send permission for the peer | 21:13 |
| zyga | (and you have that or the denial would say profile=snap.foo.bar) | 21:13 |
| zyga | the recipient needs the corresponding permission | 21:13 |
| zyga | that matches the peer | 21:13 |
| zyga | it's funny but you can craft an apparmor profile that denies sigkill with this | 21:14 |
| zyga | (from unconfined) | 21:14 |
| zyga | joedborg: does this make sense? | 21:14 |
| joedborg | zyga: how do i get at the docker-default profile? | 21:14 |
| zyga | joedborg: I don't know that, presumably something writes one, one sec | 21:14 |
| joedborg | zyga: i can't see it in `/var/lib/snapd/apparmor/profiles/` | 21:15 |
| zyga | it's not a profile made by snapd | 21:15 |
| zyga | so I suspect the snap itsef makes it | 21:15 |
| zyga | ijohnson: ^^ perhaps you know? | 21:15 |
| zyga | joedborg: in other words, no amount of changes to snapd will fix it | 21:16 |
| ijohnson | let me read the scrollback | 21:16 |
| zyga | the profile you must change is made or belongs to another package, presumably just docker | 21:16 |
| zyga | ijohnson: tl;dr; do you know if docker snap makes "docker-default" apparmor profile? | 21:16 |
| zyga | and if so, do you know how to change the code that makes it so that it has one more line | 21:16 |
| ijohnson | ah yes docker-default is generated by docker/runc when you create containers | 21:17 |
| ijohnson | in the docker snap we had to patch the docker-default profile to allow containers to start properly when dockerd is confined by snapd/apparmor | 21:17 |
| joedborg | ijohnson zyga so there's nothing i can do on a running system to patch this? | 21:18 |
| ijohnson | essentially, the default docker-default profile only allows unconfined processes to talk to it, so confined ones like dockerd inside the docker snap are denied as you see above | 21:18 |
| ijohnson | joedborg: unfortunately IIRC dockerd generates the docker-default profile in /tmp, loads it into the kernel and then deletes the /tmp profile, so no I don't think you can do to a running system to patch this | 21:19 |
| zyga | joedborg: not without fighting with docker | 21:19 |
| ijohnson | you need to patch dockerd to modify the docker-default profile to include that line | 21:19 |
| joedborg | zyga: ijohnson well containerd in my case | 21:19 |
| zyga | joedborg: you could create a profile that is the same, has one more permission, and load that | 21:19 |
| ijohnson | err yeah | 21:19 |
| joedborg | zyga: ijohnson: I can carry on in devmode and log it as a bug? | 21:20 |
| zyga | but that would have to be done again each time docker decided to reload the profiles | 21:20 |
| ijohnson | containerd/dockerd/runc/moby/the kitchen sink/a bag of chips/ | 21:20 |
| zyga | joedborg: you'd have to run docker in devmode | 21:20 |
| ijohnson | jodbord: nah I don't think so, because you need the docker-default profile to be in devmode, not the snap | 21:20 |
| ijohnson | there's a way to launch a container with containers having their apparmor profiles turned off though | 21:20 |
| ijohnson | err that was akward to say | 21:21 |
| zyga | actually | 21:21 |
| ijohnson | there's an option to dockerd/containerd/whatever to launch containers without the docker-default profile | 21:21 |
| zyga | nah, that's not a good idea | 21:21 |
| ijohnson | I can't remember off the top of my head | 21:21 |
| * zyga remains quiet | 21:21 | |
| ijohnson | zyga: yeah it's not a good idea, but it's the same as running in devmode | 21:21 |
| joedborg | zyga: ijohnson: so this is for the kubernetes-worker snap which has containerd bundled inside the snap | 21:21 |
| zyga | I was thinking about using aa_complain | 21:21 |
| zyga | but I think that only works if you can point it to a profile text | 21:21 |
| ijohnson | joedborg: ah if you're building the snap then you can just patch dockerd/containerd | 21:22 |
| joedborg | ijohnson: how? :) | 21:22 |
| ijohnson | joedborg: this is the patch we used in the docker snap for github.com/docker/docker (aka github.com/moby/moby): https://git.launchpad.net/~docker/+git/snap/tree/dockerd-patches/snappy-apparmor-tweaks.patch | 21:23 |
| * zyga doesn't know that and returns to audiobooks and assembly | 21:23 | |
| ijohnson | zyga: go do that and stop working :-) | 21:24 |
| zyga | thank you ijohnson | 21:24 |
| zyga | joedborg: you are in good hands :) | 21:24 |
| joedborg | thanks zyga, sorry to have kept you at your desk | 21:24 |
| ijohnson | joedborg: how are you building containerd? | 21:24 |
| joedborg | ijohnson https://github.com/charmed-kubernetes/snap-kubernetes-worker/blob/master/snap/snapcraft.yaml | 21:25 |
| joedborg | it's being built in the snap | 21:25 |
| ijohnson | hmm this snapcraft.yaml is a bit weird, is there a reason you use `go get ...` instead of the source spec for all these parts? | 21:26 |
| joedborg | ijohnson: i thought we had similar issue that jdstrand got into the containerd profile | 21:26 |
| joedborg | ijohnson: yeah because kubernetes doesn't build things properly | 21:26 |
| ijohnson | but anyways since you're building containerd from scratch patching it shouldn't be too much work | 21:26 |
| ijohnson | hmm well still you could let snapcraft check it out, that would save you rebuild times, but oh well | 21:27 |
| joedborg | ijohnson: can you give commit hashes to snapcraft? | 21:28 |
| ijohnson | yes, `source: github.com/....` and then do `source-commit: SHA` | 21:28 |
| ijohnson | joedborg: see https://snapcraft.io/docs/snapcraft-parts-metadata#heading--source | 21:29 |
| joedborg | ijohnson: thanks, i'll add that in the todo to look at | 21:29 |
| ijohnson | joedborg: so you need to patch https://github.com/containerd/containerd/blob/1ac546b3c4a3331a9997427052d1cb9888a2f3ef/contrib/apparmor/template.go | 21:31 |
| ijohnson | that's the "docker-default" profile in containerd AFAICT | 21:31 |
| ijohnson | I don't actually see anything named docker-default in containerd, so I assume it's probably k8s or something else that is using the docker-default name when driving containerd | 21:32 |
| joedborg | ijohnson: cool, thanks, and there's no way to do this outside of containerd upstream? I might (and probably) mixing different issues, but I think that jdstrand and I saw a similar signal violation (of containerd inside the snap) and fixed it in the template for the interdace | 21:33 |
| joedborg | *interface | 21:34 |
| ijohnson | joedborg: jdstrand is the apparmor expert so perhaps I'm mistaken but when I went through this excercise for the docker snap, we had to patch docker because the issue was not that the _snap_ processes were being denied stuff from apparmor, but rather that the _container_ processes were being denied stuff and the container processes are under a different apparmor label, so that different label has to be modified | 21:35 |
| ijohnson | If you look in docker-support interface, you can see that there's a rule to allow transition between the snap apparmor label and an apparmor profile named docker-default, so dockerd can launch a process, which will be confined by the snap's apparmor profile, then have that process transition to the container apparmor profile | 21:36 |
| ijohnson | I see that this k8s snap is using docker-support, so actually I assume that is part of why you need to use docker-default even when k8s is driving containerd, as there's no transition rule for i.e. containerd-default apparmor profile that I see | 21:37 |
| ijohnson | joedborg: the only other thing I could think of would be to launch every container with a manually specified apparmor profile, but that might interfere with users who have their own apparmor profile they want to launch containers with | 21:38 |
| joedborg | ijohnson: yeah i get that, it's an issue we've been seeing a lot - where directories inside the container are being given readonly; even though they're confined by the pivot route | 21:39 |
| joedborg | ijohnson: I suspect we added some to get the PoC working but, in fact, we need to open the containerd profile up a bit more by patching the file you suggested | 21:39 |
| joedborg | /pivot route/pivot root/ | 21:40 |
| ijohnson | joedborg: another useful patch you might want to look at from the docker snap is the one that changes pivot_root to just chroot all the time, because when you pivot_root then apparmor doesn't understand that your new files you touch are really prefixed with the old_root parameter | 21:40 |
| ijohnson | so that leads to you having to add all these random accesses that ideally wouldn't be necessary since the container isn't ever touching any host files or base snap files, it's just using it's own rootfs from $SNAP_DATA or somewhere, so it's fine to let it use whatever files it wants | 21:41 |
| ijohnson | joedborg: that patch is here: https://git.launchpad.net/~docker/+git/snap/tree/dockerd-patches/snappy-real-chroot.patch | 21:41 |
| joedborg | ijohnson: ah that’s sounding like we’re seeing the same thing, cool. Just reading that patch | 21:42 |
| ijohnson | yeah I recommended to someone a long time ago that as they start working on building k8s into a strict snap they should look at the docker snap, but apparently that advice was forgotten | 21:43 |
| ijohnson | 🤷♂️ | 21:43 |
| joedborg | ijohnson: yeah, docker isn’t cool anymore so we’ve gone down the containerd path 😊 | 21:44 |
| ijohnson | ... and yet still have the same problems 🤔 | 21:44 |
| ijohnson | 🙂 | 21:44 |
| mup | PR core-build#54 closed: initramfs: update unlock keyfile parameter <Created by cmatsuoka> <Merged by cmatsuoka> <https://github.com/snapcore/core-build/pull/54> | 21:46 |
| joedborg | ijohnson: haha yes, seem so. we had time pressure for the poc so I think the rules jdstrand put in were to get the basics cleared. we could reconcile when he’s back? it’d be nice to sort this properly (and we could get a free containerd snap too) | 21:46 |
| ijohnson | joedborg: yeah that sounds good | 21:47 |
| ijohnson | I think jdstrand is back next week iirc | 21:47 |
| joedborg | ijohnson: awesome. yeah I think so too. thanks a lot for your help :) | 21:48 |
| ijohnson | np, happy to help while z y g a should be offline :-) | 21:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
