[06:30] <mup> PR snapd#7010 closed: interfaces/docker-support: declare controls-device-cgroup <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7010>
[07:01] <mup> PR snapd#7131 closed: overlord/devicestate: detect clashing concurrent (ongoing, just finished) remodels or changes <Remodel :train:> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7131>
[07:20] <mvo> pedronis: hey, good morning! I'm about uneasy about the missing return code checks (the exec.ExitError), I will look into this and push something unless you are already on it or thing its not needed
[07:20] <pedronis> mvo: it's needed I think but also there are open questions there
[07:20] <pedronis> so we can push a couple of things to it
[07:20] <pedronis> but we'll need jamie around before landing
[07:21] <pedronis> mvo: anyway, I'm not working on that PR atm
[07:21] <pedronis> I'm trying to address jamie comments in zygmunt's one
[07:21] <mvo> pedronis: ok, according to gettenv exit 2 means "not found in db" so I will make a very targeted change just for this
[07:22] <mvo> pedronis: cool, thanks! zyga is probably around today (right zyga?)
[07:22]  * mvo actually looks in the right place to be sure
[07:25] <mvo> pedronis: I was also wondering if we should have "osutil.FindUid(name, flags)" and flags something like "LocalOnly,Getenv" (better names) instead of two functions, wdyt?
[07:45] <pedronis> mvo: does it ever make sense not to do both?
[07:45] <pedronis> anyway as I wrote in the PR, I'm a bit confused because snap-seccomp itself uses those functions
[07:45] <pedronis> but was not changed
[07:45] <pedronis> so not sure how things work at all atm
[07:45] <pedronis> :)
[07:46] <mvo> pedronis: aha, ok. I'm not that far yet I think :) I will not touch these then
[07:46] <mvo> pedronis: I pushed two trivial changes and will now add a test for malformated entries in getent()
[07:46] <mvo> pedronis: and then continue, I'm at below 10% of the PR so far :(
[07:47] <mvo> pedronis: but I think regardless of what happens we will need the tweaks I added/will add :)
[08:13] <pedronis> mvo: I pushed something to 7254, in the end I decided that having both CurrentBase and CurrentCleanBase was just confusing
[08:16] <mvo> pedronis: thanks, I have a look
[08:37] <pedronis> Chipaca: hi, given you looked 7124, I think we shouldn't have separate FindU/Gid and FindU/GidGetent ?
[08:38] <Chipaca> pedronis: but we don't
[08:38] <Chipaca> pedronis: that pr drops FindUid (makes it private)
[08:38] <pedronis> did mvo change that just now
[08:39] <Chipaca> oh wait
[08:39] <Chipaca> pedronis: it exports them via a var
[08:39] <Chipaca> :-|
[08:39] <Chipaca> hmm
[08:39] <pedronis> that is weird btw
[08:39] <pedronis> not nice docs for you
[08:40] <pedronis> anyway I don't think we should have both
[08:40] <pedronis> unless you think we should
[08:40] <pedronis> you are time that spent most thinking in that area
[08:40] <mvo> pedronis: I don't think I did, let me double check
[08:40] <pedronis> s/time/the one/
[08:41] <Chipaca> mvo: no, no, i just missed the exporting var
[08:42] <mvo> Chipaca, pedronis I posted my initial set of comments if you guys are looking anyway you can check and tell me what makes sense and what does not
[08:42] <Chipaca> pedronis: not reaching out over the network to look up a user that is either local or non-existent has value i think
[08:45] <pedronis> Chipaca: but we were missing extrausers with the previous code anyway
[08:45] <pedronis> so essentially unless we are looking up root
[08:45] <pedronis> we have to try
[08:46] <pedronis> also we are looking at passwd, not netgroup
[08:47] <mvo> Chipaca: when you did the review, did you double checked my commits? just curious as I would love to have someone double checking those
[08:49] <Chipaca> mvo: I think I did, let me double check I didn't miss anything
[08:49] <mvo> Chipaca: ta!
[08:50] <pedronis> ah, snap-seccomp works because anyway is built with cgo
[08:50] <pedronis> blargh
[08:51] <pedronis> I don't really like how confusing the state is with the functions as thy are
[08:54] <pedronis> Chipaca: do I understand correctly that if built with cgo we get the right behavior even from user.Lookup* but if not built with cgo we don't get extrausers support?
[08:54] <pedronis> out of the box
[08:54] <pedronis> but snapd itself is built without cgo usually
[08:57] <Chipaca> pedronis: well, no-ish
[08:57] <Chipaca> pedronis: snapd is not built without cgo
[08:58] <Chipaca> pedronis: godbus and seccomp both require it afaik
[08:59] <mvo> Chipaca: but those are not imported into the snapd binary or am I misremembering?
[08:59] <jamesh> snapd definitely links with godbus
[08:59] <jamesh> that's how it does polkit checks
[09:00] <Chipaca> as jamesh says
[09:00] <mvo> aha, of course
[09:00] <mvo> silly me
[09:00] <pedronis> Chipaca: so we don't need getent stuff?
[09:01] <mvo> Chipaca:more silly questions - godbus/dbus is a native dbus, so why is it using cgo?
[09:01] <Chipaca> actually maybe it isn't, in linux
[09:01] <pedronis> anyway until I'm deconfused that PR cannot land
[09:01] <Chipaca> just noticed that the ones that import C in there are freebsd and dragonfly
[09:02] <Chipaca> pedronis: thank you for stepping back like this, I got carried away with it :)
[09:02] <jamesh> hmm.  It looks like godbus will build without os/user for static_build
[09:02] <pedronis> Chipaca: I was suprised you gave it a +1 without any comments btw
[09:03] <jamesh> so it won't necessarily force cgo
[09:03] <Chipaca> pedronis: i didn't think it was nitpicking time and i didn't spot anything major
[09:03] <pedronis> but FindGid FindGidGetent is not a sane state of things
[09:03] <jamesh> see vendor/github.com/godbus/dbus/homedir_dynamic.go and homedir_static.go
[09:03] <pedronis> also we have the issue that we have functions that behave very differently
[09:03] <pedronis> depending how they are compiled
[09:03] <pedronis> me don't like
[09:04] <Chipaca> I can confirm we don't require CGO for snapd today
[09:05] <jamesh> godbus wants to know the home directory for the DBUS_COOKIE_SHA1 auth scheme noone uses
[09:05] <Chipaca> (we've even got a test that checks for this)
[09:05] <Chipaca> also, snapctl and snap-exec both are forced cgo-less
[09:13] <pedronis> Chipaca: mvo: how painful would be to write unit tests that involve extrausers for reals?
[09:17] <Chipaca> pedronis: _unit_ tests?
[09:17] <pedronis> yes, unit tests (ideally)
[09:18] <Chipaca> pedronis: 'for reals' as in via nss?
[09:18] <Chipaca> not doable
[09:19] <Chipaca> unless we added a known user to all our extrausers
[09:19] <Chipaca> including in spread
[09:25] <Chipaca> pedronis: should be doable in spread though
[09:28] <pedronis> Chipaca: we could have tests that are skipped if libnss-extrausers is not there?
[09:28] <pedronis> and install it in some cases also in spread
[09:31] <Chipaca> pedronis: what would the unit tests test?
[09:38] <jamesh> you could unshare the mount namespace, and put a socket at /var/run/nscd/socket
[09:39] <jamesh> have that feed user info into libc
[09:50] <pedronis> Chipaca: had a chat with mvo, we have  a plan I think
[09:55] <Chipaca> pedronis: … tell me more :)
[10:01] <mvo> pedronis: I hope to have a PR up in ~30min or so, it will be only about osutil.Find{Uid,Gid} and then we can merge it back into 7124
[10:01] <mvo> Chipaca: -^
[10:01] <mvo> Chipaca: the idea is to have only a single FindUid() that detect if cgo or not
=== pedronis_ is now known as pedronis
[10:33] <mup> PR snapd#7263 opened: snap: cleanup some tests, clarify some errors <Created by pedronis> <https://github.com/snapcore/snapd/pull/7263>
[10:35] <pedronis> small follow up to what was already laded for system usernames ^
[10:47] <mvo> pedronis, Chipaca https://github.com/snapcore/snapd/compare/master...mvo5:find-uid-getent?expand=1 here is the refactor for osutil.Find{Uid,Gid} we talked about, I'm adding more tests now (spread and some more unit tests). let me know if thats not what we captured
=== pedronis_ is now known as pedronis
[11:08] <pedronis> mvo: seems in the right direction and what we discussed
[11:09] <mvo> pedronis: cool, I hope I have more tests and spread ready in a bit
[11:29] <cachio> Chipaca, hey
[11:29] <cachio> Chipaca, about the error: catalog: got unexpected HTTP status code 429
[11:30] <Chipaca> cachio: yes
[11:30] <cachio> do you know how to make the catalog refresh just while we prepare the suite right?
[11:31] <Chipaca> cachio: say again?
[11:31] <cachio> Chipaca, does  it work if we saved the catalog in the snapd state
[11:31] <mup> PR snapd#7264 opened: packaging/fedora: build on RHEL8 (2.40) <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7264>
[11:31] <cachio> and then we restore it?
[11:31] <Chipaca> cachio: it should, yes; then just touch the files
[11:31] <Chipaca> cachio: restore, and touch the files
[11:31] <Chipaca> so the timestamp is fresh
[11:31] <cachio> Chipaca, which files?
[11:32] <Chipaca> cachio: /var/cache/snapd/*
[11:33] <Chipaca> cachio: there's a "sections" file and a "names" file, as well as a commands.db
[11:33] <Chipaca> cachio: just touching sections would be enough, but * makes it more future-proof :)
[11:34] <Chipaca> cachio: OTOH, if you explicitly touch sections, then even if it's not in the saved state it might still work (i'd have to check)
[11:35] <cachio> Chipaca, ok, I'll implement that
[11:35] <cachio> Chipaca, I'll ping you in case it doesn't work :)
[11:35] <cachio> thanks!!
[11:35] <Chipaca> cachio: there is a spread test for the catalog thing though
[11:35] <Chipaca> cachio: which will fail if the catalog refresh doesn't actually work
[11:40] <cachio> Chipaca, perfect
[11:44]  * mvo has finished the spread test for findUid and will continue after lunch
[11:46] <mup> PR snapd#7265 opened: osutil: add osutil.Find{Uid,Gid} <Created by mvo5> <https://github.com/snapcore/snapd/pull/7265>
[12:06] <jdstrand> pedronis, mvo: hey, I'm here and ready to work on the 3rd snap_daemon PR. perhaps we could have a quick chat (5 minutes)?
[12:07] <jdstrand> pedronis, mvo: I could also attend your standup if the timing works better
[12:14] <mvo> jdstrand: I'm at lunch, I pushed 7265 with some small tweaks to the osutil.Find{Uid,Gid}. to chat standup or 10min before the standup (or right after) would work best
[12:15] <jdstrand> mvo: that is one of the things I wanted to discuss (whether we need the fallback at all)
[12:15] <jdstrand> mvo: I'll plan on 10 minutes before. care to invite me?
[12:25] <jdstrand> mvo: reading 7265, I like it and is inline with what I was thinking but implemented better than I was thinking :)
[12:41] <pedronis> jdstrand: I made some changes to #7254 after your comments there
[12:41] <mup> PR #7254: cmd/snap-update-ns: fix pair of bugs affecting refresh of snap with layouts <Created by zyga> <https://github.com/snapcore/snapd/pull/7254>
[12:51] <jdstrand> pedronis: ack thanks. can you invite me to the standup today?
[12:52] <mvo> jdstrand: we are in the standup channel
[12:52] <jdstrand> coming
=== Girtablulu|Away is now known as Girtablulu
[14:00] <pedronis> jdstrand: I added some more comments to the PR
[14:02] <jdstrand> pedronis: ack
[14:11] <mvo> jdstrand: I pushed some tweaks to the 7265 osutil.FindUid pr, thanks for your feedback! the spread test should also pass now
[14:20] <cachio> mvo, Chipaca coudl you please take a look to #7262
[14:20] <jdstrand> mvo: \o/
[14:20] <jdstrand> mvo: it's getting close! :)
[14:21] <jdstrand> this has been a looooooonnngggg time coming :)
[14:29] <mvo> cachio: merged - and we should totally look into using this for our default sources.list :)
[14:29] <mvo> jdstrand: yeah, thanks for all your hard work on this one, we are getting close (did it merge without conflicts?)
[14:29] <cachio> mvo, sure, I'll try that today
[14:30] <mvo> cachio: thanks! no rush, but it feels like a cool improvement
[14:30] <mvo> cachio: (what I want to say is that I'm a bit excited about this I guess :)
[14:31] <cachio> mvo, nice :)
[14:32] <cachio> I want to go step by step with this
[14:32] <cachio> if go-build test doesn't fail today
[14:32] <cachio> we can go with the bigger change
[14:32] <mvo> cachio: sounds good
[14:32] <cachio> but I would like to make sure first this archive works well
[14:33] <cachio> in the meantime I'll create a pr to see how it works
[14:33] <cachio> mvo, thanks for the support
[14:33] <pedronis> mvo: let me know when I should do a proper review pass on 7625
[14:33] <pedronis> heh, 7265
[14:34] <mvo> pedronis: yes please I think its ready
[14:34] <pedronis> ok
[14:35] <mvo> Chipaca: if you could have a quick look at 7257 that would be great, hopefully easy
[14:37] <Chipaca> mvo: LGTM
[14:40] <niemeyer> mup: Hi
[14:40] <mup> niemeyer: In-com-pre-hen-si-ble-ness.
[14:40] <niemeyer> Brilliant..
[14:41] <niemeyer> Alright folks, mup can now self-identify and even steal-back its own nick through nickserv
[14:41] <niemeyer> So this should be the end of its regular silenece
[14:41] <niemeyer> silence
[14:42]  * cachio lunch
[14:42] <mvo> niemeyer: \o/
[14:43] <mvo> niemeyer: thank you!
[14:44] <niemeyer> np, sorry for how long it took
[14:45] <niemeyer> Please do let me know if it gets stuck from now on..
[14:45] <jdstrand> mvo: the initial one did not, no, but I massaged it
[14:47] <pedronis> mvo: +1 with some comments
[14:51] <jdstrand> mvo: re tests: fix user-libnss test> hehe
[14:53] <mvo> jdstrand: that was "fun" (not!)
[14:53] <mvo> pedronis: thanks, working through them now
[14:53] <jdstrand> mvo: :)
[14:59] <pedronis> mvo: sorry, I nitpicked a bit on the spread tests comments
[15:03] <mvo> pedronis: thanks, appreciated the careful review
[15:13]  * ijohnson takes a break
[15:18] <mvo> jdstrand: sorry, one more commit to the spread test for user-libnss
[15:18] <mvo> jdstrand: with that it should really pass now
[15:18] <jdstrand> hehe, np
[15:20] <jdstrand> I know how spread testing goes :)
[15:20] <jdstrand> (merged)
[15:34] <mvo> jdstrand: meh, its still not always passing, I think there is some kind of race, I'm looking
[15:39] <mup> PR snapcraft#2665 closed: Plugin catkin: disable parallel option <Created by artivis> <Closed by artivis> <https://github.com/snapcore/snapcraft/pull/2665>
[15:39] <mup> PR snapcraft#2669 opened: Plugin catkin: forward parallel build count <Created by artivis> <https://github.com/snapcore/snapcraft/pull/2669>
[15:45]  * Chipaca sets spread on fire and walks away
[15:45] <roadmr> 🔥
[15:47] <cachio> Chipaca, question
[15:47] <cachio> we restart snapd during reset for each test
[15:48] <cachio> is that triggering a catalog refresh?
[15:48] <cachio> right?
[16:22]  * cachio afk
=== pedronis_ is now known as pedronis
[18:30] <pedronis> jdstrand: not sure we understand yet the failures of that new test, it seems something is not picking up the new configuration
[18:32] <jdstrand> pedronis: I theorized in the 7265 PR it might be the systemd entry in nsswitch.conf
[18:35] <pedronis> the failure right now is silly though
[18:36]  * jdstrand takes a closer look
[18:36] <pedronis> anyway we should try to make it work with getent itself
[18:36] <pedronis> before trying with our lib
[18:36] <jdstrand> oh heh
[18:38] <jdstrand> pedronis: well, the if ./findid uid extratest can never pass cause we didn't go build anything, no?
[18:38] <mup> PR snapd#7267 opened: many: simpler access to snap-seccomp version-info <Created by pedronis> <https://github.com/snapcore/snapd/pull/7267>
[18:38] <pedronis> we are just trying though
[18:39] <pedronis> the code right now is silly
[18:39] <pedronis> not sure what it did before the current hackish state
[18:39] <pedronis> though
[18:47] <diddledan> hacky code > nocode :-p
[18:47] <diddledan> for a proper hacky code you want some epletive-ridden comments
[18:48] <diddledan> expletive*
[18:48] <diddledan> and a few comments of the form "I have no idea why this works" and "what was this supposed to do anyway?"
[19:02] <pedronis> jdstrand: so playing in the debug shell after it fails things work
[19:03] <mup> PR snapcraft#2670 opened: Plugin colcon: forward parallel build count <Created by artivis> <https://github.com/snapcore/snapcraft/pull/2670>
[19:08] <pedronis> jdstrand: I think the failure is just silly
[19:09] <pedronis> it's because of println I think
[19:12] <pedronis> testing the fix
[19:20] <pedronis> jdstrand: pushed the fix I think
[19:25] <mup> PR snapd#7268 opened: tests: add /var/cache/snapd to the snapd state to prevent error on the store <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7268>
[19:26] <jdstrand> pedronis: I also see that the println was problematic and fixed that here and it passed
[19:29] <jdstrand> pedronis: note, restore is not cleaning up extratest. it should: userdel --extrausers extratest
[19:35] <jdstrand> pedronis: I added two comments to the PR and merged/pushed to 7214
[19:36] <jdstrand> pedronis: thanks for looking at that. println to stderr... indeed, silly bug :)
[19:48]  * cachio afk
[23:33] <mup> PR snapd#7269 opened: tests: add test for services disabled during refresh hook <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7269>
[23:43] <mup> PR snapd#7270 opened: [RFC] overlord: save disabled snap svcs on unlink snap tasks <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7270>