[05:23] <mborzecki> morning
[07:19] <pstolowski> morning
[07:26] <mardy> pstolowski: Hi! You are also welcome to join us in libera.chat :-)
[07:26] <mborzecki> pstolowski: mardy hey
[07:27] <mborzecki> mardy: the channel has migrated already?
[07:32] <pstolowski> mardy: i'm there already since last week, it is just still kinda empty ;)
[07:36] <mborzecki> fwiw registered my nick for now
[07:51] <mardy> just about 20 people there
[08:12] <mborzecki> mardy: pstolowski: any reviews to look at?
[08:13] <mborzecki> i remember that pstolowski has his phase 2 bits, but some bits were distilled to separate PRs
[08:13] <mborzecki> s/bits/changes/
[08:16] <pstolowski> mborzecki: yeah, https://github.com/snapcore/snapd/pull/10284 is the distilled part (with some changes from Samuele)
[08:16] <mup> PR #10284: o/snapstate: introduce minimalInstallInfo interface <Refresh control> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10284>
[08:19]  * pstolowski feeling schizophrenic being on libera and freenode at the same time
[08:21] <mborzecki> don't feel inclined to switch until matrix bridge is up tbh
[09:13] <pstolowski> mborzecki: do you have anything that's close to land and just needs 2nd review?
[09:14] <mborzecki> pstolowski: this one probbaly https://github.com/snapcore/snapd/pull/10254 it's a longer one though 🙂 not that i didn't warn you
[09:14] <mup> PR #10254: overlord/devicestate: tasks for creating recovery systems at runtime <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10254>
[09:14] <pstolowski> ok :) i feel warned
[09:25] <mup> PR snapd#10292 opened: WIP: overlord/servicestate: omit service list if only snap given <Created by mardy> <https://github.com/snapcore/snapd/pull/10292>
[10:04] <pstolowski> mborzecki: +1, a few nitpicks
[10:07] <mborzecki> pstolowski: thanks, let me take a quick look at the comments and push patches
[10:11] <mup> PR snapd#10281 closed: release-tools/changelog.py: implement script to update all the changelog files <Skip spread> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/10281>
[10:24] <pstolowski> mborzecki: is there a problem landing https://github.com/snapcore/snapd/pull/10241 ?
[10:24] <mup> PR #10241: tests: set memory limit for snapd <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10241>
[10:25] <pstolowski> nb ijohnson has a quota test PR that also triggers oom
[10:27] <mborzecki> pstolowski: that's slightly different, ian's PRs set quota for services, this one sets a limit for snapd (and the child processes it spawns)
[10:27] <pstolowski> yeah, just wondering if our restore-prepare needs tweaking
[10:29] <pstolowski> quota PR is expecting oom
[10:43]  * pstolowski lunch
[11:43] <mborzecki> pstolowski: are the unit tests passing for you on master?
[11:54] <pstolowski> mborzecki: checking
[12:03] <mborzecki> pstolowski: i've hit https://paste.ubuntu.com/p/pvgRcwVXXy/ with this branch https://github.com/snapcore/snapd/pull/10207 but i guess it's already fixed in master
[12:03] <mup> PR #10207: vendor: bump github.com/canonical/go-tpm2 revision <Created by mvo5> <https://github.com/snapcore/snapd/pull/10207>
[12:04] <pstolowski> mborzecki: passes here
[12:17] <pstolowski> mardy:  i commented on the cause of the failures with your other approach
[12:30] <mardy> pstolowski: thanks!
[12:31] <mup> PR snapd#10254 closed: overlord/devicestate: tasks for creating recovery systems at runtime <Skip spread> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/10254>
[12:37] <pstolowski> yay! something landed finally \o/
[12:46] <mborzecki> ahah
[12:46] <mborzecki> mardy: idk if you've seen https://github.com/snapcore/snapd/pull/10282#discussion_r637882294 json and pointers is (not)fun ;)
[12:46] <mup> PR #10282: services: remember if acting on the entire snap <Created by mardy> <https://github.com/snapcore/snapd/pull/10282>
[12:46] <mardy> pstolowski: I'm still trying to understand how the information flows, but I noticed that runServiceCommand() takes the list of service names both in its last parameter, and in the Names field of the AppInstruction.
[12:48] <pstolowski> mardy: yeah it is possible Inst is passed around anyway just for actual instruction etc, there might be some redundancy
[12:49] <pstolowski> mardy: if you see a way to simplify any of that then feel free to do it
[12:49] <mardy> mborzecki: yes, thanks. So, the idea is that by default a false boolean is considered empty (if not a pointer), right?
[12:50] <mborzecki> mardy: yes, where empty == default initialization value afaiu
[12:50] <pstolowski> mardy: fwtw that was part of big refactoring of internal handling of services where we didn't have a central place that actually does the work. some details like this are still not ideal
[12:50] <pstolowski> now at least we have service-control task as a central place
[12:50] <mborzecki> mardy: the pointer is only useful if you want to know whether the thing was present in the input data (json in this case)
[13:32] <mborzecki> ijohnson: i've opened https://github.com/snapcore/snapd/pull/10293 with the patch update for debian sid
[13:32] <mup> PR #10293: packaging/debian-sid: update locale patch for the latest master <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10293>
[13:36] <mup> PR snapd#10293 opened: packaging/debian-sid: update locale patch for the latest master <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10293>
[13:54] <ijohnson> thanks bboozzoo taking a look now
[13:59] <mardy> pstolowski: I'm a bit confused about what I should change in getServiceInfos(), because this method is not computing the service list itself: it's getting it as a parameter
[14:16] <pstolowski> mardy: afaiu it's similar to appInfosFor, only limited to services. in particular it's checking if snap name was passed to mean all services. I think it needs to also set ExplicitlyRequested because if you look at runServiceCommand, it also creates service-control task, so needs to follow same logic
[14:17] <pstolowski> mardy: it eventually calls serviceControlTs(..) method that you modified
[14:18] <pstolowski> mardy: (i mean runServiceCommand calls it)
[14:20] <pstolowski> mardy: esentialy, serviceControlTs() can be called for 'snap stop|start|restart' (which you covered) but also for 'snapctl stop|start|restart' (i.e. from hooks) and runServiceCommand is the starting point for the latter
[14:23] <pstolowski> mardy: does it make sense?
[14:28] <mardy> pstolowski: yes, I didn't realize that the parameters it gets can also consist of just snap names. Yes, I will do that change, but I don't think it will help for the spread tests.
[14:29] <mardy> for the spread tests, we need to do the opposite (expanding the list of snap services) somewhere later
[14:35] <pstolowski> mardy: it might actually fix at least services-snapctl failures
[14:36] <pstolowski> because this tests restarts specific services via snapctl, and since ExplicitlyRequested is never set there, it probavly restarts all of them (?)
[15:15] <ijohnson> bboozzoo: hey so when using the cmdline.extra file for gadgets, comments and multi-line works the same as for cmdline.full right ?
[15:30] <pstolowski> experiencing network issues
[16:05] <pstolowski> ijohnson, mborzecki : in the light of yet another report of download getting stuck while in "download-snap" task handler, i'm wondering about enhancing our debug api with a method that dumps stracktraces of all go routines, do you guys think it makes sense? this might be better than asking someone for kill -abrt ?
[16:06] <ijohnson> pstolowski: makes sense to me, but remind me does the debug command require auth or root to use ?
[16:06] <ijohnson> if not, we might want to make sure that only root can do the stacktrace dumping, otherwise we could inadvertently leak secrets that snapd is working on from that endpoint
[16:07] <pstolowski> ijohnson: yeah we don't require root atm
[16:07] <ijohnson> pstolowski: could we just make the whole `/debug` endpoint for snapd socket root only ?
[16:07] <ijohnson> root/auth
[16:09] <pstolowski> ijohnson: we could, but i don't know about potentially breaking someone with it; otoh this is a debug api and not stable by definition
[16:10] <ijohnson> yeah I'm fine breaking someone relying on the debug endpoint being user accessible
[16:10] <ijohnson> I mean obvs let's run it by mvo/pedronis when they are back, but you have my +1 ⭕️
[16:10] <ijohnson> whoops meant 😀
[16:11] <pstolowski> indeed; thanks :)
[16:24] <mborzecki> ijohnson: yes, comments & multline in cmdline.extra is handled identically to cmdline.full
[16:24] <ijohnson> nice
[16:24] <mborzecki> iow it's done by the same bit of code
[16:24] <ijohnson> right, that's what I thought, thanks for confirming
[16:25] <mborzecki> ijohnson: as for downloads, hmm we have a profiling endpoint exposed over debug api, you can dump a trace of what's happening there
[16:31] <ijohnson> hmm maybe we should make that root now then
[16:32] <mborzecki> ijohnson: take a look at tests/main/debug-pprof, there should be an example, for trace you can run `curl -s -o snapd.trace --unix-socket /run/snapd.socket 'http://localhost/v2/debug/pprof/trace?seconds=30` to grab 30s and so on
[16:32] <mborzecki> then `go tool trace snapd.trace` will open a browser
[16:32] <ijohnson> nice, pstolowski ^
[16:32] <mborzecki> and it is root only
[16:33] <ijohnson> ah perfect it is already root only
[16:33] <mborzecki> i played a bit with adding custom traces too such that you could track which task is being executed
[16:46] <mborzecki> ijohnson: had to force push an update to https://github.com/snapcore/snapd/pull/10293
[16:46] <mup> PR #10293: packaging/debian-sid: update locale patch for the latest master <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10293>
[19:23] <mup> PR snapd#10294 opened: release-tools/changelog.py: refactor regexp + file reading/writing <Simple 😃> <Skip spread> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10294>
[19:51] <mup> PR snapcraft#3527 closed: ua manager: install ubuntu-advantage-tools as needed (CRAFT-67) <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3527>