[05:00] morning [06:27] Gry [06:27] Hey [06:30] zyga: hey [06:31] zyga: can you look at #5421 there's something fishy there [06:31] PR #5421: tests: replace MATCH by grep to check users created for ubuntu core [06:31] Yeah, in a moment. Barely waking up here [06:32] We found a curious bug last night [06:42] zyga: what curious bug? [06:42] this one? [06:42] First coffee [06:42] I stopped working after 10PM again [06:43] zyga: good morning! [06:43] zyga: how can I help :) [06:44] mvo: hey [06:44] Hey :-) [06:44] hey mborzecki [06:44] All good just need to wake fully [06:44] And we have a small patch to unbreak snappy in lxd [06:45] I will send it in 20 minutes, just need to clean the kitchen to make the coffee [06:45] coffee, splendid idea [06:46] 5419 is probably an easy win, has one review already and looks pretty straightforward [06:46] zyga: btw. that's my kde bug that caused no sound in hangouts: https://www.mail-archive.com/kde-bugs-dist@kde.org/msg249018.html [06:49] pstolowski|afk: good morning! when you have a chance, please check 5415, zyga has one suggestion there about the comment otherwise it is good to go (has two +1 etc) [06:50] re [06:50] ok, finally at the keyboard [06:50] mvo: looking [06:51] mvo: would you mind reviewing the apparmor change I sent a few days ago? [06:51] I'll get a 2nd review from jamie [06:52] zyga: sure, I'm just going over the list sequentially [06:52] mvo: going back to front helps in making sure we don't starve old PRs [06:54] 5409 is probably a super simple review btw [06:54] zyga: yeah, good point [06:56] PR snapd#5404 closed: i18n: add canary checking to pot file extraction [06:57] PR snapd#5419 closed: many: switch to account validation: unproven|verified [06:58] pedronis: https://github.com/snapcore/snapd/pull/5419#discussion_r198725202 (when you are around) [06:58] PR #5419: many: switch to account validation: unproven|verified [06:59] mvo: ^ [07:00] zyga: ? [07:01] of course we need to signe the new assertion (which means using the root key, so probably will not happen until the september sprint) [07:01] perhaps I'm reading the patch wrong [07:02] when is assembleAccount called? [07:02] many places [07:02] but yes you are reading the patch wrong [07:03] is just changing what the accessors see [07:03] ah, that's good then [07:03] we sign a text, not a struct [07:03] yeah, I know that, [07:04] I was worried that this is changing the data to be assembled into the text blob [07:04] ? [07:04] it changes only what comes out of the new Accessor [07:04] Validation [07:04] ok [07:05] as the comment tries to explain is really about preexisting stuff [07:05] (until we can rev them) [07:06] PR snapd#5400 closed: spread.yaml: enable main and regression suite on core18 systems [07:06] mborzecki: commented on 5421 [07:07] I'm -1 on that [07:07] PR snapd#5398 closed: tests: disable auto-refresh test on core18 [07:12] PR snapd#5322 closed: cmd/snap-confine: include CUDA runtime libraries [07:16] zyga: yeah, looks fishy, idk maybe something with the storage again [07:16] zyga: if you look at the pastebin, the output that's logged there even matches [07:17] mborzecki: yeah [07:17] it is odd [07:18] especially that this is a file [07:18] hmm let me restart that travis job a couple of times [07:19] I'll play with MATCH [07:19] but first [07:19] fix the lxd bug [07:19] then finish coffee because I'm still fuzzy [07:19] then MATCH [07:19] fuzzy MATCH(ing) === pstolowski|afk is now known as pstolowski [07:20] morning [07:20] pstolowski: hey [07:20] hehe [07:20] I'm very fuzzy matching today [07:21] on the upside I managed to convince the kids to help a little [07:21] zyga: help coding? good job man! [07:21] haha [07:21] I wish :D [07:22] zyga: no playing with the MATCH bug please, lets first attack the two core18 issues [07:23] mvo: that's fair [07:23] mborzecki: all yours [07:23] mvo: the lxd issue is tiny so I'll do that first if you don't mind [07:23] then back to core18 [07:24] zyga: sure, I don't want to boss you around :) [07:24] zyga: just a little ;) [07:32] PR snapd#5230 closed: interfaces/udisks2: also implement implicit classic slot [07:34] PR snapd#5423 opened: cmd/snap-confine: fix nvidia support under lxd [07:36] mborzecki: this is something for you https://forum.snapcraft.io/t/lxd-snaps-nvidia/6134 :) [07:36] mvo: can we remove 2.33 milestone now [07:37] I don't know how to now that issues don't show up on github [07:41] zyga: i'll check it before the standup, 18.04 host is good enough? [07:41] yes [07:41] ok [07:41] mborzecki: but must use nvidia drivers and you need to perform the test in a container [07:42] I would recommend that you perform a baseline test first [07:42] to see the denial [07:42] and then use patched snap-confine [07:42] now got to find that usb with mu 18.04 install [07:47] yay found it :) [07:52] https://forum.snapcraft.io/t/search-by-common-id/6136 this involves some store work too? [08:07] zyga: with https://github.com/mvo5/snappy/tree/core18-all-fixes-all-tests we are down to only a few failure: http://paste.ubuntu.com/p/FsTMHB6fFs/ - this is why I'm so keen to fix the snap-disconnect (i.e. route core: to system) and the core mount namespace [08:08] mvo: I will have the latter done in ~ hour [08:08] so I think today we may see all green on that PR [08:09] zyga: the stop mode issue is still a bit mysterious, I think its a race but I don't know how to fix it yet [08:09] zyga: but yeah, would be awesome to get it to green [08:20] PR snapd#5326 closed: api/snapctl: allow -h and --help for regular users [08:25] google:ubuntu-core-16-64:tests/main/snapd-notify seems to be failing overly frequently [08:32] this test looks weird, any idea what it's about? all it's doing is systemctl status snapd.service [08:33] moin moin [08:34] hands up if you just messed up the last page of the paperwork and you need to take a break before reprinting it [08:34] * Chipaca finally done except for this bit that's like a checksum of all the above [08:34] then i print that and i'm off to the postoffice and then i'm DONE [08:40] * Chipaca gets back to it [08:41] hm i'll rework tests/main/snapd-notify to use systemctl show properties rather than looking for a debug message [08:41] mborzecki: my gut feeling is, logging failure [08:42] zyga: yeah, that's why looking at ExecMainStartTimestampMonotonic and ActiveEnterTimestampMonotonic makes more sense as it comes directly form systemd [08:48] pedronis: re the wait-for-auto-connect review - you ask about s.overlord.Settle() there. what is your suggestions? I was thinking about using s.overlord.SnapManager().Ensure() but that does not mix well with the Settles() we use in the tests elsewhere. what is your suggestion here? [08:52] mmh [08:53] mvo: my problem is not so much Settle but the loop etc, why doesn't the old code don't work anymore? [08:54] pedronis: settle never converges because auto-connect waits for the restart [08:54] pedronis: so the change is never "done" [08:54] ? [08:55] let me check [08:55] something is different then [08:55] pedronis: you mean settle should work? [08:55] yes [08:55] pedronis: even if the change does not go into done state? [08:55] pedronis: aha, ok. sorry, I was not aware of this, let me look [08:55] settle doesn't check for done [08:55] it checks for ensure scheduling [08:55] pedronis: don't worry in this case, I can dig into this, I had the wrong assumption [08:57] mvo: btw, did we kill this test: https://github.com/snapcore/snapd/blob/release/2.32/overlord/managers_test.go#L969 ? [08:57] we should maybe bring it back [08:58] no, still there [08:58] wondering what it does right now on master though [08:58] * mvo nods [09:06] pedronis: aha, so I think the issue is that doAutoConnect returns retry errors. and this means that the taskrunner will schedule runing this task in the future and this is what settle looks at (are there tasks scheduled to run pending). does that make sense? [09:09] * Son_Goku groans to life [09:12] pedronis: the TestInstallCoreSnapUpdatesBootloaderAndSplitsAcrossRestart works because the configure hook of core waits for a restart [09:12] pedronis: I can make the test smarter and ensure that the wait happens in the right place (at the auto-connect task) [09:17] I guess it's more accurate that I was startled awake by thunderstorm... [09:17] mvo: no, I'm still not understand, we had tests like that before [09:21] mvo: remember the old code in setup-profiles was also like that [09:22] pedronis: ok, let me dig some more then [09:22] pedronis: I made the other test more targeted fwiw [09:28] mvo: anyway, maybe we are talking past each other, I don't know how you are changing the other test [09:30] pedronis: I think you are spot on actually. I think the difference is that in the old code we did not restart, we had a check that if the same core is installed as the core that was booted we ignore the restart [09:31] pedronis: for core18 this is different right now (we can discuss if that is good or bad :) [09:31] PR snapd#5424 opened: tests/main/snapd-notify: use systemd's service properties rater than the journal [09:31] mvo: it's the snapd always restarts stuff? [09:32] pedronis: its different because right now snapd in seeding mode is not fully installed but after it seeded itself it is fully installed, so it seems to make sense to restart into the fully installed one at this point [09:32] pedronis: yeah, its the snapd that triggeres the restart [09:32] but the other test in managers_test [09:32] pedronis: the core18 will not trigger a restart as we use the same check if the revision is the same we booted we don't restart [09:32] also has a restart [09:32] but Settle is enough [09:33] pedronis: indeed, let me look what is going on there [09:35] * Chipaca returns, triumphant [09:36] * Chipaca looks up the spelling of "pyrrhic" [09:43] pedronis: still digging but yeah, something is different, its strange the old and the new code use the same retry but in the old one it does converge not in the new [09:45] pedronis: also strange - in both the new and old world the TestInstallCoreSnapUpdatesBootloaderAndSplitsAcrossRestart test works with settle [09:50] pedronis: aha! found it - so the difference is that the old code only waited for "core" with setup-profiles. but the new code waits for all snaps that are in auto-connect to restart [09:51] mvo: yes, but they are done in order [09:51] I'm still missing something [09:51] mvo: the first settle should stop after snapd or core [09:51] then you set the system as restarted and continue, no? [09:51] what am I missing [09:52] pedronis: maybe something in the ordering is wrong, let me check [09:52] pedronis: I see that both snapd and core18 are in doing state for auto-connect - which should not be the case, right? [09:52] yes [09:52] that seems wrong [09:52] something bad with the new snapd code [09:52] pedronis: thanks, let me re-read the wait code [09:52] pedronis: yeah, probably a missing WaitAll() in the new code [09:53] we should install in order snapd core18 kernel gadget other snaps [09:53] all sequentially [09:53] pedronis: indeed [09:53] mvo: good I insisted because seems there's a real issue [09:53] pedronis: yes! [09:54] pedronis: this also explains why the other test is fine [09:54] pedronis: its probably a bug in the firstboot code only [09:56] PR core18#43 opened: Remove manpages and other documentation, cleanup empty doc dirs [09:59] #5242 is a trivial review if anyone's interested [09:59] PR #5242: tests: new test for joystick interface [10:00] duh, wrong number [10:00] #5424 is the trivial one [10:00] PR #5424: tests/main/snapd-notify: use systemd's service properties rater than the journal [10:11] pedronis: I pushed an update [10:13] ah [10:14] mvo: so, good, that was bad [10:14] pedronis: yes, thanks for the persitance in getting to the bottom of it [10:14] pedronis: there was also the unit test for the task ordering missing :( [10:20] PR snapd#5415 closed: interfaces: moved normalize method to interfaces/utils and made it public [10:22] mvo: thx, some more small comments there but looks good overall [10:31] * Chipaca reading https://forum.snapcraft.io/t/5685 very slowly [10:32] * zyga hands Chipaca a copy of for extra fun https://kernel.org/pub/linux/kernel/people/paulmck/perfbook/perfbook-1c.2017.11.22a.pdf [10:33] * Chipaca hands zyga the original documentation for tc [10:33] * Chipaca notes it was only found in russian at first [10:33] tc? [10:35] zyga: traffic control [10:36] PR snapd#5417 closed: image: block installation of parallel snap instances [10:39] anyone willing to take a look at #5389 ? [10:39] PR #5389: snap: account for parallel installs in wrappers, place info and tests [10:39] mborzecki: bring it on [10:47] Chipaca: thanks! [10:54] hmm WatchdogTimestampMonotonic=0 on 14.04 with systemd 204 [10:54] pedronis: thank you! [11:20] pedronis: I updated the pr again but no rpush with a review, need to have lunch now :) [11:20] mborzecki: after #5389 will there be still nore changes in the snap package itself? [11:20] PR #5389: snap: account for parallel installs in wrappers, place info and tests [11:21] s/nore/more/ [11:22] pedronis: there'll probably be more, the orignal branches introduced some extra helpers and renames, but those can come in later when needed [11:26] zyga: libzypp has download.max_silent_tries to auto retry downloads, i'm checking this now, if it doesn't break things i'll open a PR [11:27] oooh [11:27] yes please [11:27] that's a fantastic feature [11:27] (no pressure there apt/dnf) [11:35] PR snapd#5414 closed: corecfg: added experimental.hotplug feature flag === pstolowski is now known as pstolowski|lunch [11:42] PR snapd#5425 opened: spread: increase the number of auto retries for package downloads in opensuse [11:43] zyga: ^^ [11:43] zyga: also if you could take a look at 5424 please [11:45] done [11:46] the tests are super frustrating today [12:00] PR snapd#5426 opened: client, cmd/snap: pass snap instance name when installing from file [12:00] carved out another piece from parallel installs branch ^^ [12:02] mvo: ha [12:02] mvo: did you think about this case: core18 system, snapd.snap, core.snap, you run an app that uses core.snap as base; which snap-confine is used? which snapctl is visible? [12:03] cleaning up snap-confine made me realise this is a bit weird now [12:04] snapctl comes from where snap-confine comes [12:05] mmm, well, it should [12:05] but those are arranged differently [12:05] snap-confine is selected by snapd on the outside [12:05] snapctl is selected by snap-confine [12:05] yes [12:05] and the logic is not the same [12:05] how can it be [12:05] snapctl is picked relative to snap-confine [12:06] switching to ubuntu to check the lxd thing [12:06] mmm? there's a bit of code in snap-confine that, if you are not using "core" as a base, does extra bind mounts [12:07] this handles snap-confine but snapctl is in another place [12:07] * zyga looks at what happens [12:07] I think it's something to tidy [12:09] in that case uses_base_snap is true, no ? you said core18 and snapd present [12:09] yes but that doesn't handle /usr/bin/snapctl as far as I can see [12:10] it just does nothing for it [12:10] there's a chunk of commented-out code there [12:11] mmh, I thought snapctl was in /usr/lib/snapd [12:11] that is my confusion [12:11] we have code for that [12:11] yeah, it'd be easier [12:11] https://github.com/snapcore/snapd/blob/master/cmd/snap-confine/mount-support.c#L360 [12:11] could we ship a symlink /usr/bin/snapctl -> $libexecdir/snapd/snapctl [12:12] yeah, I'm looking at the exact same code now [12:12] anyway, I will add this to the core18 todo list for today [12:14] sounds something to discuss with mvo [12:14] agreed [12:25] zyga: which snap-cofine/snapctl - the one from core18. I think we want to ensure the tooling is in sync with snapd [12:26] yes [12:26] mvo: let's HO if you want to chat quickly ahead of the standup [12:26] zyga: sure [12:26] zyga: I have a PR that moves snapctl into /usr/lib/snapd :) [12:26] oh, that's great [12:26] thank you for doing that [12:26] zyga: one min, just preparing a cup of tea [12:26] that's going to be a godsend [12:27] mvo: I had a crazy idea just now [12:27] snapd.deb that ships just snapd.snap + entrypoint [12:27] zyga: yeah, its a fun idea to explore [12:32] mvo: ready when you are [12:36] zyga: lxd from snaps too? [12:36] yes [12:58] mvo: looking at 5392 seems I found an unrelated bug, left a comment there === pstolowski|lunch is now known as pstolowski [13:02] pedronis: thanks, I work on this [13:09] zyga: dog slow container rootfs download, ~300kBps [13:24] zyga: replace snap-confine inside the container right? [13:24] yes [13:24] and disable reexec [13:24] and this will fail but that's fine, it will need an apparmor tweak that we can do as we see the denial [13:25] ok [13:30] btw, current master https://paste.ubuntu.com/p/7crJcmKcH7/ [13:31] I saw that too, I think this is timing/racy test [13:38] zyga: https://paste.ubuntu.com/p/Q855YBK6nm/ [13:39] that's perfect, thanks [13:39] can you change the apparmor profile for snap-confine [13:39] I will tell you where [13:39] hold on [13:39] on line 269 [13:39] there are a few "remount ro" things [13:39] change "remount ro bind" there [13:40] reload and try [13:40] thank you! [13:41] yup trying it already [13:41] zyga: i've edited snap-confine.real [13:41] that's good [13:46] zyga: https://paste.ubuntu.com/p/n9536fST8D/ hmm i should probably update lxd profile too [13:48] no, not the lxd profile [13:48] mborzecki-ubuntu: what's the profile you have now? [13:48] as a sanity check [13:48] copy paste the three lines [13:49] and have a variant with and without remount [13:49] in case the pattern requires all flags to be present [13:49] but hmm [13:57] mvo, hmm, does the pi-config stuff of snpd check the existence of values in config.txt at all ? [13:58] ogra@pi2:~$ snap set core pi-config.gpu-mem-512=true [13:58] ogra@pi2:~$ snap set core pi-config.gpu-mem-512=true [13:58] ogra@pi2:~$ snap set core pi-config.gpu-mem-512=true [13:58] ogra@pi2:~$ tail -10 /boot/uboot/config.txt [13:58] device_tree_address=0x02000000 [13:58] core_freq=250 [13:58] dtoverlay=vc4-kms-v3d [13:58] gpu_mem_512=true [13:58] gpu_mem_512=true [13:58] gpu_mem_512=true [13:58] gpu_mem_512=true [13:58] gpu_mem_512=true [13:58] gpu_mem_512=true [13:58] seems it completly blindly appends lines to it [13:58] mborzecki-ubuntu: let me know what you get and I'll be right back [13:59] (not sure if thats any different in stable, that pi runs edge atm) [13:59] uh ... even worse: [14:00] ogra@pi2:~$ snap set core pi-config.gpu-mem-512=false [14:00] ogra@pi2:~$ tail -10 /boot/uboot/config.txt [14:00] core_freq=250 [14:00] dtoverlay=vc4-kms-v3d [14:00] gpu_mem_512=true [14:00] gpu_mem_512=true [14:00] gpu_mem_512=true [14:00] gpu_mem_512=true [14:00] gpu_mem_512=true [14:00] gpu_mem_512=true [14:00] gpu_mem_512=false [14:00] it doesnt toggle the existing value at all but just appends and append for each snap set call [14:00] zyga: hm actually one denial is gone, if you look at the first log, there were to, one seemd like it's inside the container's namespace (?) [14:01] what's the current denial you see [14:02] ogra_: uhhh [14:02] yeah [14:02] ogra_: let me check [14:03] zyga: https://paste.ubuntu.com/p/n9536fST8D/ look below '<--- profile replaced --->' line [14:04] and the app fails to start, correct? [14:04] zyga: yes [14:04] do you have the vanilla profile somewhere, can you diff before/after [14:04] (and did you reload the profile (just checking)) [14:05] ogra_: hm,hm,we have tests that should ensure that things are not blindly appended but obviously reality disagrees, lets me find out what is going on [14:05] zyga: apparmor_parser -r /etc/apparmor.d/usr.lib.snapd.snap-confine.real [14:07] the file inherit parts are harmless, I think [14:07] only the last one is the problem [14:08] zyga: http://paste.ubuntu.com/p/KwgWCRgskx/ [14:09] zyga: tha's inside the container [14:09] mvo, thanks, need a bug ? [14:09] sorry my input froze [14:10] mborzecki-ubuntu: the denial now is from lxd indeed [14:10]