=== jamesh_ is now known as jamesh === not_phunyguy is now known as phunyguy === not_phunyguy is now known as phunyguy === jamesh_ is now known as jamesh [07:54] morning [07:55] good morning mborzecki [07:55] mvo: hey [07:59] mvo: can you land https://github.com/snapcore/snapd/pull/10067 ? [07:59] PR #10067: overlord/snapstate: make sure that snapd current symlink is not removed during refresh [08:00] mborzecki: sure [08:01] 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] PR # closed: snapd#10035, snapd#10070, snapd#10071, snapd#10072 [08:03] mborzecki: thank you so much for this PR! merged [08:03] mborzecki: and cherry-picked [08:06] thanks! [08:06] morning [08:06] pstolowski: good morning! [08:06] pstolowski: hey [08:07] PR snapd#10067 closed: overlord/snapstate: make sure that snapd current symlink is not removed during refresh [08:11] pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/10054 ? [08:11] PR #10054: overlord/snapstate, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap [08:12] PR snapd#10064 closed: tests: simplify the reset.sh logic by removing not needed command [08:14] mvo: pstolowski: i think we can land https://github.com/snapcore/snapd/pull/10062 too, the failures are unrelated afaict [08:14] PR #10062: tests: validation sets spread test [08:15] mborzecki: sure thing, thank you! [08:17] PR snapd#10062 closed: tests: validation sets spread test [08:19] mborzecki: will do [08:19] pstolowski: thanks! [08:20] good morning guys! [08:20] hey zyga [08:20] hey mvo, how are you doing? [08:21] zyga: good, thank you! how are you? it's getting sunny here again which is super nice [08:21] 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] but it's also bound to be raining so I guess it will be just me and the bike again [08:22] so much work lately that I rarely think about stuff like that [08:22] but it's the last stretch it seems [08:25] zyga: yeah, the alone part is the same here, it's a bit annoying [08:26] 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] zyga: I'm somewhat hopeful for end-of-summer here but I'm also notouriously optimistic [08:27] we had to move our release for three weeks to pick up the documentation work and the website [08:27] but after that it looks like some normal time will happen [08:28] a week off [08:28] and regular feature work [08:28] I'm looking forward to having early EODs and warm weather for just doing stuff [08:35] hey zyga [08:35] hey :) [08:36] zyga: sounds great, total +1 on this [08:37] PR snapd#10058 closed: cla-check: Use has-signed-canonical-cla GitHub Action [08:51] 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] PR #10035: o/devicestate: split off ensuring next boot goes to run mode into new task [08:51] *been [08:53] mborzecki: hi, related to the snapd fixes, are you also working on the snap-confine profile change? [08:54] pedronis: nice, thanks [08:54] 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] 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] mvo: looking [08:55] pedronis: noticed that too, but did not look into it yet [08:57] PR snapd#10075 closed: interfaces/udisks2: allow locking /run/mount/utab for udisks 2.8.4 [09:02] ^ 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] because that interface (in the libmount sense) is not aware of mount namespaces, udisks would get very confused [09:06] 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] mvo: i preplied, not sure we want to change that without input from Sergio [09:07] *replied [09:07] PR snapd#10048 closed: o/configstate: deal with no longer valid refresh.timer=managed

