[07:05] <mborzecki> morning
[07:42] <mup> PR snapd#10034 closed: tests/main/uc20-create-partitions: fix tests cleanup <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10034>
[08:00] <pstolowski> morning
[08:01] <mborzecki> pstolowski: hey
[08:15] <mvo> good morning pstolowski and mborzecki
[08:22] <mborzecki> mvo: hey!
[08:22] <mborzecki> thanks for landing 10034
[08:22] <mup> PR snapd#10036 opened: interfaces/apparmor: allow reading /proc/sys/kernel/random/entropy_avail <Needs security review> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10036>
[08:32] <mvo> my pleasure
[09:54] <pedronis> mborzecki: hi, I re-reviewed #10006
[09:54] <mup> Bug #10006: python libglade program crashes on load <pygtk (Ubuntu):Fix Released by seb128> <https://launchpad.net/bugs/10006>
[09:54] <mup> PR #10006: cmd/snap-bootstrap: refactor handling of ubuntu-save, do not use secboot's implicit fallback <Run nested> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10006>
[09:59] <pedronis> pstolowski: hi, I have a question/consideration in #9959, does it make sense?
[09:59] <mup> PR #9959: o/snapstate: update validation sets assertions with auto-refresh <Needs Samuele review> <validation-sets :white_check_mark:> <Created by stolowski> <https://github.com/snapcore/snapd/pull/9959>
[09:59] <pstolowski> looking
[10:01] <pstolowski> pedronis: ah yes, makes sense, thank you
[10:07] <pedronis> mborzecki: #10022 needs a 2nd review
[10:07] <mup> Bug #10022: don't build-depend on mingw32 if possible <syslinux (Ubuntu):Fix Released by cjwatson> <https://launchpad.net/bugs/10022>
[10:07] <mup> PR #10022: boot, o/devicestate: split makeBootable20 into two parts  <Run nested> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10022>
[10:07] <mborzecki> pedronis: i'll take  a look, and thanks for a review in 10022
[10:48] <mborzecki> pedronis: i've updated 10006
[10:52] <pedronis> mborzecki: can you tell me more about what happens with the locking of keys in that test? you just removed the mocking now
[10:54] <pedronis> mborzecki: TestInitramfsMountsRecoverModeDegradedEncryptedDataUnencryptedSaveHappy, this is also happy because Save is ignored again?
[10:55] <mborzecki> pedronis: yeah, i was confused by the test code previously, testRecoveryHappymode() installs a separate mock which does the right check, but i forgot to remove the one in the test before committing and pushing
[10:56] <mborzecki> and it does indeed check that the lock code is called always
[10:56] <pedronis> ah, I see that now, thx
[10:59] <mborzecki> pedronis: as for the test cases i actually consider them happy in the sense that s-b does not fail, though the one with unencrypted data & encrypted save is rather artificial at this point
[10:59] <pedronis> mborzecki: I +1ed but it needs a thorough review from Ian, also left a couple final comments
[10:59] <mborzecki> thanks!
[11:12] <cachio> mborzecki, hi
[11:12] <mborzecki> cachio: hi, what's up?
[11:12] <cachio> I see some tests show
[11:12] <cachio> tests.invariant: it seems snap-confine has crashed
[11:12] <cachio> I reproduced that and I see the directory /tmp/snap.rootfs_YwlgKW created is empty
[11:13] <cachio> any idea in case the dir is empty it failed?
[11:13] <cachio> because we are making a test fail but there are not any logs, so no way to understand what happened
[11:14] <cachio> dmesg and journal logs are not showing any useful info
[11:16] <mborzecki> cachio: yeah, saw that too, but i don't any ideas at this time the directory is supposed to be removed by snap-confine, and it will abort if it cannot do so, hope that #10033 will help tracking it down
[11:16] <mup> Bug #10033: bittornado: new changes from Debian require merging <Ubuntu:Fix Released by cjwatson> <https://launchpad.net/bugs/10033>
[11:16] <mup> PR #10033: tests: run the reset.sh helper and check test invariants while the test is restored <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10033>
[11:17] <pstolowski> pedronis: wrt refresh control, is there a good way of knowing upfront if reboot will be required? i'm looking at boot participant but it modifies the system (requests reboot already)
[11:18] <pstolowski> (sorry, not requests the reboot but prepares for it, to be precise0`
[11:18] <pstolowski> )
[11:25] <cachio> pstolowski, hey, could you plase take a quick look to 10026?
[11:26] <pstolowski> cachio: sure
[11:26] <cachio> tx
[11:31] <pstolowski> +1
[11:34] <mvo> pedronis: I added a bunch more tests to 9981, hopefully captures what we discussed last week
[11:34] <mvo> pedronis: (not urgent)
[11:36] <pedronis> pstolowski: not in general, I think we need to be conservative for a while
[11:37] <pedronis> mvo: thx, it's in my queue
[11:38] <pstolowski> pedronis: i.e. just assume kernel/gadget triggers reboot?
[11:38] <pedronis> pstolowski: kernel always does, gadget sometimes, but yes
[11:39] <pedronis> pstolowski: we can refine this later
[11:39] <pedronis> if possible
[11:40] <pstolowski> pedronis: ok
[12:08] <mup> PR snapd#10026 closed: tests: use retry tool instead a loops <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10026>
[12:14] <mborzecki> pedronis: do you want to take a look at https://github.com/snapcore/snapd/pull/10036 before I land it?
[12:14] <mup> PR #10036: interfaces/apparmor: allow reading /proc/sys/kernel/random/entropy_avail <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10036>
[12:15] <pedronis> mborzecki: lgtm
[12:15] <mborzecki> ack
[12:18] <mup> PR snapd#10036 closed: interfaces/apparmor: allow reading /proc/sys/kernel/random/entropy_avail <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/10036>
[13:18] <mup> PR snapd#10037 opened: boot: export helper for clearing tried system state, add tests <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10037>
[13:33] <mup> PR snapd#10038 opened: tests: replace while commands with the retry tool <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10038>
[14:09]  * pstolowski bbl
[15:17] <cachio_> mvo, https://github.com/snapcore/spread/pull/82
[15:17] <mup> PR spread#82: Introducing tags for the test which allows to filter executions <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/82>
[15:18] <cachio_> this is the pr to add tags toe tests in spread
[15:18] <cachio_> perhaps I could add a way to mix tags
[15:18] <cachio_> so we run tests wich match with more than 1 tag
[15:19] <cachio_> any feedback is appreciated
[15:19] <cachio_> pedronis, your feedback is also appreciated
[15:21] <mvo> cachio_: I have a look
[15:24] <cachio_> mvo, tx
[15:36] <pstolowski> re
[15:58]  * cachio_ lunch
[16:25] <pedronis> ijohnson: I reviewed (finally) #9965, the code looks good but the flag/comments/usage seem confusing to me, let me know whether my comments make sense or not
[16:25] <mup> PR #9965: sysconfig, o/configstate/configcore: first step to image build pi-config application <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9965>
[16:25] <ijohnson> pedronis: thanks let me take a look at your review
[16:26] <ijohnson> and yes I fully expected the names of things to need some work on that pr
[16:28] <ijohnson> pedronis: I see your points about the some of the flags not making sense or not being clear enough, those all seem reasonable, I will try to address that today
[16:29] <pedronis> ijohnson: my other main point is that the new function needs to be pointed at something quite different recovery/seed  vs rootfs
[16:29] <pedronis> if I understand correctly
[16:30] <ijohnson> pedronis: yes the rootDir is indeed a recovery system for the preinstall vs a rootfs for the early / initramfs fs only config
[16:30] <pedronis> ok
[16:31] <pedronis> at least I understood things, so I think my suggestions are in the right direction
[16:32] <ijohnson> yes I agree they are
[16:54] <ijohnson> mvo: pedronis: could I trouble one of you for a sudo merge of #9924? the store is on fire right now, and the tests that failed there are all unrelated to the pr
[16:54] <mup> PR #9924: interfaces/docker-support: add autobind unix rules to docker-support <Needs Samuele review> <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9924>
[16:55] <mvo> ijohnson: sure, let me do that
[16:55] <ijohnson> thanks
[16:55] <mvo> ijohnson: and you can trouble me anytime
[16:55] <ijohnson> careful what you wish for!
[16:56] <ijohnson> mvo: while you are sudo merging, you could also merge #10022, it is mostly green except one unrelated failure
[16:56] <mup> Bug #10022: don't build-depend on mingw32 if possible <syslinux (Ubuntu):Fix Released by cjwatson> <https://launchpad.net/bugs/10022>
[16:56] <mup> PR #10022: boot, o/devicestate: split makeBootable20 into two parts  <Run nested> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10022>
[16:56] <mvo> ijohnson: consider it done
[16:56] <ijohnson> oh no, pr numbers in snapd have gotten so high that mup can't tell if we mean lp bugs or snapd prs :-O
[16:57] <mvo> ijohnson: I think we need to ask gustavo to bump it to 20000
[16:57]  * ogra got curious when we'll see the mingw version of snapd
[16:57] <ijohnson> mvo: yeah probably
[16:58] <ijohnson> ogra: we will see mingw snapd when win32 is implemented by wsl
[16:58] <ogra> haha
[16:59] <mup> PR snapd#9924 closed: interfaces/docker-support: add autobind unix rules to docker-support <Needs Samuele review> <Simple 😃> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9924>
[16:59] <mup> PR snapd#10022 closed: boot, o/devicestate: split makeBootable20 into two parts  <Run nested> <UC20> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10022>
[17:01] <pedronis> mvo: I looked again at #9981, asked some questions there
[17:01] <mup> PR #9981: gadget: policy for gadget/kernel refreshes <Created by mvo5> <https://github.com/snapcore/snapd/pull/9981>
[17:03] <pedronis> mvo: I looked at the new commits
[17:03] <pedronis> mostly
[17:04] <ijohnson> pedronis: in #10005, you are limiting yourself to snap-revision timestamps, but xnox made a point on the associated forum topic about using also the account-key which signed the model assertion, as that timestamp will (in the worst case) be the newest thing available, is there a reason we can't use any assertion that is valid to move time forward ?
[17:04] <mup> Bug #10005: network properties revert after reboot <gnome-system-tools (Ubuntu):Fix Released by seb128> <https://launchpad.net/bugs/10005>
[17:04] <mup> PR #10005: seed: ReadSystemEssentialAndBetterEarliestTime <Created by pedronis> <https://github.com/snapcore/snapd/pull/10005>
[17:04] <mvo> pedronis: thanks, will reply later or in my morning! I need to think about if we need to check for "f the kernel has assets and update: true the gadget does use at least one of the kernel assets" - the helper may be wrong I will double check, maybe silly me
[17:04] <pedronis> ijohnson: I use the model timestamp too
[17:04] <pedronis> itself
[17:04] <ijohnson> oh sorry didn't actually finish reading the function I see that now
[17:05] <pedronis> ijohnson: no we can't use any assertion, they could be signed by neither the brand nor the store
[17:06] <pedronis> and relevant account-key themselves must be older than the things signed with them
[17:07] <pedronis> mvo: I think the helper does the other direction
[17:07] <pedronis> which is just failing early
[17:08] <xnox> pedronis:  interesting. Yes, we need to only trust things that are chained to the root of trust.
[17:08] <bdx> you guys are probably aware, but the snapstore is down
[17:08] <xnox> pedronis:  can you detect when assertions are signed by the store though? i.e. if they chain to the root of trust, trust their timestamps?
[17:09] <xnox> (well i guess we can't trust the timestamp that one signed locally and pushed to the store)
[17:09] <xnox> sigh
[17:09] <bdx> our ci is showing https://paste.ubuntu.com/p/b7w2RdjjbC/
[17:10] <bdx> users and getting "504 Gateway Time-out" in their browsers
[17:11] <pedronis> https://status.snapcraft.io/
[17:12] <bdx> pedronis: thx
[17:14] <ijohnson> pedronis: so LoadAssertions() will load assertions into the DB that might not be signed by an unknown account-key ?
[17:14] <ijohnson> pedronis: ah I see the attack where a legitimate user creates an account-key which is signed by the store, then puts it into the image along with another assertion they signed themselves with that key but this account-key would have a root of trust that doesn't involve the brand account-key
[17:14] <ijohnson> do we currently validate that snap-revision assertions are signed by the store account-key and only the store account-key?
[17:14] <pedronis> ijohnson: yes we do
[17:15] <ijohnson> pedronis: ok, then the code you have is fine I think
[17:15] <ijohnson> well I should rephrase, I am clear on the intent of the code and it makes sense enough that I can continue with my review
[17:19] <pedronis> ijohnson: if you want to see how, see (snaprev *SnapRevision) checkConsistency in snap_asserts.go
[17:24] <ijohnson> pedronis: got it thanks for that, what do you think about expanding to also allow snap-declaration assertions given that we have a very similar check for snap-declaration as we do snap-revision assertions?
[17:27] <pedronis> ijohnson|lunch: please add a comment about that, we can (it's rare but can happen that the snap-declaration has been revised an is newer than the snap-revision)
[17:28] <ijohnson|lunch> ack
[17:44] <mup> PR snapd#10039 opened: daemon: switch preexisting daemon_test tests to apiBaseSuite and .req <Cleanup :broom:> <Skip spread> <Created by pedronis> <https://github.com/snapcore/snapd/pull/10039>
[18:10] <mup> PR snapcraft#3471 closed: Add flutter-stable and -beta extension variants <Created by MarcusTomlinson> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3471>
[22:39] <micah> this snap prometheus-alertmanager seems to load its config from /var/snap/prometheus-alertmanager/11/alertmanager.yml -- can I change that config?
[22:39] <micah> its not in the current/common directory, so I'm unsure
[23:27] <ogra> micah, look at "ls -lh /var/snap/prometheus-alertmanagercurrent" ....  and you'll find its just a symlink 😉
[23:28] <micah> ogra: yeah, but when it gets upgraded (presumably to 12) then my config file in 11 wont be used I guess
[23:30] <ogra> when 12 gets installed the content of 11 gets copied to 12 first ... and after the upgrade the symlink points to 12
[23:31] <micah> ah ok! I didn't know about copying to the next one
[23:31] <ogra> and in case you decide t run "snap revert <packagename>" the symlink will point back to 11 and the binary from 11 will be executed
[23:31] <ogra> that way the matching config will always run with the corect binary
[23:40] <micah> thanks ogra