[05:20] <mborzecki> morning
[06:02] <pstolowski> morning
[06:06] <mborzecki> pstolowski: hey
[06:12] <mardy> 'morning
[06:31] <mborzecki> mardy: hey
[06:31] <mborzecki> ehh loosk like we can't really land anything now
[06:38] <mborzecki> https://status.snapcraft.io/ does not list any problems related to deltas?
[06:43] <mborzecki> nvm i see the messages on MM, maybe it's fixed now, let's see if the tests in https://github.com/snapcore/snapd/pull/10483 are happy
[06:50] <mvo> fingers crossed
[06:54] <mborzecki> mvo: morning, and yeah
[06:54] <mborzecki> mvo: can you land https://github.com/snapcore/snapd/pull/10481 ?
[06:55] <mborzecki> mvo: also, do you think we need to involve security team in https://github.com/snapcore/snapd/pull/10483 ?
[07:08] <mvo> mborzecki: sure
[07:09] <mvo> mborzecki: and good morning!
[07:12] <mborzecki> mvo: while at it: https://github.com/snapcore/snapd/pull/10494 this one can be merged too as it fixes failures in nested tests
[07:13] <mvo> mborzecki: I really don't think we need security for 10483 - otoh it should be pretty trivial, so maybe alex can have a quick look just at this specific diff?
[07:15] <mvo> mborzecki: landed
[07:15] <mborzecki> mvo: ok, let me ping him
[07:15] <mborzecki> mvo: and thank you
[07:15] <mvo> mborzecki: thank *you* :)
[07:16] <pstolowski> hey mvo 
[07:18] <mborzecki> meh, deltas are still unhappy
[07:18] <mborzecki> i wonder if some need to be regenerated or something
[07:24] <mvo> hey pstolowski 
[07:24] <mvo> mborzecki: that is a bit disappointing
[07:25] <mvo> mborzecki: what is the latest run that failed? we might want to poke the store again
[07:25] <mborzecki> mvo: pinged you in store's mm channel
[07:26] <mborzecki> mvo: this is the latest run that failed: https://github.com/snapcore/snapd/pull/10483/checks?check_run_id=2995822939
[07:28] <mvo> mborzecki: ta
[07:28] <mvo> mborzecki: I wonder if we should disable the test to unblock, it's very tedious to review the failure logs
[07:46]  * pstolowski physio
[07:54] <mvo> mborzecki: I pushed a PR to disable the delta tests for now
[07:56] <pedronis> mborzecki: hi, https://github.com/snapcore/snapd/pull/10469 needs a master merge now?
[08:00] <mborzecki> mvo: thanks
[08:00] <mborzecki> pedronis: yes
[08:30] <mardy> should an ordinarey user be able to write into $SNAP_DATA? The folder is owned by root
[08:31] <mardy> ah, that's for system data
[08:33] <zyga> good morning
[08:33]  * zyga starts late after late evening work
[08:39] <mardy> mmm... this is "funny": the mount operation succeeds, yet the mountpoint appears to be empty and /proc/mounts does not list it. I suspect it's a namespacing issue...
[08:50] <ogra`> mardy, as i asked on mattermost already, are you sure you want to actually do that, given the content of $SNAP_DATA gets forward copied during snap updates you'd have to deal with skipping the mounted bits, moving the mountpoint back and forth with snap revert/refresh etc etc 
[08:51] <pstolowski> re
[08:51] <ogra`> mardy, $SNAP_COMMON does not get copied or moved ... i'd restrict the mounting to this dir only
[08:54] <mardy> ogra`: I'm just writing a test. We are developing a mount-control interface, and I have a simple snap to test it: https://github.com/snapcore/snapd/pull/10473/files
[08:56] <mardy> so, after installing the snap and connecting the plug, I run: "sudo test-snapd-mount-control.mntctl -o bind,ro /usr/share/fonts /var/snap/test-snapd-mount-control/current/fonts"
[08:56] <mardy> the mount apparently succeeds, but /var/snap/test-snapd-mount-control/current/fonts stays empty
[08:57] <mardy> I've now verified that the /var/snap/test-snapd-mount-control/current/fonts is empty even for the process running in the snap itself, so it would seem it's not even a namespacing issue
[09:02] <ogra`> mardy, yeah, i know about the interface (i drive the customer that asked for it), i'm just thinking the restricting your whereRegexp() to /media and $SNAP_COMMON makes handling updates eaiser since a snap refresh will copy the content of the mountpoint to the dir that becomes the new current/ symlink after refresh
[09:03] <ogra`> unless you add any code to special case mounted stuff in $SNAP_DATA
[09:04] <ogra`> and additionally the mount needs to move along with the symlink going forward/backward on snap revert and refresh
[10:13] <mvo> can someone review #10495 please? this should unblock the tests until the store is fixed
[10:22] <pstolowski> +1
[10:22] <mvo> ta
[10:25] <mborzecki> pedronis: i've updated https://github.com/snapcore/snapd/pull/10493 too now
[10:48] <pstolowski> pedronis: re #7700 and the comment about waiting for runinhibit in snapRunHook, I think we shouldn't do this for any hooks?
[10:49] <pedronis> pstolowski: yea, think so
[10:49] <pedronis> mborzecki: thx
[10:51] <pstolowski> pedronis: btw do you have a moment before standup to chat about --proceed outside of hook?
[10:52] <pedronis> pstolowski: yes, please schedule something
[10:54] <pstolowski> pedronis: done
=== alan_g_ is now known as alan_g
[11:44] <mborzecki> pedronis: fun: https://github.com/snapcore/snapd/pull/10493#issuecomment-874689293 afaict our grub-recovery.cfg doesn't like 1234-2 labels
[11:44] <pedronis> mborzecki: well, as we know it's not clear we fully like our grub-recovery.cfg
[11:44] <pedronis> mborzecki: we discussed whether there are things we should change/improve with it
[11:46] <mborzecki> pedronis: yeah, i can look into updating that to be more relaxed, there's a still a question of rolling out an update if we do it
[11:46] <pedronis> yea
[11:48] <mborzecki> pedronis: i guess for now i can leave the boot config edition unchanged like we did for the run one
[11:48] <pedronis> mborzecki: yes
[11:48] <pedronis> mborzecki: fwiw we do a regexp in seed/internal about what are valid labels
[11:48] <pedronis> *do have
[11:49] <mborzecki> oh right, i can start with that
[11:58] <pstolowski> pedronis: i don't think https://github.com/snapcore/snapd/pull/10476 needs your review, does it? i would like to land it soon once green
[11:59] <pedronis> pstolowski: not particurly, no, unless you think it does, but sounds not
[12:03] <pstolowski> pedronis: ok, thanks
[12:17] <pedronis> mborzecki: I re-reviewed https://github.com/snapcore/snapd/pull/10469
[12:18] <mborzecki> pedronis: thanks
[12:18] <mborzecki> pedronis: i think i got grub under control now, still waiting for spread test to finish, but it seems to work in a vm
[12:19] <pedronis> good
[12:30] <mardy> can anyone think of some reason why mount(2) would succeed (returns 0, and no apparmor denials are seen) yet nothing really happens (the mount point stays empty)?
[14:01] <mardy> mvo: so, the branch is https://github.com/snapcore/snapd/pull/10473
[14:02] <mardy> I just build it, run the snapd from my branch, call "snap pack" on the test directory, then install the snap with --dangerous, connect its plug, and then run "sudo test-snapd-mount-control.mntctl -o bind /usr/share/fonts /var/snap/test-snapd-mount-control/common/fonts"
[14:22] <mborzecki> pedronis: i've pushed an update to https://github.com/snapcore/snapd/pull/10469 i think i understood what you wanted to be changed in the tests, but let's see
[14:23] <mborzecki> fwiw, with changed recovery grub config the remodel spread test is passing again
[14:25] <pedronis> mborzecki: thx
[14:34] <pedronis> mborzecki: re-reviewed
[14:36] <pedronis> mborzecki: thx
[14:36] <pedronis> ijohnson[m]: I tried to answer the question in https://github.com/snapcore/snapd/pull/10437 , sorry I should have been explicit in my previous comment
[14:36] <ijohnson[m]> pedronis: thanks for that
[14:37] <pedronis> mvo: https://github.com/snapcore/snapd/pull/10437 needs your re-review
[14:41] <mvo> pedronis: sure, looking
[14:43] <mardy> mvo: nevermind my mount issue: if the mount point is under /media/**, it works. For some reason using $SNAP_COMMON doesn't, but I'll investigate
[14:44] <mardy> mmm... when accessing /var from inside a snap, is that the same /var as the system's one, or does it get somehow remapped?
[14:45] <ijohnson[m]> mardy: ah are you entering into the mount namespace of the snap to check that the mount exists ?
[14:45] <ijohnson[m]> mardy: snap mount namespaces are setup such that mounts made inside them do not propagate back to the host's mount namespace, with the exception of /media/ I think
[14:46] <ijohnson[m]> mardy: have you been introduced to the `/run/snapd/ns` directory at all?
[14:47] <mardy> mvo: nah, actually /media/** does not work either, I made a mistake before :-(
[14:48] <mardy> ijohnson[m]: no, but if I list the files from the same process that does the mount(), I guess I should be already inside the correct namespace?
[14:50] <ijohnson[m]> mardy: hmm you mean like /proc/<pid>/mountinfo ? yeah that should show the mounts from that namespace
[14:53] <mardy> ijohnson[m]: no, I'm not opening mountinfo, I'm just calling (in python) os.listfiles() on the mount point, and it's empty
[14:53] <mardy> but that's from the same process who called mount(), so it should see the files...
[14:54] <ijohnson[m]> mardy: I assume both those mount process and the python program are running inside the snap confinement ?
[14:55] <mardy> ijohnson[m]: yes, it's the same process: I'm not invoking the mount(8) binary, but directly doing mount(2) from python: https://github.com/mardy/snapd/blob/mount-control/tests/lib/snaps/test-snapd-mount-control/bin/mntctl#L115-L124
[14:55] <mardy> (using ctypes)
[14:56] <mardy> if I call the python script directly, without passing via snapd, it works
[14:56] <ijohnson[m]> mardy: silly question but did you try adding a sleep after performing the mount() ?
[14:56] <mardy> ijohnson[m]: nope, let me try...
[14:57] <mardy> nope, doesn't help (3 seconds)
[14:57] <ijohnson[m]> 😕 
[14:58] <ijohnson[m]> mardy: how can I reproduce this? just build that branch of snapd `mount-control` and then install the `test-snapd-mount-control` snap ?
[14:58] <mardy> ijohnson[m]: and connect the plug
[14:59] <mardy> ijohnson[m]: then run sudo test-snapd-mount-control.mntctl -o bind /usr/share/fonts /var/snap/test-snapd-mount-control/common/fonts
[15:00] <mardy> ijohnson[m]: you can also add "-c 'ls -l /var/snap/test-snapd-mount-control/common/fonts'" to have it run an "ls" command
[15:00] <ijohnson[m]> mardy: ack thanks, I'm trying to wrap some other things up this morning but I will take a look in my PM
[15:00] <mardy> ijohnson[m]: much appreciated, thanks!
[15:01] <ijohnson[m]> np
[15:44] <ijohnson[m]> pedronis: mvo I think I figured out why my fix for #10165 didn't work, I only did the fix for one of the two tests, so now I pushed up that same fix for the second test, do you think the PR needs to be re-reviewed all over again at this point?
[15:44] <ijohnson[m]> I'm inclined to say that yes, any previous +1's should be re-done
[15:46] <mvo> ijohnson[m]: one re-review is probably enough
[15:46] <ijohnson[m]> ok
[15:47] <pedronis> mvo: I did a pass https://github.com/snapcore/snapd/pull/10264, some comments there, let me know if I'm confused
[15:50] <mvo> pedronis: looking, but it's probably me being confused
[15:59] <pstolowski> cachio: maybe you could take a look at https://github.com/snapcore/snapd/pull/10479 ? it's a followup to the spread test you reviewed yesterday
[16:02] <cachio> pstolowski, sure
[16:05] <pstolowski> ty
[16:28] <mvo> pedronis: thanks so much for the review, excellent points. I addressed everyting except for the optimization to only call the virtual configs when there is an overlap, need to have dinner first so either later tonight or tomorrow but I pushed the rest
[16:28] <mvo> pedronis: could even be a followup (maybe?)
[17:48] <mvo> 10496 needs a review, looks like deltas are happy again
[17:48] <ijohnson[m]> cachio: can you take another look at https://github.com/snapcore/snapd/pull/10165 today? thank you
[17:49] <cachio> ijohnson[m], sure
[17:49] <ijohnson[m]> mvo: sure I will take a look at that one when I'm back from lunch
[17:49] <ijohnson[m]> thanks cachio 
[17:50] <mvo> ijohnson[m]: no rush, it's pretty trivial
[17:51] <mvo> it looks like my virtual config stuff is finally making some good progress *fingers-crossed*
=== not_phunyguy is now known as phunyguy
[18:50] <cachio> ijohnson[m], +1, left few comments inline
[19:16] <ijohnson[m]> Thanks cachio
[19:16] <cachio> yaw
=== mwhudson_ is now known as mwhudson
[20:46]  * cachio afk