[00:56] PR snapcraft#2916 closed: status: implement using the new channel-map endpoint [01:53] PR snapcraft#2992 closed: [WIP] repo: ubuntu implementation refactoring with initial package-management [03:11] PR snapcraft#3006 closed: repo: always use host release and arch for Ubuntu [05:34] morning [06:13] mvo: hey [06:20] mborzecki: good morning [06:29] PR snapd#8410 closed: many: disentagle release and snapdenv from sandbox/* [06:31] PR snapd#8412 opened: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling [06:34] PR snapd#8404 closed: client: increase timeout in client tests to 100ms [07:03] morning [07:05] good morning pstolowski [07:05] good morning [07:05] hey mvo, [07:06] mvo: I noticed we have a small problem in debian [07:06] mvo: snapd doesn't build [07:06] dh_missing: warning: usr/bin/snap-preseed exists in debian/tmp but is not installed to anywhere [07:06] I think we have different ruleset there, and leftover binaries cause builds to fail [07:06] I'll see if I can use my shiny metal rights and actually help [07:07] Had another CI run hit Travis's 50 minute limit: https://travis-ci.org/github/snapcore/snapd/builds/670392870 [07:07] jamesh: I'll move travis jobs to actions today [07:07] I think it's time [07:14] good morning zyga [07:15] 8103 needs a second review, it's important for the encryption story and to unblock the TPM work [07:16] jamesh: thanks for letting us know, once we are slightly less busy we move to gh actions to get rid of this [07:17] jamesh: also thanks for your review on 8406, I really have no better idea unfortunately how to make this better, maybe we could increase the sleep to 100ms to have a bigger "window" of time to look at. yeah, over-committed CI systems are very annyoing [07:18] (fwiw 8103 is not very long, hopefully quick review) [07:24] PR snapd#8413 opened: interfaces: don't use the owner modifier for files shared via document portal [07:27] jamesh: if 8406 looks ok enough please approve, if you feel it makes the test useless I could close and we just need to live with the errors [07:28] mvo: I think the change looks good. When run on an unloaded system, it should still verify that the timeout has been extended [07:29] zyga: session-tool fails on master https://api.travis-ci.org/v3/job/670441262/log.txt - could you please ahve a look? [07:31] Yes [07:44] PR snapd#8406 closed: usersession: extend timerange in TestExitOnIdle [07:45] pedronis_: I think the extra spread test I added covers what you wanted in my user daemons PR === pedronis_ is now known as pedronis [07:46] jamesh: thanks [08:17] pedronis: thank you for the move yesterday! [08:41] PR snapd#8414 opened: o/configstate: core config handler for persistent journal [08:41] ogra: hi, this might be interesting for you ^ (feedback is welcome) [08:54] nice ! [08:54] pstolowski, you are aware that etc/systemd/journald.conf.d/ is complete nonsense ? [08:55] ogra: heh, not, why? [08:55] pstolowski, all you need to do is to create the dir ... [08:55] ogra: you mean /var/log/journal ? [08:55] the builtin defaults are completely fine, if the dir exists journald writes the journal to disk ... if not it uses the ringbuffer ... there is no need to change the config [08:56] yeah [08:56] huh [08:56] i'D just make the code call a mkdir and be done ;) [08:56] mhm [08:57] (and rmdir if the option is unset) [08:57] ondra: is this consistent across systemd versions (core16, core18...)? [08:57] sorry, ogra ^ [08:57] notsure what ondra thinks ... but it surely is for 16 and 18 [08:57] :) [08:58] might be that 20 changed it though ... better test there .... for 16 and 18 it was always sufficient to just create the dir [08:58] * ondra agrees with anything ogra says, as long as it's not about partitions! [08:58] ! [08:59] ogra: ok, that's interesting, thank you! [09:00] thank *you* for implementing it !! [09:00] :D [09:03] ogra: ah, i read the manualy again. so it appears that "auto" is the default behavior, the existense of /var/log/journal controls the behavior [09:03] yep :) [09:04] ogra: thanks again for the enlightenment ;) [09:17] pstolowski, ogra: related review https://github.com/snapcore/snapd/pull/8414#pullrequestreview-387089711 [09:17] PR #8414: o/configstate: core config handler for persistent journal <⛔ Blocked> [09:18] zyga: thanks. we might not need the conf file at all, i'm going to verify this [09:18] thanks [09:18] ping me for a re-review please [09:36] PR snapd#8415 opened: cmd/snap-recovery-chooser: tweaks [09:45] mborzecki: I nitpicked a bit in #8415 [09:45] PR #8415: cmd/snap-recovery-chooser: tweaks [09:51] mborzecki: https://github.com/snapcore/snapd/pull/8415#pullrequestreview-387112832 [09:51] PR #8415: cmd/snap-recovery-chooser: tweaks [09:52] pedronis: zyga: thanks [09:53] I'll be back shortly, it's quite cold in the office and ought to make some warm tea [10:00] mborzecki: unit tests failed on that PR [10:00] waat? [10:00] https://www.irccloud.com/pastebin/NkNEOsNj/ [10:00] not yours but meh [10:00] mvo: ^ [10:00] is that another thing to adjust? [10:01] weird, that timeout is longer already [10:01] maybe a bug in the test/code? [10:02] zyga, mborzecki I thought I did push something there? [10:02] ah, perhaps maciek's branch is simply older [10:02] https://github.com/snapcore/snapd/pull/8412 [10:02] PR #8412: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling [10:02] mvo: yes, https://github.com/snapcore/snapd/pull/8404 has landed already [10:02] PR #8404: client: increase timeout in client tests to 100ms [10:03] mborzecki: this one is different, no? that's what 8412 is for [10:03] mborzecki: or am I misreading this? [10:03] pstolowski: mvo: i think there's a problem with persistent logs, /etc/machine-id isn't mounted from writable (at least on core20) so after reboot you get a new machine-id and a new journal [10:03] oh, w8, it is [10:03] so why did i get a new machine-id again? [10:03] mborzecki: works as expected on core18 [10:04] (i rebooted) [10:05] hmmmm ├─/etc/machine-id tmpfs[/machine-id] tmpfs ro,size=203112k,mode=75 [10:05] yeah [10:05] pstolowski: mvo: ^^ [10:05] why do we have machine-id on a tmpfs [10:05] mborzecki: we noticed this a while ago [10:05] in the review of mount-ns data [10:05] I guess we forgot to act [10:06] pstolowski: do you hav a core18 vm? can you check `findmnt /etc/machine-id` ? [10:06] mborzecki: yep, 1 sec [10:06] mborzecki: I do [10:07] /etc/machine-id /dev/sda3[/system-data/etc/machine-id] ext4 rw,relatime,data=o [10:07] so that's weird [10:08] mborzecki: from pi4 /etc/machine-id /dev/mmcblk0p2[/system-data/etc/machine-id] ext4 rw,relatime [10:08] mystery [10:08] hm it's not listed in writable-paths, what sets it up? [10:08] maybe that's our test specific setup [10:09] zyga: anything in /etc/fstab or /run/image.fstab? [10:10] zyga: i see /etc/machine-id /dev/sda3[/system-data/etc/machine-id] ext4 as well [10:10] what is /run/image.fstab? [10:10] where are writable paths set up [10:10] I mean [10:10] the list [10:11] zyga: and pi3 with core16 is the same as yours [10:11] I checked pi4 with core18 [10:11] no clue [10:11] zyga: iirc it's from our initrframfs hooks [10:11] mborzecki: hm, this is all confusing, it looks like /etc/machine-id is not in the writable-path [10:11] zyga: https://github.com/snapcore/core20/blob/master/static/etc/system-image/writable-paths [10:12] hmm - +0:+0 /system-data/etc/machine-id /etc/machine-id rw,relatime shared:+1 - ext4 /dev/sda3 rw,data=ordered [10:12] that's from our own test data [10:12] wat? [10:12] check this out [10:12] https://www.irccloud.com/pastebin/0OxSdK9y/ [10:12] mborzecki: so I remember that /var/lib/dbus/machine-id was the persistent place and an /etc/machine-id would make systemd fallback to the /var/lib/dbus/machine-id, maybe that changed? or maybe I misremember? [10:12] core 20 [10:13] it's good on core16 and core18 [10:13] mount-ns test is a bit like the os-release-zoo [10:13] anyway [10:13] brb [10:13] I'm so cold today [10:13] need to make that tea for real [10:13] core20 has different initramfs tho? [10:14] yeah [10:15] so in core build i see this: [10:15] 318: mount -o bind "${rootmnt}/writable/system-data/etc/machine-id" "${rootmnt}/etc/machine-id" [10:16] mborzecki: right, I think we need to add the machine-id to the writable-path, I wonder if that will work, i.e. if systemd will be able to commit when it can't write it atomitcally [10:16] but it's not used in core20 world anymore right? it's only ubuntu-core-initramfs now? [10:17] mborzecki: but the man-page for machine-id talks about bind-mounting it, so *hopefully* that will work [10:18] mborzecki: I think that's right, sounds like we need to sync with dimitri with it. but maybe it's enough to just add it to the writble-path? [10:24] back [10:24] much better :) [10:25] shall I report this just in case we don't fix it today? [10:26] I need a 2nd review on https://github.com/snapcore/snapd/pull/8403 to make progress please [10:26] PR #8403: sandbox/cgroup: avoid making arrays we don't use [10:26] I will adjust the comment on the next pass, this is green now [10:26] mborzecki: simple-session tool improvement https://github.com/snapcore/snapd/pull/8416 [10:26] PR #8416: tests/session-tool: reset failed session-tool units [10:27] this avoids the need to use property only available on future systemd versions [10:27] and solves accumulation of leftovers [10:27] PR snapd#8416 opened: tests/session-tool: reset failed session-tool units [10:27] mvo: Ubuntu strategy for machine-id is to be an empty file. Onboot, systemd maintains one in /run and eventually commits it to disk. So, yes it should be writable. And like Core20 snap must have it as an empty file. [10:27] mvo: bug report please. [10:28] xnox: on core20? [10:29] zyga i don't understand your question. the systemd behaviour has been like that since the dawn of time in the ubuntu timeline. [10:29] xnox: "do you want me to report a bug on core20" [10:30] zyga: yes [10:30] on it [10:31] xnox: https://github.com/snapcore/core20/issues/32 [10:31] Issue core20#32 opened: The machine-id file should be writable [10:32] tah [10:34] unit tests fail left and right, I guess travis containers are just as busy as launchpad lately [10:38] xnox: I can give you a PR if you want? or are you already on it? [10:38] mvo: no, i'm not on anything. not serving interrupts, but doing things as they were queued up for me. [10:42] ok [10:42] PR core20#33 opened: writable-path: make /etc/machine-id writable [10:46] mvo: let me try to build a core20 snap and see if it works as expected [10:52] mborzecki: \o/ [10:58] PR snapd#8412 closed: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling [11:03] hmm [11:03] /etc/machine-id /dev/mapper/ubuntu-data[/system-data/etc/machine-id] ext4 rw,relatime [11:03] /etc/machine-id tmpfs[/machine-id] tmpfs ro,size=203112k,mode=755 [11:03] why 2 mounts tho? [11:03] mvo: I opened 7700 for a review [11:04] mvo: can you please look at what we could do for the upcoming sprint to be in the green [11:04] mvo: it's a RFC, not a final solution, I could use advise on where to go next [11:04] ok, on the first boot i had 2 mounts, but now after another reboot there's just one [11:05] oh, wait, I pushed without commiting [11:06] anyway, it doesn't change much [11:06] I've restarted the CLA check a few times and it's failing [11:06] https://travis-ci.org/github/snapcore/snapd/builds/669948734 [11:07] yeah, looking good now [11:07] pstolowski: the persistent journal works now too [11:08] mborzecki: good, thanks. fwtw there is another issue with all this that I just described in my PR [11:08] mvo: ^ (sorry, I revoked your review) [11:09] pstolowski: hmm System Journal (/var/log/journal/bbac1383d2c640b3853c9f08d14e4116) is 32.0M, max 294.3M, 262.3M free, iirc he default is 10% of given block device size? [11:10] mborzecki: right, seems to match https://gist.github.com/JPvRiel/b7c185833da32631fa6ce65b40836887 [11:10] I believe reviewing https://github.com/snapcore/snapd/pull/8416 helps, I cannot explain how though [11:10] PR #8416: tests/session-tool: reset failed session-tool units [11:11] apart from having fewer units after this test [11:15] https://github.com/systemd/systemd/issues/8672 is related [11:15] zyga: it looks reasonable [11:15] mvo: what should we do to take next steps? [11:16] pstolowski: what does that mean, should I review the pr again? [11:17] zyga: what is needed, can we just open this PR as is? I think samuele had a question in there but otherwise I think I'm ok with this as a first step for the UI [11:17] mvo: the PR is open, I addressed that question now [11:17] zyga: cool [11:17] mvo: ok, I'll pursue this a little more then [11:17] zyga: has the prereq landed? [11:17] zyga: if so we could target it for 2.45 [11:17] mvo: close to it, if I can grab jamie for a moment today [11:18] mvo: I need to merge master after samuele's fixes again [11:18] nice [11:18] (the reorg) [11:18] mvo: no, please see the comment [11:18] mvo: we need to agree on what to do [11:19] pstolowski: I see now, thank you [11:20] pstolowski: this sounds to me like we want "yes/no/unset" and that maps to "persisent/disabled/auto", does that make sense? [11:20] (we want this as the possible config options?) [11:21] auto does not make that much sense on uc20 anymore though because something outside of the confinement would have to create it directly under /writable which is strange [11:21] otoh it's nice on the other releases :) [11:25] mvo: more less yes, maybe. but I fear this will be confusing. because if you say "unset" and it maps to "auto", that means that if it was "persisent" before it will still log into /var/log/journal (unless we or the user removes the dir). [11:26] pstolowski: good point [11:28] mvo: maybe we should simply override the default with persistent|none as soon as the option is ever set. but never remove /var/log/journal ? [11:29] * pstolowski lunch [11:29] mvo: pstolowski: let's chat later [11:31] PR snapcraft#3010 closed: cli: add progressive releases support to the release command [11:31] mvo: this fixes the cron issue with session-tool https://github.com/snapcore/snapd/pull/8417 [11:31] PR #8417: tests/session-tool: kill leaking closing session [11:31] PR snapd#8417 opened: tests/session-tool: kill leaking closing session [11:31] mborzecki: is [ ! -e /proc/$pid ]; a good way to check if a process exists? [11:45] thanks zyga [11:48] zyga: in shell? hmm why not, kill -0 probably doesn't work [11:48] kill 0 ofc [12:11] I've seen this error a lot this week [12:11] 2020-04-03 12:01:56 Error executing google:ubuntu-core-16-64:tests/core/reboot (apr031118-002068) : kill-timeout reached after apr031118-002068 (google:ubuntu-core-16-64:tests/core/reboot) reboot request [12:11] reboot test fails [12:12] I wonder if that's something else impacting or is that something broken for real [12:15] zyga: I re-reviewed #8123, not urgent by I left some questions and a remark there [12:15] looking [12:15] PR #8123: interfaces/network-control: bring /var/lib/dhcp from host (approach b) [12:16] pedronis: the hostfs change is not required because we reversed course on the ability to create directories that are absent on the host in response to interface connection [12:16] pedronis: so we will not create /var/lib/dhcp via hostfs [12:16] so no permissions or related cahnges [12:17] ah, makes sense [12:17] I'll see for the comment in a moment [12:17] wrapping up some branches and I'd like to break before the call [12:17] thank you for the review, I think this will land soon :) [12:20] * zyga afk for some time, see you at the standup [12:37] cachio: I'm trying to run the encryption test on the nested backend, but got a git archive error, did you see this before? [12:40] ijohnson: good morning [12:40] ijohnson: I have a question about InitramfsUbuntuDataDir and friends [12:42] ijohnson: I see you moved the definitions from dirs to boot, but I need to reference the mountpoints from the target system and not initramfs. They're using the same path but they could potentially be different, so my feeling is that I should define UbuntuDataDir in dirs again [12:47] cachio: never mind, it was a local problem [12:47] hey cmatsuoka [12:48] cmatsuoka: for now it's probably fine to just use boot.InitramfsUbuntuDataDir from wherever you need it, even if it's not during the initramfs [12:48] cmatsuoka: we had this same issue with pedronis and mborzecki for the code that reboots into a particular system for the chooser [12:48] cmatsuoka: so the variable may get renamed eventually but for now it's fine to use it [12:48] hm hm? [12:48] ijohnson: right, that's how I implemented it so I'll leave it as is for now, thanks [12:48] hey mborzecki [12:48] ah right [12:49] crazy idea, hm maybe we could have automount units for that? [12:54] mborzecki: for what? [12:54] /run/mnt/ubuntu-{seed,boot,data} [12:55] I am a bit missing how that would help? [12:56] pedronis: the mounts wouldn't need to be present all the time, we wouldn't need to call mount if we decide to mount them on deman [12:56] cmatsuoka: looks like 8103 is super close :) do you think you could address the remaining comments from maciej and chris? I think then it's ready to land [12:57] mborzecki: well we need them during initrams [12:57] the question is what happens later [12:57] pedronis: right, but we don't need them all the time later [12:57] mborzecki: who would write the units? [12:57] mvo: sure, working on that! [12:57] later == after switching the root [12:57] there's no main partition after a while [12:57] *after a while [12:57] for a while [12:58] pedronis: iirc xnox had code that generates mount units for those in ubuntu-core-initramfs [12:58] cmatsuoka: thanks! [12:59] mborzecki: there are issues, if they can come and go, any checks we do on them is problematic [12:59] yeah, that's true as well [13:00] anyway at this point is all things to think about after beta [14:13] * zyga breaks to stretch a little [14:13] back hurts [14:13] ouch :( [14:16] ijohnson: I'm not sure I understand your comment about the switch [14:16] I'm missing something [14:16] uh [14:17] pedronis: how can the caller set something on bsmark? what I did just now is have the caller pass in bsmark to the select function [14:17] here let me show you [14:17] ijohnson: we return bsmark [14:17] no? [14:18] yes, so how would the caller set properties on bsmark before calling the function ? [14:18] without passing in bsmark to the choose function ? [14:18] pedronis: https://pastebin.ubuntu.com/p/pDTXqbBsc4/ [14:18] it can set after [14:19] the info is not really used inside the function [14:19] afaict [14:19] it's used in commit later I suppose [14:20] pedronis: the issue that all of this refactor is about is preventing us from loading bootenv multiple times i.e. calling ebl.Kernel() multiple times during a single instance of boot.MarkBootSuccessful [14:20] we only want to call loadBootenv once for boot.MarkBootSuccessful [14:20] ijohnson: I understand, but maybe at this point is just easier for me to try what I have mind [14:20] and see if it works [14:20] I'm failing to explain myself it seems [14:21] yes, sorry I don't understand your suggestion [14:21] it's very simple actually [14:21] but maybe doesn't work [14:21] let me see [14:25] ijohnson: it works afaict [14:26] pedronis: can you show your code ? [14:26] at least the tests still pass [14:28] ijohnson: just this: https://paste.ubuntu.com/p/shzz4xgN3w/ [14:30] ah okay sure that works fine [14:31] sorry I didn't understand that your suggestion was to do it after the choose* function [14:32] well, I said it on irc, but not in the comment, it's actually the only way to make it work simply [14:32] but as I commented the doc comment of the helper needs to clarify that it's possible/makes sense [14:34] yes that's fine [14:36] PR snapd#8368 closed: github: goodbye travis [14:40] PR snapd#8418 opened: github: move static checks and spread over [14:47] ijohnson: the problem with non managers_test.go tests is that they don't check the logic triggered by ensureBootOk and the rest of the refresh undo [14:48] but maybe you have thoughts about that? [14:48] pedronis: what other logic do we need to check in ensureBootOk ? [14:48] ijohnson: we trigger a revert that will do a SetNextBoot , same with the undo [14:48] that's where things intersect [14:49] mmm [14:49] this is slightly depressing that we need so many permutations of this kind of test now across 3 different packages :-/ [14:50] ijohnson: I'm open to think this through just saying that the chain of boot state changes [14:50] ends with a SetNextBoot in the undo code [14:50] yes you're right there are more cases like that [14:50] in some cases [14:51] that's I think why we thought some cases would take care by themselves [14:51]