/srv/irclogs.ubuntu.com/2019/04/04/#snappy.txt

mborzeckimorning05:20
=== chihchun is now known as chihchun_afk
mupPR snapd#6686 closed: testutil: fix MockCmd for shellcheck 0.5 <Created by mvo5> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6686>05:45
zygaHey Maciek05:47
* zyga is totally sick05:47
mborzeckizyga: hey, take a day off, get some rest05:48
zygaI will :/05:52
mborzeckimvo: morning06:15
mvohey mborzecki06:15
mborzeckimvo: i was poking the fixed in #6685, looks like the current code works by accident06:17
mupPR #6685: image: prefer local for snapd/core snaps <Created by mvo5> <https://github.com/snapcore/snapd/pull/6685>06:17
mvomborzecki: yeah, its a bit puzzling, when I wrote the test things were a bit strange06:35
mvomborzecki: I haven't looked deeper, it got a bit late06:35
mvomborzecki: aha, nice find06:38
mborzeckimvo: kept wondering why i did not hit this problem before :P06:39
mvomborzecki: yeah, it looks like this code needs a closer look, maybe we can simplify and get rid of all the PreferLocal ones06:39
mborzeckimvo: fwiw, noticed something yesterday, when i did 'snap set system refresh.hold=' on a ssytem that did not refresh yet at all (eg. pristing image) it triggered a refresh06:40
mborzeckineed to check that with the tests, maybe i did something wrong06:40
mvomborzecki: that can be ok, if the image is really old snapd will detect no refresh for more than 60 days and force a refresh06:55
mborzeckimvo: right, want to double check that, we're using seed-time as reference06:55
zygaStore being silly about the description. https://usercontent.irccloud-cdn.com/file/u2vd6WZ6/Screenshot%202019-04-04%20at%2008.56.56.png06:57
zygaStore being silly about license duplicates https://usercontent.irccloud-cdn.com/file/uIzKQ0Zb/Screenshot%202019-04-04%20at%2008.59.36.png06:59
zyga Store being silly about case sensitive search for GPL https://usercontent.irccloud-cdn.com/file/EUWodfbQ/Screenshot%202019-04-04%20at%2009.00.13.png07:00
brlinzyga: Known issue07:01
brlinhttps://github.com/canonical-websites/snapcraft.io/issues/132907:02
zygaStore being silly when selecting the 2nd of the "identical" GPL v2 only licenses https://usercontent.irccloud-cdn.com/file/Mzh8Y4Q0/Screenshot%202019-04-04%20at%2009.01.41.png07:02
brlinhttps://github.com/canonical-websites/snapcraft.io/issues/158507:02
zygabrlin: thank you!07:02
brlinzyga: You're welcome!07:02
=== pstolowski|afk is now known as pstolowski
pstolowskimorning07:03
mvopstolowski: good morning07:06
mvomborzecki: not sure if seed time is the right reference, if you e.g. get the current stable image from cdimage.u.c it will be 8 month old already :/07:07
mborzeckimvo: we set it ourselves once seeding is done07:08
mvomborzecki: right07:08
mvomborzecki: what I mean is we need to think, if the snaps are 8month old we probably want to update soon07:08
brlinzyga > Store being silly about case sensitive search for GPL07:09
brlinNot reproducible at my end though07:09
zygabrlin: if you look for GPL instead of gpl, does it find anything?07:09
brlinzyga: https://usercontent.irccloud-cdn.com/file/waciCfQn/Screenshot_20190404_150931.png07:09
zygafun07:09
zygathen it is a client side search bug07:09
brlinFirefox seems to be fine07:10
mvothanks sil2100 !07:15
mupPR core18#121 closed: Make the version number date-based <Created by sil2100> <Merged by mvo5> <https://github.com/snapcore/core18/pull/121>07:15
zygamvo: https://github.com/snapcore/snapd/pull/6583 is an easy win07:18
mupPR #6583: cmd/snap-confine: move ubuntu-core fallback checks <Created by zyga> <https://github.com/snapcore/snapd/pull/6583>07:18
* mvo looks07:23
mupPR snapd#6583 closed: cmd/snap-confine: move ubuntu-core fallback checks <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6583>07:26
zygawoot thank you!07:29
zygamvo: so now shall I add a core16 fallback?07:29
brlinI wonder what technical difficulties are blocking the `core16` base?07:30
zygabrlin: I think it is mainly time07:31
zygabrlin: we want core16 to be different from core07:31
zygabrlin: it should behave like core1807:31
zygabrlin: but have the contents of core07:31
zygabrlin: the plan is  to allow "core" to be used instead of core1607:32
zyga(so no new traffic)07:32
zygabut using core18 semantics07:32
zygawhere semantics is really about how it behaves on a system using "core" as boot base07:33
brlinzyga: Thanks for the explaination07:33
pedroniszyga: mvo: I think it should be possible to tweak mvo PR about that,  I alos still wonder whether we can compute the base root path early07:34
zygapedronis: ah, right, this all started with an initial PR07:34
pedronismvo: hi, I added a card about CommandFromCore07:34
zygapedronis: we can but it should not block the work right now, I will add a patch for that soon, just trying to wrap up some of the existing fixes07:34
pedronismvo: assigned to you but feel free to give it to somebody else if that works better07:35
pedronismborzecki: image has been refactored many times, it might be that some bits are not needed anymore, I wouldn't jump to conclusions to fast either tough, it's pretty delicate/subtle, though there are tests07:36
zygapedronis: just checking, did you see https://bugzilla.suse.com/show_bug.cgi?id=1127366#c14 ?07:36
pedronisno07:37
zygathere are some interesting bits there07:37
mborzeckipedronis: sure, i was just trying to find out why i did not run into this before07:41
* zyga considers breakfast a good thing and goes07:42
mupPR snapd#6687 opened: snap-confine: set rootfs_dir in sc_invocation struct <Created by mvo5> <https://github.com/snapcore/snapd/pull/6687>07:53
mvopedronis: I create a PR to calculate the base root path (cc zyga)07:53
mvopedronis: will look at comand-from-core, thanks for this07:53
mvozyga: I can tweak my PR about core->core16 fallback, should be easy now thanks to your work  on this :)07:54
pedronismborzecki: so I think it's correct that we don't mostly need the PreferLocal anymore,  the only things that goes wrong is the message Copying vs Fetching07:54
pedroniswhich could be moved inside acquire itself07:54
pedronisthe ListContains is still not quite right though07:56
pedronisit should use local.hasName07:56
pedronismvo: mborzecki: I'll pick up that PR myself today or tomorrow08:03
mvopedronis: thank you08:04
mvozyga, pedronis just fyi - I updated the old 6418 (core16->core fallback) to use the new place for this08:10
pedronismvo: it looks right to me, but I haven't relooked at the whole PR08:15
mvopedronis: no worries, I'm looking at adding one more test that zyga suggested08:17
zygadrwxr-xr-x 6 root root 4096 Apr  4 07:46 /root08:22
zygawhy is root so weirdly open on core?08:22
zygait  does't seem to be open in real core08:23
zygajust in our tests08:23
dot-tobiasHi everyone08:23
zygaperhaps /root is from writable somewhere08:24
zygabut has wrong permissions08:24
Chipacazyga: wrong how?08:25
zygaChipaca: it's too open08:26
zygals -ld /root08:26
zygaand compare08:26
pedronisah, /root,  me was reading /root as / (the brain is funny)08:27
zyga:D08:27
zygayes some overloaded meanings abound08:27
brlindot-tobias: Hello08:29
* zyga looks at a real core device08:30
mborzeckizyga:  drwx------  4 root root 4096 Apr  4 07:04 root08:31
zygais that from core16?08:31
mborzecki1808:31
zygaok08:31
zygaso real core devices are probably ok08:31
zygaso our tests are broken somehow08:31
zygaFound it08:36
zygafixing08:36
dot-tobiasShort question re: cloud.conf in gadget snaps: Reading https://forum.snapcraft.io/t/how-to-preconfigure-custom-image/4154/15 and https://forum.snapcraft.io/t/cloud-init-with-netplan/1301/5 , it seems to me this does not work (properly / yet). The docs for gadget snaps just mention “optional cloud.conf” (https://docs.snapcraft.io/the-gadget-snap/696) Can someone confirm or point me to further resources? ping ogra 😊08:46
zygahmmm08:48
zygait seems that we don't have some modules and ubuntu core cannot be used in vmware08:49
pedronisit's quite possible08:50
pedronispstolowski: degville: hi,  I did a quick read over the hot plug docs, they look overall08:50
pedronis*look good08:50
pedronisI left a question/comment08:51
zygausing IDE disk fixes this08:53
zygaso that's good08:53
zygaand I can boot core in vmware08:54
zygaso useful :)08:54
pstolowskipedronis: ty08:55
zygazyga@core16:~$ sudo snap refresh08:56
zygaerror: cannot refresh []: cannot refresh snap-declaration for "snapweb": parsing assertion headers: expected 4 chars nesting prefix after multiline introduction "plugs:": "  network:"08:56
zygahow do we update from vanilla core systems again?08:56
zyga(this is a freshly installed core system as obtained from cdimage)08:56
pedroniszyga: there's a bug for that08:56
zygaright, I recall this discussion08:56
zygathat's not great :/ I thought the store had fixed the response08:57
pedronisthey haven't yet :/08:57
zygaso we should regenerate our core images08:57
degvillepedronis: thanks for the review! I'll work on those two comments now.08:57
pedronishttps://bugs.launchpad.net/snapstore/+bug/180277308:57
zygathey are useless08:57
mupBug #1802773: Cannot refresh from ubuntu-core 2.15.2 on raspberry pi "ubuntu core 16" image - snap-declaration for "snapweb": parsing assertion headers: expected 4 chars nesting prefix after multiline introduction "plugs:": "  network:" <regression> <snapstore-deployment> <snapd:Confirmed> <Snap08:57
mupStore:New for wgrant> <https://launchpad.net/bugs/1802773>08:57
zygapedronis: how can we escalate this?08:58
zygaI will install a core18 system in the meantime09:00
zygabut it feels like the image should be refreshed to current core1609:00
zygaand we should remove snapweb09:00
degvillepstolowski: after I've made those edits, would you like me to move the Hotplug support doc over the forum, or would you prefer to do it?09:02
zygathe qemu images are whooping 4GB09:03
zygathey are not sparse09:03
zygacompression "helps" but this looks like an oversight09:03
zygapedronis: who can I poke about that?09:03
pedroniszyga: I think mvo needs to decide what he wants to do, I poked a couple of times already in the past09:04
zygamvo: ^09:05
pstolowskidegville: please do, thank you!09:06
degvillepstolowski: np!09:06
zygasil2100: we cannot set hostname on a core18 system09:08
zygazyga@localhost:~$ sudo hostnamectl set-hostname core1809:08
zygaCould not set property: Failed to set static hostname: Read-only file system09:08
zygaon core18 the system-shutdown helper is not working09:09
zygaI just got a failure about that when rebooting09:09
zygaChipaca: ^09:09
Chipacazyga: yes we know09:09
Chipacaoh09:10
Chipacanot the last two09:10
Chipacazyga: what failure did you get? can i see?09:10
zygaa flash on the vmware screen, I'm looking at logs now09:10
Chipacazyga: if you do halt instead of reboot you should see them09:10
zygaI added persistent journal now09:10
Chipacazyga: some failures are expected (that's why the script exists)09:10
Chipacazyga: the interesting messages are post-journal09:11
Chipacai mean, systemd *execs* the system-shutdown helper09:11
zygaha09:11
zygatricky beast09:11
Chipacasystem-shutdown is pid 109:11
zygayes09:11
Chipaca:-)09:11
* Chipaca could tweak it a bit so it starts snapd and takes over the world from there09:11
sil2100zyga: hm, I thought we had this before and fixed it09:12
zygasil2100: does the fix require a new core image09:12
zygasil2100: I just installed core18 from cdimage09:12
zygaChipaca: I have the logs now, let me try to go through them09:13
Chipacazyga: if you have logs they'll say they couldn't unmount writable09:13
zygahttps://pastebin.ubuntu.com/p/WmkCSTjgnD/09:14
zygaApr 04 09:10:48 localhost kernel: systemd-shutdow: 67 output lines suppressed due to ratelimiting09:15
zygahahah09:15
zygalet me fix that09:15
Chipacazyga: that's not the helper09:15
Chipacazyga: that's systemd-shutdown09:15
Chipacazyga: to see the helper's logs, use halt09:16
Chipacaand screenshot09:16
zygahttps://usercontent.irccloud-cdn.com/file/bcoUzJVf/Screenshot%202019-04-04%20at%2011.16.38.png09:16
zygathe /dev/loop0 is interesting09:17
zygabut apart from that it looks less bad09:17
zygajust the failures are ugly to look at09:17
Chipacazyga: wrt /dev/loop0, new enough kernels auto-disassociate09:17
Chipacaso it's fine09:17
Chipacazyga: that's a successful run of the helper09:18
zygamvo: core has 2.38 in stable but snapd is 2.37.409:18
zygamvo: did we forget to push snapd snap to stable?09:18
pedronisreminder that #6577 needs a 2nd review09:19
mupPR #6577: many: make Remodel() download everything first before installing <Created by mvo5> <https://github.com/snapcore/snapd/pull/6577>09:19
zygapedronis: I will gladly look09:20
zygahttps://github.com/snapcore/snapd/pull/6642/commits/07cd4fa0911006f2bbcc6c6bebf8f1470e093765 <- this was breaking /root permissions09:21
mupPR #6642: cmd/snap-confine: prevent cwd restore permission bypass <Created by zyga> <https://github.com/snapcore/snapd/pull/6642>09:21
zygamkdir on the following line09:21
pedronispstolowski: btw were my brief comment on #6660 understandable? what is my concern there?09:21
mupPR #6660: cmd/debug: integrate new task timings with "snap debug timings" <Created by stolowski> <https://github.com/snapcore/snapd/pull/6660>09:21
* zyga goes to make coffee and goes back to do reviews 09:21
pstolowskipedronis: yes; i played yesterday with timings.Get function & a filter predicate and not making *JSON public but couldn't find a good solution09:24
pedronispstolowski: do we need to chat ?09:26
pstolowskipedronis: if you have time then yes, that would speed things up09:28
zygamborzecki: can you do a 2nd review for https://github.com/snapcore/snapd/pull/664209:28
mupPR #6642: cmd/snap-confine: prevent cwd restore permission bypass <Created by zyga> <https://github.com/snapcore/snapd/pull/6642>09:28
zygamborzecki: I will get a review from jamie as well09:28
mborzeckizyga:  sura09:28
zygamborzecki: I will be working on fixing the core16 image permissions today, I will not merge before that lands09:29
zygamborzecki: FYI: core18 was fixed and merged (with tests) yesterday09:29
mborzeckizyga: btw. can you take a look at https://github.com/snapcore/snapd/pull/6661 later on? it's the last one09:30
mupPR #6661: data/selinux, tests/main/selinux-clean: fine tune the policy, make sure that no denials are raised <SELinux> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6661>09:30
mvozyga: thats a question for cachio, we need to check where the validation stands09:31
zygamvo: is snapd.snap validated as a part of core18?09:33
zygamborzecki: sure09:33
mvozyga: re "I think mvo needs to decide what he wants to do, I poked a couple of times already in the past"> can you give me some more context? I read backlog but its not obvious, sorry09:33
mvozyga: yes09:33
mvozyga: see the trello card on releasing 2.3809:33
mvozyga: the usual flow is that QA validates core (uc16) and core18+snapd (uc18)09:34
zygamvo: stale core16 images09:34
zygamvo: our core16 images are 100% useless now09:34
zygamvo: because of a store bug that prevents old snapd from refreshing09:34
mvozyga: and if the core18 image is fine we promote that too09:34
zygamvo: we should regenerate them to have core instead of ubuntu-core and to have more recent snapd09:34
mvozyga: what is this bug? that sounds serious?09:34
zygamvo: we should also drop snapweb09:34
zygamvo: https://launchpad.net/bugs/180277309:35
zygamvo: it's trivial to reproduce, I hit this a moment ago09:35
mupBug #1802773: Cannot refresh from ubuntu-core 2.15.2 on raspberry pi "ubuntu core 16" image - snap-declaration for "snapweb": parsing assertion headers: expected 4 chars nesting prefix after multiline introduction "plugs:": "  network:" <regression> <snapstore-deployment> <snapd:Confirmed> <Snap09:35
mupStore:Confirmed for wgrant> <https://launchpad.net/bugs/1802773>09:35
mvozyga: fwiw, the core images are created by foundations, I raised the topic of stale images in the meeting with them on monday and they are on it (cc pedronis) - not sure what I need to make up my mind about but happy to do so (once I learn more :)09:35
mvozyga: "ubuntu-core" ?!?!09:35
mvozyga: we have an image that is still on ubuntu-core for download?09:35
zygamvo: yes09:36
zygathat's the reference image we have09:36
zygathat's great, right?09:36
mvozyga: that sounds incredible wrong09:36
zygaindeed09:36
mvozyga: and that is not something I was aware of, we need to fix this. where is that image linked?09:36
zygamvo: http://cdimage.ubuntu.com/ubuntu-snappy/16.04/current/09:37
zygaperhaps the whole directory should go09:37
zygaI bet the remaining images are equally broken09:37
mvozyga: the site is a bit of a mess - the right link is "ubuntu-core" and that has more current images. but we need to remove this dir09:38
zyga+109:38
pedronismvo: there are two issues, one is the images, the other is that bug, old snapd still using the old assertions end point get assertion formats they cannot parse09:40
pedronispstolowski: can you caht now?  otherwise it needs to be this afternoon or tomorrow09:41
pstolowskipedronis: now is fine09:42
pedronispstolowski: I'm in the standup09:43
mvopedronis: not disputing the bug :) was just curious what I need to make my mind about. also the fact that we have a "ubuntu-snappy" link on cdimage is highly confusing so that needs fixing too09:43
mvo6641 is ready for a re-review09:44
zygamvo: I will look, going through remodel first09:46
mvozyga: ta09:54
Chipacapedronis: https://forum.snapcraft.io/t/building-with-build-snaps-bottle-necking-on-fuse/10747 fwiw10:06
Chipacapedronis: fuse bottlenecks on lxd, because snapfuse is apparently single-threaded10:07
Chipacapedronis: setting the flag to unsquash on install doesn't work because fuse detection gets in the way10:08
Chipacapedronis: disabling fuse detection breaks the sanity check10:08
Chipacapedronis: :-(10:08
pedronisChipaca: is it urgent?10:08
Chipacapedronis: for who :-)10:09
pedronisChipaca: sorry, I'm asking why you raise it now instead of standup I suppose10:10
Chipacapedronis: ah, because i want to forget about it and get back to what i'm supposed to be doing :-)10:10
Chipacabut i don't want to strand sitter with no response beyond "yeah sucks"10:10
pedronisChipaca: you should have poked me from in the forum10:10
pedronisI think10:10
Chipacapedronis: ok, fair10:10
pedronisfor such a case10:10
Chipacapedronis: i was chatting with sitter in #snapcraft, dumping to the forum for posterity10:11
Chipacabut yeah10:11
Chipaca#snapcraft is not logged :-(10:11
pedronisanyway I don't think we recommend a _TEST thing even if it worked for building10:12
pedroniswe would need to make that more first class10:12
pedronisbut how much unclear10:12
pedronis(independetly that it chokes atm)10:12
Chipacapedronis: agreed10:14
Chipacapedronis: anyway, pinged you in the forum so it can carry on async'ly10:14
Chipacapedronis: ~3 months until they need it in production10:16
* Chipaca gets back to work^Wcoffee10:16
mupPR snapd#6688 opened: gadget: add validation of cross structure overlap and offset writes <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6688>10:26
mborzeckimvo: ^^ if you have some time for reviews10:26
mborzeckiback to zyga's PR10:27
mvomborzecki: heh, this sounds complicated :)10:27
mupPR core18#120 closed: Backport wpa_supplicant.service.d/snap.conf from core <Created by sil2100> <Merged by sil2100> <https://github.com/snapcore/core18/pull/120>10:35
Chipacastepping away for a bit, bbl10:38
mborzeckizyga: quick questions about opensuse, snapd is there in a separate project atm right?10:55
zygayes10:55
zygapending inclusion after changes raised through the security review10:55
zygaperhaps 2.39 actually ..10:55
zygaif I'm fast enough10:55
mborzeckizyga: wdyt about extending tests/upgrade/basic to add the repo and install the snapd package from there?10:56
zygayeah, it makes sense, thank you for raising it10:57
threshis there a way for snapcraft push to try a couple of times without shell scripts hacking around it?11:13
threshgot a 500 this morning in the CI11:13
mborzeckizyga: some comments under #664211:18
mupPR #6642: cmd/snap-confine: prevent cwd restore permission bypass <Created by zyga> <https://github.com/snapcore/snapd/pull/6642>11:18
zygamborzecki: thank you11:18
zygamvo: reviewed the remodelling branch11:18
zygamborzecki: replied11:21
mborzeckizyga:  i think I"m missing something about O_PATH, https://paste.ubuntu.com/p/KNcd5RXDdw/11:24
mborzeckizyga:  there's still permissions on the prefix that are needewd11:25
zygamborzecki: mmmm11:26
* zyga checks11:26
zygaah, indeed11:28
zygathat's a nice catch11:28
zygaabout O_PATH: https://www.irccloud.com/pastebin/8TPWkUbn/11:28
zygaI will adjust the code11:28
mborzeckizyga: ok11:29
pedronispstolowski: I +1ed 6665 with a small comment11:42
pstolowskity11:43
zygamvo: https://github.com/snapcore/snapd/pull/6641#pullrequestreview-22271238211:54
mupPR #6641: snap-gdb-shim: switch to the SUDO_UID when available <Created by mvo5> <https://github.com/snapcore/snapd/pull/6641>11:54
zygamborzecki: looking at https://github.com/snapcore/snapd/pull/6661/files now11:56
mupPR #6661: data/selinux, tests/main/selinux-clean: fine tune the policy, make sure that no denials are raised <SELinux> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6661>11:56
mborzeckizyga: thanks!11:58
pedroniszyga: I made a comment in #667312:16
mupPR #6673: cmd,tests: forcibly discard mount namespace when bases change <Created by zyga> <https://github.com/snapcore/snapd/pull/6673>12:16
zygapedronis: looking12:17
zygapedronis: that's a fair comment, for now using os-release should work but I agree ideally we'd know better. I will think about what to do best12:18
zygapedronis: we can actually use /run for that, it's always a tmpfs that we get for free12:37
zygapedronis: the only problem is that this doesn't tell us the name of the base in absence of that file, which may require a fallback to something else (or switch to probing)12:38
pedroniszyga: that is always bound mount from the host, no?12:38
zygayes12:38
pedronisis not sharing back though?12:38
zygaI don't understand what you mean12:38
zygawe don't need to mount anything, just leave a trace what we did12:38
zygalike we write to /run/snapd/ns/12:39
zygawe can also store meta-data file for each mount namespace there12:39
pedroniszyga: I think I need to understand a bit more what you plan to use, we need to find the info that correspond to the namespace12:41
zygapedronis: we can either determine the base snap name without any lookaside storage12:42
zygapedronis: that may be possible12:42
zygapedronis: or we can simply write the name of the base12:42
zygapedronis: just like we do with mount profiles12:42
pedroniszyga: sorry, we need the name of the base that the namespace was made with12:43
pedronisnot just the name of the current revision name12:43
zygapedronis: I know that, that's what I meant12:43
zygapedronis: we may be able to infer that from the mount namespace alone12:44
zygapedronis: but if that's hard or we cannot do that for whatever reason we can just write that down when we first create it12:44
pedroniszyga: I agree,  I'm still not sure where you plan to write it down12:44
zygapedronis: if we write it down it can go to /run/snapd/ns/$SNAP_INSTANCE_NAME.info12:44
pedroniszyga: and we remove it and rewrite when we discard the namespace ?12:45
zygayes12:45
zygapedronis: just like the mount profiles12:45
zygapedronis: we might even include it in the mount profile itself as a comment but I think that'd be a stretch12:45
pedronisthat's were I get lost, I though the mount profile is written by snapd12:46
zygapedronis: there are two files12:46
zygapedronis: snapd writes "what I want"12:46
zygapedronis: snap-update-ns writes "what I did"12:46
pedronisok, so a sibling to the latter12:46
zygapedronis: snap-confine could write "what I started with" until that logic migrates to snap-update-ns12:46
mborzeckimvo: need to step out and skip the standup, i'll send a note in the forum12:46
zyga(I'm working on moving most of the initialization to go but that's a slow process that I don't spend much time on)12:47
zygapedronis: but yes, a sibling to what the lower layers write12:47
pedroniszyga: I think I got lost because for a while it sounded like our only option was to read something from inside the namespace12:48
pedronisbut that's not true12:48
pedronisI don't think we need to be clever here, if blunt and simple works I'm all for it12:48
zygapedronis: that's correct, we can do both12:48
zygapedronis: reading something from inside the namespace has one advantage12:48
zygapedronis: migration is easier12:48
zygapedronis: no new data is required12:48
zygapedronis: I agree12:48
zygapedronis: less complexity is good12:48
Chipacapstolowski: your changes to #3/4 LGTM12:49
zygapedronis: perhaps base snaps could have one extra file12:49
zygapedronis: /meta/snap.info with simple key=value pairs :)12:49
pedroniszyga: no, that is not simple12:49
zygapedronis: for things we wish we could get out of the yaml12:49
pedronisis simple for us12:49
pedronisbut not overall12:49
Chipacapstolowski: there's still the bit where you have a day constant that i suspect isn't tested :-)12:49
zygapedronis: perhaps, I think it'd be nice though, it's not like it's a complex cost for anyone12:50
pedroniszyga: we have meta/snap.yaml12:50
pedronisit has a name too12:50
zygayes but parsing a simpler format with limited data would be beneficial12:50
pedronisI understand but that just weird12:50
zygaparsing yaml is pretty hard12:50
pstolowskiChipaca: ty! did you notice the change to doForget?12:50
zygayeah, I'm not seriously proposing it for this use case12:51
pedroniswe can write the simple stuff ourselves12:51
pedronissomewhere12:51
zygaI think it'd be nice but it's not required for sure12:51
zygapedronis: that brings me back to one idea12:51
zygathe key=value facts file12:51
zygawe could stop trusting environment12:51
zygaI would love if we did that12:51
pedronisI don't think is nice to be clear12:51
pedronisfor the record12:51
Chipacapstolowski: I did! you could check it's auto before trying to remove it, no?12:51
zygait's a pending security hole if someone finds a way to abuse it by confusing snap-confine12:51
pedroniszyga: confuse what?12:52
zygaconfuse snap-confine by setting variables we depend on and provide additional command line options12:52
pstolowskiChipaca: yes, indeed! will add12:52
zygawe could write information that a snap has classic confinement and not depend on --classic12:52
Chipacapstolowski: no biggie as it's a reasonably fast op12:52
Chipacapstolowski: but, ¯\_(ツ)_/¯12:52
zygawe could write information about the base snap and not depend on --base-snap-name or whatever the argument is12:52
zygawe could write snap instance name and not depend on the environment variable12:53
zygathat would be much stronger guarantee than what we have now where all of this is untrusted input12:53
pedronisthat's sounds an ok plan, but sound still different than current12:53
zygathat we need to carefully parse and cross check12:53
pedronisproblem12:53
zygayes12:53
pedronisthat's about current snap revision12:53
zygabecause it would be snapd writing that12:53
pedronisnot current actual namespace12:53
zygaand here it would be snap-confine12:53
pstolowskiChipaca: nb, i noticed that doForget test(s) don't set set-id on the test task, not sure it's a an issue or not, we probably are not testing doForget as througly as we could (but since all these bits are well tested separately, than maybe that's ok)12:54
zygabut if we agree on some basic snap-confine provided "here is what I did" file in /run that helps me a lot12:54
zygabecause I will also immediately use it for nvidia migration later on12:54
zygae.g. I could write: base_snap_name=foo and nvidia_mode=legacy-mount12:54
Chipacapstolowski: patches welcome :-þ12:54
zyga(or legacy-symlinks)12:55
zygapedronis: so if you agree on storing simple meta-data like that I'm super happy to pursue that12:55
zygapedronis: in addition I can do one special optimization12:55
zyga:)12:55
zygapedronis: if base snap name is "core" I will not write it12:55
zygathen migration is free :)12:55
zygayou can migrate from core to core18 and from core18 to core12:56
zygaand from old snapd that didn't know about this to new snapd that does12:56
pstolowskiChipaca: yep, i can re-visit this soonish, didn't touch it now as it's too easy to get side-tracked ;)12:56
Chipacapstolowski: *applause*12:57
pstolowskii think doForget should error-out if set id is not provided, since client requires it afair12:57
mupPR snapd#6665 closed: overlord/ifacestate: implement String() method of HotplugDeviceInfo for better logs/messages <Hotplug 🔌> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6665>13:03
ogradot-tobias, i dodnt touch cloud-init, specifically not on core ... but ondra has some experience with it13:03
=== ricab is now known as ricab|lunch
dot-tobiasogra Thanks, will ping him with a forum question13:16
ograhe is here as well but might not watch IRC all the time13:17
=== cpaelzer_ is now known as cpaelzer
zygamborzecki: can you have a quick look at https://github.com/snapcore/snapd/pull/6642 -- I fixed the issue with O_PATH13:40
mupPR #6642: cmd/snap-confine: prevent cwd restore permission bypass <Created by zyga> <https://github.com/snapcore/snapd/pull/6642>13:40
Chipacamvo, pedronis, I'll be offline for a bit while I relocate back; this was a bad idea ;-)13:48
pedronisChipaca: ok13:48
Chipacacmatsuoka: I'll review the delete-user pr when i'm back13:48
zygamborzecki: I started going through the selinux change but it's not easy13:48
Chipacaif that's ok13:48
Chipaca(otherwise i could probably sneak it in now while t'internet still works here ...)13:48
zygait's hard to map a type to a comment with a path that justifies it13:48
mvoChipaca: no worries, see you13:49
cmatsuokaChipaca: no hurry, I'll work on that after finishing a snapcraft assignment that should take the entire afternoon13:53
* cmatsuoka vs patchelf13:54
mborzeckire13:59
mborzeckizyga:  thanks! ;)13:59
mborzeckizyga: ping me if you have questions13:59
zygaack13:59
mupPR snapcraft#2522 opened: build providers: add API for friendly instance type names <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2522>14:01
=== kali___ is now known as kaliy
=== ricab|lunch is now known as ricab
=== chrisccoulson_ is now known as chrisccoulson
mupPR snapcraft#2521 closed: cli: cleanup environment detection <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2521>14:52
zygahttps://github.com/snapcore/snapd/pull/6684 needs a 2nd review15:17
mupPR #6684: overlord,tests: perform soft refresh check in doInstall <Created by zyga> <https://github.com/snapcore/snapd/pull/6684>15:17
* cachio lunch15:54
brlinStrange warning in Solus 4 https://usercontent.irccloud-cdn.com/file/vnB4i37M/Screenshot_20190404_235706.png15:57
brlinIs this normal?  I assume this is the problem that causes the `execv failed: No such file or directory` error when running core18 snaps. https://usercontent.irccloud-cdn.com/file/nxT13Vc8/Screenshot_20190405_000148.png16:05
zygare16:10
zygabrlin: looking16:10
brlin<316:10
zygabrlin: so I'm confused as to what you are referring to16:11
zygain the first image I see that sudo resets PATH16:11
brlinSolus appears to not set /snap/bin as one of the PATHs in the sudoers policy16:11
zygabrlin: run sudo env | grep PATH16:11
zygain the second case16:12
zygaI don't see any errors at all, unless I'm missing something16:12
zygajust regular debug output16:12
brlinzyga: Is the "current mount profile (after applying changes)" line expected to be (none)?16:13
brlinzyga: ```16:13
brlin$ sudo env | grep PATH16:13
brlinPATH=/usr/sbin:/usr/bin:/sbin:/bin16:13
brlin```16:13
zygayes, because user mount profiles are not preserved yet16:13
zygathere's a long effort that is close to being do to change that16:14
=== pstolowski is now known as pstolowski|afk
zygabrlin: in addition it is a special mount entry that we only apply if the source and destination exist16:15
zygabrlin: it is a mount entry for the document portal16:16
brlinOh, that makes sense now, thanks for the help.16:16
zygabrlin: thanks for checking, sorry that this is not documented more clearly16:18
zygaperhaps we should log something when the error is ENOENT16:18
brlinWell it is a debug output, I expect no more what it already is.16:19
zygaI'm working on the next batch of refactoring for that area16:19
zygathe whole interaction will be much easier to follow and extend16:19
zygaI strongly want to finish that to allow me to open a new feature16:20
zygawhere the ~/snap directory is gone :)16:20
zygabut ... complexity and layering16:20
zygamvo: wanna do a 2nd review for https://github.com/snapcore/snapd/pull/6684 ? :)16:21
mupPR #6684: overlord,tests: perform soft refresh check in doInstall <Created by zyga> <https://github.com/snapcore/snapd/pull/6684>16:21
zygamvo: refresh app awareness awaits :)16:21
cachioniemeyer, hey, I have another PR to review https://github.com/snapcore/spread/pull/7016:23
mupPR spread#70: Close ssh connection when reboot is stuck <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/70>16:23
cachioit is an old one which is still needed16:23
cachioniemeyer, thanks16:25
brlinIt seems that Solus have solved the core18 snaps' launching problem: https://dev.getsol.us/R3609:5fd02a7c03eca0f53aa389a701385558cf7fd42316:37
zygamvo: if still around: https://github.com/snapcore/snapd/pull/6687/files#r27226609516:37
mupPR #6687: snap-confine: set rootfs_dir in sc_invocation struct <Created by mvo5> <https://github.com/snapcore/snapd/pull/6687>16:37
mupPR snapd#6667 closed: tests: enable tests that write /etc/{hostname,timezone} on core18 <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6667>16:37
mvozyga: thanks ,I check it16:38
mvo6595 needs a review (should be easy)16:38
pedronismvo: cachio asked for the timeout to be 5m16:40
pedronisthat hasn't been applied afaics16:40
zygamvo: reviewed16:41
mvopedronis: uh, thanks, will fix16:41
zygaah, indeed16:41
zygamvo: wait16:41
zygaI can change that in a suggestion16:41
zygaand you can just click on "commit" :D16:41
zygamvo: look16:42
zygamvo: refresh and click on what you like :)16:42
zygajdstrand: hey16:44
zygajdstrand: gentle ping for https://github.com/snapcore/snapd/pull/660516:44
mupPR #6605: cmd/libsnap,osutil: fix parsing of mountinfo <Created by zyga> <https://github.com/snapcore/snapd/pull/6605>16:44
sil2100zyga: re: the hostname problem (sorry for looking into it only now) - what image were you using?16:44
zygasil2100: hey16:45
zygasil2100: I got the image following the usual links from the ubuntu download website16:45
zygasil2100: I debugged it a little and it's curious16:45
zygait *gets* set16:45
zygabut there is still an error displayed16:45
sil2100zyga: ah, yeah, ok, so it's fixed in the current one, as I thought this was the thing we were fixing in systemd AFAIK16:45
zygalet me show you16:45
zygacurrent one?16:45
sil2100zyga: so if you fetch the latest daily, it seems to work16:45
mupPR snapd#6689 opened: tests: run create-user on core devices <Created by mvo5> <https://github.com/snapcore/snapd/pull/6689>16:45
zygahmm16:46
zygadaily image or daily snap?16:46
zygasorry, please be specific16:46
jdstrandzyga: yes, I have that and 3 other PR reviews. Is this super-urgent and blocking a whole bunch of things? I'm still working through a rather large todo list and PR reviews are high on it. hoping to get to it today otherwise tomorrow16:46
sil2100zyga: http://cdimage.ubuntu.com/ubuntu-core/18/20190404/ <- try using this one16:46
zygaI just booted the vm and hostname is reset16:46
zygajdstrand: no, not urgent16:46
sil2100zyga: and tell me if it's still busted there16:46
cachiomvo, pedronis yes, we need 5 minutes of timeout because otherwise it fails on the boards16:46
zygasil2100: can I refresh to a different channel?16:47
sil2100zyga: (since maybe my test-case is wrongish, it's been a long day)16:47
jdstrandok, then I will proceed with the priorities I outlined earlier in the week and will get to it16:47
zygajdstrand: +1, thank you16:47
zygasil2100: I can try a fresh vm tomorrow, since it is marginally more work,16:47
sil2100zyga: guess you can try the core18 from edge/beta too16:48
sil2100Since I suppose you reproduced it on the stable one, right?16:48
cachioin the past we updated all the short timeout to 5 minutes because of that16:48
zygasil2100: or I can refresh to new channel to see16:48
zygaI can try that16:48
zygacorrect16:48
zygasil2100: trying core18 from edge16:49
sil2100Fingers crossed!16:49
cachiomvo, I can make that change if you want16:50
zygasil2100: edge works16:50
zygasil2100: even after reboot16:51
zygasil2100: thanks!16:51
sil2100zyga: thank mvo for that! He prepped the systemd changes to make it work o/16:51
zygamvo: thank you then :-)16:51
sil2100zyga: but phew, for a moment thought the systemd changes weren't enough, and I just released those to -updates16:52
zygahaha16:52
zygasome cold sweat :)16:52
zygait's great, thanks16:52
zygahmmm16:53
zygaare core18 builds going?16:53
mvocachio: thank you16:54
zygaone of my tests failed two hours ago because core18 didn't have /var/lib/snapd/snap16:54
zygaer16:54
zygaI meant /var/lib/snapd/void16:54
cachiomvo, change done, now waiting for tests results16:55
mvocachio: ta16:55
cachiomvo, yaw16:55
* zyga EODs16:58
zygamvo: if you can review / approve 6684 I would love to end the day with that16:59
mvozyga: sure17:02
degvillepedronis: pstolowski|afk: those docs are now published - https://docs.snapcraft.io/developing-hotplug-interfaces and https://docs.snapcraft.io/hotplug-support. I still need to add some 'hardware interface' clarifications to the relevant interface docs.17:05
brlinI wonder what's the usage of `/var/lib/snapd/void` ?17:05
sil2100zyga: it should have, since we had a new core18 package yesterday, an hour ago and even a few minutes ago!17:08
pedronisdegville: thx17:12
brlinzyga: Thanks for checking out the Solus issue!17:13
pedronisdegville: let me know if you need help to find out which ones those are17:13
degvillepedronis: thanks, will do!17:14
zygabrlin: AFK17:20
zygabrlin: literally holding my daughter and typing with one finger17:21
brlinzyga: Take your time, then ;)17:21
=== JanC is now known as Guest19220
=== JanC_ is now known as JanC
zygadrat17:50
zyga++ stat -c %a /var/lib/snapd/void17:51
zygastat: cannot stat '/var/lib/snapd/void': No such file or directory17:51
zygaearlier in the log I see17:52
zyga+ rsync -av --delete /home/gopath/src/github.com/snapcore/snapd/tests/snapd-state/snapd-lib/snaps /var/lib/snapd17:52
zygaI bet the state is corrupt somehow and something removes writable directories17:52
zygaI'll run that test in isolation to see if it passes17:52
zygabrlin: the void directory is a fallback location17:54
zygabrlin: when snap-confine starts a program and cannot represent the original working directory inside the changed filesystem17:54
zygabrlin: it uses /var/lib/snapd/void17:54
mupPR snapd#6684 closed: overlord,tests: perform soft refresh check in doInstall <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6684>18:00
zygamvo: around?18:04
zygamvo: what writes /writable/system-data/?18:15
zygais that ubuntu-image?18:15
zygamvo: I just realised that all of the work to make core18 have skeleton tree was 100% useless18:15
pedroniszyga: ?18:17
zygapedronis: /var/lib/snapd and /var/cache/snapd are bind-mount to system-data/18:17
zygaand the content in the boot base is irrelevant18:17
zygathe real change has to be done elsewhere18:17
pedroniszyga: does it mean the opengl stuff also never worked on core?18:18
zygapedronis: yes, it was never supported on core to begin with18:18
zygapedronis: unless the snap ships all the stuff internally18:19
zygapedronis: I need to untangle this tomorrow18:19
zygait's too late today18:19
pedroniszyga: anyway we need to discuss because making it ubuntu-image problem sounds kind of wrong18:19
zygapedronis: I think we need to start by saying what we want to have18:20
zygacomparing it to what we have now across bases and history cruft18:20
zygaI'm just puzzled by how this works18:20
zygathis is also compounded by writable paths and general complexity associated with how that operates18:20
pedronisit's fine, we can discuss tomorrow/next week18:21
zygaand what is the configuration of the initial mount namespace on core devices18:21
zygayes, I'm just puzzled by how complex this turned out to be, it was supposed to be a bugfix for a low-priority CVE18:21
pedronisI thought we were already using void18:22
pedronisfor some things18:22
zygayes, but nothing tested it on core1818:23
zygait doesn't work18:23
pedronisotoh we know that core 16 mount setup with core !=  core 18 core1818:23
zygayes, exactly18:23
zygaon core16 it does work18:23
zygabecause what the core snap carries does matter :)18:23
pedronisthat is still confusing to me18:24
pedronisthe host has still writable parts18:24
pedronisthat come from /writable18:24
pedronisdid we simplify writable path config too much on core18 ?18:25
zygapedronis: I don't know what the config for core18 is18:25
zygapedronis: we should sit down and discuss this for core20 context18:26
zygapedronis: at some point in the past I wanted to have a mini-version of snap-confine that does just the mount namespace setup for the initial mount namespace to replace the shell logic currently in the initrd18:26
pedronisheh, sounds like a super tangent18:26
zygabecause keeping that in the core-xxx repo on the side not helping with understanding what is going in18:26
zyga*on18:26
zygayes, to some extent yes18:27
zygabut the point was to bring the magic closer to snapd repo18:27
zygaso that we see what's the state on boot18:27
zygaright now that is locked away in an untested shell script in an initrd somewhere18:27
zygait's not exactly transparent :)18:27
zygain some way it also relates to snapd.snap and bases18:28
zygait would be good to have snapd in the loop of the initial mount namespace construction18:28
zygaright now we cannot evolve it easily18:28
zygapedronis: while I have you: how long should I allow running app to inhibit refresh?18:33
pedronisI need to think a bit, we have a policy for servers, but not sure it applies here18:39
zygaI will set one week as a strawman for now18:40
pedroniszyga: so yes writable-path config for /var/lib/snapd for core 18 and core 16 is different18:43
zygaI remember that mvo wanted to simplify it18:43
pedroniswe'll need to chat with him18:44
mvozyga, pedronis I'm sort of here but would like to EOD - anything urgent?18:45
mvoI did simplify wirtable-path, does it mean anyhting is missing?18:45
mvoalso remember core18 is smaller than core so some things just did not made any sense anymore18:46
pedronismvo: yes,  /var/lib/snapd is missing stuff in core 1818:46
pedronisapparently18:46
zygamvo: I think it's a conversation for tomorrow18:46
pedronisbut is not urgent18:46
zygamvo: we are untangling the layers of why things are broken :)18:46
pedronisand will probably be complicated to untangle at this point18:46
pedronisanyway18:46
mvopedronis, zyga right - its missing stuff because in core we installed snapd so all the right dirs were there18:46
mvoin core18 we don't anymore18:46
mvozyga: oh, i see18:46
pedronismaybe18:47
mvozyga: its /var/lib/snapd in writable-path, correct?18:47
pedronisbut the writable-path config is also different18:47
pedronisit was synced , now is persistent18:47
mvopedronis: I think we can make it transition18:47
mvothat should fix things18:47
mvothis means whatever was there before will be surfaced but only once18:47
pedronisanyway I need to learn what those things mean :)18:48
mvosynced is a terrible setting18:48
pedronisand is not a topic for tonight at this point18:48
mvowe should not use this18:48
mvoyeah, lets sync on this tomorrow18:48
zygamvo: I think the issue is that what we assumed was the case,  directories being present in core16 because of mount namespace layout, are not true in core18 because the real set is constructed by ubuntu-image18:48
mvozyga: setting it to transition is worth a shoot18:48
mvozyga: lets chat about this in the morning18:48
pedronismvo: it's already set to transition18:50
pedronisanyway  it's likely actually that the answer is more stuff that the snapd snap needs to do on setup18:54
zygare, sorry, small accident19:02
zygaheh19:24
zygaso19:24
zygago time sucks19:24
zygatime is not restored across serialization19:25
zyganot perfectly19:25
zygathis upsets our conflict checker19:25
mupPR snapd#6690 opened: overlord/snapstate: inhibit refresh for up to a week <Created by zyga> <https://github.com/snapcore/snapd/pull/6690>19:36
* zyga EODs again, for real19:38

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