| 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> | 06:30 |
|---|---|---|
| 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:01 |
| 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:20 |
| 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:21 |
| mvo | pedronis: cool, thanks! zyga is probably around today (right zyga?) | 07:22 |
| * mvo actually looks in the right place to be sure | 07:22 | |
| 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:25 |
| 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:45 |
| 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:46 |
| mvo | pedronis: but I think regardless of what happens we will need the tweaks I added/will add :) | 07:47 |
| pedronis | mvo: I pushed something to 7254, in the end I decided that having both CurrentBase and CurrentCleanBase was just confusing | 08:13 |
| mvo | pedronis: thanks, I have a look | 08:16 |
| pedronis | Chipaca: hi, given you looked 7124, I think we shouldn't have separate FindU/Gid and FindU/GidGetent ? | 08:37 |
| 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:38 |
| 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:39 |
| 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:40 |
| Chipaca | mvo: no, no, i just missed the exporting var | 08:41 |
| 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:42 |
| 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:45 |
| pedronis | also we are looking at passwd, not netgroup | 08:46 |
| 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:47 |
| Chipaca | mvo: I think I did, let me double check I didn't miss anything | 08:49 |
| mvo | Chipaca: ta! | 08:49 |
| pedronis | ah, snap-seccomp works because anyway is built with cgo | 08:50 |
| pedronis | blargh | 08:50 |
| pedronis | I don't really like how confusing the state is with the functions as thy are | 08:51 |
| 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:54 |
| Chipaca | pedronis: well, no-ish | 08:57 |
| Chipaca | pedronis: snapd is not built without cgo | 08:57 |
| Chipaca | pedronis: godbus and seccomp both require it afaik | 08:58 |
| 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 | 08:59 |
| 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:00 |
| 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:01 |
| 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:02 |
| 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:03 |
| Chipaca | I can confirm we don't require CGO for snapd today | 09:04 |
| 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:05 |
| pedronis | Chipaca: mvo: how painful would be to write unit tests that involve extrausers for reals? | 09:13 |
| Chipaca | pedronis: _unit_ tests? | 09:17 |
| pedronis | yes, unit tests (ideally) | 09:17 |
| Chipaca | pedronis: 'for reals' as in via nss? | 09:18 |
| Chipaca | not doable | 09:18 |
| Chipaca | unless we added a known user to all our extrausers | 09:19 |
| Chipaca | including in spread | 09:19 |
| Chipaca | pedronis: should be doable in spread though | 09:25 |
| 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:28 |
| Chipaca | pedronis: what would the unit tests test? | 09:31 |
| jamesh | you could unshare the mount namespace, and put a socket at /var/run/nscd/socket | 09:38 |
| jamesh | have that feed user info into libc | 09:39 |
| pedronis | Chipaca: had a chat with mvo, we have a plan I think | 09:50 |
| Chipaca | pedronis: … tell me more :) | 09:55 |
| 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 | 10:01 |
| === pedronis_ is now known as pedronis | ||
| mup | PR snapd#7263 opened: snap: cleanup some tests, clarify some errors <Created by pedronis> <https://github.com/snapcore/snapd/pull/7263> | 10:33 |
| pedronis | small follow up to what was already laded for system usernames ^ | 10:35 |
| 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 | 10:47 |
| === pedronis_ is now known as pedronis | ||
| pedronis | mvo: seems in the right direction and what we discussed | 11:08 |
| mvo | pedronis: cool, I hope I have more tests and spread ready in a bit | 11:09 |
| cachio | Chipaca, hey | 11:29 |
| cachio | Chipaca, about the error: catalog: got unexpected HTTP status code 429 | 11:29 |
| Chipaca | cachio: yes | 11:30 |
| cachio | do you know how to make the catalog refresh just while we prepare the suite right? | 11:30 |
| 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:31 |
| Chipaca | cachio: /var/cache/snapd/* | 11:32 |
| 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:33 |
| 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:34 |
| 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:35 |
| cachio | Chipaca, perfect | 11:40 |
| * mvo has finished the spread test for findUid and will continue after lunch | 11:44 | |
| mup | PR snapd#7265 opened: osutil: add osutil.Find{Uid,Gid} <Created by mvo5> <https://github.com/snapcore/snapd/pull/7265> | 11:46 |
| 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:06 |
| jdstrand | pedronis, mvo: I could also attend your standup if the timing works better | 12:07 |
| 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:14 |
| 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:15 |
| jdstrand | mvo: reading 7265, I like it and is inline with what I was thinking but implemented better than I was thinking :) | 12:25 |
| 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:41 |
| jdstrand | pedronis: ack thanks. can you invite me to the standup today? | 12:51 |
| mvo | jdstrand: we are in the standup channel | 12:52 |
| jdstrand | coming | 12:52 |
| === Girtablulu|Away is now known as Girtablulu | ||
| pedronis | jdstrand: I added some more comments to the PR | 14:00 |
| jdstrand | pedronis: ack | 14:02 |
| mvo | jdstrand: I pushed some tweaks to the 7265 osutil.FindUid pr, thanks for your feedback! the spread test should also pass now | 14:11 |
| 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:20 |
| jdstrand | this has been a looooooonnngggg time coming :) | 14:21 |
| 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:29 |
| 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:30 |
| cachio | mvo, nice :) | 14:31 |
| 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:32 |
| 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:33 |
| mvo | pedronis: yes please I think its ready | 14:34 |
| pedronis | ok | 14:34 |
| mvo | Chipaca: if you could have a quick look at 7257 that would be great, hopefully easy | 14:35 |
| Chipaca | mvo: LGTM | 14:37 |
| niemeyer | mup: Hi | 14:40 |
| mup | niemeyer: In-com-pre-hen-si-ble-ness. | 14:40 |
| niemeyer | Brilliant.. | 14:40 |
| 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:41 |
| * cachio lunch | 14:42 | |
| mvo | niemeyer: \o/ | 14:42 |
| mvo | niemeyer: thank you! | 14:43 |
| niemeyer | np, sorry for how long it took | 14:44 |
| 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:45 |
| pedronis | mvo: +1 with some comments | 14:47 |
| jdstrand | mvo: re tests: fix user-libnss test> hehe | 14:51 |
| mvo | jdstrand: that was "fun" (not!) | 14:53 |
| mvo | pedronis: thanks, working through them now | 14:53 |
| jdstrand | mvo: :) | 14:53 |
| pedronis | mvo: sorry, I nitpicked a bit on the spread tests comments | 14:59 |
| mvo | pedronis: thanks, appreciated the careful review | 15:03 |
| * ijohnson takes a break | 15:13 | |
| 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:18 |
| jdstrand | I know how spread testing goes :) | 15:20 |
| jdstrand | (merged) | 15:20 |
| mvo | jdstrand: meh, its still not always passing, I think there is some kind of race, I'm looking | 15:34 |
| 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:39 |
| * Chipaca sets spread on fire and walks away | 15:45 | |
| roadmr | 🔥 | 15:45 |
| cachio | Chipaca, question | 15:47 |
| cachio | we restart snapd during reset for each test | 15:47 |
| cachio | is that triggering a catalog refresh? | 15:48 |
| cachio | right? | 15:48 |
| * cachio afk | 16:22 | |
| === pedronis_ is now known as pedronis | ||
| pedronis | jdstrand: not sure we understand yet the failures of that new test, it seems something is not picking up the new configuration | 18:30 |
| jdstrand | pedronis: I theorized in the 7265 PR it might be the systemd entry in nsswitch.conf | 18:32 |
| pedronis | the failure right now is silly though | 18:35 |
| * 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:36 |
| 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:38 |
| 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:39 |
| diddledan | hacky code > nocode :-p | 18:47 |
| diddledan | for a proper hacky code you want some epletive-ridden comments | 18:47 |
| 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?" | 18:48 |
| pedronis | jdstrand: so playing in the debug shell after it fails things work | 19:02 |
| mup | PR snapcraft#2670 opened: Plugin colcon: forward parallel build count <Created by artivis> <https://github.com/snapcore/snapcraft/pull/2670> | 19:03 |
| pedronis | jdstrand: I think the failure is just silly | 19:08 |
| pedronis | it's because of println I think | 19:09 |
| pedronis | testing the fix | 19:12 |
| pedronis | jdstrand: pushed the fix I think | 19:20 |
| 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:25 |
| jdstrand | pedronis: I also see that the println was problematic and fixed that here and it passed | 19:26 |
| jdstrand | pedronis: note, restore is not cleaning up extratest. it should: userdel --extrausers extratest | 19:29 |
| jdstrand | pedronis: I added two comments to the PR and merged/pushed to 7214 | 19:35 |
| jdstrand | pedronis: thanks for looking at that. println to stderr... indeed, silly bug :) | 19:36 |
| * cachio afk | 19:48 | |
| 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:33 |
| 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> | 23:43 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!