[06:06] <mborzecki> morning
[06:52] <mborzecki> zyga: hey
[06:52] <mborzecki> you around?
[07:05] <zyga> mborzecki sure, can I help you somehow?
[07:05] <mborzecki> zyga: nvidia biarch is borked
[07:05] <mborzecki> let me show you where
[07:07] <mborzecki> zyga: https://github.com/snapcore/snapd/blob/master/cmd/snap-confine/mount-support-nvidia.c#L276 the last thing that sc_mkdir_and_mount_and_glob_files() does is remount the target directory ro, so sc_glob_files_only() then fails with filesystem is read only https://paste.ubuntu.com/26523173/
[07:12] <mborzecki> zyga: https://github.com/snapcore/snapd/blob/master/cmd/snap-confine/mount-support-nvidia.c#L276 the last thing that sc_mkdir_and_mount_and_glob_files() does is remount the target directory ro, so sc_glob_files_only() then fails with filesystem is read only https://paste.ubuntu.com/26523173/
[07:14] <zyga> aha
[07:14] <zyga> this makes sense, feel free to move that outside so that things work correctly
[07:15] <mborzecki> ok
[07:15] <zyga> thank you for spotting that
[07:15] <zyga> it feels like a RC3 though
[07:17] <mborzecki> yeah, built a new rig, put an old nvidia card inside since prices are absurd, tried to debug the spotify issue some guy raised in a forum and ended up debugging nvidia issues instead :)
[07:23] <zyga> :-)
[07:23] <zyga> it's good that you are using nvidia though, most of the laptop developers don't notice that
[07:29] <zyga> hey mvo
[07:29] <mvo> hwy zyga
[07:29] <mvo> hey
[07:34] <mborzecki> mvo: morning
[07:34] <mvo> good morning mborzecki
[07:49] <mup> PR snapd#4603 opened: cmd/snap-confine: fix read-only filesystem when mounting nvidia files in biarch <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/4603>
[07:49] <mborzecki> zyga: ^^
[07:49] <zyga> yep
[07:49] <zyga> looking
[07:49] <mborzecki> sadly there are no tests for this code, so i just tested it locally
[07:50] <mborzecki> the good news is that i can run snaps again :)
[07:51] <mborzecki> ohmygiraffe works
[07:54] <kalikiana> good morning, snappers
[07:56] <mborzecki> kalikiana: morning
[07:59] <mvo> mborzecki: oh, nice. something for 2.31?
[08:02] <zyga> mborzecki +1
[08:03] <zyga> we need to ensure that some round of testing on proprietary nvidia driver is done for 2.31
[08:04] <mup> PR snapd#4601 closed: tests: update kill-timeout focused on making tests pass on boards <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4601>
[08:06] <mvo> zyga: is this 2.31 cherry-pick material?
[08:06] <zyga> yes
[08:06] <zyga> definitely
[08:07] <mborzecki> i have an old 9600gt card, so can check with 340xx driver, no vulkan though
[08:07] <zyga> mvo note that it affects _biarch_ systems
[08:07] <mvo> zyga: so fedora/solus?
[08:07] <mborzecki> mvo: yes, otherwise it's not possible to assemble a mount on on my arch box
[08:07] <zyga> mvo yes
[08:07] <mvo> ta
[08:07]  * mvo looks
[08:11] <mvo> mborzecki: 4598 is ready right? it has a +1 from neal
[08:11] <mborzecki> mvo: yes
[08:13] <mup> PR snapd#4598 closed: packaging: create /var/lib/snapd/lib/{gl,gl32,vulkan} as part of packaging <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4598>
[08:17] <mvo> mborzecki: fwiw, once the tests are green I will merge 4603, my commment about comments can always added later
[08:19] <mborzecki> mvo: ok
[08:20] <mvo> mborzecki: I also noticed this is already used in the code without much comment so :)
[08:20]  * mvo has a wtf moment when he noticed that unsquashfs will return "0" if the unpack fails
[08:21] <zyga> ....
[08:21] <zyga> well
[08:21] <zyga> yeah, that is weird
[08:21] <mvo> zyga: my thoughts exactly :)
[08:22] <mvo> zyga: I will send a patch to upstream and to our ppa
[08:22] <mup> PR snapd#4602 closed: tests: use root path to /home/test/tmp to avoid lack of space issue <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4602>
[08:22] <mvo> zyga: but holly cow
[08:22] <mborzecki> mvo: is there anything special about the file?
[08:23] <zyga> maybe it's _just_ a bug
[08:23] <mvo> mborzecki: which file? the squashfs file that is unpacked? I unpacked it into a tmpfs
[08:23] <mvo> mborzecki: and that had not enough space
[08:23] <mborzecki> mvo: right, the squashfs file
[08:23] <mborzecki> oh
[08:23] <mvo> mborzecki: sergio figured out this is what broke the prepare-image test on arm64
[08:24] <mvo> mborzecki: it uses a tmpfs on /tmp which is not big enough to hold the tmp files we need to prepare the image (the armhf kernel must be unpacked there)
[08:24] <mvo> zyga: it probably is
[08:24] <mborzecki> mvo: btw. does it try to clean up the fiels it extracted?
[08:25] <mborzecki> s/fiels/files/
[08:25] <mvo> mborzecki: I doubt it, let me check
[08:25] <mvo> mborzecki: no
[08:27] <mborzecki> right, don't know what i was expecting
[08:28] <mborzecki> it'd be funny to put squashfs through afl and see what comes out
[08:29] <zyga> afl?
[08:29] <mborzecki> american fuzzy loop
[08:30] <mborzecki> http://lcamtuf.coredump.cx/afl/
[08:30] <zyga> oh, cool
[08:32] <zyga> anyone wants to do a quick HO test with me
[08:38] <pstolowski> mornings!
[08:38] <zyga> hey hey
[08:50] <mborzecki> tak, tak
[08:50] <mborzecki> wrong window :)
[08:50] <mborzecki> pstolowski: morning
[08:51] <zyga> mborzecki tak tak, wrong window
[08:51] <mborzecki> mvo: restarted the build in 4603, looked like an unrelated failure
[08:51] <mvo> ok
[09:17] <Chipaca> mvo: o/
[09:17] <Chipaca> mvo: looking at 4600, do config changes always end in a *json.RawMessage?
[09:18] <mvo> Chipaca: I'm not 100% certain, that sounds like something pstolowski may know
[09:18] <Chipaca> pstolowski: ^?
[09:19] <Chipaca> pstolowski: in config's transaction.go, do config changes always finish in a *json.RawMessage?
[09:19] <mvo> Chipaca: fwiw, the PR is a bit of a RFC for gustavo if this is validation we want or not
[09:19] <Chipaca> mvo: ack
[09:19] <mvo> ogra_: did things work once the typo in the core.service.rsyslog.disable=true was fixed btw?
[09:19] <mvo> Chipaca: I mean, don't kill yourself over the review :) it might be in vain :(
[09:20] <mvo> Chipaca: even though I think it would be good to have this validation
[09:21] <pstolowski> Chipaca, mvo yes
[09:21] <Chipaca> pstolowski: ack
[09:21] <Chipaca> pstolowski: why are they *json.RawMessage, btw? instead of just json.RawMessage i mean
[09:22] <milp_media> ogra_: you might be right, but that whole ubuntu sso thing seems fishy to me. i dont want to be dependant on any service that is not hosted on my own devices :/
[09:22] <pstolowski> Chipaca, dunno. I haven't implemented that
[09:22] <mvo> zyga: I pushed a PR upstream for unsquashfs, I wonder what we should do in the meantime, we could do crzay stuff like compare the targetdir with the unsquahfs -ls output but that sounds slightly crazy. but maybe patience is a virtue and I wait for upstream
[09:22] <pstolowski> Chipaca, kyrofa might remember ;)
[09:23] <Chipaca> pstolowski: blame says gustavo
[09:23] <Chipaca> :-)
[09:23] <Chipaca> i'll ask
[09:23] <pstolowski> Chipaca, okay. blame has better memory than me, that's for sure
[09:23] <mvo> Chipaca: ta
[09:25] <zyga> mvo: what is the worst thing that can. happen if we use broken unsquashfs?
[09:26] <mvo> zyga: we unpack a kernel on arm, no space left on /boot but we don't detect the failure, continue and the boot fails (and reverts)
[09:26] <mvo> zyga: so hopefully its not too bad
[09:27] <mvo> zyga: but still fugly that we don't detect it when it happens
[09:27] <Chipaca> mvo: zyga: we found a bug in unsquashfs?
[09:27] <mborzecki> mvo: shall i merge 4603?
[09:27] <mvo> Chipaca: bug/feature/missing-thing - unsquashfs does not report anything on unsqushfs failures
[09:27] <mvo> Chipaca: like exit(0)
[09:28] <mvo> Chipaca: is what it will always do
[09:28] <Chipaca> ah
[09:28] <mvo> Chipaca: which is slightly annoying, we had this bug on arm64 that prepare-image failed and there was a kernel missing
[09:28] <mvo> Chipaca: and it turns out, tmpfs was too small and unsquashfs told the world: "failed..." on stdoutbut never reported it via exitcode
[09:29] <mvo> mborzecki: let me look
[09:29] <mvo> mborzecki: yes please, I will cherry-pick
[09:29] <mvo> afterwards
[09:29] <zyga> mvo: could we bundle fixed unsquashfs in the package?
[09:29] <mvo> zyga: yeah, I think that would work
[09:29] <mup> PR snapd#4603 closed: cmd/snap-confine: fix read-only filesystem when mounting nvidia files in biarch <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/4603>
[09:30] <mvo> zyga: I was just thinking we could do a strings.Contains("failed", strings.ToLower(string(unsquashfsOutput)))"
[09:30] <zyga> I mean, the risk is rather small because we check for hashes and the store does validation as well
[09:30] <mvo> zyga:as a first approximation
[09:30] <zyga> mvo, a simple "failed" thing works too, yeah
[09:30] <mvo> zyga: I think I will do that and hope for upstream, once upstram merges it I can SRU it at least into the ubuntu-world
[09:31] <mvo> zyga: anyway, thanks for the brainstorm :)
[09:31] <zyga> :-)
[09:31]  * mvo is also slightly shocked that there is no open upstream bugreport about this
[09:32] <mup> PR snapd#4604 opened: cmd/snap-confine: create lib/{gl,gl32,vulkan} under /var/lib/snapd and chown as root:root <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/4604>
[09:33] <mborzecki> zyga: ^^ this a workaround for the problem with missing /var//lib/snapd/lib that we had on fedora
[09:33] <zyga> reading
[09:34] <zyga> mborzecki one comment
[09:36] <mborzecki> zyga: uhh right :/ silly me
[09:43] <Chipaca> pstolowski: want me to add the roundrobin writer to strutil?
[09:45] <mborzecki> zyga: mvo: is 4604 a 2.31 material too?
[09:45] <zyga> yes
[09:45] <pstolowski> Chipaca, no, thanks, I'm on it
[09:45] <Chipaca> pstolowski: ok
[09:46]  * Chipaca carries on down the PRs
[09:46] <mvo> mborzecki: that is only needed for re-exec?
[09:46] <mvo> mborzecki: and bi-arch? or more generic?
[09:48] <mborzecki> mvo: bi- and multi- arch, same thing as in #4598 but works around packaging errors
[09:48] <mup> PR #4598: packaging: create /var/lib/snapd/lib/{gl,gl32,vulkan} as part of packaging <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4598>
[09:53] <ogra_> mvo, i only tesetd on a pi and it was fine there (on the actual board i'm currently trying to get a grub-uboot implementation going, so have no booting OS atm) ... it was just the typo (and i checked, it wasnt there in a former local iteration of the gadget which explains why it worked before)
[09:54] <mvo> ogra_: thanks
[09:59] <mvo> ogra_: fwiw, we are exploring if we can add error reporting for this easily (the general case is harder but we might get away with somehting core specific to catch unknown options)
[09:59] <ogra_> mvo, that would be awesome
[09:59] <mvo> ogra_: pr #4600 iirc
[09:59] <mup> PR #4600: configstate: validate known core.* options <Created by mvo5> <https://github.com/snapcore/snapd/pull/4600>
[10:00] <ogra_> nice round number
[10:00]  * mvo back to squashfs
[10:00] <mvo> ogra_: heh, indeed
[10:00] <ogra_> heh. you too ?
[10:00]  * ogra_ is banging his head against grubs squashfs implementation on arm
[10:01] <mvo> ogra_: I am just fixing silly issue around unsquashfs
[10:01] <ogra_> sadly the u-boot api and grub dont want to play together well ..
[10:01] <mvo> ogra_: autsch!
[10:01] <ogra_> would be so nice to have the arm boards read from the snaps diurectly as well ..
[10:02] <ogra_> (beyond being able to use secureboot from grub)
[10:03] <mvo> ogra_: oh yes! there were patches for pre-v4 squashfs for u-boot, I guess this never went anywhere? or is the plan to use uboot->grub anyway?
[10:04] <ogra_> mvo, well, i have to implement a secureboot solution, one option thats known to work is using a uboot.FIT image, but that means essentially all contents of system-boot need to go into a blobby signed image (doesnt work well with split-initrd or de-coupling of any snaps if tehy ship separate parts for system-boot) ...
[10:05] <ogra_> the other option is grub ... and use signed single files directly from the snaps
[10:05] <mvo> ogra_: so uboot chainloads to grub?
[10:06] <mvo> ogra_: that sounds interessting for the main borads as well if this works well
[10:06] <ogra_> well ... kind of ... uboot keeps running and provides an api to grub ... so it does all the HW heavylifitng ... grub then just uses syscalls to talk to uboot
[10:07] <ogra_> the prob is the api ... it is a) very young and b) the board i'm currently using has a 2015 u-boot tree as base ... grub works very well until i try to load anything from any disk
[10:08] <ogra_> so sadly i might have to turn down the idea :/
[10:08] <ogra_> (i can actually use the loopback feature and list contents of snaps etc ... just the loading explodes completely)
[10:09]  * mvo nods
[10:10]  * Chipaca ~> afk (physio)
[10:25] <mup> PR snapd#4605 opened: snap: detect unsquashfs write failures <Created by mvo5> <https://github.com/snapcore/snapd/pull/4605>
[10:31] <zyga>  mvo one comment there
[10:42]  * Chipaca tries again to go to physio
[10:44] <mvo> zyga: ta
[10:44] <zyga> mvo nothing strong, just could save (perhaps) some memory
[10:45] <mvo> zyga: easier to read this way too, I like it
[10:45] <mvo> zyga: also Chipaca comment is a good one, the amount of output can be huge (because it does not stop and will report and error for every file). so a special writer seems like the better option
[10:50] <zyga> Cloning into '/home/gopath/.cache/govendor/golang.org/x/crypto'...
[10:50] <zyga> error: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function.
[10:50] <zyga> I saw this a few times today
[10:56] <zyga> brb
[11:18] <zyga> mvo, can we merge 4576?
[11:36] <mup> PR snapd#4596 closed: osutil: allow using many globs in EnsureDirState <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4596>
[11:46] <mup> PR snapd#4606 opened: snap: use custom unsquashfsStderrWriter for unsquashfs error detection <Created by mvo5> <https://github.com/snapcore/snapd/pull/4606>
[11:46] <mvo> Chipaca: I pushed a potential fix using the custom writer -^
[11:46] <mborzecki> any idea why snap app services are enabled (but not started) in doLinkSnap() while app socket services are enabled and started in statSnapServices() ?
[11:47] <oSoMoN> sergiusens, hey, I was talking to Ken about building snaps on 18.04 and he said there's a bug in snapcraft that makes those not run on other ubuntu releases
[11:47] <oSoMoN> is there a bug report I can subscribe to?
[11:49] <seb128> sergiusens, he also said you stated you would have it fixed by past-thursday :)
[11:58] <pedronis> mborzecki: socket services are recent,  where do we start then app services?
[11:59] <mup> PR snapd#4589 closed: many: remove "content" argument from snaptest.MockSnap() <Created by mvo5> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4589>
[12:00] <zyga> Chipaca, I think we should have fewer implementations of mkdirallchown
[12:00] <zyga> +1 on the patch there but I think we could use the secure variant instead
[12:00] <zyga> chipaca we could make osutil/secure/ package and stick them there
[12:01] <mborzecki> pedronis: in 'start-snap-services', but they get enabled in link-snap
[12:01] <mborzecki> pedronis: the sockets are enabled and started in start-snap-services
[12:02] <mup> PR snapd#4586 closed: cmd/snap: add completion conversion helper to increase DRY <Created by chipaca> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4586>
[12:04] <zyga> Chipaca question on 4581
[12:05] <pedronis> zyga: what do you mean with empty there?
[12:05] <zyga> empty byte array
[12:06] <pedronis> I don't think I understand the question, maybe John will
[12:06] <zyga> pedronis maybe I misunderstood how that works, I'll talk to john
[12:07] <pedronis> it's a shortcut, in the case there's no config at all (for any snap) and we are setting to the nil config for this one
[12:07] <pedronis> there's nothing to do
[12:07] <pedronis> basically
[12:08] <zyga> I think I understand that now, snapcfg is the value we want to set, not the data store
[12:14] <zyga> mvo is 2.31 something that is now living in a branch?
[12:14] <zyga> I'd like to merge 1st 2.32 PRs that are not ready for 2.31 rc3
[12:21] <mvo> zyga: 2.31 is in a branch
[12:21] <mvo> zyga: only cherry-picks
[12:22] <zyga> excellent
[12:22] <zyga> I'm chopping various things for sun profiles
[12:22] <zyga> and trying to figure out how to avoid some duplication and annoying maintenance
[12:22] <zyga> and since it's not a 2.31 thing anymore I'm doing some improvements to how apparmor backend works
[12:24] <mvo> zyga: yeah, you can go wild in master
[12:25] <zyga> cool
[12:26] <zyga> jdstrand, 4590 needs your +1 then, I'll land the follow-ups on top so that we can see I didn't break what we got working last week
[12:26] <zyga> and the branches can be nicer when we're not after minimal diffs anymore
[13:02] <mup> PR snapd#4607 opened: wrappers: cleanup enabled service sockets <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/4607>
[13:29]  * Son_Goku sighs
[13:30] <Son_Goku> I wish people would stop counting on LSM stacking to save their butts
[13:30] <zyga> hey Son_Goku
[13:31] <Son_Goku> zyga: hey
[13:51]  * kalikiana going to have lunch, back in a bit
[14:01] <pedronis> Chipaca: not super urgent but a review of #4497 would be appreciated
[14:01] <mup> PR #4497: many: make rebooting of core on refresh immediate, refactor logic around it <Created by pedronis> <https://github.com/snapcore/snapd/pull/4497>
[14:01] <pedronis> niemeyer: for later, the PR I mentioned is: #4582
[14:01] <mup> PR #4582: many: at seeding try to capture cloud information into core config under "cloud" <Created by pedronis> <https://github.com/snapcore/snapd/pull/4582>
[14:09] <niemeyer> pedronis: Thanks!
[14:23] <flexiondotorg> zyga: Who can we work with to get snapd updated in openSUSE?
[14:24] <flexiondotorg> Sadly most desktop application snaps are wearing their sad face when run on openSUSE right now.
[14:27] <ikey> "lets turn that frown upside down"
[14:27] <zyga> flexiondotorg me or anyone who will do the packaging
[14:27]  * ikey remembers his motivational training
[14:28] <flexiondotorg> zyga: Any chance you could find the time to rev snapd in openSUSE to 2.30?
[14:29] <flexiondotorg> It's currently 2.27 so doesn't know about the desktop and desktop-legacy interfaces.
[14:29] <flexiondotorg> ikey: o/
[14:29] <ikey> \o
[14:30] <zyga> flexiondotorg let me try tomorrow and if not let's find somoene
[14:33] <zyga> flexiondotorg I have opensuse ready and booted, just some higher priority things
[14:34] <zyga> flexiondotorg we don't have anyone on it that's dedicated
[14:34] <zyga> flexiondotorg I can review PRs to that repo
[14:35] <flexiondotorg> Thanks zyga
[14:45] <Chipaca> mvo: indeed partial writes make this a lot harder
[14:45] <Chipaca> mvo: sorry :-) i'll still get something up for you to look at in case you like it
[14:46] <mvo> Chipaca: yeah, its a bit annoying
[14:46] <mvo> Chipaca: sure, happy to have a look how you solved it
[14:47] <mvo> Chipaca: that reminds me, my test case needs an upper case "Failed to..."
[14:51] <mup> PR snapd#4582 closed: many: at seeding try to capture cloud information into core config under "cloud" <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/4582>
[14:54] <kalikiana> re
[15:01] <Chipaca> mvo: https://pastebin.ubuntu.com/26524851/ -- i'm not at all sure this is actually simpler (but i know how to make it more complex :) )
[15:01]  * Chipaca -> school run
[15:09] <mvo> Chipaca: in a meeting right now, let me read it but thanks a bunch for looking into this in any case
[15:10] <mvo> Chipaca: it looks a bit shorter at least
[15:15] <zyga> jdstrand hello, how was your flight home/
[15:17] <jdstrand> zyga: uneventful :)
[15:17] <jdstrand> zyga: I saw the request for 4590 re-review
[15:17] <zyga> just as it should be
[15:17] <jdstrand> zyga: exactly :)
[15:17] <zyga> I didn't do the full thing there, just added your FIXME comments
[15:18] <zyga> and confirmed with mvo that release is branched off and can merge things
[15:18] <zyga> I'm chopping my sun profile work into smaller pieces now
[15:20] <jdstrand> cool
[15:20] <jdstrand> let me look at 4590 real quick then
[15:25] <zyga> cool, thanks!
[15:27] <zyga> oh ... cooooool :)
[15:27]  * zyga just realised something very useful :)
[15:36] <pedronis> Chipaca: did you do anything about saving screenshots on install? or still pending
[15:36] <Chipaca> pedronis: that's phase 2
[15:36] <Chipaca> pedronis: and requires answers to Questions
[15:36] <pedronis> Chipaca: phase 2 of what?
[15:36] <Chipaca> pedronis: of snapshots
[15:37] <Chipaca> pedronis: phase 1 is just manual
[15:37] <pedronis> Chipaca: I said screenshots, not snapshots
[15:37] <pedronis> something discussed in NYC
[15:37] <Chipaca> pedronis: questions such as "if you snapshot rev2 and upgrade to rev3 and restore what do you even restore"
[15:37] <Chipaca> pedronis: /o
[15:37] <Chipaca> pedronis: I did nothing about any metadata
[15:37] <pedronis> ok, thx
[15:37] <Chipaca> icons, screenshots, title, description, etc etc etc
[15:38] <Chipaca> all the same bag imo
[15:38] <pedronis> Chipaca: because we had some discusions about saving more, saving differently, right?
[15:38] <pedronis> but hasn't happend, ok
[15:39] <Chipaca> pedronis: we decided those for all attributes that were per snap not per rev the store is authoritative, and we should keep a cache locally
[15:39] <Chipaca> correct, no progress
[15:40] <kyrofa> pstolowski, Chipaca, yeah I got nothing on the *json.RawMessage
[15:40] <Chipaca> kyrofa: thank you for reminding me :-D
[15:40] <pstolowski> :)
[15:40] <Chipaca> niemeyer: in configstate/config we use *json.RawMessage everywhere instead of json.RawMessage. Why?
[15:40] <Chipaca> niemeyer: (asking you because git blames you)
[15:44] <mvo> Chipaca: he is in a meeting
[15:44] <Chipaca> mvo: i am in no rush :-)
[15:56]  * cachio__ lunch
[16:06] <mup> PR snapd#4608 opened: cmd/snap-confine: allow snap-update-ns to chown things <Created by zyga> <https://github.com/snapcore/snapd/pull/4608>
[16:06] <zyga> jdstrand can you please look at ^^
[16:06] <zyga> it's a two liner
[16:09] <jdstrand> zyga: done
[16:09] <zyga> mvo ^
[16:10] <zyga> I'll work on updates to the tests to check relevant new features as a user, just ran into this by dogfooding
[16:12] <niemeyer> Chipaca: Heya
[16:12] <niemeyer> Chipaca: There is/was a serious bug about it in the json package
[16:12] <niemeyer> Chipaca: It misbehaves wildly in some situations
[16:12] <Chipaca> ah
[16:13] <Chipaca> niemeyer: i see
[16:13] <niemeyer> Chipaca: Let me see if I can find some quick info
[16:13] <Chipaca> niemeyer: i use json.RawMessage directly in snapshots; let me know if i'll get bitten
[16:13] <Chipaca> niemeyer: or taht :-D
[16:14] <niemeyer> Chipaca: https://github.com/golang/go/issues/14493, probably
[16:18] <pedronis> ah, that
[16:36] <zyga> + systemctl stop snapd.service snapd.socket
[16:36] <zyga> Job for snapd.service canceled.
[16:50] <Chipaca> niemeyer: it's fun reading through that issue how people flip from "meh" to "WTF LETS FIX THIS" as they forget and then get bitten and waste time trying to figure it out
[16:52] <Chipaca> niemeyer: OTOH, it's fixed, so ¯\_(ツ)_/¯
[16:53] <zyga> so "it will be out in debian 10"
[16:53] <zyga> in X years
[16:54] <Chipaca> zyga: it's not go's fault ubuntu always ships a go that is no longer supported by them
[16:55] <Chipaca> much as it irks me, it really isn't :-)
[16:55] <Chipaca> and this particular bug has been fixed since 1.8, which is a year old already
[17:06] <Chipaca> zyga: what do you mean with "fewer implementations of mkdirallchown"?
[17:07] <zyga> Chipaca we have one in snap-update-ns that's tailored for security
[17:07] <Chipaca> zyga: ahh... i forget
[17:07] <Chipaca> zyga: I can take on unifying those two
[17:07] <Chipaca> i see no reason not to use the securer one everywhere :-)
[17:07] <zyga> yeah
[17:08] <zyga> how does osutil/secure sound?
[17:08] <zyga> those would require jamie review to change
[17:08] <zyga> but we could just move the current code there first
[17:08] <zyga> have a look, maybe something about it is silly and unreasonable, maybe not
[17:08] <Chipaca> zyga: sounds like a good, reasonable plan
[17:09] <Chipaca> zyga: but, going back a bit, you said "+1 to my changes there", but didn't actually +1 :-D
[17:09] <zyga> I will add secure symlink next, definitely this week
[17:09] <zyga> oh, sorry
[17:09] <zyga> looking
[17:09] <Chipaca> :-)
[17:09] <Chipaca> zyga: also your comment on #4581 feels reviewish but doesn't have a +1 either
[17:09] <mup> PR #4581: overlord/configstate/config: make SetSnapConfig delete on empty <Created by chipaca> <https://github.com/snapcore/snapd/pull/4581>
[17:09] <Chipaca> zyga: #4587 is the other one fwiw
[17:09] <mup> PR #4587: osutil: make MkdirAllChown clean the path passed in <Created by chipaca> <https://github.com/snapcore/snapd/pull/4587>
[17:10] <mup> PR snapd#4581 closed: overlord/configstate/config: make SetSnapConfig delete on empty <Created by chipaca> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4581>
[17:10] <mup> PR snapd#4587 closed: osutil: make MkdirAllChown clean the path passed in <Created by chipaca> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4587>
[17:11] <Chipaca> yuss
[17:12] <niemeyer> Chipaca: Yeah, it's fixed.. we just need to check if all the versions of Go we care about are fixed too
[17:12] <niemeyer> Chipaca: I would guess not
[17:12] <Chipaca> niemeyer: 1.8+
[17:12] <niemeyer> Yeah, so risky still
[17:13] <Chipaca> I'll update snapshots to use *json.RawMessage for now then
[17:13] <Chipaca> as part of the rebase
[17:13] <Chipaca> just one more pr to land for that to happen :-)
[17:13] <zyga> Chipaca one question posted on 4585
[17:17] <Chipaca> zyga: i confess to not understanding your question :-(
[17:18] <Chipaca> zyga: care to expand it a little bit?
[17:18] <zyga> sure
[17:18] <zyga> sorry about tyhat
[17:18] <zyga> is there any practical difference between using `inst` there? (instead of the current code that does `&inst`)
[17:18] <zyga> inst is a function anyway, right?
[17:19] <Chipaca> zyga: no, inst is the instruction, a thing sent by the user that got decoded a little bit further up
[17:19] <Chipaca> zyga: op is the function
[17:19] <zyga> ah, sorry then
[17:20] <Chipaca> zyga: you couldn't do &foo if foo was a function
[17:20] <Chipaca> afaik
[17:21] <Chipaca> ah yes you can
[17:21] <Chipaca> eh
[17:21] <pstolowski> Chipaca, no run-hook in snap topical tasks
[17:22] <Chipaca> pstolowski: i know it's not there currently; should it be?
[17:22] <Chipaca> pstolowski: or does hookstate have its own conflict thing?
[17:24] <Chipaca> I suspect you can currently install, remove, refresh snaps that are in the middle of running a hook
[17:26] <mup> PR snapd#4609 opened: interfaces/apparmor: use a helper to set the scope <Created by zyga> <https://github.com/snapcore/snapd/pull/4609>
[17:26] <zyga> Chipaca ^
[17:26] <Chipaca> ack
[17:27] <Chipaca> zyga: currently looking at one for bboozzoo
[17:27] <zyga> thanks!
[17:27] <kalikiana> grrr, python env mocking is tedious
[17:27] <kalikiana> will have to finish that tomorrow
[17:27] <zyga> mocking environment?
[17:27] <zyga> just mock os.environ and os.getenv (probably?)
[17:29] <kalikiana> zyga: I'm trying to check that a variable is unset via `assert_has_calls` which by default expects my actual env. So I mocked `os.environ` with a {} stanza. Except other calls that aren't related to this now return magic mock values for variables I'm *not* setting there
[17:29] <kalikiana> Somehow the almost-empty mock env has values in it that I did not set
[17:30] <pstolowski> Chipaca, hookmgr has a SetBlocked handler which only prevents other hooks from running (there can be one hook at a time). but what you're asking seems entirely possible unless I'm missing something...
[17:30] <zyga> mocking is tricky, perhaps the import path matters, you may see unmocked environment depending on "how" you look (what's the imported object)
[17:30] <kalikiana> zyga: Like MagicMock<name=environ.get() id=12345>
[17:30] <zyga> how did you mock environ?
[17:30] <kalikiana> Those are the values that show up in other places now
[17:31] <kalikiana> zyga: patch('os.environ')
[17:31] <zyga> is anything importing environ locally? patch has its limits as to what it can reference
[17:32] <kalikiana> zyga: there's os.environ.copy, which works as expected with my {} value. the other code is using os.getenv
[17:32] <pstolowski> Chipaca, so yes, maybe run-hook should be there. I can look take this
[17:33] <kalikiana> zyga: the os.getenv seems to be getting those MagicMock values now
[17:34]  * zyga dinner
[17:35] <pedronis> Chipaca: are you adding changes that run only hooks?  so far  hooks are taken care by other conflicts, because usually there are other tasks that will conflict
[17:35] <pedronis> like link-snap
[17:36] <pstolowski> pedronis, but link-snap can be Done, and configure snap runs afterwards
[17:36] <pedronis> pstolowski: no
[17:36] <pedronis> the normal conflict logic
[17:36] <pedronis> is about full changes
[17:36] <pedronis> not single tasks
[17:36] <pedronis> (unless it was changed recently)
[17:36] <kalikiana> elopio: in case you happen to have any ideas on that... ^^ otherwise I'll be digging in the docs tomorrow morning over my coffee
[17:37]  * kalikiana wrapping up for today
[17:37] <zyga> kalikiana I can tell you why this happens
[17:37] <zyga> and perhaps how to fix it
[17:37] <zyga> tomorrow would be best :)
[17:38] <pedronis> pstolowski: it's not about link-snap being done, it's about the full change that has link-snap in it being done
[17:38] <kalikiana> zyga: Awesome. Let's chat tomorrow then
[17:38] <pstolowski> pedronis, ah oh you'r right
[17:38] <pedronis> pstolowski: your new code is a bit different (different tradeoffs there)
[17:39] <pedronis> and concerns onyl connect/disconnect
[17:39] <pstolowski> pedronis, yes, that's what put me on a wrong track
[17:39] <pstolowski> so yeah, we should be good, sorry for confusion Chipaca
[17:39] <Chipaca> pstolowski: now i'm wondering about configure hooks
[17:40] <pedronis> they might die, saying that snap is not there anymore I suppose
[17:42] <pedronis> Chipaca: we dont' seem to do conflict checks around configure/Configure
[17:44] <pedronis> so there might be an issue there
[17:44] <pedronis> in some corner cases
[18:05] <niemeyer> Stepping out for a while
[18:09] <mup> PR snapd#4609 closed: interfaces/apparmor: use a helper to set the scope <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4609>
[18:15] <mup> PR snapd#4608 closed: cmd/snap-confine: allow snap-update-ns to chown things <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4608>
[18:15] <zyga> Chipaca small follow-up if you want to look
[18:15] <zyga> jdstrand ^
[18:16] <zyga> jdstrand this is what it would look like, interfaces could now add sun snippets
[18:16] <mup> PR snapd#4610 opened: interfaces/apparmor: early support for snap-update-ns snippets <Created by zyga> <https://github.com/snapcore/snapd/pull/4610>
[18:16] <zyga> jdstrand we could even use it to reduce permissions on snap-update-ns (the base set0
[18:16] <zyga> so even things like content interface would be explicit
[18:17] <zyga> jdstrand if you can look at that (design, not security related) and +1 that I'll carry on with the backend parts that generate it
[18:17] <zyga> jdstrand and obviously the layout parts
[18:28] <jdstrand> zyga: ack
[18:30] <mup> PR snapd#4585 closed: daemon: refactor snapFooMany helpers a little <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/4585>
[18:58] <kyrofa> jdstrand, dumb question: can you give me a use-case for the fuse-support interface?
[18:59] <kyrofa> jdstrand, someone is suggesting we add it to the nextcloud snap, but I'm not certain I understand what it does
[19:11] <jdstrand> kyrofa: fuse allows you to mount userspace filesystems: https://www.kernel.org/doc/Documentation/filesystems/fuse.txt
[19:14] <jdstrand> kyrofa: the interface has a number of limitations. it would be best to understand the specific use case the request is trying to support
[19:15] <kyrofa> jdstrand, I haven't quite figured that out, yet. I'll get back in touch once I have a better picture
[19:22] <kyrofa> jdstrand, looks like someone is `rclone mount`ing into the data dir and hoping Nextcloud can use it
[19:23] <jdstrand> kyrofa: the interface allows for performing the mount, not consuming one
[19:24] <jdstrand> I suggest instead the bind mount unit I'm doing, or perhaps the content interface
[19:25] <kyrofa> jdstrand, so what, use fuse for another directory, and bind-mount that one into the snap?
[19:26] <jdstrand> I was saying skip fuse altogether. perhaps what you said would work
[19:27] <kyrofa> jdstrand, yeah fuse might be the only way rclone supports this type of thing (not sure). I'll suggest they try that
[19:28]  * jdstrand has never used rclone
[19:32] <kyrofa> Me neither
[19:34] <jdstrand> kyrofa: it might be interesting to see the denials that nextcloud pops out when they fuse mount something into nextcloud's SNAP_DATA or SNAP_COMMON (that is how I understood your use case anyway)
[19:34] <kyrofa> Indeed, my understanding as well. I'll ask
[19:51] <el_tigro1> I'm trying to gain a better understanding of snaps and I have a couple of questions that hopefully someone can answer
[19:53] <kyrofa> Hey there el_tigro1, welcome!
[19:53] <kyrofa> Fire away
[19:55] <el_tigro1> My understanding is that each snap runs in its own mount namespace. So grabbed the PID dockerd (running as a snap) and decided to have some fun with nsenter "sudo nsenter -t <docker_pid> -a bash". If I understand correctly I'm within the dockerd processe's mount namespace, and the filesystem root is ubuntu core
[19:56] <el_tigro1> kyrofa: thanks
[19:58] <kyrofa> el_tigro1, indeed, that's my understanding as well
[19:59] <kyrofa> (if by "ubuntu core" you mean "the core snap")
[19:59] <el_tigro1> yes that's what I mean
[20:00] <el_tigro1> So I guess the "pivot_root" system call was used to change the root filesystem?
[20:01] <el_tigro1> Then from within the namespace, I did "mount -l" to see what is mounted and I can see that the parent namespace's filesystem is mounted read/write on "/var/lib/snapd/hostfs"
[20:02] <el_tigro1> Doesn't that mean that the dockerd process has read/write access to the parent root filesystem? I'm kind of confused because I thought one of the main purposes of snaps was to sandbox the process
[20:04] <kyrofa> el_tigro1, note that there are _several_ aspects to this "sandbox"
[20:05] <kyrofa> el_tigro1, snaps are also confined with apparmor, which will deny any access to that area
[20:05] <kyrofa> el_tigro1, seccomp is also part of the story, which covers syscalls
[20:06] <kyrofa> el_tigro1, every `plug` declared by the snap is essentially a snippet of apparmor and allows syscalls
[20:06] <kyrofa> s/allows/allowed/
[20:08] <el_tigro1> kyrofa: thanks that makes sense
[20:08] <kyrofa> el_tigro1, the new rootfs isn't even really part of the security story-- it's part of the "run the same way everywhere" story
[20:10] <el_tigro1> kyrofa: so I can access that mountpoint from the nsenter shell because the apparmor profile and seccomp are obviously not applied in this case
[20:10] <kyrofa> Indeed
[20:11] <kyrofa> el_tigro1, try this
[20:11] <kyrofa> Install a snap that has an application available to run. I assume the `docker` snap has the `docker` command... right?
[20:11] <el_tigro1> yes
[20:12] <kyrofa> el_tigro1, run `snap run --shell docker`
[20:12] <kyrofa> el_tigro1, instead of running the docker command, it'll give you a shell with the exact same confinement as that command
[20:13] <kyrofa> el_tigro1, basically what you've already done, but it gives you a clearer picture of what kind of access docker has
[20:14] <el_tigro1> kyrofa: WOW, very cool
[20:15] <mup> PR snapd#4611 opened: overlord/configstate/config: make [GS]etSnapConfig use *RawMessage <Created by chipaca> <https://github.com/snapcore/snapd/pull/4611>
[20:15] <el_tigro1> I'm so happy I just learned that "trick"
[20:15] <kyrofa> el_tigro1, super handy when debugging as well
[20:16] <el_tigro1> I have another question related to snaps/lxc/zfs. Could be a bit trickier though
[20:18] <kyrofa> Hit me. If I can't answer someone here will
[20:23] <el_tigro1> ok. So I have 2 lxc containers running using 2 different images (centos and ubuntu), both using the default zfs pool. In the parent namespace, I do "zfs list" and can see (using "zfs get all <VOLUME>") that both images have mountpoints and are mounted. The container volumes on the other hand have mountpoints but are NOT mounted. Makes sense so far since the container volumes are mounted within the lxc snap's namespace
[20:25] <zyga> hey el_tigro1
[20:25] <zyga> I didn't read the backlog but if you have any questions about the execution environment I'm happy to provide answers
[20:25] <zyga> (mount namespaces and how they are set up)
[20:25] <el_tigro1> zyga: thanks a lot!
[20:27] <zyga> jdstrand so... any direction?
[20:28] <el_tigro1> I decide to have some more nsenter fun so I grab the PID of the lxcfs process, and do "nsenter -t <lxcfs_pid> -a bash" and now i'm in the lxd snap's mount namespace. I do "df -T" and for some reason I see a mount for each image, and 1 mount for the ubuntu container, but NO mount for the centos container!
[20:28] <el_tigro1> Here's the line for the ubuntu container's zfs volume mount:
[20:28] <el_tigro1> "default/containers/juju-ade8fe-0                                                zfs       12882432   774784  12107648   7% /var/snap/lxd/common/lxd/storage-pools/default/containers/juju-ade8fe-0"
[20:29] <el_tigro1> yet my centos container is running, and I can get a shell in it with "lxc exec <centos_name> bash"
[20:30] <__chip__> zyga: could you take a look at #4611 please? should be relatively easy
[20:30] <mup> PR #4611: overlord/configstate/config: make [GS]etSnapConfig use *RawMessage <Created by chipaca> <https://github.com/snapcore/snapd/pull/4611>
[20:30] <zyga> yes
[20:30] <__chip__> ta
[20:30] <zyga> ah, this
[20:30] <__chip__> :-)
[20:31] <el_tigro1> kyrofa: what I'm saying is probably poorly explained or doesn't make much sense  :D
[20:31] <zyga> can you look at https://github.com/snapcore/snapd/pull/4610 ? mabe jamie will look and I can land it
[20:31] <mup> PR #4610: interfaces/apparmor: early support for snap-update-ns snippets <Created by zyga> <https://github.com/snapcore/snapd/pull/4610>
[20:31] <__chip__> zyga: yes
[20:31] <__chip__> el_tigro1: maybe you want stgraber ?
[20:32] <kyrofa> el_tigro1, hmm, you're right, this is a little beyond me and more specific to lxd. stgraber is the guy you want here
[20:32] <zyga> el_tigro1 I can answer any question about how snapd use mount namespaces
[20:32] <zyga> el_tigro1 though I'm not an expert on lxc/lxd which can do its own things
[20:33] <kyrofa> __chip__, are you like Neal, changing your nick every once in a while to keep people on their toes?
[20:34] <__chip__> kyrofa: I have different defauts on different boxes
[20:34] <potato> I need to ... shave!
[20:34] <potato> *ha*
[20:34] <zyga> MBUAHAHAHA
[20:34] <potato> ohhh
[20:34] <potato> darn
[20:34] <elvis> __chip__ let's swap nics for 24 hours ;)
[20:35] <__chip__> kyrofa: I also have Chipakeitor as a fallback
[20:35] <kyrofa> Haha
[20:35] <kyrofa> At least you always type the same way
[20:35] <__chip__> kyrofa: I used to have BeerServ as well, but then they got all "you can't pretend to be a network service"
[20:35] <kyrofa> Really.
[20:35] <__chip__> yes
[20:35] <__chip__> so I keep expecting them to implement an _actual_ BeerServ
[20:36] <kyrofa> That's what that means to me as well
[20:36] <__chip__> maybe it's for admins only
[20:36] <kyrofa> They probably have one, but only to employees
[20:36] <kyrofa> Yeah
[20:37] <__chip__> zyga: why is spec.sunSnippet a map[string]map indexed by spec.sunSnippetName?
[20:37] <__chip__> zyga: if the map's only ever going to have one entry seems daft
[20:38] <zyga> sunSnippetName?
[20:38] <__chip__> ehm
[20:38] <__chip__> snapName
[20:38] <zyga> right
[20:38] <zyga> so
[20:38] <zyga> I think you are right
[20:38]  * __chip__ 's waiting for the "but..."
[20:38] <zyga> I think it could be just a layout thing, the spec is in the repo
[20:38] <zyga> created in the repo
[20:39] <zyga> there's no but here, I just made it obvious that there is a thing there
[20:39] <zyga> and if that changes it's not a bug that's quirky
[20:39] <zyga> though I a gree
[20:39] <zyga> *agree
[20:39] <zyga> I think that the scope idea, once improved, could help to make that cleaner, I didn't think that through though
[20:42] <__chip__> zyga: I'm not sure if it's a bug that you'd keep the sunSnippet for other scopes alive around setScope invocations
[20:42] <zyga> so yes, there's going to be a few calls
[20:42] <zyga> but they will all come from one snap
[20:43] <zyga> so that's why I'm saying you are right that we could perhaps drop the map entirely
[20:43] <zyga> the good thing is that it doesn't matter for interfaces, just for the backend
[20:44] <__chip__> zyga: should I +1 this, or should I wait a bit?
[20:44] <zyga> __chip__ if you mentally +1 and tell me I'll land it when jamie approves
[20:45] <zyga> just say what you think, no rush
[20:45] <__chip__> zyga: I mean, I can +1 this as it stands, and my +1 is still valid if you pare down the map
[20:45] <zyga> ok
[20:45] <__chip__> zyga: should i do that?
[20:46] <zyga> yeah
[20:46] <zyga> omg, sublime text is awesome :-)
[21:36] <nacc> if I was to use SNAPCRAFT_CONTAINER_BUILDS=1 instead of cleanbuild, and I'm buillding froma  git repository; will `snapcraft` correctly detect my changing branches and needing to change what was put in the container? Or will I need to add some git hooks to remove the container when the branch changnes?
[21:48] <kalikiana> nacc: the container uses the files straight from your repository, they're not copied - assuming I got your question right
[21:48] <nacc> kalikiana: they are bind mounted in?
[21:48] <nacc> kalikiana: how would that work with a remote lxd??
[21:49] <jdstrand> zyga: I've not been able to look at it yet-- lots of pings and store stuff
[21:49] <kalikiana> nacc: in that case it's mounting via sshfs
[21:49] <nacc> kalikiana: magic! :)
[21:49] <kalikiana> nacc: Yeah :-D bit of a pain to make it work behind the scenes, but awesome when it works
[21:49] <nacc> kalikiana: ok, so basically cwd's conntents are reflected to the container -- what if that resultls in buildl-deps changing? they willl get built if need be, right? (e.g., a part versionn changed in a branch)
[21:50] <kalikiana> nacc: same as a "normal" build. the only difference it builds/installs in the container
[21:50] <nacc> kalikiana: cool, thanks1
[21:52] <kalikiana> nacc: lemme know how it goes when you try it. I'm always interested in feedback, good or bad
[21:52]  * kalikiana happens to be the person implementing that feature
[21:52] <nacc> kalikiana: what happens if multiple builds go on at the same time with S_C_B=1 ?
[21:53] <kalikiana> nacc: each project (read: source folder) will use its own container
[21:53] <nacc> kalikiana: right, but if you have two branches in the same git repo
[21:54] <nacc> they will end up sharing hte container?
[21:54] <kalikiana> nacc: since you can only checkout one branch at a time, yes
[21:54] <nacc> kalikiana: not entirely true with work trees :)
[21:54] <nacc> kalikiana: but yeah ok
[21:54] <kalikiana> nacc: work trees?
[21:55] <nacc> kalikiana: `man git-worktee`
[21:55] <nacc> *`man git-worktree`
[21:55] <nacc> let's you checkout multiple working trees simultaneously
[21:56] <kalikiana> hmmm interesting. never used that
[21:56] <nacc> kalikiana: yeah, it's new-ish
[21:56] <nacc> really handy for some cases
[21:56] <kalikiana> nacc: if it's for testing purposes, you could change the name in snapcraft.yaml to get a different container
[21:56] <nacc> but our use case is that we've added a self-test to our snap for as-snapped testing
[21:56] <nacc> so our CI builds the snap and then runs that app
[21:57] <nacc> that build is a bit slow, and most of it is the same between snaps
[21:58] <kalikiana> yeah. builds would be faster this way, compared to cleanbuild
[21:59] <kalikiana> just keep in mind that changes in parts will still require you to clean those parts
[22:01] <nacc> ack
[22:03] <zyga> jdstrand ack