[04:51] <mup> PR snapd#7607 closed: tests: launch the lxd images folowing the pattern ubuntu:${VERSION_ID} <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7607>
[05:05] <mborzecki> morning
[05:05] <mvo> hey mborzecki
[05:06] <mborzecki> mvo: how are the tests today? i'm seeing quite a lot of red
[05:07] <mborzecki> uhh lxd-nofuse/lxd-snapfuse
[05:07] <mvo> mborzecki: yeah, cachio debugged/fixed this one
[05:07] <mvo> mborzecki: mergering master should help :)
[05:07] <mborzecki> ah, great
[05:11] <zyga> o/
[05:13] <zyga> good morning
[05:13] <zyga> I'll grab some breakfast
[05:14] <mvo> zyga: good morning
[05:25] <mborzecki> zyga:  hey
[05:25] <zyga> o/
[05:26] <mborzecki> mvo:  can tou take a look at https://github.com/snapcore/snapd/pull/7605 ? i think i'm missing something, /etc/systemd as a whole is listed in writable, shouldn't mkdir /etc/systemd/journal.conf.d succeed then?
[05:26] <mup> PR #7605: tests: configure the journald service for core systems <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7605>
[05:30] <zyga> mborzecki: are you sure /etc/systemd is writable? can you provide a reference?
[05:31] <mborzecki> zyga: ahh damn, looking at the wrong tab
[05:31] <mborzecki> zyga: it's writable on core18, on core there's individual directories under /etc/systemd
[05:31] <zyga> that's how I remember it
[05:31] <zyga> yeah, it's complex :/
[05:32] <mvo> mborzecki: sure, looking
[05:32] <mborzecki> mvo: nvm, mystery solved :P
[05:32] <mvo> mborzecki: but iirc on core16 not all of it is writable just selected parts
[05:33] <mborzecki> mvo: right, i have writable-paths from both open and was looking at the other tab :P
[05:34] <zyga> mborzecki: I wonder if xnox experiment to use systemd in initrd works out
[05:38]  * mvo nods
[05:38] <zyga> I'll get to https://github.com/snapcore/snapd/pull/7547 now
[05:38] <mup> PR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>
[05:41] <mborzecki> zyga: can you take a look at https://github.com/snapcore/snapd/pull/7602 ?
[05:41] <mup> PR #7602: overlord/many: extend check snap callback to take snap container <Remodel :train:> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7602>
[05:46] <mvo> zyga: 1825298 is interessting
[05:53] <mvo> dear launchpad, please stop timing out on me when I try to do bugtriage, love michael
[05:57]  * mvo stops bug triage
[06:05] <zyga> mvo: looking now
[06:07] <zyga> mvo: oh boy
[06:07] <zyga> mvo: how do we leave this misery
[06:08] <mvo> zyga: thats a tricky one :(
[06:08] <mvo> zyga: also super annoying :(
[06:09] <zyga> mvo: one way out would be to stop using /etc for apparmor profiles we use
[06:09] <zyga> this would involve shipping empty replacements that do NOP
[06:09] <zyga> and shipping files in /var forever
[06:13] <mvo> zyga: does apparmor support /var ?
[06:13] <zyga> mvo: ubuntu's apparmor does, that's how our profiles work
[06:13] <zyga> mvo: other distributions don't until apparmor 3.0 eventually ships
[06:13] <zyga> mvo: we wrote snapd.apparmor.service to support that and use it on some systems
[06:14] <zyga> mvo: solus has a custom solution for loading profiles but it supports /var/lib/snapd/apparmor already
[06:14] <zyga> mvo: we can do it
[06:16] <mvo> zyga: interessting, might be a good option, I feel I lack some details though, need to look closer after I did this bug triage
[06:17] <zyga> mvo: sure, we can talk about it later today
[06:17] <zyga> mvo: I mainly worry about how to fit this into our framework of rollbacks
[06:18] <amurray> mvo zyga: you could change the profile in /etc to '#include if exists </var/...>' so that it can hang around and still work even if the one in /var goes away
[06:19] <zyga> amurray: hey :)
[06:19] <amurray> mvo zyga: ah sorry - this was added in AppArmor 2.13 iirc so won't work for older releases
[06:19] <zyga> amurray: last time we looked it was not supported
[06:19] <zyga> :D
[06:19] <zyga> exactly
[06:19] <amurray> hey zyga
[06:19] <zyga> so it's great, but we cannot have it
[06:19] <amurray> bah ignore me then... oh well
[06:21] <zyga> amurray: shipping software on linux is easy they said ;)
[06:21] <zyga> amurray: while we have you
[06:21] <zyga> amurray: do you have a moment to look at an idea we had
[06:22] <zyga> amurray: it's described here https://github.com/snapcore/snapd/pull/7547
[06:22] <mup> PR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>
[06:22] <amurray> zyga: sure it's easy shipping software when you have snapd - shipping snapd however... that is harder :)
[06:22] <amurray> zyga: give me a sec
[06:22] <zyga> amurray: but the basic idea is that we mount a cgroup v1 hierarchy at /run/snapd/cgroup
[06:22] <zyga> amurray: no controllers just name=snapd
[06:22] <zyga> amurray: and we use it in v1 and v2 worlds so that we can always track processes
[06:22] <zyga> amurray: we want holes to be poked in the idea
[06:24] <amurray> zyga: my cgroups knowledge is a bit rusty but I will try and brush up and then review it in the next day or so if that's ok?
[06:26] <zyga> amurray: perfect
[06:26] <zyga> thank you so much
[06:26]  * zyga takes bit out for a walk
[06:27] <amurray> zyga: no worries
[06:30] <mvo> amurray: hey, do you happen to know if there is a "if file.exists()" primitive in apparmor profiles? or an include_if_availalbe? context is bug 1825298
[06:30] <mup> Bug #1825298: apparmor.service fails to start when apt install/remove snapd due to snapd profile error <snapd:Confirmed> <snapd (Ubuntu):Triaged> <snapd (Ubuntu Cosmic):Won't Fix> <https://launchpad.net/bugs/1825298>
[07:03] <pstolowski> morning
[07:06] <pstolowski> mborzecki: hey, have you looked at https://paste.ubuntu.com/p/S48Gyv5mpp/ recently? snapd deb failing to install inside container
[07:06] <mborzecki> pstolowski: he, it's fixed in master by cachio
[07:07] <pstolowski> mborzecki: ah, great
[07:11] <mvo> hey pstolowski !
[07:11] <pstolowski> o/
[07:13] <pstolowski> mvo: do you think https://bugs.launchpad.net/snapd/+bug/1824162 is medium prio? i was contemplating setting it critical for a while :/
[07:13] <mup> Bug #1824162: /usr/lib/snapd/snapd:11:runtime:runtime:runtime:runtime:runtime <artful> <bionic> <cosmic> <xenial> <zesty> <snapd:Triaged by stolowski> <snapd (Ubuntu):In Progress> <https://launchpad.net/bugs/1824162>
[07:15] <pstolowski> mvo: it may occur regardless of experimental hotplug flag, and may prevent clean shutdown of snapd; of course only if there is udev netlink activity
[07:27] <mvo> pstolowski: I saw the report
[07:27] <mvo> pstolowski: it should probably be high or criticial, yes
[07:27] <mvo> pstolowski: do we have a fix yet or is it complicated?
[07:28] <pstolowski> mvo: no fix but should be simple, i needed to think a bit about it. should have a PR today
[07:43] <mup> PR snapd#7593 closed: recovery-tool: add sfdisk wrapper <Created by cmatsuoka> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7593>
[07:55] <mup> PR snapd#7609 opened: snap-recovery: remove "usedPartitions" from sfdisk.Create() <Created by mvo5> <https://github.com/snapcore/snapd/pull/7609>
[08:06] <stealthybox> davdunc, could we update the aws-cli snap?   It's getting a bit too old to use with EKS.  Anything Weaveworks can do to help get this bumped regularly?  Cheers
[08:21] <zyga> mborzecki: updated https://github.com/snapcore/snapd/pull/7547
[08:22] <mup> PR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>
[08:22] <zyga> mborzecki: I think I applied everything that we agreed upon
[08:23] <zyga> mborzecki: I didn't apply the changes that would impact people with the feature disabled for now
[08:25] <zyga> mborzecki: I'll do a small patch on top that enables this unconditionally to see if things fall apart
[08:25] <zyga> mborzecki: and then move this to happen for all snap confinement types
[08:28] <oSoMoN> pedronis, hello! would you mind voting on https://forum.snapcraft.io/t/auto-connecting-the-personal-files-interface-for-the-chromium-snap-part-ii/13705 ?
[08:34] <mup> PR snapd#7602 closed: overlord/many: extend check snap callback to take snap container <Remodel :train:> <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7602>
[08:35] <pedronis> mborzecki: please do a follow up to #7602 where you swap *snap.Info and snap.Container
[08:35] <mup> PR #7602: overlord/many: extend check snap callback to take snap container <Remodel :train:> <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7602>
[08:36] <mborzecki> pedronis: sure
[08:36] <pedronis> thanks
[08:38] <pedronis> mborzecki: to be clear, it means that now args are  snap, snapf, curSnap
[08:38] <mborzecki> pedronis: yup, got it :P
[08:38] <pedronis> mborzecki: I suppose snap, curSnap, snapf   if it's a bit simpler
[08:38] <pedronis> but snapf first is strange because of most of these will not use it
[08:39] <pedronis> it's ancillary to the infos
[08:48] <zyga> mborzecki: trivial https://github.com/snapcore/snapd/pull/7610
[08:48] <mup> PR #7610: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <https://github.com/snapcore/snapd/pull/7610>
[08:49] <mup> PR snapd#7610 opened: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <https://github.com/snapcore/snapd/pull/7610>
[08:56] <pstolowski> mvo: hmm i take my words back, a simple fix doesn't cut it for go-udev, it may get more complicated to fix
[09:06] <zyga> mborzecki: we need to talk
[09:06] <zyga> mborzecki: about classic mount ns
[09:06] <zyga> mborzecki: I have an idea that will resolve a problem that was raised during one of the review cycles
[09:06] <zyga> mborzecki: essentially, we can keep using the mount namespace we inherit when starting
[09:06] <zyga> mborzecki: and not re-associate back to pid-1 ns
[09:06] <zyga> mborzecki: I need to go for a haircut now, let's chat later, ok?
[09:07] <mborzecki> zyga: sgtm, deep in some bits of gadget/devicestate now
[09:07] <zyga> mborzecki: but the main idea is: since there is no persistence we don't need to re-associate with pid-1 ns
[09:07] <zyga> mborzecki: and anything we need to do (like tracking) can be forked to a helper that re-associates and quits
[09:07] <zyga> mborzecki: so that main process is where it was originally
[09:07] <zyga> mborzecki: this means that there is perfect compatibility and no impact to any odd setups that work today
[09:08]  * zyga runs for an errand, ttyl
[09:12] <mvo> pstolowski: uh, thats slightly sad
[09:32] <mup> PR snapd#7577 closed: overlord: set fake serial in TestRemodelSwitchToDifferentKernel <Created by mvo5> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7577>
[09:34] <kjackal> hi jdstrand. I am settign up microk8s with strict confinement. Partially works. I have a problem with a pod getting a denial.
[09:34] <kjackal> apparmor="DENIED" operation="ptrace" profile="snap.microk8s.daemon-kubelet//systemd_run" pid=3034 comm="mount" requested_mask="trace" denied_mask="trace" peer="snap.microk8s.daemon-kubelet"
[09:34] <kjackal> Have you seen this before?
[09:40] <Chipaca> kjackal: it's a bit early
[09:40] <kjackal> yeap I know, no worries. I will ping him later as well
[09:44] <pedronis> Chipaca: hi, I did a pass on #7608
[09:44] <mup> PR #7608: o/snapstate, etc: SnapState.Channel -> TrackingChannel, and a setter <Created by chipaca> <https://github.com/snapcore/snapd/pull/7608>
[09:44] <Chipaca> just reading it
[09:44] <Chipaca> pedronis: the "Invalid channel" was because that way it looked just like the error we would get from the store :-)
[09:45] <Chipaca> perhaps too cute a reason tho
[09:59] <Chipaca> pedronis: pushed fixes for those and the spread tests
[09:59] <Chipaca> now going for a walk, bbiab
[10:06] <pedronis> Chipaca: made a naming remark
[10:08] <mup> PR snapd#7611 opened: overlord/many: switch order of check snap parameters <Remodel :train:> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7611>
[10:08] <mborzecki> pedronis: ^^
[10:10] <pedronis> mborzecki: thanks, the fact that nil now are packed together seems to mean it's the right thing
[10:10] <mborzecki> pedronis: wodnering about moving it further to the right, after flags though
[10:11] <pedronis> we tend to put flags as right as we can
[10:12] <pedronis> you would have:   nil, snapstate.Flags{}, nil
[10:12] <pedronis> doesn't look better to me,  any particular reason?
[10:12] <pedronis> if we want to shuffle again
[10:13] <pedronis> having after the first snap would still be best (except it needs more signature churn)
[10:16] <mborzecki> pedronis: otoh, it's probably good as it is in the PR, infos come first, then the container, then the rest
[10:17] <pedronis> mborzecki: as I said I'm fine with the PR state
[10:17] <mborzecki> ack
[10:20] <pstolowski> mborzecki: do you have a moment for https://github.com/snapcore/snapd/pull/7518 ?
[10:20] <mup> PR #7518: cmd/snap: sort tasks in snap debug timings output <Created by stolowski> <https://github.com/snapcore/snapd/pull/7518>
[10:21] <mborzecki> pstolowski: sure
[10:21] <pstolowski> thanks!
[10:22] <mup> PR snapd#7612 opened: gadget: add a public helper for parsing gadget metadata <Remodel :train:> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7612>
[10:22] <mborzecki> another simple one ^^
[10:23] <mup> PR snapd#7355 closed: interfaces/apparmor: load multiple profiles in a single batch <Complex> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/7355>
[10:24] <pstolowski> pedronis: what's the problem with the transition in the store wrt #7092?
[10:24] <mup> PR #7092: packaging: use snapd type and snapcraft 3.x <⛔ Blocked> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7092>
[10:24] <mup> PR snapd#7613 opened: snap-recovery: add minimal binary so that we can use spread on it <Created by mvo5> <https://github.com/snapcore/snapd/pull/7613>
[10:25] <amurray> mvo: there is 'include if exists' - see say https://gitlab.com/apparmor/apparmor/blob/master/profiles/apparmor.d/abstractions/dri-enumerate#L11 for an example - but this is only supported by apparmor 2.13
[10:25] <amurray> and as zyga mentioned, this 2.13 is not available everywhere so include if exists is probably not a realistic option
[10:26] <pedronis> pstolowski: the store checks that the type of snap upload is like the one of the last approved one
[10:26] <pedronis> so for snapd is app
[10:26] <pedronis> atm
[10:27] <pedronis> we are discussing with the store to have an override
[10:27] <pedronis> it's a good check in general, but a couple of cases show we need an override
[10:27] <pstolowski> pedronis: i see, thanks
[10:28] <mvo> amurray: thank you!
[10:33] <mup> PR snapcraft#2751 closed: tests: move cli store push/upload tests to FakeStoreCommands <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2751>
[10:51] <mup> PR snapcraft#2745 closed: tests: update spread tests to account for content snaps <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2745>
[11:12]  * pstolowski lunch
[11:52] <Chipaca> zyga: question about #7610, about something i only noticed thanks to pstolowski
[11:52] <mup> PR #7610: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <https://github.com/snapcore/snapd/pull/7610>
[11:52] <zyga> sure
[11:52] <zyga> looking at the PR
[11:52] <Chipaca> zyga: why do you call geteuid etc when you've already loaded their answers via getresuid?
[11:53] <zyga> Chipaca: there's no good reason, it's just a bunch of old code few people touch
[11:53] <zyga> Chipaca: some reasons as that it is easier to ... reason about the code in front of you
[11:53] <zyga> vs some state
[11:53] <zyga> Chipaca: but I think that it can be simplified
[11:53] <zyga> pstolowski, Chipaca: would you mind if I open the simplification as another PR if this goes green?
[11:54] <zyga> I have some more in this area anyway
[11:54] <pstolowski> zyga: absolutely
[11:54] <Chipaca> zyga: what's currently 'if geteuid() != 0 { bail } ' could go right after the call to getresuid and make it easier to follow without needing the extra syscall
[11:54] <zyga> yeah
[11:54] <Chipaca> etc etc
[11:54] <zyga> I missed that, I looked at other parts and got confused
[11:54] <Chipaca> zyga: also these are all nits :-)
[11:55] <zyga> sure, but it's good to ask such questions
[11:55] <zyga> thank you for that!
[11:55] <zyga> :)
[11:56] <Chipaca> :)
[11:57] <Chipaca> speaking of questions, lunchp?
[11:57]  * Chipaca goes
[11:58] <zyga> Chipaca: lunchp? is redundant, no?
[11:58] <zyga> it's either lunch? or lunchp
[11:58] <zyga> ;-)
[11:59] <Chipaca> I was questioning the very question, probably
[11:59] <zyga> Chipaca: (lunch?)? ;-)
[12:00] <mup> PR snapd#7610 closed: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7610>
[12:00] <zyga> it's in :)
[12:05] <mup> PR snapd#7611 closed: overlord/many: switch order of check snap parameters <Remodel :train:> <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/7611>
[12:38] <zyga> mborzecki: I opened a draft of device cgroup changes I had stashed, there's more piled on top to support v2 but I think this is a good thing to discuss before
[12:38] <zyga> mborzecki: if you have some time now we can discuss before the standup
[12:38] <mup> PR snapd#7614 opened: cmd/snap-confine: implement snap-device-helper internally <Created by zyga> <https://github.com/snapcore/snapd/pull/7614>
[12:38] <zyga> mborzecki: it's not complex, I think it's actually more readable than before
[12:39] <mborzecki> zyga: ok, let me grab a coffee
[12:41] <jdstrand> pstolowski: re 7355> woohoo, merged! :)
[12:41] <zyga> jdstrand: hey :)
[12:42] <zyga> jdstrand: there's something that you could look at to help us move forward with a feature
[12:42] <zyga> jdstrand: we also asked alex and he said he would look tomorrow
[12:42] <zyga> jdstrand: but it would be good for you be aware of it in general
[12:42] <zyga> jdstrand: https://github.com/snapcore/snapd/pull/7547
[12:42] <mup> PR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>
[12:43] <zyga> jdstrand: the main idea is that we mount a new cgroup at /run/snapd/cgroup, just for tracking processes
[12:43] <zyga> jdstrand: no controllers are involved
[12:43] <jdstrand> kjackal: yes, I have seen that and it should be addressed in snapd 2.42. what does 'snap version' give you?
[12:43] <zyga> jdstrand: and we use this method in v1 and v2 worlds where it surprisingly works for as long as v1 is in the kernel
[12:44] <zyga> jdstrand: we have some more plans for this, there's a follow up branch that adds and configures a release agent for the cgroup
[12:44] <zyga> jdstrand: and this lets us solve some long-standing bugs with resouce leaks as well as get the new desired features for app termination notification in snapd
[12:50] <kjackal> jdstrand: 2.42+git1515.143caf4~ubuntu16.04.1
[12:50] <jdstrand> zyga: thanks for making me aware. the idea seems ok otoh
[12:52] <kjackal> jdstrand: I am now in the middle of another addon that is failing. This time with:
[12:52] <kjackal> apparmor="DENIED" operation="exec" info="no new privs" error=-1 profile="snap.microk8s.daemon-containerd" name="/usr/local/bin/proxy-init" pid=16941 comm="runc:[2:INIT]" requested_mask="x" denied_mask="x" fsuid=0 ouid=0 target="cri-containerd.apparmor.d"
[12:53] <jdstrand> kjackal: can you paste /var/lib/snapd/apparmor/profiles/snap.microk8s.daemon-kubelet ?
[12:54] <kjackal> jdstrand: Here it is http://paste.ubuntu.com/p/fqpxsZscF9/
[12:55] <jdstrand> kjackal: re the paste> you don't have the rule in there. can you paste /snap/microk8s/current/meta/snap.yaml ?
[12:56] <kjackal> jdstrand: http://paste.ubuntu.com/p/4qMqPQjmKn/
[12:57] <jdstrand> hmm, I thought that was in 2.42
[12:57]  * jdstrand double checks
[13:02] <jdstrand> kjackal: ok, I misread the trace denial. I saw it the other way around (kubelet tracing itself) but not kubelet's systemd-run needing to trace kubelet
[13:03] <jdstrand> kjackal: do you know what operation is causing that?
[13:04] <kjackal> jdstrand: I shared this via email https://docs.google.com/document/d/1mGHEfgiTDB6Nd11vgkPgCY2OrzrDGk7vmEgjrJGTKmA/edit?usp=sharing
[13:04] <jdstrand> kjackal: also, what kernel is this on?
[13:04] <kjackal> jdstrand: its on Linux ip-172-31-20-243 4.15.0-1051-aws #53-Ubuntu SMP Wed Sep 18 13:35:53 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
[13:04] <mup> PR #53: Negate the integration build tag <Created by elopio> <Merged by elopio> <https://github.com/snapcore/snapd/pull/53>
[13:05] <jdstrand> kjackal: ok, I understand the denial and will create a PR
[13:05] <jdstrand> kjackal: do you have more information on the proxy-init denial?
[13:06] <kjackal> I am looking into this now
[13:06] <kjackal> What other info would you like me to gather?
[13:07] <jdstrand> kjackal: like with the subpath, the context of the denial. also, this looks to be in aws, can you give me ssh access?
[13:08] <kjackal> of course, looking for you in LP
[13:10] <kjackal> jdstrand: try ubuntu@ec2-54-161-115-99.compute-1.amazonaws.com
[13:13] <jdstrand> kjackal: ok, I'm in
[13:13] <kjackal> there is a tmux session I am using, in case you want to show me anything
[13:14] <jdstrand> kjackal: what are you doing to trigger the proxy-init denial?
[13:14] <kjackal> jdstrand: it is part of the "microk8s.enable linkerd"
[13:15] <kjackal> all containers are failing. I suspect the reason is "securityContext:
[13:15] <kjackal>           runAsUser: 2103"
[13:16] <kjackal> jdstrand: see "sudo microk8s.kubectl edit -n linkerd       deployment.apps/linkerd-controller"
[13:16] <jdstrand> that is entirely possible
[13:18] <jdstrand> kjackal: that user doesn't exist on the main system, but more importantly, I think nnp (no new privs) may get in the way if you try to use a different user than what it is running as. try making that root
[13:18] <kjackal> jdstrand: the problem is that the manifests I apply are not mine
[13:19] <kjackal> jdstrand: I am deploying the linkerd manifests. If they fail out of the box they will fail from anyone
[13:20] <jdstrand> joedborg: are you using linkerd? have you seen/fixed kjackal's denial in kubernetes-worker?
[13:20] <roadmr> linkerd sounds like a startup's name (linkerd.io! we link all your links!)
[13:21] <joedborg> jdstrand: i'm not using linkered.  kjackal sent me some last week which one in particular?
[13:21] <jdstrand> joedborg: the one from 30 minutes ago
[13:21] <jdstrand> (in this channel)
[13:21]  * joedborg found it
[13:22] <jdstrand> kjackal: nnp presents a number of challenges
[13:22] <kjackal> joedborg: these are the instructions for deploying linkerd https://linkerd.io/2/getting-started/
[13:22] <roadmr> I was just joking about linkerd.io :(
[13:23] <jdstrand> roadmr: clearly not ;)
[13:23] <roadmr> well it's like "I was just joking about Donald Trump being president" :(
[13:23] <joedborg> jdstrand: kjackal: Ah, no i haven't, but this may be where k8s-worker and microk0s diverge; microk8s runs a lot of infra outside of k8s, where as k8s-worker runs it inside, so there will be differences like this (where the paths are different, for example)
[13:24] <joedborg> thanks kjackal, I'll test it on my local cluster
[13:24] <jdstrand> joedborg: that's fine, thanks
[13:25] <kjackal> joedborg: jdstrand, do you think we could say that "certain workloads will not run on strictly confined setup" ?
[13:25] <jdstrand> kjackal: have you tried to use a layout for /usr/local/bin?
[13:25] <jdstrand> kjackal: it isn't in the current snap.yaml
[13:26] <jdstrand> kjackal: ala https://github.com/charmed-kubernetes/snap-kubernetes-worker/blob/master/snap/snapcraft.yaml
[13:26] <kjackal> jdstrand:  I can add it now
[13:26] <jdstrand> kjackal: I would like to better understand what is happening before saying that (re certain workloads)
[13:27] <joedborg> jdstrand kjackal I think it's possible to run all workloads under strict confinement, but that it hignes on getting runc to use chroot, rather than pivot (as we discussed yesterday)
[13:32] <jdstrand> kjackal: can you not install/refresh microk8s on the aws node I'm on for a bit (I'll let you know)?
[13:32] <jdstrand> kjackal: also, what is a command I can run to trigger the subpath denial?
[13:32] <joedborg> jdstrand: kjackal: ah, so I do get that same issue when running linkerd `[86131.195871] audit: type=1400 audit(1571232667.748:226767): apparmor="DENIED" operation="exec" info="no new privs" error=-1 profile="snap.kubernetes-worker.containerd" name="/usr/local/bin/proxy-init" pid=10902 comm="runc:[2:INIT]" requested_mask="x" denied_mask="x" fsuid=0 ouid=0 target="cri-containerd.apparmor.d"`
[13:33] <kjackal> jdstrand: try this: sudo microk8s.kubectl apply -f  /var/snap/microk8s/current/testcfgmount.yaml
[13:34] <jdstrand> joedborg: is it possible to intercept that 'runAsUser: 2103' bit and remove it/make it '0'?
[13:35] <joedborg> jdstrand: it might be possible, but would probably mean no cross compatibility with running clusters
[13:36] <Chipaca> zyga: I had my hand on "close window" before mvo finished saying "weird users of qemu"
[13:36]  * Chipaca stays well away
[13:37] <joedborg> jdstrand: just FYI, that's user 2103 from within containerd
[13:37] <pedronis> Chipaca: any reason not to change .Full already ? just more spread tests breaking? or something else
[13:37] <joedborg> jdstrand: like with the pivot root stuff
[13:38] <Chipaca> pedronis: no reason
[13:38] <Chipaca> pedronis: oh wait, you mean the implementation?
[13:38] <pedronis> Chipaca: yes
[13:38] <jdstrand> kjackal, joedborg: I need to study the linkerd issue. nnp is, shall we say, problematic atm for all the LSMs (including apparmor)
[13:38] <pedronis> it's used only in errors afaict
[13:39] <Chipaca> pedronis: I think it'll break some things without the patch
[13:39] <Chipaca> let me check though
[13:39] <joedborg> jdstrand: i imagine there's something been done to the docker snap to allow that
[13:40] <pedronis> Chipaca: afai see all usages are in cmd/snap/error.go
[13:40] <pedronis> Chipaca: this is Full, not Clean
[13:40] <Chipaca> pedronis: that's usage of .Full, usage of canon(ical)ize are in snapstate
[13:41] <pedronis> I don't see Full in snapstate
[13:41] <pedronis> (I might have removed it myself at some point, don't know, I don't see it anymore)
[13:41] <Chipaca> pedronis: my question is what happens if you do --channel=//foo//, shut down snapd ,restart with the new one that calls Canonize
[13:42] <pedronis> Chipaca: what it has to do with Full() tough?
[13:42] <pedronis> that's a different issue
[13:42] <Chipaca> ?
[13:42] <pedronis> Chipaca: yes, we want the patch very soon
[13:42] <Chipaca> pedronis: then we're talking cross-porpoises
[13:42] <Chipaca> pedronis: what're you talking about :-)
[13:43] <pedronis> Chipaca: change Channel.Full to be Canonize
[13:43] <pedronis> what code would explode ?
[13:43] <roadmr> 🐬
[13:43] <Chipaca> pedronis: i thought we were talking about making channel.Canonize call Channel.Full
[13:44] <Chipaca> pedronis: you were talking about renaming Channel.Full to be Channel.Canonize?
[13:44] <pedronis> no?
[13:44] <pedronis> no
[13:44] <pedronis> no
[13:44] <pedronis> no
[13:44]  * Chipaca giggles
[13:44] <pedronis> Chipaca: use the code in Canonize as implementation for Full
[13:45] <Chipaca> ahhhhhhh
[13:45] <pedronis> then make channel.Full use Channel.Full
[13:45] <pedronis> still a bit unclear what this would make explode
[13:45] <pedronis> more than the current state of the PR
[13:45] <Chipaca> nothing, probably -- it's more lenient
[13:45] <Chipaca> might break some tests that could be checking the lack of leniency
[13:45] <pedronis> ok
[13:46] <Chipaca> other than that it should be about the same
[13:46] <pedronis> as I said, Channel.Full afaict is only used in cmd/snap/error.go
[13:46] <pedronis> Chipaca: I know we need to fix Clean at some point
[13:46] <pedronis> because in the new world is weird
[13:46] <pedronis> but that came come later
[13:46] <Chipaca> ok
[13:46] <pedronis> when we do the actual default track work
[13:46] <Chipaca> I'll write the code but not push it unless there's another reason to iterate this PR
[13:47] <Chipaca> I'll push it as separate otherwise
[13:47] <pedronis> as you prefer
[13:47] <pedronis> but let's try to use Full in full (as name)
[13:48] <jdstrand> ijohnson: hey did you do anything to docker to avoid denials like this:
[13:48] <jdstrand> [86131.195871] audit: type=1400 audit(1571232667.748:226767): apparmor="DENIED" operation="exec" info="no new privs" error=-1 profile="snap.kubernetes-worker.containerd" name="/usr/local/bin/proxy-init" pid=10902 comm="runc:[2:INIT]" requested_mask="x" denied_mask="x" fsuid=0 ouid=0 target="cri-containerd.apparmor.d"
[13:49] <jdstrand> ijohnson: the theory is it has to do with runAsUser
[13:52] <jdstrand> kjackal, joedborg (cc joeubuntu, amurray): so that denial ^ is going to require not insignificant investigation, possibly from other members of the security team. nnp is quite problematic with LSMs with current kernels
[13:52] <joedborg> jdstrand: nnp?
[13:53] <jdstrand> kjackal, joedborg (cc joeubuntu, amurray): this is unplanned work and beyond the strict mode k8s minimum viable product
[13:53] <jdstrand> joedborg: 'no new privs'
[13:54]  * joedborg reads kernel.org
[13:54] <joedborg> jdstrand: ah okay, cool
[13:54] <kjackal> jdstrand: This denial came up with the DNS addon and had to patch it https://github.com/ubuntu/microk8s/blob/feature/strict-v3/microk8s-resources/actions/coredns.yaml#L105
[13:55] <jdstrand> kjackal (cc joedborg, joeubuntu, amurray): without investigating, my concern is this may require kernel changes for things to run unpatched
[13:55] <kjackal> meaning, it is rather common
[13:55] <jdstrand> kjackal: indeed. this isn't 'just a snapd' thing
[13:57] <jdstrand> kjackal (cc joeubuntu, amurray and joedborg): I suggest trying to work around it for now. if you need me/the security team to investigate it, we likely need to discuss prioritizing/resourcing with Tim and Joe
[13:58] <kjackal> yeap
[13:58] <ijohnson> jdstrand: hmm I don't think I've seen that one before
[13:58] <jdstrand> it's possible kernel changes are not invloved. eg, there are code changes in snap-confine that I did to work around it. something similar might be possible in linkerd or containerd (beyond the oci change, etc)
[13:58] <jdstrand> ijohnson: ack, thanks
[13:59] <jdstrand> but it depends on what the investigation reveals
[13:59] <ijohnson> did y'all find anything more about pivot_root last night? I didn't see anything more on IRC after I left
[13:59] <jdstrand> kjackal: unless you escalate to Tim and joeubuntu, I'm not planning to look into it atm
[14:02] <jdstrand> kjackal: ok, swinging back to the trace denial
[14:02] <jdstrand> kjackal: I'm tinkering with the instance now
[14:02] <kjackal> thanks
[14:03] <mup> PR snapd#7606 closed: tests: run apt update in lxd containers <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/7606>
[14:03] <kjackal> I will report back to Tim and we will see what we have to do about it. Thank you jdstrand
[14:14] <zyga> jdstrand: anything I can help with?
[14:14]  * zyga has highlight on snap-confine
[14:17] <jdstrand> zyga: no, snap-confine is fine. I was just talking about some hoops I had to jump through wrt nnp for context on the linkerd denial
[14:17] <zyga> ack
[14:17] <jdstrand> kjackal: ok, I'm done. with this: https://paste.ubuntu.com/p/HFpmhxVph5/ I'm able to run that test command
[14:18] <jdstrand> kjackal: those may not be the rules I use in the PR, but if you want to manually add them for testing, etc, there they are
[14:18] <jdstrand> kjackal: logging out of the instance. I'll ping you with the PR
[14:18] <kjackal> thank you jdstrand
[14:29] <kjackal> jdstrand: I tried to setup a branch on the latest channel for build of the strict microk8s. It seems things are blocked into maual review. Is there a way to automate the pushes?
[14:32] <jdstrand> kjackal: yes. I have to step away but will do that when I get back
[14:35] <kjackal> cool
[14:41] <oSoMoN> jdstrand, could you please vote/comment on https://forum.snapcraft.io/t/auto-connecting-the-personal-files-interface-for-the-chromium-snap-part-ii/13705 ?
[14:41] <oSoMoN> (thanks in advance!)
[15:06]  * zyga goes for a nap
[15:06] <zyga> ttyl
[15:21] <mup> PR snapcraft#2752 opened: errors: migrate handful of errors to SnapcraftException <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/2752>
[15:22] <kjackal_v2> jdstrand: joedborg: the prometheus operator is also failing with the same error:
[15:22] <kjackal_v2> apparmor="DENIED" operation="exec" info="no new privs" error=-1 profile="snap.microk8s.daemon-containerd" name="/bin/operator"
[15:22]  * cachio lunch
[15:23] <joedborg> kjackal_v2: +1
[16:06] <mup> PR snapd#7615 opened: policy: implement CanRemove policy for the snapd type <Created by mvo5> <https://github.com/snapcore/snapd/pull/7615>
[16:57] <zyga> mvo: I'm fixing the bug with snapd removal
[17:08] <Chipaca> ttfn!
[17:20] <ogra> zyga, by makiing it unremovable ? :)
[17:20] <zyga> ogra: not today :)
[17:20] <ogra> hehe
[17:30]  * ijohnson goes to lunch before mborzecki's schedule PR makes my brain hurt more
[17:37] <zyga> ijohnson|lunch: just schedule lunch, it will solve itself
[17:38] <ogra> snap feed --time=
[17:38] <ogra> ??
[17:43]  * zyga needs to finish this tomorrow
[17:43] <zyga> time to rest
[18:05] <mup> PR snapd#7616 opened: interfaces/many: allow systemd-run to mount volume subPaths plus cleanups <Created by jdstrand> <https://github.com/snapcore/snapd/pull/7616>
[18:07] <jdstrand> kjackal: fyi: https://github.com/snapcore/snapd/pull/7616
[18:07] <mup> PR #7616: interfaces/many: allow k8s/systemd-run to mount volume subPaths plus cleanups <Created by jdstrand> <https://github.com/snapcore/snapd/pull/7616>
[18:07] <kjackal> awesome, thank you jdstrand
[18:07] <jdstrand> kjackal: is that instance still up for me to verify ^ for the subPath fix?
[18:08] <kjackal> yes it is up
[18:08] <jdstrand> kjackal: do you mind if I check now real quick?
[18:08] <kjackal> jdstrand: no go ahead
[18:09] <kjackal> jdstrand: any suggestion for this:
[18:09] <kjackal> [ 4041.214192] audit: type=1400 audit(1571247745.450:4392): apparmor="DENIED" operation="open" profile="snap.microk8s.daemon-kubelet" name="/var/lib/snapd/hostfs/usr/lib/x86_64-linux-gnu/libnvidia-ml.so.390.116" pid=4464 comm="kubelet" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
[18:09] <kjackal> [ 4041.214196] audit: type=1400 audit(1571247745.450:4393): apparmor="DENIED" operation="open" profile="snap.microk8s.daemon-kubelet" name="/var/lib/snapd/hostfs/usr/lib/i386-linux-gnu/libnvidia-ml.so.390.116" pid=4464 comm="kubelet" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
[18:09] <kjackal> [ 4041.361220] audit: type=1400 audit(1571247745.598:4394): apparmor="DENIED" operation="open" profile="snap.microk8s.daemon-containerd" name="/etc/nvidia-container-runtime/config.toml" pid=15115 comm="nvidia-containe" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
[18:09] <kjackal> trying to see if gpu support is broken or not
[18:11] <jdstrand> kjackal: I'm going to defer to zyga for that. it is curious you are seeing this in hostfs. I would expect the libs to be in /var/lib/snapd/lib/gl
[18:11] <jdstrand> kjackal: perhaps you could use a layout for /etc/nvidia-container-runtime and have it look in the right place?
[18:12] <jdstrand> kjackal: but usr/lib/i386-linux-gnu/libnvidia-ml.so.390.116 is not what I expected
[18:12] <jdstrand> kjackal: that said, zyga has looked at gpu support a lot. he may be eod atm
[18:12] <kjackal> ok, I will see what I can do
[18:12] <kjackal> thanks
[18:13] <zyga> jdstrand: there are cases where we take it from hostfs
[18:13] <jdstrand> kjackal: I'll ping you in a minute when I'm done with the test
[18:13] <jdstrand> (subPath)
[18:13] <zyga> kjackal: I'm EOD, please drop me a note with the details
[18:14] <kjackal> cool zyga, let me try some things first
[18:22] <jdstrand> kjackal: ok, this would be the result of when that PR is applied: https://paste.ubuntu.com/p/fFXYP5kNvk/, and with that 'sudo microk8s.kubectl apply -f /var/snap/microk8s/current/testcfgmount.yaml' doesn't have the ptrace trace denial
[18:22] <jdstrand> kjackal: so, cool :)
[18:22] <jdstrand> kjackal: logging out now
[18:23] <kjackal> many thanks jdstrand
[18:23] <jdstrand> kjackal: np. when you see that merged, the next day's core from edge will have it
[18:24] <kjackal> great
[18:24] <jdstrand> kjackal: this should be in 2.43
[18:25] <jdstrand> kjackal: are you using the same snap name for strict mode microk8s? I ask cause I don't see any failed reviews for microk8s
[18:26] <kjackal> let me fire the lp builder
[18:27] <jdstrand> kjackal: I'll look at the snap.yaml in the instance and create a snap decl
[18:28] <kjackal> just queued a build
[18:28] <kjackal> ETA in 13 minutes (estimated)
[18:37] <jdstrand> kjackal: it will fail due to the review-tools needing an update for your use of personal-files. I've made that change but it will need to be manually approved for now
[18:38] <jdstrand> kjackal: but you can ping my to manually approve in the meantime
[18:39] <kjackal> is this a one off approval or do I need to ping you on every new revision I want to push to the channel branch?
[18:39] <kjackal> jdstrand: ^
[18:40] <jdstrand> kjackal: until the review-tools are in prod, every one. after that, it will remember
[18:41] <kjackal> jdstrand: cool, for now I can wait. We may want to push one in a couple of weeks
[18:41] <jdstrand> roadmr: can you pull 20191016-1840UTC? just doc changes and 2 override additions
[18:41] <jdstrand> kjackal: ^ that has the change I mentioned
[18:42] <jdstrand> kjackal: ok, it should all be in place then
[18:42] <jdstrand> roadmr: and hi! :)
[18:42] <kjackal> awesome
[19:05] <roadmr> jdstrand: hey! sure thing
[19:06] <roadmr> jdstrand: when can we chat re: stateful review-tools?
[19:06]  * cachio afk
[19:07] <jdstrand> roadmr: tomorrow?
[19:08] <jdstrand> roadmr: and thanks! :)
[19:08] <roadmr> jdstrand: sure, can I go by your google calendar?
[19:09] <roadmr> jdstrand: looks like there's no such tag in the repo :/
[19:10] <jdstrand> roadmr: yes, please look at the calendar
[19:10]  * jdstrand looks at tag
[19:11] <roadmr> thanks!
[19:12] <jdstrand> roadmr: 20191016-1840UTC ? I see it in https://git.launchpad.net/review-tools?h=master
[19:13] <jdstrand> https://git.launchpad.net/review-tools/tag/?id=20191016-1840UTC
[19:13] <roadmr> jdstrand: ah! it's there now
[19:13] <jdstrand> roadmr: oh hah, I pushed the tag, not the commit :)
[19:13] <roadmr> jdstrand: some refreshing shenanigans, it seems
[19:13] <roadmr> 😆
[19:14] <ijohnson> jdstrand: re #7616 am I correct in thinking that mount rules for `/var/snap/@{SNAP_INSTANCE_NAME}/*/**` would allow a snap revision to mount stuff over another revision?
[19:14] <mup> PR #7616: interfaces/many: allow k8s/systemd-run to mount volume subPaths plus cleanups <Created by jdstrand> <https://github.com/snapcore/snapd/pull/7616>
[19:14] <jdstrand> ijohnson: yep
[19:14] <jdstrand> ijohnson: hence my comment about refresh
[19:14] <ijohnson> is that intentional ?
[19:15] <jdstrand> ijohnson: did you see the commit message? I avoided SNAP_REVISION after all thinking that refresh would be potentially problematic
[19:15] <ijohnson> jdstrand: I did I just don't quite understand the reasoning
[19:16] <ijohnson> if they needs mounts to persist across refreshes then they should definitely use common
[19:16] <ijohnson> my original point was just that maybe someday they don't need to care about mounts persisting across  revisions and instead have incompatible data between revisions that they explicitly don't want to be shared across revisions
[19:17] <ijohnson> if that was the case they could then transition to mounting things on top of $SNAP_DATA, which would be per revision
[19:17] <jdstrand> ijohnson: I suspect that, like lxd, k8s will use the mechanism to not restart itself so pods aren't blown away
[19:17] <ijohnson> yes that is probably true
[19:17] <ijohnson> does lxd have rules to do mounts across revisions in $SNAP_DATA though?
[19:18] <jdstrand> ijohnson: lxd's profile allows transitioning to unconfined so there are no rules
[19:18] <jdstrand> ijohnson: (beyond allowing it to get to that point)
[19:18] <ijohnson> ah right yeah
[19:18] <ijohnson> lxd is special
[19:18] <jdstrand> ijohnson: iirc, this was why common/ was enforced in kubernetes-support
[19:19] <jdstrand> (ie, the refresh point, not lxd :)
[19:19] <jdstrand> it's been a while since I wrote those rules, but iirc I was enforcing best practice
[19:19] <ijohnson> jdstrand: perhaps given this we should just stick with common
[19:20] <jdstrand> ijohnson: ok. easy enough. we can rediscuss if there is a use case for SNAP_DATA
[19:20] <ijohnson> sure sounds good
[19:20] <ijohnson> feel free to force push if you want to remove that most recent commit
[19:39] <jdstrand> ijohnson: I am tempted to do that since I noticed typos in some commit messages ;) however, I did a revert commit so the discussion is captured for posterity
[19:47] <ijohnson> jdstrand: ack
[19:56] <mup> PR snapd#7617 opened: snap-confine.apparmor.in: harden pivot_root until we have full mediation <Created by jdstrand> <https://github.com/snapcore/snapd/pull/7617>
[20:03] <jdstrand> roadmr: fyi, https://dashboard.snapcraft.io/snaps/microk8s/feedback/ has been giving a 504 for... 30 minutes?
[20:03] <jdstrand> roadmr: ish
[20:03] <roadmr> thanks jdstrand, fighting some minor fires store-side
[20:04] <jdstrand> roadmr: ok, just fyi. I'm not blocked
[20:04] <roadmr> right - seems to be widespread, get the same for e.g. core
[20:07] <jdstrand> roadmr: fyi, it is not *all* snaps. eg https://dashboard.snapcraft.io/snaps/telegraf/feedback/ is fine
[20:18] <roadmr> ah, interesting
[20:41] <roadmr> jdstrand: nessita found it's an n+1 queries problem which will hit snaps with many uploads
[20:41] <jdstrand> interesting
[20:42] <jdstrand> roadmr: I did see this with https://dashboard.snapcraft.io/snaps/app-outlet/ (notice no feedback/ in url) and it sometimes displayed
[20:42] <nessita> jdstrand, if you browse by revision, it should work
[20:43] <nessita> jdstrand, like https://dashboard.snapcraft.io/snaps/core/revisions/<revision>/
[20:44] <jdstrand> roadmr, nessita: well, I would need to know the revision. I guess I could snap info it.... also, I don't know if this makes a difference, but as a data point, I added scores of snap decls for snaps for pulseaudio, each with a feedback comment
[20:44] <jdstrand> (yesterday)
[20:45] <jdstrand> continuing that today
[20:47] <jdstrand> nessita: fyi, 504 with https://dashboard.snapcraft.io/snaps/app-outlet/revisions/1/
[20:50] <nessita> jdstrand, odd, it opened quite fast for me. Reloading...
[20:50] <nessita> hum 504 now
[20:50] <nessita> roadmr, maybe we have some sca fe with troubles?
[20:50] <jdstrand> nessita: that one didn't have many revisions iirc
[20:51] <roadmr> nessita: could be, or maybe the app units, one could have landed on an evil host
[20:51] <jdstrand> nessita: I just saw one with 73 revisions that was fine
[20:51] <nessita> jdstrand, yeah, we may have more than one issue. I mean, the linear queries is a problem and I've writen a test for it (that fails right now)
[20:51] <nessita> jdstrand, if you reload a few times https://dashboard.snapcraft.io/snaps/app-outlet/revisions/1/, do you get the right page from time to time?
[20:52] <jdstrand> nessita: I see it now
[20:52] <jdstrand> (30 revisions)
[20:53] <nessita> jdstrand, we may issues with one of the fe units or app units, let us look
[20:54] <roadmr> nessita: I would think app/backend units, the fe is just haproxy, shouldn't have any problems plus hasn't changed in a while
[20:56] <nessita> roadmr, jdstrand moving this to our private channel
[20:56] <roadmr> +1
[20:56] <nessita> roadmr, jdstrand #snastore please
[20:56] <nessita> roadmr, jdstrand #snapstore please
[21:34] <mup> PR snapcraft#2753 opened: cli: use click utilities for registering on push <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2753>
[22:00] <mup> PR snapd#7604 closed: many: address issues related to explicit/implicit channels for image building <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7604>