[05:24] morning [05:43] Hey [05:44] mborzecki: I will be around in about an hour [05:44] zyga: hey [06:02] PR snapd#8745 closed: tests: port xdg-open-compat to session-tool [06:03] PR snapd#8746 closed: tests: port xdg-open to session-tool [06:21] thanks! [06:21] mborzecki: it would be good to review the f32 branch from sergio [06:21] once merged we should have those green ticks again [06:21] zyga: yeah, i'll be pushing some fixes there [06:21] ok [06:24] brb again [06:33] mvo: hey [06:35] good morning mborzecki [06:39] PR snapd#8743 closed: tests: cherry-pick test fixes from master (2.45) [06:46] 8351 needs a second review [06:48] PR snapd#8661 closed: configcore: add "service.console-conf.disable" config option [07:03] mornings! [07:04] PR snapd#8748 opened: overlord: refuse to install snaps providing user daemons on Ubuntu 14.04 [07:09] #8533 has been unblocked and needs 2nd review [07:09] PR #8533: image, tests: core18 early config [07:18] PR snapd#8749 opened: configcore: show better error when disabling services [07:36] jamesh: https://github.com/snapcore/snapd/pull/8748#pullrequestreview-418929307 [07:36] PR #8748: overlord: refuse to install snaps providing user daemons on Ubuntu 14.04 [07:36] mvo: thank you for 8749! [07:39] pstolowski: hi, could you review #8351 ? also do poke Sergio about your #8533 [07:39] PR #8351: config: apply vitality-hint immediately when the config changes [07:39] PR #8533: image, tests: core18 early config [07:39] pedronis: yes [07:41] pstolowski: https://github.com/snapcore/snapd/pull/8709#pullrequestreview-418926302 [07:41] PR #8709: cmd/snap-preseed, systemd: fix handling of fuse.squashfuse when preseeding (2/3) [07:41] zyga: thanks! [07:43] zyga: thanks. I guess one difference is that the systemd in CentOS 7 supports user units, but they just don't run the instance. [07:44] jamesh: what do you mean by supports? [07:44] whereas 14.04's systemd doesn't support them at all [07:44] ah [07:44] centos 7 is compiled without that IIRC [07:44] (explicitly disabled) [07:44] but those are shades of gray I suppose [07:44] zyga: as in "systemctl --user --global enable some-user-unit.service" succeeds on Centos 7 [07:44] Ah, that's weird, perhaps not fully disabled [07:45] but officially disabled per RH design [07:46] zyga: I guess in some ways it is similar to Ubuntu Core, where it is technically possible to set up a user session but it is rarely done [07:47] (granted that will probably change in future) [07:56] brb, small breakfast [07:58] hmm ... is there a snapctl command that can tell me if seeding is done ? [08:04] ogra: snapctl? [08:04] or snap? [08:07] zyga, snapctl (or the rest API) [08:07] a customer wants to check the state from inside a snap [08:08] i know he could loop over the changes output via the API until "Initialize device" is marked "done" but wanted to know if there is a more integrated and less hackish way [08:08] I don't think this is supported [08:08] let me look [08:09] nope [08:09] not supported [08:10] ok, then looping over changes it is ... [08:18] ogra: the snapd.seeded.service systemd service runs "snap wait system seed.loaded". If you've got access to the REST API, you could presumably do the same === ijohnson_ is now known as ijohnson === tomwardill_ is now known as tomwardill [08:20] jamesh, hmm, how would i get that info from the api beyond looping over the changes ? [08:21] i dont see any rest endpoint that could give me that info beyond the chnages output [08:21] ogra: check what /usr/bin/snap does? [08:22] ogra: looks like it's effectively doing "snap get system seed.loaded" every 500 milliseconds until it returns true [08:22] oh, its aconfig option !!! [08:22] thanks ! i totally forgot about that bit [08:23] so there *is* a snapctl way in the end :) [08:23] snapctl will only give you your own config though, right? [08:24] not the system snap's configuration [08:24] it queries "system" in that case [08:24] well, a gadget can do that [08:24] iirc [08:24] you'd know more about that than me :-) [08:26] ogra: snap and snapctl have different APIs [08:27] zyga, well, he has the API ... so using "GET /v2/snaps/[name]/conf" should be able to get him "system" configs, no ? [08:28] what do you mean by "he has the API" [08:28] presumably he has snapd-control level access [08:28] ah [08:28] in that case then yes [08:28] teh question comes from a customer with brand store [08:28] right [08:28] geez, we need customer docs for this 1 [08:28] ! [08:28] * ogra makes note [08:29] i know some that *actually* loop over the changes output to find out :) [08:48] * zyga debugs with -seed= again [08:55] hey, reproduced [08:56] ha [08:56] funny [08:56] let's write an invariant to find the culprit [08:57] some test nukes some packages but doesn't reload systemd [08:57] so the invariant will find systemd that complains abou "some units changed on disk" [08:57] and this will lead us to the test breaking stuff [09:27] eureka [09:27] tests/upgrade/basic user unit snapd.session-agent.socket changed on disk [09:29] in fact [09:29] it keeps getting changed [09:29] hmmmm [09:29] I wonder why [09:29] maybe we should not be agressively rewriting it [09:30] but that's just noise [09:31] there's another test that breaks dbus.socket [09:53] fortunately not that many tests need fixes [09:53] about half a dozen one liners [09:53] mvo: ^ another lesson learned [09:53] systemd units have state in memory and on disk [09:54] and in memory state is the one that counts [09:54] units are modified by tests and left stale in memory after "restoring" the disk state [09:56] zyga: oh, woah, nice catch [09:56] yeah, it manifested as a random failure that I managed to reproduce with -seed [09:56] and ended up being a dbus.socket that's only in memory because the disk file is gone [09:56] and cannot be activated (because no .service because also gone) [09:57] so some test removes dbus.socket/service but the bigger find is that the in-memory state matters [09:57] writing and running tests is useful but I think we could write a boot about integration testing now [10:03] *book :) [10:04] PR snapd#8747 closed: tests: port snap-routine-portal-info to session-tool [10:15] reading bug reports, it's amazing how people quickly jump to reinstalling the system whenever even a smallest issue pops up [10:16] mborzecki: do you know windows made that super easy? [10:16] maybe it's something they learned and apply to other OSes [10:16] zyga: seriously? my experience with windows reinstall was that it was a huge waste of time [10:17] mborzecki: why? [10:17] though it did fix issues there ;) [10:17] mborzecki: btw, windows 10 reinstall is super easy [10:17] it's literally 4 clicks from the start menu [10:17] and the experience is quite nice [10:17] haven't reinstalled 10 yet, maybe i'm not using it enough [10:17] (can preserve apps or just data, entirely unattended) [10:18] mborzecki: https://github.com/snapcore/snapd/pull/8693/files has some spurious changes [10:18] PR #8693: tests: add fedora 32 to spread.yaml [10:18] are they relevant or can we nuke them? [10:18] zyga: yeah, noticed them after i pushed the changes, don't think it's worth another spread run though [10:18] ok [10:18] I resolved a conflict [10:19] but didn't notice this until after [10:19] hah ;) [10:19] one thing that I miss on !windows is family management [10:19] once it's green, we should land it [10:19] yeah [10:19] control over time, app access and even browser page access [10:20] haha i talked with mvo about a killed linux app that could do that [10:20] I really wish we had something comparable [10:20] which would be great, and we probably have the right technology to do it already [10:20] yeah but it's either really well done and useful or it's not useful at all :/ [10:20] loopholes => not useful, poor UX => not useful, not localized => not usefl [10:20] though I bet it would be good to have something and work from there [10:21] I think one major problem is that it requires central authority and accounts to be easy to use [10:21] well, we don't have anything there yet, so ;) [10:21] yeah [10:21] pedronis: added some comments under https://github.com/snapcore/snapd/pull/8569 [10:21] PR #8569: o/assertstate,asserts: use bulk refresh to refresh snap-declarations [10:21] even limiting time is hard [10:21] gdm would have to cooperate [10:22] pedronis: is that the last PR in that series? [10:22] mborzecki: I saw, thx [10:22] * zyga needs to manage browser tabs, brb [10:22] mborzecki: it's the main one, but no, as the description says there will be a couple more follow ups, one is almost ready, but is not proposed yet [10:22] ok [10:24] zyga: can you take another look at #8639? [10:24] PR #8639: o/cmdstate: handle ignore flag on exec-command tasks (1/N) [10:24] ha [10:24] sure [10:24] I just looked at another branch and though that was the same one :) [10:26] heh, something weird about go compiler [10:26] hmm? [10:27] took #8749, dropped the default: case like so https://paste.ubuntu.com/p/NJSFCQkF8N/, then compiler complains about ./services.go:90:1: missing return at end of function [10:27] PR #8749: configcore: show better error when disabling services [10:28] by looking at the code i see that there always is a return in the switch cases there [10:30] PR core20#69 opened: hooks: remove redundancy from timezone settings [10:30] heh this is weird https://play.golang.org/p/-Bxl-hPNAeu [10:31] mborzecki: heh, this looks familiar [10:31] mborzecki: go doesn't do proper analysis [10:31] mvo: yup, the pr you opened [10:31] erring on the "compile quickly" [10:31] mborzecki: I was surprised as well [10:31] it sucks but it's a known issue [10:32] mvo: wonder whether the compiler would even determine that panic code is unreachable [10:32] mborzecki: if it can it should be easy to fix the bug :) [10:32] mborzecki: I mean, the bug/issue that this requires a return still [10:32] famous last words :P [10:32] lol [10:32] actually rofl [10:33] yeah, I will not try or I get sucked into a black hole [10:33] mborzecki: I wonder if: if false { println("DUPA") } [10:33] mborzecki: compiles in a way where DUPA is gone from the binary [10:34] haha if works https://play.golang.org/p/j2VSn1xex7K [10:34] mborzecki: woah, that's "funny [10:34] " [10:35] it's weird especially considering that go's switch/case degenerates to if-then-else tree [10:37] PR snapd#8351 closed: config: apply vitality-hint immediately when the config changes [10:38] funny that it's quite happy when there's a default case [10:41] so bools in a switch are tri-state, true, false, something-in-between [10:41] pedronis: is #8569 ready to be reviewed? [10:41] PR #8569: o/assertstate,asserts: use bulk refresh to refresh snap-declarations [10:42] mborzecki: that's peculiar [10:45] PR core20#69 closed: hooks: remove redundancy from timezone settings [10:47] mvo: thank you for that test btw [10:47] mvo: I wonder if we could integrate some other things there [10:47] like timedatectl [10:47] time to file a bug in go [10:47] setting timezone [10:47] setting NTP [10:47] all super basic stuff [10:47] but it was broken on more than one occasion [10:48] PR snapd#8750 opened: tests: improve oom-vitality tests [10:49] zyga: you mean 69? [10:49] mvo: you proposed something relevant yesterday [10:49] to snapd [10:49] zyga: aha, yes - we can, it's strange, partly this is covered by interface tests [10:49] zyga: but I suspect it's much cleaner to have a dedicated test [10:50] yeah but I think there's a difference, those tools often use DBus APIs [10:50] and perhaps there's some factor where a tool modifies something directly vs issues a call [10:50] so good to have a dedicated basic-management test like that [10:51] pstolowski: https://github.com/snapcore/snapd/pull/8351#discussion_r431028861 [10:51] PR #8351: config: apply vitality-hint immediately when the config changes [10:52] zyga: cool, thanks, I will work on the system-settings a bit more I guess [10:52] I actually wonder [10:52] in your core system deployments [10:52] did you set things like hostname, timezone and ntp manually? [10:54] * zyga breaks focus to finish some reviews [10:57] pstolowski: https://github.com/snapcore/snapd/pull/8639#pullrequestreview-419079982 [10:57] PR #8639: o/cmdstate: handle ignore flag on exec-command tasks (1/N) [11:00] * zyga found the broken test [11:03] PR core20#70 opened: Fix localtime [11:06] PR core20#71 opened: tests: support test_etc-writable-symlinks.sh outside snapcraft [11:13] brb, coffee [11:14] it's hard to use the coffee machine [11:14] when one kid is constantly remote-schooling in the kitchen :D [11:21] pstolowski: zyga: I asked a new question in #8691 [11:22] PR #8691: tests/lib/bin: a TODO to improve the naming and uniformity of utilities [11:24] pstolowski: #8569, yes [11:24] PR #8569: o/assertstate,asserts: use bulk refresh to refresh snap-declarations [11:24] there are some open questions and tweaks but in itself is good to go [11:24] pedronis: looking [11:26] replied [11:29] gah, I wish spread used mosh instead of ssh [11:29] mosh is so nice for roaming [11:30] PR core18#154 opened: Test timezone contents and symlink [11:35] PR core20#71 closed: tests: support test_etc-writable-symlinks.sh outside snapcraft [11:38] pedronis: testing.global.session testing.netgate testing.global.invariant [11:38] pedronis: how do those sound to you? [11:42] zyga: I'm thinking, I'm actually starting to think that for most of them the problem is a bit self-inflicted [11:48] pedronis: because they have before/after parts? [11:48] * zyga probably doesn't follow [11:49] no, because it's not clear why we need most of them on PATH, they aren't used enough to justify that and the naming problem it creates [11:50] pedronis: they are on PATH so that they do not require sourcing via $TESTSLIB [11:50] in the past we had functions that were bad for other reasons [11:50] what it has to do with sourcing [11:51] but required sourcing and this was tedious [11:51] ah, because they were functions and not on path, you had to get them somehow [11:51] these are not functions [11:51] right [11:51] also as I said, the tedious argument doesn't apply to many of them [11:51] they are not used enough to make it a good reason [11:51] sure but some are super common, are you suggesting to drop them from path/ [11:51] i think part of the reason they are in PATH is to make them usable to debug tests easily when in spread shell [11:51] oh, that's useful as well [11:51] * zyga didn't think about that [11:52] you can paste stuff from a test and it mostly works [11:52] (depending on how much you paste) [11:53] pstolowski: again, I don't thinkt that argument applies equally [11:53] to all of them [11:53] I don't see a problem with them being on path [11:54] testing.foo is a good solution if we need to have unique prefix [11:54] yes. i'm not even too concerned about testing.not tbh, it's just a prefix [11:57] at some we can't start to explain that we did something to avoid tedious, and then circle back there, testing.negate is fairly tedious [11:57] tbh [11:58] pedronis: not is not tedious :) [11:58] exactly [11:58] pedronis: but importing something or $TESTSLIB/tools/not is also a bit tedious [11:58] but not might be a special case [11:58] my point is that I don't trying to find a solution that works for all of them [11:58] is a goal [11:58] as it's super short and small and frankly not related to snapd [11:58] I think handling session-tool and retry-tool should be the goal [11:59] as those are used fairly often [11:59] the rest can be another class that doesn't need the same solution [12:00] also shellcheck is not helping here [12:00] in which sense? [12:01] that it forces quotes, I understand where it comes from [12:01] but sometimes you know what is in your env var [12:02] one more idea is to multiplex everything through one tool that is on path [12:03] snapd-testing ... [12:03] snapd-test-helper ... or whatever we name it [12:03] that might be part of the solution [12:03] then it's one item on path and pretty easy to even handle migrations [12:03] yes that's an interesting idea [12:03] (assuming we migrated to the tool in the first place) [12:04] we could even call it [12:04] "the-tool" [12:04] except that's for core 20 [12:04] no [12:04] I was totally kidding :) [12:04] testctl sounds like systemd thing [12:05] though "testctl foo" could invoke "testctl-$foo" so we'd have a simple way to use various languages (like we do now) [12:05] so I kind of like that more and more [12:06] and the rest would not need to be on path eventually [12:09] * zyga is surprised [12:09] -.mount unit changed on disk https://www.irccloud.com/pastebin/oVaMhye7/ [12:09] how is this even possible?!? [12:09] mborzecki: ^ any ideas? [12:10] ah, we modify fstab [12:10] fun [12:10] fixed now [12:11] zyga: yup, the test touches fstab [12:11] and -.mount is autogenerated [12:12] yeah [12:12] another one from another test [12:12] rtkit-daemon.service changed on disk https://www.irccloud.com/pastebin/JCg4YEkc/ [12:12] the test installs/removes that [12:20] pedronis: do you have a feeling on what to do with test names then? [12:20] pedronis: which way do we go [12:21] not yet [12:21] s/test names/test tool names/ [12:21] ok [12:21] I'm happy to apply the change, when you make up your mind [12:21] I think we could also have a two-tier system, where some rarely used tools can have different properties [12:22] as some are indeed used a few times in the entire project [12:22] cachio: is the ubuntu 16.04-32 (note -32) image using EFI? [12:23] zyga, it supports uefi, not really sure if it is using it, I suppose that yes [12:23] ok [12:23] I think it boots in legacy mode [12:23] but that's fine [12:24] I was just curious [12:24] zyga: when do you want to discuss refresh awareness, tomorrow morning after the desktop meeting? [12:24] zyga, we are not enabling any security feature, so by default iirc the systems use the legacy mode [12:24] pedronis: that sounds great [12:24] pedronis: I will push all my changes by then [12:25] pedronis: I took a small detour to fix various random issues I ran into but test coverage is close to being good now and I still have a few more patches to send [12:25] zyga, but it is managed by the gce hypervisor and there is not details in the docs about it [12:25] cachio: ok, I guess it's uncommon to use 32 bit systems so maybe they don't boot them with uefi [12:25] zyga, just few line explaining the high level [12:25] cachio: I only noticed because boot/efi was missing [12:28] cachio: hi! could you take a look at https://github.com/snapcore/snapd/pull/8533 ? also updated #8710 [12:28] PR #8533: image, tests: core18 early config