/srv/irclogs.ubuntu.com/2020/07/31/#snappy.txt

=== benfrancis9 is now known as benfrancis
mupPR snapd#9083 opened: tests: new parameters nested uc20 <Run nested> <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9083>01:48
mborzeckimorning05:10
pedronismborzecki: hi06:53
mborzeckipedronis: hey06:53
pedronismborzecki: should I force merge 9082 ?06:54
mborzeckipedronis: yes, i looked at the failures and they are unrelated06:55
pedronisok06:55
pedronismborzecki: anything else that requires my immediate attention?06:58
mupPR snapd#9082 closed: interfaces/system-key: in WriteSystemKey during tests, don't call ParserFeatures <Bug> <Simple 😃> <Test Robustness> <Created by anonymouse64> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9082>06:59
mborzeckipedronis: no i think that's it for now, there's some more review i'll be doing and probably need a 2nd reviewer too06:59
pedronismborzecki: I commented in #9080  and I think #9079 needs a review07:04
mupPR #9080: osutil/disks: use xerrors to indicate a fs label wasn't found <Simple 😃> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9080>07:04
mupPR #9079: gadget/install: retrieve command lines from bootloader <Simple 😃> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9079>07:04
pedronismborzecki: did you see my comment/question in #9078?07:04
mupPR #9078: [RFC] boot: fancy marshaller for modeenv values <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9078>07:04
mborzeckipedronis: yes, i've finished the change and ii'll be pushing that in a couple of minutes07:05
pedronisok07:05
mborzeckipedronis: goconfigparser is something we keep bundled like other deps right?07:35
pedronismborzecki: in which sense?07:36
mborzeckipedronis: ah ok, it's vendored like everything else, for minute there i thought we were moving it around in the tree07:37
mborzeckipedronis: anywyas, the trouble it we need to land a fix in goconfigparser for json to work07:38
pedronis:/07:38
pedroniswe package it at least for debian afaict: golang-github-mvo5-goconfigparser-dev07:38
mborzeckiit's confused by [] in lists07:38
pedronisah, because the section stuff07:39
pedronisdoesn't have ^$ ?07:39
pedronisI was wondering about that, also not sure why it's like that07:39
mborzeckioerheks: yes, regexp.MustCompile(`\[(?P<header>[^]]+)\]`), but should be regexp.MustCompile(`^\[(?P<header>[^]]+)\]`)07:39
mborzeckipedronis: ^^07:40
pedronismborzecki: fedora has:  Provides:      bundled(golang(github.com/mvo5/goconfigparser))07:43
pedronisnot sure what that means07:43
mborzeckipedronis: i've updated #9078 and opened https://github.com/mvo5/goconfigparser/pull/607:54
mupPR #9078: [RFC] boot: fancy marshaller for modeenv values <UC20> <â›” Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9078>07:54
mupPR mvo5/goconfigparser#6: configparser: fix section header regex to not interfere with JSON data <Created by bboozzoo> <https://github.com/mvo5/goconfigparser/pull/6>07:54
mborzeckiEighth_Doctor: hey, any clue what changed in rawhide recently that could break building go binaries statically? the usual set of flags does not build a static bin anymore, heck even -ldflags=-extldflags=-static does not11:53
Eighth_Doctorugh11:54
Eighth_DoctorGo 1.15 landed11:54
Eighth_Doctoryou probably should file a bug report about this11:54
mborzeckiEighth_Doctor: hm -buildmode pie used to trigger external linker, but it does not anymore, i need to explicitly pass -linkmode external to get what i want11:56
Eighth_DoctorGo compiler upstream doesn't really have a concept of "stability" here11:58
mborzeckiheh11:58
=== pedronis_ is now known as pedronis
ijohnsonmorning folks12:23
cachioijohnson, hello12:30
ijohnsonhi cachio12:30
cachiogood morning12:30
ijohnsonhey cmatsuoka12:32
cmatsuokaijohnson: hi, good morning12:33
=== benfrancis4 is now known as benfrancis
ijohnsoncachio: something like https://pastebin.ubuntu.com/p/MB9Qh36xkd/ should work for you to run nested jobs via github actions nightly14:12
ijohnsonI didn't filter for all the relevant jobs, so that just runs all nested suites on 20.04, but that should be easy to fix with spead, etc.14:12
ijohnsons/spead/spread/14:12
cachioyes14:15
cachionow I am comparing the times with and without kvm14:15
cachioijohnson, so then I can adjust the workers needed to run when we use the "Run nested" label14:16
ijohnsonright14:16
cachiothanks for the code, I'll add that to the PR that I am creting14:16
cachiocreating14:16
cachioijohnson, what I am testing now is to add more cores when kvm is not used14:18
cachioperhaps that help as well14:18
ijohnsonah true, I remember that bug, yes having more cores should help us I think14:18
ijohnsonmborzecki: pedronis: I sent an invite for next week to brainstorm about bootstate refactorings/improvements before we add resealing to the mix there14:22
mborzeckiijohnson: ack14:22
* cachio lunch14:50
cmatsuokaijohnson: with L1 focal it boots correctly, and then strange things happen15:02
ijohnsoninteresting, strange things as in similar strange things we see on GCE ?15:02
cmatsuokait didn't reboot, but it locked. I wonder if GCE has a watchdog for such cases15:03
cmatsuokaI'll keep it locked to see if it still reboots after some time15:03
cmatsuokain the meantime I'll look for my cell phone, I think lost it15:04
ijohnsoninteresting one thing we don't know is when the machine reboots vs when it hangs from output to the serial file15:04
ijohnsoncachio: it would be good to also add the debug output of the serial log for a nested VM when we fail to prepare, currently the debug-each I have that shows the boot log only runs if the execute section is where we fail15:22
ijohnsoncachio: i.e. from this log, I can't see the serial boot log of the nested VM15:22
ijohnsonhttps://github.com/snapcore/snapd/pull/9083/checks?check_run_id=93025239515:22
mupPR #9083: tests: new parameters nested uc20 <Run nested> <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9083>15:22
cmatsuokaijohnson: it's really freezing at random points during the boot process, and I think it could be possible that GCE is rebooting the machine if it reaches that state15:46
ijohnsoncmatsuoka: ahhh very interesting15:46
cmatsuokamaybe we could display an RTC  timestamp at the very beginning of userspace to measure the time between reboots?15:48
pedronisijohnson: sorry, but I xerrors.Errorf is more meant to stack errors that comes from other packages, than to label own errors but making it nested on the spot, that is a bit odd15:56
pedroniss/but making/by making/15:56
ijohnsonpedronis: sorry I was just trying to do the easy thing with less code but I can make an Error type15:56
pedronisijohnson: yes, but in this case the easy thing is also a bit odd :)15:56
ijohnsonwell the error isn't currently surfaced to the user anywhere and we have much more confusing errors coming from the initramfs so I just shrugged when writing the code15:57
ijohnsonbut it appears that shrugging was not justified15:57
pedronisijohnson: no, in general we should try to improve our errors, I had cmatsuoka remove some nesting on some secboot errors for example15:58
cmatsuokayep, at a certain point it becomes too verbose and not very informative15:59
pedronisanyway when writing a ErrorMatches test it's always a chance to consider if the error is ok, can be improved16:00
ijohnsonSure16:06
cmatsuokaijohnson: watchdog: BUG: soft lockup - CPU#1 stuck for 23s! [systemd-modules:210]16:08
cmatsuokahttps://paste.ubuntu.com/p/9hsS2jHhc8/16:09
pedronisijohnson: I re-reviewed #891716:10
mupPR #8917: osutil/disks: add mock disk and tests for happy path of mock disks <UC20> <â›” Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8917>16:10
cachiossh ijohnson I'll add that16:10
pedronisdo we print the go version we use to build the snapds in the spread tests?16:13
ijohnsoncmatsuoka: ah good find, I was about to say what we could try is some kind of fifo that qemu logs the serial log to, where at the other end of the fifo we prepend the real host (well L1) time stamps for each line that was output to the log16:22
ijohnsoncachio: thanks16:22
cmatsuokaijohnson: adding timestamps is a good idea16:24
ijohnsonpedronis: thanks, I responded to one point about checking the uniqueness of the dev strings, unfortunately we can't enforce that from MockMountPointDisksToPartionMapping16:24
cmatsuokaijohnson: I think ts(1) does exactly that16:25
* ijohnson is always learning about new tools that do exactly what I think of without complicated shell scripts :-)16:25
cmatsuokaubuntu@nested-test:~$ uname | ts16:26
cmatsuokaJul 31 13:25:59 Linux16:26
pedronisijohnson: I see, I made a comment, I'm not sure we want to go there but well disks are actually complicated16:32
ijohnsonpedronis: sorry I didn't understand, where are you not sure we should go there ?16:33
pedronisijohnson: my suggestion in the new comment16:33
ijohnsonlet me look16:33
ijohnsonsorry I still don't understand, how could we have a single MountPoint map to multiple MockDiskMappings ?16:34
ijohnsonwhich one would be used ?16:34
ijohnsonpedronis: ^16:34
pedronisijohnson: sorry, let me stare at this more carefully, there is something odd here for sure16:37
ijohnsoncmatsuoka: mmm for the purposes of debugging spread runs I wonder why we even need a file for the serial log, we could just have qemu have serial go to stdout and then pipe that through ts, and that just goes to journald because we run qemu through systemd-run ?16:37
ijohnsonpedronis: ok I will wait to push any more patches to the mockdisk PR until we are aligned16:38
pedronisijohnson: I see16:40
pedronisijohnson: made this make more sense: https://github.com/snapcore/snapd/pull/8917/files#r46371711616:44
mupPR #8917: osutil/disks: add mock disk and tests for happy path of mock disks <UC20> <â›” Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8917>16:44
pedronisheh, maybe this makes more sense16:45
ijohnsonpedronis: so then you mean to check if the pointers to MockDiskMapping are different, and if the pointers are different that they have unique DevNum strings ?16:46
pedronisyes, though I really probably mean to use map and to take pointers, taking values of MockDiskMapping is odd16:47
pedronisit's not a very valuey struct16:47
pedronisto me16:47
ijohnsoncachio: cmatsuoka: I pushed this: https://github.com/snapcore/snapd/pull/9065/commits/05fe6d8f383396e6a53d2cb70739e4c632b483f4 if it is useful I will submit a separate PR for that16:48
mupPR #9065: debug: forward systemd journald output to serial console when booting for uc20 <Precious Logs> <Run nested> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9065>16:48
ijohnsonpedronis: mmm ok16:48
pedronisijohnson: I mean, go for example will not let you compate two of them16:50
pedronis*compare16:50
pedronisthat makes it non valuey in my book16:50
ijohnsonpedronis: the only reason they are not comparable is because they have a map in them16:51
pedronisijohnson: yes16:51
ijohnsonwhich perhaps is your whole point16:51
pedronispartly yes16:51
ijohnsonmmm alright I can do that then I don't think anything should be broken doing it this way16:53
pedronisijohnson: yes, I don't think it should mess too much with your downstream PRs16:54
pedronismaybe a few more & somewhere16:54
ijohnsonpedronis: ok updated 9080 and 891717:25
=== ijohnson is now known as ijohnson|lunch
pedronisijohnson|lunch: re-reviewed, closer but not quite18:29
=== ijohnson|lunch is now known as ijohnson
ijohnsonyes I saw18:29
* pedronis is also a bit melting here18:31
pedronisijohnson: sorry this is becoming a bit an adventure in the ontology of disks18:33
ijohnsonhaha it's ok18:33
ijohnsonlet's just remember to squash merge everything otherwise we will inevitably have to relive this ontology18:33
pedronisijohnson: +1 on 908018:53
ijohnson\o/18:54
pedronisI changed the summary as well18:54
ijohnsonah yes I suppose that is now out of date18:54
ijohnsons/is/was/18:54
pedronisijohnson: reviewed 8987 as well, one small thing (which I should have mentioned before :/ )18:59
ijohnsonpedronis: you mean 8917 ?19:00
ijohnsonah yes I see19:00
cmatsuokaijohnson, cachio: I think I tweaked the configuration enough to have the nested machine working with ovmf on my local amd, but I'll do some more tests and try to minimize the amount of changed variables19:06
ijohnsonnice!19:07
cachiocmatsuoka, nice19:09
cachioI am still testing with no kvm19:09
cachiois not so bad19:09
cmatsuokacachio: there are two independent things that cause problems here: smp and rng passthrough19:09
cmatsuokaif I disable rng passthrough and set smp to 1, it tends to be much more stable19:10
cachiocmatsuoka, how did you disabled rng passthrough?19:18
cmatsuokacachio: I removed  "-object rng-random,filename=/dev/hwrng,id=rng0 -device virtio-rng-pci,rng=rng0" from the parameter list, but it depends on you having these parameters already or not19:19
cachioin my last pr I added that19:20
cachiobut didnt see much change with or without it19:20
cmatsuokaI had to remove it here, otherwise I get a consistent crash19:20
cachioif I disable kvm the test takes about 8 minutes more to run19:20
cmatsuokaso it seems that from my original set of parameters, if I set -smp 1 and remove rng passthrough it works, but my L0 host is a Ryzen719:22
cmatsuokaI'll try with a generic host to see if it still works19:22
cmatsuokacachio: this is what worked for me: https://paste.ubuntu.com/p/yZhhrFBwBR/19:23
cachioit is pretty similar to what we use on nested on google19:24
cmatsuokayes, I changed more parameters initially then I placed them back to find which ones were actually causing the problem19:25
cachiobu you run kvm19:25
cmatsuokabut it could depend on the version of their L0 host and cpu as well19:25
cachioI am runnign qemu with kvm enalbed19:25
cachioI think it is the same19:25
cmatsuokaand this is L0 focal + L1 focal19:26
cmatsuokait's more likely that they have L0 bionic (+patches maybe) + L1 focal19:26
cachioyes19:26
cachiowho knows19:27
cmatsuoka8 minutes more is not that bad if it's reliable19:27
cachiocmatsuoka, seems to be acceptable19:28
cachioI am running 2 tests together to see the time19:28
ijohnsonyah 8 minutes is acceptable to me19:43
cachioijohnson, cmatsuoka updated #908320:26
mupPR #9083: tests: new parameters nested uc20 <Run nested> <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9083>20:26
cachiowith the no kvm scenario20:26
ijohnsonnice thanks cachio20:30
cachioijohnson, yaw, let see the action results20:31

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!