[02:51] PR snapcraft#2670 closed: Plugin colcon: forward parallel build count [02:54] PR snapcraft#2681 closed: spread: fix unbound variable error [03:03] PR snapcraft#2675 closed: docs: quick init for lxd in HACKING.md [05:09] morning [05:49] morning! so much red :/ has anyone looked for patterns yet? [06:09] mvo: morning, no haven't looked yet, i'm pushing golang-github-snapcore-gettext to fedora, but noticed that it was all red yesterday evening too [06:11] mborzecki: ok, I have a look [06:15] PR snapd#7317 opened: httputil: rework protocol error detection [06:17] looks like fastly is very unhappy 503, protocl_errors etc [06:18] mvo: looked at 3 prs, 3 different errors :/ [06:18] mborzecki: I looked at the test update one from samuele and it was all protocol_errors [06:18] mborzecki: another one a mix of protocol_errors and 503 [06:18] mborzecki: but yes, there is odd stuff in the mix in others too [06:18] mborzecki: looks like the protocol_error detection is buggy :/ [06:19] mvo: perhaps looking at the string was more reliable [06:20] mborzecki: its funny, I think some of the ones failing use the old string detection so its something fishy, maybe the old code was buggy too and we were just lucky :( [06:21] mborzecki: but yeah, pushed a PR with better string based detection and slightly improved debug [06:21] mvo: hm type assertion didn't work then? [06:22] mborzecki: maybe or its something strange with the imports, the bundled golang h2_bundle.go does not have a StreamError [06:22] mborzecki: so it might be some strange interplay between the bunblded http2 and the x/net/http2 [06:23] mborzecki: I can dig some more, its just so time consuming :/ [06:23] mvo: there's http2StreamError, iirc h2_bundle is x/net/http2 impoted and all funcs and types with http2 prefix [06:23] mborzecki: yeah, this is the one my latest PR string matches on. its not an exported error unfortunately [06:23] mvo: //go:generate bundle -o h2_bundle.go -prefix http2 -underscore golang.org/x/net/http2 [06:24] mborzecki: what is strange is that I had a tiny go example client running against a (c) http2 server that would produce stream errors and there I got the right errors [06:25] mborzecki: anyway, I will investigate but it seems worthwhile to unblock ppl [06:25] mborzecki: also 7312 would be good [06:25] mborzecki: I saw some issues where the non-home user could not be deleteted because some process was still in use which I think this PR will fix (--force ftw!) [06:25] mvo: did you import x/net/http2 in that tiny go client? [06:26] mborzecki: yeah, I have to for the error casting [06:27] mborzecki: it is also imported in httputil [06:28] mborzecki: http://paste.ubuntu.com/p/5m7nzf3yNZ/ [06:28] mvo: ok, the docs on https://godoc.org/net/http say we'd need to use ConfigureTransport() or sth [06:28] mborzecki: hm, I thought its doing this automatically [06:28] mvo: perhaps then it would always use the http client from imported lib [06:29] mborzecki: let me try that [06:29] mborzecki: and indeed [06:29] mborzecki: my test code is doing this [06:29] mborzecki: because I need to override the tls config :( [06:30] mborzecki: if we override the default transport will it fall back to http1 automatically? [06:30] mborzecki: also - I wonder if its worth it vs string detection :/ [06:30] mvo: yeah, i'd assume that it falls back autmatically [06:33] mvo: quckly scanning the code, it sets up the handler of upgrading the protocol once h2 is seen in ALPN [06:34] mvo: so your client code would start with http.Transport{} and then call http2.ConigureTransport() on that [06:36] mborzecki: let me play with this [06:39] PR snapd#7307 closed: mkversion.sh: fix version from git checkouts [06:47] PR snapd#7273 closed: gadget, overlord/devicestate: rename Position/Layout [06:55] hi, anyone has an idea on how to find out what's keeping loop devices busy in https://bugs.launchpad.net/snapd/+bug/1841137 ? [06:55] Bug #1841137: /dev/loopX devices left around for removed snap revisions [06:57] Chipaca: hey, good morning, you are around early :) [06:58] hey pedronis ! still lots of reds but some progress in hopefully making things less red [06:58] mvo: forgot to close irc before suspending the notebook yesterday, so it connected as soon as i opened it to catch up with the world over breakfast [06:58] mvo: good morning =( [06:58] er, i meant =( [06:58] wait [06:58] =) [06:58] there [06:58] Chipaca: hahaha [06:58] silly half-asleep hands [06:59] * mvo kicks Chipaca from irc again until he has tea/coffee [06:59] PR snapd#7271 closed: many: generalize assertstate.Batch to asserts.Batch, have assertstate.AddBatch [06:59] * Chipaca disappears [07:05] Chipaca: hey, good morning [07:06] mvo: you're trying out wth ConfigureTransport then? in which case I'll stop going through 7317 ;) [07:07] mborzecki: I'm looking into it but I think its not worth waiting [07:07] mborzecki: I'm also more and more convinced that whatever we do we need a test :/ === pstolowski|afk is now known as pstolowski [07:07] morning [07:08] mborzecki: which is annoying, I think I will snap my libevent-server hack from nghttp2 [07:08] mborzecki: so that we have something we can test against - one issue with this is that http2 is tls so I need keys and for self-signed I need to alter the config which lead to the issue that my test client was not quite in the same state as the real one [07:09] mborzecki: did I mention its annoying ;) [07:09] PR snapd#7306 closed: overlord/configstate: sort patch keys to have deterministic order with snap set [07:10] thanks pstolowski ! nice to see this one goingin [07:11] mvo: yeah, that was the last one [07:12] mvo: can you take another look at https://github.com/snapcore/snapd/pull/7219 ? it's close to landing [07:12] PR #7219: devicestate/firstboot: check for missing bases early [07:13] pstolowski: hey [07:13] pstolowski: yes! fiddling a bit with tests then I'm ready [07:20] a review for 7312 would be great, it should eliminiate another relatively common test failure [07:21] Good morning [07:23] hey zyga [07:27] Sleeping with two kids and a dog on one bed [07:27] Ugh [07:27] Brb [07:34] looks like some redditors attended the london meetup https://www.reddit.com/r/linux/comments/ctxaff/rebootless_kernel_updates_by_canonical/ [07:36] PR snapd#7318 opened: many: merging asserts.Batch Precheck with CommitTo and other clarifications [07:36] mvo: ^ the follow to asserts.Batch PR [07:46] PR snapd#7312 closed: tests: use user-tool to remove test user in the non-home test [07:50] PR snapd#7319 opened: spread.yaml: set centos-7-64 to manual [07:53] Chipaca: have you have a moment, can you double check the last changes to https://github.com/snapcore/snapd/pull/7149 [07:53] PR #7149: cmd: add snap model command; daemon: add /v2/model, /v2/model/serial REST APIs [07:55] pedronis: sure, in a bit [07:58] pedronis: hey, on pi3 the aa_parser timings are indeed showing significant difference with multiple runs vs all-at-once, the later is 3x faster with just 11 snaps. i'm writing a summary [07:58] thank you [08:02] pstolowski: nice work [08:03] pstolowski: nice! [08:03] mvo: have you seen this error /usr/sbin/deluser: `/sbin/userdel jim' returned error code 8. Exiting. ? [08:04] mvo: accroding to manapage, 8 means the user is logged in [08:04] jim? [08:04] mborzecki: yes [08:04] mborzecki: this is what the Pr you most recently reviewed was about [08:04] mborzecki: using the new "user-tool del" that uses deluser --force [08:06] mvo: wondering why userdel believes the user is logged in though [08:06] mvo: could that be somehow related to issues that zyga saw on arch [08:06] mborzecki: I think it checks for processes that are owned by the user [08:06] mborzecki: the issue from zyga was about systemd creating files or something? [08:07] mvo: why did you add --remove to userdel? [08:07] mvo: pam I think [08:07] mvo: and why only to one branch of the code? [08:07] zyga: it removes the homedir as well [08:08] aha, and should it run on core too? [08:08] zyga: uh, you are right, the second one should also have it for completness [08:08] mvo: I rebased https://github.com/snapcore/snapd/pull/7309/files [08:08] zyga: the current master should fix the practical failure but it still should be consistent [08:08] zyga: ta [08:08] PR #7309: tests: re-implement user tool in python [08:09] mvo: if you can review that I'll follow up with extra --remove in the core case [08:09] I'm looking at chipaca's PR [08:24] PR snapd#7320 opened: snap/pack, cmd_pack: 'snap pack --check-skeleton' checks interfaces [08:25] * Chipaca curses at his trackpad [08:26] zyga: thank you for looking at my pr [08:38] oh, drat, import cycle [08:39] in tests [08:39] :-( [08:42] PR snapd#7320 closed: snap/pack, cmd_pack: 'snap pack --check-skeleton' checks interfaces [08:43] Chipaca: :( and i was looking at it [08:43] mborzecki: sorry :-( it shouldn't change too much, but import cycles are hard to fix [08:43] mborzecki: need to shunt the code around [08:44] Chipaca: fwiw you could just call snap.SanitizePlugsSlots [08:44] or not :/ need to import interfaces/builtin [08:50] pstolowski: question for you sir [08:50] pstolowski: why is 'plug/slot sanitization not possible from snap command'? [08:56] Chipaca: because that would required all interfaces/ code to be imported, since they define BeforePrepare* methods [08:56] pstolowski: and why is that bad? [08:59] Chipaca: i don't know, might be historical; at some point i was told it was bad ;). maybe pedronis / mvo / zyga remember [08:59] zyga: experimental.parallel-instances is set to true, shouldn't there be a feature file in /var/lib/snapd/features? [08:59] Chipaca: as in statically? [09:00] Chipaca: because some are only sanitized after connections I guess [09:00] but I think it'd be nice to have a pre-check [09:00] mborzecki: not always, check for is-exported [09:00] mborzecki: only exported flags have this [09:00] Chipaca: I'm not sure what's the question there? [09:01] 1 sec [09:02] pedronis: in cmd/snap/main.go we replace snap.SanitizePlugSlots with a nop [09:02] PR snapd#7320 opened: snap/pack, cmd_pack: 'snap pack --check-skeleton' checks interfaces [09:03] pedronis: there's even a comment, // plug/slot sanitization not used nor possible from snap command, make it no-op [09:03] Chipaca: ah [09:03] not sure [09:03] pedronis: but, for #1617302, I _want_ that code to be usable [09:03] Bug #1617302: Specifying a nonexistent plug does cause any errors [09:03] you'll have to dig [09:03] pedronis: so, I dropped that no-op'ing [09:03] pedronis: and nothing broke :-) [09:03] zyga: hm maybe we should export all experimental flags by default [09:04] zyga: no, this is about BeforePreparePlug|Slot, so static [09:04] mborzecki: we had that discussion [09:04] mborzecki: and it was nacked [09:04] Chipaca: might be there is a size concern [09:04] zyga: duh, ok [09:04] mborzecki: I think it's fine to export it if there is a purpose [09:04] pstolowski: interesting, then I don't know [09:04] pedronis: might be; locally snapd goes from 17 to 18 MBs with that import (from 10 to 11 stripped) [09:07] zyga: heh, so we're missing a bit that updates the feature files on startup :/ [09:07] mborzecki: hmm? [09:08] zyga: the feature files are created only when core config is updated [09:08] Chipaca: ok, i've an idea [09:08] mborzecki: I thought we had that, because I kept looking at that code at the time and it was full of interesting avenues [09:08] mborzecki: ah, because you exported it now and it is gone === alan_g is now known as alan_g_ [09:08] zyga: s/gone/not added/ [09:08] yeah [09:09] pstolowski: sounds dangerous [09:10] Chipaca: i think we did this because they were already validated on install, so you're not paying the penalty every time on snap run - execApp reads the yaml and calls Sanitize internally (and is noop atm) [09:11] pstolowski: can you comment on the PR? [09:11] pstolowski: it sounds like i need to do some trickery :-) [09:11] * Chipaca is fine with trickery [09:14] sure [09:15] PR snapd#7310 closed: devicestate: add missing test for remodeling possibly removing required flag [09:17] PR snapd#7318 closed: many: merging asserts.Batch Precheck with CommitTo and other clarifications [09:22] Chipaca: commented [09:24] * pstolowski will learn one day the spelling of "negligible" after looking it up again in the dict [09:25] reminder: #7135 needs a 2nd review [09:25] PR #7135: image: support prepare-image --classic for snapd snap only images [09:34] Chipaca: https://github.com/snapcore/snapd/pull/7185#pullrequestreview-278861850 [09:34] PR #7185: boot, etc.: refactor boot to have a lookup with different imps [09:42] PR snapd#7314 closed: tests/main/mount-ns: account for clone_children in cpuset cgroup on 18.04 [09:49] zyga: I can't refactor all the things in a single PR :-/ [09:49] it'd be un-reviewable [09:49] Chipaca: that's fine, that's why my general comment was "how do you want to proceed" [09:49] zyga: "find a nice sandy riverbank and have a cookout" [09:51] * Chipaca goes for coffee [10:02] PR snapd#7135 closed: image: support prepare-image --classic for snapd snap only images [10:03] pedronis: https://forum.snapcraft.io/t/apparmor-parser-performance-on-pi3/12909 [10:04] PR snapd#7311 closed: snap/naming: introduce SnapRef, Snap, and SnapSet [10:13] pstolowski, i wish you would test on actual low-end HW (imx6ull like rigado uses or a beaglebone/pocketbeagle) with actual single core CPU and respectively slow IO channels ... but it is awesome that this gets finally attacked at least :) [10:14] pi's are way to powerful :) [10:14] ogra: haha, pi3 is the slowest hw i have :) [10:15] pstolowski: boot with cpu=1 [10:15] or whatever the switch is [10:15] cpus iirc [10:16] zyga, that doesnt help shpwing actual single core IO performance [10:16] IO performance? [10:16] i think the point of the test has already been proved [10:17] yeah, the prob with real single core arm is that your IO is just something like a fifo ... if you take multi-core HW you still use the fast IO channels. even if you limite the number of cores [10:17] yeah, it has [10:17] and it obviously will be much worse on weaker specs [10:17] but it wont give you realistic numbers as we get them on real customer HW [10:17] right [10:17] pstolowski: maxcpus https://elixir.bootlin.com/linux/latest/source/Documentation/admin-guide/kernel-parameters.txt#L2368 if you want to play with it [10:17] * ogra points at 30min firstboot times :) [10:18] mborzecki: yeah i know, i used it for some other test the other day [10:18] pstolowski: each time we do an interation on performance :P [10:23] PR snapd#7321 opened: tests: restore dpkg selections after upgrade-from-2.15 test [10:26] where did mvo go? [10:26] PR snapd#7322 opened: tests: wait_for_service shows status after actual first minute [10:26] mborzecki: ^ two trivial PRs [10:29] mborzecki: slightly longer but also simple https://github.com/snapcore/snapd/pull/7309 [10:29] PR #7309: tests: re-implement user tool in python [10:35] PR snapd#7323 opened: features, overlord: make parallel-installs exported, export flags on startup [10:35] zyga: feature flag ^^ [10:36] looking [10:36] mborzecki: please also change cmd/libsnap-confine-private/feature.[ch] [10:37] duh /o\ [10:37] :D [10:38] ok, i can change that bit in the PR with C code [10:39] mborzecki: add one liner test to TestControlFile as well please [10:39] and if you do roll the C changes to flags here as well [10:39] so that the other PR focuses only on the meat of the parallel installs for classic snaps [10:40] d'oh [10:40] I found why interfaces-fuse_support misbehaved [10:41] I'll also rename the directory because it's silly [10:45] hey mvo [10:45] mvo: can you look at https://github.com/snapcore/snapd/pull/7319 - there's a question there [10:45] PR #7319: spread.yaml: set centos-7-64 to manual [10:46] PR snapd#7319 closed: spread.yaml: set centos-7-64 to manual [10:46] thanks! [10:46] zyga: yes, already replied, the most recent run of another PR was green so it seems to be resolved already [10:46] mvo: I think 7321 is something you want to look at as well [10:46] zyga: I saw it on 2 PRs before hence the change but looks short-lived [10:46] (super simple) [10:47] and if you still want to after that, https://github.com/snapcore/snapd/pull/7309 to put the user-tool change to rest [10:47] PR #7309: tests: re-implement user tool in python [10:47] zyga: thanks, I will followup in this one [10:47] zyga: on the 7321 [10:48] PR snapd#7324 opened: image: improve/tweak some warning/error messages [10:48] Chipaca: ^ [10:48] pedronis: thank you [10:49] PR snapd#7325 opened: cmd/libsnap-confine-private: add checks for parallel instances feature flag [10:49] zyga: ^^ [10:50] +1 [10:51] PR snapd#7225 closed: tests: don't repeat checks <⛔ Blocked> [10:51] mborzecki: can you do a pass over the two simple PRs I sent [10:51] I'll grab coffee and be back soon [10:54] zyga: I added a comment to 7321, might be best to have a quick HO so that we can chat about the intended result [11:02] mvo: d'oh! [11:02] man so dpkg is not doing what I thought it would do [11:03] zyga: no worries, its a common misconception [11:03] zyga: this mechasnism is *really* old :) [11:03] PR snapd#7299 closed: sanity: report proper errror when fuse is needed but not available [11:03] mvo: so are you saying there is no reliably way to save/restore actual packages installed on the system? [11:03] zyga: is that what you want? checkpoint; do stuff; restore? [11:03] yes [11:04] so that this test doesn't impact the set of installed packages that much [11:04] zyga: ok, let me scratch my head about this a wee bit [11:04] zyga: there is no trivial way I think [11:05] mvo: also, to actually describe the snasphot [11:05] so that we can use the same thing for detection [11:05] (in other words, to produce a diff when test doesn't clean up) [11:14] zyga: sure, will do [11:14] zyga: I add something in python-apt to tests/lib/bin [11:15] mvo: super, thank you [11:15] zyga: there is actually a tool called apt-clone which we might use but it won't remove stuff, should be easy to add though [11:15] zyga: sry that this is so hard :/ [11:15] PR snapd#7326 opened: tests: unmount fuse connections only if not initially mounted [11:15] mvo: for context, I have an in-flight tool called "testbed-tool" that invokes helper tools to examine system state (it uses mountinfo-tool currently) [11:15] mvo: if we do it right I can use your apt code once you have it [11:16] mvo: please look at 7326 -- that's the last leak on classic [11:16] it took me longer because I was blind and didn't see - vs + in the diff :) [11:16] this unblocks mount-ns test again [11:16] we still leak stuff on core, mainly in mysterious ways (mounts that are not accounted to any snaps) [11:16] but this is a major step towards not leaking anything anymore :) [11:17] PR snapd#7321 closed: tests: restore dpkg selections after upgrade-from-2.15 test [11:18] mborzecki: I'll focus on re-enabling mount-ns test on classic only now [11:18] zyga: btw. parallel installs PR also broke socket activation under enforcing selinux, wonde if there's osmething wrong with the contexts after the bind mount [11:18] zyga: obviously nothing in the logs :P [11:18] mborzecki: oh, that's odd [11:18] https://www.irccloud.com/pastebin/6v7HDXvH/ [11:18] do you have the log? [11:19] zyga: no, need to try locally and probably disable noaudit [11:19] +1 [11:25] I'll spawn some tests and return to 7218 [11:26] mvo: https://github.com/Originate/scriptkeeper [11:31] pstolowski: can #7219 be landed? [11:31] PR #7219: devicestate/firstboot: check for missing bases early [11:37] zyga: https://forum.snapcraft.io/t/cannot-use-gimp-snap-as-rawtherapee-external-editor/12908 heh /tmp [11:38] zyga: I pushed something to your PR [11:38] zyga: I am running a test locally now [11:39] mvo: which one? [11:39] zyga: the apt one [11:39] ah, I closed it [11:39] you can reopen though [11:39] zyga: yeah, once the local run is done I will [11:40] mvo: what did you push, gh is not showing anything (because it is closeD) [11:40] * zyga hugs mvo for not giving up :) [11:40] * zyga just ran 1400 tests without leaking a single mount entry [11:40]