/srv/irclogs.ubuntu.com/2018/02/05/#snappy.txt

mborzeckimorning06:06
mborzeckizyga: hey06:52
mborzeckiyou around?06:52
zygamborzecki sure, can I help you somehow?07:05
mborzeckizyga: nvidia biarch is borked07:05
mborzeckilet me show you where07:05
mborzeckizyga: 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:07
=== zyga_ is now known as zyga
mborzeckizyga: 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
zygaaha07:14
zygathis makes sense, feel free to move that outside so that things work correctly07:14
mborzeckiok07:15
zygathank you for spotting that07:15
zygait feels like a RC3 though07:15
mborzeckiyeah, 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:17
zyga:-)07:23
zygait's good that you are using nvidia though, most of the laptop developers don't notice that07:23
zygahey mvo07:29
mvohwy zyga07:29
mvohey07:29
mborzeckimvo: morning07:34
mvogood morning mborzecki07:34
mupPR 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
mborzeckizyga: ^^07:49
zygayep07:49
zygalooking07:49
mborzeckisadly there are no tests for this code, so i just tested it locally07:49
mborzeckithe good news is that i can run snaps again :)07:50
mborzeckiohmygiraffe works07:51
kalikianagood morning, snappers07:54
mborzeckikalikiana: morning07:56
mvomborzecki: oh, nice. something for 2.31?07:59
zygamborzecki +108:02
zygawe need to ensure that some round of testing on proprietary nvidia driver is done for 2.3108:03
mupPR 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:04
mvozyga: is this 2.31 cherry-pick material?08:06
zygayes08:06
zygadefinitely08:06
mborzeckii have an old 9600gt card, so can check with 340xx driver, no vulkan though08:07
zygamvo note that it affects _biarch_ systems08:07
mvozyga: so fedora/solus?08:07
mborzeckimvo: yes, otherwise it's not possible to assemble a mount on on my arch box08:07
zygamvo yes08:07
mvota08:07
* mvo looks08:07
mvomborzecki: 4598 is ready right? it has a +1 from neal08:11
mborzeckimvo: yes08:11
mupPR 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:13
mvomborzecki: fwiw, once the tests are green I will merge 4603, my commment about comments can always added later08:17
mborzeckimvo: ok08:19
mvomborzecki: 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 fails08:20
zyga....08:21
zygawell08:21
zygayeah, that is weird08:21
mvozyga: my thoughts exactly :)08:21
mvozyga: I will send a patch to upstream and to our ppa08:22
mupPR 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
mvozyga: but holly cow08:22
mborzeckimvo: is there anything special about the file?08:22
zygamaybe it's _just_ a bug08:23
mvomborzecki: which file? the squashfs file that is unpacked? I unpacked it into a tmpfs08:23
mvomborzecki: and that had not enough space08:23
mborzeckimvo: right, the squashfs file08:23
mborzeckioh08:23
mvomborzecki: sergio figured out this is what broke the prepare-image test on arm6408:23
mvomborzecki: 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
mvozyga: it probably is08:24
mborzeckimvo: btw. does it try to clean up the fiels it extracted?08:24
mborzeckis/fiels/files/08:25
mvomborzecki: I doubt it, let me check08:25
mvomborzecki: no08:25
mborzeckiright, don't know what i was expecting08:27
mborzeckiit'd be funny to put squashfs through afl and see what comes out08:28
zygaafl?08:29
mborzeckiamerican fuzzy loop08:29
mborzeckihttp://lcamtuf.coredump.cx/afl/08:30
zygaoh, cool08:30
zygaanyone wants to do a quick HO test with me08:32
pstolowskimornings!08:38
zygahey hey08:38
mborzeckitak, tak08:50
mborzeckiwrong window :)08:50
mborzeckipstolowski: morning08:50
zygamborzecki tak tak, wrong window08:51
mborzeckimvo: restarted the build in 4603, looked like an unrelated failure08:51
mvook08:51
=== d__ed is now known as d_ed
Chipacamvo: o/09:17
Chipacamvo: looking at 4600, do config changes always end in a *json.RawMessage?09:17
mvoChipaca: I'm not 100% certain, that sounds like something pstolowski may know09:18
Chipacapstolowski: ^?09:18
Chipacapstolowski: in config's transaction.go, do config changes always finish in a *json.RawMessage?09:19
mvoChipaca: fwiw, the PR is a bit of a RFC for gustavo if this is validation we want or not09:19
Chipacamvo: ack09:19
mvoogra_: did things work once the typo in the core.service.rsyslog.disable=true was fixed btw?09:19
mvoChipaca: I mean, don't kill yourself over the review :) it might be in vain :(09:19
mvoChipaca: even though I think it would be good to have this validation09:20
pstolowskiChipaca, mvo yes09:21
Chipacapstolowski: ack09:21
Chipacapstolowski: why are they *json.RawMessage, btw? instead of just json.RawMessage i mean09:21
milp_mediaogra_: 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
pstolowskiChipaca, dunno. I haven't implemented that09:22
mvozyga: 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 upstream09:22
pstolowskiChipaca, kyrofa might remember ;)09:22
Chipacapstolowski: blame says gustavo09:23
Chipaca:-)09:23
Chipacai'll ask09:23
pstolowskiChipaca, okay. blame has better memory than me, that's for sure09:23
mvoChipaca: ta09:23
zygamvo: what is the worst thing that can. happen if we use broken unsquashfs?09:25
mvozyga: 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
mvozyga: so hopefully its not too bad09:26
mvozyga: but still fugly that we don't detect it when it happens09:27
Chipacamvo: zyga: we found a bug in unsquashfs?09:27
mborzeckimvo: shall i merge 4603?09:27
mvoChipaca: bug/feature/missing-thing - unsquashfs does not report anything on unsqushfs failures09:27
mvoChipaca: like exit(0)09:27
mvoChipaca: is what it will always do09:28
Chipacaah09:28
mvoChipaca: which is slightly annoying, we had this bug on arm64 that prepare-image failed and there was a kernel missing09:28
mvoChipaca: and it turns out, tmpfs was too small and unsquashfs told the world: "failed..." on stdoutbut never reported it via exitcode09:28
mvomborzecki: let me look09:29
mvomborzecki: yes please, I will cherry-pick09:29
mvoafterwards09:29
zygamvo: could we bundle fixed unsquashfs in the package?09:29
mvozyga: yeah, I think that would work09:29
mupPR 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:29
mvozyga: I was just thinking we could do a strings.Contains("failed", strings.ToLower(string(unsquashfsOutput)))"09:30
zygaI mean, the risk is rather small because we check for hashes and the store does validation as well09:30
mvozyga:as a first approximation09:30
zygamvo, a simple "failed" thing works too, yeah09:30
mvozyga: I think I will do that and hope for upstream, once upstram merges it I can SRU it at least into the ubuntu-world09:30
mvozyga: anyway, thanks for the brainstorm :)09:31
zyga:-)09:31
* mvo is also slightly shocked that there is no open upstream bugreport about this09:31
mupPR 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:32
mborzeckizyga: ^^ this a workaround for the problem with missing /var//lib/snapd/lib that we had on fedora09:33
zygareading09:33
zygamborzecki one comment09:34
mborzeckizyga: uhh right :/ silly me09:36
Chipacapstolowski: want me to add the roundrobin writer to strutil?09:43
mborzeckizyga: mvo: is 4604 a 2.31 material too?09:45
=== ufee1dead is now known as ikey
zygayes09:45
pstolowskiChipaca, no, thanks, I'm on it09:45
Chipacapstolowski: ok09:45
* Chipaca carries on down the PRs09:46
mvomborzecki: that is only needed for re-exec?09:46
mvomborzecki: and bi-arch? or more generic?09:46
mborzeckimvo: bi- and multi- arch, same thing as in #4598 but works around packaging errors09:48
mupPR #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:48
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:53
=== chihchun_afk is now known as chihchun
mvoogra_: thanks09:54
mvoogra_: 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 awesome09:59
mvoogra_: pr #4600 iirc09:59
mupPR #4600: configstate: validate known core.* options <Created by mvo5> <https://github.com/snapcore/snapd/pull/4600>09:59
ogra_nice round number10:00
* mvo back to squashfs10:00
mvoogra_: heh, indeed10:00
ogra_heh. you too ?10:00
* ogra_ is banging his head against grubs squashfs implementation on arm10:00
mvoogra_: I am just fixing silly issue around unsquashfs10:01
ogra_sadly the u-boot api and grub dont want to play together well ..10:01
mvoogra_: autsch!10:01
ogra_would be so nice to have the arm boards read from the snaps diurectly as well ..10:01
ogra_(beyond being able to use secureboot from grub)10:02
mvoogra_: 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:03
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:04
ogra_the other option is grub ... and use signed single files directly from the snaps10:05
mvoogra_: so uboot chainloads to grub?10:05
mvoogra_: that sounds interessting for the main borads as well if this works well10: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 uboot10:06
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 disk10:07
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:08
* mvo nods10:09
* Chipaca ~> afk (physio)10:10
=== chihchun is now known as chihchun_afk
mupPR snapd#4605 opened: snap: detect unsquashfs write failures <Created by mvo5> <https://github.com/snapcore/snapd/pull/4605>10:25
zyga mvo one comment there10:31
* Chipaca tries again to go to physio10:42
mvozyga: ta10:44
zygamvo nothing strong, just could save (perhaps) some memory10:44
mvozyga: easier to read this way too, I like it10:45
mvozyga: 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 option10:45
=== lool- is now known as lool
zygaCloning into '/home/gopath/.cache/govendor/golang.org/x/crypto'...10:50
zygaerror: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function.10:50
zygaI saw this a few times today10:50
zygabrb10:56
=== chihchun_afk is now known as chihchun
zygamvo, can we merge 4576?11:18
mupPR snapd#4596 closed: osutil: allow using many globs in EnsureDirState <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4596>11:36
mupPR snapd#4606 opened: snap: use custom unsquashfsStderrWriter for unsquashfs error detection <Created by mvo5> <https://github.com/snapcore/snapd/pull/4606>11:46
mvoChipaca: I pushed a potential fix using the custom writer -^11:46
mborzeckiany idea why snap app services are enabled (but not started) in doLinkSnap() while app socket services are enabled and started in statSnapServices() ?11:46
oSoMoNsergiusens, 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 releases11:47
oSoMoNis there a bug report I can subscribe to?11:47
seb128sergiusens, he also said you stated you would have it fixed by past-thursday :)11:49
=== chihchun is now known as chihchun_afk
pedronismborzecki: socket services are recent,  where do we start then app services?11:58
mupPR snapd#4589 closed: many: remove "content" argument from snaptest.MockSnap() <Created by mvo5> <Merged by zyga> <https://github.com/snapcore/snapd/pull/4589>11:59
zygaChipaca, I think we should have fewer implementations of mkdirallchown12:00
zyga+1 on the patch there but I think we could use the secure variant instead12:00
zygachipaca we could make osutil/secure/ package and stick them there12:00
mborzeckipedronis: in 'start-snap-services', but they get enabled in link-snap12:01
mborzeckipedronis: the sockets are enabled and started in start-snap-services12:01
mupPR 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:02
zygaChipaca question on 458112:04
pedroniszyga: what do you mean with empty there?12:05
zygaempty byte array12:05
pedronisI don't think I understand the question, maybe John will12:06
zygapedronis maybe I misunderstood how that works, I'll talk to john12:06
pedronisit'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 one12:07
pedronisthere's nothing to do12:07
pedronisbasically12:07
zygaI think I understand that now, snapcfg is the value we want to set, not the data store12:08
zygamvo is 2.31 something that is now living in a branch?12:14
zygaI'd like to merge 1st 2.32 PRs that are not ready for 2.31 rc312:14
mvozyga: 2.31 is in a branch12:21
mvozyga: only cherry-picks12:21
zygaexcellent12:22
zygaI'm chopping various things for sun profiles12:22
zygaand trying to figure out how to avoid some duplication and annoying maintenance12:22
zygaand since it's not a 2.31 thing anymore I'm doing some improvements to how apparmor backend works12:22
mvozyga: yeah, you can go wild in master12:24
zygacool12:25
zygajdstrand, 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 week12:26
zygaand the branches can be nicer when we're not after minimal diffs anymore12:26
mupPR snapd#4607 opened: wrappers: cleanup enabled service sockets <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/4607>13:02
* Son_Goku sighs13:29
Son_GokuI wish people would stop counting on LSM stacking to save their butts13:30
zygahey Son_Goku13:30
Son_Gokuzyga: hey13:31
=== chihchun_afk is now known as chihchun
=== chihchun is now known as chihchun_afk
* kalikiana going to have lunch, back in a bit13:51
pedronisChipaca: not super urgent but a review of #4497 would be appreciated14:01
mupPR #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
pedronisniemeyer: for later, the PR I mentioned is: #458214:01
mupPR #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:01
niemeyerpedronis: Thanks!14:09
flexiondotorgzyga: Who can we work with to get snapd updated in openSUSE?14:23
flexiondotorgSadly most desktop application snaps are wearing their sad face when run on openSUSE right now.14:24
ikey"lets turn that frown upside down"14:27
zygaflexiondotorg me or anyone who will do the packaging14:27
* ikey remembers his motivational training14:27
flexiondotorgzyga: Any chance you could find the time to rev snapd in openSUSE to 2.30?14:28
flexiondotorgIt's currently 2.27 so doesn't know about the desktop and desktop-legacy interfaces.14:29
flexiondotorgikey: o/14:29
ikey\o14:29
zygaflexiondotorg let me try tomorrow and if not let's find somoene14:30
zygaflexiondotorg I have opensuse ready and booted, just some higher priority things14:33
zygaflexiondotorg we don't have anyone on it that's dedicated14:34
zygaflexiondotorg I can review PRs to that repo14:34
flexiondotorgThanks zyga14:35
Chipacamvo: indeed partial writes make this a lot harder14:45
Chipacamvo: sorry :-) i'll still get something up for you to look at in case you like it14:45
mvoChipaca: yeah, its a bit annoying14:46
mvoChipaca: sure, happy to have a look how you solved it14:46
mvoChipaca: that reminds me, my test case needs an upper case "Failed to..."14:47
mupPR 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:51
kalikianare14:54
Chipacamvo: 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 run15:01
mvoChipaca: in a meeting right now, let me read it but thanks a bunch for looking into this in any case15:09
mvoChipaca: it looks a bit shorter at least15:10
zygajdstrand hello, how was your flight home/15:15
jdstrandzyga: uneventful :)15:17
jdstrandzyga: I saw the request for 4590 re-review15:17
zygajust as it should be15:17
jdstrandzyga: exactly :)15:17
zygaI didn't do the full thing there, just added your FIXME comments15:17
zygaand confirmed with mvo that release is branched off and can merge things15:18
zygaI'm chopping my sun profile work into smaller pieces now15:18
jdstrandcool15:20
jdstrandlet me look at 4590 real quick then15:20
zygacool, thanks!15:25
zygaoh ... cooooool :)15:27
* zyga just realised something very useful :)15:27
pedronisChipaca: did you do anything about saving screenshots on install? or still pending15:36
Chipacapedronis: that's phase 215:36
Chipacapedronis: and requires answers to Questions15:36
pedronisChipaca: phase 2 of what?15:36
Chipacapedronis: of snapshots15:36
Chipacapedronis: phase 1 is just manual15:37
pedronisChipaca: I said screenshots, not snapshots15:37
pedronissomething discussed in NYC15:37
Chipacapedronis: questions such as "if you snapshot rev2 and upgrade to rev3 and restore what do you even restore"15:37
Chipacapedronis: /o15:37
Chipacapedronis: I did nothing about any metadata15:37
pedronisok, thx15:37
Chipacaicons, screenshots, title, description, etc etc etc15:37
Chipacaall the same bag imo15:38
pedronisChipaca: because we had some discusions about saving more, saving differently, right?15:38
pedronisbut hasn't happend, ok15:38
Chipacapedronis: we decided those for all attributes that were per snap not per rev the store is authoritative, and we should keep a cache locally15:39
Chipacacorrect, no progress15:39
kyrofapstolowski, Chipaca, yeah I got nothing on the *json.RawMessage15:40
Chipacakyrofa: thank you for reminding me :-D15:40
pstolowski:)15:40
Chipacaniemeyer: in configstate/config we use *json.RawMessage everywhere instead of json.RawMessage. Why?15:40
Chipacaniemeyer: (asking you because git blames you)15:40
mvoChipaca: he is in a meeting15:44
Chipacamvo: i am in no rush :-)15:44
* cachio__ lunch15:56
mupPR 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
zygajdstrand can you please look at ^^16:06
zygait's a two liner16:06
jdstrandzyga: done16:09
zygamvo ^16:09
zygaI'll work on updates to the tests to check relevant new features as a user, just ran into this by dogfooding16:10
niemeyerChipaca: Heya16:12
niemeyerChipaca: There is/was a serious bug about it in the json package16:12
niemeyerChipaca: It misbehaves wildly in some situations16:12
Chipacaah16:12
Chipacaniemeyer: i see16:13
niemeyerChipaca: Let me see if I can find some quick info16:13
Chipacaniemeyer: i use json.RawMessage directly in snapshots; let me know if i'll get bitten16:13
Chipacaniemeyer: or taht :-D16:13
niemeyerChipaca: https://github.com/golang/go/issues/14493, probably16:14
pedronisah, that16:18
zyga+ systemctl stop snapd.service snapd.socket16:36
zygaJob for snapd.service canceled.16:36
Chipacaniemeyer: 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 out16:50
Chipacaniemeyer: OTOH, it's fixed, so ¯\_(ツ)_/¯16:52
zygaso "it will be out in debian 10"16:53
zygain X years16:53
Chipacazyga: it's not go's fault ubuntu always ships a go that is no longer supported by them16:54
Chipacamuch as it irks me, it really isn't :-)16:55
Chipacaand this particular bug has been fixed since 1.8, which is a year old already16:55
Chipacazyga: what do you mean with "fewer implementations of mkdirallchown"?17:06
zygaChipaca we have one in snap-update-ns that's tailored for security17:07
Chipacazyga: ahh... i forget17:07
Chipacazyga: I can take on unifying those two17:07
Chipacai see no reason not to use the securer one everywhere :-)17:07
zygayeah17:07
zygahow does osutil/secure sound?17:08
zygathose would require jamie review to change17:08
zygabut we could just move the current code there first17:08
zygahave a look, maybe something about it is silly and unreasonable, maybe not17:08
Chipacazyga: sounds like a good, reasonable plan17:08
Chipacazyga: but, going back a bit, you said "+1 to my changes there", but didn't actually +1 :-D17:09
zygaI will add secure symlink next, definitely this week17:09
zygaoh, sorry17:09
zygalooking17:09
Chipaca:-)17:09
Chipacazyga: also your comment on #4581 feels reviewish but doesn't have a +1 either17:09
mupPR #4581: overlord/configstate/config: make SetSnapConfig delete on empty <Created by chipaca> <https://github.com/snapcore/snapd/pull/4581>17:09
Chipacazyga: #4587 is the other one fwiw17:09
mupPR #4587: osutil: make MkdirAllChown clean the path passed in <Created by chipaca> <https://github.com/snapcore/snapd/pull/4587>17:09
mupPR 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
mupPR 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:10
Chipacayuss17:11
niemeyerChipaca: Yeah, it's fixed.. we just need to check if all the versions of Go we care about are fixed too17:12
niemeyerChipaca: I would guess not17:12
Chipacaniemeyer: 1.8+17:12
niemeyerYeah, so risky still17:12
ChipacaI'll update snapshots to use *json.RawMessage for now then17:13
Chipacaas part of the rebase17:13
Chipacajust one more pr to land for that to happen :-)17:13
zygaChipaca one question posted on 458517:13
Chipacazyga: i confess to not understanding your question :-(17:17
Chipacazyga: care to expand it a little bit?17:18
zygasure17:18
zygasorry about tyhat17:18
zygais there any practical difference between using `inst` there? (instead of the current code that does `&inst`)17:18
zygainst is a function anyway, right?17:18
Chipacazyga: no, inst is the instruction, a thing sent by the user that got decoded a little bit further up17:19
Chipacazyga: op is the function17:19
zygaah, sorry then17:19
Chipacazyga: you couldn't do &foo if foo was a function17:20
Chipacaafaik17:20
Chipacaah yes you can17:21
Chipacaeh17:21
pstolowskiChipaca, no run-hook in snap topical tasks17:21
Chipacapstolowski: i know it's not there currently; should it be?17:22
Chipacapstolowski: or does hookstate have its own conflict thing?17:22
ChipacaI suspect you can currently install, remove, refresh snaps that are in the middle of running a hook17:24
mupPR snapd#4609 opened: interfaces/apparmor: use a helper to set the scope <Created by zyga> <https://github.com/snapcore/snapd/pull/4609>17:26
zygaChipaca ^17:26
Chipacaack17:26
Chipacazyga: currently looking at one for bboozzoo17:27
zygathanks!17:27
kalikianagrrr, python env mocking is tedious17:27
kalikianawill have to finish that tomorrow17:27
zygamocking environment?17:27
zygajust mock os.environ and os.getenv (probably?)17:27
kalikianazyga: 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 there17:29
kalikianaSomehow the almost-empty mock env has values in it that I did not set17:29
pstolowskiChipaca, 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
zygamocking is tricky, perhaps the import path matters, you may see unmocked environment depending on "how" you look (what's the imported object)17:30
kalikianazyga: Like MagicMock<name=environ.get() id=12345>17:30
zygahow did you mock environ?17:30
kalikianaThose are the values that show up in other places now17:30
kalikianazyga: patch('os.environ')17:31
zygais anything importing environ locally? patch has its limits as to what it can reference17:31
kalikianazyga: there's os.environ.copy, which works as expected with my {} value. the other code is using os.getenv17:32
pstolowskiChipaca, so yes, maybe run-hook should be there. I can look take this17:32
kalikianazyga: the os.getenv seems to be getting those MagicMock values now17:33
* zyga dinner17:34
pedronisChipaca: 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 conflict17:35
pedronislike link-snap17:35
pstolowskipedronis, but link-snap can be Done, and configure snap runs afterwards17:36
pedronispstolowski: no17:36
pedronisthe normal conflict logic17:36
pedronisis about full changes17:36
pedronisnot single tasks17:36
pedronis(unless it was changed recently)17:36
kalikianaelopio: in case you happen to have any ideas on that... ^^ otherwise I'll be digging in the docs tomorrow morning over my coffee17:36
* kalikiana wrapping up for today17:37
zygakalikiana I can tell you why this happens17:37
zygaand perhaps how to fix it17:37
zygatomorrow would be best :)17:37
pedronispstolowski: it's not about link-snap being done, it's about the full change that has link-snap in it being done17:38
kalikianazyga: Awesome. Let's chat tomorrow then17:38
pstolowskipedronis, ah oh you'r right17:38
pedronispstolowski: your new code is a bit different (different tradeoffs there)17:38
pedronisand concerns onyl connect/disconnect17:39
pstolowskipedronis, yes, that's what put me on a wrong track17:39
pstolowskiso yeah, we should be good, sorry for confusion Chipaca17:39
Chipacapstolowski: now i'm wondering about configure hooks17:39
pedronisthey might die, saying that snap is not there anymore I suppose17:40
pedronisChipaca: we dont' seem to do conflict checks around configure/Configure17:42
pedronisso there might be an issue there17:44
pedronisin some corner cases17:44
niemeyerStepping out for a while18:05
mupPR 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:09
mupPR 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
zygaChipaca small follow-up if you want to look18:15
zygajdstrand ^18:15
zygajdstrand this is what it would look like, interfaces could now add sun snippets18:16
mupPR snapd#4610 opened: interfaces/apparmor: early support for snap-update-ns snippets <Created by zyga> <https://github.com/snapcore/snapd/pull/4610>18:16
zygajdstrand we could even use it to reduce permissions on snap-update-ns (the base set018:16
zygaso even things like content interface would be explicit18:16
zygajdstrand if you can look at that (design, not security related) and +1 that I'll carry on with the backend parts that generate it18:17
zygajdstrand and obviously the layout parts18:17
jdstrandzyga: ack18:28
mupPR snapd#4585 closed: daemon: refactor snapFooMany helpers a little <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/4585>18:30
kyrofajdstrand, dumb question: can you give me a use-case for the fuse-support interface?18:58
kyrofajdstrand, someone is suggesting we add it to the nextcloud snap, but I'm not certain I understand what it does18:59
jdstrandkyrofa: fuse allows you to mount userspace filesystems: https://www.kernel.org/doc/Documentation/filesystems/fuse.txt19:11
jdstrandkyrofa: the interface has a number of limitations. it would be best to understand the specific use case the request is trying to support19:14
kyrofajdstrand, I haven't quite figured that out, yet. I'll get back in touch once I have a better picture19:15
kyrofajdstrand, looks like someone is `rclone mount`ing into the data dir and hoping Nextcloud can use it19:22
jdstrandkyrofa: the interface allows for performing the mount, not consuming one19:23
jdstrandI suggest instead the bind mount unit I'm doing, or perhaps the content interface19:24
kyrofajdstrand, so what, use fuse for another directory, and bind-mount that one into the snap?19:25
jdstrandI was saying skip fuse altogether. perhaps what you said would work19:26
kyrofajdstrand, yeah fuse might be the only way rclone supports this type of thing (not sure). I'll suggest they try that19:27
* jdstrand has never used rclone19:28
kyrofaMe neither19:32
jdstrandkyrofa: 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
kyrofaIndeed, my understanding as well. I'll ask19:34
el_tigro1I'm trying to gain a better understanding of snaps and I have a couple of questions that hopefully someone can answer19:51
kyrofaHey there el_tigro1, welcome!19:53
kyrofaFire away19:53
el_tigro1My 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 core19:55
el_tigro1kyrofa: thanks19:56
kyrofael_tigro1, indeed, that's my understanding as well19:58
kyrofa(if by "ubuntu core" you mean "the core snap")19:59
el_tigro1yes that's what I mean19:59
el_tigro1So I guess the "pivot_root" system call was used to change the root filesystem?20:00
el_tigro1Then 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:01
el_tigro1Doesn'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 process20:02
kyrofael_tigro1, note that there are _several_ aspects to this "sandbox"20:04
kyrofael_tigro1, snaps are also confined with apparmor, which will deny any access to that area20:05
kyrofael_tigro1, seccomp is also part of the story, which covers syscalls20:05
kyrofael_tigro1, every `plug` declared by the snap is essentially a snippet of apparmor and allows syscalls20:06
kyrofas/allows/allowed/20:06
el_tigro1kyrofa: thanks that makes sense20:08
kyrofael_tigro1, the new rootfs isn't even really part of the security story-- it's part of the "run the same way everywhere" story20:08
el_tigro1kyrofa: so I can access that mountpoint from the nsenter shell because the apparmor profile and seccomp are obviously not applied in this case20:10
kyrofaIndeed20:10
kyrofael_tigro1, try this20:11
kyrofaInstall a snap that has an application available to run. I assume the `docker` snap has the `docker` command... right?20:11
el_tigro1yes20:11
kyrofael_tigro1, run `snap run --shell docker`20:12
kyrofael_tigro1, instead of running the docker command, it'll give you a shell with the exact same confinement as that command20:12
kyrofael_tigro1, basically what you've already done, but it gives you a clearer picture of what kind of access docker has20:13
el_tigro1kyrofa: WOW, very cool20:14
mupPR snapd#4611 opened: overlord/configstate/config: make [GS]etSnapConfig use *RawMessage <Created by chipaca> <https://github.com/snapcore/snapd/pull/4611>20:15
el_tigro1I'm so happy I just learned that "trick"20:15
kyrofael_tigro1, super handy when debugging as well20:15
el_tigro1I have another question related to snaps/lxc/zfs. Could be a bit trickier though20:16
kyrofaHit me. If I can't answer someone here will20:18
el_tigro1ok. 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 namespace20:23
zygahey el_tigro120:25
zygaI didn't read the backlog but if you have any questions about the execution environment I'm happy to provide answers20:25
zyga(mount namespaces and how they are set up)20:25
el_tigro1zyga: thanks a lot!20:25
zygajdstrand so... any direction?20:27
el_tigro1I 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_tigro1Here'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:28
el_tigro1yet my centos container is running, and I can get a shell in it with "lxc exec <centos_name> bash"20:29
__chip__zyga: could you take a look at #4611 please? should be relatively easy20:30
mupPR #4611: overlord/configstate/config: make [GS]etSnapConfig use *RawMessage <Created by chipaca> <https://github.com/snapcore/snapd/pull/4611>20:30
zygayes20:30
__chip__ta20:30
zygaah, this20:30
__chip__:-)20:30
el_tigro1kyrofa: what I'm saying is probably poorly explained or doesn't make much sense  :D20:31
zygacan you look at https://github.com/snapcore/snapd/pull/4610 ? mabe jamie will look and I can land it20:31
mupPR #4610: interfaces/apparmor: early support for snap-update-ns snippets <Created by zyga> <https://github.com/snapcore/snapd/pull/4610>20:31
__chip__zyga: yes20:31
__chip__el_tigro1: maybe you want stgraber ?20:31
kyrofael_tigro1, hmm, you're right, this is a little beyond me and more specific to lxd. stgraber is the guy you want here20:32
zygael_tigro1 I can answer any question about how snapd use mount namespaces20:32
zygael_tigro1 though I'm not an expert on lxc/lxd which can do its own things20:32
kyrofa__chip__, are you like Neal, changing your nick every once in a while to keep people on their toes?20:33
=== zyga is now known as potato
__chip__kyrofa: I have different defauts on different boxes20:34
potatoI need to ... shave!20:34
=== __chip__ is now known as zyga
potato*ha*20:34
zygaMBUAHAHAHA20:34
potatoohhh20:34
potatodarn20:34
=== zyga is now known as __chip__
=== potato is now known as elvis
elvis__chip__ let's swap nics for 24 hours ;)20:34
=== elvis is now known as zyga
__chip__kyrofa: I also have Chipakeitor as a fallback20:35
kyrofaHaha20:35
kyrofaAt least you always type the same way20: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
kyrofaReally.20:35
__chip__yes20:35
__chip__so I keep expecting them to implement an _actual_ BeerServ20:35
kyrofaThat's what that means to me as well20:36
__chip__maybe it's for admins only20:36
kyrofaThey probably have one, but only to employees20:36
kyrofaYeah20:36
__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 daft20:37
zygasunSnippetName?20:38
__chip__ehm20:38
__chip__snapName20:38
zygaright20:38
zygaso20:38
zygaI think you are right20:38
* __chip__ 's waiting for the "but..."20:38
zygaI think it could be just a layout thing, the spec is in the repo20:38
zygacreated in the repo20:38
zygathere's no but here, I just made it obvious that there is a thing there20:39
zygaand if that changes it's not a bug that's quirky20:39
zygathough I a gree20:39
zyga*agree20:39
zygaI think that the scope idea, once improved, could help to make that cleaner, I didn't think that through though20:39
__chip__zyga: I'm not sure if it's a bug that you'd keep the sunSnippet for other scopes alive around setScope invocations20:42
zygaso yes, there's going to be a few calls20:42
zygabut they will all come from one snap20:42
zygaso that's why I'm saying you are right that we could perhaps drop the map entirely20:43
zygathe good thing is that it doesn't matter for interfaces, just for the backend20:43
__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 approves20:44
zygajust say what you think, no rush20:45
__chip__zyga: I mean, I can +1 this as it stands, and my +1 is still valid if you pare down the map20:45
zygaok20:45
__chip__zyga: should i do that?20:45
zygayeah20:46
zygaomg, sublime text is awesome :-)20:46
naccif 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:36
kalikiananacc: the container uses the files straight from your repository, they're not copied - assuming I got your question right21:48
nacckalikiana: they are bind mounted in?21:48
nacckalikiana: how would that work with a remote lxd??21:48
jdstrandzyga: I've not been able to look at it yet-- lots of pings and store stuff21:49
kalikiananacc: in that case it's mounting via sshfs21:49
nacckalikiana: magic! :)21:49
kalikiananacc: Yeah :-D bit of a pain to make it work behind the scenes, but awesome when it works21:49
nacckalikiana: 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:49
kalikiananacc: same as a "normal" build. the only difference it builds/installs in the container21:50
nacckalikiana: cool, thanks121:50
kalikiananacc: lemme know how it goes when you try it. I'm always interested in feedback, good or bad21:52
* kalikiana happens to be the person implementing that feature21:52
nacckalikiana: what happens if multiple builds go on at the same time with S_C_B=1 ?21:52
kalikiananacc: each project (read: source folder) will use its own container21:53
nacckalikiana: right, but if you have two branches in the same git repo21:53
naccthey will end up sharing hte container?21:54
kalikiananacc: since you can only checkout one branch at a time, yes21:54
nacckalikiana: not entirely true with work trees :)21:54
nacckalikiana: but yeah ok21:54
kalikiananacc: work trees?21:54
nacckalikiana: `man git-worktee`21:55
nacc*`man git-worktree`21:55
nacclet's you checkout multiple working trees simultaneously21:55
kalikianahmmm interesting. never used that21:56
nacckalikiana: yeah, it's new-ish21:56
naccreally handy for some cases21:56
kalikiananacc: if it's for testing purposes, you could change the name in snapcraft.yaml to get a different container21:56
naccbut our use case is that we've added a self-test to our snap for as-snapped testing21:56
naccso our CI builds the snap and then runs that app21:56
naccthat build is a bit slow, and most of it is the same between snaps21:57
kalikianayeah. builds would be faster this way, compared to cleanbuild21:58
kalikianajust keep in mind that changes in parts will still require you to clean those parts21:59
naccack22:01
zygajdstrand ack22:03

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