[05:52] kissiel: hey, good morning! ready when you are to talk about this autopkgtest failure, probably just a glitch but wanted to double check with you :) [06:12] morning [06:12] hey mborzecki [06:13] mvo: hey [06:15] mvo: ah, i see you're already pushing master merge to each pr [06:15] mborzecki: well, some :) [06:15] mborzecki: but yeah, we have some easy fixes [06:15] mborzecki: I still need to cherry pick this for 2.37 [06:16] mvo: well, no we're going to break google again :) [06:17] mborzecki: haaha - that was a funny error [06:17] I can see the headlines: "search engine gigant down because snapd runs too many tests…" [06:22] PR snapd#6442 opened: tests: workaround missing go dependencies in debian-9 [06:23] PR snapd#6400 closed: overlord/snapstate: use an ad-hoc error when no results [06:26] PR snapd#6389 closed: cmd/snap: small refactor of cmd_info's channel handling [07:32] mvo, zyga: any idea about https://forum.snapcraft.io/t/snapd-2-37-breaks-existing-snap-installation/9717 ? [07:39] morphis: thanks. we are looking at this now [07:42] mvo: thanks, if you need any live debugging let me know [07:46] morning peeps [07:48] mvo: zyga: pedronis: 6443 is something I'd like in 2.37 if it can sneak in [07:48] PR snapd#6443 opened: daemon, polkit: pid_t is signed [07:49] Chipaca: sure thing, it looks like we need a .1 soon because of a regression that morphis just reported [07:49] Chipaca: so lets try to move things quickly [07:49] * Chipaca shakes fist at morphis [07:49] * Chipaca hugs morphis tho [07:49] Chipaca: :-) [07:49] mborzecki: any idea what might have changed in snap-confine that gives the error that morphis reported above? [07:50] mvo: no, looking at it [07:50] mborzecki: ta, I was just looking at the diff (lots of churn :/ but nothing so far that looks obvious [07:52] mborzecki: I still have the 2.37 core snap installed, if you want me anything to try let me know [07:52] Goood morning [07:52] mborzecki: thanks, please consider this priority as it breaks users, we will push hard for a fix [07:52] otherwise I am going to revert core [07:52] zyga: hey, good morning [07:53] zyga: bad news, morphis found a regression in 2.37 === cpaelzer__ is now known as cpaelzer [07:53] I think it is not a regression [07:53] Let me wake up and get to my keyboard [07:53] morphis: also, could you please "snap refresh --candidate core" on all your boxes? that would be awesome because it helps us to find these issues slightly earlier (and candidate is as good as stable in 99,9% of the cases) [07:54] zyga: thank you! keen to learn more :) [07:55] Usage of home outside of /home always required local configuration changes [07:56] mvo: our dev boxes use candidate already but not our CI ones [07:57] zyga: these are classic snaps and things were working well for < 2.37 [07:57] zyga: can HOME/HOMEDIRS be set/extended manually? [07:59] * Chipaca ⇝ school run [08:01] zyga: do you know what code change created this denial? [08:03] mvo: afaict it's snap-confine, though it seems to have the right rules to allow creating directories inside @{HOME} === pstolowski|afk is now known as pstolowski [08:05] mornings [08:06] morphis: did you need to tweak /etc/apparmor.d/tunables/home.d for jenkins before? [08:06] mborzecki: iirc HOME really just means /home :/ [08:07] mborzecki: I wonder what change in s-c caused this, I mean, if this used to work something most have changed [08:07] mvo: morphis mentioned that he's using /var/lib/jenkins as $HOME for jenkins [08:08] mborzecki: yeah, what I mean is iirc apparmor tools are not very smart about @{HOME} when it is outside of /home but I may be wrong here [08:09] Chipaca: your PR fails in the unit tests, could you pleae check and force-push (force-push so that we can still easily cherry-pick into 2.37) [08:11] mborzecki: no, I didn't [08:12] mborzecki, mvo, zyga: the problem appears with classic snaps (go, snapcraft) so theoretically they should be allowed to write to $HOME regardless where it is [08:13] morphis: the error is from snap-confine: it'll have the same policy no matter what snap it is running for [08:15] morphis: its trivial to reproduce feel free to revert your snap [08:17] mvo: wonder if adding /var/lib/jenkins/ to /etc/apparmor.d/tunables/home would fix it [08:17] morphis: hm, this is strange - with the previous stable core (2.36.3) I get the same error - what was your previous version [08:18] mborzecki: I'm pretty sure it would [08:18] mborzecki: the interessting question is why is it breaking for morphis now [08:18] mborzecki: but wasn't before [08:18] mborzecki: and an we fix it for him somehow :) [08:18] re [08:18] mvo: so: [08:19] (I mean, not just for him for everyone who might be affected) [08:19] mvo: the change is a result of removing quirks code [08:19] oh [08:19] mvo: https://pastebin.canonical.com/p/Rs8TmRYGJS/ [08:19] zyga: tell us more :P [08:19] mvo: this is not a regression in my eyes for the following reason [08:19] mvo: home directories outside of /home require local configuration to work [08:19] mvo: they may have worked partially before [08:19] mvo: but we never tested or supported that [08:20] mvo: it may have been that some subset of snaps operated okey [08:20] mvo: but it's unclear if all interfaces were supported or if not all, why not [08:20] mvo: my advice is as follows: [08:20] mvo: for the people affected the same advice present on the forum stands [08:21] mvo: edit local apparmor configuration to indicate HOMES contains /var/lib/jenkins [08:21] mvo: for 2.38 we can look at properly supporting thta [08:21] mvo: including looking across the stack at assumptions, writing tests, etc [08:22] mvo: so that's that [08:23] mvo: in my eyes this is not a regression, it is a change but not all changes are regressions [08:23] zyga: we don't have a plan to make that work for 2.38 tough, unclear how much work it is even [08:23] mvo: it was an unsupported feature before [08:23] zyga: right, people can workaround this issue. I would still like to understand if there is anything we can do and how the quirks changed things. even if its not technically a regression if it breaks existing installs thats not good (tm) [08:23] mvo: the reason is very simple, to allow quirks in /var/lib/lxd snap-confine had wide permissions to /var/lib [08:24] zyga: aha, that makes sense [08:24] * mvo scratches head [08:24] right, so /var/lib/jenkins would fit in there nicely [08:24] yeah, now it all makes sense [08:24] * zyga goes to make coffee [08:25] sorry, stayed up late last night [08:25] I still feel very uneasy about this [08:25] * mvo scratches head harder [08:25] mvo: it's a bit unclear we can fix it easily for 2.38 tough, I wouldn't count on it [08:26] pedronis: yeah, I'm fine with that, I am not looking for a general fix [08:26] pedronis: just wondering if we can do anything about this particular case without things being horrible [08:26] we could restore the permission, write a a few tests, add test-var user with home in /var/lib/test, run some selection of tests that run as an user there [08:26] mvo: well, if we put back the open rule is a general fix but it means living with it forever [08:26] do we want that [08:26] sounds a jdstrand discussion [08:26] pedronis: I agree [08:27] the bad aspect of $HOME is that it can in theory point anywhere [08:27] I think supporting /var/lib/jenkins explicitly is sane [08:27] but /var/lib/snapd is not :) [08:27] pedronis: yeah, that is the question I want to discuss. its fine if we say "no" it just want to make sure we explored options [08:27] so there's that balance [08:27] zyga: indeed [08:27] * mvo scratches head more [08:28] sorry for beinig late, I hope the explanation makes some sense [08:28] zyga: yeah, the key was the apparmor /var/lib quirks thing, that was the missing piece for me [08:30] zyga: so what is the workaround you mentioned for this? [08:30] morphis: let me pull up the forum post [08:30] morphis: add one line to the proper config fle [08:30] file [08:31] zyga: i've edited apparmor tunables locall, but creating /var/lib/jenkins/snap/hello-world/27 fails, isn't /var/lib/ ro in the mount ns? [08:32] ahhh [08:32] interestng [08:32] interesting [08:32] so [08:32] this is deeper than thata [08:32] * zyga needs to wake up first [08:33] morphis: thanks for the pastebin above, what does "snap list --all core" say? [08:34] zyga: yeah, /var/lib is still from the core snap, we mount over /var/lib/snapd and quirks did a bind mount over /var/lib from what i can see [08:34] oh, interessting [08:34] the quirks code added /var/lib/lxd [08:34] but at the same time made /var/lib writable [08:35] mvo: https://pastebin.canonical.com/p/6b2J4rMFdD/ [08:36] zyga: could we have snapd set up the snap mount profile accordingly when we find well-known home dirs in /var/lib? [08:36] Chipaca: nevermind, I pushed the (trivial) update [08:37] yes but it would not work for /var/lbi [08:37] morphis: thank you [08:37] the quirks code was special [08:37] it knew how to move /var/lib/snapd aside [08:37] and move it back [08:37] it also ran in a context where that is possible [08:37] regular mount namespace updates cannot do that [08:38] first of all, they have no logic for evading "shadowing" using mount --move [08:38] and second of all, they have no scratch space outside of the filesystem [08:38] the old quirk code ran before we did the pivot [08:38] I have two quick ideas: [08:39] 1) explicit support for jenkins [08:39] add /var/lib/jenkinks to the core snaap [08:39] make appropriate changes to snapd / snap-confine to match [08:39] 2) add home rewriting [08:40] for home outside of /home we synthesize a home directory and bind mount the original location to /var/lib/snapd/homes/$LOGNAME [08:40] the 2nd idea seems better [08:40] it would mean we can essentially have any home directory on the outside [08:40] zyga: another thing, the case reported by morphis was for classic snaps, so we don't need to do anything about mount there (yet) [08:41] mborzecki: apparently snap-confine still insisted on making a directory for the classic snap [08:41] zyga: yeah, very much so [08:41] * zyga drinks coffee [08:44] zyga, mborzecki, mvo: if there is nothing else I can do right now, I will revert back to the old core snap to get our CI back working [08:50] Chipaca: do you want to take a look at the changes in #6333, some patches were pushed since you approved it [08:50] PR #6333: daemon: introduce /v2/connections snapd API endpoint [08:52] mborzecki: hey, would you take a look at another hotplug PR? https://github.com/snapcore/snapd/pull/6106 [08:52] PR #6106: overlord/ifacestate: handler for hotplug-update-slot tasks [08:53] pstolowski: sure [08:53] ty [08:54] zyga: hm, how hard would home rewriting be? [08:55] mborzecki: will do [08:55] mvo: thanks [08:55] mborzecki: does 6016 need a master merge ? [08:55] Chipaca: thanks [08:55] mvo: snap-confine would bind mount home from wherver it was to a new location such as /var/lib/snapd/homes [08:55] pedronis: heh, yes, pushed it to some other prs but not that one [08:55] mvo: snapd would cooperate, telling snap-confine the home for each username [08:56] mvo: there are some complications there though [08:56] zyga: that sounds not too complicated [08:56] zyga: oh? [08:56] for instance, it cannot be done by snap-update-ns [08:56] so snap-confine would need to do it [08:56] so it would have to have the permission to do it [08:57] this is complicated because again, it is open ended [08:57] but we could begin with a simple constraint [08:57] $HOME is either /home/* or /var/lib/jenkins [08:57] that would solve it for "now" [08:57] zyga: it looks like it is sc_nonfatal_mkpath needing read access to all parent directories of the home dir as it does its mkdirat/openat chain [08:57] indeed [08:58] mvo: in the end the $HOME variable (alongside things that use it, like $SNAP_USER_DATA) would use the rewritten location [08:58] mvo: for lesses impact we could only do this if $HOME is not somewhere in /home/ [08:58] zyga: the later sounds more compelling [08:58] if there was a version that assumed the home directory existed as a starting point, the AppArmor @{HOME} tweakable would be enough [08:59] it's just me, or the forum is flaky today [08:59] jamesh: here the problem is that home for jenkins is not representable in the mount namespace at all [09:01] zyga: once that's solved, presumably it's still going to hit the denial for opening /var though [09:01] jamesh: at that time we would make a fixed change to the apparamor profile to allow writes to /var/lib/snapd/homes [09:04] mborzecki: what was the conclusion about --disconnected? [09:05] Chipaca: i don't think there was any [09:05] pedronis: yo [09:05] Chipaca: yes [09:05] pedronis: snap connections --disconnected? [09:06] yes? [09:06] pedronis: me and one other person thought it might be a good idea, wdyt? [09:06] Chipaca: used where? [09:06] with a snap, without a snap? [09:07] anyway I cannot reach the forum today, it gives me network errors half the time [09:07] pedronis: seems to work here [09:07] fine here too. take your foot off the cable [09:08] pedronis: https://paste.ubuntu.com/p/f3HGxwDcwk/ the forum message, i have it working with and without a snap [09:08] mborzecki: the original suggestion was to show everything if a snap is given [09:09] Chipaca: I'm not super happy with the need to of headers or footers to explain what has been left out [09:10] pedronis: ok, so you'd opt for the original suggestion, show both conencted & disconnected when a snap is given, show only connected when no snap is given? [09:10] yes [09:10] we still have all for the 2nd case, no? [09:10] or is that yet something else [09:11] pedronis: you mean --all? [09:11] yes [09:11] pedronis: i'm assuming it's in (seems uncontroversial anyway) [09:14] PR snapd#6333 closed: daemon: introduce /v2/connections snapd API endpoint [09:18] mvo: one more observation, while morphis may revert now, once 2.37.1 is out, he will be back in hot water [09:20] zyga: yes, reverting doesn't solve anything long term for him [09:21] mborzecki: the next bit is 6387? does it need a merge and tweaks ? [09:22] pedronis: yup, pushing it now [09:22] spread tests should be green now [09:23] zyga: can i have a second review of 6443? it's green an' all [09:24] Chipaca: certainly sir, looking [09:24] Son_Goku: ping [09:24] Chipaca: ah, user ids are tricky [09:24] Chipaca: thank you for the patch! [09:24] ikr [09:25] zyga: is there already a date for 2.37.1? [09:25] 6443 needs a second review [09:25] mvo: zyga is on it [09:25] ta [09:25] morphis: there are some interface patches from jamie lined up [09:25] zyga: and btw. I didn't picked /var/lib/jenkins as HOME for jenkins, it does that by default [09:26] mvo: merged [09:26] morphis: right, I know [09:27] PR snapd#6443 closed: daemon, polkit: pid_t is signed [09:28] mvo: https://github.com/snapcore/snapd/pull/6442#pullrequestreview-197449157 [09:28] PR #6442: tests: workaround missing go dependencies in debian-9 [09:28] mvo: let's land it :) [09:30] can somebody merge master into 6440 ? [09:30] zyga: will land in a wee bit [09:30] mmm [09:32] zyga: debina-9 is also merged now [09:32] PR snapd#6442 closed: tests: workaround missing go dependencies in debian-9 [09:32] having 6413 would be good, but this needs a second review [09:33] but I guess its fine, we can use salsa [09:33] mvo: zyga: see #webops internally [09:34] zyga: looks like more people are affected, can we revert the quirks code? [09:36] I see no other way [09:36] zyga: iirc that was 6123 [09:36] but please let me spend a moment [09:36] I will write a quick test [09:36] perhaps we don't need all of quirks [09:36] I wonder why nobody tested candidate :/ [09:36] morphis: did you file a bug for this? [09:36] zyga: you write a test and I prepare the revert [09:36] mvo: no wait [09:37] mvo: I don't think we want the full blown revert [09:37] mvo: it's more complex because of other changes there [09:37] mvo: please give me two hours [09:37] zyga: no, just a forum topic [09:37] morphis: can you file a quick one, just so that we have a bug number [09:37] morphis: that should be fine [09:37] mvo: or rather, give me 15 minutes for quick assesment [09:38] morphis, zyga I can write it if we need it [09:38] zyga: ok [09:38] PR snapd#6437 closed: snap/channel: improve channel parsing [09:38] zyga: I think its fine to revert and least risky but if you justneed 15min thats fine :) [09:38] mvo: revert won't apply cleanly anymore [09:38] that's what I mean by more work [09:38] zyga: aha, ok [09:39] zyga: conflicts are trivial [09:41] PR snapd#6444 opened: Revert "cmd/snap-confine,snap-update-ns: discard quirks" <⛔ Blocked> [09:41] zyga: ocd or something but I let you investigate and we can talk in a wee bit [09:41] k [09:42] thanks [09:42] we can look at tests [09:43] zyga: yeah, given the urgency I want this as a plan B, feel free to poke and look for better solutions [09:43] k [09:43] make sense [09:44] zyga: having a proper regression test would be good too [09:47] PR snapd#6445 opened: [RFC] snap-confine: revert "cmd/snap-confine,snap-update-ns: discard quirks" [09:56] mvo: hey! I finally managed to reproduce that crash; working on a fix [10:00] mvo: so [10:00] https://www.irccloud.com/pastebin/tx6qPpbI/ [10:00] mvo: this coupled with mkdir on /var/lib/jenkins in core / core18 fixes the issue without bringing back quirks [10:01] mvo: can we have a call about this? [10:02] zyga: a call about what? [10:02] about the approach to take [10:02] I would really prefer not to bring back all of quirks [10:04] zyga: a mkdir in core / core18 ? you mean we need to add that dir to the snaps? [10:04] yes [10:04] mmh [10:04] kissiel: no worries, is it something on our side? [10:04] mvo: leaning towards that, yes [10:05] kissiel: do you have more details? if it is something on our side we would like to fix it :) [10:05] mvo: damn, sorry, misunderstood, more on our side :) [10:05] i mean cert's [10:05] kissiel: haha, no worries [10:05] so stay tuned [10:05] kissiel: thank you [10:07] zyga, pedronis the small size of this is compelling - the fact that we need to add the dir not so much :/ [10:07] mvo: adding the directory is cheap, adding that code restores cost for each snap startup [10:08] zyga: yeah, I can see that [10:09] zyga, pedronis we could add the dir until we have better !/home support? [10:09] what is that dir visible to? [10:09] we need to roll out new core anyway, this would mean the change is smaller [10:09] pedronis: in 2.36 quirks made the full real host /var/lib visible [10:09] pedronis: in 2.37 only the /var/lib from base snap is visible [10:10] pedronis: apparmor prevents access as usual though this is fixed by local configuration of HOMEDIRS [10:11] zyga: do we do something special about extrausers? [10:11] no [10:11] zyga: imo HOMEDIRS is not enough, we open each intermediate path and need r there [10:11] this is only affecting classic [10:11] and the patch I pasted only configures this for classic [10:11] mborzecki: I know, that was not what I meant [10:12] zyga: we have a couple of interfaces that gives access to var/lib stuff, are you saying they are broken in 2.37 now? [10:12] ah ok [10:12] mborzecki: look at my patch above [10:12] they owuld have worked in 2.36 if all var/lib was mounted [10:12] pedronis: no [10:12] pedronis: which interfaces are those? [10:12] hi, is using snap install --dangerous ./mysnap.snap --name mysnap_2 the right way to do a parallel install from a local snap? [10:12] I didn't know you could parallel install local snaps [10:13] zyga: account-control, classic-support, network-control [10:13] ackk: yes [10:13] pedronis: what I'm describing so far only affects core systems [10:13] er [10:13] popey: you can [10:13] classic systems [10:13] nice [10:13] on core systems there are other things in /var/lib [10:13] zyga: they all give access to bits of var/lib [10:13] are they broken now? or not? [10:13] I imagine they needs the bits from host [10:13] mborzecki, but if you're installing directly from the store you should just use mysnap_2 right? [10:13] are they using mount support properly? [10:14] ackk: yes [10:14] pedronis: looking at master [10:14] pedronis: excluding /var/lib/snapd/* [10:14] yes [10:14] pedronis: and excluding /var/lib/extrausers [10:15] because? [10:15] I see the following [10:15] anyway, I get this: https://pastebin.ubuntu.com/p/fY9gcjw2bJ/ [10:15] (because those have bind mounts) [10:15] https://www.irccloud.com/pastebin/lH6YnuB2/ [10:16] I would have to check each one to see if they are for core or classic, hold on [10:16] ackk: that's because the snap file has metadata that states the snap name is 'mysnap' so you need to pass the name you want separately, for the store install we know everything just having mysnap_2 [10:16] pedronis: alsa is affected [10:16] zyga: see [10:16] mvo: so we have a bit of a bigger problem [10:17] I think there's more though [10:17] the quirks code only ran if you had /var/lib/lxd AFAIR [10:17] so no lxd, no quirks [10:17] let me double check [10:17] pedronis: we could bring the code for each of those (alsa, dhcp) to the appropriate interface [10:17] if they really affect classic [10:17] Chipaca: pong [10:18] yes, just saying that 2.37 might be broken more than just related to jenkins [10:18] pedronis: quirk code ran unconditionally on core (but not core18) and only on classic systems [10:19] Son_Goku: ok to use snappy in that context [10:19] cool [10:19] I'm going to change it then [10:19] zyga: still, some snaps might be broken now [10:19] apparently [10:20] pedronis, zyga if thats the case the quirks revert seems to be the best option, yes? [10:20] mvo: I disagree, I think bringing back quirks is the wrong move [10:20] mvo: would not say best, it's the quickest [10:21] looking at the list we need /var/lib/alsa/ for alsa, /var/lib/usbutils for hardware-observe, /var/lib/systemd/catalog/database for log-observe and /var/lib/dhcp/ for network-control [10:22] apparently none of those are tested [10:22] of course [10:22] I'd like to remind us that quirks were done in response to a bug in a similar situation (release broke lxd) [10:22] we don't have complete tests for all permissions for any interface afaik [10:22] we didn't anticipate it would open more things than it did [10:22] bringing that back would not only fix those, but allow us to make the same mistake, silently, again [10:23] zyga: I understand, but breaking things is not good [10:23] pedronis: I agree on that [10:23] zyga: how long would it take to do the correct fix for all relevant things? [10:23]