| mup | PR snapcraft#3286 opened: [legacy backport] v1 plugins: lock godep's dependencies <maintenance> <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3286> | 00:25 |
|---|---|---|
| mup | PR snapcraft#3286 closed: [legacy backport] v1 plugins: lock godep's dependencies <maintenance> <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3286> | 01:26 |
| mup | PR snapd#9348 opened: tests: print all the serial logs for the nested test <Run nested> <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9348> | 04:08 |
| mborzecki | morning | 06:09 |
| mborzecki | mvo: hey, saw your comment under #9347, i can start looking into this | 06:11 |
| mup | PR #9347: tests/lib/nested.sh: use more focused cloud-init config for uc20 <Run nested> <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9347> | 06:11 |
| mvo | mborzecki: thanks | 06:11 |
| mvo | mborzecki: might be as simple as adding something to nested_start_core_vm_unit that already waits for ssh | 06:11 |
| mborzecki | mhm | 06:12 |
| mborzecki | anyway, looks like we're becoming experts in cloud init too | 06:12 |
| mvo | mborzecki: yeah :( | 06:14 |
| mvo | mborzecki: fun, looks like your better logging PR passed except for the smoke test where the change to add logging broke the test because now snap-confine prints stuff to stderr | 06:14 |
| mborzecki | hahah | 06:14 |
| mborzecki | mvo: oh, and idk if you seen the comment about the nested suite sending 150MB of project data to the vm | 06:16 |
| mborzecki | maybe that's the kernel/core/snapd snaps repacked | 06:17 |
| mvo | mborzecki: yeah, I think | 06:18 |
| mvo | mborzecki: looks like we need to either put them elsewhere or clean them after the repacking | 06:19 |
| mvo | mborzecki: but yeah, looks like there is some junk around | 06:19 |
| mvo | mborzecki: given that all the other tests have passed and that is super rare - what functional change could have triggered this? or do you think it's pure coincidence? | 06:21 |
| mvo | mborzecki: i.e. could the cloud-init chnage be responsible? | 06:21 |
| mborzecki | feels like a stroke of luck | 06:21 |
| mvo | mborzecki: ok | 06:22 |
| mborzecki | mvo: as for SNAP_DEBUG=1 in environment, we could drop an override for snapd.service, i had that at some point, but then dropped it and added to /etc/environemnt (because no logs were appearing which i didn't know at the time to be caused by the MaxLevel* in journald) | 06:34 |
| mvo | mborzecki: yeah, let me quickly push an idea | 06:36 |
| mborzecki | ok, runing the nested suite now | 06:36 |
| mvo | mborzecki: 9349 use your idea but puts it only for the snapd unit | 06:37 |
| mborzecki | mvo: this is what i have for the snap command; https://paste.ubuntu.com/p/s4zr5NsKX4/ | 06:37 |
| mvo | mborzecki: that looks reasonable, a bit less central than I had hoped, we can't put it into nested_start_for_core_vm ? this waits for ssh already? | 06:38 |
| mup | PR snapd#9349 opened: tests: change snapd.spread-tests-run-mode-tweaks.sh to add logging <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9349> | 06:39 |
| mborzecki | mvo: hm, need to double check it wouldn't conflict the nested/manual tests | 06:39 |
| mvo | mborzecki: yeah, probably fine | 06:43 |
| mvo | mborzecki: mostly wondering | 06:43 |
| mborzecki | mvo: just checked and it should be fine, i've updated the patch | 06:44 |
| mvo | mborzecki: it's also true that muddeling the concept of start and wait-for-ready is a bit strange so keeping seprarate may well be better | 06:44 |
| mvo | mborzecki: cool | 06:44 |
| mvo | mborzecki: again, mostly thinking that this is okay since we wait for ssh already | 06:44 |
| mborzecki | maybe i should just push all of it into #9343 | 06:45 |
| mup | PR #9343: tests: more logging for UC20 kernel test <Run nested> <Test Robustness> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9343> | 06:45 |
| mborzecki | i mean ian's path + the tweak for waiting | 06:45 |
| mborzecki | s/path/patch/ | 06:46 |
| mvo | mborzecki: yes | 06:48 |
| mvo | mborzecki: I think that's good | 06:48 |
| pedronis | mvo: mborzecki: should we sync? | 06:52 |
| mborzecki | pedronis: at 9? i'll grab some tea | 06:53 |
| pedronis | yes | 06:53 |
| pedronis | mvo: I'm quite confused by #9349 given Ian's merge from yesterday | 06:54 |
| mup | PR #9349: tests: change snapd.spread-tests-run-mode-tweaks.sh to add logging <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9349> | 06:54 |
| mborzecki | NOMATCH is something that was added to spread recently? | 06:58 |
| mvo | pedronis: maybe I'm confused, let me double check what happend :( | 06:58 |
| mvo | pedronis: oh, I see :( | 06:59 |
| pedronis | mvo: afaik on master we don't use the function you are changing anymore | 06:59 |
| pedronis | mvo: I asked you to comment on that yesterday evening when Ian asked to merge and you said yes :) | 07:00 |
| mvo | pedronis: it is fine | 07:00 |
| pedronis | I'm going to the SU | 07:00 |
| mvo | pedronis: it's just that it's all a bit of a maze and apparently I did not had enough tea yet | 07:00 |
| mvo | pedronis: joining | 07:00 |
| pstolowski | morning | 07:01 |
| mup | PR snapd#9349 closed: tests: change snapd.spread-tests-run-mode-tweaks.sh to add logging <Run nested> <UC20> <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/9349> | 07:04 |
| zyga-kaveri | hello | 07:07 |
| mvo | good morning pstolowski and zyga-kaveri | 07:07 |
| zyga-kaveri | how is core20? | 07:07 |
| zyga-kaveri | I'm baby-sitting still | 07:07 |
| zyga-kaveri | pstolowski: hi | 07:12 |
| zyga-kaveri | pstolowski: could you please look at https://pastebin.ubuntu.com/p/QMJ3x8HK4h/ | 07:12 |
| zyga-kaveri | is anything there out of place for preseeding? | 07:13 |
| zyga-kaveri | ah | 07:13 |
| zyga-kaveri | it's not the full list | 07:13 |
| zyga-kaveri | I think I know what is going on | 07:13 |
| * zyga-kaveri fixes | 07:16 | |
| pstolowski | zyga-kaveri: seems 3 tasks are missing (your new tasks I presume) | 07:22 |
| zyga-kaveri | pstolowski: yeah exactly :D | 07:23 |
| zyga-kaveri | man that run was pretty green overall | 07:25 |
| zyga-kaveri | one test adjustment, some random failures | 07:25 |
| zyga-kaveri | but otherwise it's pretty good | 07:25 |
| zyga-kaveri | I was only running core and main tests for ubuntu 16 | 07:25 |
| mborzecki | mvo: pedronis: i've updated #9347 | 08:15 |
| mup | PR #9347: tests/lib/nested.sh: use more focused cloud-init config for uc20 <Run nested> <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9347> | 08:15 |
| mvo | mborzecki: \o/ | 08:25 |
| zyga-kaveri | your video froze mvo | 08:32 |
| pedronis | mborzecki: mvo: so it takes around 50-60 to a resealing in the non-accel vms, almost 80 with two run kernels I suppose | 08:49 |
| mborzecki | wow | 08:49 |
| pedronis | mborzecki: we are hasing 2 or 3 kernels each time plus other ops plus signing | 08:49 |
| pedronis | mvo: mborzecki: actually not, we are not hashing kernels yet, so it will become even slower | 08:51 |
| zyga-kaveri | raspberry pi 1 speed! | 08:51 |
| pedronis | mborzecki: mvo: anyway, we should be able to cut down on the number of reseals, we get so many because current unasserted kernel logic | 08:52 |
| pedronis | speeding them up is a different matter, it would need help from secboot, also there are trade offs | 08:53 |
| mup | PR snapd#9350 opened: [RFC] store: handle v2 error when fetching assertions <Created by stolowski> <https://github.com/snapcore/snapd/pull/9350> | 08:59 |
| pstolowski | pedronis: hi, let me know if this is what you had in mind ^ ; i'll add tests if the idea is ok | 09:00 |
| * pstolowski back to snapshots | 09:03 | |
| mup | PR snapd#9351 opened: tests: simplify repack_snapd_snap_with_deb_content_and_run_mode_first_boot_tweaks <Cleanup :broom:> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9351> | 09:19 |
| zyga-kaveri | 2020-09-15 11:22:19 Executing google:ubuntu-20.04-64:tests/main/preseed (sep150911-454471) (1/1)... | 09:32 |
| zyga-kaveri | 2020-09-15 11:22:55 Successful tasks: 1 | 09:32 |
| zyga-kaveri | :D | 09:32 |
| zyga-kaveri | pstolowski: we should find a quiet time | 09:41 |
| zyga-kaveri | pstolowski: and demo vscode to the team | 09:41 |
| zyga-kaveri | pstolowski: it's such a gamechanger | 09:41 |
| zyga-kaveri | okay, onto snap-confine changes | 09:43 |
| zyga-kaveri | NOW IS THE TIME :D | 09:43 |
| pstolowski | zyga-kaveri: is it vim vs emacs vs vscode now? ;) | 09:54 |
| zyga-kaveri | pstolowski: for some people it's always vim vs emacs | 09:55 |
| zyga-kaveri | pstolowski: the rest just uses code | 09:55 |
| gko | For me, it's emacs AND code. | 10:03 |
| zyga-kaveri | I'm code and vim | 10:04 |
| zyga-kaveri | but vim much less often | 10:04 |
| zyga-kaveri | mainly inertia | 10:04 |
| zyga-kaveri | but code is just genuinely better | 10:04 |
| gko | code's remote is nice. | 10:06 |
| gko | Especially with low memory/disk space VM... | 10:06 |
| zyga-kaveri | yeah | 10:06 |
| zyga-kaveri | I only wish it had riscv and mips binaries | 10:07 |
| gko | Too bad for Perl, it needs at least 5.18 and my platform uses 5.8.9... | 10:08 |
| mvo | mborzecki: 9347 is at 1:28h already, I wonder if that is good or bad | 10:17 |
| mborzecki | mvo: looks like it's slow, i was able to see the logs and minimal smoke hit a 40m timeout | 10:17 |
| * mvo nods | 10:17 | |
| mborzecki | maybe we should have a longer kill timeout for the nested suite | 10:18 |
| mvo | yeah, might be needed | 10:18 |
| mvo | let's see if it completes, I really hope it does (successfully) | 10:18 |
| mvo | mborzecki: and it just failed in 2020-09-15T10:22:06.2176601Z - google-nested:ubuntu-20.04-64:tests/nested/manual/minimal-smoke :( | 10:22 |
| mborzecki | mvo: so that's it, hit a timeout on minimal/smoke, but otherwise the rest ran succesfuly | 10:22 |
| mvo | mborzecki: aha, timeout? ok, that sounds very promising | 10:23 |
| mborzecki | mvo: yup, a spread kill timeout | 10:23 |
| mvo | mborzecki: yeah, just saw it. sounds like we should raise it and re-run, can you do that? | 10:23 |
| mvo | mborzecki: but that is *very* encouraging, well done mborzecki and ijohnson :) | 10:23 |
| mvo | mborzecki: I really hope/think this could give us the reliable tests we need | 10:24 |
| mborzecki | mvo: hm i'd be leaning towards landing the PR and bumping the timeout in another one, looking at the logs, the test stated at 9:10, and at 9:49 it was executing tests/smoke/remove in the nested vm, so just running slow/late | 10:27 |
| mborzecki | wdyt? | 10:28 |
| mvo | mborzecki: works for me | 10:28 |
| mvo | mborzecki: done | 10:29 |
| mborzecki | thanks! | 10:29 |
| mvo | mborzecki: ok, let's include the longer timeout as one of the needed next steps (is more logging next?) | 10:29 |
| mup | PR snapd#9347 closed: tests/lib/nested.sh: use more focused cloud-init config for uc20 <Run nested> <Simple 😃> <UC20> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9347> | 10:29 |
| mborzecki | mvo: pedronis: #9343 is updated with a bump in kill timeouts and a little tweak for cleaning the serial log before each test (otherwise it would accumulate) | 10:41 |
| mup | PR #9343: tests: more logging for UC20 kernel test <Run nested> <Test Robustness> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9343> | 10:41 |
| pedronis | mvo: what's #9348 and how it relates to #9343 ? (cc mborzecki ) | 10:43 |
| mup | PR #9348: tests: print all the serial logs for the nested test <Run nested> <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9348> | 10:43 |
| mup | PR #9343: tests: more logging for UC20 kernel test <Run nested> <Test Robustness> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9343> | 10:43 |
| mborzecki | pedronis: looks like a followup/tweak we could do once 9343 lands? | 10:45 |
| pedronis | mborzecki: the changes to the cloud init config don't make sense anymore in it? don't we land some other code now? | 10:46 |
| pedronis | heh, didn't we land | 10:46 |
| mborzecki | ah, damn missed those | 10:46 |
| pedronis | it needs a master merge? | 10:47 |
| pedronis | ah, it just had one? | 10:47 |
| pedronis | mborzecki: mvo: fwiw I'm working on resealing a bit less even with unasserted kernels | 10:48 |
| mvo | mborzecki: does it make sense to land 9343 as it is or instead pull the logging changes out and land that separately and then we merge master into the kernel reseal tests? | 10:49 |
| mvo | (cc pedronis -^) | 10:49 |
| mborzecki | we can land the logging first maybe? | 10:50 |
| mborzecki | i can cherry pick the relevant commits and open a PR | 10:50 |
| pedronis | mborzecki: would think it's better, reading 9343 is a bit hard | 10:56 |
| mborzecki | ok | 10:56 |
| mborzecki | hmm landing #9311 would make it smaller still | 10:57 |
| mup | PR #9311: nested: add support to telnet to serial port in nested VM <Run nested> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9311> | 10:57 |
| pedronis | mborzecki: that looks ok and has a lot of +1 (it's a bit repetitive but so nested.sh) | 11:01 |
| mvo | mborzecki: I merged master into 9311, once that is green we should merge to master | 11:02 |
| mvo | mborzecki: also 9311 is not really needed for gce, we can always merge later | 11:03 |
| mborzecki | mhm | 11:03 |
| mvo | but in a meeting so only have 10% of my brain | 11:03 |
| mup | PR snapd#9352 opened: test: improve logging in nested tests <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9352> | 11:15 |
| mborzecki | pedronis: mvo: ^^ just the logging | 11:15 |
| mvo | mborzecki: \o/ | 11:23 |
| * mvo is still in a meeting | 11:23 | |
| dariball | I am putting together a custom-image as documented, works pretty good. I would now like to have some configurations changed like "sudo snap set somesnap foo=bar" for my custom-image, what would be the right way to do this? create a snap which is exclusively running those inside the configure hook? like a configuration-snap ? or is there some other prefered way? | 11:26 |
| ogra_ | dariball, the typical way is to use a custom gadget and set them in the "defaults:" in snapycraft.yaml | 11:29 |
| ogra_ | err. | 11:30 |
| ogra_ | s/snapcraft.yaml/gadget.yaml/ | 11:30 |
| ogra_ | dariball, https://snapcraft.io/docs/gadget-snap | 11:31 |
| dariball | therefore I would fork the default gadget snap, right, because I can only have one gadget snap? | 11:31 |
| ogra_ | yep | 11:31 |
| dariball | and I would add arbitrary commands to the prepare-device hook? | 11:32 |
| ogra_ | no | 11:32 |
| ogra_ | yu would use the gadget.yaml options for connections and for setting defaults | 11:32 |
| ogra_ | look at the linked page above | 11:32 |
| dariball | and running arbitrary commands shall be avoided? | 11:33 |
| dariball | because let's say I have some cli tool from a snap I would like to run ? | 11:34 |
| ogra_ | well, thats not esaily doable without forking the snap and make some daemonized script | 11:35 |
| ogra_ | (snaps can not easily call commands from other snaps, else you'd be able to just create a snap full of scripts) | 11:36 |
| dariball | okok, thx this already helps alot, hope I get along with connections + defaults, looks good... | 11:37 |
| ogra_ | if you have a brands stroe you *can* use the snapd REST API from a config or tooling snap though, that allows things beyond the limited stuff | 11:37 |
| ogra_ | *brand store | 11:37 |
| ogra_ | but use of the interface (snapd-control) that gives you access to the API is brand store bound, snaps in the global store do not get permission to use it | 11:38 |
| pedronis | mvo: mborzecki: I haven't proposed but here are the changes to reseal less with unasserted kernels: https://github.com/pedronis/snappy/commit/39746b163590c1f6be0103362b8fcfeee0f32338 | 11:40 |
| zyga-kaveri | ok, modified snap-confine and some small bits, running in complain snap-confine apparmor profile to see what we get | 11:57 |
| mup | PR snapd#9185 closed: secboot: use the snapcore/secboot native recovery key type <UC20> <Created by cmatsuoka> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/9185> | 12:05 |
| mborzecki | funny when spread hangs on discarding a node | 12:05 |
| zyga-kaveri | brb | 12:07 |
| mborzecki | pedronis: the patch looks reasonable, are you going to propose a branch? | 12:10 |
| pedronis | mborzecki: probably not before we get other things in | 12:12 |
| mup | PR snapcraft#3285 closed: v1 plugins: lock godep's dependencies <maintenance> <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3285> | 12:12 |
| mup | PR snapcraft#3287 opened: elf: reduce noise in the developer debug logs <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3287> | 12:12 |
| mborzecki | pedronis: ack | 12:12 |
| mborzecki | heh, finally the nested suite is in progress in 9352 | 12:24 |
| zyga-kaveri | exported tools work | 12:29 |
| zyga-kaveri | I need to adjust apparmor profiles | 12:29 |
| zyga-kaveri | but it's functional | 12:29 |
| zyga-kaveri | \o/ | 12:29 |
| mborzecki | heh, nested tests i ran: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! | 12:43 |
| mborzecki | that's when rebooting from install -> run | 12:44 |
| cachio | mborzecki, I saww that yesterday as erll | 12:44 |
| cachio | well | 12:44 |
| cmatsuoka | ouch | 12:44 |
| mborzecki | and random, didn't happen in other tests | 12:45 |
| cachio | it is very sporadic | 12:45 |
| cmatsuoka | maybe -smp 1 could help mitigate this? | 12:46 |
| cachio | cmatsuoka, I saw that just running with kvm enabled | 12:47 |
| cachio | mborzecki, are you running with kvm enalbed right? | 12:47 |
| cmatsuoka | cachio: what version of ovmf are we using in our tests? | 12:48 |
| cachio | cmatsuoka, need to check, I tried with the latest and the once from proposed | 12:48 |
| mborzecki | cachio: no, this was on gcp | 12:51 |
| mborzecki | cmatsuoka: and the whichever version of qemu/ovmf we have in focal | 12:52 |
| cachio | mborzecki, how did you run the test on gcp? | 12:52 |
| mborzecki | cachio: spread -debug -v google-nested:ubuntu-20.04-64:tests/nested/... | 12:52 |
| cachio | cmatsuoka, this is the version we use of ovmf 0~20191122.bd85bf54-2ubuntu3 | 12:53 |
| cachio | mborzecki, well the tests in core20 suite will run without kvm but tests on manual suite will run with kvm enabled | 12:54 |
| cachio | mborzecki, don't know which test showed that panic | 12:54 |
| mborzecki | cachio: tests/nested/manual/core20-early-config so maybe it's related to kvm | 12:55 |
| cachio | mborzecki, ahh, I think so | 12:55 |
| cmatsuoka | the groovy ovmf is a bit newer, but many changes in upstream since then | 12:58 |
| mup | PR snapd#9311 closed: nested: add support to telnet to serial port in nested VM <Run nested> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9311> | 13:00 |
| mvo | mborzecki: 9352 just passesd | 13:24 |
| mborzecki | mvo: just landed it | 13:24 |
| mvo | mborzecki: \o/ third in a row, fingers crossed | 13:24 |
| mborzecki | mvo: i'll cherry pick the spread test from your PR and add it to kernel reseal one | 13:24 |
| mvo | mborzecki: sounds great | 13:24 |
| mvo | mborzecki: feel free to close the test PR from me once you chrry-picked to the right place | 13:25 |
| mup | PR snapd#9352 closed: test: improve logging in nested tests <Run nested> <UC20> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9352> | 13:25 |
| zyga-kaveri | I'm going to break for lunch now | 13:42 |
| zyga-kaveri | ttyl | 13:42 |
| mborzecki | pedronis: mvo: i've updated https://github.com/snapcore/snapd/pull/9331 | 13:44 |
| mup | PR #9331: boot: reseal when changing kernel <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9331> | 13:44 |
| mborzecki | mvo: i've dropped the patch where test waits for snap to become available, we have that covered in nested setup now | 13:45 |
| mup | PR snapd#9338 closed: tests: add nested core20 kernel reseal test <Run nested> <UC20> <Created by mvo5> <Closed by bboozzoo> <https://github.com/snapcore/snapd/pull/9338> | 13:45 |
| mup | PR snapd#9351 closed: tests: simplify repack_snapd_snap_with_deb_content_and_run_mode_first_boot_tweaks <Cleanup :broom:> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9351> | 13:45 |
| pedronis | mborzecki: reviewed, it needs a 2nd review | 13:49 |
| mborzecki | pedronis: thanks! | 13:49 |
| mborzecki | mvo: ijohnson: could you take a look at #9331? | 13:49 |
| mup | PR #9331: boot: reseal when changing kernel <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9331> | 13:49 |
| ijohnson | sure | 13:49 |
| mvo | mborzecki: of course | 13:51 |
| pedronis | my own comment on it are addressed in #9340 (which is adding some tests, a doing some refacor/renames) | 13:52 |
| mup | PR #9340: boot: streamline bootstate20.go reseal and tests changes <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9340> | 13:52 |
| === tomwardill_ is now known as tomwardill | ||
| === bluesabre_ is now known as bluesabre | ||
| === marosg_ is now known as marosg | ||
| === beisner_ is now known as beisner | ||
| === coreycb_ is now known as coreycb | ||
| === philroche_ is now known as philroche | ||
| === urluck_ is now known as urluck | ||
| === ogra_ is now known as Guest47593 | ||
| mup | PR snapd#9321 closed: cmd/snap/model: specify grade in the model command output <Simple 😃> <UC20> <Created by anonymouse64> <Merged by anonymouse64> <https://github.com/snapcore/snapd/pull/9321> | 14:10 |
| ijohnson | mborzecki: pedronis I had a question on the unit tests in 9331 | 14:17 |
| ijohnson | perhaps I have misunderstood something | 14:17 |
| pedronis | there is at least one test that my PR changes to do something quite different | 14:18 |
| pedronis | more in line with the name | 14:18 |
| pedronis | ah, something else, yes I think some of those tests are cheating a bit | 14:19 |
| pedronis | because the mock bootloader has limitations | 14:20 |
| ijohnson | ok, is it alright that we aren't testing the same way things are being used in real life? | 14:20 |
| pedronis | well, we just feed all that to a mocked secboot reseal | 14:20 |
| pedronis | that just check that the processing looked alright | 14:21 |
| ijohnson | do we have anywhere that unit tests end to end with the actual expected boot chain? or is that just unnecessary since we will have the spread tests | 14:21 |
| pedronis | so I think it's ok but might confuse us later, but it's not a blocker atm | 14:21 |
| ijohnson | ok, thanks | 14:21 |
| pedronis | ijohnson: yes, we do have tests that are more realistic | 14:21 |
| pedronis | seal_test.go own tess are more realistic | 14:21 |
| ijohnson | ok sounds good | 14:21 |
| pedronis | the comment about checking the env seems legit though | 14:22 |
| pedronis | it might be easier to pick it up in my PR tough | 14:22 |
| ijohnson | that's fine with me | 14:24 |
| ijohnson | I'm finishing up a reivew of 9340 now btw | 14:24 |
| === xnox1 is now known as xnox | ||
| mborzecki | 16.04 failed with google:ubuntu-16.04-64:tests/main/lxd:snapd_cgroup_neither | 14:51 |
| * mvo is off for a couple of minutes to taxi kids around, tg for emergencies | 14:53 | |
| cachio | mborzecki, again for me https://paste.ubuntu.com/p/6F4jypF8Wd/ | 14:55 |
| mup | PR snapd#9353 opened: [RFC] configcore: do not error in console-conf.disable for install mode <Created by mvo5> <https://github.com/snapcore/snapd/pull/9353> | 14:55 |
| mborzecki | hm looks like we still need to do something about the client timeout | 14:59 |
| ijohnson | mborzecki: which client timeout? | 15:00 |
| mborzecki | doTimeout in client.go | 15:00 |
| mborzecki | was doing a couple of kernel reseal runs on gcp and one failed with cannot communicate with the server, client timeout exceeded and so on | 15:00 |
| mborzecki | it's set to 50s atm, maybe we should have an env variable to make it longer | 15:01 |
| * cachio lunch | 15:12 | |
| ijohnson | ah | 15:18 |
| ijohnson | yeah we should probably bump that timeout, there are reports of it not being long enough on the forum too on like rpi or somethin I think | 15:18 |
| mborzecki | mhm | 15:20 |
| mup | PR snapcraft#3238 closed: db: introduce generalized datastore <enhancement> <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3238> | 15:22 |
| jdstrand | zyga-kaveri: hey, please have snap-confine PRs still go through us. feel free to ping me and I'll assign someone | 15:29 |
| jdstrand | not that you weren't going to do that, just wanted to make sure people know :) | 15:30 |
| jdstrand | cc pedronis ^ | 15:30 |
| * jdstrand would cc mvo but he's not here atm :) | 15:30 | |
| mborzecki | hm mvo isn't around anymore? | 15:43 |
| ijohnson | yeah he said he had to go taxi his kids around | 15:43 |
| mborzecki | ijohnson: ah, thanks for the info | 15:43 |
| ijohnson | mborzecki: any pr's I should review right now? | 15:44 |
| ijohnson | I reviewed both 9331 and 9340 this morning | 15:44 |
| mborzecki | ijohnson: you can try #9341 | 15:48 |
| mup | PR #9341: tests: add nested core20 gadget reseal test <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9341> | 15:48 |
| ijohnson | mborzecki: ack | 15:48 |
| mborzecki | pedronis: #9331 spread run finished, there's failure in nested tests, tests/nested/core20/basic which expected test-snapd-sh to be there but looking at the log it got 400 over the API, and minimal-smoke which looks like it was just killed (?) but went throught 3 out of 4 smoke tests by that time | 15:51 |
| mup | PR #9331: boot: reseal when changing kernel <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9331> | 15:51 |
| pedronis | mborzecki: minimal smoke failed also on 18 ? | 15:57 |
| ijohnson | pedronis: mborzecki I have seen minimal smoke fail on 18 a few times and have an idea why it failed | 15:57 |
| ijohnson | I reproduced it yesterday but ran out of time to fully debug | 15:57 |
| mborzecki | pedronis: yeah, but there it seems like an ssh thing | 15:58 |
| ijohnson | essentially when we create the users and specify them as sudoers by writing the files, somehow those files get lost/ reset to be empty when the vm is shut down | 15:58 |
| ijohnson | so I was going to try by doing a sync inside the vm before shutting down, then a sync outside the vm before starting it up again | 15:58 |
| mborzecki | funny, becuse the setup was successful, `cannot connect: cannot connect to external:ubuntu-core-18-64: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none password], no supported methods remain` | 15:58 |
| mborzecki | ijohnson: wooo, but we call shutdown in the vm, don't we? | 15:59 |
| ijohnson | yes | 15:59 |
| ijohnson | it's bugs all the way up and down the stack in nested vms | 15:59 |
| mborzecki | haha | 15:59 |
| mborzecki | apaprently ;) | 15:59 |
| ijohnson | but I would not be worried about minimal smoke on 18 if that was the only thing that failed | 15:59 |
| mborzecki | anyways, i don't think the failures in nested core20 were related to the kernel reseal test | 16:00 |
| mborzecki | and we really should do the repacking somewhere outside of $SPREAD_PATH, `2020-09-15T15:30:24.5255269Z 2020-09-15 15:13:31 Project content is packed for delivery (350.90MB).` | 16:01 |
| pedronis | fun | 16:01 |
| ijohnson | oof | 16:01 |
| pedronis | mborzecki: ijohnson: so you are saying I should force merge #9331 and the tweak my follow-up? | 16:02 |
| mup | PR #9331: boot: reseal when changing kernel <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9331> | 16:02 |
| mborzecki | pedronis: yeah, i'll add a note about failures to the PR | 16:02 |
| ijohnson | pedronis: let me have a look again at the test to make sure I'm not errantly identifying the fialure without looking at the logs | 16:03 |
| mborzecki | ha, ok i see why nested/basic failed, it's `error: cannot communicate with server: timeout exceeded while waiting for response` | 16:03 |
| mborzecki | so the timeout tweak | 16:03 |
| mborzecki | and the test does something weird, there's `nested_exec "sudo snap install test-snapd-sh" || true` with a note that ssh sometimes dies at this point> | 16:05 |
| ijohnson | mmm very suspicious | 16:05 |
| pedronis | it should do --no-wait maybe? | 16:05 |
| ijohnson | but pedronis mborzecki I looked at the non-uc20 logs for 9331 and I wouldn't be worried about any of those other failures | 16:05 |
| pedronis | mmh, it doesn't have two +1 atm | 16:06 |
| ijohnson | sorry my comment wasn't a +1 because I was unsure about the test situation | 16:07 |
| ijohnson | pedronis: I +1d it formally | 16:07 |
| pedronis | thx | 16:07 |
| pedronis | mborzecki: I'll thos kernel_status checks to mine | 16:08 |
| pedronis | *I'll add those | 16:08 |
| mborzecki | pedronis: cool, thank you | 16:08 |
| pedronis | mborzecki: this one is also adding the spread test, right? | 16:10 |
| mborzecki | pedronis: yes, the kernel-reseal test was successful | 16:11 |
| pedronis | mborzecki: ijohnson: merged | 16:13 |
| mborzecki | pedronis: thanks! | 16:13 |
| ijohnson | \o/ | 16:13 |
| ijohnson | mvo will be happy | 16:13 |
| pedronis | now I will to deal with conflicts in mine | 16:13 |
| mvo | pedronis: yeah just saw it \o/ | 16:14 |
| mvo | ijohnson: I am! | 16:14 |
| mvo | mborzecki: \o/ as well | 16:14 |
| mvo | pedronis: I assume you merge master into 9340 now? | 16:14 |
| ijohnson | :-) | 16:14 |
| mborzecki | mvo: shall i merge master to #9341 and make it non-draft? | 16:15 |
| mup | PR #9341: tests: add nested core20 gadget reseal test <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9341> | 16:15 |
| mvo | mborzecki: yeah | 16:15 |
| mvo | mborzecki: it has this really ugly wart that it makes assumptions about the edition | 16:16 |
| mup | PR snapd#9331 closed: boot: reseal when changing kernel <Run nested> <UC20> <Created by bboozzoo> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9331> | 16:16 |
| mup | PR snapd#9343 closed: tests: more logging for UC20 kernel test <Run nested> <Test Robustness> <UC20> <Created by bboozzoo> <Closed by bboozzoo> <https://github.com/snapcore/snapd/pull/9343> | 16:16 |
| mvo | mborzecki: let me quickly look if I could improve that with some python-json | 16:16 |
| ijohnson | mborzecki: mvo: in that pr, what does the XXX2 about? how were you trying to update ubuntu-boot? | 16:17 |
| mvo | ijohnson: just bumping the editon there | 16:17 |
| ijohnson | does it still fail like that? the nested test is green on that pr | 16:17 |
| ijohnson | maybe that was an old failure? | 16:17 |
| pedronis | it will do nothing if the content is the same though | 16:17 |
| ijohnson | pedronis: the content is changed | 16:18 |
| ijohnson | whitespace is added to the recovery grub | 16:18 |
| pedronis | ah, ok | 16:18 |
| mborzecki | shall i propose this? https://paste.ubuntu.com/p/fKWpKjkGNz/ | 16:18 |
| mvo | yeah, so I think it will still be a problem, I can do a separate PR that illustrates the problem, I think it might be a bug in our | 16:19 |
| mvo | in our gadget update code | 16:19 |
| ijohnson | mborzecki: lgtm ship it | 16:19 |
| mvo | mborzecki: seems reasonable, I think we got a similar request recently by a customer | 16:19 |
| mborzecki | i can set spread to run the nested suite when opening the PR and we'll see about that basic test | 16:20 |
| * mvo nods | 16:20 | |
| mup | PR snapd#9354 opened: client: bump the default request timeout to 120s <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9354> | 16:21 |
| mvo | ijohnson: but to be clear about the XXX2, I think it can wait for later this week :) | 16:21 |
| ijohnson | ack | 16:21 |
| ijohnson | I +1d it, lgtm with some small suggestions that can be a followup with no-spread tag | 16:22 |
| mvo | \o/ | 16:22 |
| mvo | I will work on a cleanup with better python3 json instead of the fugly shell|pipes | 16:28 |
| ijohnson | you could use an imcprehensible set of jq expressions too :-) | 16:30 |
| ijohnson | pedronis: instead of CloudInitRestrictOptions.DisableCloudInit as an opt name, what do you think about AlwaysDisableAfterLocalDatasourcesRun ? | 16:32 |
| ijohnson | that conveys that it onnly disables after it runs, and lets us make clear that it applies to both NoCloud and now None too | 16:33 |
| ijohnson | (and potentially others as we see fit later) | 16:33 |
| pedronis | seems ok, do we need the "Always" bit in it? | 16:33 |
| ijohnson | mmm maybe not | 16:34 |
| ijohnson | DisableAfterLocalDatasourcesRun seems to make sense enough | 16:34 |
| mborzecki | ijohnson: added a comment explaining what's happening https://github.com/snapcore/snapd/pull/9341/files#r488806612 | 16:39 |
| mup | PR #9341: tests: add nested core20 gadget reseal test <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9341> | 16:39 |
| ijohnson | mborzecki: thanks looking now | 16:39 |
| ijohnson | mborzecki: ah thanks that makes sense I forgot that the edition for the different structures can be different | 16:40 |
| mborzecki | ijohnson: (cc mvo) that error is pure accident by trying simple s/edition: 1/edition: 2/ | 16:40 |
| mvo | mborzecki: haha - so the sed is too stupid? | 16:40 |
| pedronis | ijohnson: mborzecki: I merged master and updated #9340 | 16:40 |
| mup | PR #9340: boot: streamline bootstate20.go reseal and tests changes <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9340> | 16:40 |
| ijohnson | can I mark 9341 ready for review ? | 16:40 |
| ijohnson | pedronis: thanks will try to look before my meeting in 5 minutes | 16:41 |
| mvo | ijohnson: +1 | 16:41 |
| mvo | ijohnson: done that now | 16:41 |
| mborzecki | mvo: the regular gadget update spread test mangles gadget.yaml with some python code, but we can improve the nested test later | 16:41 |
| mvo | mborzecki: yeah, that sounds reaonable. as long as its a silly issue on the test side by me I'm happy | 16:42 |
| mvo | mborzecki: I put this on my todo | 16:42 |
| mborzecki | mvo: take a look at tests/core/gadget-update-pc/generate.py | 16:42 |
| ijohnson | pedronis: ah it occurs to me now that all of the tests using MockTrustedAssetsBootloader are not using ExtractedRunKernelImageBootloader logic, they are all using EnvRefExtractedKernelBootloader's instead | 16:44 |
| ijohnson | not sure if that's an issue or not | 16:44 |
| mvo | mborzecki: cool, thanks | 16:44 |
| ijohnson | because you have snap_kernel in the bl vars, which won't be the case for a real trusted assets bl because it will be an extracted one | 16:44 |
| * ijohnson -> meeting | 16:45 | |
| pedronis | ijohnson: that is something that comes from the previous PR | 16:45 |
| pedronis | that we just landed | 16:45 |
| ijohnson | pedronis: yes I didn't realize it before | 16:45 |
| ijohnson | sorry | 16:45 |
| ijohnson | I don't think it's an issue | 16:45 |
| ijohnson | well | 16:45 |
| ijohnson | I don't think it's a large issue | 16:45 |
| mborzecki | ijohnson: yeah, mocking the bootloader was a bit pita | 16:45 |
| pedronis | well, bootloader is a zoo that we need to fix | 16:45 |
| ijohnson | but it would be nice if we had at least one unit tests that uses both the extracted run kernel image impl and the trusted assets bl | 16:45 |
| ijohnson | because we won't have any devices in the wild for a while yet that are env ref kernel and trusted assets | 16:46 |
| * ijohnson -> really meeting now | 16:46 | |
| * mvo needs to taxi around again | 16:46 | |
| pedronis | mborzecki: anyway this a problem we get from bootloadertest, no? | 16:47 |
| pedronis | WithTrusted assets has the wrong mix of things? | 16:47 |
| pedronis | all the test do is tab := bootloadertest.Mock("trusted", "").WithTrustedAssets() | 16:48 |
| mborzecki | pedronis: yes | 16:48 |
| mborzecki | pedronis: we'd have to have different variants of trusted assets bootloader, one for extracted run kernel, and another for env ref | 16:49 |
| pedronis | well, we really need one for now | 16:49 |
| pedronis | but we have kind of the wrong one :) | 16:49 |
| pedronis | anyway this is not something I can solve in my PR | 16:50 |
| pedronis | I can add a TODO though | 16:50 |
| ijohnson | we won't have any trusted assets bootloaders in practice that are env ref (like uboot or lk) until we add support for more bootloaders to uc20 | 16:50 |
| ijohnson | the only trusted bootloader today will be grub which is always extracted run kernel on uc20 | 16:50 |
| ijohnson | yes I think TODO is fine | 16:51 |
| pedronis | yes, what I meant with we need one | 16:51 |
| ijohnson | right | 16:51 |
| mborzecki | iirc tried to use extracted run kernel one at some point, but it was a bit larger than any of the prs i had at the time | 16:51 |
| mborzecki | i can look into that tomorrow morning | 16:52 |
| ijohnson | I don't doubt that it was incredibly complicated | 16:52 |
| pedronis | yes, it has a more involved interface | 16:52 |
| ijohnson | mock bootloaders is a mess of a zoo on top of the mess of a zoo that is already bootloaders | 16:52 |
| pedronis | and usually we have helpers around it | 16:52 |
| pedronis | mborzecki: it might not be worth fixing it, before we fix the zoo | 16:52 |
| mborzecki | that may be true | 16:52 |
| pedronis | mborzecki: ijohnson: added this: https://github.com/snapcore/snapd/pull/9340/commits/e22c82485de00a54aeb8a31bec7092af89778991 | 16:56 |
| mup | PR #9340: boot: streamline bootstate20.go reseal and tests changes <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9340> | 16:57 |
| ijohnson | pedronis: thanks | 16:57 |
| ijohnson | +1d the pr | 16:58 |
| mborzecki | take a look at the changelog: https://bodhi.fedoraproject.org/updates/FEDORA-2020-0d5e544db7 | 17:04 |
| mborzecki | /var/lib/snapd/snap/bin is in secure_path in sudo config on fedora now | 17:04 |
| mborzecki | zyga: Eighth_Doctor: ^^ | 17:04 |
| zyga | mborzecki In a call | 17:13 |
| zyga | but will look soon | 17:13 |
| cachio | ijohnson, hey, I updated #9326 | 17:15 |
| mup | PR #9326: tests: fix for basic20 test running on external backend and rpi <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9326> | 17:15 |
| zyga | bodhi seems very slow somehow | 17:15 |
| cachio | if you could take quick look would be nice | 17:16 |
| ijohnson | thanks cachio will look in a bit | 17:16 |
| Eighth_Doctor | mborzecki: yay! | 17:18 |
| pedronis | mborzecki: ijohnson: I proposed #9355 | 17:28 |
| mup | PR #9355: boot: with unasserted kernels reseal if there's a hint modeenv changed <Run nested> <Skip spread> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9355> | 17:28 |
| mup | PR snapd#9355 opened: boot: with unasserted kernels reseal if there's a hint modeenv changed <Run nested> <Skip spread> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9355> | 17:31 |
| mborzecki | mvo: pedronis: i've pinged you in #9354 | 18:24 |
| mup | PR #9354: client: bump the default request timeout to 120s <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9354> | 18:24 |
| ijohnson | mmm minimal-smoke on uc18 strikes again in that pr | 18:24 |
| ijohnson | I'll file a pr with that sync idea I had, I don't think it can hurt anything | 18:24 |
| mborzecki | ijohnson: ok, if not i'll try to investigate that tomorrow | 18:25 |
| ijohnson | note that I only reproduced it by running spread in a loop, don't use the -repeat option to spread, because that will just reuse the same vm | 18:25 |
| ijohnson | i.e. try something like `for i in $(seq 1 10); do spread --debug google-nested:ubuntu-18.04-64:tests/nested/manual/minimal-smoke; done` | 18:26 |
| mborzecki | uhh #9341 has spread jobs still queued | 18:26 |
| mup | PR #9341: tests: add nested core20 gadget reseal test <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9341> | 18:26 |
| ijohnson | mborzecki: hmm try canceling the jobs and restarting them maybe? | 18:27 |
| mborzecki | i'll let it sit for a while | 18:27 |
| mborzecki | actually need to wrap it up, chase the kids to their beds otherwise they are going to have hard time getting up for school tomorrow | 18:28 |
| ijohnson | oh man don't ever run `go test -count=100 ./...` unless you want to cry and be scared about our tests | 18:28 |
| mborzecki | hahah | 18:28 |
| ijohnson | so many tests get broken somehow | 18:28 |
| ijohnson | perhaps we should have a spread test which just does this | 18:29 |
| pedronis | why? | 18:33 |
| mvo | I see that the checks for 9340 got canceled, why? | 18:33 |
| ijohnson | I assume lots of tests aren't actually properly cleaning themselves up | 18:33 |
| ijohnson | mvo: I just canceled them to restart them | 18:34 |
| ijohnson | they were stuck | 18:34 |
| mvo | thanks! | 18:34 |
| ijohnson | restarted now | 18:34 |
| pedronis | ijohnson: I'm not even sure go check supports that tbh | 18:34 |
| ijohnson | pedronis: oh really ? huh | 18:35 |
| pedronis | it's a go test option after all | 18:35 |
| pedronis | we have one Test in the go level sense per package | 18:35 |
| pedronis | so not sure if it makes sense, what it does in practice for us | 18:35 |
| ijohnson | mmm I guess gocheck doesn't even have a count function | 18:36 |
| ijohnson | err s/function/option/ | 18:36 |
| pedronis | it doesn't | 18:37 |
| ijohnson | still feels like this should work though | 18:37 |
| ijohnson | and to be fair for low numbers of count it does seem to work | 18:37 |
| pedronis | heh | 18:38 |
| pedronis | anyway I fear is a matter where feelings don't count. if we have enough seen flakiness without poking for more | 18:38 |
| pedronis | mvo: I canceled them because I had more to push | 18:38 |
| mvo | ok | 18:39 |
| ijohnson | yes I'm ok if we just want to shove this under the rug until it seems relevant | 18:39 |
| pedronis | mvo: I'm not sure how you want to proceed landing wise | 18:39 |
| pedronis | mvo: do we need to sync? | 18:39 |
| mvo | pedronis about the order? | 18:39 |
| pedronis | yes, and whether you want to have some control over what we run tests for vs not | 18:39 |
| pedronis | mvo: we have 6 PRs for 2.47: https://github.com/snapcore/snapd/milestone/32 | 18:40 |
| pedronis | plus the one Maciej asked to consider | 18:41 |
| mvo | pedronis: which one is the one from maciej? | 18:41 |
| mvo | pedronis: yeah, maybe a sync would be best | 18:41 |
| pedronis | mvo: https://github.com/snapcore/snapd/pull/9354 | 18:41 |
| mup | PR #9354: client: bump the default request timeout to 120s <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9354> | 18:41 |
| pedronis | mvo: we can sync if you want | 18:42 |
| mvo | pedronis: yeah, let me grab my headset | 18:42 |
| mvo | pedronis: ready when you are in the standup channel | 18:43 |
| mup | PR snapd#9340 closed: boot: streamline bootstate20.go reseal and tests changes <Run nested> <UC20> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9340> | 18:51 |
| mvo | ijohnson: do you think you could have a look at 9355 ? | 18:54 |
| ijohnson | mvo: sure it is in my queue for today, should I look now? | 18:54 |
| mvo | ijohnson: I had hoped to land it in my day but I'm not sure that is feasible :/ | 18:55 |
| ijohnson | mvo: ack I will look at it now then | 18:55 |
| mup | PR snapd#9353 closed: configcore: do not error in console-conf.disable for install mode <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9353> | 19:02 |
| mvo | ijohnson: thank you! | 19:06 |
| mup | PR snapd#9354 closed: client: bump the default request timeout to 120s <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9354> | 19:07 |
| mup | PR snapd#9356 opened: sysconfig,o/devicestate: mv DisableNoCloud to DisableAfterLocalDatasourcesRun <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9356> | 19:12 |
| ijohnson | mvo: pedronis: approved 9355 conditional on my understanding being correct about a question on gadget update codeflow | 19:12 |
| * zyga will sort out family and will return to make some more changes to export manager | 19:14 | |
| pedronis | ijohnson: I answered, let me know if the answer makes sense | 19:14 |
| * ijohnson looks | 19:14 | |
| ijohnson | pedronis: yep that's basically what I thought so good to go from me | 19:15 |
| mup | PR snapd#9357 opened: interfaces/builtin/process-control: add scheduler_setattr to seccomp <Needs security review> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9357> | 19:47 |
| mvo | thanks | 19:48 |
| cmatsuoka | mvo: the gadget update failed | 19:51 |
| mvo | cmatsuoka: oh no, in what way? | 19:52 |
| cmatsuoka | mvo: it tries to compare the volume name and ends with "cannot find device matching structure #0 ("mbr"): unexpected number of matches (0) for /sys/block/*/ubuntu-data-20833fbc-9153-4ccd-b00b-d687b76dfd0a" | 19:53 |
| zyga | jdstrand lol | 19:53 |
| zyga | jdstrand funny | 19:53 |
| jdstrand | my review comment for that? ^ :) | 19:53 |
| zyga | yes | 19:53 |
| mvo | cmatsuoka: woah, that is strange | 19:53 |
| pedronis | mvo: cmatsuoka: that's a more fundamental problem, not really related to reseal | 19:53 |
| zyga | jdstrand I asked about RR scheduling | 19:54 |
| zyga | it feels unsafe | 19:54 |
| cmatsuoka | pedronis: yes, exactly | 19:54 |
| mvo | cmatsuoka: thanks! and yes, not related to reseal :( | 19:54 |
| zyga | unless process control is very privileged already | 19:54 |
| cmatsuoka | mvo: the full error message is in my SU notes, now checking the gadget update code | 19:55 |
| mvo | cmatsuoka: thanks, I think worst case is we need to skip this PR for 2.47~rc1 | 19:55 |
| jdstrand | zyga: it is quite privileged already | 19:55 |
| jdstrand | you can kill or renice an process | 19:56 |
| zyga | jdstrand the risk here, as you surely know, is that rr scheduling can lock out all processes and hang the system for most practical purposes | 19:56 |
| ijohnson | fixed my typo :-) | 19:56 |
| zyga | ijohnson do you know if the desire is to use RR scheduling or anything else? | 19:56 |
| cmatsuoka | mvo: did we test gadget updates recently? it could be there since we added the uuid to ubuntu-data | 19:56 |
| jdstrand | zyga: sure. so can creative use of kill :) | 19:56 |
| ijohnson | zyga: all I know is what's on the forum post | 19:56 |
| ijohnson | cmatsuoka: we should have gadget asset update spread tests that run on every pr | 19:57 |
| zyga | jdstrand ish, we can mediate kill, there's no real way around runaway RR process | 19:57 |
| zyga | ijohnson let me read it | 19:57 |
| ijohnson | cmatsuoka: see tests/core/gadget-update-pc | 19:57 |
| cmatsuoka | ijohnson: ah ok, that's good, thanks | 19:57 |
| ijohnson | maybe the test is insufficient for what we changed in uc20 though | 19:58 |
| jdstrand | zyga: plus, we can't really mediate to the level that we would like since we can't control the pid or the sched_attr struct | 19:58 |
| zyga | oh, right :( | 19:58 |
| jdstrand | zyga: I'm saying, we can mediate kill, and we do, but we grant it in process-control | 19:58 |
| zyga | I wish we had that CPU pinning interface so that we can ensure at least one core remains for system apps | 19:59 |
| zyga | that's fine, I just wanted to raise this | 19:59 |
| jdstrand | if we get some finer controls, we can move towards breaking this up | 19:59 |
| ijohnson | cmatsuoka: I think that mborzecki had a comment about gadget asset updates this morning and there was something for him to work on related to this area | 19:59 |
| ijohnson | (specifically around the device mapper nodes,etc.) | 19:59 |
| zyga | done | 20:00 |
| cmatsuoka | ijohnson: the existing test is unencrypted only, and it fails with the encrypted device name with the uuid I think | 20:00 |
| ijohnson | cmatsuoka: though looking at the generate.py, it only handles ubuntu-seed edition updates afaict | 20:00 |
| ijohnson | cmatsuoka: right | 20:01 |
| ijohnson | yeah so I think this is the bug that mborzecki was referring to this morning | 20:01 |
| pedronis | https://github.com/snapcore/snapd/pull/9341/files has the bug in the comment | 20:02 |
| mup | PR #9341: tests: add nested core20 gadget reseal test <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9341> | 20:02 |
| pedronis | in the test | 20:02 |
| ijohnson | cachio: sorry one more comment on 9326 | 20:03 |
| pedronis | mvo: I'm not sure I understand the test in #9341, isn't it changing a file that we ignore | 20:04 |
| mup | PR #9341: tests: add nested core20 gadget reseal test <Run nested> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9341> | 20:04 |
| cmatsuoka | ok, so maciek is aware | 20:04 |
| ijohnson | pedronis: isn't the recovery grub in the boot chain for run mode though? | 20:05 |
| ijohnson | pedronis: ahhhh but that's the recovery grub executable not the config | 20:05 |
| ijohnson | good catch | 20:05 |
| pedronis | the config should come from snapd | 20:05 |
| cachio | ijohnson, done | 20:06 |
| ijohnson | pedronis: but is the .cfg file included in the resealing ? | 20:06 |
| pedronis | no | 20:06 |
| pedronis | it's not a trusted asset either way | 20:06 |
| pedronis | so I'm not quite sure what that test does | 20:07 |
| ijohnson | right so a "realer" test would involve changing grubx64.efi right? | 20:07 |
| ijohnson | thanks cachio, +1 | 20:07 |
| cachio | ijohnson, thanks | 20:07 |
| pedronis | ijohnson: yes | 20:07 |
| ijohnson | hmm I wonder if we can just pad 0's on the end or something | 20:07 |
| cachio | I am going to pay some taxes now | 20:07 |
| cachio | I'll be back in 30 minutes | 20:07 |
| * cachio afk | 20:08 | |
| pedronis | ijohnson: mvo: anyway in theory we should drop those .conf files from the gadget | 20:08 |
| pedronis | they should be ignored anyway | 20:08 |
| mvo | pedronis: uh, good catch indeed. but the test seems to indicate we do reseal? | 20:08 |
| pedronis | mvo: well, we reseal at each reboot | 20:08 |
| pedronis | so not sure | 20:08 |
| mvo | at least it did indicate that at some point when I was running it | 20:08 |
| mvo | pedronis: fun :( | 20:08 |
| mvo | pedronis: ok, then the test is no good | 20:09 |
| pedronis | I mean, it will mean something after my branch lands | 20:09 |
| mvo | pedronis: let me mark it as blocked and it will need a better test | 20:09 |
| ijohnson | mvo: pedronis: I can try modifying the test to just pad 0's on the end of grubx64.efi, not sure if that will boot or not | 20:09 |
| mvo | ijohnson: \o/ | 20:10 |
| pedronis | ijohnson: it's a pe binary I think, there might be some tools to change something in it and then sign it again | 20:11 |
| ijohnson | mmm yeah but padding 0's on the end is easier if it works | 20:11 |
| pedronis | it has section with sizes, so not sure what adding 0 add it end does | 20:12 |
| pedronis | I would suspect not to work but maybe I'm wrong | 20:12 |
| pedronis | maybe it works but it's a noop, not sure | 20:12 |
| mvo | easy enough to try :) | 20:13 |
| ijohnson | alright building image with 0 appended to the grubx64.efi | 20:13 |
| ijohnson | let's see what happens | 20:13 |
| ijohnson | nope didn't work | 20:15 |
| ijohnson | https://www.irccloud.com/pastebin/jeEA8icw/ | 20:16 |
| ijohnson | alright let's look at these pe tools then | 20:16 |
| pedronis | ijohnson: you stripped the signature added the zero and signed it again with snakeoil? | 20:17 |
| pedronis | (just making sure= | 20:18 |
| ijohnson | ... no? I just added 0's | 20:18 |
| pedronis | it's signed | 20:18 |
| ijohnson | I guess I didn't realize I needed to resign grub | 20:18 |
| ijohnson | ah | 20:18 |
| ijohnson | does shim verify the signature of grub too? | 20:18 |
| * cmatsuoka afk for a few minutes to replace a faulty network card | 20:18 | |
| ijohnson | do I need to modify shim at all? I already resigned shim with snakeoil | 20:18 |
| pedronis | ijohnson: yes | 20:19 |
| pedronis | but if you modify grub, you need to sign with snakeoil too | 20:19 |
| pedronis | I mean strip signature and sign with snakeoil | 20:19 |
| pedronis | the same thing we do with shim | 20:19 |
| ijohnson | I guess the question is, do I need to modify shim beyond just strip signature from shim and resign shim with snakeoil | 20:19 |
| ijohnson | ack ok I will try that then | 20:19 |
| pedronis | ijohnson: no, that's all you need to do, is the same princicple by which we can then sign kernel with snakeoil | 20:20 |
| pedronis | so far we didn't need to sign grub again | 20:20 |
| ijohnson | ok | 20:20 |
| pedronis | but if we change it we have to | 20:20 |
| pedronis | ijohnson: for all I know changing what signs it might enough of a change | 20:21 |
| pedronis | but not sure | 20:21 |
| ijohnson | does the tpm resealing use the full hash of the file? or maybe it strips the signature and hashes the actual binary content minus the signature | 20:22 |
| pedronis | ijohnson: the latter, but it happens only on cmatsuoka PR | 20:22 |
| pedronis | before it only matters what are the ca | 20:22 |
| pedronis | certificates | 20:22 |
| ijohnson | right | 20:23 |
| pedronis | but as I said it might be that signing grub changes that, we would have to look at tcg log | 20:23 |
| ijohnson | alright I'm at least able to boot if I resign grub with snakeoil, I'm just gonna wait to see if install finishes with grub resigned with snakeoil too just to be extra sure | 20:25 |
| ijohnson | then I'll try my resigned with snakeoil grub that is padded with a 0 | 20:25 |
| mvo | added a comment and marked as blocked | 20:26 |
| mvo | ijohnson: feel free to remove blocked once your changes are there :) | 20:26 |
| ijohnson | sure | 20:26 |
| mvo | ta | 20:27 |
| ijohnson | mmm this seems not good | 20:30 |
| ijohnson | stateengine.go:150: state ensure error: devicemgr: cannot mark boot successful: cannot compose run mode boot chains: cannot find expected boot asset bootx64.efi in modeenv | 20:31 |
| pedronis | that is weird | 20:33 |
| ijohnson | this is with an edge snapd | 20:33 |
| ijohnson | but with rebuilt kernel and gadget snaps so maybe I broke something | 20:34 |
| ijohnson | let me retry this boot | 20:34 |
| pedronis | ijohnson: mvo: the kernel test has the same problem btw, we are reinstalling the same, we should at least unpack it and add a file to the initramfs or something. anyway doing that is much more typical , we do it already | 20:35 |
| mvo | ok | 20:35 |
| ijohnson | mmm good point | 20:36 |
| ijohnson | yeah this is what I expected to be needed eventually, I can also work on preparing that kind of change too | 20:36 |
| pedronis | it's probably more important than the gadget | 20:36 |
| ijohnson | I already do something similar in the kernel-failover test to make the initramfs panic | 20:36 |
| pedronis | right now | 20:36 |
| pedronis | yea, that covers our case a bit but given it fails to boot, we don't have e2e reseal story with the failover one | 20:37 |
| pedronis | we just reseal back to the one kernel from before | 20:37 |
| ijohnson | mmm I can still reproduce this issue with cannot find expected boot asset in modeenv | 20:37 |
| pedronis | that is weird, is install failing to write the right bit in modeenv | 20:38 |
| ijohnson | pedronis: if it's okay I would like to keep trying to understand this failure, because it just started today | 20:38 |
| pedronis | yes | 20:38 |
| pedronis | we are installing and booting in spread though | 20:39 |
| ijohnson | right | 20:39 |
| pedronis | so seems something misaligned | 20:39 |
| pedronis | or we have a silly bug that spread doesn't exercise | 20:39 |
| ijohnson | now I'm trying just a fresh boot with all edge snaps without rebuilding anything, so all ms key signatures | 20:40 |
| ijohnson | pedronis: we should probably have a spread test too which just tries to boot uc20 with all edge snaps, but with snapd from spread | 20:40 |
| pedronis | btw the gadget-reseal test failed on that PR | 20:41 |
| pedronis | weird, stat: cannot stat '/run/mnt/ubuntu-seed/device/fde/ubuntu-data.sealed-key' | 20:41 |
| ijohnson | alright I reproduced this install failure with all edge snaps | 20:42 |
| ijohnson | so we have a real problem somewhere | 20:42 |
| pedronis | snapd from edge as well? | 20:44 |
| ijohnson | pedronis: yes | 20:44 |
| ijohnson | pedronis: here's a full log and all the snaps I used to build the image: https://pastebin.ubuntu.com/p/wgMYSfKZjv/ | 20:44 |
| * ijohnson needs to step out for a little bit, will return to this in 10ish minutes | 20:44 | |
| pedronis | ijohnson: I suspect kernel doesn't have yet the right bits? | 20:45 |
| pedronis | I mean snap-boostrap dropping values when it deals with modeenv ? | 20:45 |
| ijohnson | pedronis: ah I bet you are right | 20:46 |
| mvo | kernel gets the initrd bits from the beta channel so that would be 2.46 until we release 2.47~rc1 | 20:46 |
| ijohnson | pedronis: I will verify that theory when I get back | 20:46 |
| mvo | thanks, I need to drop, will read standup doc in the morning or mattermost/tg | 20:46 |
| * mvo waves | 20:46 | |
| ijohnson | ok back now | 21:13 |
| zyga | ijohnson how is sealing going? | 21:15 |
| ijohnson | We seem to have regressions with the set of snaps on edge | 21:15 |
| ijohnson | Presumably because something with resealing is broken with the old snap-bootstrap in the initramfs | 21:16 |
| zyga | so no 2.47? | 21:16 |
| ijohnson | not tonight at least | 21:17 |
| ijohnson | unclear if this is a blocker or not | 21:17 |
| ijohnson | I mean obviously it's a blocker that edge images can't boot | 21:17 |
| ijohnson | err can't seed/isntall | 21:17 |
| zyga | right | 21:18 |
| zyga | oh well | 21:18 |
| zyga | I think everyone working on this is really tired | 21:18 |
| zyga | so no surprise something doesn't work sometimes | 21:18 |
| ijohnson | yeah I think we just missed it due to a lack of testing for this scenario specifically, with edge kernel snap instead of rebuilt kernel snap | 21:19 |
| zyga | ah, we always rebuild the kernel for compatibility? | 21:19 |
| ijohnson | yes in spread tests, we rebuild the initramfs to have the new snap-bootstrap bits | 21:20 |
| zyga | drat I need to go to the office | 21:20 |
| zyga | (I'm coding from bed) | 21:20 |
| zyga | anyway, time to sleep | 21:20 |
| ijohnson | pedronis: yes so rebuilding with snap-bootstrap from the edge snapd works to boot an edge uc20 image | 21:20 |
| zyga | I'll finish this in the morning | 21:20 |
| ijohnson | bye zyga! | 21:20 |
| zyga | good luck guys | 21:20 |
| ijohnson | have a good evening | 21:20 |
| zyga | you too :) | 21:20 |
| ijohnson | :-) | 21:21 |
| ijohnson | pedronis: I'm looking into what bits from snap-bootstrap might be effected here | 21:21 |
| pedronis | ijohnson: it the modeenv manipulations I suspect, they don't preserve fields they don't know about | 21:22 |
| ijohnson | yes that's my suspicions as well | 21:22 |
| ijohnson | currently looking at InitramfsRunModeSelectSnapsToMount | 21:22 |
| ijohnson | but what I'm confused about is that nothing should have changed to require a rewrite of the modeenv on first boot of run mode | 21:23 |
| pedronis | ijohnson: do we have base set as try for some reason? | 21:30 |
| ijohnson | pedronis: I don't think so | 21:30 |
| ijohnson | but I'm trying to get a peek at the modeenv as written by install mode before run mode snapd runs | 21:30 |
| ijohnson | because I think part of what happens is that we can't run and so snapd uninstalls things | 21:31 |
| ijohnson | pedronis: no, the modeenv is not rewritten from the initramfs during first boot | 21:39 |
| ijohnson | pedronis: after snap-bootstrap exits, I still see the same modeenv with current_trusted_boot_assets, etc. in it | 21:40 |
| ijohnson | so it must be something else | 21:40 |
| pedronis | this very weird | 21:40 |
| ijohnson | oh wait that was with the new snapd | 21:40 |
| ijohnson | sorry let me re-run that test | 21:40 |
| pedronis | heh | 21:40 |
| === mwhudson_ is now known as mwhudso | ||
| === mwhudso is now known as mwhudson | ||
| ijohnson | pedronis: ok so after the snap-bootstrap in the kernel snap runs, the modeenv does _not_ have the keys in it | 21:43 |
| ijohnson | but try_base is not set | 21:43 |
| ijohnson | so it's a bit unclear to me why we would rewrite the modeenv | 21:43 |
| ijohnson | but rewriting the modeenv seems to be the issue here I think | 21:43 |
| pedronis | ijohnson: I agree, seems a bug? | 21:44 |
| ijohnson | yes probably a bug | 21:44 |
| pedronis | anyway we need to teach modeenv to behave correctly | 21:44 |
| ijohnson | that too | 21:44 |
| pedronis | but would still be good to understand the bug | 21:44 |
| pedronis | you might have to look at the code in 2.46 though | 21:44 |
| pedronis | because things have changed since | 21:44 |
| ijohnson | ah yes very good point | 21:44 |
| ijohnson | actually wait | 21:44 |
| ijohnson | no we need to look at 2.45.2 (?) code | 21:45 |
| ijohnson | I don't think that ubuntu-core-initramfs was ever rev'd | 21:45 |
| pedronis | oh | 21:45 |
| ijohnson | actually I don't even know what version of snapd snap-bootstrap is in the kernel snap right now | 21:45 |
| * ijohnson goes to go digging | 21:45 | |
| ijohnson | so ubuntu-core-initramfs is at version 36, uploaded on may 15th | 21:46 |
| ijohnson | so snapd 2.45+20.04 is what's in the initramfs right now | 21:47 |
| pedronis | ijohnson: it was writing to modeenv incondtionally | 21:50 |
| ijohnson | pedronis: yeah so we always write the modeenv from the initramfs in 2.45 | 21:50 |
| ijohnson | ah yes | 21:50 |
| ijohnson | we found the same code :-) | 21:50 |
| ijohnson | hmm, I'm not sure what to do | 21:50 |
| ijohnson | maybe we need to re-release snapd snap to the previous one, and then get xnox to re-build the initramfs asap and release a new kernel snap that doesn't unconditionally write the modeenv | 21:51 |
| ijohnson | pedronis: thoughts ? | 21:51 |
| pedronis | ijohnson: how is the code in 2.46 ? | 21:53 |
| ijohnson | pedronis: let me look quickly | 21:53 |
| ijohnson | pedronis: 2.46.1 does not always write modeenv | 21:54 |
| ijohnson | pedronis: let me quickly check that 2.46.1 snap-bootstrap boots properly | 21:54 |
| ijohnson | pedronis: 2.46.1 snap-bootstrap works | 22:00 |
| pedronis | ok, notice that once we switch to v1 for sealed keys we'll have a bigger problem | 22:01 |
| ijohnson | pedronis: how so ? | 22:01 |
| ijohnson | oh | 22:01 |
| ijohnson | I see | 22:01 |
| ijohnson | yeah that will be a problem | 22:01 |
| pedronis | the secboot inside old kernel won't know what to do with it | 22:02 |
| ijohnson | I think the right thing to do is to get new initramfs re-spun as soon as possible | 22:02 |
| ijohnson | regardless of what we do with snapd on edge right now | 22:02 |
| pedronis | anyway we need a fix in master to teach modeenv to keep and write back any fields it doesn't use/know | 22:04 |
| ijohnson | pedronis: yes I'm working on that right now | 22:04 |
| ijohnson | pedronis: https://github.com/snapcore/snapd/pull/9358 | 22:20 |
| mup | PR #9358: boot/modeenv: track unknown keys in Read and put back into modeenv during Write <UC20> <âš Critical> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9358> | 22:20 |
| ijohnson | mmm maybe I should add a DeepEquals test for that too | 22:21 |
| * ijohnson goes to write one | 22:21 | |
| mup | PR snapd#9358 opened: boot/modeenv: track unknown keys in Read and put back into modeenv during Write <UC20> <âš Critical> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9358> | 22:22 |
| ijohnson | ok, done | 22:26 |
| ijohnson | pedronis: I'm gonna eod now, but I will be semi-around for reviews or to land things if needed, otherwise I will be working early tomorrow | 22:27 |
| === ahayzen_ is now known as ahayzen | ||
| pedronis | cmatsuoka: are you still around? | 23:06 |
| cmatsuoka | pedronis: yes | 23:06 |
| pedronis | I just landed my PR, could you merge master into #9277 ? | 23:06 |
| mup | PR #9277: secboot: add boot manager profile to pcr protection profile <Run nested> <UC20> <â›” Blocked> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9277> | 23:06 |
| cmatsuoka | pedronis: sure, doing it now | 23:07 |
| pedronis | thx | 23:07 |
| mup | PR snapd#9355 closed: boot: with unasserted kernels reseal if there's a hint modeenv changed <Run nested> <Skip spread> <UC20> <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9355> | 23:08 |
| mup | PR snapcraft#3288 opened: cmake v2 plugin: add help for cmake generators <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/3288> | 23:13 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!