[05:14] <mborzecki> morning
[05:51] <mardy> 'morning!
[05:55] <mborzecki> mardy: heya
[06:14] <mardy> what are the dynamic plug attributes?
[06:16] <pstolowski> morning
[06:17] <mardy> hi pstolowski!
[06:31] <mborzecki> pstolowski: hey
[06:31] <mborzecki> mardy: pstolowski will probably know better
[06:32] <pstolowski> hey, what is that?
[06:32] <mborzecki> pstolowski: mardy was asking about dynamic plug attributes
[06:37] <mardy> yep, I just don't know what they are :-) And consequently, I'm trying to figure out how I should handle them in processing the snapctl requests for the mount interface. Should I just merge the two maps (static and dynamic attributes) together?
[06:46] <pstolowski> mardy: they come from interface hooks, so may be set dynamically at runtime when plug-slot get connected when 1) snap(s) implement plug/slot interface hooks and 2) the interface actually expects and allows some attributes to bet set at runtime
[06:47] <pstolowski> mardy: here is an overview https://snapcraft.io/docs/interface-hooks
[06:50] <pstolowski> mardy: i'm missing the context of what you're doing in snapctl, but i doubt you have to do anything with dynamic attrs. can you pastebin your snapctl code?
[06:52] <pstolowski> mardy: the basic idea with dynamic attributes is that some attributes may need to be determined at runtime (they cannot be hardcoded in snap.yaml for the plug/slot), .e.g a path of some resource/device
[07:01] <mborzecki> anyone remembers why we have `any-python` as an interpreter in the retry tool?
[07:09] <mvo> mborzecki: I can't say for sure but iirc it was something with fedora or so but I could be wrong. I remember looking at one of the commits that changed it and wondered about it
[07:10] <mborzecki> mvo: i'll open a quick pr dropping it, i think only 14.04 is using an older version of python3 (3.4 in fact)
[07:10] <mborzecki> other distros have at least 3.5
[07:10] <mvo> mborzecki: cool
[07:11] <mardy> pstolowski: thanks! Yes, I think in the case of the mount-control interface all attributes need to be static
[07:11] <pstolowski> mardy: excellent
[07:16] <mborzecki> mvo: https://github.com/snapcore/snapd/pull/10523 let's see if it gets green
[07:44]  * pstolowski physio
[07:45] <mvo> mborzecki: \o/ thank you! (sry, in a meeting)
[08:19] <zyga-mbp> good morning
[08:29] <mardy> zyga-mbp: hi!
[08:29] <zyga-mbp> hey mardy 
[08:29] <zyga-mbp> another rainy day
[08:30] <zyga-mbp> even air is soaking wet 
[08:55] <pstolowski> re
[09:34] <pedronis> mvo: hi, I reviewed the timings PRs
[09:43] <pstolowski> pedronis: hi, i've updated https://github.com/snapcore/snapd/pull/10511 but was a bit confused when implementing TestBackstore test for pool. It seems I needed to use two revisions in the test to hit isNewer check inside pool otherwise it woudln't get added to the backstore. I didn't dig deeper into this
[09:52] <pedronis> pstolowski: yes, that sounds the expected behavior
[09:53] <pstolowski> pedronis: but the old revision wasn't in the pool, that's what i don't get
[09:54] <pedronis> pstolowski: we have our check based on what we requested
[09:54] <pedronis> I'll look
[09:54] <pedronis> maybe I'm misunderstanding
[09:54] <pstolowski> i'm sure it works as designed and is correct, i was just confused, maybe something in my test setup is wrong
[09:55] <pedronis> pstolowski: what did you expect?
[09:57] <pedronis> pstolowski: notice that you are adding an unresolved but not with the revision set to RevisionNotKnown
[09:57] <pstolowski> pedronis: i expected it to pass after s/s.rev1_3333/s.rev1_1111/  in TestBackstore
[09:57] <pstolowski> pedronis: ah
[09:57] <pedronis> that's probably the source of your confusion
[09:58] <pstolowski> yeah i think that's it
[09:58] <pstolowski> thanks
[09:58] <pedronis> we should probably do that, it makes for a simpler test
[09:59] <pstolowski> sure, i'll try
[10:00] <pedronis> there are examples of that in other tests
[10:02] <pstolowski> indeed, they are setting atXXX "manually"
[10:05] <pstolowski> yep, works, updated
[10:06] <mardy> what is the systemd emulation (like in https://github.com/snapcore/snapd/blob/master/overlord/snapstate/backend/mountunit.go#L33-L38)? Do I have to do the same stuff in the mount-control interface, when creating the unit?
[10:12] <mborzecki> meh, we can't mock time.Now() in a central way
[10:17] <pstolowski> mborzecki: oh, what's the catch?
[10:17] <pstolowski> mvo: https://github.com/snapcore/snapd/pull/10514 has +3
[10:18] <mborzecki> pstolowski: nah, i'm just stating the fact that we don't have facilities to mock time centrally, maybe it's just not useful, but still annoying 🙂 i have a test that involves timestamps
[10:19] <pstolowski> mborzecki: ah ok, i thought there was some issue with that. yeah, we have timeNow() in a couple of places 
[10:21] <pstolowski> mardy: systemd emulation is used when preseeding (e.g. creating ubuntu image with snapd partially set up).
[10:22] <pstolowski> mardy: the idea is to not call real systemd because we aren't running on the real target system yet
[10:26] <zyga-mbp> mborzecki write a wrapper that runs unshare, switches to a user and time ns and exposes offset adjustment functions
[10:26] <zyga-mbp> global fake time
[10:26] <zyga-mbp> (per process)
[10:27] <zyga-mbp> (I'm not serious)
[10:27] <pstolowski> mardy: your interface is going to create mount units right?
[10:27] <mborzecki> zyga-mbp: even then, it would require cap_sys_admin?
[10:27] <zyga-mbp> mborzecki IIRC you could do it from a user ns
[10:28] <mardy> pstolowski: yes, when it receives a "snapctl mount --persistent ..." command
[10:29] <pstolowski> mardy: ok, that's not going to happen during preseeding
[10:29] <pstolowski> mardy: so you don't have to do anything special afaiu
[10:29] <mardy> pstolowski: nice! So, noemulation needed
[10:37] <zyga-mbp> mborzecki, I don't recall
[10:37] <zyga-mbp> don't you get CAP_SYS_ADMIN that's valid inside the user ns?
[11:08] <pedronis> pstolowski: I reviewed https://github.com/snapcore/snapd/pull/10511  we should talk about next steps at some point
[11:09] <pstolowski> pedronis: thanks, maybe later today/tomorrow?
[11:09] <pedronis> yes
[11:25] <pstolowski> hmm do we use gofmt 1.9 or 1.10?
[11:26] <pedronis> mvo: https://github.com/snapcore/snapd/pull/10264 needs more work, the new test is not quite testing the right thing so it doesn't show the problem
[11:30] <mvo> pedronis: I have a look, thank you!
[12:09] <mardy> we don't have any place in snapd where we are calling mount(8) from golang, do we?
[12:09] <mardy> sorry, meant mount(2)
[12:16] <pstolowski> i need 2nd review for https://github.com/snapcore/snapd/pull/10511 from somebody who doesn't fear assertions ;)
[12:18] <cachio_> ijohnson[m], hi, same error with the new model
[12:19] <cachio_> I am debugging a test again to check that
[12:22] <pstolowski> mardy: there is something in gadget/install, not sure if that counts. you may want to grep the codebase by syscall.Mount
[12:24] <zyga-mbp> mardy: we do
[12:24] <zyga-mbp> mardy: tons of places in snap-update-ns
[12:24] <zyga-mbp> mardy: there are lots of helpers as well
[12:25] <zyga-mbp> for all kinds of special ways to mount
[12:25] <zyga-mbp> mardy let me know if you have questions, I'd love to share 
[12:25] <zyga-mbp> (if anything I know is still useful)
[12:26] <mardy> zyga-mbp, pstolowski: looks like grepping for syscall.Mount helped, thanks :-)
[12:26] <mardy> I previously found a place when systemd-mount was invoked, but I don't see an advantage in using that
[12:28] <zyga-mbp> mardy: it depends on what you want to achieve
[12:29] <zyga-mbp> mardy: systemd does track mounts that happen in the system
[12:29] <zyga-mbp> mardy: (if it can see them from the initial mount namespace)
[12:30] <zyga-mbp> mardy: some things have systemd unit relations and going with systemd APIs may have advantages in terms of seeing how the system reacts as a whole
[12:30] <zyga-mbp> mardy: systemd still calls mount which uses libmount below the surface, so it's still the same thing regardless 
[12:31] <zyga-mbp> mardy: using system inside snap mount namespaces is most likely wrong
[12:31] <zyga-mbp> mardy: and depending on how you do it, you may be crossing the mount namespace boundary, which is relevant when you want to know what really happens in the end
[12:31] <zyga-mbp> mardy: e.g. where will the mount actually happen
[12:35] <pstolowski> mardy: nb it may make sense to have a spread test that checks snapd in lxc with your new code as this has been always a source of unexpected surprises 
[12:36] <zyga-mbp> yes
[12:36] <zyga-mbp> mardy in lxd you cannot do arbitrary mounts
[12:36] <zyga-mbp> only a subset is allowed
[12:36] <zyga-mbp> mainly you cannot mount new filesystems
[12:36] <zyga-mbp> you may only bind mount an existing mount
[12:36] <zyga-mbp> and you may mount fuse 
[12:36] <zyga-mbp> good point pstolowski 
[12:38] <zyga-mbp> mardy remember that lxd is not used as a generic backend for all tests, only select logic runs there 
[12:38] <zyga-mbp> and it's usually found late that "it breaks in lxd"
[12:56] <pedronis> mardy: also notice that what we do when we create systemd mount units is starting them, that will also do the mount
[12:57] <mardy> pedronis: right. But here I'm also supporting the case where the mount unit is not created, and the mount is only temporary
[12:58] <mardy> changing topic, anyone has seen this error before? https://github.com/snapcore/snapd/pull/10438/checks?check_run_id=3056393341
[12:58] <mardy> I guess it's not related to my changes, as most other distros passed the test fine, but you never know
[13:15] <zyga-mbp> hmm
[13:15] <zyga-mbp> maybe version skew?
[13:15] <zyga-mbp> I don't recall if this test specifically is affected but around releases, all kinds of tests were getting into funny state
[13:15] <zyga-mbp> wasn't there a release recently?
[13:25] <mardy> zyga-mbp: yes, there was 2.51.2 a few days ago
[13:37] <zyga-mbp> mardy: although having said that, I don't see anything in the logs
[13:49] <cachio_> ijohnson[m], https://paste.ubuntu.com/p/w37jDPhTnZ/
[13:50] <cachio_> this is the journal log 
[13:50] <ijohnson[m]> cachio_: what is `snap changes` on that device ?
[13:51] <cachio_> user1@localhost:~$ snap changes
[13:51] <cachio_> ID   Status  Spawn               Ready               Summary
[13:51] <cachio_> 1    Done    today at 12:29 UTC  today at 12:29 UTC  Initialize system state
[13:51] <cachio_> 2    Done    today at 12:29 UTC  today at 12:29 UTC  Initialize device
[13:51] <cachio_> 3    Done    today at 12:31 UTC  today at 12:33 UTC  Install "core" snap from file "core-from-snapd-deb.snap"
[13:51] <ijohnson[m]> cachio_: and what is `journalctl --no-pager -u snapd`? 
[13:52] <cachio_> https://paste.ubuntu.com/p/QrYvSKfsKb/
[13:52] <ijohnson[m]> cachio_: hmm which test is this from ?
[13:54] <cachio_> ijohnson[m], google-nested:ubuntu-16.04-64:tests/nested/manual/cloud-init-never-used-not-vuln:refresh
[13:56] <ijohnson[m]> cachio_: what is `cloud-init status --long` ?
[13:56] <cachio_> user1@localhost:~$ cloud-init status --long
[13:56] <cachio_> status: disabled
[13:56] <cachio_> detail:
[13:56] <cachio_> Cloud-init disabled by /etc/cloud/cloud-init.disabled
[14:07] <ijohnson[m]> cachio_: ah I know the issue, I need to work on something else right now so I can't provide you the full details of how to fix it, but essentially this test assumes it is building with an old snapd from circa 2.44, without the fix, but now it is being booted with the fix.
[14:08] <ijohnson[m]> cachio_: in the meantime, for amd64, armhf, and arm64, can you download the core snap and snapd snaps for 2.45 and upload all of those snaps to a bucket somewhere we can download them?
[14:08] <cachio_> ijohnson[m], yes
[14:09] <ijohnson[m]> that is the only way to make this test rightfully pass, is that we need to build the nested image with an old snapd, then refresh to the one we are building 
[14:09] <ijohnson[m]> thanks cachio_ 
[14:10] <cachio_> just for amd64 we need the snaps, right?
[14:10] <cachio_> ijohnson[m], 
[14:11] <ijohnson[m]> cachio_: does this test run on devices too?
[14:11] <ijohnson[m]> oh wait it's a nested test
[14:11] <ijohnson[m]> cachio_: ok yeah just amd64 then since we don't do nested tests on other arches
[14:22] <mardy> anyone knows what is the difference between /run/systemd/system/ and /run/systemd/transient/? I could find any clear info on what a system "transient" unit it
[14:22] <mardy> *is
[14:25] <mborzecki> mardy: systemd-run creates a transient unit
[14:25] <mborzecki> i'm guessing it ends up there
[14:33] <mardy> mborzecki: thanks
[14:58] <mardy> when a PR is growing big, is it OK to split it into two (a prerequisite and the main one), even if the prerequisite does not do anything useful by itself?
[14:58] <zyga-mbp> mardy https://github.com/systemd/systemd/blob/cc03890a9d76103769213da9f13d18387e17212f/man/systemd.unit.xml#L370
[14:59] <zyga-mbp> mardy I think so, it's very typical 
[14:59] <zyga-mbp> mardy the only use case I see is here: https://github.com/systemd/systemd/blob/4e95bc56dfe19b24a84b207ef369d98faa12d160/src/basic/path-lookup.c#L301
[15:01] <mardy> zyga-mbp: thanks (for both answers) :-)
[15:04] <pstolowski> mardy: yeah we often do that. sometimes you end up with a prerequisite of a prerequisite :}
[15:17] <cachio__> ijohnson[m],  still uploading the snps
[15:18] <cachio__> it is very slow
[15:23] <ijohnson[m]> cachio__: no rush, I'm in meetings still for a while
[15:23] <pstolowski> pedronis: will you have time tomorrow around 11:30 to talk about next steps?
[15:25] <pedronis> pstolowski: yes, sorry, was in meetings
[15:25] <pstolowski> np
=== cachio__ is now known as cachio
[20:45]  * cachio afk
[20:56] <ijohnson[m]> cachio: I opened https://github.com/snapcore/snapd/pull/10527 which will address the issues on those cloud-init nested tests which were giving us issues