/srv/irclogs.ubuntu.com/2019/10/16/#snappy.txt

mupPR 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>04:51
mborzeckimorning05:05
mvohey mborzecki05:05
mborzeckimvo: how are the tests today? i'm seeing quite a lot of red05:06
mborzeckiuhh lxd-nofuse/lxd-snapfuse05:07
mvomborzecki: yeah, cachio debugged/fixed this one05:07
mvomborzecki: mergering master should help :)05:07
mborzeckiah, great05:07
zygao/05:11
zygagood morning05:13
zygaI'll grab some breakfast05:13
mvozyga: good morning05:14
mborzeckizyga:  hey05:25
zygao/05:25
mborzeckimvo:  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
mupPR #7605: tests: configure the journald service for core systems <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7605>05:26
zygamborzecki: are you sure /etc/systemd is writable? can you provide a reference?05:30
mborzeckizyga: ahh damn, looking at the wrong tab05:31
mborzeckizyga: it's writable on core18, on core there's individual directories under /etc/systemd05:31
zygathat's how I remember it05:31
zygayeah, it's complex :/05:31
mvomborzecki: sure, looking05:32
mborzeckimvo: nvm, mystery solved :P05:32
mvomborzecki: but iirc on core16 not all of it is writable just selected parts05:32
mborzeckimvo: right, i have writable-paths from both open and was looking at the other tab :P05:33
zygamborzecki: I wonder if xnox experiment to use systemd in initrd works out05:34
* mvo nods05:38
zygaI'll get to https://github.com/snapcore/snapd/pull/7547 now05:38
mupPR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>05:38
mborzeckizyga: can you take a look at https://github.com/snapcore/snapd/pull/7602 ?05:41
mupPR #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:41
mvozyga: 1825298 is interessting05:46
mvodear launchpad, please stop timing out on me when I try to do bugtriage, love michael05:53
* mvo stops bug triage05:57
zygamvo: looking now06:05
zygamvo: oh boy06:07
zygamvo: how do we leave this misery06:07
mvozyga: thats a tricky one :(06:08
mvozyga: also super annoying :(06:08
zygamvo: one way out would be to stop using /etc for apparmor profiles we use06:09
zygathis would involve shipping empty replacements that do NOP06:09
zygaand shipping files in /var forever06:09
mvozyga: does apparmor support /var ?06:13
zygamvo: ubuntu's apparmor does, that's how our profiles work06:13
zygamvo: other distributions don't until apparmor 3.0 eventually ships06:13
zygamvo: we wrote snapd.apparmor.service to support that and use it on some systems06:13
zygamvo: solus has a custom solution for loading profiles but it supports /var/lib/snapd/apparmor already06:14
zygamvo: we can do it06:14
mvozyga: interessting, might be a good option, I feel I lack some details though, need to look closer after I did this bug triage06:16
zygamvo: sure, we can talk about it later today06:17
zygamvo: I mainly worry about how to fit this into our framework of rollbacks06:17
amurraymvo 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 away06:18
zygaamurray: hey :)06:19
amurraymvo zyga: ah sorry - this was added in AppArmor 2.13 iirc so won't work for older releases06:19
zygaamurray: last time we looked it was not supported06:19
zyga:D06:19
zygaexactly06:19
amurrayhey zyga06:19
zygaso it's great, but we cannot have it06:19
amurraybah ignore me then... oh well06:19
zygaamurray: shipping software on linux is easy they said ;)06:21
zygaamurray: while we have you06:21
zygaamurray: do you have a moment to look at an idea we had06:21
zygaamurray: it's described here https://github.com/snapcore/snapd/pull/754706:22
mupPR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>06:22
amurrayzyga: sure it's easy shipping software when you have snapd - shipping snapd however... that is harder :)06:22
amurrayzyga: give me a sec06:22
zygaamurray: but the basic idea is that we mount a cgroup v1 hierarchy at /run/snapd/cgroup06:22
zygaamurray: no controllers just name=snapd06:22
zygaamurray: and we use it in v1 and v2 worlds so that we can always track processes06:22
zygaamurray: we want holes to be poked in the idea06:22
amurrayzyga: 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:24
zygaamurray: perfect06:26
zygathank you so much06:26
* zyga takes bit out for a walk06:26
amurrayzyga: no worries06:27
mvoamurray: 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 182529806:30
mupBug #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>06:30
=== pstolowski|afk is now known as pstolowski
pstolowskimorning07:03
pstolowskimborzecki: hey, have you looked at https://paste.ubuntu.com/p/S48Gyv5mpp/ recently? snapd deb failing to install inside container07:06
mborzeckipstolowski: he, it's fixed in master by cachio07:06
pstolowskimborzecki: ah, great07:07
mvohey pstolowski !07:11
pstolowskio/07:11
pstolowskimvo: do you think https://bugs.launchpad.net/snapd/+bug/1824162 is medium prio? i was contemplating setting it critical for a while :/07:13
mupBug #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:13
pstolowskimvo: it may occur regardless of experimental hotplug flag, and may prevent clean shutdown of snapd; of course only if there is udev netlink activity07:15
mvopstolowski: I saw the report07:27
mvopstolowski: it should probably be high or criticial, yes07:27
mvopstolowski: do we have a fix yet or is it complicated?07:27
pstolowskimvo: no fix but should be simple, i needed to think a bit about it. should have a PR today07:28
mupPR snapd#7593 closed: recovery-tool: add sfdisk wrapper <Created by cmatsuoka> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7593>07:43
mupPR snapd#7609 opened: snap-recovery: remove "usedPartitions" from sfdisk.Create() <Created by mvo5> <https://github.com/snapcore/snapd/pull/7609>07:55
stealthyboxdavdunc, 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?  Cheers08:06
zygamborzecki: updated https://github.com/snapcore/snapd/pull/754708:21
mupPR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>08:22
zygamborzecki: I think I applied everything that we agreed upon08:22
zygamborzecki: I didn't apply the changes that would impact people with the feature disabled for now08:23
zygamborzecki: I'll do a small patch on top that enables this unconditionally to see if things fall apart08:25
zygamborzecki: and then move this to happen for all snap confinement types08:25
oSoMoNpedronis, 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:28
mupPR 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:34
pedronismborzecki: please do a follow up to #7602 where you swap *snap.Info and snap.Container08:35
mupPR #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:35
mborzeckipedronis: sure08:36
pedronisthanks08:36
pedronismborzecki: to be clear, it means that now args are  snap, snapf, curSnap08:38
mborzeckipedronis: yup, got it :P08:38
pedronismborzecki: I suppose snap, curSnap, snapf   if it's a bit simpler08:38
pedronisbut snapf first is strange because of most of these will not use it08:38
pedronisit's ancillary to the infos08:39
zygamborzecki: trivial https://github.com/snapcore/snapd/pull/761008:48
mupPR #7610: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <https://github.com/snapcore/snapd/pull/7610>08:48
mupPR snapd#7610 opened: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <https://github.com/snapcore/snapd/pull/7610>08:49
pstolowskimvo: hmm i take my words back, a simple fix doesn't cut it for go-udev, it may get more complicated to fix08:56
zygamborzecki: we need to talk09:06
zygamborzecki: about classic mount ns09:06
zygamborzecki: I have an idea that will resolve a problem that was raised during one of the review cycles09:06
zygamborzecki: essentially, we can keep using the mount namespace we inherit when starting09:06
zygamborzecki: and not re-associate back to pid-1 ns09:06
zygamborzecki: I need to go for a haircut now, let's chat later, ok?09:06
mborzeckizyga: sgtm, deep in some bits of gadget/devicestate now09:07
zygamborzecki: but the main idea is: since there is no persistence we don't need to re-associate with pid-1 ns09:07
zygamborzecki: and anything we need to do (like tracking) can be forked to a helper that re-associates and quits09:07
zygamborzecki: so that main process is where it was originally09:07
zygamborzecki: this means that there is perfect compatibility and no impact to any odd setups that work today09:07
* zyga runs for an errand, ttyl09:08
=== jacekn_ is now known as jacekn
mvopstolowski: uh, thats slightly sad09:12
mupPR snapd#7577 closed: overlord: set fake serial in TestRemodelSwitchToDifferentKernel <Created by mvo5> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7577>09:32
kjackalhi jdstrand. I am settign up microk8s with strict confinement. Partially works. I have a problem with a pod getting a denial.09:34
kjackalapparmor="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
kjackalHave you seen this before?09:34
Chipacakjackal: it's a bit early09:40
kjackalyeap I know, no worries. I will ping him later as well09:40
pedronisChipaca: hi, I did a pass on #760809:44
mupPR #7608: o/snapstate, etc: SnapState.Channel -> TrackingChannel, and a setter <Created by chipaca> <https://github.com/snapcore/snapd/pull/7608>09:44
Chipacajust reading it09:44
Chipacapedronis: the "Invalid channel" was because that way it looked just like the error we would get from the store :-)09:44
Chipacaperhaps too cute a reason tho09:45
Chipacapedronis: pushed fixes for those and the spread tests09:59
Chipacanow going for a walk, bbiab09:59
pedronisChipaca: made a naming remark10:06
mupPR 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
mborzeckipedronis: ^^10:08
pedronismborzecki: thanks, the fact that nil now are packed together seems to mean it's the right thing10:10
mborzeckipedronis: wodnering about moving it further to the right, after flags though10:10
pedroniswe tend to put flags as right as we can10:11
pedronisyou would have:   nil, snapstate.Flags{}, nil10:12
pedronisdoesn't look better to me,  any particular reason?10:12
pedronisif we want to shuffle again10:12
pedronishaving after the first snap would still be best (except it needs more signature churn)10:13
mborzeckipedronis: otoh, it's probably good as it is in the PR, infos come first, then the container, then the rest10:16
pedronismborzecki: as I said I'm fine with the PR state10:17
mborzeckiack10:17
pstolowskimborzecki: do you have a moment for https://github.com/snapcore/snapd/pull/7518 ?10:20
mupPR #7518: cmd/snap: sort tasks in snap debug timings output <Created by stolowski> <https://github.com/snapcore/snapd/pull/7518>10:20
mborzeckipstolowski: sure10:21
pstolowskithanks!10:21
mupPR 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
mborzeckianother simple one ^^10:22
mupPR 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:23
pstolowskipedronis: what's the problem with the transition in the store wrt #7092?10:24
mupPR #7092: packaging: use snapd type and snapcraft 3.x <⛔ Blocked> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7092>10:24
mupPR 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:24
amurraymvo: 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.1310:25
amurrayand as zyga mentioned, this 2.13 is not available everywhere so include if exists is probably not a realistic option10:25
pedronispstolowski: the store checks that the type of snap upload is like the one of the last approved one10:26
pedronisso for snapd is app10:26
pedronisatm10:26
pedroniswe are discussing with the store to have an override10:27
pedronisit's a good check in general, but a couple of cases show we need an override10:27
pstolowskipedronis: i see, thanks10:27
mvoamurray: thank you!10:28
mupPR 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:33
mupPR 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>10:51
* pstolowski lunch11:12
Chipacazyga: question about #7610, about something i only noticed thanks to pstolowski11:52
mupPR #7610: cmd/snap-confine: remove leftover condition from capability world <Created by zyga> <https://github.com/snapcore/snapd/pull/7610>11:52
zygasure11:52
zygalooking at the PR11:52
Chipacazyga: why do you call geteuid etc when you've already loaded their answers via getresuid?11:52
zygaChipaca: there's no good reason, it's just a bunch of old code few people touch11:53
zygaChipaca: some reasons as that it is easier to ... reason about the code in front of you11:53
zygavs some state11:53
zygaChipaca: but I think that it can be simplified11:53
zygapstolowski, Chipaca: would you mind if I open the simplification as another PR if this goes green?11:53
zygaI have some more in this area anyway11:54
pstolowskizyga: absolutely11:54
Chipacazyga: 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 syscall11:54
zygayeah11:54
Chipacaetc etc11:54
zygaI missed that, I looked at other parts and got confused11:54
Chipacazyga: also these are all nits :-)11:54
zygasure, but it's good to ask such questions11:55
zygathank you for that!11:55
zyga:)11:55
Chipaca:)11:56
Chipacaspeaking of questions, lunchp?11:57
* Chipaca goes11:57
zygaChipaca: lunchp? is redundant, no?11:58
zygait's either lunch? or lunchp11:58
zyga;-)11:58
ChipacaI was questioning the very question, probably11:59
zygaChipaca: (lunch?)? ;-)11:59
mupPR 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
zygait's in :)12:00
mupPR 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:05
=== ricab is now known as ricab|lunch
zygamborzecki: 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 before12:38
zygamborzecki: if you have some time now we can discuss before the standup12:38
mupPR snapd#7614 opened: cmd/snap-confine: implement snap-device-helper internally <Created by zyga> <https://github.com/snapcore/snapd/pull/7614>12:38
zygamborzecki: it's not complex, I think it's actually more readable than before12:38
mborzeckizyga: ok, let me grab a coffee12:39
jdstrandpstolowski: re 7355> woohoo, merged! :)12:41
zygajdstrand: hey :)12:41
zygajdstrand: there's something that you could look at to help us move forward with a feature12:42
zygajdstrand: we also asked alex and he said he would look tomorrow12:42
zygajdstrand: but it would be good for you be aware of it in general12:42
zygajdstrand: https://github.com/snapcore/snapd/pull/754712:42
mupPR #7547: many: use a dedicated named cgroup hierarchy for tracking <Created by zyga> <https://github.com/snapcore/snapd/pull/7547>12:42
zygajdstrand: the main idea is that we mount a new cgroup at /run/snapd/cgroup, just for tracking processes12:43
zygajdstrand: no controllers are involved12:43
jdstrandkjackal: yes, I have seen that and it should be addressed in snapd 2.42. what does 'snap version' give you?12:43
zygajdstrand: and we use this method in v1 and v2 worlds where it surprisingly works for as long as v1 is in the kernel12:43
zygajdstrand: we have some more plans for this, there's a follow up branch that adds and configures a release agent for the cgroup12:44
zygajdstrand: 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 snapd12:44
kjackaljdstrand: 2.42+git1515.143caf4~ubuntu16.04.112:50
jdstrandzyga: thanks for making me aware. the idea seems ok otoh12:50
kjackaljdstrand: I am now in the middle of another addon that is failing. This time with:12:52
kjackalapparmor="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:52
jdstrandkjackal: can you paste /var/lib/snapd/apparmor/profiles/snap.microk8s.daemon-kubelet ?12:53
kjackaljdstrand: Here it is http://paste.ubuntu.com/p/fqpxsZscF9/12:54
jdstrandkjackal: re the paste> you don't have the rule in there. can you paste /snap/microk8s/current/meta/snap.yaml ?12:55
kjackaljdstrand: http://paste.ubuntu.com/p/4qMqPQjmKn/12:56
jdstrandhmm, I thought that was in 2.4212:57
* jdstrand double checks12:57
jdstrandkjackal: 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 kubelet13:02
jdstrandkjackal: do you know what operation is causing that?13:03
kjackaljdstrand: I shared this via email https://docs.google.com/document/d/1mGHEfgiTDB6Nd11vgkPgCY2OrzrDGk7vmEgjrJGTKmA/edit?usp=sharing13:04
jdstrandkjackal: also, what kernel is this on?13:04
kjackaljdstrand: 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/Linux13:04
mupPR #53: Negate the integration build tag <Created by elopio> <Merged by elopio> <https://github.com/snapcore/snapd/pull/53>13:04
jdstrandkjackal: ok, I understand the denial and will create a PR13:05
jdstrandkjackal: do you have more information on the proxy-init denial?13:05
kjackalI am looking into this now13:06
kjackalWhat other info would you like me to gather?13:06
jdstrandkjackal: like with the subpath, the context of the denial. also, this looks to be in aws, can you give me ssh access?13:07
kjackalof course, looking for you in LP13:08
kjackaljdstrand: try ubuntu@ec2-54-161-115-99.compute-1.amazonaws.com13:10
jdstrandkjackal: ok, I'm in13:13
kjackalthere is a tmux session I am using, in case you want to show me anything13:13
jdstrandkjackal: what are you doing to trigger the proxy-init denial?13:14
kjackaljdstrand: it is part of the "microk8s.enable linkerd"13:14
kjackalall containers are failing. I suspect the reason is "securityContext:13:15
kjackal          runAsUser: 2103"13:15
kjackaljdstrand: see "sudo microk8s.kubectl edit -n linkerd       deployment.apps/linkerd-controller"13:16
jdstrandthat is entirely possible13:16
jdstrandkjackal: 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 root13:18
kjackaljdstrand: the problem is that the manifests I apply are not mine13:18
kjackaljdstrand: I am deploying the linkerd manifests. If they fail out of the box they will fail from anyone13:19
jdstrandjoedborg: are you using linkerd? have you seen/fixed kjackal's denial in kubernetes-worker?13:20
roadmrlinkerd sounds like a startup's name (linkerd.io! we link all your links!)13:20
joedborgjdstrand: i'm not using linkered.  kjackal sent me some last week which one in particular?13:21
jdstrandjoedborg: the one from 30 minutes ago13:21
jdstrand(in this channel)13:21
* joedborg found it13:21
jdstrandkjackal: nnp presents a number of challenges13:22
kjackaljoedborg: these are the instructions for deploying linkerd https://linkerd.io/2/getting-started/13:22
roadmrI was just joking about linkerd.io :(13:22
jdstrandroadmr: clearly not ;)13:23
roadmrwell it's like "I was just joking about Donald Trump being president" :(13:23
joedborgjdstrand: 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:23
joedborgthanks kjackal, I'll test it on my local cluster13:24
jdstrandjoedborg: that's fine, thanks13:24
kjackaljoedborg: jdstrand, do you think we could say that "certain workloads will not run on strictly confined setup" ?13:25
jdstrandkjackal: have you tried to use a layout for /usr/local/bin?13:25
jdstrandkjackal: it isn't in the current snap.yaml13:25
jdstrandkjackal: ala https://github.com/charmed-kubernetes/snap-kubernetes-worker/blob/master/snap/snapcraft.yaml13:26
kjackaljdstrand:  I can add it now13:26
jdstrandkjackal: I would like to better understand what is happening before saying that (re certain workloads)13:26
=== ricab|lunch is now known as ricab
joedborgjdstrand 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:27
jdstrandkjackal: can you not install/refresh microk8s on the aws node I'm on for a bit (I'll let you know)?13:32
jdstrandkjackal: also, what is a command I can run to trigger the subpath denial?13:32
joedborgjdstrand: 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:32
kjackaljdstrand: try this: sudo microk8s.kubectl apply -f  /var/snap/microk8s/current/testcfgmount.yaml13:33
jdstrandjoedborg: is it possible to intercept that 'runAsUser: 2103' bit and remove it/make it '0'?13:34
joedborgjdstrand: it might be possible, but would probably mean no cross compatibility with running clusters13:35
Chipacazyga: I had my hand on "close window" before mvo finished saying "weird users of qemu"13:36
* Chipaca stays well away13:36
joedborgjdstrand: just FYI, that's user 2103 from within containerd13:37
pedronisChipaca: any reason not to change .Full already ? just more spread tests breaking? or something else13:37
joedborgjdstrand: like with the pivot root stuff13:37
Chipacapedronis: no reason13:38
Chipacapedronis: oh wait, you mean the implementation?13:38
pedronisChipaca: yes13:38
jdstrandkjackal, joedborg: I need to study the linkerd issue. nnp is, shall we say, problematic atm for all the LSMs (including apparmor)13:38
pedronisit's used only in errors afaict13:38
Chipacapedronis: I think it'll break some things without the patch13:39
Chipacalet me check though13:39
joedborgjdstrand: i imagine there's something been done to the docker snap to allow that13:39
pedronisChipaca: afai see all usages are in cmd/snap/error.go13:40
pedronisChipaca: this is Full, not Clean13:40
Chipacapedronis: that's usage of .Full, usage of canon(ical)ize are in snapstate13:40
pedronisI don't see Full in snapstate13:41
pedronis(I might have removed it myself at some point, don't know, I don't see it anymore)13:41
Chipacapedronis: my question is what happens if you do --channel=//foo//, shut down snapd ,restart with the new one that calls Canonize13:41
pedronisChipaca: what it has to do with Full() tough?13:42
pedronisthat's a different issue13:42
Chipaca?13:42
pedronisChipaca: yes, we want the patch very soon13:42
Chipacapedronis: then we're talking cross-porpoises13:42
Chipacapedronis: what're you talking about :-)13:42
pedronisChipaca: change Channel.Full to be Canonize13:43
pedroniswhat code would explode ?13:43
roadmr🐬13:43
Chipacapedronis: i thought we were talking about making channel.Canonize call Channel.Full13:43
Chipacapedronis: you were talking about renaming Channel.Full to be Channel.Canonize?13:44
pedronisno?13:44
pedronisno13:44
pedronisno13:44
pedronisno13:44
* Chipaca giggles13:44
pedronisChipaca: use the code in Canonize as implementation for Full13:44
Chipacaahhhhhhh13:45
pedronisthen make channel.Full use Channel.Full13:45
pedronisstill a bit unclear what this would make explode13:45
pedronismore than the current state of the PR13:45
Chipacanothing, probably -- it's more lenient13:45
Chipacamight break some tests that could be checking the lack of leniency13:45
pedronisok13:45
Chipacaother than that it should be about the same13:46
pedronisas I said, Channel.Full afaict is only used in cmd/snap/error.go13:46
pedronisChipaca: I know we need to fix Clean at some point13:46
pedronisbecause in the new world is weird13:46
pedronisbut that came come later13:46
Chipacaok13:46
pedroniswhen we do the actual default track work13:46
ChipacaI'll write the code but not push it unless there's another reason to iterate this PR13:46
ChipacaI'll push it as separate otherwise13:47
pedronisas you prefer13:47
pedronisbut let's try to use Full in full (as name)13:47
jdstrandijohnson: 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:48
jdstrandijohnson: the theory is it has to do with runAsUser13:49
jdstrandkjackal, 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 kernels13:52
joedborgjdstrand: nnp?13:52
jdstrandkjackal, joedborg (cc joeubuntu, amurray): this is unplanned work and beyond the strict mode k8s minimum viable product13:53
jdstrandjoedborg: 'no new privs'13:53
* joedborg reads kernel.org13:54
joedborgjdstrand: ah okay, cool13:54
kjackaljdstrand: 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#L10513:54
jdstrandkjackal (cc joedborg, joeubuntu, amurray): without investigating, my concern is this may require kernel changes for things to run unpatched13:55
kjackalmeaning, it is rather common13:55
jdstrandkjackal: indeed. this isn't 'just a snapd' thing13:55
jdstrandkjackal (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 Joe13:57
kjackalyeap13:58
ijohnsonjdstrand: hmm I don't think I've seen that one before13:58
jdstrandit'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
jdstrandijohnson: ack, thanks13:58
jdstrandbut it depends on what the investigation reveals13:59
ijohnsondid y'all find anything more about pivot_root last night? I didn't see anything more on IRC after I left13:59
jdstrandkjackal: unless you escalate to Tim and joeubuntu, I'm not planning to look into it atm13:59
jdstrandkjackal: ok, swinging back to the trace denial14:02
jdstrandkjackal: I'm tinkering with the instance now14:02
kjackalthanks14:02
mupPR 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
kjackalI will report back to Tim and we will see what we have to do about it. Thank you jdstrand14:03
zygajdstrand: anything I can help with?14:14
* zyga has highlight on snap-confine14:14
jdstrandzyga: no, snap-confine is fine. I was just talking about some hoops I had to jump through wrt nnp for context on the linkerd denial14:17
zygaack14:17
jdstrandkjackal: ok, I'm done. with this: https://paste.ubuntu.com/p/HFpmhxVph5/ I'm able to run that test command14:17
jdstrandkjackal: those may not be the rules I use in the PR, but if you want to manually add them for testing, etc, there they are14:18
jdstrandkjackal: logging out of the instance. I'll ping you with the PR14:18
kjackalthank you jdstrand14:18
kjackaljdstrand: 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:29
jdstrandkjackal: yes. I have to step away but will do that when I get back14:32
kjackalcool14:35
oSoMoNjdstrand, 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!)14:41
* zyga goes for a nap15:06
zygattyl15:06
=== pedronis_ is now known as pedronis
mupPR snapcraft#2752 opened: errors: migrate handful of errors to SnapcraftException <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/2752>15:21
kjackal_v2jdstrand: joedborg: the prometheus operator is also failing with the same error:15:22
kjackal_v2apparmor="DENIED" operation="exec" info="no new privs" error=-1 profile="snap.microk8s.daemon-containerd" name="/bin/operator"15:22
* cachio lunch15:22
joedborgkjackal_v2: +115:23
mupPR snapd#7615 opened: policy: implement CanRemove policy for the snapd type <Created by mvo5> <https://github.com/snapcore/snapd/pull/7615>16:06
zygamvo: I'm fixing the bug with snapd removal16:57
=== pstolowski is now known as pstolowski|afk
Chipacattfn!17:08
ograzyga, by makiing it unremovable ? :)17:20
zygaogra: not today :)17:20
ograhehe17:20
* ijohnson goes to lunch before mborzecki's schedule PR makes my brain hurt more17:30
=== ijohnson is now known as ijohnson|lunch
zygaijohnson|lunch: just schedule lunch, it will solve itself17:37
ograsnap feed --time=17:38
ogra??17:38
* zyga needs to finish this tomorrow17:43
zygatime to rest17:43
=== ijohnson|lunch is now known as ijohnson
mupPR 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:05
jdstrandkjackal: fyi: https://github.com/snapcore/snapd/pull/761618:07
mupPR #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
kjackalawesome, thank you jdstrand18:07
jdstrandkjackal: is that instance still up for me to verify ^ for the subPath fix?18:07
kjackalyes it is up18:08
jdstrandkjackal: do you mind if I check now real quick?18:08
kjackaljdstrand: no go ahead18:08
kjackaljdstrand: 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=018: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=018: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=018:09
kjackaltrying to see if gpu support is broken or not18:09
jdstrandkjackal: 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/gl18:11
jdstrandkjackal: perhaps you could use a layout for /etc/nvidia-container-runtime and have it look in the right place?18:11
jdstrandkjackal: but usr/lib/i386-linux-gnu/libnvidia-ml.so.390.116 is not what I expected18:12
jdstrandkjackal: that said, zyga has looked at gpu support a lot. he may be eod atm18:12
kjackalok, I will see what I can do18:12
kjackalthanks18:12
zygajdstrand: there are cases where we take it from hostfs18:13
jdstrandkjackal: I'll ping you in a minute when I'm done with the test18:13
jdstrand(subPath)18:13
zygakjackal: I'm EOD, please drop me a note with the details18:13
kjackalcool zyga, let me try some things first18:14
jdstrandkjackal: 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 denial18:22
jdstrandkjackal: so, cool :)18:22
jdstrandkjackal: logging out now18:22
kjackalmany thanks jdstrand18:23
jdstrandkjackal: np. when you see that merged, the next day's core from edge will have it18:23
kjackalgreat18:24
jdstrandkjackal: this should be in 2.4318:24
jdstrandkjackal: are you using the same snap name for strict mode microk8s? I ask cause I don't see any failed reviews for microk8s18:25
kjackallet me fire the lp builder18:26
jdstrandkjackal: I'll look at the snap.yaml in the instance and create a snap decl18:27
kjackaljust queued a build18:28
kjackalETA in 13 minutes (estimated)18:28
jdstrandkjackal: 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 now18:37
jdstrandkjackal: but you can ping my to manually approve in the meantime18:38
kjackalis 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
kjackaljdstrand: ^18:39
jdstrandkjackal: until the review-tools are in prod, every one. after that, it will remember18:40
kjackaljdstrand: cool, for now I can wait. We may want to push one in a couple of weeks18:41
jdstrandroadmr: can you pull 20191016-1840UTC? just doc changes and 2 override additions18:41
jdstrandkjackal: ^ that has the change I mentioned18:41
jdstrandkjackal: ok, it should all be in place then18:42
jdstrandroadmr: and hi! :)18:42
kjackalawesome18:42
roadmrjdstrand: hey! sure thing19:05
roadmrjdstrand: when can we chat re: stateful review-tools?19:06
* cachio afk19:06
jdstrandroadmr: tomorrow?19:07
jdstrandroadmr: and thanks! :)19:08
roadmrjdstrand: sure, can I go by your google calendar?19:08
roadmrjdstrand: looks like there's no such tag in the repo :/19:09
jdstrandroadmr: yes, please look at the calendar19:10
* jdstrand looks at tag19:10
roadmrthanks!19:11
jdstrandroadmr: 20191016-1840UTC ? I see it in https://git.launchpad.net/review-tools?h=master19:12
jdstrandhttps://git.launchpad.net/review-tools/tag/?id=20191016-1840UTC19:13
roadmrjdstrand: ah! it's there now19:13
jdstrandroadmr: oh hah, I pushed the tag, not the commit :)19:13
roadmrjdstrand: some refreshing shenanigans, it seems19:13
roadmr😆19:13
ijohnsonjdstrand: 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
mupPR #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
jdstrandijohnson: yep19:14
jdstrandijohnson: hence my comment about refresh19:14
ijohnsonis that intentional ?19:14
jdstrandijohnson: did you see the commit message? I avoided SNAP_REVISION after all thinking that refresh would be potentially problematic19:15
ijohnsonjdstrand: I did I just don't quite understand the reasoning19:15
ijohnsonif they needs mounts to persist across refreshes then they should definitely use common19:16
ijohnsonmy 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 revisions19:16
ijohnsonif that was the case they could then transition to mounting things on top of $SNAP_DATA, which would be per revision19:17
jdstrandijohnson: I suspect that, like lxd, k8s will use the mechanism to not restart itself so pods aren't blown away19:17
ijohnsonyes that is probably true19:17
ijohnsondoes lxd have rules to do mounts across revisions in $SNAP_DATA though?19:17
jdstrandijohnson: lxd's profile allows transitioning to unconfined so there are no rules19:18
jdstrandijohnson: (beyond allowing it to get to that point)19:18
ijohnsonah right yeah19:18
ijohnsonlxd is special19:18
jdstrandijohnson: iirc, this was why common/ was enforced in kubernetes-support19:18
jdstrand(ie, the refresh point, not lxd :)19:19
jdstrandit's been a while since I wrote those rules, but iirc I was enforcing best practice19:19
ijohnsonjdstrand: perhaps given this we should just stick with common19:19
jdstrandijohnson: ok. easy enough. we can rediscuss if there is a use case for SNAP_DATA19:20
ijohnsonsure sounds good19:20
ijohnsonfeel free to force push if you want to remove that most recent commit19:20
jdstrandijohnson: 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 posterity19:39
ijohnsonjdstrand: ack19:47
mupPR 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>19:56
jdstrandroadmr: fyi, https://dashboard.snapcraft.io/snaps/microk8s/feedback/ has been giving a 504 for... 30 minutes?20:03
jdstrandroadmr: ish20:03
roadmrthanks jdstrand, fighting some minor fires store-side20:03
jdstrandroadmr: ok, just fyi. I'm not blocked20:04
roadmrright - seems to be widespread, get the same for e.g. core20:04
jdstrandroadmr: fyi, it is not *all* snaps. eg https://dashboard.snapcraft.io/snaps/telegraf/feedback/ is fine20:07
roadmrah, interesting20:18
roadmrjdstrand: nessita found it's an n+1 queries problem which will hit snaps with many uploads20:41
jdstrandinteresting20:41
jdstrandroadmr: I did see this with https://dashboard.snapcraft.io/snaps/app-outlet/ (notice no feedback/ in url) and it sometimes displayed20:42
nessitajdstrand, if you browse by revision, it should work20:42
nessitajdstrand, like https://dashboard.snapcraft.io/snaps/core/revisions/<revision>/20:43
jdstrandroadmr, 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 comment20:44
jdstrand(yesterday)20:44
jdstrandcontinuing that today20:45
jdstrandnessita: fyi, 504 with https://dashboard.snapcraft.io/snaps/app-outlet/revisions/1/20:47
nessitajdstrand, odd, it opened quite fast for me. Reloading...20:50
nessitahum 504 now20:50
nessitaroadmr, maybe we have some sca fe with troubles?20:50
jdstrandnessita: that one didn't have many revisions iirc20:50
roadmrnessita: could be, or maybe the app units, one could have landed on an evil host20:51
jdstrandnessita: I just saw one with 73 revisions that was fine20:51
nessitajdstrand, 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
nessitajdstrand, 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:51
jdstrandnessita: I see it now20:52
jdstrand(30 revisions)20:52
nessitajdstrand, we may issues with one of the fe units or app units, let us look20:53
roadmrnessita: I would think app/backend units, the fe is just haproxy, shouldn't have any problems plus hasn't changed in a while20:54
nessitaroadmr, jdstrand moving this to our private channel20:56
roadmr+120:56
nessitaroadmr, jdstrand #snastore please20:56
nessitaroadmr, jdstrand #snapstore please20:56
mupPR snapcraft#2753 opened: cli: use click utilities for registering on push <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2753>21:34
mupPR 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>22:00

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