| mvo | 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 :) | 05:52 |
|---|---|---|
| mborzecki | morning | 06:12 |
| mvo | hey mborzecki | 06:12 |
| mborzecki | mvo: hey | 06:13 |
| mborzecki | mvo: ah, i see you're already pushing master merge to each pr | 06:15 |
| mvo | mborzecki: well, some :) | 06:15 |
| mvo | mborzecki: but yeah, we have some easy fixes | 06:15 |
| mvo | mborzecki: I still need to cherry pick this for 2.37 | 06:15 |
| mborzecki | mvo: well, no we're going to break google again :) | 06:16 |
| mvo | mborzecki: haaha - that was a funny error | 06:17 |
| mvo | I can see the headlines: "search engine gigant down because snapd runs too many tests…" | 06:17 |
| mup | PR snapd#6442 opened: tests: workaround missing go dependencies in debian-9 <Created by mvo5> <https://github.com/snapcore/snapd/pull/6442> | 06:22 |
| mup | PR snapd#6400 closed: overlord/snapstate: use an ad-hoc error when no results <Created by chipaca> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6400> | 06:23 |
| mup | PR snapd#6389 closed: cmd/snap: small refactor of cmd_info's channel handling <Created by chipaca> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6389> | 06:26 |
| morphis | mvo, zyga: any idea about https://forum.snapcraft.io/t/snapd-2-37-breaks-existing-snap-installation/9717 ? | 07:32 |
| mvo | morphis: thanks. we are looking at this now | 07:39 |
| morphis | mvo: thanks, if you need any live debugging let me know | 07:42 |
| Chipaca | morning peeps | 07:46 |
| Chipaca | mvo: zyga: pedronis: 6443 is something I'd like in 2.37 if it can sneak in | 07:48 |
| mup | PR snapd#6443 opened: daemon, polkit: pid_t is signed <Simple 😃> <Created by chipaca> <https://github.com/snapcore/snapd/pull/6443> | 07:48 |
| mvo | Chipaca: sure thing, it looks like we need a .1 soon because of a regression that morphis just reported | 07:49 |
| mvo | Chipaca: so lets try to move things quickly | 07:49 |
| * Chipaca shakes fist at morphis | 07:49 | |
| * Chipaca hugs morphis tho | 07:49 | |
| morphis | Chipaca: :-) | 07:49 |
| mvo | mborzecki: any idea what might have changed in snap-confine that gives the error that morphis reported above? | 07:49 |
| mborzecki | mvo: no, looking at it | 07:50 |
| mvo | mborzecki: ta, I was just looking at the diff (lots of churn :/ but nothing so far that looks obvious | 07:50 |
| morphis | mborzecki: I still have the 2.37 core snap installed, if you want me anything to try let me know | 07:52 |
| zyga | Goood morning | 07:52 |
| mvo | mborzecki: thanks, please consider this priority as it breaks users, we will push hard for a fix | 07:52 |
| morphis | otherwise I am going to revert core | 07:52 |
| mvo | zyga: hey, good morning | 07:52 |
| mvo | zyga: bad news, morphis found a regression in 2.37 | 07:53 |
| === cpaelzer__ is now known as cpaelzer | ||
| zyga | I think it is not a regression | 07:53 |
| zyga | Let me wake up and get to my keyboard | 07:53 |
| mvo | 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:53 |
| mvo | zyga: thank you! keen to learn more :) | 07:54 |
| zyga | Usage of home outside of /home always required local configuration changes | 07:55 |
| morphis | mvo: our dev boxes use candidate already but not our CI ones | 07:56 |
| morphis | zyga: these are classic snaps and things were working well for < 2.37 | 07:57 |
| mborzecki | zyga: can HOME/HOMEDIRS be set/extended manually? | 07:57 |
| * Chipaca ⇝ school run | 07:59 | |
| mvo | zyga: do you know what code change created this denial? | 08:01 |
| mborzecki | mvo: afaict it's snap-confine, though it seems to have the right rules to allow creating directories inside @{HOME} | 08:03 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | mornings | 08:05 |
| mborzecki | morphis: did you need to tweak /etc/apparmor.d/tunables/home.d for jenkins before? | 08:06 |
| mvo | mborzecki: iirc HOME really just means /home :/ | 08:06 |
| mvo | mborzecki: I wonder what change in s-c caused this, I mean, if this used to work something most have changed | 08:07 |
| mborzecki | mvo: morphis mentioned that he's using /var/lib/jenkins as $HOME for jenkins | 08:07 |
| mvo | 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:08 |
| mvo | 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:09 |
| morphis | mborzecki: no, I didn't | 08:11 |
| morphis | 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:12 |
| jamesh | morphis: the error is from snap-confine: it'll have the same policy no matter what snap it is running for | 08:13 |
| mvo | morphis: its trivial to reproduce feel free to revert your snap | 08:15 |
| mborzecki | mvo: wonder if adding /var/lib/jenkins/ to /etc/apparmor.d/tunables/home would fix it | 08:17 |
| mvo | morphis: hm, this is strange - with the previous stable core (2.36.3) I get the same error - what was your previous version | 08:17 |
| mvo | mborzecki: I'm pretty sure it would | 08:18 |
| mvo | mborzecki: the interessting question is why is it breaking for morphis now | 08:18 |
| mvo | mborzecki: but wasn't before | 08:18 |
| mvo | mborzecki: and an we fix it for him somehow :) | 08:18 |
| zyga | re | 08:18 |
| zyga | mvo: so: | 08:18 |
| mvo | (I mean, not just for him for everyone who might be affected) | 08:19 |
| zyga | mvo: the change is a result of removing quirks code | 08:19 |
| mborzecki | oh | 08:19 |
| morphis | mvo: https://pastebin.canonical.com/p/Rs8TmRYGJS/ | 08:19 |
| mborzecki | zyga: tell us more :P | 08:19 |
| zyga | mvo: this is not a regression in my eyes for the following reason | 08:19 |
| zyga | mvo: home directories outside of /home require local configuration to work | 08:19 |
| zyga | mvo: they may have worked partially before | 08:19 |
| zyga | mvo: but we never tested or supported that | 08:19 |
| zyga | mvo: it may have been that some subset of snaps operated okey | 08:20 |
| zyga | mvo: but it's unclear if all interfaces were supported or if not all, why not | 08:20 |
| zyga | mvo: my advice is as follows: | 08:20 |
| zyga | mvo: for the people affected the same advice present on the forum stands | 08:20 |
| zyga | mvo: edit local apparmor configuration to indicate HOMES contains /var/lib/jenkins | 08:21 |
| zyga | mvo: for 2.38 we can look at properly supporting thta | 08:21 |
| zyga | mvo: including looking across the stack at assumptions, writing tests, etc | 08:21 |
| zyga | mvo: so that's that | 08:22 |
| zyga | mvo: in my eyes this is not a regression, it is a change but not all changes are regressions | 08:23 |
| pedronis | zyga: we don't have a plan to make that work for 2.38 tough, unclear how much work it is even | 08:23 |
| zyga | mvo: it was an unsupported feature before | 08:23 |
| mvo | 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 |
| zyga | mvo: the reason is very simple, to allow quirks in /var/lib/lxd snap-confine had wide permissions to /var/lib | 08:23 |
| mvo | zyga: aha, that makes sense | 08:24 |
| * mvo scratches head | 08:24 | |
| mborzecki | right, so /var/lib/jenkins would fit in there nicely | 08:24 |
| mvo | yeah, now it all makes sense | 08:24 |
| * zyga goes to make coffee | 08:24 | |
| zyga | sorry, stayed up late last night | 08:25 |
| mvo | I still feel very uneasy about this | 08:25 |
| * mvo scratches head harder | 08:25 | |
| pedronis | mvo: it's a bit unclear we can fix it easily for 2.38 tough, I wouldn't count on it | 08:25 |
| mvo | pedronis: yeah, I'm fine with that, I am not looking for a general fix | 08:26 |
| mvo | pedronis: just wondering if we can do anything about this particular case without things being horrible | 08:26 |
| zyga | 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 |
| pedronis | mvo: well, if we put back the open rule is a general fix but it means living with it forever | 08:26 |
| pedronis | do we want that | 08:26 |
| pedronis | sounds a jdstrand discussion | 08:26 |
| zyga | pedronis: I agree | 08:26 |
| zyga | the bad aspect of $HOME is that it can in theory point anywhere | 08:27 |
| zyga | I think supporting /var/lib/jenkins explicitly is sane | 08:27 |
| zyga | but /var/lib/snapd is not :) | 08:27 |
| mvo | 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 |
| zyga | so there's that balance | 08:27 |
| mvo | zyga: indeed | 08:27 |
| * mvo scratches head more | 08:27 | |
| zyga | sorry for beinig late, I hope the explanation makes some sense | 08:28 |
| mvo | zyga: yeah, the key was the apparmor /var/lib quirks thing, that was the missing piece for me | 08:28 |
| morphis | zyga: so what is the workaround you mentioned for this? | 08:30 |
| zyga | morphis: let me pull up the forum post | 08:30 |
| zyga | morphis: add one line to the proper config fle | 08:30 |
| zyga | file | 08:30 |
| mborzecki | 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:31 |
| zyga | ahhh | 08:32 |
| zyga | interestng | 08:32 |
| zyga | interesting | 08:32 |
| zyga | so | 08:32 |
| zyga | this is deeper than thata | 08:32 |
| * zyga needs to wake up first | 08:32 | |
| mvo | morphis: thanks for the pastebin above, what does "snap list --all core" say? | 08:33 |
| mborzecki | 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 |
| mvo | oh, interessting | 08:34 |
| zyga | the quirks code added /var/lib/lxd | 08:34 |
| zyga | but at the same time made /var/lib writable | 08:34 |
| morphis | mvo: https://pastebin.canonical.com/p/6b2J4rMFdD/ | 08:35 |
| mborzecki | zyga: could we have snapd set up the snap mount profile accordingly when we find well-known home dirs in /var/lib? | 08:36 |
| mvo | Chipaca: nevermind, I pushed the (trivial) update | 08:36 |
| zyga | yes but it would not work for /var/lbi | 08:37 |
| mvo | morphis: thank you | 08:37 |
| zyga | the quirks code was special | 08:37 |
| zyga | it knew how to move /var/lib/snapd aside | 08:37 |
| zyga | and move it back | 08:37 |
| zyga | it also ran in a context where that is possible | 08:37 |
| zyga | regular mount namespace updates cannot do that | 08:37 |
| zyga | first of all, they have no logic for evading "shadowing" using mount --move | 08:38 |
| zyga | and second of all, they have no scratch space outside of the filesystem | 08:38 |
| zyga | the old quirk code ran before we did the pivot | 08:38 |
| zyga | I have two quick ideas: | 08:38 |
| zyga | 1) explicit support for jenkins | 08:39 |
| zyga | add /var/lib/jenkinks to the core snaap | 08:39 |
| zyga | make appropriate changes to snapd / snap-confine to match | 08:39 |
| zyga | 2) add home rewriting | 08:39 |
| zyga | for home outside of /home we synthesize a home directory and bind mount the original location to /var/lib/snapd/homes/$LOGNAME | 08:40 |
| zyga | the 2nd idea seems better | 08:40 |
| zyga | it would mean we can essentially have any home directory on the outside | 08:40 |
| mborzecki | 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:40 |
| zyga | mborzecki: apparently snap-confine still insisted on making a directory for the classic snap | 08:41 |
| mborzecki | zyga: yeah, very much so | 08:41 |
| * zyga drinks coffee | 08:41 | |
| morphis | 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:44 |
| mborzecki | Chipaca: do you want to take a look at the changes in #6333, some patches were pushed since you approved it | 08:50 |
| mup | PR #6333: daemon: introduce /v2/connections snapd API endpoint <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6333> | 08:50 |
| pstolowski | mborzecki: hey, would you take a look at another hotplug PR? https://github.com/snapcore/snapd/pull/6106 | 08:52 |
| mup | PR #6106: overlord/ifacestate: handler for hotplug-update-slot tasks <Hotplug 🔌> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6106> | 08:52 |
| mborzecki | pstolowski: sure | 08:53 |
| pstolowski | ty | 08:53 |
| mvo | zyga: hm, how hard would home rewriting be? | 08:54 |
| Chipaca | mborzecki: will do | 08:55 |
| Chipaca | mvo: thanks | 08:55 |
| pedronis | mborzecki: does 6016 need a master merge ? | 08:55 |
| mborzecki | Chipaca: thanks | 08:55 |
| zyga | mvo: snap-confine would bind mount home from wherver it was to a new location such as /var/lib/snapd/homes | 08:55 |
| mborzecki | pedronis: heh, yes, pushed it to some other prs but not that one | 08:55 |
| zyga | mvo: snapd would cooperate, telling snap-confine the home for each username | 08:55 |
| zyga | mvo: there are some complications there though | 08:56 |
| mvo | zyga: that sounds not too complicated | 08:56 |
| mvo | zyga: oh? | 08:56 |
| zyga | for instance, it cannot be done by snap-update-ns | 08:56 |
| zyga | so snap-confine would need to do it | 08:56 |
| zyga | so it would have to have the permission to do it | 08:56 |
| zyga | this is complicated because again, it is open ended | 08:57 |
| zyga | but we could begin with a simple constraint | 08:57 |
| zyga | $HOME is either /home/* or /var/lib/jenkins | 08:57 |
| zyga | that would solve it for "now" | 08:57 |
| jamesh | 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 |
| zyga | indeed | 08:57 |
| zyga | mvo: in the end the $HOME variable (alongside things that use it, like $SNAP_USER_DATA) would use the rewritten location | 08:58 |
| zyga | mvo: for lesses impact we could only do this if $HOME is not somewhere in /home/ | 08:58 |
| mvo | zyga: the later sounds more compelling | 08:58 |
| jamesh | if there was a version that assumed the home directory existed as a starting point, the AppArmor @{HOME} tweakable would be enough | 08:58 |
| pedronis | it's just me, or the forum is flaky today | 08:59 |
| zyga | jamesh: here the problem is that home for jenkins is not representable in the mount namespace at all | 08:59 |
| jamesh | zyga: once that's solved, presumably it's still going to hit the denial for opening /var though | 09:01 |
| zyga | jamesh: at that time we would make a fixed change to the apparamor profile to allow writes to /var/lib/snapd/homes | 09:01 |
| Chipaca | mborzecki: what was the conclusion about --disconnected? | 09:04 |
| mborzecki | Chipaca: i don't think there was any | 09:05 |
| Chipaca | pedronis: yo | 09:05 |
| pedronis | Chipaca: yes | 09:05 |
| Chipaca | pedronis: snap connections --disconnected? | 09:05 |
| pedronis | yes? | 09:06 |
| Chipaca | pedronis: me and one other person thought it might be a good idea, wdyt? | 09:06 |
| pedronis | Chipaca: used where? | 09:06 |
| pedronis | with a snap, without a snap? | 09:06 |
| pedronis | anyway I cannot reach the forum today, it gives me network errors half the time | 09:07 |
| Chipaca | pedronis: seems to work here | 09:07 |
| popey | fine here too. take your foot off the cable | 09:07 |
| mborzecki | pedronis: https://paste.ubuntu.com/p/f3HGxwDcwk/ the forum message, i have it working with and without a snap | 09:08 |
| pedronis | mborzecki: the original suggestion was to show everything if a snap is given | 09:08 |
| pedronis | Chipaca: I'm not super happy with the need to of headers or footers to explain what has been left out | 09:09 |
| mborzecki | 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 |
| pedronis | yes | 09:10 |
| pedronis | we still have all for the 2nd case, no? | 09:10 |
| pedronis | or is that yet something else | 09:10 |
| mborzecki | pedronis: you mean --all? | 09:11 |
| pedronis | yes | 09:11 |
| mborzecki | pedronis: i'm assuming it's in (seems uncontroversial anyway) | 09:11 |
| mup | PR snapd#6333 closed: daemon: introduce /v2/connections snapd API endpoint <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6333> | 09:14 |
| zyga | mvo: one more observation, while morphis may revert now, once 2.37.1 is out, he will be back in hot water | 09:18 |
| pedronis | zyga: yes, reverting doesn't solve anything long term for him | 09:20 |
| pedronis | mborzecki: the next bit is 6387? does it need a merge and tweaks ? | 09:21 |
| mborzecki | pedronis: yup, pushing it now | 09:22 |
| mborzecki | spread tests should be green now | 09:22 |
| Chipaca | zyga: can i have a second review of 6443? it's green an' all | 09:23 |
| zyga | Chipaca: certainly sir, looking | 09:24 |
| Chipaca | Son_Goku: ping | 09:24 |
| zyga | Chipaca: ah, user ids are tricky | 09:24 |
| zyga | Chipaca: thank you for the patch! | 09:24 |
| Chipaca | ikr | 09:24 |
| morphis | zyga: is there already a date for 2.37.1? | 09:25 |
| mvo | 6443 needs a second review | 09:25 |
| Chipaca | mvo: zyga is on it | 09:25 |
| mvo | ta | 09:25 |
| zyga | morphis: there are some interface patches from jamie lined up | 09:25 |
| morphis | zyga: and btw. I didn't picked /var/lib/jenkins as HOME for jenkins, it does that by default | 09:25 |
| Chipaca | mvo: merged | 09:26 |
| zyga | morphis: right, I know | 09:26 |
| mup | PR snapd#6443 closed: daemon, polkit: pid_t is signed <Simple 😃> <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6443> | 09:27 |
| zyga | mvo: https://github.com/snapcore/snapd/pull/6442#pullrequestreview-197449157 | 09:28 |
| mup | PR #6442: tests: workaround missing go dependencies in debian-9 <Created by mvo5> <https://github.com/snapcore/snapd/pull/6442> | 09:28 |
| zyga | mvo: let's land it :) | 09:28 |
| pedronis | can somebody merge master into 6440 ? | 09:30 |
| mvo | zyga: will land in a wee bit | 09:30 |
| zyga | mmm | 09:30 |
| mvo | zyga: debina-9 is also merged now | 09:32 |
| mup | PR snapd#6442 closed: tests: workaround missing go dependencies in debian-9 <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6442> | 09:32 |
| mvo | having 6413 would be good, but this needs a second review | 09:32 |
| mvo | but I guess its fine, we can use salsa | 09:33 |
| pedronis | mvo: zyga: see #webops internally | 09:33 |
| mvo | zyga: looks like more people are affected, can we revert the quirks code? | 09:34 |
| zyga | I see no other way | 09:36 |
| mvo | zyga: iirc that was 6123 | 09:36 |
| zyga | but please let me spend a moment | 09:36 |
| zyga | I will write a quick test | 09:36 |
| zyga | perhaps we don't need all of quirks | 09:36 |
| zyga | I wonder why nobody tested candidate :/ | 09:36 |
| zyga | morphis: did you file a bug for this? | 09:36 |
| mvo | zyga: you write a test and I prepare the revert | 09:36 |
| zyga | mvo: no wait | 09:36 |
| zyga | mvo: I don't think we want the full blown revert | 09:37 |
| zyga | mvo: it's more complex because of other changes there | 09:37 |
| zyga | mvo: please give me two hours | 09:37 |
| morphis | zyga: no, just a forum topic | 09:37 |
| zyga | morphis: can you file a quick one, just so that we have a bug number | 09:37 |
| mvo | morphis: that should be fine | 09:37 |
| zyga | mvo: or rather, give me 15 minutes for quick assesment | 09:37 |
| mvo | morphis, zyga I can write it if we need it | 09:38 |
| mvo | zyga: ok | 09:38 |
| mup | PR snapd#6437 closed: snap/channel: improve channel parsing <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6437> | 09:38 |
| mvo | zyga: I think its fine to revert and least risky but if you justneed 15min thats fine :) | 09:38 |
| zyga | mvo: revert won't apply cleanly anymore | 09:38 |
| zyga | that's what I mean by more work | 09:38 |
| mvo | zyga: aha, ok | 09:38 |
| mvo | zyga: conflicts are trivial | 09:39 |
| mup | PR snapd#6444 opened: Revert "cmd/snap-confine,snap-update-ns: discard quirks" <⛔ Blocked> <Created by mvo5> <https://github.com/snapcore/snapd/pull/6444> | 09:41 |
| mvo | zyga: ocd or something but I let you investigate and we can talk in a wee bit | 09:41 |
| zyga | k | 09:41 |
| zyga | thanks | 09:42 |
| zyga | we can look at tests | 09:42 |
| mvo | zyga: yeah, given the urgency I want this as a plan B, feel free to poke and look for better solutions | 09:43 |
| zyga | k | 09:43 |
| zyga | make sense | 09:43 |
| mvo | zyga: having a proper regression test would be good too | 09:44 |
| mup | PR snapd#6445 opened: [RFC] snap-confine: revert "cmd/snap-confine,snap-update-ns: discard quirks" <Created by mvo5> <https://github.com/snapcore/snapd/pull/6445> | 09:47 |
| kissiel | mvo: hey! I finally managed to reproduce that crash; working on a fix | 09:56 |
| zyga | mvo: so | 10:00 |
| zyga | https://www.irccloud.com/pastebin/tx6qPpbI/ | 10:00 |
| zyga | mvo: this coupled with mkdir on /var/lib/jenkins in core / core18 fixes the issue without bringing back quirks | 10:00 |
| zyga | mvo: can we have a call about this? | 10:01 |
| pedronis | zyga: a call about what? | 10:02 |
| zyga | about the approach to take | 10:02 |
| zyga | I would really prefer not to bring back all of quirks | 10:02 |
| pedronis | zyga: a mkdir in core / core18 ? you mean we need to add that dir to the snaps? | 10:04 |
| zyga | yes | 10:04 |
| pedronis | mmh | 10:04 |
| mvo | kissiel: no worries, is it something on our side? | 10:04 |
| kissiel | mvo: leaning towards that, yes | 10:04 |
| mvo | kissiel: do you have more details? if it is something on our side we would like to fix it :) | 10:05 |
| kissiel | mvo: damn, sorry, misunderstood, more on our side :) | 10:05 |
| kissiel | i mean cert's | 10:05 |
| mvo | kissiel: haha, no worries | 10:05 |
| kissiel | so stay tuned | 10:05 |
| mvo | kissiel: thank you | 10:05 |
| mvo | zyga, pedronis the small size of this is compelling - the fact that we need to add the dir not so much :/ | 10:07 |
| zyga | mvo: adding the directory is cheap, adding that code restores cost for each snap startup | 10:07 |
| mvo | zyga: yeah, I can see that | 10:08 |
| mvo | zyga, pedronis we could add the dir until we have better !/home support? | 10:09 |
| pedronis | what is that dir visible to? | 10:09 |
| zyga | we need to roll out new core anyway, this would mean the change is smaller | 10:09 |
| zyga | pedronis: in 2.36 quirks made the full real host /var/lib visible | 10:09 |
| zyga | pedronis: in 2.37 only the /var/lib from base snap is visible | 10:09 |
| zyga | pedronis: apparmor prevents access as usual though this is fixed by local configuration of HOMEDIRS | 10:10 |
| pedronis | zyga: do we do something special about extrausers? | 10:11 |
| zyga | no | 10:11 |
| mborzecki | zyga: imo HOMEDIRS is not enough, we open each intermediate path and need r there | 10:11 |
| zyga | this is only affecting classic | 10:11 |
| zyga | and the patch I pasted only configures this for classic | 10:11 |
| zyga | mborzecki: I know, that was not what I meant | 10:11 |
| pedronis | 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 |
| mborzecki | ah ok | 10:12 |
| zyga | mborzecki: look at my patch above | 10:12 |
| pedronis | they owuld have worked in 2.36 if all var/lib was mounted | 10:12 |
| zyga | pedronis: no | 10:12 |
| zyga | pedronis: which interfaces are those? | 10:12 |
| ackk | 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 |
| popey | I didn't know you could parallel install local snaps | 10:12 |
| pedronis | zyga: account-control, classic-support, network-control | 10:13 |
| mborzecki | ackk: yes | 10:13 |
| zyga | pedronis: what I'm describing so far only affects core systems | 10:13 |
| zyga | er | 10:13 |
| mborzecki | popey: you can | 10:13 |
| zyga | classic systems | 10:13 |
| popey | nice | 10:13 |
| zyga | on core systems there are other things in /var/lib | 10:13 |
| pedronis | zyga: they all give access to bits of var/lib | 10:13 |
| pedronis | are they broken now? or not? | 10:13 |
| pedronis | I imagine they needs the bits from host | 10:13 |
| ackk | mborzecki, but if you're installing directly from the store you should just use mysnap_2 right? | 10:13 |
| pedronis | are they using mount support properly? | 10:13 |
| mborzecki | ackk: yes | 10:14 |
| zyga | pedronis: looking at master | 10:14 |
| zyga | pedronis: excluding /var/lib/snapd/* | 10:14 |
| pedronis | yes | 10:14 |
| zyga | pedronis: and excluding /var/lib/extrausers | 10:14 |
| pedronis | because? | 10:15 |
| zyga | I see the following | 10:15 |
| pedronis | anyway, I get this: https://pastebin.ubuntu.com/p/fY9gcjw2bJ/ | 10:15 |
| zyga | (because those have bind mounts) | 10:15 |
| zyga | https://www.irccloud.com/pastebin/lH6YnuB2/ | 10:15 |
| zyga | I would have to check each one to see if they are for core or classic, hold on | 10:16 |
| mborzecki | 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 |
| zyga | pedronis: alsa is affected | 10:16 |
| pedronis | zyga: see | 10:16 |
| pedronis | mvo: so we have a bit of a bigger problem | 10:16 |
| zyga | I think there's more though | 10:17 |
| zyga | the quirks code only ran if you had /var/lib/lxd AFAIR | 10:17 |
| zyga | so no lxd, no quirks | 10:17 |
| zyga | let me double check | 10:17 |
| zyga | pedronis: we could bring the code for each of those (alsa, dhcp) to the appropriate interface | 10:17 |
| zyga | if they really affect classic | 10:17 |
| Son_Goku | Chipaca: pong | 10:17 |
| pedronis | yes, just saying that 2.37 might be broken more than just related to jenkins | 10:18 |
| zyga | pedronis: quirk code ran unconditionally on core (but not core18) and only on classic systems | 10:18 |
| Chipaca | Son_Goku: ok to use snappy in that context | 10:19 |
| Son_Goku | cool | 10:19 |
| Son_Goku | I'm going to change it then | 10:19 |
| pedronis | zyga: still, some snaps might be broken now | 10:19 |
| pedronis | apparently | 10:19 |
| mvo | pedronis, zyga if thats the case the quirks revert seems to be the best option, yes? | 10:20 |
| zyga | mvo: I disagree, I think bringing back quirks is the wrong move | 10:20 |
| pedronis | mvo: would not say best, it's the quickest | 10:20 |
| zyga | 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:21 |
| zyga | apparently none of those are tested | 10:22 |
| pedronis | of course | 10:22 |
| zyga | I'd like to remind us that quirks were done in response to a bug in a similar situation (release broke lxd) | 10:22 |
| pedronis | we don't have complete tests for all permissions for any interface afaik | 10:22 |
| zyga | we didn't anticipate it would open more things than it did | 10:22 |
| zyga | bringing that back would not only fix those, but allow us to make the same mistake, silently, again | 10:22 |
| pedronis | zyga: I understand, but breaking things is not good | 10:23 |
| zyga | pedronis: I agree on that | 10:23 |
| pedronis | zyga: how long would it take to do the correct fix for all relevant things? | 10:23 |
| pedronis | that's a bit the question | 10:23 |
| zyga | it's just planning what to do to fix the issue | 10:23 |
| zyga | adding each of those to the approrpiate interface is easy, just one line in each of the interfaces mount profiles | 10:23 |
| pedronis | in away that don't risk perturbing something else | 10:23 |
| pedronis | zyga: do we need to add them to core* as well? | 10:24 |
| pedronis | no, because this are normal mounts? | 10:24 |
| pedronis | I mean they would go through the full mount mechanism? | 10:24 |
| zyga | pedronis: alsa and usbutils, yes, dhcp and systemd, no | 10:25 |
| pedronis | anyway we would need tests for all of them | 10:25 |
| pedronis | given we are at this conjuncture | 10:25 |
| zyga | tests for functionality would be complex but a test that a snap can see host's files is easy to add (we have some of those) | 10:26 |
| zyga | (see or write to, as appropriate) | 10:26 |
| zyga | the code in snap-update-ns that can poke writable holes has one property | 10:26 |
| zyga | it works great on top of read-only tree | 10:26 |
| zyga | it should not be used on top of /var/lib where some things are writable | 10:27 |
| zyga | IMO it's safer to just make the directories required by those interfaces explicitly | 10:27 |
| zyga | it is safer because the writable-mimic system takes a snapshot of the directory | 10:27 |
| zyga | this way we avoid that entirely | 10:28 |
| zyga | in addition, quirks are not represented | 10:28 |
| zyga | in the mount profiles | 10:28 |
| zyga | they don't participate in the diff calculation | 10:28 |
| zyga | mainly in fact because of lack of ability to represent the operations performed (move of /var/lib/snapd) | 10:29 |
| zyga | so | 10:29 |
| zyga | what shall we do? | 10:29 |
| zyga | I worry if 2.37 broke any core devices | 10:30 |
| zyga | with network-manager | 10:30 |
| zyga | but we got no reports of this during testing cycle | 10:30 |
| pedronis | zyga: have a HO | 10:32 |
| mvo | +1 for HO | 10:33 |
| zyga | I'm there | 10:33 |
| zyga | hold on :) | 10:34 |
| zyga | actually, we're not in that much of hot water | 10:35 |
| zyga | quriks never brought random directories from the host, they made /var/lib writable | 10:35 |
| zyga | and brought /var/lib/lxd | 10:35 |
| zyga | so all the other interfaces accessing stuff | 10:35 |
| zyga | that was never there | 10:35 |
| zyga | so that's much easier to reason about :) | 10:35 |
| Chipaca | jamesh: you're not around by chance are you? | 10:55 |
| xnox | so my lxd is back to bad | 10:59 |
| xnox | yesterday "apt install --reinstall snapd" fixed things, but i guess I simply got rolled back to older snapd. | 10:59 |
| xnox | / core snap. | 10:59 |
| xnox | today i have this: | 10:59 |
| xnox | https://paste.ubuntu.com/p/hwBMf2Rwwp/ | 10:59 |
| mvo | zyga: I did a codesearch for adduser /var/lib - there are a bunch of things that also add a system user there - so if any of those is using (classic) snaps we are in trouble, right? the puppet user is one that might affect us | 11:01 |
| zyga | mvo: interesting | 11:02 |
| zyga | mvo: if it's a small list we can handle them the same way | 11:02 |
| zyga | mvo: if it's not a small ist | 11:02 |
| mvo | zyga: there are tons more but I'm not sure how relevant they are | 11:02 |
| zyga | right :/ | 11:02 |
| mvo | zyga: I mean, mariadb, spamd, sendmail, rabbitmq | 11:02 |
| zyga | I would say we should fix jenkins and perhaps some high-profile ones only | 11:02 |
| pedronis | mvo: zyga: so my forum troubles seems related to the vpn, it works if I'm out of the vpn, doesn't if I'm on the vpn. didn't change vpn config recently tough | 11:02 |
| mvo | bacula | 11:02 |
| zyga | mvo: it is unlikely rabbit would need snap apps | 11:03 |
| mvo | pacemaker etc | 11:03 |
| zyga | mvo: the key question is: is this user going to run snap apps | 11:03 |
| mvo | zyga: yeah, thats what I mean, I don't know if any of these are relevant | 11:03 |
| mvo | zyga: but potentially its quite a few | 11:03 |
| mvo | tomcat | 11:03 |
| zyga | mmm | 11:03 |
| mvo | nagios | 11:04 |
| mvo | icinga | 11:04 |
| mvo | sbuild :P | 11:04 |
| sergiusens | ondra: for the time being you might have luck with setting the version to 8 | 11:04 |
| ondra | sergiusens I'm building for core16, so that would do for me for now | 11:05 |
| zyga | mvo: https://github.com/snapcore/snapd/pull/6446 | 11:06 |
| zyga | mvo: I'll work on a regression test now | 11:07 |
| mup | PR #6446: cmd/snap-confine: add special case for Jenkins <Created by zyga> <https://github.com/snapcore/snapd/pull/6446> | 11:07 |
| mup | PR snapd#6446 opened: cmd/snap-confine: add special case for Jenkins <Created by zyga> <https://github.com/snapcore/snapd/pull/6446> | 11:07 |
| sergiusens | zyga: you say"it is not a supported feature", do you have a compiled list of what ARE supported features? | 11:09 |
| zyga | sergiusens: yes, only /home/* is supported officially | 11:09 |
| zyga | anything else is not | 11:10 |
| sergiusens | I asked if you have a compiled list | 11:10 |
| zyga | sergiusens: the list is: /home/* | 11:10 |
| sergiusens | what are the limitations of classic confinement? | 11:10 |
| sergiusens | where is this documented? | 11:10 |
| zyga | I don't understand the question sorry, | 11:10 |
| zyga | limitations of classic confinement? | 11:10 |
| zyga | sergiusens: classic confinment is subject to the same limitations that snapd has for users as for non-classic snaps | 11:11 |
| zyga | I don't believe we have a page listing limitations in snapd | 11:12 |
| zyga | perhaps we should have one | 11:12 |
| sergiusens | zyga: when you say something is supported or not, it would be ideal to know what is and what isn't without needing to be on IRC at the right time | 11:13 |
| zyga | I don't disagree | 11:13 |
| zyga | it's just hard to have a comprehensive list that is correct | 11:13 |
| zyga | we learn the hard way about some of those | 11:13 |
| zyga | https://forum.snapcraft.io/t/limitations-in-snapd/9718 | 11:14 |
| zyga | it's a wiki, please feel free to expand this | 11:15 |
| zyga | degville: https://forum.snapcraft.io/t/limitations-in-snapd/9718 FYI | 11:16 |
| degville | thanks zyga! | 11:17 |
| zyga | I'm sure it is not a complete list | 11:17 |
| zyga | mvo: how does this look like? | 11:21 |
| zyga | https://www.irccloud.com/pastebin/wSR2jWvJ/ | 11:21 |
| zyga | just a quick smoke test | 11:21 |
| zyga | I've hand-tested this on my patched system (I have not tried the real apt install yet) | 11:21 |
| mvo | zyga: looks good | 11:21 |
| zyga | cool, I'll propose this separately | 11:21 |
| mvo | zyga: we have a test-snapd-sh | 11:22 |
| zyga | oh, indeed | 11:22 |
| zyga | sorry, just local testing memory :) | 11:22 |
| mvo | zyga: that you can probably use, use "install_local" for it | 11:22 |
| mvo | zyga: no worries | 11:22 |
| zyga | will do | 11:22 |
| mup | PR core#100 opened: hooks: add /var/lib/jenkins for compat <Created by mvo5> <https://github.com/snapcore/core/pull/100> | 11:23 |
| zyga | I mistyped "jenkins" as "jenkinks" way too many times today | 11:25 |
| mup | PR core18#115 opened: hooks: add /var/lib/jenkins for compat <Created by mvo5> <https://github.com/snapcore/core18/pull/115> | 11:26 |
| mup | PR snapd#6447 opened: polkit: cast pid to uint32 to keep polkit happy for now <Created by chipaca> <https://github.com/snapcore/snapd/pull/6447> | 11:27 |
| mvo | zyga: one more complication - core18 will need a stable release (which we have not scheduled) by foundations for the jenkins fix to work | 11:32 |
| zyga | mvo: pushed a test | 11:34 |
| zyga | mvo: no :) | 11:34 |
| zyga | mvo: core 18 never had quirks | 11:35 |
| mup | PR snapd#6440 closed: cmd/snap: fix typo in cmd_wait.go <Simple 😃> <Created by sparkiegeek> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6440> | 11:35 |
| zyga | mvo: one less thing to worry about :) | 11:35 |
| zyga | we can just make core18 work on separate timeline (not a regression!) | 11:35 |
| sparkiegeek | yay | 11:35 |
| zyga | mvo: right? | 11:35 |
| mvo | zyga: aha, I like that | 11:38 |
| zyga | mvo: I took your suggestion | 11:38 |
| zyga | we could add more users this way | 11:38 |
| mvo | zyga: so we can remove the core18 PR? | 11:38 |
| zyga | and especially once we can drop the magic patch | 11:38 |
| zyga | I would keep it | 11:38 |
| zyga | but not release on this schedule | 11:39 |
| zyga | right? | 11:39 |
| mvo | zyga: its YAGNI to me but I don't care enough to argue hard | 11:39 |
| zyga | it means people can use core18 based snaps | 11:39 |
| zyga | where before they could not | 11:39 |
| mvo | zyga: aha, nice | 11:39 |
| mvo | zyga: ok, sorry, I misunderstood, I will update the core18 description | 11:39 |
| zyga | it's not YAGNI in the sense that we can get this bug back as soon as one of the snaps migrate to core18 for whatever reason | 11:39 |
| zyga | (layouts carrot) | 11:39 |
| pstolowski | mborzecki: can you take another look at https://github.com/snapcore/snapd/pull/6394? i think Sergio addressed your comment and we could land it | 11:40 |
| mup | PR #6394: tests: iterate getting journal logs to support delay on boards on daemon-notify test <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6394> | 11:40 |
| mvo | zyga: no, sorry, core18#115 is fine and no YAGNI. I thought we were talking about my suggestion in the task.yaml in the jenkins PR | 11:40 |
| mup | PR core18#115: hooks: add /var/lib/jenkins for compat <Created by mvo5> <https://github.com/snapcore/core18/pull/115> | 11:40 |
| zyga | ah, I see | 11:40 |
| zyga | nb, your suggestion was good too :) | 11:40 |
| pstolowski | mborzecki: great, thanks! | 11:41 |
| mvo | zyga: I updated the core18 description - good to know that we can push that at a different pace | 11:41 |
| zyga | yes, one less craze today | 11:41 |
| mvo | sil2100: could you please have a look at https://github.com/snapcore/core/pull/100 | 11:42 |
| mup | PR snapd#6394 closed: tests: iterate getting journal logs to support delay on boards on daemon-notify test <Created by sergiocazzolato> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6394> | 11:42 |
| mup | PR core#100: hooks: add /var/lib/jenkins for compat <Created by mvo5> <https://github.com/snapcore/core/pull/100> | 11:42 |
| sil2100 | mvo: sure o/ | 11:47 |
| sil2100 | mvo: ok, both approved, sadly I don't have write access to the core repo | 11:51 |
| sil2100 | As I guess that's still under the snappy team, right? | 11:51 |
| mborzecki | zyga: E: Package 'jenkins' has no installation candidate ? | 11:52 |
| sil2100 | (we have core18 and core16, right?) | 11:52 |
| mup | PR core18#115 closed: hooks: add /var/lib/jenkins for compat <Created by mvo5> <Merged by sil2100> <https://github.com/snapcore/core18/pull/115> | 11:52 |
| zyga | mborzecki: drat :) | 11:54 |
| mborzecki | zyga: tried it on 18.04, so maybe it actually works on 16.04 | 11:54 |
| zyga | https://packages.ubuntu.com/search?keywords=jenkins&searchon=names&suite=cosmic§ion=all | 11:54 |
| zyga | where's wally? | 11:55 |
| mup | PR snapd#6448 opened: snapcraft.yaml: fix XBuildDeb PATH for go-1.10 <Created by mvo5> <https://github.com/snapcore/snapd/pull/6448> | 11:55 |
| sparkiegeek | http://pkg.jenkins-ci.org/debian/ | 11:55 |
| mup | PR core#100 closed: hooks: add /var/lib/jenkins for compat <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/core/pull/100> | 11:55 |
| zyga | sparkiegeek: is that *the* way to install jenkins? | 11:56 |
| zyga | I mean, is that what we do internally? | 11:56 |
| zyga | mvo: now to trigger core build | 11:58 |
| mvo | zyga: yeah, its running | 12:03 |
| zyga | mvo: can you please look at https://github.com/snapcore/snapd/pull/6446 again | 12:03 |
| mup | PR #6446: cmd/snap-confine: add special case for Jenkins <⚠ Critical> <Created by zyga> <https://github.com/snapcore/snapd/pull/6446> | 12:03 |
| zyga | sorry, the jenkins package is not in the archive :/ | 12:03 |
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/6441#pullrequestreview-197518397 | 12:09 |
| mup | PR #6441: overlord/snapstate: validate instance names early <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6441> | 12:09 |
| mborzecki | zyga: i suppose it's ok to add the external repo given that tis is the only source | 12:09 |
| zyga | yeah | 12:09 |
| sparkiegeek | zyga: hmm, can't speak for every team, but that's what some teams use, yes | 12:17 |
| zyga | sparkiegeek: thank you, that just makes our tests more relevant | 12:17 |
| zyga | I can now take a break and have breakfast, I guess | 12:19 |
| mvo | zyga: sounds like an excellent plan | 12:20 |
| mvo | zyga: thank you | 12:20 |
| * ogra is surprised people dont seem to use a jenkins snap | 12:21 | |
| ogra | (there is one maintained by snapcrafters ... even seems reasonably up to date) | 12:22 |
| pedronis | mborzecki: 6016 can land now, right? | 12:22 |
| mborzecki | pedronis: yup, let me merge it | 12:23 |
| mup | PR snapd#6016 closed: snap/naming: move various name validation helpers to separate package <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6016> | 12:23 |
| popey | ogra: it's not classic, so it's a bit inert, I have requested classic for it | 12:32 |
| zyga | re | 12:32 |
| ogra | popey, +1 | 12:32 |
| popey | Is there some way to query the store to find out which snaps require core18? | 12:36 |
| zyga | https://github.com/snapcore/snapd/pull/6413 needs a 2nd review | 12:38 |
| mup | PR #6413: packaging: import debian salsa packaging work, add sbuild test and use in spead <Created by mvo5> <https://github.com/snapcore/snapd/pull/6413> | 12:38 |
| zyga | popey: not that I know of | 12:38 |
| popey | Ok, I'll ask store people :) | 12:38 |
| zyga | https://github.com/snapcore/snapd/pull/6426 needs a 2nd review | 12:43 |
| mup | PR #6426: cmd/snap-confine: refactor and cleanup of seccomp loading <Created by zyga> <https://github.com/snapcore/snapd/pull/6426> | 12:43 |
| zyga | mvo: do we need 2.37 PRs for 2.37.1 material or do you plan to do a master-based release? | 12:47 |
| mup | PR snapd#6447 closed: polkit: cast pid to uint32 to keep polkit happy for now <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6447> | 12:49 |
| mborzecki | mvo: the spread test in #6252 needs tweaking, because we're actually using snapFromSender() now, it's looking wehther the calling pid is part of snap. cgroup | 12:57 |
| mup | PR #6252: userd: handle help urls which requires prepending XDG_DATA_DIRS <Created by kenvandine> <https://github.com/snapcore/snapd/pull/6252> | 12:57 |
| Chipaca | pedronis: did you see my note on the #6280 ? | 12:58 |
| mup | PR #6280: cmd/snap: make 'snap warnings' output yamlish <Created by chipaca> <https://github.com/snapcore/snapd/pull/6280> | 12:58 |
| zyga | mvo: FIY https://tracker.debian.org/pkg/snapd - 2.37 has now migrated to testing | 12:59 |
| mborzecki | off to pick up the kids | 13:03 |
| diddledan | popey https://www.irccloud.com/pastebin/O8qqJ1D3/ | 13:07 |
| pedronis | Chipaca: mmh, so the output there is yaml parsable but is also localized? | 13:08 |
| pedronis | strange combination | 13:08 |
| Chipaca | pedronis: … yeah i guess it is | 13:08 |
| Chipaca | it doesn't have to be, i was just rotating the original list | 13:08 |
| * diddledan pokenprod the pope! popey... | 13:08 | |
| pedronis | Chipaca: do we do that anywhere else? | 13:09 |
| diddledan | has he gone? | 13:09 |
| pedronis | I mean that = this combo | 13:09 |
| Chipaca | pedronis: info is the only other yaml-ish, and that's not i18n'ed | 13:09 |
| diddledan | I think that's a flawed query actually. it only returns 2000 results | 13:12 |
| pedronis | Chipaca: bit of a mess | 13:12 |
| pedronis | Chipaca: seems we need --localized and not just --unicode | 13:14 |
| Chipaca | ? | 13:14 |
| Chipaca | localisation already has a standard way of turning it off across the board | 13:14 |
| pedronis | Chipaca: it makes sense to localize it, but not if it goes to a tty and program wants to parse it | 13:14 |
| pedronis | not-tty | 13:14 |
| Chipaca | zyga: question about 6446 (which might already have been discussed here but i missed it) | 13:16 |
| Chipaca | zyga: why not add /var/lib/jenkins to homes? | 13:16 |
| zyga | homes? | 13:16 |
| zyga | to HOMEDIRS? | 13:16 |
| Chipaca | HOMEDRIS | 13:17 |
| Chipaca | yes | 13:17 |
| Chipaca | or no? | 13:17 |
| Chipaca | /etc/apparmor.d/tunables/home:@{HOME}=@{HOMEDIRS}/*/ /root/ | 13:17 |
| Chipaca | or HOMEDIRS | 13:18 |
| Chipaca | either, probably | 13:18 |
| zyga | I don't know if we can do that from our package sanely | 13:18 |
| Chipaca | I dunno enough to know :-) | 13:18 |
| zyga | I think this is a question to jdstrand | 13:18 |
| Chipaca | zyga: yes we can | 13:18 |
| Chipaca | zyga: look at /etc/apparmor.d/tunables/home.d/ | 13:18 |
| zyga | yes but that has more implications (beyond snpd) | 13:19 |
| zyga | snapd | 13:19 |
| zyga | I think we could but I didn't think about doing this | 13:19 |
| Chipaca | zyga: if we did it, would it work? | 13:19 |
| zyga | it would still require the remaining patches | 13:19 |
| zyga | but I don't see why not | 13:19 |
| Chipaca | right, I'm not liking the duplication we're adding for this | 13:20 |
| Chipaca | whereas if it's adding it to homes or homedirs, it's less duplication imho | 13:20 |
| Chipaca | if it worked, and if jdstrand is ok with it | 13:20 |
| zyga | where would you add that? | 13:21 |
| zyga | in /etc/apparmor.d/ | 13:21 |
| === ricab is now known as ricab|lunch | ||
| zyga | adding files to /etc/ is problematic | 13:21 |
| Chipaca | zyga: why? | 13:22 |
| zyga | it cannot be easily changed | 13:22 |
| Chipaca | AIUI it'd be in /etc/apparmor.d/tunables/home.d/ | 13:22 |
| zyga | we cannot handle the reexec case then | 13:22 |
| zyga | it becomes a conffile that lives beyond the lifetime of the package | 13:22 |
| zyga | all smell in my opinion | 13:22 |
| zyga | we have bugs that keep us holding onto specific names in /etc because of apt bugs | 13:23 |
| pedronis | also why would we do this | 13:23 |
| pedronis | and not jenkins | 13:23 |
| zyga | yeah | 13:23 |
| zyga | good point | 13:23 |
| zyga | jenkins is free to do this (not that they are aware it is useful) | 13:23 |
| pedronis | Chipaca: seems the most simple thing would be not to localize those strings in 6280, not great tough | 13:25 |
| mvo | zyga: I will cherry pick all the easy ones into 2.37 | 13:43 |
| mvo | zyga: only if there are conflicts I won't | 13:43 |
| mvo | zyga: slightly sad that 6446 is not finished (spread) | 13:43 |
| mvo | zyga: jenkins postinst fails in the spread test | 13:45 |
| mvo | zyga: ERROR: no java executeable found or something | 13:46 |
| pedronis | heh | 13:47 |
| jdstrand | zyga and Chipaca: I just commented on something similar in https://github.com/snapcore/snapd/pull/6446 | 13:48 |
| mup | PR #6446: cmd/snap-confine: add special case for Jenkins <⚠ Critical> <Created by zyga> <https://github.com/snapcore/snapd/pull/6446> | 13:48 |
| jdstrand | zyga and Chipaca: the thing is, zyga is right, HOMEDIRS only makes sense when it is something that really is home directories for actual users. eg, nfs users, etc. if we added that for every snap in the manner Chipaca described, sure it would work for snap-confine, but it would also affect non-snap policy as well as snap policy that uses @{HOME} | 13:51 |
| Chipaca | jdstrand: to be clear this was just for jenkins, which isn't a "regular" user but it isn't a snap either | 13:52 |
| jdstrand | zyga and Chipaca: but we have the beginnings of a similar system with /var/lib/snapd/apparmor/snap-confine | 13:52 |
| mvo | zyga: if you don't mind I will split 6446 into the first commit and the test and push the test as a second PR, this way we can get the fix into master and 2.37 while iterating on the test. sounds sensible? | 13:52 |
| jdstrand | Chipaca: I know. I read all backscroll and the PR | 13:52 |
| Chipaca | jdstrand: you're a better person than I am | 13:52 |
| zyga | AFK | 13:53 |
| jdstrand | Chipaca: but I wouldn't want HOMEDIRS updated on my system to allow /var/lib/jenkins since that is a surprising side affect on the other policy on my system. snap-confine should have that, not global policy | 13:53 |
| jdstrand | Chipaca: hehe :) I'm not sure how you came to that conclusion! ;) | 13:54 |
| Chipaca | jdstrand: by gathering too few facts and jumping to conclusions, like I always do | 13:54 |
| Chipaca | which … is kinda the point | 13:54 |
| jdstrand | heh, I knew what you meant but was disagreeing with the conclusion :) | 13:55 |
| mup | PR snapd#6449 opened: cmd/snap-confine: add special case for Jenkins <Created by mvo5> <https://github.com/snapcore/snapd/pull/6449> | 13:56 |
| mup | PR snapd#6106 closed: overlord/ifacestate: handler for hotplug-update-slot tasks <Hotplug 🔌> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6106> | 13:59 |
| mup | PR snapd#6441 closed: overlord/snapstate: validate instance names early <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6441> | 14:01 |
| mup | PR snapd#6445 closed: [RFC] snap-confine: revert "cmd/snap-confine,snap-update-ns: discard quirks" <⛔ Blocked> <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/6445> | 14:06 |
| mup | PR snapd#6444 closed: [RFC] snap-confine: revert "cmd/snap-confine,snap-update-ns: discard quirks" (2.37) <⛔ Blocked> <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/6444> | 14:07 |
| zyga | hey jdstrand | 14:09 |
| zyga | (standup) | 14:10 |
| zyga | mvo: thank you, I will fix the java thing shortly | 14:10 |
| mvo | zyga: thank you | 14:10 |
| mvo | zyga: and welcome back :) | 14:10 |
| zyga | jdstrand: about home directories, I have some ideas myself, I would like to detach ideas from the current fire (unless the idea is feasible as a solution right now) | 14:15 |
| zyga | Chipaca: unsigned is a sign of sign issue | 14:15 |
| zyga | jdstrand: as one item I wanted to avoid depending on the environment | 14:16 |
| zyga | jdstrand: we have too many things conveyed this way | 14:16 |
| === chrisccoulson_ is now known as chrisccoulson | ||
| diddledan | popey: https://www.irccloud.com/pastebin/fcCcNH8d/ | 14:26 |
| morphis | mvo: there seems to be another thing which "broke" with 2.37: https://bugs.launchpad.net/charm-etcd/+bug/1813678 | 14:28 |
| mup | Bug #1813678: Snap install error: snap "etcd" is not compatible with --classic <field-critical> <Etcd Charm:New> <https://launchpad.net/bugs/1813678> | 14:28 |
| mup | PR snapd#6280 closed: cmd/snap: make 'snap warnings' output yamlish <⛔ Blocked> <Created by chipaca> <Closed by chipaca> <https://github.com/snapcore/snapd/pull/6280> | 14:28 |
| === ricab|lunch is now known as ricab | ||
| diddledan | popey: if you want to replicate my test, I've put the source online at https://github.com/diddlesnaps/snap-bases (many thanks to the uappexplorer folks who I stole the majority of the api code from) | 14:37 |
| morphis | mvo: the k8s folks are updating the charm but it actually broke existing setups, not sure if you heard about this one yet | 14:42 |
| zyga | jdstrand: https://github.com/snapcore/snapd/pull/6446 updated with the comment you asked for | 14:45 |
| mup | PR #6446: cmd/snap-confine: add special case for Jenkins <⚠ Critical> <Created by zyga> <https://github.com/snapcore/snapd/pull/6446> | 14:45 |
| zyga | morphis, mvo: looking at etcd thing | 14:46 |
| zyga | ah | 14:46 |
| zyga | I think this is another of the "feature not a bug" case :/ | 14:46 |
| niemeyer | https://twitter.com/gniemeyer/status/1090260317416841217 | 14:49 |
| niemeyer | Snap thread | 14:49 |
| morphis | zyga: yeah, it's not critical and gets fixed from the charm but revealed that the etcd snap was still installed with --classic after it got strict confined a year ago | 14:50 |
| zyga | mmm | 14:50 |
| zyga | yeah | 14:50 |
| morphis | zyga: just wanted that you guys are aware of this as I am not sure if it will cause issues elsewhere too | 14:50 |
| zyga | thank you | 14:50 |
| mborzecki | Chipaca: https://github.com/snapcore/snapd/pull/6252/commits/c7d97e0cacc2b40a8d51cd46f5668ff05f403c31 do you recall if there was some trouble with cla checker at the time? | 14:52 |
| mup | PR #6252: userd: handle help urls which requires prepending XDG_DATA_DIRS <Created by kenvandine> <https://github.com/snapcore/snapd/pull/6252> | 14:52 |
| Chipaca | mborzecki: what do you mean? | 14:54 |
| Chipaca | mborzecki: you trying to remember why that's set there? | 14:54 |
| mborzecki | Chipaca: yeah | 14:54 |
| Chipaca | mborzecki: yeah i expect it broke at some point | 14:55 |
| Chipaca | it needs the whole history to work properly | 14:55 |
| Chipaca | was this PR's kenvandine's? | 14:55 |
| mborzecki | Chipaca: yes | 14:55 |
| Chipaca | 's? | 14:55 |
| Chipaca | yeah | 14:55 |
| Chipaca | he'd signed it with a weird account, that just happened to be in the tree already | 14:55 |
| Chipaca | you can see it in the output, probably | 14:56 |
| Chipaca | yeah | 14:56 |
| mborzecki | Chipaca: briefly thought about reverting it, zyga asked for it, but if the cla checker is going to stop me then i'd leave it :P | 14:56 |
| Chipaca | one of those is not like the other | 14:56 |
| Chipaca | yep | 14:56 |
| Chipaca | it _should_ work, dunno why it didn't | 14:57 |
| Chipaca | we can try :-) | 14:57 |
| Chipaca | mborzecki: zyga: https://travis-ci.org/snapcore/snapd/jobs/461903781 | 14:58 |
| Chipaca | it's because kenvandine hasn't signed the cla, wouldn't you know | 14:58 |
| zyga | eh | 14:58 |
| zyga | can we special case in the code :-) ? | 14:58 |
| mborzecki | haha | 14:58 |
| Chipaca | and launchpad doesn't let me ask "does this account have any other emails @canonical.com" | 14:59 |
| Chipaca | at least AFAIK | 14:59 |
| mborzecki | hm, kenvandine could reset author if the 'offending' commit | 14:59 |
| Chipaca | yes, yes he could | 14:59 |
| Chipaca | but having the whole history is the right thing to do | 15:00 |
| Chipaca | so, eh | 15:00 |
| kenvandine | I'm afk until Thursday | 15:00 |
| kenvandine | :-) | 15:00 |
| kenvandine | What was wrong with the author line? | 15:00 |
| diddledan | if Ken signed the CLA and his employer is Canonical, does that mean that Canonical are assigning their copyright to Canonical? that sounds like a deep rabbit hole ;-p | 15:00 |
| mborzecki | kenvandine: hehe nothing about you without you :) | 15:00 |
| Chipaca | kenvandine: TINC! HAND. | 15:01 |
| kenvandine | Time for airplane mode! | 15:01 |
| mborzecki | Chipaca: kenvandine: i'll push the changes to the spread test so that it'll be green and then i'll request changes to get it sorted out | 15:02 |
| ackk | hi, if you have multiple services in a snap, can you start/stop/enable/disable them individually via snapctl inside a hook? | 15:08 |
| zyga | ackk: I think you can now | 15:09 |
| ackk | zyga, is it via "${SNAP_INSTANCE_NAME}.$service" ? | 15:09 |
| ogra | jdstrand, seeing you adding all these interface docs ... i think it might be helful having a filed "causes manual review on upload" or some such, so people are aware before using an interface in their snapcraft.yaml | 15:09 |
| zyga | ackk: I don't recall, sorry, let me check if this is documented | 15:10 |
| zyga | hmm | 15:11 |
| zyga | perhaps this is not allowed | 15:11 |
| zyga | pstolowski, mborzecki: looking at https://github.com/snapcore/snapd/pull/5777 - do you know if this got resurrected? | 15:11 |
| mup | PR #5777: wrappers/services.go: don't start disabled services <Created by anonymouse64> <Closed by anonymouse64> <https://github.com/snapcore/snapd/pull/5777> | 15:11 |
| jdstrand | zyga: well, that's why I said for the future. however, I think it is useful to have a vague understanding of the future so we don't do something now that would conflict with it | 15:12 |
| jdstrand | zyga: but we've done that | 15:12 |
| zyga | jdstrand: I have some ideas if you'd like to know | 15:12 |
| zyga | I can share those quickly | 15:12 |
| mborzecki | zyga: 5777 went in as #5979 | 15:13 |
| mup | PR #5979: install: don't start disabled services <Squash-merge> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/5979> | 15:13 |
| zyga | mborzecki: thank you, super | 15:13 |
| jdstrand | ogra: as for updating the interfaces with needs manual connection, that is more of a degville thing. I'm just following established prcedure. it might be nice though | 15:14 |
| jdstrand | zyga: maybe for another time? we should chat about it though | 15:14 |
| ogra | jdstrand, well, you already have "Auto-Connect: true/false" ... the above was more about ... perhaps i shouldnt bother, since using this interface will get me into manual review anyway ... | 15:15 |
| ogra | i.e. more about the store than about the later installation of the snap | 15:15 |
| degville | ogra / jdstrand: I can add a note about triggering a manual review to the interface overview. | 15:16 |
| ogra | that would be a great addition i think | 15:16 |
| ogra | saving some support questions in the forum | 15:16 |
| degville | cool, I'll do that now. | 15:17 |
| mborzecki | ackk: yes snapctl <start|stop|restart> $SNAP_INSTANCE_NAME.$service | 15:17 |
| ackk | mborzecki, I see. kinda related, we currently run "snapctl start --enable $SNAP_INSTANCE_NAME" + "snapctl restart --reload $SNAP_INSTANCE_NAME" in the config hook to ensure the service is enabled and restarted if already enable (after config change). is there a way to do this in a single shot? afaict that takes a long time as it restarts the service twice | 15:19 |
| mborzecki | ackk: no, i don't think so, you're looking for plain `systemctl enable` if i understand correctly? | 15:22 |
| ackk | mborzecki, no I want to enable it but if it's already enabled and running I want to restart | 15:22 |
| ackk | (as the config might have been changed) | 15:22 |
| mborzecki | ackk: ok, so systemct is-enabled ... && systemctl restart then? | 15:23 |
| ackk | mborzecki, yeah something like that, but I guess you can't do it with just snapctl, right? | 15:23 |
| jdstrand | degville: there are only a few that trigger a manual review, and we do have text for those (cc ogra) in the actual interface page | 15:24 |
| mborzeck1 | ackk: heh, connection problems, idk if you got my last message, imo you'd need to parse snapctl services output | 15:25 |
| jdstrand | eg, https://forum.snapcraft.io/t/the-block-devices-interface/9721 has "Consumers of this interface require a snap declaration for distribution via the Snap Store." | 15:25 |
| ogra | yeah, its only 3 or 4 ... but i still think it would be helpful having that in the doc directly | 15:25 |
| jdstrand | but https://forum.snapcraft.io/t/the-u2f-devices-interface/9722 does not since it is manually connected (which is expressed in the large table) | 15:26 |
| ogra | now thats very ... uhm ... whats a snap declaration ... means i need to read up about that first | 15:26 |
| jdstrand | ogra: that snap declaration is a link | 15:26 |
| jdstrand | to https://forum.snapcraft.io/t/process-for-aliases-auto-connections-and-tracks/455/ | 15:26 |
| ogra | feels like a text adventure :) | 15:27 |
| jdstrand | well, people shouldn't be using these typically ;) | 15:27 |
| zyga | jdstrand: ack, another time is fine | 15:27 |
| zyga | I feel so tired today, I'll take a nap | 15:28 |
| degville | jdstrand / ogra: thanks. I'll tread lightly so that no one'e eaten by a grue, but add a note to look out for these specific cases. | 15:28 |
| ackk | mborzeck1, ah I see yeah that might work, thanks | 15:42 |
| pedronis | pstolowski: question in 6322, sorry I didn't ask it before | 15:58 |
| pstolowski | looking | 15:59 |
| rharper | does snapd exec udevadm trigger ? if so when? does it ever use 'udevadm settle' ? | 16:02 |
| pedronis | mborzecki: some comments in 6387 | 16:05 |
| zyga | rharper: yes it does | 16:06 |
| pedronis | Chipaca: #6415 needs a re-review | 16:12 |
| mup | PR #6415: snapstate, snap: allow update/switch requests with risk only channel to DTRT <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6415> | 16:12 |
| pstolowski | pedronis: replied | 16:12 |
| Chipaca | rharper: https://github.com/snapcore/snapd/blob/master/interfaces/udev/udev.go#L104 | 16:14 |
| pedronis | pstolowski: re-replied :) | 16:17 |
| rharper | Chipaca: ok; are you aware of any snaps using subsystem=block ? | 16:18 |
| Chipaca | rharper: how would a snap do that? | 16:18 |
| Chipaca | maybe it's a question for pstolowski :-) | 16:19 |
| rharper | I'm not sure, there is this call, https://github.com/snapcore/snapd/blob/master/interfaces/udev/udev.go#L79 | 16:19 |
| pstolowski | pedronis: sure, will do, fair point, ty! | 16:19 |
| rharper | I'm not familiar enough with the code base or interface in snap to know if only certain subsystem triggers are allowed | 16:20 |
| Chipaca | rharper: grepping, that's called with nil, []string{"input"}, []string{"input/joystick"}, and []string{"input", "tty"} | 16:20 |
| Chipaca | rharper: at least currently | 16:21 |
| Chipaca | rharper: why? | 16:21 |
| rharper | I'm looking at logs on a system where after the core snap and the livepatch snap are installed, I can see block device rules get triggered; trying to see if that's related or not | 16:21 |
| Chipaca | zyga: ^ ? | 16:23 |
| Chipaca | rharper: I don't see anything that says to do that with subsusytem=block | 16:24 |
| Chipaca | rharper: but, I'm not familiar with this part of the codebase; zyga or pstolowski would know for sure | 16:24 |
| rharper | Chipaca: ok, thanks | 16:26 |
| pstolowski | rharper, Chipaca: interfaces/udev/backend_test.go tells more about cases where we retrigger udev rules; test case names give context | 16:26 |
| pstolowski | rharper: actually, we do "trigger --subsystem-nomatch=input", per comment in the code:"// By default, trigger for all events except the input subsystem " | 16:30 |
| rharper | yes, was just looking at that | 16:30 |
| pstolowski | rharper: that would explain the block devices case? | 16:30 |
| rharper | yes | 16:30 |
| rharper | yes it does | 16:30 |
| pstolowski | rharper: does it cause issues? | 16:33 |
| zyga | I’m sorry for the lag. I’m not feeling very well. Thank you for handling this pstolowski | 16:34 |
| pstolowski | np | 16:35 |
| rharper | pstolowski: it is, but I'm not sure exactly why; replaying of rules is sometime resulting in missing symlinks; | 16:41 |
| pstolowski | uhm.. not good | 16:48 |
| zyga | mvo: the jenkins core is now in edge | 16:52 |
| mvo | zyga: great | 16:59 |
| mvo | zyga: now we just need a green pr | 16:59 |
| zyga | it failed on github network :/ | 17:05 |
| zyga | with a bit of luck it will go green now | 17:05 |
| zyga | did you merge a 2.37 backport of just the fix (sans the test?) | 17:05 |
| zyga | ah, I see it is also pending | 17:06 |
| === pstolowski is now known as pstolowski|afk | ||
| mup | PR snapd#6449 closed: cmd/snap-confine: add special case for Jenkins <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6449> | 17:29 |
| zyga | jdstrand: nothing for .1 but if you have a moment, I'd love an actionable review comment on https://github.com/snapcore/snapd/pull/6347 | 17:38 |
| mup | PR #6347: many: allow snap-update-ns to write user mount profile <Per-user mount ns 🐎> <Created by zyga> <https://github.com/snapcore/snapd/pull/6347> | 17:38 |
| zyga | morphis: hey | 17:47 |
| zyga | morphis: did you ended up filing that bug? | 17:47 |
| morphis | zyga: no | 18:03 |
| jdstrand | zyga: it's on my list but not for today | 18:09 |
| zyga | thanks, just checking | 18:09 |
| zyga | not urgent | 18:09 |
| sergiusens | zyga: hey, as the resident OS snapd ecosystem rep, do we have guides to make snapd work on systems where systemd is not pid 1? | 18:50 |
| zyga | hey | 18:51 |
| zyga | I think we have no written document about this | 18:51 |
| zyga | we have the older systemd that was patched to run alongside another init system | 18:51 |
| zyga | from snapd's point of view not that many things differ | 18:52 |
| zyga | but I don't think we have any document about this | 18:52 |
| sergiusens | zyga: the reason I ask is because there's an MX Linux developer wanting to enable snapd there | 18:53 |
| zyga | MX linux? | 18:53 |
| zyga | do invite him over to join us here | 18:53 |
| zyga | I think it takes thinking and cooperation but it can definitely be done | 18:53 |
| sergiusens | zyga: should I tell him to just ping you? | 18:53 |
| zyga | please | 18:53 |
| zyga | thanks for raising this! | 18:54 |
| zyga | mvo: 2.37.1 ready | 19:02 |
| zyga | shall I dput? | 19:02 |
| mvo | zyga: sounds good to me | 19:12 |
| zyga | okay | 19:13 |
| zyga | here goes nothing | 19:13 |
| zyga | done | 19:13 |
| mup | PR snapd#6448 closed: snapcraft.yaml: fix XBuildDeb PATH for go-1.10 <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6448> | 19:47 |
| zyga | Pharaoh_Atem: hey, we released 2.37.1 with an important fix for using snaps in jenkins | 19:55 |
| zyga | (we broke usability badly) | 19:55 |
| zyga | Pharaoh_Atem: if you have a moment to release the .1 package it would be great | 19:55 |
| mup | PR snapd#6446 closed: cmd/snap-confine: add special case for Jenkins <⚠ Critical> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6446> | 20:18 |
| mup | PR snapd#6450 opened: release: 2.37.1 <Created by mvo5> <https://github.com/snapcore/snapd/pull/6450> | 21:26 |
| Pharaoh_Atem | zyga: considering I haven't released 2.37 at all yet, yeah I'll look at 2.37.1 | 21:39 |
| Pharaoh_Atem | zyga: insofar as the MX Linux guy wanting to enable snapd, it should be relatively easy to repackage the deputy systemd to run on non-systemd distros | 21:40 |
| mwhudson | zyga: oh hey did you update snapd on salsa? | 23:05 |
| zyga | I sent a PR | 23:05 |
| roadmr | yum | 23:05 |
| zyga | can I push to salsa debian/snapd directly? | 23:06 |
| mwhudson | zyga: ah maybe not | 23:06 |
| mwhudson | i can fix that though | 23:06 |
| mwhudson | zyga: ok you should have access now | 23:08 |
| zyga | thank you, I have access now | 23:08 |
| mwhudson | i merged your PR though so you can't test it by pushing that oh well | 23:08 |
| zyga | mwhudson: that's fine, I'm sure 2.37.2 or 2.38 is soon to follow :) | 23:09 |
| mwhudson | zyga: what's the status of getting the debian packaging into snapcore/snapd? | 23:09 |
| zyga | mwhudson: it was proposed by mvo | 23:10 |
| zyga | mwhudson: we have a card for it and will get it merged | 23:10 |
| mwhudson | zyga: ok | 23:10 |
| zyga | it includes a spread test that really uses it properly (with sbuild and dual build) | 23:10 |
| zyga | I will update it with 2.37.1 release tomorrow | 23:10 |
| mwhudson | will we replaced the snapd branch with something that's basically master with a single extra non-merge commit that changes the 'debian' symlink then? | 23:10 |
| mwhudson | er | 23:11 |
| zyga | yes :) | 23:11 |
| mwhudson | snapd debian branch on salsa i mean | 23:11 |
| mwhudson | awesome | 23:11 |
| mwhudson | zyga: huh i see prs from mvo here https://salsa.debian.org/debian/snapd/merge_requests | 23:11 |
| zyga | yeah | 23:11 |
| mwhudson | but i guess those will get subsumed by the other work? | 23:11 |
| zyga | I noticed too, I'll ask him tomorrow | 23:11 |
| zyga | I think so as well | 23:12 |
| mwhudson | something is a bit odd about them, maybe they are based on revisions i force-pushed over or something | 23:13 |
| mwhudson | ah yes they are | 23:13 |
| mwhudson | how the heck to i sign up to get an email when someone proposed a PR | 23:13 |
| mwhudson | ah 'watch' i guess | 23:14 |
| zyga | yeah | 23:14 |
| zyga | watch | 23:14 |
| mwhudson | anyway lunchtime | 23:23 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!