| mborzecki | morning | 05:23 |
|---|---|---|
| pstolowski | morning | 07:19 |
| mardy | pstolowski: Hi! You are also welcome to join us in libera.chat :-) | 07:26 |
| mborzecki | pstolowski: mardy hey | 07:26 |
| mborzecki | mardy: the channel has migrated already? | 07:27 |
| pstolowski | mardy: i'm there already since last week, it is just still kinda empty ;) | 07:32 |
| mborzecki | fwiw registered my nick for now | 07:36 |
| mardy | just about 20 people there | 07:51 |
| mborzecki | mardy: pstolowski: any reviews to look at? | 08:12 |
| 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:13 |
| 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:16 |
| * pstolowski feeling schizophrenic being on libera and freenode at the same time | 08:19 | |
| mborzecki | don't feel inclined to switch until matrix bridge is up tbh | 08:21 |
| pstolowski | mborzecki: do you have anything that's close to land and just needs 2nd review? | 09:13 |
| 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:14 |
| 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> | 09:25 |
| pstolowski | mborzecki: +1, a few nitpicks | 10:04 |
| mborzecki | pstolowski: thanks, let me take a quick look at the comments and push patches | 10:07 |
| 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:11 |
| 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:24 |
| pstolowski | nb ijohnson has a quota test PR that also triggers oom | 10:25 |
| 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:27 |
| pstolowski | quota PR is expecting oom | 10:29 |
| * pstolowski lunch | 10:43 | |
| mborzecki | pstolowski: are the unit tests passing for you on master? | 11:43 |
| pstolowski | mborzecki: checking | 11:54 |
| 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:03 |
| pstolowski | mborzecki: passes here | 12:04 |
| pstolowski | mardy: i commented on the cause of the failures with your other approach | 12:17 |
| mardy | pstolowski: thanks! | 12:30 |
| 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:31 |
| pstolowski | yay! something landed finally \o/ | 12:37 |
| 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:46 |
| pstolowski | mardy: yeah it is possible Inst is passed around anyway just for actual instruction etc, there might be some redundancy | 12:48 |
| 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:49 |
| 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) | 12:50 |
| 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:32 |
| 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:36 |
| ijohnson | thanks bboozzoo taking a look now | 13:54 |
| 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 | 13:59 |
| 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:16 |
| pstolowski | mardy: it eventually calls serviceControlTs(..) method that you modified | 14:17 |
| pstolowski | mardy: (i mean runServiceCommand calls it) | 14:18 |
| 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:20 |
| pstolowski | mardy: does it make sense? | 14:23 |
| 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:28 |
| mardy | for the spread tests, we need to do the opposite (expanding the list of snap services) somewhere later | 14:29 |
| pstolowski | mardy: it might actually fix at least services-snapctl failures | 14:35 |
| pstolowski | because this tests restarts specific services via snapctl, and since ExplicitlyRequested is never set there, it probavly restarts all of them (?) | 14:36 |
| 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:15 |
| pstolowski | experiencing network issues | 15:30 |
| 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:05 |
| 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:06 |
| 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:07 |
| 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:09 |
| 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:10 |
| pstolowski | indeed; thanks :) | 16:11 |
| 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:24 |
| 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:25 |
| ijohnson | hmm maybe we should make that root now then | 16:31 |
| 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:32 |
| 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:33 |
| 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> | 16:46 |
| 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:23 |
| 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> | 19:51 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!