| mborzecki | morning | 05:20 |
|---|---|---|
| pstolowski | morning | 06:02 |
| mborzecki | pstolowski: hey | 06:06 |
| mardy | 'morning | 06:12 |
| mborzecki | mardy: hey | 06:31 |
| mborzecki | ehh loosk like we can't really land anything now | 06:31 |
| mborzecki | https://status.snapcraft.io/ does not list any problems related to deltas? | 06:38 |
| 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:43 |
| mvo | fingers crossed | 06:50 |
| mborzecki | mvo: morning, and yeah | 06:54 |
| mborzecki | mvo: can you land https://github.com/snapcore/snapd/pull/10481 ? | 06:54 |
| mborzecki | mvo: also, do you think we need to involve security team in https://github.com/snapcore/snapd/pull/10483 ? | 06:55 |
| mvo | mborzecki: sure | 07:08 |
| mvo | mborzecki: and good morning! | 07:09 |
| 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:12 |
| 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:13 |
| 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:15 |
| pstolowski | hey mvo | 07:16 |
| mborzecki | meh, deltas are still unhappy | 07:18 |
| mborzecki | i wonder if some need to be regenerated or something | 07:18 |
| mvo | hey pstolowski | 07:24 |
| mvo | mborzecki: that is a bit disappointing | 07:24 |
| 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:25 |
| mborzecki | mvo: this is the latest run that failed: https://github.com/snapcore/snapd/pull/10483/checks?check_run_id=2995822939 | 07:26 |
| 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:28 |
| * pstolowski physio | 07:46 | |
| mvo | mborzecki: I pushed a PR to disable the delta tests for now | 07:54 |
| pedronis | mborzecki: hi, https://github.com/snapcore/snapd/pull/10469 needs a master merge now? | 07:56 |
| mborzecki | mvo: thanks | 08:00 |
| mborzecki | pedronis: yes | 08:00 |
| mardy | should an ordinarey user be able to write into $SNAP_DATA? The folder is owned by root | 08:30 |
| mardy | ah, that's for system data | 08:31 |
| zyga | good morning | 08:33 |
| * zyga starts late after late evening work | 08:33 | |
| 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:39 |
| 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:50 |
| pstolowski | re | 08:51 |
| ogra` | mardy, $SNAP_COMMON does not get copied or moved ... i'd restrict the mounting to this dir only | 08:51 |
| 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:54 |
| 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:56 |
| 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 | 08:57 |
| 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:02 |
| ogra` | unless you add any code to special case mounted stuff in $SNAP_DATA | 09:03 |
| ogra` | and additionally the mount needs to move along with the symlink going forward/backward on snap revert and refresh | 09:04 |
| mvo | can someone review #10495 please? this should unblock the tests until the store is fixed | 10:13 |
| pstolowski | +1 | 10:22 |
| mvo | ta | 10:22 |
| mborzecki | pedronis: i've updated https://github.com/snapcore/snapd/pull/10493 too now | 10:25 |
| pstolowski | pedronis: re #7700 and the comment about waiting for runinhibit in snapRunHook, I think we shouldn't do this for any hooks? | 10:48 |
| pedronis | pstolowski: yea, think so | 10:49 |
| pedronis | mborzecki: thx | 10:49 |
| pstolowski | pedronis: btw do you have a moment before standup to chat about --proceed outside of hook? | 10:51 |
| pedronis | pstolowski: yes, please schedule something | 10:52 |
| pstolowski | pedronis: done | 10:54 |
| === alan_g_ is now known as alan_g | ||
| 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:44 |
| 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:46 |
| 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:48 |
| mborzecki | oh right, i can start with that | 11:49 |
| 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:58 |
| pedronis | pstolowski: not particurly, no, unless you think it does, but sounds not | 11:59 |
| pstolowski | pedronis: ok, thanks | 12:03 |
| pedronis | mborzecki: I re-reviewed https://github.com/snapcore/snapd/pull/10469 | 12:17 |
| 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:18 |
| pedronis | good | 12:19 |
| 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)? | 12:30 |
| mardy | mvo: so, the branch is https://github.com/snapcore/snapd/pull/10473 | 14:01 |
| 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:02 |
| 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:22 |
| mborzecki | fwiw, with changed recovery grub config the remodel spread test is passing again | 14:23 |
| pedronis | mborzecki: thx | 14:25 |
| pedronis | mborzecki: re-reviewed | 14:34 |
| 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:36 |
| pedronis | mvo: https://github.com/snapcore/snapd/pull/10437 needs your re-review | 14:37 |
| mvo | pedronis: sure, looking | 14:41 |
| 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:43 |
| 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:44 |
| 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:45 |
| ijohnson[m] | mardy: have you been introduced to the `/run/snapd/ns` directory at all? | 14:46 |
| mardy | mvo: nah, actually /media/** does not work either, I made a mistake before :-( | 14:47 |
| 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:48 |
| ijohnson[m] | mardy: hmm you mean like /proc/<pid>/mountinfo ? yeah that should show the mounts from that namespace | 14:50 |
| 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:53 |
| ijohnson[m] | mardy: I assume both those mount process and the python program are running inside the snap confinement ? | 14:54 |
| 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:55 |
| 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:56 |
| mardy | nope, doesn't help (3 seconds) | 14:57 |
| ijohnson[m] | 😕 | 14:57 |
| 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:58 |
| mardy | ijohnson[m]: then run sudo test-snapd-mount-control.mntctl -o bind /usr/share/fonts /var/snap/test-snapd-mount-control/common/fonts | 14:59 |
| 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:00 |
| ijohnson[m] | np | 15:01 |
| 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:44 |
| mvo | ijohnson[m]: one re-review is probably enough | 15:46 |
| ijohnson[m] | ok | 15:46 |
| pedronis | mvo: I did a pass https://github.com/snapcore/snapd/pull/10264, some comments there, let me know if I'm confused | 15:47 |
| mvo | pedronis: looking, but it's probably me being confused | 15:50 |
| 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 | 15:59 |
| cachio | pstolowski, sure | 16:02 |
| pstolowski | ty | 16:05 |
| 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?) | 16:28 |
| 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:48 |
| 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:49 |
| mvo | ijohnson[m]: no rush, it's pretty trivial | 17:50 |
| mvo | it looks like my virtual config stuff is finally making some good progress *fingers-crossed* | 17:51 |
| === not_phunyguy is now known as phunyguy | ||
| cachio | ijohnson[m], +1, left few comments inline | 18:50 |
| ijohnson[m] | Thanks cachio | 19:16 |
| cachio | yaw | 19:16 |
| === mwhudson_ is now known as mwhudson | ||
| * cachio afk | 20:46 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!