mupPR snapd#7703 closed: cmd/snap: make 'snap list' shorten latest/$RISK to $RISK <Channels 🏷️> <Simple 😃> <Created by chipaca> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7703>06:21
mvohey mborzecki06:24
mborzeckimvo: hey06:26
mupPR snapd#7454 closed: interfaces: extend the fwupd slot to be implicit on classic <Created by jhenstridge> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7454>06:36
zygagood morning07:06
mborzeckizyga: hey07:06
zyga5 days left07:06
zygalet's make it count!07:06
=== Girtablulu|Away is now known as Girtablulu
mupPR snapd#7717 closed: tests/docker-smoke: add minimal docker smoke test <Simple 😃> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7717>07:18
mborzeckimvo: some conflicts in #771407:19
mupPR #7714: overlord: refactor mgrsSuite and extract kernelSuite <Created by mvo5> <https://github.com/snapcore/snapd/pull/7714>07:19
mborzeckimvo: i can look into that, unless you've already fixed it07:19
mvomborzecki: missed the ping, either way is fine07:23
mvohey zyga07:23
mborzeckimvo: ok, i'll push it07:24
mborzeckimvo: #7715 is based on 7714 isn't it?07:30
mupPR #7715: overlord: add base->base remodel undo tests and fixes <Created by mvo5> <https://github.com/snapcore/snapd/pull/7715>07:30
mvomborzecki: correct07:31
mborzeckimvo: ok, i'll update that one too then07:32
mvomborzecki: it also needs a review and some more things (that samuele pointed out) but lets do the mechanical bits first07:32
mvomborzecki: hm, looks like 7665 is in some strange state, says its waiting for CI for me, mabe we ned to push master to it07:33
mborzeckimvo: yeah, i'll be addressing your comments and will push there07:34
mvomborzecki: are you working on the remodel-kernel-undo-tests PR? it has no conflicts so I could just address the feedback and push and ask you to review :) ?07:56
mborzeckimvo: no, i just pushed a little patch to 7714 and gave it +107:58
mvomborzecki: cool, I will just address samuele points in 7682 then and push and then you can review that too,07:58
mborzeckimvo: i think we can land 7714 and then update 7715, otherwise i'll get messy i think07:58
mvomborzecki: its a subset so review should be trivial07:58
mvomborzecki: aha, even better07:59
mvomborzecki: one small issue is that 7714 includes 768207:59
mvomborzecki: so I need to make this one ok first ./08:00
mvomborzecki: sorry, I should have done the refactor indepedantly08:00
=== pstolowski|afk is now known as pstolowski
zygahey pawel08:12
mborzeckipstolowski: hey08:21
zygamborzecki: trying to fix the name=snapd branch now08:47
zygamborzecki: made it conditional on v1/v2 mode, to use different directories08:48
zygahoping /sys/fs/cgroup/snapd survives into lxd08:48
zygaand permissions there are sufficient08:48
mborzeckizyga: you likely need to merge master, paralle instances landed on friday08:48
zygamborzecki: I know, I plan to soon08:48
mborzeckimvo: left a comment about retry-tool in place of the while loop, feels a bit awkward bc of grep08:49
mborzeckimvo: so i'll skip this one, and the mockSuccessfulReboot(), actually that second one is mroe interesting, it assumes snap_mode = try, but we don't switch the gadget snap during reboot, only base/kernel08:49
mvomborzecki: aha, right - good point08:57
mvomborzecki: sorry, I missed that08:57
mborzeckimvo: no worries, i think we could make mockSuccessfulReboot work with some tweaking to catch unintended test cases08:58
mvomborzecki: *nod*08:58
zygachrome said to restart09:02
zygamvo: ^09:02
Chipacamborzecki: mo'in. Does your +1 to #7669 apply to #7670? :)09:55
mupPR #7669: snap, cmd/snap: support (but warn) deprecated multi-slash channel <Channels 🏷️> <Needs Samuele review> <Created by chipaca> <Closed by chipaca> <https://github.com/snapcore/snapd/pull/7669>09:55
mupPR #7670: cmd/snap: support (but warn) using deprecated multi-slash channel <Channels 🏷️> <Needs Samuele review> <Created by chipaca> <https://github.com/snapcore/snapd/pull/7670>09:55
mborzeckiChipaca: let me take a look09:56
mborzeckiChipaca: idk, the suffering strings are gone ;)09:58
Chipacamborzecki: ikr09:58
Chipacamborzecki: OTOH valid value + error is a weird flex09:58
Chipacamborzecki: how did you make a comment on an error message that is no longer there?10:01
mborzeckiChipaca: btw. do we use the british or us spelling in the messages?10:04
Chipacamborzecki: us :-/10:11
zygaanother test pass,10:14
zygatest passed10:19
zygathat's very promising10:19
zygarunning more challenging test10:19
Chipacamvo: #7714 has two +1s but it includes #7682 which has some nits pointed out by pedronis10:40
Chipacamvo: up to you whether to merge it and then followup or what10:40
mupPR #7714: overlord: refactor mgrsSuite and extract kernelSuite <Created by mvo5> <https://github.com/snapcore/snapd/pull/7714>10:40
mupPR #7682: overlord: add kernel remodel undo tests and fix undo <Priority :racehorse:> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7682>10:40
mupPR #7682: overlord: add kernel remodel undo tests and fix undo <Priority 🏇> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7682>10:41
Chipacabetter :)10:41
mvoChipaca: I'm just finishing the unit test asked for in 7682, should be ready in some minutes10:53
zygamaking progress, next up is the lxd test10:54
mvoChipaca, mborzecki  7682 should be ready now, I addressed the comments from samuele11:11
zygaupdated cgroup dir based on v1 / v2 mode in snapd side, resuming tests11:14
mvoChipaca, mborzecki actually, yeah, probably/possible easier if I merge 7714 and do my other bits as a followup, up to you if you feel this is easier to review11:15
zygamborzecki: the changes are not too terrible, not great11:16
Chipacamvo: sgtm11:16
zygamborzecki: we should do runc-like change later on11:16
mborzeckimvo: landing 7714 sgtm11:16
mupPR snapd#7714 closed: overlord: refactor mgrsSuite and extract kernelSuite <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7714>11:16
mvomborzecki: 7665 now has conflicts, let me know if I should help. and a strange spread failure11:19
mvoChipaca, mborzecki 7682 should be much simpler to review now :)11:20
zygatests passed11:20
zygamoving on11:20
mborzeckimvo: that failure in 7665 looks like something raised in the forums during the weekend11:20
zygarunning the lxd test now11:20
mborzeckimvo:  https://forum.snapcraft.io/t/snap-remove-fails-because-of-user-data/1399011:21
Chipacamborzecki: saw that one, still puzzled by it11:21
* Chipaca replies with as much11:22
mvomborzecki: ok, feel free to restart the test if its unrelated11:28
pstolowskizyga: i've responded to #767511:30
mupPR #7675: release: preseed mode flag <Prebaking 🍞> <Simple 😃> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7675>11:30
zygapstolowski: thank you, looking now11:30
zygapstolowski: replied11:32
zygamvo: lxd test has passed with the new feature enabled *outside*11:32
pstolowskizyga: thanks11:33
zygamvo: this is promising, looking at making the inside case now11:33
zygamvo: the other case is just test adjustment which will later on become a change to lxd11:34
zygamvo: this is really promising, I think it will work11:35
mvozyga: cool11:35
mborzeckiChipaca: hmm regarding snap download, there's some code in image/helpers.go that just calls os.Exit(1) on sigint ;)11:46
Chipacamborzecki: does it also Finalize the progress bar?11:46
Chipaca(also, yes, exit 1 would probably be the right thing to do)11:47
Chipaca(but after finalizing the progress bar)11:47
mborzeckiit's Finished() followed by os.Exit()11:47
Chipacaperfect :)11:47
ChipacaFinished, Finalize, Fwhatever11:47
mborzeckihm also, snap download --remote leaves a *.partial file behind, but one that goes via snapd does not?11:47
Chipacamborzecki: --direct doesn't leave a .partial when interrupted?11:48
Chipacathat'd be a(nother) regression11:48
mborzeckiChipaca: --direct does leave *.partial, the non-direct one does not11:49
* mborzecki tries to parse what he just wrote11:49
Chipacamborzecki: ah, that's a known regression11:50
Chipacamborzecki: non-direct does not know how to resume11:50
Chipacaand indeed it'll take a refactor of the download endpoint to do it11:50
mvoChipaca: yeah, I think we need to talk what to do - I think I will give your idea of "snap download --indirect" a shoot and then we can iterate on this without regressions11:57
mvomborzecki: thanks for looking at this so carefully!11:57
Chipacamvo: my biggest concern is that i don't see how to get resume without a rest api refactor11:58
Chipacaactually thinking a bit more, it's a minor refactor, a change to which things are obligatory i guess12:00
Chipacaand a change to how it's used for sure12:00
Chipacathat's my brain coming back to me with this that i'd been mulling over the weekend12:00
Chipacamvo: for snap download, what you want to do is first do an exact search by name, and take the info you get back to (1) check whether you have that file already, (2) check if you have a partial, (3) download exactly that revision, maybe with a resume in it12:01
Chipacaso the refactor for download is (a) always take the revision, and (b) optioanlly take a resume position12:02
Chipacathat's at the api level; internally it's a bit bigger of a change because snap download itself shouldn't be doing the two requests, i guess? in that maybe there's a way to not have to hit info again server-side12:02
Chipacain the fast case at least12:02
Chipacabut that can be a later optimization12:03
Chipacawe could make snap download take the snap id, and then reconstructing the download irl is easier?12:04
zygamvo: I'm working on the denial, no idea what is it yet12:09
zygamvo: I've simplified the test and will iterate interactively on the next shell12:11
mborzeckiChipaca: for starters we could just write the data to *.partial file and rename, seeing *.snap that isn't really complete is bit confusing12:12
mborzeckicachio: did you have a patched version of spread that can run tests in a specific order?12:13
cachiomborzecki, yes12:13
mborzeckican you point me to it?12:14
zygacurrently stuck on one denial:12:14
zygacannot remount /sys/fs/cgroup read-write: Permission denied12:14
zygaNov 04 12:13:45 nov041205-146458 audit[28564]: AVC apparmor="DENIED" operation="mount" info="failed flags match" error=-13 profile="lxd-my-ubuntu_</var/snap/lxd/common/lxd>" name="/sys/fs/cgroup/" pid=28564 comm="snap-confine" flags="rw, remount"12:14
cachiomborzecki, sure12:14
zygaI changed the apparmor profile of lxd to contain12:15
zyga  mount options=(remount, rw) /sys/fs/cgroup/,12:15
cachiomborzecki,  https://cachio.s3.amazonaws.com/spread/spread212:15
zygabut perhaps I misunderstand, need to check what those paths map to in practice12:15
mborzeckizyga: isn't that a profile that lxd created for the container?12:15
zygamborzecki: yes12:15
mborzeckicachio: thanks!12:15
zygamborzecki: the denial is for the profile from lxd12:15
cachiomborzecki, np12:15
zygamborzecki: that is what I am changing12:15
zygamborzecki: inside the test I patch and reload that profile12:16
zygamborzecki: on the container host12:16
mborzeckiah ok12:16
cachiojust use: -order -workers 112:16
cachiomborzecki, this will use 1 instance for all the tests12:16
cachioand run the tests in the order you add them in the command line12:17
mvoChipaca: thank you, yeah, in some sense its an optimization12:18
zygamborzecki: interesting, I can switch all of the apparmor for lxd into complain mode12:18
mborzeckicachio: ok, running it now12:18
zygaand it doesn't help12:18
zygaI probably don't understand what is really going on12:19
mvomborzecki: 7665 has now conflicts :/12:21
mborzeckimvo: mhm, first trying to catch the problem with removal of /var/snap/12:21
mvomborzecki: no worries, I can fix the conflicts12:24
* pstolowski lunch12:26
mborzeckimvo: thanks for merging master to the remodel branch!12:29
mvomborzecki: no worries, I want to see it merged :)12:31
Chipacamvo: is there an IRL bug that triggered #7682?12:36
mupPR #7682: overlord: add kernel remodel undo tests and fix undo <Priority 🏇> <Created by mvo5> <https://github.com/snapcore/snapd/pull/7682>12:36
cachiozyga, hey12:36
mvoChipaca: no real life bug12:36
cachiodid you see something like this: https://paste.ubuntu.com/p/6NYyBbjtMW/12:37
Chipacamvo: phew12:37
mvoChipaca: it was just the result of missing undo testing in the handlers code12:37
mvoChipaca: yeah12:37
cachioit is failing on ubunto-core-18 tests on pi3 for edge cahnnel12:37
* mvo gets some lunch12:37
zygamaking some more progress12:38
zygahey cachio12:38
zygaI realized lxd start / stop rewrites the container profile12:38
zygaso my changes are not effective12:38
zygaI started the container12:38
zygaand use apparmor parser to switch it to complain mode after taht12:39
zygaI used aa-status to check what the outcome is after taht12:39
zygaand see that some of the chagnes are applied, there are a number of processes with complain profiles12:39
zygabut I can see that the bulk of the container is confined12:40
zygaI'm exploring why12:40
zygathe output of aa-status is12:40
zygain case someone figures out why12:40
cachiozyga, all this is related to the mount namespaces error?12:46
zygacachio: no, this is ongoing work on new cgroup12:47
cachioahh, ok12:48
zygaI managed to switch snap-confine profile into complain mode12:54
zygait seems there's no propagation of any kind12:55
zygaso changing the profile on the host doesn't impact profiles derived from it in any way12:55
zygabut that's fine12:55
zygasnap-confine inside is in complain mode but I still get a denial12:55
zygalooking at why now :)12:55
zygaperhaps I just put the wrong one in complain mode12:55
zygait's not re-execing12:55
zygamore profiles in complain mode https://www.irccloud.com/pastebin/cwRZuduL/12:56
zygamvo: in the meantime I'm running all tests to see if anything is impacted , 50% there, I'll know soon12:58
mborzeckigoogle:ubuntu-core-18-64 .../tests/main/remodel-gadget# ls /var/snap/pc12:58
mborzecki1001  100212:58
mborzeckiheh, wtf??12:58
* diddledan sees zyga wants complaints so moans that it's not bed time yet12:59
zygaI got ALLOWED messages13:06
zygaso it did switch to complain mode13:06
zygaand the things mentioned were really about what is new in the code13:06
zygabut I got the error still13:07
zygabut even though i still got an error from ount13:07
zygachecking why13:07
zygathe error in case anyone sees error in my logic https://www.irccloud.com/pastebin/9JmCN4RA/13:08
* Chipaca wanders off mumbling something about lunch13:08
zygaChipaca: good idea13:08
zygaI'll update the test to do what I did manually13:08
zygaand wrap up for lunch13:08
zygatest modified, restarted clean13:11
zygaI think it is better13:11
zygaI'll double check the code, maybe there is something silly in my patch13:11
zygamvo: we are all doomed ;-)13:13
zygaI want more of https://mspoweruser.com/microsoft-4-day-workweek/ though ;)13:14
zygabut first, let's ship it for vancouver13:15
zygamvo: regular tests are move than 90% complete now13:15
zygamvo: if anything exploded I'll know soon13:15
mupPR snapd#7718 opened: sandbox/seccomp: accept build ID generated by Go toolchain <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7718>13:26
mborzeckimvo: zyga: it'd be nice to get it for 2.4313:26
mborzeckialso pinged jdstrand for a review though the bit is trivial13:26
cachiomborzecki, hey, this error is hapenning when updating the arch image https://travis-ci.org/snapcore/spread-cron/builds/607000141#L205613:34
cachioany idea ?13:35
mborzeckicachio: yeah, looks like the package needs an update13:37
mborzeckilet me see if there's anything i can do about it13:37
* zyga is afk for lunch now13:39
zygaonly lxd and mount-ns tests failed, so good and expected13:39
cachiomborzecki, nice thanks13:40
mborzeckicachio: pinged the guy who maintains the package with a patch that should fix it, let's see if it lands today/tomorrow, if not i can fork the package, put it on github and we can pull it from that location13:43
cachiomborzecki, nice thanks!!13:45
zygastgraber: hello, can you please help me how lxd uses apparmor profiles13:53
zygastgraber: I'm trying to understand how to switch the entire container into complain mode13:54
zygastgraber: I tried running apparmor_parser -r --complain /var/snap/lxd/common/lxd/security/...13:54
zygastgraber: using aa-status on the host I can see some changes but I still see a number of confined profiles13:54
zygaer, enforcing profiles13:54
zygae.g. the output of aa-status from the container host is13:55
zygaI would appreciate if you can shed some light on this13:55
stgraberLXD regenerates and reload the container profile on container startup, files are mostly there to inspect but changes won't persist13:55
zygastgraber: right, but is there more than one file to change?14:07
zygastgraber: I'm getting things I don't quite understand, after a lot of hand-called apparmor_parser --complian --reload I get some ALLOW but still some EPERM and nothing logged with DENIED14:07
zygastgraber: is it safe to change any profiles after the container is started?14:08
zygastgraber: note that I suspect that "deny" rules silence logging14:09
zygastgraber: so perhaps what I'm hitting is a deny somewhere14:09
zygastgraber: alternatively14:10
zygastgraber: is there a knob on lxd that would allow me to run a container with complain more apparmor?14:10
zyga15 minute break14:23
zygastgraber: I'm all ears if you have some ideas14:23
zygastgraber: but I'll be sending patches to lxd to help with this case, it's a one-liner *I believe* for now14:23
stgraberzyga: what'14:25
stgraberzyga: what's the one liner?14:25
zygastgraber: to remount /sys/fs/cgroup rw14:25
zygaall I need is14:25
zygamount -o remount,rw /sys/fs/cgroup14:25
zygamkdir /sys/fs/cgroup/snapd14:25
zygamount -o remount,ro /sys/fs/cgroup14:25
zygamount -t cgroup cgroup /sys/fs/cgroup/snapd -o none,name=snapd14:26
zygathat's all14:26
zygaI believe all but the rw remount are permitted14:26
stgraberzyga: I'm reasonably sure that we cannot allow that without causing a big security issue14:26
zygabecause of remount or because of something else14:26
zygaplease tell me what you think, it's important in our design14:27
stgraberbecause of the apparmor rule generator being broken14:27
zygastgraber: is there anything we can do?14:28
stgraberif rw,remount hits the parser bug, no14:28
stgraberadding such a rule would allow every single mounts to succeed14:28
stgraberzyga: why are you trying to mount your own v1 hierarchy anyway?14:29
zygastgraber: can we do something else to avoid the need to mkdir, can we mkdir early in systemd somewhere?14:29
zygastgraber: for tracking processes correctly, it's the only way we found that works well in v1 and v2 mode14:30
stgraberzyga: how do you do it in v2 where /sys/fs/cgroup is a cgroup2 mount?14:30
zygastgraber: in /run/snapd/cgroup14:30
stgraberah so you're assuming that a cgroup2 system will still have cgroup1 enabled in the kernel?14:30
zygastgraber: I wanted it to be in /run/snapd/cgroup now but this is more complex to do so that it works in lxd correctly14:31
zygastgraber: yes, for now yes14:31
zygastgraber: we need to work on extending cgroup2 usage in systemd14:31
stgraberzyga: why would /run/snapd/cgroup be harder in LXD?14:31
stgraberzyga: that should actually be easier. Obviously you'll have the problem that this will fail on any system which doesn't have LXD on the host14:31
zygastgraber: because /run is changed by lxd and we need to change it so that it is not necessary to have that mount inside the per-snap mount namespace14:32
mupPR snapd#7670 closed: cmd/snap: support (but warn) using deprecated multi-slash channel <Channels 🏷️> <Needs Samuele review> <Created by chipaca> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7670>14:32
stgraberzyga: so you're going to break a LOT of systems, but that's going to happen regardless so long as you want your own named controller14:32
zygastgraber: can you explain how we will break things?14:32
stgraberzyga: unprivileged users are only allowed to mount existing controllers14:32
stgraberzyga: if your host doesn't have a name=snapd controller, no container will be allowed to mount it14:32
zygathe host will have it14:32
stgraberno it won't14:33
zygaperhaps I misunderstand14:33
stgrabernot for anyone who's not using the snap14:33
zygaand are there people who are not using lxd as a snap that will run into this?14:33
stgraberso you'll break all chromebooks, all gentoo users, all alpine users, most arch users, ...14:33
zygaI see14:33
zygathat's bad news indeed14:33
zygaI wonder if we have some ideas as a way out then14:33
zygastgraber: can you tell me more about the rule14:34
stgraberwe actually ran into that very same problem when systemd introduced name=systemd in the first place14:34
zygastgraber: about what makes the cgroup mount fail if it doesn't exist on the host14:34
stgraberany distro which wasn't using systemd was unable to run systemd containers because of it14:34
zygastgraber: note that the feature is still not on by default, or merged for that matter14:34
mborzeckimvo: got a fix for the test failure that occurred in #7665, i'll propse it separately unless the travis job that's currently running fails14:34
zygastgraber: we just need _a_ way to track processes reliably14:34
mupPR #7665:  devicestate: add support for gadget->gadget remodel  <Priority 🏇> <Remodel 🚋> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7665>14:34
zygastgraber: and we have found none better than this14:34
stgraberzyga: the kernel effectively looks at all existing controllers (same list you'd see in /proc/self/cgroups) and anything that's not in that list, you can't mount unless you're real root outside of a cgroup namespace14:35
zygaI see14:35
zygaand it treats the name=... thing as a controller in this sense?14:35
zyganote that our hierarchy has no controllers at all14:36
zygabut I'm sure you know that, just checking if I understand right14:36
stgraberzyga: given that your current plan will not work on any distro which has fully transitioned to cgroup2 (no more cgroup1 in kernel) and will break for a lot of container users, it doesn't seem to me like a particularly good plan :)14:36
stgraberzyga: yes14:36
zygastgraber: there are no better plans yet14:36
stgraberzyga: you wouldn't want an unprivileged container being able to mount thousands of those, allocating global kernel structs14:36
zygastgraber: pure v2 doesn't let us track processes because we'll interfere with systemd14:36
stgraberzyga: can't you just keep snap-confine as the parent of the processes in the snap and make it a subreaper?14:37
mborzeckizyga: stgraber: tbh we don't seem to have a reliable, race free way of tracking processes in v2 world without complicating things14:38
zygastgraber: if we do that, we know if a process dies14:38
zygastgraber: but not if there are any left14:38
stgraberzyga: but you know that when you die all subprocesses will too14:38
zygastgraber: we'd need to match fork and exit reliably but that's not what subreaper can do14:38
zygastgraber: I'm not sure I follow, are you saying that we can then kill snap-confine tracking process to kill all the children?14:39
stgraberzyga: yes14:39
zygastgraber: that's not what we are trying to do14:39
zygastgraber: the goal is to know when it's safe to refresh14:40
zygastgraber: when no apps are running14:40
zygastgraber: in a zygote-like mode when snap-confine sticks around and watches processes die14:40
zygastgraber: we could perhaps know something useful but I don't believe we can do what we want to with just that14:41
zygamborzecki, mvo: I think we need to abort and come up with another idea14:43
zygabased on what stgraber said above I don't believe this is something we can salvage unfortunately14:43
zygathis still leaves us with no tracking, let alone notification past v114:44
zygastgraber: how did you fix the issue affecting systemd needing to mount name=systemd14:45
stgraberzyga: most distros switched to systemd, the rest are mounting the systemd controller on the host anyway14:46
zygastgraber: I see14:46
mvomborzecki: thank you14:47
zygamvo, mborzecki: can we meet later today14:48
stgraberzyga: so sadly there's no nice way that I know for you to get notified when a namespace goes away, otherwise you could use something like a uts namespace14:48
zygaI have an idea14:48
zygastgraber: I planned to use release agent on the name=snapd hierarchy14:49
zygabut thinking about it now14:49
zygahow to work without any cgroups14:49
stgraberzyga: have one namespace per snap, dump all new processes in there, then when you want to know if the snap is safe to refresh, check if any process uses it14:49
zygastgraber: one uts namespace?14:49
zygastgraber: and in v2 mode?14:49
zygastgraber: in v1 we can use the freezer for that14:49
zygastgraber: (note that we need to have per-app understanding in our current model but perhaps we could drop that)14:50
zygastgraber: but what the name=snapd idea was about is something that works both in v1 and v2 mode14:50
stgraberzyga: doesn't matter v1/v2, this wouldn't use cgroups at all14:50
zygaI didn't understand14:50
zygauts namespace14:50
zygaso iterate over process14:50
zygaand check which uts namespace is used by each?14:51
zygaand check if any of those match that of our snap processes?14:51
stgraberyeah, that's the part that's not amazing, having to iterate but otherwise should work fine14:51
zygathat might w14:51
zygathat might work14:51
zygaI was thinking to use the existing mount namespace14:51
zygagiven that we already create one for strict snaps14:51
zygaand are about to for classic snaps14:51
stgraberyeah but your mntns are per snap + per user or something so that wouldn't work so well14:52
mupPR snapd#7682 closed: overlord: add kernel remodel undo tests and fix undo <Priority 🏇> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7682>14:52
zygastgraber: yes14:52
zygastgraber: so you are saying we could create and save one for each pkg.app pair14:52
stgraberthe tracking with a uts namespace would still be bypassable by any snap allowed to do fancy clone/unshare, but that's unlikely to really matter, you're not building security on this, if a snap wants to make itself break on refresh, whatever :)14:53
zygastgraber: correct but the kind of snaps that would be hurt by this (think things like docker) really would as they would get unexpected refresh semantics14:54
zygastgraber: but thank you for the idea, I think this is workable14:54
stgraberzyga: yeah, on initial startup, you'd unshare the uts namespace, bind-mount its inode somewhere in /run/snap as you do for mntns, then any new process gets added using setns and to see if it's safe, you iterate through /proc for anything that has a matching inode on ns/uts14:54
zygastgraber: correct14:55
zygastgraber: I agree this would work14:55
stgraberyou could even make actual use of the uts namespace and give each snap a unique hostname14:55
zygastgraber: I think that will break snaps14:55
zygastgraber: especially it would be unexpected for snap with classic confinement14:55
stgraberindeed :)14:56
zygastgraber: or maybe the time namespace14:56
stgraberone thing to keep in mind is that if the hostname changes on the host, it won't in the snap14:56
zygastgraber: yeah, I think it's a good route but we need to plan which namespace to use14:56
zygauts doesn't feel like it14:56
stgraberyou certainly don't want to use ipc, mnt, net, pid or user14:56
zygaI would ideally do it via mount namespace and just save those with finer granularity14:56
stgraberso you have two options really14:56
mborzeckifeels like it's the same borderline use as the pids controller right now14:57
zygaI think mount is indeed unworkable on second thought14:57
stgrabermount is the most used namespace, a lot of software mess with it and snapd already has more than one mntns per snap, so seems pretty hard to track14:57
stgrabercgroup and uts are the two easy ones, cgroup may actually be a better option14:58
stgraberhmm, though it may seem weird14:58
stgraberyeah, no, don't use cgroup, you'll break lxd :)14:58
stgraberuts won't break us though, the only issue with it is the hostname changes not getting propagated until a snap restart, but I don't think anyone will really notice/care14:59
stgraberzyga: we could probably do something with pidfds though, but that's likely to be an issue due to lack of kernel support right now and it still being in very active development (by us)15:00
zygastgraber: how about netlink15:00
zygaand observing all process activity15:00
* cachio lunch15:00
zygaso match fork/exec/exit15:00
stgraberzyga: I didn't think we had a reliable API for that in netlink, short of having to ptrace stuff15:01
zygastgraber: I think there's something, not ptrace, there's a program that shows that (fork exec across all system) using netlink15:01
stgraberzyga: also, that'll break when snapd restarts as you'll miss all those events15:01
zygabut yeah, I don't know how that works yet15:01
zygastgraber: yes, it'd have to be a special process that's never quitting15:01
stgraberand if it's relying on tracing, then it won't work in containers15:01
zygazygote like per snap15:01
zygaAFAIK it is pure netlink15:02
mborzeckistgraber: alternative we considered before was putting the process in a well-known cgroup name eg. snap.app under the cgroup created for it by systemd (just one level deeper), but that'd involve walking /proc/*/cgroup15:02
mborzeckiand walking /proc/*/cgroup feels like a bad idea15:02
zygain v2 everything is somewhere in a hierarchy somewhere15:03
stgraberzyga: forkstat/netlink connector requires real root15:04
zygastgraber: I see15:04
zygastgraber: thank you for checking15:04
stgraberzyga: it's also apparently lossy, under load there's no delivery guarantee15:05
zygastgraber: thank you, this is even more relevant15:06
zygastgraber: anything we could use to tag a process with15:07
zygastgraber: I don't mind enumeration15:07
zygait won't be done all the time15:07
zygajust in special moments15:07
zygaI just want to see if we can identify a process as belonging to a snap15:07
zygaeven weird things like containers15:08
zygastgraber: thank you for all the input, it was invaluable,15:08
zygaI think we need to rethink the idea15:08
stgraberzyga: there have been discussions about ptags in the past but nothing ever got merged15:09
mupPR snapd#7719 opened: tests/main/gadget-update-pc: cleanup per-revision directories <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7719>15:09
mborzeckimvo: this addresses the failures due to /var/snap/pc/<rev> directories being left behind in the tests15:10
mborzeckimvo: ^^15:10
stgraberzyga: effectively arbitrary process tags which would be inherited and could even be marked immutable in some cases, it was one of the ideas put forth to solve the audit id issue for containers but after years of bikeshedding, nothing really happened15:10
mvomborzecki: nice, *thank you*15:11
zygastgraber: utterly crazy idea, set timerclask_ns to a unique value, use that15:13
diddledanI prefer timertelegram_ns :-p15:14
diddledanslack is second place :-D15:14
mvomborzecki: but in a meeting right now so will look in a bit15:16
mupPR snapd#7719 closed: tests/main/gadget-update-pc: cleanup per-revision directories <Simple 😃> <Created by bboozzoo> <Closed by bboozzoo> <https://github.com/snapcore/snapd/pull/7719>15:36
zygamvo: do you have time today?15:48
zygamvo: for a 10 minute call?15:48
zygamborzecki: do you have any technical ideas?15:49
zygamborzecki: or can we exchange ideas before your EOD15:49
zygamborzecki: it's fine to say no, perhaps at this stage having some rest would help15:49
zygamborzecki: I don't see a way to solve this issue now15:49
mborzeckizyga: tomorrow morning, i'm taking kids to some extra classes in 1015:50
zygathank you15:50
xnoxstgraber:  can snapd run inside lxd, on travis, on arm64? https://travis-ci.org/snapcore/core20/jobs/600808794?utm_medium=notification&utm_source=github_status15:59
xnoxstoopkid:  i see that core snap fails to setup security profiles?15:59
xnoxstoopkid:  unping15:59
xnoxstoopkid:  i see that core snap fails to setup security profiles, and i'm trying to get arm64 ci going with travis.15:59
zygait seems udevadm failed16:00
zygaexit code 216:00
xnoxi'll restart the build to see if things got any better16:01
mupPR snapd#7680 closed: daemon: parse and reject invalid channels in snap ops <Channels 🏷️> <Simple 😃> <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/7680>16:06
stgraberxnox: just try to run the snap install twice16:07
stgraberxnox: that's usually enough for those16:07
Chipaca#7671 needs two (2!) reviews, and it's the last chunk of the channels preparatory work16:07
mupPR #7671: overlord/patch: normalize tracking channel in state <Channels 🏷️> <Needs Samuele review> <Created by chipaca> <https://github.com/snapcore/snapd/pull/7671>16:07
zygaChipaca: is it okay if I call it quits now, sorry, if it was one review I'd jump in16:08
zygabut I need to break, think and move a little16:08
Chipacazyga: you can call it quits any time you need to16:08
Chipacazyga: also: stepping away from the keyboard to think is not quitting16:08
zygaand all my work is essentially down the drain today16:08
zygaso need to think if anything can save it16:08
* Chipaca hugs zyga 16:08
* diddledan hugs too16:08
* zyga hugs Chipaca 16:08
zygathank you guys!16:08
* zyga hugs diddledan too :)16:09
zygaanything, as crazy as possible, that technically works, is good16:09
zygaI'll grab my bike and go for a ride16:09
mvozyga: was in meetings, sorry16:14
oSoMoNjdstrand, when do you plan on making a release of review-tools that includes https://git.launchpad.net/review-tools/commit/?id=31c894178a8fd110b9f65f0ee532b7e11384f8b8 ?16:33
jdstrandoSoMoN: let me prepare a release now16:41
oSoMoNperfect, thanks!16:41
jdstrandroadmr: hi! would you mind pulling 20191104-1644UTC?16:46
roadmrjdstrand: sure16:47
jdstrandroadmr: thanks :)16:49
jdstrandoSoMoN: fyi ^ (will be a bit before in production)16:49
zygajdstrand: no name=snapd for now16:58
zygajdstrand: if you want to know more I can summarize16:59
zygajdstrand: if not we're going to discuss more tomorrow to see if we have other options16:59
jdstrandzyga: I read backscroll16:59
jdstrandzyga: it's too bad cause it seemed really elegant. guess it was too elegant, but good thing we got stgraber in the loop16:59
zygayes, the lesson I learned is to ask more experts early17:00
zygaI wasn't aware of the extra limitations on rootless containers17:00
zygaone idea17:01
xnoxstgraber:  har har har17:01
zygastgraber: can you answer one more question please17:01
zygastgraber: if we set a subreaper per snaps17:01
zyga*per snap*17:01
zygastgraber: will we be able to follow that chain reliably17:01
zygafrom random process, to their parent pid, to the subreaper17:01
zygaso that effectively, as long as the "watcher" subreapers are alive17:01
zygawe can reliably trace lineage?17:01
zygaso the idea is that instead of killing the watchers to kill the "container"17:02
zygawe can just scan all processes and try to match them to the reaper17:03
zygahmmm, reading the man page it seems not useful17:04
zygabecause it is not preserved across fork/clone17:04
zygabut is across exec17:04
zygaso we could watch our immediately forked process die17:04
zygaand if they had any children processes, we could watch them die if their parent (that we started) did quit17:05
zygabut I don't see anything in the man page linking the descendants to the reaper in a way that can be read17:05
* zyga goes to experiment17:06
zygamaybe it's not too bad actually17:06
=== pstolowski is now known as pstolowski|afk
mupPR snapcraft#2783 closed: cli: run review tools before pushing to the store if available <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2783>17:43
zygastgraber ^ this works17:45
zygajdstrand: ^ this works17:45
zygawe can track all children existence17:45
zygait's even better because the reaper can quit when they all all gone17:45
zygaso all we need to do is to unlink a "this snap is busy" file then17:46
zygastgraber: ^17:46
mupPR snapcraft#2785 closed: remote-build: add initial command unit tests <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2785>17:46
zygaa super-simple interactive reaper17:49
zygaI don't know if this can be made to work with proper pid tracking (e.g. how would this ever work for services)17:49
zygabut it's *something*17:49
zyganeed to ponder on it more17:49
ondrasergiusens did we change way SNAPCRAFT_PROJECT_DIR behaves? I'm getting empty string there18:08
ondrasergiusens SNAPCRAFT_PROJECT_NAME and SNAPCRAFT_PROJECT_GRADE seems to work fine18:08
sergiusensondra: no we did not change it18:14
ondrasergiusens then there is regression, as suddenly I'm getting empty string18:15
sergiusensondra: on stable?18:22
ondrasergiusens stable and candidate18:22
ondrasergiusens at least what I tested18:22
sergiusensondra: well stable has not changed for the pass 2 months18:23
ondrasergiusens hmm, then it must be this particular snap triggering it18:23
ondrasergiusens defo happening also with stable18:31
litheumit seems like snap is doing something terribly wrong with apparmor on lubuntu 18.04. i've installed both snap-store and vlc and both of them just throw tons of apparmor errors when i try to start them.21:33
Chipacalitheum: could you open a topic about this in the forum?21:49
Chipacalitheum: it should just work :-) if it doesn't we want to fix it21:49
Chipacabut I, personally, don't want to even think about fixing it at this time21:49
Chipaca:-) 'm off to bed21:50
mupPR snapcraft#2791 opened: introduce bind-ssh support <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/2791>23:23

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!