[09:07] pstolowski: thanks, let's make sure we can do a followup, it's not a blocker [09:07] mvo: ok [09:10] hmm but it looks the test failed on reboot, that's weird, i'm sure it passed when i run it manually [09:11] ah we need || ture, weird.. [09:16] 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] pstolowski: I wonder if adding ubuntu to /etc/hosts is enough [09:16] mvo: yes i saw that (it's from sudo i think) [09:16] (but it could/should be there already) [09:17] pstolowski: yeah, sudo tries to be smart because it can have per-host configuration in sudoers.cfg [09:25] mvo: this is something we would do on core image I assume? [09:26] (rather than trying to modify config on writable etc) [09:28] pstolowski: it's a good question, I can check after my meeting [10:02] PR snapd#10076 opened: tests: fix snap-advise-command check for 429 [10:11] pedronis: ^- this should make your (and other) PRs much more happy [10:14] pstolowski: ok if I merge 10068 as squash-merge? [10:15] mvo: yes; let's wait for nested though, i pushed a fix [10:15] pstolowski: thanks [10:16] pstolowski: does that work? I mean, sudo complains but it still reboots? [10:16] mvo: it's an error from ssh dropping connection; we do that in other places [10:16] pstolowski: aha, makes sense [10:17] pstolowski: thanks! [10:18] 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] pstolowski: probably a race [10:20] could be [10:37] PR snapd#9981 closed: gadget: policy for gadget/kernel refreshes [10:55] PR snapcraft#3489 opened: errors: introduce details_from_called_process_error() helper [11:37] PR snapd#10076 closed: tests: fix snap-advise-command check for 429 [11:42] PR snapd#10077 opened: o/configstate: fix panic with a sequence of config unset ops over same path [11:44] 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] yes [11:45] mborzecki: what's the current thinking on this, do we have ideas already :) ? [11:47] 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] PR snapd#10078 opened: tests: handle "snap refresh" returning an empty reply gracefully [11:52] mborzecki: great, is that something you can work on? and something we could even cramp into 2.49.2? [11:53] it's in my list [11:53] mborzecki: \o/ thanks! [12:08] 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] I'm not a coillaborator?) [12:08] I have no visibility on it (https://dashboard.snapcraft.io/snaps/snappy-debug/ is 404) [12:17] mvo: pedronis: the caching does not work afaict [12:17] and there's this warning in the task that restores the cache: `Warning: tunneling socket could not be established, statusCode=403` [12:22] tasks that run on github's infra ahve caching working correctly [12:39] mborzecki: it's a firewall issue? [12:40] ha https://github.com/actions/cache/issues/478 [12:41] although the issue describes a close event [12:43] jdstrand: let me chase this [12:46] mvo: thanks! it's fixed now :) [12:58] PR snapd#10079 opened: api: provide meaningful error message on connect/disconnect for non-installed snap [13:33] thanks mvo for merging all those pr's of mine that had 2 +1s but were red for a long time [13:56] ijohnson: sure thing! [14:18] cachio: there is a question re nested suites that maybe you can clarify in https://github.com/snapcore/snapd/pull/10068 [14:18] PR #10068: o/configstate: don't pass --root=/ when masking/unmasking/enabling/disabling services

[14:46] pstolowski, let me take a look [14:53] pstolowski, done [15:06] cachio: thanks; does it make sense then to enable 20.04 for nested/core tests or are they bound to fail? [15:07] pstolowski, if you run the test for system 20.04 on suite nested/core [15:07] pstolowski: https://github.com/snapcore/snapd/pull/10068/checks?check_run_id=2192201052 looks real [15:07] PR #10068: o/configstate: don't pass --root=/ when masking/unmasking/enabling/disabling services

[15:07] the vm wont be a real uc20 [15:07] 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] cachio: what do you mean? [15:07] because the vm won't have secure boot [15:08] well it's just not a secure boot + tpm uc20 vm [15:08] mborzecki: is it okay to squash-merge https://github.com/snapcore/snapd/pull/10054 ? [15:08] PR #10054: overlord/snapstate, wrappers: add dependency on usr-lib-snapd.mount for services on core with snapd snap [15:09] on nested/core [15:09] NESTED_ENABLE_TPM: '$(HOST: echo "${NESTED_ENABLE_TPM:-false}")' [15:09] NESTED_ENABLE_SECURE_BOOT: '$(HOST: echo "${NESTED_ENABLE_SECURE_BOOT:-false}")' [15:09] and in nested/core20 [15:09] NESTED_ENABLE_TPM: '$(HOST: echo "${NESTED_ENABLE_TPM:-true}")' [15:09] NESTED_ENABLE_SECURE_BOOT: '$(HOST: echo "${NESTED_ENABLE_SECURE_BOOT:-true}")' [15:09] this is the main dif [15:10] mvo: oh [15:10] 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] if we need to make sure the test run with secboot and tpm so we need to run in nested/core20 [15:11] 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] 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] 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] ijohnson, that makes sense [15:14] mvo: does rsyslog exist on core18? [15:15] ijohnson, i'll try to implement that [15:19] pstolowski: I think so, I can double check [15:20] pstolowski: oh, looks I'm wrong [15:20] pstolowski: seems like we don't [15:20] mvo: i think we're abusing rsyslog in gadget.defaults in the tests [15:21] mvo: and this triggers the problem. fwtw i think uc20 doesn't have rsyslog either [15:21] pstolowski: yeah, that definitely does not has it [15:21] pstolowski: meh, ok [15:21] pstolowski: abuse sounds not great [15:22] mvo: unfortunately rsyslog was the only flag we could test, that's why (because ssh is problematic to test :/) [15:22] pstolowski: I see [15:22] pstolowski: could we use resolved now? [15:22] mvo: yes i think so [15:22] mvo: i'm on it [15:23] \o/ [15:27] * cachio lunch [15:48] PR snapd#10080 opened: interfaces/u2f-devices: add HyperFIDO Pro (https://launchpad.net/bugs/1919268) [15:50] mvo: ok the test passed with systemd-resolved; updated and pushed [15:55] mvo: so if anyone uses similar gadget defaults without reflection, this means problems after this PR [15:57] pstolowski: I guess we could do a git grep for this pattern maybe? [15:57] 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] mvo: i mostly worried about our users [15:57] snap confine profile magic? :-) [15:58] what are you changing? [15:58] pstolowski: oh, I see [15:58] *i'm [15:58] pstolowski: what do you mean reflection for gadget defaults ? [15:58] 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] some tests are missing still [15:59] 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] oh so before using that would not break anything and now using that would break something ? [15:59] mborzecki: does not look bad at all actually [15:59] ijohnson: which doesn't cause problems in early config, but strikes later [15:59] ah interesting [16:00] pstolowski: right so if someone cargo culted a rsyslog setting that would now cause problems [16:00] ijohnson: yeah beacause we always go over all services on snap set [16:00] 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] mvo: maybe it's 2.50 material then? i don't know [16:18] 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] s/doesn't/those/ [16:20] pedronis: yes [16:20] mvo: #9999 needs a master merge and is also unblocked? [16:20] PR #9999: snapstate: add "kernel-assets" to featureSet <⛔ Blocked> [16:20] pstolowski: I still think is the right path forward given that services are not the same across versions [16:30] pedronis: i could look at this as a prerequsite for existing PR [16:35] pedronis, is this the error you saw? https://paste.ubuntu.com/p/4vW7vqH6k4/ [16:36] google:ubuntu-20.04-64 ...# ls /var/lib/snapd [16:36] state.json [16:36] this is what I see [16:36] cachio: yes, something like that [16:37] 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] pedronis: yes, error codes were useless [16:38] pedronis: we would probably need a separate call to systemctl status [16:45] yes [16:45] I fear that's what we would need [16:45] pstolowski: I think #10077 is reasonable but I have a probably annoying question [16:45] Bug #10077: libforms1: new changes from Debian require merging [16:45] PR #10077: o/configstate: fix panic with a sequence of config unset ops over same path [16:48] pedronis: thanks, i need to stare a bit a this [16:49] *at [16:49] what are some really big snaps in the store? i need to test something [16:51] pstolowski: try the 0ad snap it's pretty big our [16:51] pstolowski: some of the jetbrains snap are big [16:51] *iirc [16:51] ack, thanks === ijohnson is now known as ijohnson|lunch [16:53] ok, so snap abort correctly cancels the context while inside download task [16:57] 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] mborzecki: yeah, I will chat with pedronis if/how we could land this givne that he is off tomorrow [17:06] 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] I'm asking because of a bug we keep propagating: [17:06] https://github.com/snapcore/snapd/compare/master...bboozzoo:bboozzoo/snapd-security-profiles-stay-around#diff-48ad8c3c5d0a39f5ce40d31863a4ae42289a624cfcded61f571ce66fd984a729R583 [17:06] here [17:06] mborzecki: got a verbal okay from samuele to land without him (the current diff looks fine) [17:06] this is never doing anything, right? [17:07] because apparmor unload program relies on the ability to parse the profile and find the profile names again [17:07] so we remove things from disk and apparmor just NOPs [17:07] cc mborzecki pedronis ^ [17:07] mborzecki: there's a typo: SecuityBackendDiscardingLate Security [17:08] zyga: only gone from disk, we have a race when snapd is refreshing and there is a reboot in the middle [17:08] ah [17:08] then that is good IMO [17:09] I'd drop a comment so that what I said about unload profiles doesn't get lost [17:09] yeah, raciness on poweroff is very tricky === ijohnson|lunch is now known as ijohnson === TooLmaN_ is now known as TooLmaN === pfsmorigo is now known as Guest8748 === Ringtailed_Fox is now known as RingtailedFox === ogra_ is now known as Guest8511 === urluck_ is now known as urluck === ba is now known as bandali [18:38] PR snapd#10081 opened: [RFC] interfaces, interfaces/apparmor, overlord/snapstate: late removal of snap-confine apparmor profiles [19:08] * cachio afk [21:41] PR snapcraft#3490 opened: build(deps): bump pyyaml from 5.3 to 5.4 === Eighth_Doctor is now known as Conan_Kudo === Conan_Kudo is now known as Eighth_Doctor === Eighth_Doctor is now known as Conan_Kudo === Conan_Kudo is now known as Eighth_Doctor