mupPR snapd#7580 opened: spread.yaml: exclude vendor dir <Simple πŸ˜ƒ> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7580>02:15
mupPR snapd#7581 opened: tests: add netplan test on ubuntu core <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7581>02:29
mupPR snapcraft#2748 opened: meta: warn about command mangling <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2748>02:36
mupPR 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
mborzeckischool run, back in 30 or so05:20
mborzeckimvo: morning06:02
mupPR 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
mupPR 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
mvohey mborzecki06:07
mvomborzecki: good morning06:07
mborzeckimvo:  can you take a look at https://github.com/snapcore/snapd/pull/7571 ?06:08
mupPR #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
mvomborzecki: uh, I think I wanted to do this yesterday already :/06:09
mvomborzecki: let me do this *now*06:09
zygasorry, had super stubborn kids06:16
zygaone is still at home06:16
zygaugh :|06:16
zygaok, let's try to focus...06:17
mborzeckimvo: thanks!06:19
zygamvo: https://twitter.com/samerhaj/status/118212146481375641906:19
zygaand check out the next tweet there, vmware working on this?06:20
zygaesxi for raspi anyone?06:20
mborzeckizyga: wow, nice06:20
zygaI, for one, would not mind a raspberry pi with a proper boot stack06:21
mborzeckizyga:  any clue how does uefi/acpi combo could handle hats?06:21
zygano idea06:21
mvomborzecki: 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
mborzeckimvo: looking06:26
mborzeckimvo: 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
mvomborzecki: right, landing it is fine06:29
mvomborzecki: I was mostly wondering if consumers of the ProcGroup() need to know what matcher to create06:29
mvomborzecki: or if we could simply keep using a string here06:30
mvomborzecki: and the string in the future might be "snapd"06:30
mvomborzecki: or is this not how it will work :) ?06:30
* mvo obviously has less context06:30
mborzeckimvo: hm you'd have to pass name=snapd to be explicit, or "" when you mean a unified hierarchy, feels a bit too magical06:33
mvomborzecki: hm, but we know internally in the cgroup module if we are unified or mixed so could we leverage this knowledge?06:35
zygamvo: we use cgroup v1 even in unified mode06:35
zygaso perhaps it's all going to be simplified eventually06:35
zygas/use/going to use/06:35
mvook, 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
zygawe 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 release06:37
mborzeckimvo: 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
zygait is my understanding that by the time we are done it will use a different selector or a hard-coded string for the new location06:38
zygaI really don't think we need to do more now06:38
zygajust land and iterate06:38
zygameanwhile, I'm back to https://github.com/snapcore/snapd/pull/757606:38
mupPR #7576: spread.yaml,.gitignore: sync ignored files <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/7576>06:38
zygawhy does something so simple have to be so hard :D06:39
mvozyga: 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
zygauserd is using cgroups?06:40
mvozyga: I did a "git grep ProcGroup" and a match there06:40
mborzeckizyga: yeah, it is, that's the next thing, we'll probably need some api that encodes information about how snap cgrups are made06:40
mvozyga: fwiw, I disagree with "why does something so simple have to be so hard"06:41
mborzeckizyga: it's for checking whcih snap the talking to userd over dbus comes from06:41
zygaoh, I didn't notice that06:41
zygaI just read that code06:41
mvomborzecki: 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 package06:41
zygamvo: (I was referring to the gitignore PR)06:42
mvozyga: oh, sorry06:42
mvozyga: then I misunderstood :)06:42
zygamvo: ignoring in spread and git consistently is haaard :)06:42
mvozyga: I thought it was about my bikesheding api here :)06:42
zygaand it should not06:42
zygano no, sorry06:42
mvozyga: git hard> makes sense ;)06:42
zygamborzecki: I wonder if the pid usage in userd could go away06:42
zygaand use /proc/pid/root instead06:42
zygabut that's a separate conversation06:43
zygawe discussed this with james, as long term API for discovering snap processes06:43
zygabrb, need tea, coffee06:43
zygaI'll catch up on the thread here06:43
mborzeckimvo: 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-n06:43
zygabut mostly want to unbreak that gitignore branch06:43
* zyga loves the acronym s-u-n06:43
mborzeckimvo: 'groups are made' as in how the path under the relevant group mount point is constructured for a snap :)06:44
mvomborzecki: oh, this is spread out right now? yeah, a good reason to consolidate it :)06:44
mvomborzecki: 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 exa06:47
mvoctly we do cgroups outside of the cgroup package. does that make sense?06:47
zygaback with coffee06:49
zygamvo: I agree that eventually we should instead have an API that refers to app / hook tracking06:50
zygaand similar pid discovery06:50
zygaand encapsulate the cgroup logic there06:50
zygaI still would merge this as-is and iterate because velocity and not-worse-than-before06:50
zygamborzecki: last night I needed to sober up after looking at 22MB binaries06:51
zygaso I started making those few-kb, built-with-C, hello-world programs that don't need libc06:51
zygathat was refreshing06:51
zygaofftopic: I found a great use for raspi zero -- it's the cheapest reliable serial logging device I've had06:52
zygajust plug a wire and log away06:53
mborzeckimvo: right, i mean it makes sense, we should be able to figure everything out in cgrup package given the security tag06:53
zygamborzecki: I would not even use cgroup as the wrapper06:53
zygawe may end up not using cgroups eventually06:53
zygathe package should be conceptual - pid tracking and discovery, it may use cgroups internally06:54
mborzeckizyga: snaptrack? :P06:54
zygabut what it should really have is functions like PidsOfSnap and SnapOfPid or similar06:54
zygapidof ;-)06:54
mborzeckimvo: 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 pacakge06:54
mupPR #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
zygaback to gitignore06:55
mvomborzecki: 7578> sure - the title is promising for sure!06:58
mborzeckimvo: hope the content doesn't disappoint :P06:58
=== pstolowski|afk is now known as pstolowski
mborzeckipstolowski:  hey06:59
zygahey pawel :)06:59
* zyga reads how git does .gitignore07:04
zyga /o\07:06
zygagit has a screen full of global variables07:06
mvomborzecki: love it07:15
mvomborzecki: I would even go a bit further and hide pidsCgroupDir from the outside07:15
mvomborzecki: but fine as is07:16
mupPR 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
zygamborzecki: so I had a look at those exclude patterns07:47
zygaand I have the following compromise07:47
zygamborzecki: git needs /foo to exclude a file ./foo won't cut it07:47
zygamborzecki: spread's use of tar is exactly the opposite07:47
zygamborzecki: it requires ./foo and /foo won't cut it07:48
zygaI think it's fine if I document it07:48
zygait's better then sending garbage07:48
zygaand better than not working07:48
zygaI don't see any way out of this for now07:48
* mvo has some eoan problems today and regrets upgrading to it08:22
zygamvo: what happens?08:23
zygamvo: I'm running on a similar-ish thinkpad and apart from suspend resume killing input (ha ha) it works okay (sob)08:23
Chipacafitting that you regret upgrading to eoan in the morning08:25
mvozyga: its on my desktop, sound issues, stuttering and it always select the wrong output on login so I need to go to gnome-settings etc08:26
zygamborzecki: oh god08:26
zygamborzecki: tar has --exclude-vcs-ignores08:26
zygamborzecki: it reads .gitignore directly08:26
zygacan I just path spread instead?08:26
zygamvo: man, that sucks :/08:27
zygamvo: it's the future though, better bite the bullet and report them08:27
mvozyga: doing that but its a bit of a time sink this is why I regret it08:28
mvoChipaca: hahaha08:29
mvoChipaca: took a while until the penny dropped08:29
Chipacamvo: no regrets08:32
mborzeckizyga: hm can we teach spread about --exclude-vcs-ignores then?08:32
zygaI just did locally08:32
mborzeckiit'll take a while to land it though08:32
zygait doesn't seem to work08:33
zygarunning a debug session to see if the files really got copied08:33
zygait's driving me insane08:33
zygaI hate doing this, it breaks all C testing I work on08:33
mborzeckiwell, i'm trying to teach postrm and snap-mgmt about the snap.mount unit :/ so i feel you08:33
* zyga hugs mborzecki :)08:34
mborzeckibtw. 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 EBUSY08:35
zygaare you sure it stopped containers?08:35
mborzeckiwell, i guess i hope it did08:36
zyga--debug doesn't work if project prepare fails08:40
zygaah, no, spread is picky where --debug is08:40
zygamborzecki: the plot thickens, the ignore patterns work08:43
zygabut it's removal that matters now :D08:43
zygaman, dpkg, you sure are picky08:43
Chipacazyga: what're you fighting against? I think I missed that bit08:46
zygaChipaca: being able to spread a tree with built files while also passing debian build package thing that hates them08:47
zygaChipaca: almost there though08:47
zyganow that I see what the error message is08:47
Chipacazyga: ah08:48
Chipacaondra: 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 step08:48
ondraChipaca I did some progress with more logs08:49
ondraChipaca in the meeting now, but will report back08:49
mborzeckizyga:  there was a snap.mount unit before?08:57
zygamborzecki: I got it :)08:57
zygamborzecki: so... back to your question08:58
zygathere used to be but we nuked it and got the snap.mount.service08:58
zygamborzecki: why?08:59
zygamborzecki: but I don't recall exactly why08:59
zygamborzecki: gobs of complexity08:59
zygamborzecki: note there's also a snapd-generator08:59
zygamborzecki: look at cmd/snapd-generator/08:59
zygait creates a mount unit AFAIR08:59
mborzeckilooking at some postinst helpers, and there's a comment about snap.mount in 2.3108:59
zygaI think we used to have snap.mount08:59
mborzeckideb postinst08:59
zygabut it was too "strong"09:00
zygaso we moved it to the generator conditionally09:00
zygaor something like that09:00
zygaand the snap.mount.service is a separate beast for $REASONS09:00
zygaI honestly don't recall, I'd have to dig09:00
* Chipaca steps away for a bit (washing machine beeping)09:00
mborzeckizyga: no need, just curious09:01
zygaone more place and I think this is good09:04
zygamborzecki: heh, now the delta code sends the crap again09:09
zygaor is it...09:09
zygaI don't see it in the tree anymore09:10
zygabut I do see it in the tarball we make early on09:10
mborzeckiehh debhelper09:34
Chipacahttps://bugs.launchpad.net/snapd/+bugs?field.status%3Alist=NEW is a thing of beauty09:37
seb128Chipaca, try that on the ubuntu package :)09:38
* Chipaca kickbans seb128 09:38
seb128sorry *g*09:38
zygamborzecki: it's fun, disabled the repack helper for a second and I see tar is still ask to send the stuff over09:38
zygamborzecki: maybe it's a bug in tar09:38
zygatar is not respecting those09:42
zygalooking deeper09:42
zygaman this used to be easier09:42
mvoChipaca: no open bugs09:44
mvoChipaca: \o/09:45
zygamvo: no open bugs?09:46
zygadid we just close them for 5 minutes to feel better ;) ?09:46
mvozyga: thats what LP told me09:46
mvoThere are currently no open bugs.09:46
mvozyga: in https://bugs.launchpad.net/snapd/+bugs?field.status%3Alist=NEW09:46
mvozyga: its a lie of course, it should be "no open bugs in this filtered view"09:46
zygaI see it now09:47
mvobut I like the other string better09:47
zyga369 open bugs make a man feel honest about himself and that to err is human09:47
mvobeing a software developer humbles me every single day^Wminute09:47
Chipacazyga: #1761621 sounds like fun for you09:48
Chipacazyga: it's private because of a stacktrace, i presume, let me know if you can't see it09:49
zygaI can09:49
zygaI'll look later todaty09:50
Chipacaah i see you in the list, even09:50
Chipacaanybody with better stacktrace-reading chops than myself (basically all of y'all), #1762686 is a puzzle09:57
Chipacahah! I'd forgotten the "Ubuntu was created for the red-eyed onanists engaged in anal balancing act"10:01
ChipacaI wonder if there's a non-escalating way of telling them that was unappropriate10:08
Chipacainappropriate, even10:08
cjwatsonThat isn't just obvious ban-fodder?10:13
Chipacamvo (and maybe cachio when he's around): anything we should do about #1734845 ?10:13
mupBug #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
Chipacacjwatson: 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 stuff10:14
Chipacaand I am going to call people out on it10:14
cjwatsonWas this on Launchpad?10:14
cjwatsonI failed to find it10:14
Chipacacjwatson: #181858410:15
mupBug #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
cjwatsonOK, 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 action10:16
Chipacacjwatson: will do10:17
Chipacamvo: your input (or your redirect to somebody else) on #1747735 would be good10:26
mupBug #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
mvoChipaca: in a meeting, will get back to you10:27
Chipacaa bug for snapd/ubuntu: gnome display manager it's slow with devuan10:31
* Chipaca is nearly WATed out10:32
zygamborzecki: debugged it10:32
mborzeckizyga: ok, i think i have worked around dh_systemd_start, but heh it's ugly10:32
ograChipaca, stop complaining, just fix it !10:33
ogra(its just a quick port to sysvinit to fix that devuan problem)10:34
Chipacaogra: in the _description_ of the bug they say "it was only solved by installing the Nvidia drivers."10:34
Chipacaogra: so the bug is... probably? that nouveau drivers are slow?10:35
ograah, then it is easy, just make the nvidia drivers a hard dep of snapd10:35
Chipacachocolate por la noticia10: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
mupPR 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
mupPR snapcraft#2743 closed: meta: warn about command mangling <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/2743>10:49
mupPR 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
ondraChipaca hi10:53
ondraChipaca I added more logs and essentially this is all down to sha384 :(10:54
Chipacaondra: I feel like you owe me a beer or something for not believing me10:54
ondraChipaca as when we preparing seed, we calculate sha384 while we are parsing seed.yaml10:54
Chipacaondra: would've saved us both a few hours of work :)10:55
ondraChipaca I'm happy to get you beer :)10:55
ondraChipaca but it's not key generation, this is snap assertion validation10:55
Chipacaondra: 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
ondraChipaca 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 snapc10:56
Chipacaondra: _before_ even looking at key gen etc i said it was probably still the sha310:56
ondraChipaca I loop through all snaps, and it's not taking me as much time10:56
ondraChipaca ah, sorry for that, I did miss sha3 message from you10:57
ondraChipaca I did not know we calculate sha3 before we start seeding, I though we do that at time of actual snap seeding10:57
* pstolowski walk, bbiab10:57
ondraChipaca but cpu load is indeed something puzzling10:57
mupPR snapd#7580 closed: spread.yaml: exclude vendor dir <Simple πŸ˜ƒ> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7580>10:58
Chipacaondra: look for an email with subject 'booting the pi', 26 Jan 201710:58
mvoChipaca: snap on trusty - oh well10:58
Chipacamvo: ikr11:00
ondraChipaca ahh nice, found it11:00
=== asymptotically is now known as Guest37136
=== asymptotically2 is now known as asymptotically
mvosil2100: thanks for merging the latest core18 PR from ijohnson ! our of curiosity, when do we plan the next stable core18 snap release?11:03
mvosil2100: we have this fix and the netplan update in edge, would be great if those would make it to stable soon(ish)11:04
Chipacaondra: wrt looping through all the snaps: did you unmount them and drop caches first11:04
Chipacaondra: it's possible a large chunk of it is i/o11:04
Chipacadepends on how slow the sd card is maybe11:04
ondraChipaca no I did not umout them11:04
Chipacaondra: i'm assuming having them mounted keeps them in the cache somehow but could be wrong11:05
Chipacaondra: also, what are you using to hash them from shell?11:05
sil2100mvo: 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 Monday11:05
mvosil2100: amazing, thank you11:05
ondraChipaca may be some, but I doubt entire snap and all, that would be half of all mem available11:05
Chipacaondra: because sha3-384 is not sha38411:06
ondraChipaca I just called sha384sum11:06
Chipacaondra: yeah, that's the wrong hash11:06
ondraChipaca ah11:06
Chipacathat's an easy hash :)11:06
ondraChipaca which one to use then?11:06
Chipacaondra: snap install sha3384 ?11:06
mvopedronis: 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
diddledanre: 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
mupPR 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
ondraChipaca yep. now time alines more11:15
ondraChipaca damn it, no cheap wins :(11:15
Chipacadiddledan: is there a way to do it incrementally so a user can also have local stuff?11:16
diddledanthose caches only parse files that are distributed within the snap or runtime-snap so the user won't have their own local stuff, AFAIK11:17
Chipacathen why is it done at install at all?11:18
Chipacawhy isn't it in the snap itself?11:18
Chipacaanyway, ⇝ lunch11:18
diddledanit is in the snap currently as part of desktop-launch script which wraps the actual command11:18
diddledanI was thinking we could extract it from running every launch to only running once when the snap is installed or refreshed11:19
diddledanat 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 faster11:20
diddledani.e. in the launch script it has equivalent of `if refreshed, rebuild caches, then run the app`11:22
mvomborzecki: did you had a chance to look at the error log in 7572?11:23
mupPR 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
mborzeckiyeah, 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 arch11:30
mborzeckimvo: ^^11:30
mvomborzecki: ok, thats fine. lets assume its just a glitch11:32
mborzeckimvo:  might be serious though, afaict it was related to thread local storage11:33
mvomborzecki: hm, ok11:33
mborzeckizyga: tests/main/lxd is leaking this: https://paste.ubuntu.com/p/zkY96DMsT6/11:37
mborzeckizyga: or, iow lxd is leakingthis11:38
mborzeckii suppose the debug output should include findmnt11:38
zygathat's curious11:38
zygathe common/ns/mntns thing is a bind mounted mount namespace11:38
zygaI really really wish lxd removal would clean up11:38
zygamborzecki: can we paper over this with lxd-tool patch?11:39
zygamborzecki: tests/lib/bin/lxd-tool11:39
zygathough it's more serious as it seems to break actual "snap remove", right?11:39
mborzeckizyga: we can, stil, removing lxd snap does not clean this up11:39
mupPR snapd#7582 opened: spread: include mounts list in task debug output <Simple πŸ˜ƒ> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7582>11:42
mborzeckizyga: ^^11:42
zygamborzecki: +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
mborzeckizyga: this is covered by spread-shellcheck already11:50
zygadebug: | ?11:50
mborzeckizyga:  i mean spread.yaml11:50
mborzeckihmm let me double check11:50
zygamborzecki: https://github.com/snapcore/spread/pull/8911:52
mupPR spread#89: runner: pass --exclude-vcs{,-ignores} to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89>11:52
mborzeckizyga: yup, debug && debug-each are covered by spread-shellcheck11:52
zygamborzecki: fair enough, +111:52
zygathough I'd still prefer to have less things in the main yaml11:52
zygabut that's not a problem for this11:52
zygamborzecki: I think we should re-check the leak checker11:53
zygamborzecki: and see if we can land it for classic11:53
zygamborzecki: even if core is still leaking other stuff I didn't fix11:53
mborzeckiChipaca: mvo: https://github.com/snapcore/spread/pull/89 might interest you as well11:53
mupPR spread#89: runner: pass --exclude-vcs{,-ignores} to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89>11:53
zygamborzecki: we'd know earlier11:53
mborzeckizyga: wodner if that's something that snapd could police as well11:56
zygamborzecki: that's a whole new world there11:56
mborzeckiotoh lxd is a special snowflake11:56
=== ricab is now known as ricab|lunch
mupPR 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
mborzeckireally simple PR ^^12:07
ChipacaI wish I knew what these stacktraces were about12:19
Chipacathey're just … noise12:19
mupPR 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
mborzeckiChipaca: which ones?12:42
Chipacamborzecki: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1841384 or https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1762686 for ex12:42
mupBug #1841384: snapd crashed with SIGSEGV <amd64> <apport-crash> <eoan> <snapd (Ubuntu):New> <https://launchpad.net/bugs/1841384>12:42
mborzeckiChipaca: 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 socket12:47
mupBug #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
mupPR snapcraft#2748 closed: meta: warn about command mangling <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2748>12:49
mborzeckiChipaca: 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:114012:50
mborzeckiChipaca: hm memory corruption perhaps?12:50
zygaman, it's raining like hell again12:53
* mborzecki was hoping for a bike ride today12:53
zygame too12:54
zygait's looking like Saturday will be nice12:54
zyga20C and sun, wow12:54
mupPR snapd#7584 opened: .gitignore: pair of trivial changes <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/7584>13:07
zygamborzecki: ^13:10
mupPR snapd#7585 opened: spread.yaml: drop exclude list, use .gitignore <Created by zyga> <https://github.com/snapcore/snapd/pull/7585>13:12
zygapstolowski: it's a one liner to measure the time it really took to run configure13:18
zygapstolowski: I can show you where to patch13:18
zygapstolowski: it's useful to do this if you have the rest of the setup13:18
pstolowskizyga: sure13:19
zygapstolowski: on an os/exec.Cmd you can get .ProcessState which has .SystemTime and .UserTime13:20
zygajust log them after the process terminates13:20
zygait's that simple13:20
pstolowskizyga: ah, that, right, i think we discussed that at some point some time ago13:21
zygayeah, JFinallyDI13:21
=== ricab|lunch is now known as ricab
zygamvo: if you can review the spread change it woulod help me a little13:38
mvozyga: sure thing13:39
mvozyga: you may need to remind me toromorrow though13:39
mvozyga: today is super busy :/13:39
mvo(for me)13:39
zygamvo: it's a one liner13:39
zygahttps://github.com/snapcore/spread/pull/89 :)13:40
mupPR spread#89: runner: pass --exclude-vcs-ignores to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89>13:40
cmatsuokacachio: 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.txt13:51
cmatsuokacachio: look for "build failed"13:51
cmatsuokacachio: ah ok, just found an error messages several lines above that13:52
cmatsuokacachio: but that line number doesn't make any sense :\13:52
ijohnsonondra, Chipaca if you want faster SHA3 on arm, please +1 this issue https://github.com/golang/go/issues/2814813:58
zygacachio: perhaps you could look at https://github.com/snapcore/spread/pull/8914:07
mupPR spread#89: runner: pass --exclude-vcs-ignores to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89>14:07
zygapstolowski: can you please look at https://github.com/snapcore/snapd/pull/7584 -- 3 lines14:10
mupPR #7584: .gitignore: pair of trivial changes <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/7584>14:10
roadmrdoes snapd identify if a snap requires a specific content snap and install the content snap automagically?14:18
roadmr(i.e. snapian)14:18
mborzeckiroadmr: afaik snap.yaml must name a default-provider for a given content plug14:21
roadmrmborzecki: and the named default-provider snap gets auto-installed/14:21
mborzeckiroadmr: yes14:30
roadmrthanks mborzecki :)14:31
Chipacaijohnson: out of curiosity, how hard would it be to pull that in to our snapd build for armhf?15:17
roadmryou're pulling my armhf15:17
ijohnsonnot 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 crypto15:18
ijohnsonChipaca: ^15:18
ijohnsonthe big thing why I don't think we should do that is because now we have to maintain that SHA3 crypto code15:18
Chipacayeah i get that15:18
Chipacabut https://go-review.googlesource.com/q/hashtag:%22crypto-assembly%22+(status:open)15:19
Chipacadoes not fill me with hope15:19
ijohnsonChipaca: 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 years15:21
Chipacabrb, rebooting15:33
mupPR 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
mvopedronis: do you mind if I merge 7462 (the  grade support for models in uc20?)15:52
pedronismvo: I can do it, one sec15:53
mupPR 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
mvopedronis: thanks15:54
mvopedronis: 7556 can probably also be merged, the suggestions could be done in a follouwp, this would unblock the next review I think(?)15:55
pedronismvo: it's about documenting, maybe renaming TargetFunc, right ?15:56
ondraChipaca I managed to shave 40s from that boot now :)15:57
pedronismvo: and a typo15:57
pedronismvo: I can merge and then I would apply those in the follow up15:57
pedronisif that's ok15:57
ondraChipaca it will need reviewing from someone who can understand more implications of what I changed15:57
pstolowskimborzecki: some questions/comments to schedule fix PR15:57
ondraChipaca but thats ~80s to ~40s15:58
mvopedronis: yeah, lets do it this way (merge+followup)15:58
ondraChipaca see bootchart https://private-fileshare.canonical.com/~okubik/bootchart-20191010-1545.svg15:58
mvopedronis: thank you!15:59
ondraogra https://private-fileshare.canonical.com/~okubik/bootchart-20191010-1545.svg15:59
mupPR 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
ondramvo pedronis this could be something we might consider for landing once polished, see the difference in cpu from 60 to ~100 second16:00
ondrahttps://private-fileshare.canonical.com/~okubik/bootchart-20191010-1545.svg and https://private-fileshare.canonical.com/~okubik/bootchart-20191008-1710.svg16:00
Chipacaondra: what is the change?16:00
mvoondra: what exactly?16:00
mvoondra: do you have a diff or something?16:00
ondraChipaca mvo diff soon16:01
ondraChipaca mvo but I'm calculating sha3384 in parallel16:01
ondraso this will be even faster on multicore devices16:01
ondraon 4cores this cut hashing to half, and it's mostly because one snap is 3x bigger then next biggest16:02
ondramvo Chipaca one of you just need to review if I'm not messing up with snap seeding order16:04
pedroniswe are touching that code quite a bit atm16:04
pedronisso it's probably easiest to just see the diff and measure it and do it again if it's useful16:05
ondrapedronis are any of your changes parallelising hash calculation?16:06
pedronisno, but we intergrating UC20 code16:06
pedroniswhich does still differently16:06
ondrapedronis right16:06
ondraI will prepare clean diff16:06
pedronisis this a change in seed16.go ?16:06
pedronisor the older code?16:06
ondraI still think this is worth to consider, as we sit there for 2 minutes with one core maxed and rest of the system is idling16:07
ondrapedronis in seed16.go16:07
pedronisthat shouldn't change a lot anymore16:07
pedronisbut we'll see16:07
pedronisclean diff is a good start16:07
ondrapedronis +116:08
ondraChipaca mvo it influenced order how snaps are seeded16:14
ondraso it will still need some tweaking there16:14
pedronisondra: that doesn't sound right16:19
pedronisto be clear16:19
ondrapedronis well this is first shot and I'm just checking how to address this16:19
pedronis#7558 is ready for review16:22
mupPR #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
pedronisChipaca: there are also bits in boot there ^16:22
Chipacai don't think i'll get to that today16:23
pedronisjust fyi16:23
mupPR 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
zygapedronis: you have two entries in the meeting notes for Thursday16:56
zygais one of them for Friday or should they be merged?16:56
Chipacazyga: one is pedronis' clone17:00
zygathat explains it :)17:00
Chipacaanyhoo, i'm outa here17:01
Chipacasee y'all tomorrow17:01
zygagood call :)17:01
* zyga hugs Chipaca 17:01
zygabye :)17:01
* Chipaca hugs back17:01
zyganiemeyer: hello, low priority but also low effort request to look at a small change to spread: https://github.com/snapcore/spread/pull/8917:18
mupPR spread#89: runner: pass --exclude-vcs-ignores to tar <Created by zyga> <https://github.com/snapcore/spread/pull/89>17:19
* cachio afk17:40
mupPR snapcraft#2749 opened: storeapi: add StoreErrorList to handle store errors <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2749>20:34
joedborghey zyga21:01
zygait's a little late so I may not respond instantly21:02
zygabut please leave me a message and I'll get to it as soon as I can21:02
joedborgzyga: ah apologies, i keep missing you21:02
joedborgjust wondering if you can help with what exception i need to add to apparmor for21: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
zygano need to apologize, it's not our fault :)21:03
zygathis 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
zygaif you can craft the docker-default profile then it's a one liner to change21:04
zygaAFAIR, without checking something like: signal (receive) peer=snap.kubernetes-worker.kubelet,21:05
zyganote that the permission applies on both sides21:05
zygaso the snap.kubernetes-worker.kubelet needs a send permission with the appropriate peer21:06
zygajoedborg: does this make sense?21:06
zygajoedborg: you can constrain the signal further with set=(...)21:07
joedborgzyga: is there a quick line i can get round it with?21:07
joedborg* that i can add to app armor21:07
zygasignal (receive) set=(kill) peer=snap.kubernetes-worker.kubelet,21:07
zygathough note that the peer here specifies the snap name21:07
zygaso only that snap (and only that snap) would work21:07
zygait may require some more ooomph21:08
zygaperhaps the signal could be accepted from any snap that can send it21:08
zygaso you could just say:21:08
zygasignal (receive) set=(kill) peer=snap.*21:08
zyga(make sure to add the trailing comma)21:08
joedborgzyga: thanks, i'll try that now21:09
joedborgit's just a further PoC, so I can arrest it if and when it works :)21:09
zygajoedborg: cool :)21:11
zygajoedborg: yeah, try that, let me know if it works or not21:11
joedborgzyga: 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
zygathat's not the profile21:12
zygalook at what I said above21:12
zygathis is confining the process using the docker-default profile21:12
zygawhatever owns and writes *that* profile must change21:12
zygawhen you have two processes with apparmor profiles21:12
zygaand they signal each other21:12
zygathe sender needs the send permission for the peer21:13
zyga(and you have that or the denial would say profile=snap.foo.bar)21:13
zygathe recipient needs the corresponding permission21:13
zygathat matches the peer21:13
zygait's funny but you can craft an apparmor profile that denies sigkill with this21:14
zyga(from unconfined)21:14
zygajoedborg: does this make sense?21:14
joedborgzyga: how do i get at the docker-default profile?21:14
zygajoedborg: I don't know that, presumably something writes one, one sec21:14
joedborgzyga: i can't see it in `/var/lib/snapd/apparmor/profiles/`21:15
zygait's not a profile made by snapd21:15
zygaso I suspect the snap itsef makes it21:15
zygaijohnson: ^^ perhaps you know?21:15
zygajoedborg: in other words, no amount of changes to snapd will fix it21:16
ijohnsonlet me read the scrollback21:16
zygathe profile you must change is made or belongs to another package, presumably just docker21:16
zygaijohnson: tl;dr; do you know if docker snap makes "docker-default" apparmor profile?21:16
zygaand if so, do you know how to change the code that makes it so that it has one more line21:16
ijohnsonah yes docker-default is generated by docker/runc when you create containers21:17
ijohnsonin the docker snap we had to patch the docker-default profile to allow containers to start properly when dockerd is confined by snapd/apparmor21:17
joedborgijohnson zyga so there's nothing i can do on a running system to patch this?21:18
ijohnsonessentially, 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 above21:18
ijohnsonjoedborg: 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 this21:19
zygajoedborg: not without fighting with docker21:19
ijohnsonyou need to patch dockerd to modify the docker-default profile to include that line21:19
joedborgzyga: ijohnson well containerd in my case21:19
zygajoedborg: you could create a profile that is the same, has one more permission, and load that21:19
ijohnsonerr yeah21:19
joedborgzyga: ijohnson: I can carry on in devmode and log it as a bug?21:20
zygabut that would have to be done again each time docker decided to reload the profiles21:20
ijohnsoncontainerd/dockerd/runc/moby/the kitchen sink/a bag of chips/21:20
zygajoedborg: you'd have to run docker in devmode21:20
ijohnsonjodbord: nah I don't think so, because you need the docker-default profile to be in devmode, not the snap21:20
ijohnsonthere's a way to launch a container with containers having their apparmor profiles turned off though21:20
ijohnsonerr that was akward to say21:21
ijohnsonthere's an option to dockerd/containerd/whatever to launch containers without the docker-default profile21:21
zyganah, that's not a good idea21:21
ijohnsonI can't remember off the top of my head21:21
* zyga remains quiet21:21
ijohnsonzyga: yeah it's not a good idea, but it's the same as running in devmode21:21
joedborgzyga: ijohnson: so this is for the kubernetes-worker snap which has containerd bundled inside the snap21:21
zygaI was thinking about using aa_complain21:21
zygabut I think that only works if you can point it to a profile text21:21
ijohnsonjoedborg: ah if you're building the snap then you can just patch dockerd/containerd21:22
joedborgijohnson: how? :)21:22
ijohnsonjoedborg: 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.patch21:23
* zyga doesn't know that and returns to audiobooks and assembly21:23
ijohnsonzyga: go do that and stop working :-)21:24
zygathank you ijohnson21:24
zygajoedborg: you are in good hands :)21:24
joedborgthanks zyga, sorry to have kept you at your desk21:24
ijohnsonjoedborg: how are you building containerd?21:24
joedborgijohnson https://github.com/charmed-kubernetes/snap-kubernetes-worker/blob/master/snap/snapcraft.yaml21:25
joedborgit's being built in the snap21:25
ijohnsonhmm 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
joedborgijohnson: i thought we had similar issue that jdstrand got into the containerd profile21:26
joedborgijohnson: yeah because kubernetes doesn't build things properly21:26
ijohnsonbut anyways since you're building containerd from scratch patching it shouldn't be too much work21:26
ijohnsonhmm well still you could let snapcraft check it out, that would save you rebuild times, but oh well21:27
joedborgijohnson: can you give commit hashes to snapcraft?21:28
ijohnsonyes, `source: github.com/....` and then do `source-commit: SHA`21:28
ijohnsonjoedborg: see https://snapcraft.io/docs/snapcraft-parts-metadata#heading--source21:29
joedborgijohnson: thanks, i'll add that in the todo to look at21:29
ijohnsonjoedborg: so you need to patch https://github.com/containerd/containerd/blob/1ac546b3c4a3331a9997427052d1cb9888a2f3ef/contrib/apparmor/template.go21:31
ijohnsonthat's the "docker-default" profile in containerd AFAICT21:31
ijohnsonI 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 containerd21:32
joedborgijohnson: 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 interdace21:33
ijohnsonjoedborg: 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 modified21:35
ijohnsonIf 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 profile21:36
ijohnsonI 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 see21:37
ijohnsonjoedborg: 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 with21:38
joedborgijohnson: 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 route21:39
joedborgijohnson: 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 suggested21:39
joedborg /pivot route/pivot root/21:40
ijohnsonjoedborg: 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 parameter21:40
ijohnsonso 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 wants21:41
ijohnsonjoedborg: that patch is here: https://git.launchpad.net/~docker/+git/snap/tree/dockerd-patches/snappy-real-chroot.patch21:41
joedborgijohnson: ah that’s sounding like we’re seeing the same thing, cool.  Just reading that patch21:42
ijohnsonyeah 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 forgotten21:43
joedborgijohnson: 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
mupPR 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
joedborgijohnson: 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
ijohnsonjoedborg: yeah that sounds good21:47
ijohnsonI think jdstrand is back next week iirc21:47
joedborgijohnson: awesome. yeah I think so too.   thanks a lot for your help :)21:48
ijohnsonnp, 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!