[07:54] <mborzecki> morning
[07:55] <mvo> good morning mborzecki
[07:55] <mborzecki> mvo: hey
[07:59] <mborzecki> mvo: can you land https://github.com/snapcore/snapd/pull/10067 ?
[07:59] <mup> PR #10067: overlord/snapstate: make sure that snapd current symlink is not removed during refresh <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10067>
[08:00] <mvo> mborzecki: sure
[08:01] <mvo> mborzecki: looking, I'm going over the list of open PRs right now, there are some more that were ready and just failed because of things like snap-advice failing
[08:02] <mup> PR # closed: snapd#10035, snapd#10070, snapd#10071, snapd#10072
[08:03] <mvo> mborzecki: thank you so much for this PR! merged
[08:03] <mvo> mborzecki: and cherry-picked
[08:06] <mborzecki> thanks!
[08:06] <pstolowski> morning
[08:06] <mvo> pstolowski: good morning!
[08:06] <mborzecki> pstolowski: hey
[08:07] <mup> PR snapd#10067 closed: overlord/snapstate: make sure that snapd current symlink is not removed during refresh <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10067>
[08:11] <mborzecki> pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/10054 ?
[08:11] <mup> PR #10054: overlord/snapstate, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10054>
[08:12] <mup> PR snapd#10064 closed: tests: simplify the reset.sh logic by removing not needed command <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10064>
[08:14] <mborzecki> mvo: pstolowski: i think we can land https://github.com/snapcore/snapd/pull/10062 too, the failures are unrelated afaict
[08:14] <mup> PR #10062: tests: validation sets spread test <validation-sets :white_check_mark:> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10062>
[08:15] <mvo> mborzecki: sure thing, thank you!
[08:17] <mup> PR snapd#10062 closed: tests: validation sets spread test <validation-sets :white_check_mark:> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10062>
[08:19] <pstolowski> mborzecki: will do
[08:19] <mborzecki> pstolowski: thanks!
[08:20] <zyga> good morning guys!
[08:20] <mvo> hey zyga
[08:20] <zyga> hey mvo, how are you doing?
[08:21] <mvo> zyga: good, thank you! how are you? it's getting sunny here again which is super nice
[08:21] <zyga> oh, I envy you the sun then :) Here's it's finally getting warmer, it looks like Saturday will be perfect for some time (alone) outside
[08:22] <zyga> but it's also bound to be raining so I guess it will be just me and the bike again
[08:22] <zyga> so much work lately that I rarely think about stuff like that
[08:22] <zyga> but it's the last stretch it seems
[08:25] <mvo> zyga: yeah, the alone part is the same here, it's a bit annoying
[08:26] <zyga> mvo: some of our parents got vaccinated already but there's a huge queue and we're not likely to get anything more this year
[08:26] <mvo> zyga: I'm somewhat hopeful for end-of-summer here but I'm also notouriously optimistic
[08:27] <zyga> we had to move our release for three weeks to pick up the documentation work and the website
[08:27] <zyga> but after that it looks like some normal time will happen
[08:28] <zyga> a week off
[08:28] <zyga> and regular feature work
[08:28] <zyga> I'm looking forward to having early EODs and warm weather for just doing stuff
[08:35] <pstolowski> hey zyga
[08:35] <zyga> hey :)
[08:36] <mvo> zyga: sounds great, total +1 on this
[08:37] <mup> PR snapd#10058 closed: cla-check: Use has-signed-canonical-cla GitHub Action <Skip spread> <Created by MarcusTomlinson> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10058>
[08:51] <pedronis> mvo: that fix that you are non finding in Ian's PR, seems to have be done already in the prereq: https://github.com/snapcore/snapd/pull/10035/commits/73e4cb21bf7c7d0f54ba1205beb9ad866c017845
[08:51] <mup> PR #10035: o/devicestate: split off ensuring next boot goes to run mode into new task <Simple 😃> <UC20> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10035>
[08:51] <pedronis> *been
[08:53] <pedronis> mborzecki: hi, related to the snapd fixes, are you also working on the snap-confine profile change?
[08:54] <mvo> pedronis: nice, thanks
[08:54] <mvo> pstolowski: looks like 10068 just needs one more small check if we can reuse the same test and then it can go in. nice job there!
[08:55] <pedronis> mvo: mborzecki: does the caching of spread test results still works as before? I have the impression all tests are re-run now, but maybe I'm just confused
[08:55] <pstolowski> mvo: looking
[08:55] <mborzecki> pedronis: noticed that too, but did not look into it yet
[08:57] <mup> PR snapd#10075 closed: interfaces/udisks2: allow locking /run/mount/utab for udisks 2.8.4 <Needs Samuele review> <Created by knitzsche> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10075>
[09:02] <zyga> ^ on that last merged PR: one of the files that udisks/libmount wanted to read, not sure if it's that one in /run, was tricky to share
[09:02] <zyga> because that interface (in the libmount sense) is not aware of mount namespaces, udisks would get very confused
[09:06] <mvo> pedronis, mborzecki if you have an example where you see this I can have a look after my meeting, it should be visible in the logs of the runs what stamp files were used
[09:06] <pstolowski> mvo: i preplied, not sure we want to change that without input from Sergio
[09:07] <pstolowski> *replied
[09:07] <mup> PR snapd#10048 closed: o/configstate: deal with no longer valid refresh.timer=managed <Bug> <Needs Samuele review> <Squash-merge> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10048>
[09:07] <mvo> pstolowski: thanks, let's make sure we can do a followup, it's not a blocker
[09:07] <pstolowski> mvo: ok
[09:10] <pstolowski> hmm but it looks the test failed on reboot, that's weird, i'm sure it passed when i run it manually
[09:11] <pstolowski> ah we need || ture, weird..
[09:16] <mvo> pstolowski: I see an error that it can't resolve "ubuntu" which is kind of expected given that we turned off resolving :) but of course annoying
[09:16] <mvo> pstolowski: I wonder if adding ubuntu to /etc/hosts is enough
[09:16] <pstolowski> mvo: yes i saw that (it's from sudo i think)
[09:16] <mvo> (but it could/should be there already)
[09:17] <mvo> pstolowski: yeah, sudo tries to be smart because it can have per-host configuration in sudoers.cfg
[09:25] <pstolowski> mvo: this is something we would do on core image I assume?
[09:26] <pstolowski> (rather than trying to modify config on writable etc)
[09:28] <mvo> pstolowski: it's a good question, I can check after my meeting
[10:02] <mup> PR snapd#10076 opened: tests: fix snap-advise-command check for 429 <Simple 😃> <Test Robustness> <Created by mvo5> <https://github.com/snapcore/snapd/pull/10076>
[10:11] <mvo> pedronis: ^- this should make your (and other) PRs much more happy
[10:14] <mvo> pstolowski: ok if I merge 10068 as squash-merge?
[10:15] <pstolowski> mvo: yes; let's wait for nested though, i pushed a fix
[10:15] <mvo> pstolowski: thanks
[10:16] <mvo> pstolowski: does that work? I mean, sudo complains but it still reboots?
[10:16] <pstolowski> mvo: it's an error from ssh dropping connection; we do that in other places
[10:16] <mvo> pstolowski: aha, makes sense
[10:17] <mvo> pstolowski: thanks!
[10:18] <pstolowski> mvo: i'm not sure why it didn't fail before when i run this test, maybe it's not always reporting an error (?)
[10:18] <mvo> pstolowski: probably a race
[10:20] <pstolowski> could be
[10:37] <mup> PR snapd#9981 closed: gadget: policy for gadget/kernel refreshes <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9981>
[10:55] <mup> PR snapcraft#3489 opened: errors: introduce details_from_called_process_error() helper <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3489>
[11:37] <mup> PR snapd#10076 closed: tests: fix snap-advise-command check for 429 <Simple 😃> <Test Robustness> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10076>
[11:42] <mup> PR snapd#10077 opened: o/configstate: fix panic with a sequence of config unset ops over same path <Bug> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10077>
[11:44] <mvo> mborzecki: iirc we need to also have some code that keeps snap-confine apparmor profiles around during snapd refreshes, did I remember this correctly?
[11:44] <mborzecki> yes
[11:45] <mvo> mborzecki: what's the current thinking on this, do we have ideas already :) ?
[11:47] <mborzecki> mvo: yes, setup-profiles glob needs ot be altered such that the old profiles stay around and only get cleaned up when given revision of the snapd snap is being removed
[11:47] <mup> PR snapd#10078 opened: tests: handle "snap refresh" returning an empty reply gracefully <Created by mvo5> <https://github.com/snapcore/snapd/pull/10078>
[11:52] <mvo> mborzecki: great, is that something you can work on? and something we could even cramp into 2.49.2?
[11:53] <mborzecki> it's in my list
[11:53] <mvo> mborzecki: \o/ thanks!
[12:08] <jdstrand> mvo: hey, I don't know it you saw, but I was able to commit to the snappy-debug tree we setup, but the uploads all failed: https://launchpad.net/~jdstrand/+snap/snappy-debug/+build/1347999 https://launchpad.net/~jdstrand/+snap/snappy-debug/+build/1348000 ("Cannot upload new revisions for name=snappy-debug"). Is this something you could look at, give to someone, tell me who to talk to? (I feel like maybe
[12:08] <jdstrand> I'm not a coillaborator?)
[12:08] <jdstrand> I have no visibility on it (https://dashboard.snapcraft.io/snaps/snappy-debug/ is 404)
[12:17] <mborzecki> mvo: pedronis: the caching does not work afaict
[12:17] <mborzecki> and there's this warning in the task that restores the cache: `Warning: tunneling socket could not be established, statusCode=403`
[12:22] <mborzecki> tasks that run on github's infra ahve caching working correctly
[12:39] <pedronis> mborzecki: it's a firewall issue?
[12:40] <mborzecki> ha https://github.com/actions/cache/issues/478
[12:41] <mborzecki> although the issue describes a close event
[12:43] <mvo> jdstrand: let me chase this
[12:46] <jdstrand> mvo: thanks! it's fixed now :)
[12:58] <mup> PR snapd#10079 opened: api: provide meaningful error message on connect/disconnect for non-installed snap <Bug> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10079>
[13:33] <ijohnson> thanks mvo for merging all those pr's of mine that had 2 +1s but were red for a long time
[13:56] <mvo> ijohnson: sure thing!
[14:18] <pstolowski> cachio: there is a question re nested suites that maybe you can clarify in https://github.com/snapcore/snapd/pull/10068
[14:18] <mup> PR #10068: o/configstate: don't pass --root=/ when masking/unmasking/enabling/disabling services <Run nested> <Squash-merge> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10068>
[14:46] <cachio> pstolowski, let me take a look
[14:53] <cachio> pstolowski, done
[15:06] <pstolowski> cachio: thanks; does it make sense then to enable 20.04 for nested/core tests or are they bound to fail?
[15:07] <cachio> pstolowski, if you run the test for system 20.04 on suite nested/core
[15:07] <mvo> pstolowski: https://github.com/snapcore/snapd/pull/10068/checks?check_run_id=2192201052 looks real
[15:07] <mup> PR #10068: o/configstate: don't pass --root=/ when masking/unmasking/enabling/disabling services <Run nested> <Squash-merge> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10068>
[15:07] <cachio> the vm wont be a real uc20
[15:07] <mvo> pstolowski: "2021-03-25T10:39:52.8546726Z - Run configure hook of "core" snap (run hook "configure": [disable rsyslog.service] failed with exit status 1: Failed to disable unit: Unit file rsyslog.service does not exist." - that's odd, your test does not touch this, does it?
[15:07] <ijohnson> cachio: what do you mean?
[15:07] <cachio> because the vm won't have secure boot
[15:08] <ijohnson> well it's just not a secure boot + tpm uc20 vm
[15:08] <mvo> mborzecki: is it okay to squash-merge https://github.com/snapcore/snapd/pull/10054 ?
[15:08] <mup> PR #10054: overlord/snapstate, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap <Squash-merge> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10054>
[15:09] <cachio> on nested/core
[15:09] <cachio> NESTED_ENABLE_TPM: '$(HOST: echo "${NESTED_ENABLE_TPM:-false}")'
[15:09] <cachio> NESTED_ENABLE_SECURE_BOOT: '$(HOST: echo "${NESTED_ENABLE_SECURE_BOOT:-false}")'
[15:09] <cachio> and in nested/core20
[15:09] <cachio> NESTED_ENABLE_TPM: '$(HOST: echo "${NESTED_ENABLE_TPM:-true}")'
[15:09] <cachio> NESTED_ENABLE_SECURE_BOOT: '$(HOST: echo "${NESTED_ENABLE_SECURE_BOOT:-true}")'
[15:09] <cachio> this is the main dif
[15:10] <pstolowski> mvo: oh
[15:10] <cachio> ijohnson, depends on the test, if it is ok to run the test with that configuration so we can run it in nested/core
[15:11] <cachio> if we need to make sure the test run with secboot and tpm so we need to run in nested/core20
[15:11] <ijohnson> cachio: I guess we should think about unifying tests/nested/core and tests/nested/core20, and possibly making the default for tests/nested/core for uc20 to be w/o tpm and secboot, but allow specific tests to opt-in to secboot and tpm in that suite
[15:12] <ijohnson> it would mean that if for example I go add a new uc specific test to tests/nested/core, it will run on an unencrypted uc20 VM, and if I wanted to also run it for uc20 with secboot and tpm, then I would create a variant of the test with env vars in the spread task.yaml
[15:13] <ijohnson> we would then have to filter out duplicated non-uc20 systems, since non-uc20 systems operate the same with and without secboot/tpm, but I think the gain of unifying those suites is that we can more easily get uc20 coverage by just adding a file to the tests/nested/core suite without needing to remember about tests/nested/core20 suite, or potentially worse just duplicating an entire task.yaml except for one line
[15:13] <cachio> ijohnson, that makes sense
[15:14] <pstolowski> mvo: does rsyslog exist on core18?
[15:15] <cachio> ijohnson, i'll try to implement that
[15:19] <mvo> pstolowski: I think so, I can double check
[15:20] <mvo> pstolowski: oh, looks I'm wrong
[15:20] <mvo> pstolowski: seems like we don't
[15:20] <pstolowski> mvo: i think we're abusing rsyslog in gadget.defaults in the tests
[15:21] <pstolowski> mvo: and this triggers the problem. fwtw i think uc20 doesn't have rsyslog either
[15:21] <mvo> pstolowski: yeah, that definitely does not has it
[15:21] <mvo> pstolowski: meh, ok
[15:21] <mvo> pstolowski: abuse sounds not great
[15:22] <pstolowski> mvo: unfortunately rsyslog was the only flag we could test, that's why (because ssh is problematic to test :/)
[15:22] <mvo> pstolowski: I see
[15:22] <mvo> pstolowski: could we use resolved now?
[15:22] <pstolowski> mvo: yes i think so
[15:22] <pstolowski> mvo: i'm on it
[15:23] <mvo> \o/
[15:27]  * cachio lunch
[15:48] <mup> PR snapd#10080 opened: interfaces/u2f-devices: add HyperFIDO Pro (https://launchpad.net/bugs/1919268) <Created by oSoMoN> <https://github.com/snapcore/snapd/pull/10080>
[15:50] <pstolowski> mvo: ok the test passed with systemd-resolved; updated and pushed
[15:55] <pstolowski> mvo: so if anyone uses similar gadget defaults without reflection, this means problems after this PR
[15:57] <mvo> pstolowski: I guess we could do a git grep for this pattern maybe?
[15:57] <mvo> mborzecki: can I help in any way with the snap-confine profile thing? one more meeting then I have a bit of time I hope :)
[15:57] <pstolowski> mvo: i mostly worried about our users
[15:57] <zyga> snap confine profile magic? :-)
[15:58] <zyga> what are you changing?
[15:58] <mvo> pstolowski: oh, I see
[15:58] <pstolowski> *i'm
[15:58] <ijohnson> pstolowski: what do you mean reflection for gadget defaults ?
[15:58] <mborzecki> mvo: let me know how bad it looks https://github.com/snapcore/snapd/compare/master...bboozzoo:bboozzoo/snapd-security-profiles-stay-around?expand=1
[15:58] <mborzecki> some tests are missing still
[15:59] <pstolowski> ijohnson: using service.rsyslog.disable=true in gadget defaults for early config on systems where it doesn't make sense because the unit doesn't exist
[15:59] <ijohnson> oh so before using that would not break anything and now using that would break something ?
[15:59] <mvo> mborzecki: does not look bad at all actually
[15:59] <pstolowski> ijohnson: which doesn't cause problems in early config, but strikes later
[15:59] <ijohnson> ah interesting
[16:00] <mvo> pstolowski: right so if someone cargo culted a rsyslog setting that would now cause problems
[16:00] <pstolowski> ijohnson: yeah beacause we always go over all services on snap set
[16:00] <ijohnson> I think it's unlikely it would happen for our users, but gadgets have a history of being cargo culted and doing very weird things
[16:01] <pstolowski> mvo: maybe it's 2.50 material then? i don't know
[16:18] <pedronis> pstolowski: I think we discussed the idea of checking if a service exist at all at some point or use system exit codes (but doesn't changed between releases)
[16:18] <pedronis> s/doesn't/those/
[16:20] <pstolowski> pedronis: yes
[16:20] <pedronis> mvo: #9999 needs a master merge and is also unblocked?
[16:20] <mup> PR #9999: snapstate: add "kernel-assets" to featureSet <⛔ Blocked> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9999>
[16:20] <pedronis> pstolowski: I still think is the right path forward given that services are not the same across versions
[16:30] <pstolowski> pedronis: i could look at this as a prerequsite for existing PR
[16:35] <cachio> pedronis, is this the error you saw? https://paste.ubuntu.com/p/4vW7vqH6k4/
[16:36] <cachio> google:ubuntu-20.04-64 ...# ls /var/lib/snapd
[16:36] <cachio> state.json
[16:36] <cachio> this is what I see
[16:36] <pedronis> cachio: yes, something like that
[16:37] <pedronis> pstolowski: yes, we need to decide what to do exactly though, because the last time I'm not sure we reached a conclusion except that the error codes weren't useful
[16:38] <pstolowski> pedronis: yes, error codes were useless
[16:38] <pstolowski> pedronis: we would probably need a separate call to systemctl status
[16:45] <pedronis> yes
[16:45] <pedronis> I fear that's what we would need
[16:45] <pedronis> pstolowski: I think #10077 is reasonable but I have a probably annoying question
[16:45] <mup> Bug #10077: libforms1: new changes from Debian require merging <Ubuntu:Fix Released by cjwatson> <https://launchpad.net/bugs/10077>
[16:45] <mup> PR #10077: o/configstate: fix panic with a sequence of config unset ops over same path <Bug> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10077>
[16:48] <pstolowski> pedronis: thanks, i need to stare a bit a this
[16:49] <pstolowski> *at
[16:49] <pstolowski> what are some really big snaps in the store? i need to test something
[16:51] <ijohnson> pstolowski: try the 0ad snap it's pretty big our
[16:51] <pedronis> pstolowski: some of the jetbrains snap are big
[16:51] <ijohnson> *iirc
[16:51] <pstolowski> ack, thanks
[16:53] <pstolowski> ok, so snap abort correctly cancels the context while inside download task
[16:57] <mborzecki> mvo: wrapipng it up for today, pushed some tweaks https://github.com/snapcore/snapd/compare/master...bboozzoo:bboozzoo/snapd-security-profiles-stay-around happy to discuss tomorrow if the branch makes sense
[17:00] <mvo> mborzecki: yeah, I will chat with pedronis if/how we could land this givne that he is off tomorrow
[17:06] <zyga> I had a look at the change and conceptually it looks good, I'm a bit puzzled though, was that profile removed from memory or is it only a problem if the file is gone from disk?
[17:06] <zyga> I'm asking because of a bug we keep propagating:
[17:06] <zyga> https://github.com/snapcore/snapd/compare/master...bboozzoo:bboozzoo/snapd-security-profiles-stay-around#diff-48ad8c3c5d0a39f5ce40d31863a4ae42289a624cfcded61f571ce66fd984a729R583
[17:06] <zyga> here
[17:06] <mvo> mborzecki: got a verbal okay from samuele to land without him (the current diff looks fine)
[17:06] <zyga> this is never doing anything, right?
[17:07] <zyga> because apparmor unload program relies on the ability to parse the profile and find the profile names again
[17:07] <zyga> so we remove things from disk and apparmor just NOPs
[17:07] <zyga> cc mborzecki pedronis ^
[17:07] <pedronis> mborzecki: there's a typo: SecuityBackendDiscardingLate Security
[17:08] <mvo> zyga: only gone from disk, we have a race when snapd is refreshing and there is a reboot in the middle
[17:08] <zyga> ah
[17:08] <zyga> then that is good IMO
[17:09] <zyga> I'd drop a comment so that what I said about unload profiles doesn't get lost
[17:09] <zyga> yeah, raciness on poweroff is very tricky
[18:38] <mup> PR snapd#10081 opened: [RFC] interfaces, interfaces/apparmor, overlord/snapstate: late removal of snap-confine apparmor profiles <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10081>
[19:08]  * cachio afk
[21:41] <mup> PR snapcraft#3490 opened: build(deps): bump pyyaml from 5.3 to 5.4 <dependencies> <python> <Created by dependabot[bot]> <https://github.com/snapcore/snapcraft/pull/3490>