[05:45] <mborzecki> morning
[06:04] <mardy> 'morning!
=== oSoMoN_ is now known as oSoMoN
[06:43] <mborzecki> mardy: hey
[06:47] <mvo> good morning mardy and mborzecki
[06:53] <mardy> mvo: hi :-)
[06:53] <mborzecki> mvo: hey
[07:13] <mborzecki> hmm `/home/gopath/src/github.com/snapcore/snapd/tests/lib/prepare-restore.sh: line 518: go: command not found` during prepare on 21.04
[07:14] <mborzecki> mvo: can you land https://github.com/snapcore/snapd/pull/10293 ? the failures are unrelated
[07:14] <mup> PR #10293: packaging/debian-sid: update locale patch for the latest master <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10293>
[07:17] <mvo> mborzecki: sure
[07:18] <mborzecki> mvo: thanks!
[07:19] <pstolowski> morning
[07:19] <mvo> good morning pstolowski
[07:20] <mup> PR snapd#10293 closed: packaging/debian-sid: update locale patch for the latest master <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10293>
[07:34] <mborzecki> pstolowski: hey
[07:35] <mup> PR snapd#10295 opened: interfaces/apparmor: limit the number of jobs when running with a single CPU <Needs security review> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10295>
[07:36] <mborzecki> hm should have added an [RFC] tag for that
[07:37] <mardy> mborzecki: "#define DEFAULT_JOBS_MAX -8 ... long jobs_max = -DEFAULT_JOBS_MAX;" I'm not sure I understand the logic here
[07:39] <mborzecki> mardy: haha, yeah that's fun, you need to take a look at process_jobs_arg(), auto_tune_parameters() and setup_parallel_compile(), and then there's a large'ish macro for spawning work
[07:40] <mborzecki> i think i got it right, but it's best if amurray takes a look (or maybe jjohansen if he has some time)
[07:44] <pstolowski> mborzecki: might be worth expanding the comment for -j0
[07:46] <pstolowski> it's not obvious at all that giving -j0 is different than omitting it :/
[08:00] <mardy> pstolowski: looks like your suggestion was spot on, now the spread tests are successful (well, some of them): https://github.com/snapcore/snapd/pull/10292
[08:00] <mup> PR #10292: WIP: overlord/servicestate: omit service list if only snap given <Created by mardy> <https://github.com/snapcore/snapd/pull/10292>
[08:03] <pstolowski> mardy: ok, glad it was helpful. also it's good that i still somewhat understand what's going on there ;)
[08:16] <mardy> pstolowski: and this is the parameter that appears to be redundant. The change is very small: https://github.com/snapcore/snapd/pull/10296
[08:16] <mup> PR #10296: overlord/hookstate/ctlcmd: remove unneeded parameter <Created by mardy> <https://github.com/snapcore/snapd/pull/10296>
[08:20] <mup> PR snapd#10296 opened: overlord/hookstate/ctlcmd: remove unneeded parameter <Created by mardy> <https://github.com/snapcore/snapd/pull/10296>
[08:47] <pstolowski> mardy: thanks, +1
[08:50] <mup> PR snapd#10297 opened: [RFC] cmd/snap: stacktrace debug endpoint <Created by stolowski> <https://github.com/snapcore/snapd/pull/10297>
=== ogra_ is now known as Guest30743
[08:53] <pstolowski> mborzecki: wdyt ^ ?
[09:09] <mborzecki> pstolowski: i think it looks reasonable, added some questions
[09:11] <mborzecki> pedronis: hi, regarding your note, i've opened https://github.com/snapcore/snapd/pull/10295
[09:11] <mup> PR #10295: [RFC] interfaces/apparmor: limit the number of jobs when running with a single CPU <Needs security review> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10295>
[09:11] <pstolowski> mborzecki: thanks
[09:12] <pedronis> mborzecki: shouldn't we also look at memory?
[09:18] <mborzecki> pedronis: i'd like to refactor that code a bit when introducing tweaks for the memory, have something like `conserveMemory` flag similar to `conserveCPU` and pass that to the function that picks the number of jobs
[09:20] <mborzecki> pedronis: fwiw, apparmor_parser if none of the jobs settings were passed looks at the memory too, but it seems to be a very rough estimate, and the -j0 only kicks in when there's 2MB of free mem
[09:21] <mborzecki> pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/10255 ?
[09:21] <mup> PR #10255: tests/nested/core/core20-create-recovery: verify that recovery system can be created at runtime <Run nested> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10255>
[09:21] <pstolowski> sure
[09:22] <mborzecki> thanks
[09:37] <mborzecki> pstolowski: ah, forgot, you can make the stacktrace require post too
[09:37] <pstolowski> mborzecki: hmm what for?
[09:37] <mborzecki> pstolowski: (as related to the comment you added in 10255)
[09:38] <mborzecki> pstolowski: that way it will require root/pokit access
[09:39] <pstolowski> mborzecki: ah, so get rid of get and do everything via post ?
[09:47] <pedronis> mborzecki: to me sounds like if we have <256mb we should also allow only a single process?
[09:53] <pedronis> mborzecki: why did mark 10295 needs security review?
[09:53] <mborzecki> pedronis: we need to parse meminfo for that
[09:54] <mborzecki> pedronis: but yeah, 256mb seems reasonable
[09:55] <mborzecki> and the label was added to have someone more familiar to have a pass from someone more familiar with apparmor_parser internals
[09:56] <mborzecki> dropped the label now
[10:10] <pedronis> mborzecki: you should probably ping JJ explicitly I suppose to ask for comments
[10:21] <mup> PR snapd#10298 opened: Add spread test for classic snaps content slots <Created by alfonsosanchezbeato> <https://github.com/snapcore/snapd/pull/10298>
[10:31] <mup> PR snapd#10299 opened: store: extra log message, a few minor cleanups <Simple 😃> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10299>
[10:44] <pstolowski> that elusive download getting stuck issue is annoying :(
=== ogra changed the topic of #snappy to: please move to libera.chat
=== ogra changed the topic of #snappy to: please move to #snappy on libera.chat
[11:05] <mup> Bug #1929539 opened: pressing ctrl-alt-del on an attached USB KBD allows rebooting locked down devices <Snappy:New> <https://launchpad.net/bugs/1929539>
[11:09]  * pstolowski errand
[11:36] <mup> PR snapd#10300 opened: osutil: a helper to find out the total amount of memory in the system <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10300>
=== pedronis_ is now known as pedronis
[12:44] <pedronis> pstolowski: hi, I did a pass on https://github.com/snapcore/snapd/pull/10290 lots of comments/questions there
[12:44] <mup> PR #10290: o/snapstate: add helpers for setting and querying holding time for snaps <Needs Samuele review> <Refresh control> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10290>
[12:45] <pstolowski> pedronis: thanks
[12:50] <pedronis> mborzecki: is it a known issue that we get restore timeouts on Debian 10 ?
[12:50] <pedronis> this is with the interface tests
[12:51] <mup> PR snapd#10301 opened: seed/seedtest, overlord/devicestate: move seed validation helper to seedtest <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10301>
[12:52] <mborzecki> pedronis: no, not really, it was green with the patch tweaks from yesterday, do you have a log maybe?
[12:52] <pedronis> mborzecki: here, https://github.com/snapcore/snapd/pull/9043/checks?check_run_id=2664664397
[12:52] <mup> PR #9043: daemon: replace access control flags on commands with access checkers <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/9043>
[12:55] <mborzecki> pedronis: i think i've seen it, cachio was debugging a similar issue before, happens randomly when we're pking snapd.socket and systemd wakes up the snapd daemon, for some reason there's no EOF/socket not getting closed after receiving a complete response
[12:58] <pedronis> mborzecki: fwiw it seems very consistent atm on that PR with debian 10
[12:59] <mborzecki> pedronis: we've seen it when adding fedora 34, maybe it's specific to a particular systemd version
[12:59] <mborzecki> (or go)
[13:01] <mup> PR snapd#10302 opened: overlord, overlord/devicestate: allow for reloading modeenv in devicemgr when testing <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10302>
[13:21] <mup> PR snapd#10294 closed: release-tools/changelog.py: refactor regexp + file reading/writing <Simple 😃> <Skip spread> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/10294>
[13:30] <mborzecki> pedronis: this commit was added for fedora 34  https://github.com/snapcore/snapd/pull/10249/commits/ae8b6f68961eec5a7948af73d50cb11e03c33846
[13:30] <mup> PR #10249: tests: adding support for fedora-34 <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10249>
[13:37] <pedronis> mborzecki: something changed about systemd socket activation then?
[13:37] <pedronis> mborzecki: you are saying we might need this also for Debian 10 ?
[13:40] <pedronis> mvo: I forgot to mention this: https://github.com/snapcore/snapd/pull/10298#issuecomment-847825704
[13:40] <mup> PR #10298: tests: add spread test for classic snaps content slots <Created by alfonsosanchezbeato> <https://github.com/snapcore/snapd/pull/10298>
[13:40] <mvo> pedronis: aha, I need to uplaod this? can do
[13:41] <pedronis> mvo: yes, I think you should look at the PR as the snaps are defined in it as well
[13:42] <mvo> +1
[13:44] <pedronis> mvo: it's also in my queue to look at the PR but I have things before in the queue
[13:45] <mborzecki> pedronis: it's a bit weird, but yes, i was able to reproduce this locally too
[13:45] <pedronis> mborzecki: let's make a PR then? as I said it's also annoying because the timeout is long
[13:47] <mborzecki> pedronis: sure, fwiw, fedora 34 is using systemd 248, debian 10 - 247, i have 248 locally too
[13:51] <mup> PR snapd#9043 closed: daemon: replace access control flags on commands with access checkers <Created by jhenstridge> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9043>
[13:56] <mup> PR snapd#10303 opened: tests/lib/reset: make nc exit after a while when connection is idle <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10303>
[15:18] <pedronis> mborzecki: I commented on https://github.com/snapcore/snapd/pull/10291
[15:18] <mup> PR #10291: overlord/snapstate, overlord/devicestate: exclusive change conflict check <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10291>
=== E_Eickmeyer is now known as Eickmeyer
[15:40]  * ijohnson is back
[15:47] <mup> PR snapcraft#3528 opened: [wip] apt cache: raise PackageNotFoundError for packages without candidate <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3528>
[16:54] <ijohnson> pedronis: do you have strong opinions on aligning the fields for `snap quota group1` output ?
[16:54] <ijohnson> currently we do not align the fields at all, but I think we should align it like this :
[16:54] <ijohnson> https://pastebin.ubuntu.com/p/MFHH864783/
[16:55] <ijohnson> for example we align the output for snap info and snap model
[17:00] <ijohnson> also curious how you feel about ordering of the fields
[17:01] <pedronis> ijohnson: we should align them the same way we align snap info fields
[17:01] <ijohnson> I would propose this: https://pastebin.ubuntu.com/p/2djbXWTjMn/
[17:02] <ijohnson> pedronis: ack so we should align them, what order should we put them in ?
[17:02] <ijohnson> I note that it looks a little bit weird that the first field, name, has a lot of whitespace between the "name" and the actual name
[17:03] <pedronis> ijohnson: name, memory, subgroups, snaps
[17:03] <ijohnson> ack, what about current-memory vs max-memory ? which should go first ?
[17:03] <ijohnson> also /me -> lunch
[17:04] <pedronis> ijohnson: there's also the question if max should be there
[17:04] <pedronis> ijohnson: we removed it from command line and snap quotas
[17:04] <ijohnson> Yeah I was thinking maybe removing max- prefix too
[17:04] <pedronis> ijohnson: first constraint then current
[17:04] <pedronis> like in snap quotas
[17:07] <pedronis> ijohnson: current-memory is very long, that's why we get the aligment issue
[17:08] <pedronis> ijohnson: one option is to have "constraints:" and "current:" sections
[17:08] <pedronis> as info as channels: and notes:
[17:10] <pedronis> *HAS
[17:10] <pedronis> *has
[17:12] <mup> PR snapd#10303 closed: tests/lib/reset: make nc exit after a while when connection is idle <Simple 😃> <Created by bboozzoo> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/10303>
[17:21] <ijohnson> pedronis: I like the constraints and current sections, let me quick prototype that out
[17:25] <ijohnson> pedronis: how does this look https://pastebin.ubuntu.com/p/c5DYtS4XWf/ ?
[17:26] <pedronis> ijohnson: looks reasonable to me
[17:26] <ijohnson> this is with fixed alignment https://pastebin.ubuntu.com/p/qbWFGJgbcV/
[17:26] <ijohnson> ok great
[17:27] <pedronis> ijohnson: you should look at snap info for alignment because it seems it aligns subsections differently, unless I'm confused
[18:35] <ijohnson> pedronis: will do thanks
[19:47] <mup> PR snapd#10304 opened: systemd: add CurrentMemoryUsage to get current memory usage for a unit <Simple 😃> <Skip spread> <quota> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10304>
[20:02] <mup> PR snapd#10305 opened: daemon/api_quotas.go: support updating quotas with ensure action <quota> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10305>