[00:56] <mup> PR snapcraft#2916 closed: status: implement using the new channel-map endpoint <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/2916>
[01:53] <mup> PR snapcraft#2992 closed: [WIP] repo: ubuntu implementation refactoring with initial package-management <Created by cjp256> <Closed by cjp256> <https://github.com/snapcore/snapcraft/pull/2992>
[03:11] <mup> PR snapcraft#3006 closed: repo: always use host release and arch for Ubuntu <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3006>
[05:34] <mborzecki> morning
[06:13] <mborzecki> mvo: hey
[06:20] <mvo> mborzecki: good morning
[06:29] <mup> PR snapd#8410 closed: many: disentagle release and snapdenv from sandbox/* <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8410>
[06:31] <mup> PR snapd#8412 opened: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8412>
[06:34] <mup> PR snapd#8404 closed: client: increase timeout in client tests to 100ms <Simple 😃> <Created by mvo5> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8404>
[07:03] <pstolowski> morning
[07:05] <mvo> good morning pstolowski
[07:05] <zyga> good morning
[07:05] <zyga> hey mvo,
[07:06] <zyga> mvo: I noticed we have a small problem in debian
[07:06] <zyga> mvo: snapd doesn't build
[07:06] <zyga> dh_missing: warning: usr/bin/snap-preseed exists in debian/tmp but is not installed to anywhere
[07:06] <zyga> I think we have different ruleset there, and leftover binaries cause builds to fail
[07:06] <zyga> I'll see if I can use my shiny metal rights and actually help
[07:07] <jamesh> Had another CI run hit Travis's 50 minute limit: https://travis-ci.org/github/snapcore/snapd/builds/670392870
[07:07] <zyga> jamesh: I'll move travis jobs to actions today
[07:07] <zyga> I think it's time
[07:14] <mvo> good morning zyga
[07:15] <mvo> 8103 needs a second review, it's important for the encryption story and to unblock the TPM work
[07:16] <mvo> jamesh: thanks for letting us know, once we are slightly less busy we move to gh actions to get rid of this
[07:17] <mvo> 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] <mvo> (fwiw 8103 is not very long, hopefully quick review)
[07:24] <mup> PR snapd#8413 opened: interfaces: don't use the owner modifier for files shared via document portal <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/8413>
[07:27] <mvo> 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] <jamesh> 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] <mvo> zyga: session-tool fails on master https://api.travis-ci.org/v3/job/670441262/log.txt - could you please ahve a look?
[07:31] <zyga> Yes
[07:44] <mup> PR snapd#8406 closed: usersession: extend timerange in TestExitOnIdle <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8406>
[07:45] <jamesh> pedronis_: I think the extra spread test I added covers what you wanted in my user daemons PR
[07:46] <pedronis> jamesh: thanks
[08:17] <zyga> pedronis: thank you for the move yesterday!
[08:41] <mup> PR snapd#8414 opened: o/configstate: core config handler for persistent journal <Created by stolowski> <https://github.com/snapcore/snapd/pull/8414>
[08:41] <pstolowski> ogra: hi, this might be interesting for you ^ (feedback is welcome)
[08:54] <ogra> nice !
[08:54] <ogra> pstolowski, you are aware that etc/systemd/journald.conf.d/ is complete nonsense ?
[08:55] <pstolowski> ogra: heh, not, why?
[08:55] <ogra> pstolowski, all you need to do is to create the dir ...
[08:55] <pstolowski> ogra: you mean /var/log/journal ?
[08:55] <ogra> 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] <ogra> yeah
[08:56] <pstolowski> huh
[08:56] <ogra> i'D just make the code call a mkdir and be done ;)
[08:56] <pstolowski> mhm
[08:57] <ogra> (and rmdir if the option is unset)
[08:57] <pstolowski> ondra: is this consistent across systemd versions (core16, core18...)?
[08:57] <pstolowski> sorry, ogra ^
[08:57] <ogra> notsure what ondra thinks ... but it surely is for 16 and 18
[08:57] <ogra> :)
[08:58] <ogra> 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] <ogra> !
[08:59] <pstolowski> ogra: ok, that's interesting, thank you!
[09:00] <ogra> thank *you* for implementing it !!
[09:00] <ogra> :D
[09:03] <pstolowski> 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] <ogra> yep :)
[09:04] <pstolowski> ogra: thanks again for the enlightenment ;)
[09:17] <zyga> pstolowski, ogra: related review https://github.com/snapcore/snapd/pull/8414#pullrequestreview-387089711
[09:17] <mup> PR #8414: o/configstate: core config handler for persistent journal <⛔ Blocked> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8414>
[09:18] <pstolowski> zyga: thanks. we might not need the conf file at all, i'm going to verify this
[09:18] <zyga> thanks
[09:18] <zyga> ping me for a re-review please
[09:36] <mup> PR snapd#8415 opened: cmd/snap-recovery-chooser: tweaks <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8415>
[09:45] <pedronis> mborzecki: I nitpicked a bit in #8415
[09:45] <mup> PR #8415: cmd/snap-recovery-chooser: tweaks <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8415>
[09:51] <zyga> mborzecki: https://github.com/snapcore/snapd/pull/8415#pullrequestreview-387112832
[09:51] <mup> PR #8415: cmd/snap-recovery-chooser: tweaks <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8415>
[09:52] <mborzecki> pedronis: zyga: thanks
[09:53] <zyga> I'll be back shortly, it's quite cold in the office and ought to make some warm tea
[10:00] <zyga> mborzecki: unit tests failed on that PR
[10:00] <mborzecki> waat?
[10:00] <zyga> https://www.irccloud.com/pastebin/NkNEOsNj/
[10:00] <zyga> not yours but meh
[10:00] <zyga> mvo: ^
[10:00] <zyga> is that another thing to adjust?
[10:01] <mborzecki> weird, that timeout is longer already
[10:01] <mborzecki> maybe a bug in the test/code?
[10:02] <mvo> zyga, mborzecki I thought I did push something there?
[10:02] <zyga> ah, perhaps maciek's branch is simply older
[10:02] <mvo> https://github.com/snapcore/snapd/pull/8412
[10:02] <mup> PR #8412: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8412>
[10:02] <mborzecki> mvo: yes, https://github.com/snapcore/snapd/pull/8404 has landed already
[10:02] <mup> PR #8404: client: increase timeout in client tests to 100ms <Simple 😃> <Created by mvo5> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8404>
[10:03] <mvo> mborzecki: this one is different, no? that's what 8412 is for
[10:03] <mvo> mborzecki: or am I misreading this?
[10:03] <mborzecki> 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] <mborzecki> oh, w8, it is
[10:03] <mborzecki> so why did i get a new machine-id again?
[10:03] <pstolowski> mborzecki: works as expected on core18
[10:04] <pstolowski> (i rebooted)
[10:05] <mborzecki> hmmmm ├─/etc/machine-id  tmpfs[/machine-id]   tmpfs      ro,size=203112k,mode=75
[10:05] <zyga> yeah
[10:05] <mborzecki> pstolowski: mvo: ^^
[10:05] <zyga> why do we have machine-id on a tmpfs
[10:05] <zyga> mborzecki: we noticed this a while ago
[10:05] <zyga> in the review of mount-ns data
[10:05] <zyga> I guess we forgot to act
[10:06] <mborzecki> pstolowski: do you hav a core18 vm? can you check `findmnt /etc/machine-id` ?
[10:06] <pstolowski> mborzecki: yep, 1 sec
[10:06] <zyga> mborzecki: I do
[10:07] <zyga>  /etc/machine-id /dev/sda3[/system-data/etc/machine-id] ext4   rw,relatime,data=o
[10:07] <zyga> so that's weird
[10:08] <zyga> mborzecki: from pi4 /etc/machine-id /dev/mmcblk0p2[/system-data/etc/machine-id] ext4   rw,relatime
[10:08] <zyga> mystery
[10:08] <mborzecki> hm it's not listed in writable-paths, what sets it up?
[10:08] <zyga> maybe that's our test specific setup
[10:09] <mborzecki> zyga: anything in /etc/fstab or /run/image.fstab?
[10:10] <pstolowski> zyga: i see  /etc/machine-id /dev/sda3[/system-data/etc/machine-id] ext4 as well
[10:10] <zyga> what is /run/image.fstab?
[10:10] <zyga> where are writable paths set up
[10:10] <zyga> I mean
[10:10] <zyga> the list
[10:11] <pstolowski> zyga: and pi3 with core16 is the same as yours
[10:11] <zyga> I checked pi4 with core18
[10:11] <zyga> no clue
[10:11] <mborzecki> zyga: iirc it's from our initrframfs hooks
[10:11] <mvo> mborzecki: hm, this is all confusing, it looks like /etc/machine-id is not in the writable-path
[10:11] <mborzecki> zyga: https://github.com/snapcore/core20/blob/master/static/etc/system-image/writable-paths
[10:12] <zyga> hmm - +0:+0 /system-data/etc/machine-id /etc/machine-id rw,relatime shared:+1 - ext4 /dev/sda3 rw,data=ordered
[10:12] <zyga> that's from our own test data
[10:12] <zyga> wat?
[10:12] <zyga> check this out
[10:12] <zyga> https://www.irccloud.com/pastebin/0OxSdK9y/
[10:12] <mvo> 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] <zyga> core 20
[10:13] <zyga> it's good on core16 and core18
[10:13] <zyga> mount-ns test is a bit like the os-release-zoo
[10:13] <zyga> anyway
[10:13] <zyga> brb
[10:13] <zyga> I'm so cold today
[10:13] <zyga> need to make that tea for real
[10:13] <mborzecki> core20 has different initramfs tho?
[10:14] <mvo> yeah
[10:15] <mborzecki> so in core build i see this:
[10:15] <mborzecki> 318:    mount -o bind "${rootmnt}/writable/system-data/etc/machine-id" "${rootmnt}/etc/machine-id"
[10:16] <mvo> 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] <mborzecki> but it's not used in core20 world anymore right? it's only ubuntu-core-initramfs now?
[10:17] <mvo> mborzecki: but the man-page for machine-id talks about bind-mounting it, so *hopefully* that will work
[10:18] <mvo> 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] <zyga> back
[10:24] <zyga> much better :)
[10:25] <zyga> shall I report this just in case we don't fix it today?
[10:26] <zyga> I need a 2nd review on https://github.com/snapcore/snapd/pull/8403 to make progress please
[10:26] <mup> PR #8403: sandbox/cgroup: avoid making arrays we don't use <Skip spread> <Created by zyga> <https://github.com/snapcore/snapd/pull/8403>
[10:26] <zyga> I will adjust the comment on the next pass, this is green now
[10:26] <zyga> mborzecki: simple-session tool improvement https://github.com/snapcore/snapd/pull/8416
[10:26] <mup> PR #8416: tests/session-tool: reset failed session-tool units <Created by zyga> <https://github.com/snapcore/snapd/pull/8416>
[10:27] <zyga> this avoids the need to use property only available on future systemd versions
[10:27] <zyga> and solves accumulation of leftovers
[10:27] <mup> PR snapd#8416 opened: tests/session-tool: reset failed session-tool units <Created by zyga> <https://github.com/snapcore/snapd/pull/8416>
[10:27] <xnox> 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] <xnox> mvo: bug report please.
[10:28] <zyga> xnox: on core20?
[10:29] <xnox>  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] <zyga> xnox: "do you want me to report a bug on core20"
[10:30] <xnox> zyga:  yes
[10:30] <zyga> on it
[10:31] <zyga> xnox: https://github.com/snapcore/core20/issues/32
[10:31] <mup> Issue core20#32 opened: The machine-id file should be writable <Created by zyga> <https://github.com/snapcore/core20/issue/32>
[10:32] <xnox> tah
[10:34] <zyga> unit tests fail left and right, I guess travis containers are just as busy as launchpad lately
[10:38] <mvo> xnox: I can give you a PR if you want? or are you already on it?
[10:38] <xnox> mvo:  no, i'm not on anything. not serving interrupts, but doing things as they were queued up for me.
[10:42] <mvo> ok
[10:42] <mup> PR core20#33 opened: writable-path: make /etc/machine-id writable <Created by mvo5> <https://github.com/snapcore/core20/pull/33>
[10:46] <mborzecki> mvo: let me try to build a core20 snap and see if it works as expected
[10:52] <mvo> mborzecki: \o/
[10:58] <mup> PR snapd#8412 closed: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling <Simple 😃> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8412>
[11:03] <mborzecki> hmm
[11:03] <mborzecki> /etc/machine-id /dev/mapper/ubuntu-data[/system-data/etc/machine-id] ext4   rw,relatime
[11:03] <mborzecki> /etc/machine-id tmpfs[/machine-id]                                   tmpfs  ro,size=203112k,mode=755
[11:03] <mborzecki> why 2 mounts tho?
[11:03] <zyga> mvo: I opened 7700 for a review
[11:04] <zyga> mvo: can you please look at what we could do for the upcoming sprint to be in the green
[11:04] <zyga> mvo: it's a RFC, not a final solution, I could use advise on where to go next
[11:04] <mborzecki> ok, on the first boot i had 2 mounts, but now after another reboot there's just one
[11:05] <zyga> oh, wait, I pushed without commiting
[11:06] <zyga> anyway, it doesn't change much
[11:06] <zyga> I've restarted the CLA check a few times and it's failing
[11:06] <zyga> https://travis-ci.org/github/snapcore/snapd/builds/669948734
[11:07] <mborzecki> yeah, looking good now
[11:07] <mborzecki> pstolowski: the persistent journal works now too
[11:08] <pstolowski> mborzecki: good, thanks. fwtw there is another issue with all this that I just described in my PR
[11:08] <pstolowski> mvo: ^ (sorry, I revoked your review)
[11:09] <mborzecki> 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] <pstolowski> mborzecki: right, seems to match https://gist.github.com/JPvRiel/b7c185833da32631fa6ce65b40836887
[11:10] <zyga> I believe reviewing https://github.com/snapcore/snapd/pull/8416 helps, I cannot explain how though
[11:10] <mup> PR #8416: tests/session-tool: reset failed session-tool units <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8416>
[11:11] <zyga> apart from having fewer units after this test
[11:15] <zyga> https://github.com/systemd/systemd/issues/8672 is related
[11:15] <mvo> zyga: it looks reasonable
[11:15] <zyga> mvo: what should we do to take next steps?
[11:16] <mvo> pstolowski: what does that mean, should I review the pr again?
[11:17] <mvo> 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] <zyga> mvo: the PR is open, I addressed that question now
[11:17] <mvo> zyga: cool
[11:17] <zyga> mvo: ok, I'll pursue this a little more then
[11:17] <mvo> zyga: has the prereq landed?
[11:17] <mvo> zyga: if so we could target it for 2.45
[11:17] <zyga> mvo: close to it, if I can grab jamie for a moment today
[11:18] <zyga> mvo: I need to merge master after samuele's fixes again
[11:18] <mvo> nice
[11:18] <zyga> (the reorg)
[11:18] <pstolowski> mvo: no, please see the comment
[11:18] <pstolowski> mvo: we need to agree on what to do
[11:19] <mvo> pstolowski: I see now, thank you
[11:20] <mvo> pstolowski: this sounds to me like we want "yes/no/unset" and that maps to "persisent/disabled/auto", does that make sense?
[11:20] <mvo> (we want this as the possible config options?)
[11:21] <mvo> 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] <mvo> otoh it's nice on the other releases :)
[11:25] <pstolowski> 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] <mvo> pstolowski: good point
[11:28] <pstolowski> 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] <pedronis> mvo: pstolowski: let's chat later
[11:31] <mup> PR snapcraft#3010 closed: cli: add progressive releases support to the release command <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3010>
[11:31] <zyga> mvo: this fixes the cron issue with session-tool https://github.com/snapcore/snapd/pull/8417
[11:31] <mup> PR #8417: tests/session-tool: kill leaking closing session <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/8417>
[11:31] <mup> PR snapd#8417 opened: tests/session-tool: kill leaking closing session <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/8417>
[11:31] <zyga> mborzecki: is [ ! -e /proc/$pid ]; a good way to check if a process exists?
[11:45] <mvo> thanks zyga
[11:48] <mborzecki> zyga: in shell? hmm why not, kill -0 <pid> probably doesn't work
[11:48] <mborzecki> kill 0 ofc
[12:11] <zyga> I've seen this error a lot this week
[12:11] <zyga> 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] <zyga> reboot test fails
[12:12] <zyga> I wonder if that's something else impacting or is that something broken for real
[12:15] <pedronis> zyga: I re-reviewed #8123, not urgent by I left some questions and a remark there
[12:15] <zyga> looking
[12:15] <mup> PR #8123: interfaces/network-control: bring /var/lib/dhcp from host (approach b) <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/8123>
[12:16] <zyga> 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] <zyga> pedronis: so we will not create /var/lib/dhcp via hostfs
[12:16] <zyga> so no permissions or related cahnges
[12:17] <pedronis> ah, makes sense
[12:17] <zyga> I'll see for the comment in a moment
[12:17] <zyga> wrapping up some branches and I'd like to break before the call
[12:17] <zyga> 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] <cmatsuoka> 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] <cmatsuoka> ijohnson: good morning
[12:40] <cmatsuoka> ijohnson: I have a question about InitramfsUbuntuDataDir and friends
[12:42] <cmatsuoka> 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] <cmatsuoka> cachio: never mind, it was a local problem
[12:47] <ijohnson> hey cmatsuoka
[12:48] <ijohnson> 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] <ijohnson> cmatsuoka: we had this same issue with pedronis and mborzecki for the code that reboots into a particular system for the chooser
[12:48] <ijohnson> cmatsuoka: so the variable may get renamed eventually but for now it's fine to use it
[12:48] <mborzecki> hm hm?
[12:48] <cmatsuoka> ijohnson: right, that's how I implemented it so I'll leave it as is for now, thanks
[12:48] <ijohnson> hey mborzecki
[12:48] <mborzecki> ah right
[12:49] <mborzecki> crazy idea, hm maybe we could have automount units for that?
[12:54] <zyga> mborzecki: for what?
[12:54] <mborzecki> /run/mnt/ubuntu-{seed,boot,data}
[12:55] <pedronis> I am a bit missing how that would help?
[12:56] <mborzecki> 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] <mvo> 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] <pedronis> mborzecki: well we need them during initrams
[12:57] <pedronis> the question is what happens later
[12:57] <mborzecki> pedronis: right, but we don't need them all the time later
[12:57] <pedronis> mborzecki: who would write the units?
[12:57] <cmatsuoka> mvo: sure, working on that!
[12:57] <mborzecki> later == after switching the root
[12:57] <pedronis> there's no main partition after a while
[12:57] <pedronis> *after a while
[12:57] <pedronis> for a while
[12:58] <mborzecki> pedronis: iirc xnox had code that generates mount units for those in ubuntu-core-initramfs
[12:58] <mvo> cmatsuoka: thanks!
[12:59] <pedronis> mborzecki: there are issues, if they can come and go, any checks we do on them is problematic
[12:59] <mborzecki> yeah, that's true as well
[13:00] <pedronis> anyway at this point is all things to think about after beta
[14:13]  * zyga breaks to stretch a little
[14:13] <zyga> back hurts
[14:13] <roadmr> ouch :(
[14:16] <pedronis> ijohnson: I'm not sure I understand your comment about the switch
[14:16] <pedronis> I'm missing something
[14:16] <ijohnson> uh
[14:17] <ijohnson> 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] <ijohnson> here let me show you
[14:17] <pedronis> ijohnson: we return bsmark
[14:17] <pedronis> no?
[14:18] <ijohnson> yes, so how would the caller set properties on bsmark before calling the function ?
[14:18] <ijohnson> without passing in bsmark to the choose function ?
[14:18] <ijohnson> pedronis: https://pastebin.ubuntu.com/p/pDTXqbBsc4/
[14:18] <pedronis> it can set after
[14:19] <pedronis> the info is not really used inside the function
[14:19] <pedronis> afaict
[14:19] <pedronis> it's used in commit later I suppose
[14:20] <ijohnson> 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] <ijohnson> we only want to call loadBootenv once for boot.MarkBootSuccessful
[14:20] <pedronis> ijohnson: I understand, but maybe at this point is just easier for me to try what I have mind
[14:20] <pedronis> and see if it works
[14:20] <pedronis> I'm failing to explain myself it seems
[14:21] <ijohnson> yes, sorry I don't understand your suggestion
[14:21] <pedronis> it's very simple actually
[14:21] <pedronis> but maybe doesn't work
[14:21] <pedronis> let me see
[14:25] <pedronis> ijohnson: it works afaict
[14:26] <ijohnson> pedronis: can you show your code ?
[14:26] <pedronis> at least the tests still pass
[14:28] <pedronis> ijohnson: just this: https://paste.ubuntu.com/p/shzz4xgN3w/
[14:30] <ijohnson> ah okay sure that works fine
[14:31] <ijohnson> sorry I didn't understand that your suggestion was to do it after the choose* function
[14:32] <pedronis> well, I said it on irc, but not in the comment, it's actually the only way to make it work simply
[14:32] <pedronis> but as I commented the doc comment of the helper needs to clarify that it's possible/makes sense
[14:34] <ijohnson> yes that's fine
[14:36] <mup> PR snapd#8368 closed: github: goodbye travis <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/8368>
[14:40] <mup> PR snapd#8418 opened: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>
[14:47] <pedronis> 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] <pedronis> but maybe you have thoughts about that?
[14:48] <ijohnson> pedronis: what other logic do we need to check in ensureBootOk ?
[14:48] <pedronis> ijohnson: we trigger a revert that will do a SetNextBoot , same with the undo
[14:48] <pedronis> that's where things intersect
[14:49] <ijohnson> mmm
[14:49] <ijohnson> this is slightly depressing that we need so many permutations of this kind of test now across 3 different packages :-/
[14:50] <pedronis> ijohnson: I'm open to think this through just saying that the chain of boot state changes
[14:50] <pedronis> ends with a SetNextBoot in the undo code
[14:50] <ijohnson> yes you're right there are more cases like that
[14:50] <pedronis> in some cases
[14:51] <pedronis> that's I think why we thought some cases would take care by themselves
[14:51] <pedronis> btw
[14:51] <pedronis> this logic on the undo path
[14:51] <pedronis> of a refresh
[14:53] <ijohnson> yes
[14:57] <pedronis> ijohnson: related to that the other important moving part is: snapstate.WaitRestart
[14:59] <mup> PR core20#33 closed: writable-path: make /etc/machine-id writable <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/core20/pull/33>
[15:00] <ijohnson> mborzecki: I will push up what I have on writing grub.cfg on ubuntu-boot from snapd so you can pick it up on monday if you have time
[15:00] <ijohnson> pedronis: yes indeed it's unclear where all we are currently covering the calls to boot from WaitRestart
[15:01] <pedronis> ijohnson: some of the managers_test should reach that
[15:01] <zyga> degville: [offtopic] did you finish the game?
[15:02] <ijohnson> pedronis: yes but I don't know if we have enough coverage of it is my point
[15:02] <mborzecki> ijohnson: going out now, please ping in the review
[15:02] <ijohnson> mborzecki: ack
[15:02] <ijohnson> have a nice weekend
[15:02] <mborzecki> ijohnson: thanks, you too!
[15:02] <degville> zyga: no, not yet. I've really been taking it slow, and playing it only on difficult, because I know it's not long.
[15:03] <zyga> degville: I'm very curious what's at the end
[15:03] <zyga> HL3 confirmed or what :D ?
[15:03] <zyga> but don't tell me
[15:03] <zyga> just tell me if it's worth that 1000 _once you know the ending_
[15:03] <degville> zyga: yeah, I've tried to skip all discussions about it so I don't know either.
[15:05] <pedronis> ijohnson: it's main API, is GetCurrentBoot so maybe related to looking more at state, what we need is all the state about this boot env state transitions to end up checking what that does return
[15:05] <degville> [off topic, cont.] I don't think there's any way it can warrant 1000 on its own. Despite being very good, it's an experiment in immersion more than anything. Which I think is the best VR can offer at the moment. But it is seeing City 17 from your own eyes, and totally convincing.
[15:08] <mup> PR snapd#8416 closed: tests/session-tool: reset failed session-tool units <Simple 😃> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8416>
[15:10] <mup> PR snapd#8419 opened: Add libnvidia-rtcore as Nvidia library <Created by joedborg> <https://github.com/snapcore/snapd/pull/8419>
[15:10] <pedronis> ijohnson: the main chain  I think is always something like:  SetNextBoot -> bootloader cfg -> initramfs -> MarkBootSuccessful -> GetCurrentBoot (-> undo or revert SetNextBoot)
[15:11] <pedronis> ijohnson: that should be coverable from boot mostly, with a few managers_test on top to check actual integration
[15:12] <zyga> brb
[15:15] <pedronis> ijohnson: so maybe the first step is actually to add checks for GetCurrentBoot results in the significant places in all the TestHappy in boot_test.go
[15:18] <mup> PR snapd#8420 opened: httputil: increase testRetryStrategy max timelimit to 5s <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8420>
[15:27] <zyga> re
[15:35] <mvo> can someone do a second review for 8103 please?
[15:36] <zyga> mvo: I'm on it
[15:36] <zyga> mid way
[15:36] <mvo> zyga: \o/
[15:36] <mvo> zyga: thank you
[15:38] <mup> PR snapd#8419 closed: Add libnvidia-rtcore as Nvidia library <Created by joedborg> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8419>
[15:44] <mup> PR snapd#8329 closed: interfaces: allow raw access to USB printers <Created by ogra1> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8329>
[15:46] <mup> PR snapcraft#2987 closed: spread tests: set appropriate default base in snapcraft.yamls <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2987>
[15:53] <zyga> 7/9 files
[15:55] <zyga> https://github.com/snapcore/snapd/pull/8103#pullrequestreview-387367998
[15:55] <mup> PR #8103: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8103>
[15:55] <zyga> I can +1 if somone will follow up
[15:56] <zyga> would +1 except for https://github.com/snapcore/snapd/pull/8103#discussion_r403103510
[15:56] <zyga> mvo: ^
[16:05] <ijohnson> pedronis: sorry was in meetings
[16:05] <ijohnson> pedronis: how much of these additional tests do you want in the open PR?
[16:06] <pedronis> ijohnson: should we have a quick HO?
[16:06] <ijohnson> pedronis: sure
[16:06] <ijohnson> SU?
[16:06] <pedronis> yes
[16:07] <ijohnson> cool I'm there
[16:08] <cmatsuoka> zyga: addressing that
[16:08] <zyga> thanks
[16:08] <zyga> I can +1 if you do
[16:12] <cmatsuoka> zyga: test "$(stat --printf='%u %a' /var/lib/snapd/device/fde/recovery-key)" = "0 600"
[16:12] <zyga> on focal this is my path
[16:12] <zyga>  /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/snap/bin:/var/lib/snapd/snap/bin
[16:12] <zyga> note the duplicate /snap/bin
[16:12] <zyga> and the non-ubuntu /var/lib/snapd/snap/bin
[16:12] <zyga> can anyone confirm?
[16:22] <ijohnson> zyga: I have the same
[16:22] <zyga> thanks
[16:22] <zyga> probably one-too-many helpers
[16:23] <ijohnson> to be clear, I have the duplicated /snap/bin and non-ubuntu /var/lib/snapd/snap/bin too
[16:23] <zyga> ack
[16:23] <pedronis> it's the same on bionic fwiw
[16:25] <zyga> fun
[16:25] <zyga> we must have done that earlier
[16:25] <zyga> I'll look on Monday
[16:25] <zyga> or maybe earlier
[16:25] <zyga> in our job interviews we should only have one question
[16:26] <zyga> How do you add one item to PATH
[16:26] <zyga> that will filter out everyone, including us
[16:26] <zyga> ;)
[16:26] <ijohnson> hooray
[16:33] <zyga> filed it just in case
[16:39] <zyga> mvo, cmatsuoka: https://github.com/snapcore/snapd/pull/8103#pullrequestreview-387413837
[16:39] <mup> PR #8103: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8103>
[16:49] <cmatsuoka> zyga: fixed as suggested, thanks
[16:57] <zyga> https://github.com/snapcore/snapd/pull/8418/checks#step:8:9
[16:57] <mup> PR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>
[16:57] <zyga> govendor cannot build on go from xenial?
[17:05] <zyga> mvo: do you remember why we depend on 1.9 and not 1.10?
[17:10] <mvo> zyga: centos 7 iirc
[17:10] <zyga> I see
[17:10] <zyga> oh well
[17:10] <zyga> I'll do something
[17:10] <zyga> I solved the silly go in azure
[17:11] <zyga> they pre populate GIGs of software into the image
[17:11] <mvo> zyga: if we run a spread unittest task everywhere I think we are also good
[17:11] <zyga> including go 1.11+
[17:11] <zyga> I think we don't but I'll check
[17:11] <mvo> zyga, cmatsuoka thanks for working on 8103, looks like it's ready to go once tests have run
[17:11] <mvo> zyga: thanks!
[17:12] <zyga> heh
[17:12] <zyga> xenial cannot run our static checks
[17:12] <zyga> it's like moving in a maze
[17:12] <zyga> without lights
[17:12] <zyga> and knowing there are turds everywhere
[17:12] <mvo> zyga: xenial cannot run our static checks? why not?
[17:12] <zyga> https://github.com/snapcore/snapd/pull/8418/checks#step:10:18
[17:13] <mup> PR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>
[17:13] <zyga> because shellcheck in xenial is too old
[17:13] <zyga> we never noticed
[17:15] <mvo> zyga: woah, interessting
[17:15] <mvo> zyga: and sad
[17:15] <mvo> zyga: I thought travis would run those in xenial too - apparently not
[17:15] <zyga> everyone does custom shit
[17:15] <zyga> at least
[17:16] <zyga> and I really have to give them credit
[17:16] <zyga> github has excellent docs on what they do
[17:16] <zyga> and what's custom
[17:17]  * mvo nods
[17:17] <zyga> I wonder...
[17:18] <zyga> (insert harry potter mystery music)
[17:19] <zyga> mvo: check out https://github.com/snapcore/snapd/pull/8418/checks
[17:19] <mup> PR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>
[17:19] <zyga> wow :)
[17:20] <mup> PR snapcraft#3011 opened: repo: introduce install_source() and install_gpg_key() to Ubuntu <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3011>
[17:20] <zyga> mvo: snapd work in azure
[17:20] <mvo> zyga: nice
[17:20] <zyga> *snaps
[17:20] <mvo> zyga: looking
[17:20] <zyga> so shellcheck snap
[17:20] <mvo> zyga: yay
[17:20] <ijohnson> zyga: is it still a problem where "/" is owned by the vsts user or something
[17:21] <zyga> ?
[17:21] <zyga> vsts?
[17:21] <zyga> test
[17:21] <zyga> yes
[17:21] <zyga> snap-confine will refuse to work
[17:21] <ijohnson> :-(
[17:21] <ijohnson> but I found someone to talk to about that on MS side
[17:21] <zyga> can I help?
[17:22] <ijohnson> I can CC you on my email
[17:22] <ijohnson> the problem before was that we couldn't find anyone to talk to about who manages this on the github/microsoft side
[17:24] <mvo> zyga: fun (not!) - the travis run for 8103 has not even started yet
[17:25] <zyga> heh
[17:33] <pedronis> I pushed a comment there,  might need formatting in a follow up
[17:33] <zyga> it seems master is broken
[17:33] <zyga> https://api.travis-ci.org/v3/job/670608294/log.txt
[17:33] <zyga>     - google:ubuntu-core-20-64:tests/core/writablepaths
[17:33] <pedronis> https://github.com/snapcore/snapd/pull/8103/files#diff-13724d8f1f88168c790f24aafb4d2d13R137
[17:33] <mup> PR #8103: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8103>
[17:33] <zyga> https://www.irccloud.com/pastebin/5FyicYVN/
[17:36] <mup> PR snapd#8421 opened: tests: enable unit tests on debian-sid again <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8421>
[17:39] <zyga> cachio: could you please review https://github.com/snapcore/snapd/pull/8418
[17:39] <mup> PR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>
[17:40] <cachio> zyga, sure
[17:40] <zyga> thanks!
[17:40] <zyga> this will unlock more chance to improve things
[17:40] <zyga> accelerate team-wide feedback
[17:41] <zyga> and get rid of the 50 minute limit
[17:47] <cachio> zyga, where are these going to run?
[17:47] <cachio> your box?
[17:47] <zyga> no
[17:47] <zyga> those run in azure
[17:47] <zyga> in the normal gh runners
[17:47] <cachio> ahh
[17:47] <zyga> btw, mvo: I got 16GB of ram into my box so we could easily run those 32 runners for extra overflow capacity
[17:48] <zyga> there's some cruft to shed
[17:48] <zyga> mainly to make it faster
[17:48] <zyga> but that's all _after_ this one
[18:08] <mup> PR snapcraft#3012 opened: plugins: install required apt sources and keys to system <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3012>
[18:13] <cachio> zyga, left you some questions in the PR
[18:14] <cachio> and closed it by mistake
[18:14] <cachio> then I reopened it
[18:16] <zyga> cachio: :D
[18:16] <zyga> cachio: I replied
[18:18] <zyga> cachio: remember that this is a long term game:
[18:18] <zyga> cachio: we get instant responsiveness
[18:18] <zyga> cachio: we get faster end-to-end
[18:18] <zyga> cachio: we get lower costs
[18:18] <zyga> cachio: if we cut rebuilds this is even faster
[18:18] <zyga> cachio: essentially boot into a system and start away
[18:19] <cachio> I think it is ok with this new workflow and if someone complains in the future we can revert to the run all the systems+ in parallel
[18:19] <zyga> yeah
[18:19] <zyga> I think this will be faster than old workflow already
[18:19] <cachio> the good part is that if some system fails, we just restart this one
[18:19] <zyga> we...
[18:19] <zyga> not yet
[18:19] <zyga> github has this but it's not released yet
[18:20] <cachio> ahh
[18:20] <cachio> well in the future well save that money too
[18:21] <cachio> +1
[18:21] <zyga> thanks!
[18:21] <zyga> let's improve it together
[18:23] <cachio> so far it worked very well
[18:23] <cachio> that's really possitive
[18:28] <mup> PR snapd#8422 opened: tests: skip "/etc/machine-id" in "writablepaths" test <Simple 😃> <⚠ Critical> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8422>
[18:29] <mvo> zyga: ^- this will unbreak master
[18:29] <zyga> ta
[18:29] <zyga> looking
[18:29] <mup> PR snapd#8417 closed: tests/session-tool: kill leaking closing session <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8417>
[18:30] <zyga> hmm
[18:31] <zyga> core20 tests
[18:31] <zyga> when do we run them
[18:31] <zyga> in first boot?
[18:34] <ijohnson> zyga: we run core20 tests during run mode after we reboot
[18:35] <zyga> mvo: ^
[18:35] <zyga> is this consistent with your patch?
[18:37] <ijohnson> zyga: yes "first boot" for uc20 -> first boot run mode
[18:37] <zyga> ahhh
[18:37] <ijohnson> there's also install mode, which is technically the "first" boot, but we don't call it that
[18:37] <ijohnson> install mode boot is more like the zeroth boot
[18:40] <mvo> zyga: yes
[18:40] <mvo> zyga: it's what maciej also saw, next reboot is fine for strange reasons
[18:41] <mvo> zyga: I think it happens when the /etc/machine-id is empty, then systemd gets confused somehow
[18:41] <zyga> systemd does have a first boot concept
[18:41] <zyga> not confuse
[18:41] <zyga> it's just a mode we never see in ubuntu
[18:41] <zyga> it's been normally used in fedora for a while IIRC
[18:41] <mvo> zyga: right, sorry, my wording was too strong
[18:41] <zyga> well
[18:41] <zyga> until now :)
[18:43] <zyga> https://www.freedesktop.org/software/systemd/man/systemd-firstboot.html
[18:43] <mvo> zyga: for 8418 I think we should try to also run go-latest unit tests in a GH runner and then remove the unit tests from travis entirely. but that is followup material it seems
[18:43] <zyga> go-latest? snap?
[18:44] <mvo> zyga: yes
[18:44] <mvo> zyga: sorry for being cryptic
[18:44] <zyga> sure thing
[18:45] <zyga> please don't apologize for anything :)
[18:45] <mvo> zyga: sorry
[18:45] <zyga> haha :)
[18:45] <mvo> :P
[18:45] <mvo> couldn't resist :)
[18:45] <zyga> After the machine ID is established, systemd(1) will attempt to save it to /etc/machine-id. If this fails, it will attempt to bind-mount a temporary file over /etc/machine-id. It is an error if the file system is read-only and does not contain a (possibly empty) /etc/machine-id file.
[18:45] <zyga> systemd-machine-id-commit.service(8) will attempt to write the machine ID to the file system if /etc/machine-id or /etc are read-only during early boot but become writable later on.
[18:46] <mvo> zyga: yeah, I think systemd is still a bit confused because it should be possible to save /etc/machine-id (unless it tries a save-and-atmoic-move
[18:46] <zyga> it may try too early perhaps?
[18:46] <zyga> I don't know
[18:46] <mvo> zyga: it still procceeds to the bind-mount step it seems
[18:46] <mvo> zyga: yeah, it's not clear from the docs, I am too lazy to look at the source
[18:46]  * mvo should probably do that still :(
[18:46] <zyga> maye not today
[18:47] <zyga> I think it's okay to EOD at 9PM on Friday
[18:50] <mvo> maybe, I still want a) master green b) recovery-key PR on master c) GH actions on master d) icecream!
[18:52] <zyga> mvo: e) pony
[18:52] <zyga> or pony ice cream, as you wish
[18:52] <mvo> zyga: nay, I'm vegetarian
[18:53] <zyga> it's like eating a very large lump of grass anyway
[18:53] <mvo> zyga: hahaha
[19:12] <zyga> I think gnome software is leaking
[19:12] <zyga> it's using more memory than FF does
[19:12] <zyga> the background service I don't even see
[19:47] <mup> PR snapcraft#3011 closed: repo: introduce install_source() and install_gpg_key() to Ubuntu <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3011>
[20:07] <mvo> zyga: I think travis defeated me for today, still no test run finished of my list of things I want so I will check back in my morning. have a good weekend
[20:07]  * mvo waves
[20:07] <zyga> o/
[20:42] <mup> PR snapd#8420 closed: httputil: increase testRetryStrategy max timelimit to 5s <Simple 😃> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8420>
[20:43] <mup> PR snapd#8103 closed: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8103>
[20:44] <King_InuYasha> kyrofa: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/XNKHAJAKFEJKMO46HJBSHPGMSUQLCWU5/
[20:50] <kyrofa> King_InuYasha, yeah I saw it. That's what I get for waiting to respond I suppose
[20:51] <kyrofa> I actually resolved the needinfo ping before he sent that email, as you can see from the activity log he provided in the email :P
[20:52] <kyrofa> King_InuYasha, I wasn't going to respond to the email though, unless I should
[20:53] <King_InuYasha> kyrofa: you probably should, just be cordial and explain what's going on ;)
[20:53] <kyrofa> So I need to respond in multiple places? Not the most efficient of processes
[20:54] <King_InuYasha> kyrofa: nah, just respond in the main email and close out the bugs accordingly
[20:57] <King_InuYasha> the main email is to let the rest of the community know you're alive :)
[21:11] <mup> PR snapd#8423 opened: tests: uc20 nested suite part II <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/8423>
[21:41] <mup> PR snapd#8422 closed: tests: skip "/etc/machine-id" in "writablepaths" test <Simple 😃> <⚠ Critical> <Created by mvo5> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8422>
[23:51] <mup> PR snapd#8424 opened: cmd/snap-bootstrap/initramfs-mounts: cross-check partitions when mounting <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8424>