/srv/irclogs.ubuntu.com/2020/04/03/#snappy.txt

mupPR snapcraft#2916 closed: status: implement using the new channel-map endpoint <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/2916>00:56
mupPR 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>01:53
mupPR 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>03:11
mborzeckimorning05:34
mborzeckimvo: hey06:13
mvomborzecki: good morning06:20
mupPR snapd#8410 closed: many: disentagle release and snapdenv from sandbox/* <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8410>06:29
mupPR snapd#8412 opened: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8412>06:31
mupPR 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>06:34
pstolowskimorning07:03
mvogood morning pstolowski07:05
zygagood morning07:05
zygahey mvo,07:05
zygamvo: I noticed we have a small problem in debian07:06
zygamvo: snapd doesn't build07:06
zygadh_missing: warning: usr/bin/snap-preseed exists in debian/tmp but is not installed to anywhere07:06
zygaI think we have different ruleset there, and leftover binaries cause builds to fail07:06
zygaI'll see if I can use my shiny metal rights and actually help07:06
jameshHad another CI run hit Travis's 50 minute limit: https://travis-ci.org/github/snapcore/snapd/builds/67039287007:07
zygajamesh: I'll move travis jobs to actions today07:07
zygaI think it's time07:07
mvogood morning zyga07:14
mvo8103 needs a second review, it's important for the encryption story and to unblock the TPM work07:15
mvojamesh: thanks for letting us know, once we are slightly less busy we move to gh actions to get rid of this07:16
mvojamesh: 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 annyoing07:17
mvo(fwiw 8103 is not very long, hopefully quick review)07:18
mupPR 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:24
mvojamesh: 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 errors07:27
jameshmvo: I think the change looks good.  When run on an unloaded system, it should still verify that the timeout has been extended07:28
mvozyga: session-tool fails on master https://api.travis-ci.org/v3/job/670441262/log.txt - could you please ahve a look?07:29
zygaYes07:31
mupPR snapd#8406 closed: usersession: extend timerange in TestExitOnIdle <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8406>07:44
jameshpedronis_: I think the extra spread test I added covers what you wanted in my user daemons PR07:45
=== pedronis_ is now known as pedronis
pedronisjamesh: thanks07:46
zygapedronis: thank you for the move yesterday!08:17
mupPR snapd#8414 opened: o/configstate: core config handler for persistent journal <Created by stolowski> <https://github.com/snapcore/snapd/pull/8414>08:41
pstolowskiogra: hi, this might be interesting for you ^ (feedback is welcome)08:41
ogranice !08:54
ograpstolowski, you are aware that etc/systemd/journald.conf.d/ is complete nonsense ?08:54
pstolowskiogra: heh, not, why?08:55
ograpstolowski, all you need to do is to create the dir ...08:55
pstolowskiogra: you mean /var/log/journal ?08:55
ograthe 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 config08:55
ograyeah08:56
pstolowskihuh08:56
ograi'D just make the code call a mkdir and be done ;)08:56
pstolowskimhm08:56
ogra(and rmdir if the option is unset)08:57
pstolowskiondra: is this consistent across systemd versions (core16, core18...)?08:57
pstolowskisorry, ogra ^08:57
ogranotsure what ondra thinks ... but it surely is for 16 and 1808:57
ogra:)08:57
ogramight be that 20 changed it though ... better test there .... for 16 and 18 it was always sufficient to just create the dir08:58
* ondra agrees with anything ogra says, as long as it's not about partitions!08:58
ogra!08:58
pstolowskiogra: ok, that's interesting, thank you!08:59
ograthank *you* for implementing it !!09:00
ogra:D09:00
pstolowskiogra: ah, i read the manualy again. so it appears that "auto" is the default behavior,  the existense of /var/log/journal controls the behavior09:03
ograyep :)09:03
pstolowskiogra: thanks again for the enlightenment ;)09:04
zygapstolowski, ogra: related review https://github.com/snapcore/snapd/pull/8414#pullrequestreview-38708971109:17
mupPR #8414: o/configstate: core config handler for persistent journal <â›” Blocked> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8414>09:17
pstolowskizyga: thanks. we might not need the conf file at all, i'm going to verify this09:18
zygathanks09:18
zygaping me for a re-review please09:18
mupPR snapd#8415 opened: cmd/snap-recovery-chooser: tweaks <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8415>09:36
pedronismborzecki: I nitpicked a bit in #841509:45
mupPR #8415: cmd/snap-recovery-chooser: tweaks <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8415>09:45
zygamborzecki: https://github.com/snapcore/snapd/pull/8415#pullrequestreview-38711283209:51
mupPR #8415: cmd/snap-recovery-chooser: tweaks <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8415>09:51
mborzeckipedronis: zyga: thanks09:52
zygaI'll be back shortly, it's quite cold in the office and ought to make some warm tea09:53
zygamborzecki: unit tests failed on that PR10:00
mborzeckiwaat?10:00
zygahttps://www.irccloud.com/pastebin/NkNEOsNj/10:00
zyganot yours but meh10:00
zygamvo: ^10:00
zygais that another thing to adjust?10:00
mborzeckiweird, that timeout is longer already10:01
mborzeckimaybe a bug in the test/code?10:01
mvozyga, mborzecki I thought I did push something there?10:02
zygaah, perhaps maciek's branch is simply older10:02
mvohttps://github.com/snapcore/snapd/pull/841210:02
mupPR #8412: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8412>10:02
mborzeckimvo: yes, https://github.com/snapcore/snapd/pull/8404 has landed already10:02
mupPR #8404: client: increase timeout in client tests to 100ms <Simple 😃> <Created by mvo5> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8404>10:02
mvomborzecki: this one is different, no? that's what 8412 is for10:03
mvomborzecki: or am I misreading this?10:03
mborzeckipstolowski: 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 journal10:03
mborzeckioh, w8, it is10:03
mborzeckiso why did i get a new machine-id again?10:03
pstolowskimborzecki: works as expected on core1810:03
pstolowski(i rebooted)10:04
mborzeckihmmmm ├─/etc/machine-id  tmpfs[/machine-id]   tmpfs      ro,size=203112k,mode=7510:05
zygayeah10:05
mborzeckipstolowski: mvo: ^^10:05
zygawhy do we have machine-id on a tmpfs10:05
zygamborzecki: we noticed this a while ago10:05
zygain the review of mount-ns data10:05
zygaI guess we forgot to act10:05
mborzeckipstolowski: do you hav a core18 vm? can you check `findmnt /etc/machine-id` ?10:06
pstolowskimborzecki: yep, 1 sec10:06
zygamborzecki: I do10:06
zyga /etc/machine-id /dev/sda3[/system-data/etc/machine-id] ext4   rw,relatime,data=o10:07
zygaso that's weird10:07
zygamborzecki: from pi4 /etc/machine-id /dev/mmcblk0p2[/system-data/etc/machine-id] ext4   rw,relatime10:08
zygamystery10:08
mborzeckihm it's not listed in writable-paths, what sets it up?10:08
zygamaybe that's our test specific setup10:08
mborzeckizyga: anything in /etc/fstab or /run/image.fstab?10:09
pstolowskizyga: i see  /etc/machine-id /dev/sda3[/system-data/etc/machine-id] ext4 as well10:10
zygawhat is /run/image.fstab?10:10
zygawhere are writable paths set up10:10
zygaI mean10:10
zygathe list10:10
pstolowskizyga: and pi3 with core16 is the same as yours10:11
zygaI checked pi4 with core1810:11
zygano clue10:11
mborzeckizyga: iirc it's from our initrframfs hooks10:11
mvomborzecki: hm, this is all confusing, it looks like /etc/machine-id is not in the writable-path10:11
mborzeckizyga: https://github.com/snapcore/core20/blob/master/static/etc/system-image/writable-paths10:11
zygahmm - +0:+0 /system-data/etc/machine-id /etc/machine-id rw,relatime shared:+1 - ext4 /dev/sda3 rw,data=ordered10:12
zygathat's from our own test data10:12
zygawat?10:12
zygacheck this out10:12
zygahttps://www.irccloud.com/pastebin/0OxSdK9y/10:12
mvomborzecki: 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
zygacore 2010:12
zygait's good on core16 and core1810:13
zygamount-ns test is a bit like the os-release-zoo10:13
zygaanyway10:13
zygabrb10:13
zygaI'm so cold today10:13
zyganeed to make that tea for real10:13
mborzeckicore20 has different initramfs tho?10:13
mvoyeah10:14
mborzeckiso in core build i see this:10:15
mborzecki318:    mount -o bind "${rootmnt}/writable/system-data/etc/machine-id" "${rootmnt}/etc/machine-id"10:15
mvomborzecki: 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 atomitcally10:16
mborzeckibut it's not used in core20 world anymore right? it's only ubuntu-core-initramfs now?10:16
mvomborzecki: but the man-page for machine-id talks about bind-mounting it, so *hopefully* that will work10:17
mvomborzecki: 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:18
zygaback10:24
zygamuch better :)10:24
zygashall I report this just in case we don't fix it today?10:25
zygaI need a 2nd review on https://github.com/snapcore/snapd/pull/8403 to make progress please10:26
mupPR #8403: sandbox/cgroup: avoid making arrays we don't use <Skip spread> <Created by zyga> <https://github.com/snapcore/snapd/pull/8403>10:26
zygaI will adjust the comment on the next pass, this is green now10:26
zygamborzecki: simple-session tool improvement https://github.com/snapcore/snapd/pull/841610:26
mupPR #8416: tests/session-tool: reset failed session-tool units <Created by zyga> <https://github.com/snapcore/snapd/pull/8416>10:26
zygathis avoids the need to use property only available on future systemd versions10:27
zygaand solves accumulation of leftovers10:27
mupPR snapd#8416 opened: tests/session-tool: reset failed session-tool units <Created by zyga> <https://github.com/snapcore/snapd/pull/8416>10:27
xnoxmvo: 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
xnoxmvo: bug report please.10:27
zygaxnox: on core20?10:28
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
zygaxnox: "do you want me to report a bug on core20"10:29
xnoxzyga:  yes10:30
zygaon it10:30
zygaxnox: https://github.com/snapcore/core20/issues/3210:31
mupIssue core20#32 opened: The machine-id file should be writable <Created by zyga> <https://github.com/snapcore/core20/issue/32>10:31
xnoxtah10:32
zygaunit tests fail left and right, I guess travis containers are just as busy as launchpad lately10:34
mvoxnox: I can give you a PR if you want? or are you already on it?10:38
xnoxmvo:  no, i'm not on anything. not serving interrupts, but doing things as they were queued up for me.10:38
mvook10:42
mupPR core20#33 opened: writable-path: make /etc/machine-id writable <Created by mvo5> <https://github.com/snapcore/core20/pull/33>10:42
mborzeckimvo: let me try to build a core20 snap and see if it works as expected10:46
mvomborzecki: \o/10:52
mupPR snapd#8412 closed: httputil: increase httpclient timeout in TestRetryRequestTimeoutHandling <Simple 😃> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8412>10:58
mborzeckihmm11:03
mborzecki/etc/machine-id /dev/mapper/ubuntu-data[/system-data/etc/machine-id] ext4   rw,relatime11:03
mborzecki/etc/machine-id tmpfs[/machine-id]                                   tmpfs  ro,size=203112k,mode=75511:03
mborzeckiwhy 2 mounts tho?11:03
zygamvo: I opened 7700 for a review11:03
zygamvo: can you please look at what we could do for the upcoming sprint to be in the green11:04
zygamvo: it's a RFC, not a final solution, I could use advise on where to go next11:04
mborzeckiok, on the first boot i had 2 mounts, but now after another reboot there's just one11:04
zygaoh, wait, I pushed without commiting11:05
zygaanyway, it doesn't change much11:06
zygaI've restarted the CLA check a few times and it's failing11:06
zygahttps://travis-ci.org/github/snapcore/snapd/builds/66994873411:06
mborzeckiyeah, looking good now11:07
mborzeckipstolowski: the persistent journal works now too11:07
pstolowskimborzecki: good, thanks. fwtw there is another issue with all this that I just described in my PR11:08
pstolowskimvo: ^ (sorry, I revoked your review)11:08
mborzeckipstolowski: 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:09
pstolowskimborzecki: right, seems to match https://gist.github.com/JPvRiel/b7c185833da32631fa6ce65b4083688711:10
zygaI believe reviewing https://github.com/snapcore/snapd/pull/8416 helps, I cannot explain how though11:10
mupPR #8416: tests/session-tool: reset failed session-tool units <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8416>11:10
zygaapart from having fewer units after this test11:11
zygahttps://github.com/systemd/systemd/issues/8672 is related11:15
mvozyga: it looks reasonable11:15
zygamvo: what should we do to take next steps?11:15
mvopstolowski: what does that mean, should I review the pr again?11:16
mvozyga: 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 UI11:17
zygamvo: the PR is open, I addressed that question now11:17
mvozyga: cool11:17
zygamvo: ok, I'll pursue this a little more then11:17
mvozyga: has the prereq landed?11:17
mvozyga: if so we could target it for 2.4511:17
zygamvo: close to it, if I can grab jamie for a moment today11:17
zygamvo: I need to merge master after samuele's fixes again11:18
mvonice11:18
zyga(the reorg)11:18
pstolowskimvo: no, please see the comment11:18
pstolowskimvo: we need to agree on what to do11:18
mvopstolowski: I see now, thank you11:19
mvopstolowski: 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:20
mvoauto 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 strange11:21
mvootoh it's nice on the other releases :)11:21
pstolowskimvo: 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:25
mvopstolowski: good point11:26
pstolowskimvo: 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:28
* pstolowski lunch11:29
pedronismvo: pstolowski: let's chat later11:29
mupPR 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
zygamvo: this fixes the cron issue with session-tool https://github.com/snapcore/snapd/pull/841711:31
mupPR #8417: tests/session-tool: kill leaking closing session <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/8417>11:31
mupPR snapd#8417 opened: tests/session-tool: kill leaking closing session <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/8417>11:31
zygamborzecki: is [ ! -e /proc/$pid ]; a good way to check if a process exists?11:31
mvothanks zyga11:45
mborzeckizyga: in shell? hmm why not, kill -0 <pid> probably doesn't work11:48
mborzeckikill 0 ofc11:48
zygaI've seen this error a lot this week12:11
zyga2020-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 request12:11
zygareboot test fails12:11
zygaI wonder if that's something else impacting or is that something broken for real12:12
pedroniszyga: I re-reviewed #8123, not urgent by I left some questions and a remark there12:15
zygalooking12:15
mupPR #8123: interfaces/network-control: bring /var/lib/dhcp from host (approach b) <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/8123>12:15
zygapedronis: 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 connection12:16
zygapedronis: so we will not create /var/lib/dhcp via hostfs12:16
zygaso no permissions or related cahnges12:16
pedronisah, makes sense12:17
zygaI'll see for the comment in a moment12:17
zygawrapping up some branches and I'd like to break before the call12:17
zygathank you for the review, I think this will land soon :)12:17
* zyga afk for some time, see you at the standup12:20
cmatsuokacachio: I'm trying to run the encryption test on the nested backend, but got a git archive error, did you see this before?12:37
cmatsuokaijohnson: good morning12:40
cmatsuokaijohnson: I have a question about InitramfsUbuntuDataDir and friends12:40
cmatsuokaijohnson: 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 again12:42
cmatsuokacachio: never mind, it was a local problem12:47
ijohnsonhey cmatsuoka12:47
ijohnsoncmatsuoka: for now it's probably fine to just use boot.InitramfsUbuntuDataDir from wherever you need it, even if it's not during the initramfs12:48
ijohnsoncmatsuoka: we had this same issue with pedronis and mborzecki for the code that reboots into a particular system for the chooser12:48
ijohnsoncmatsuoka: so the variable may get renamed eventually but for now it's fine to use it12:48
mborzeckihm hm?12:48
cmatsuokaijohnson: right, that's how I implemented it so I'll leave it as is for now, thanks12:48
ijohnsonhey mborzecki12:48
mborzeckiah right12:48
mborzeckicrazy idea, hm maybe we could have automount units for that?12:49
zygamborzecki: for what?12:54
mborzecki/run/mnt/ubuntu-{seed,boot,data}12:54
pedronisI am a bit missing how that would help?12:55
mborzeckipedronis: 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 deman12:56
mvocmatsuoka: 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 land12:56
pedronismborzecki: well we need them during initrams12:57
pedronisthe question is what happens later12:57
mborzeckipedronis: right, but we don't need them all the time later12:57
pedronismborzecki: who would write the units?12:57
cmatsuokamvo: sure, working on that!12:57
mborzeckilater == after switching the root12:57
pedronisthere's no main partition after a while12:57
pedronis*after a while12:57
pedronisfor a while12:57
mborzeckipedronis: iirc xnox had code that generates mount units for those in ubuntu-core-initramfs12:58
mvocmatsuoka: thanks!12:58
pedronismborzecki: there are issues, if they can come and go, any checks we do on them is problematic12:59
mborzeckiyeah, that's true as well12:59
pedronisanyway at this point is all things to think about after beta13:00
* zyga breaks to stretch a little14:13
zygaback hurts14:13
roadmrouch :(14:13
pedronisijohnson: I'm not sure I understand your comment about the switch14:16
pedronisI'm missing something14:16
ijohnsonuh14:16
ijohnsonpedronis: how can the caller set something on bsmark? what I did just now is have the caller pass in bsmark to the select function14:17
ijohnsonhere let me show you14:17
pedronisijohnson: we return bsmark14:17
pedronisno?14:17
ijohnsonyes, so how would the caller set properties on bsmark before calling the function ?14:18
ijohnsonwithout passing in bsmark to the choose function ?14:18
ijohnsonpedronis: https://pastebin.ubuntu.com/p/pDTXqbBsc4/14:18
pedronisit can set after14:18
pedronisthe info is not really used inside the function14:19
pedronisafaict14:19
pedronisit's used in commit later I suppose14:19
ijohnsonpedronis: 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.MarkBootSuccessful14:20
ijohnsonwe only want to call loadBootenv once for boot.MarkBootSuccessful14:20
pedronisijohnson: I understand, but maybe at this point is just easier for me to try what I have mind14:20
pedronisand see if it works14:20
pedronisI'm failing to explain myself it seems14:20
ijohnsonyes, sorry I don't understand your suggestion14:21
pedronisit's very simple actually14:21
pedronisbut maybe doesn't work14:21
pedronislet me see14:21
pedronisijohnson: it works afaict14:25
ijohnsonpedronis: can you show your code ?14:26
pedronisat least the tests still pass14:26
pedronisijohnson: just this: https://paste.ubuntu.com/p/shzz4xgN3w/14:28
ijohnsonah okay sure that works fine14:30
ijohnsonsorry I didn't understand that your suggestion was to do it after the choose* function14:31
pedroniswell, I said it on irc, but not in the comment, it's actually the only way to make it work simply14:32
pedronisbut as I commented the doc comment of the helper needs to clarify that it's possible/makes sense14:32
ijohnsonyes that's fine14:34
mupPR snapd#8368 closed: github: goodbye travis <Created by zyga> <Closed by zyga> <https://github.com/snapcore/snapd/pull/8368>14:36
mupPR snapd#8418 opened: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>14:40
pedronisijohnson: 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 undo14:47
pedronisbut maybe you have thoughts about that?14:48
ijohnsonpedronis: what other logic do we need to check in ensureBootOk ?14:48
pedronisijohnson: we trigger a revert that will do a SetNextBoot , same with the undo14:48
pedronisthat's where things intersect14:48
ijohnsonmmm14:49
ijohnsonthis is slightly depressing that we need so many permutations of this kind of test now across 3 different packages :-/14:49
pedronisijohnson: I'm open to think this through just saying that the chain of boot state changes14:50
pedronisends with a SetNextBoot in the undo code14:50
ijohnsonyes you're right there are more cases like that14:50
pedronisin some cases14:50
pedronisthat's I think why we thought some cases would take care by themselves14:51
pedronisbtw14:51
pedronisthis logic on the undo path14:51
pedronisof a refresh14:51
ijohnsonyes14:53
pedronisijohnson: related to that the other important moving part is: snapstate.WaitRestart14:57
mupPR core20#33 closed: writable-path: make /etc/machine-id writable <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/core20/pull/33>14:59
ijohnsonmborzecki: 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 time15:00
ijohnsonpedronis: yes indeed it's unclear where all we are currently covering the calls to boot from WaitRestart15:00
pedronisijohnson: some of the managers_test should reach that15:01
zygadegville: [offtopic] did you finish the game?15:01
ijohnsonpedronis: yes but I don't know if we have enough coverage of it is my point15:02
mborzeckiijohnson: going out now, please ping in the review15:02
ijohnsonmborzecki: ack15:02
ijohnsonhave a nice weekend15:02
mborzeckiijohnson: thanks, you too!15:02
degvillezyga: no, not yet. I've really been taking it slow, and playing it only on difficult, because I know it's not long.15:02
zygadegville: I'm very curious what's at the end15:03
zygaHL3 confirmed or what :D ?15:03
zygabut don't tell me15:03
zygajust tell me if it's worth that 1000 _once you know the ending_15:03
degvillezyga: yeah, I've tried to skip all discussions about it so I don't know either.15:03
pedronisijohnson: 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 return15: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:05
mupPR 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:08
mupPR snapd#8419 opened: Add libnvidia-rtcore as Nvidia library <Created by joedborg> <https://github.com/snapcore/snapd/pull/8419>15:10
pedronisijohnson: the main chain  I think is always something like:  SetNextBoot -> bootloader cfg -> initramfs -> MarkBootSuccessful -> GetCurrentBoot (-> undo or revert SetNextBoot)15:10
pedronisijohnson: that should be coverable from boot mostly, with a few managers_test on top to check actual integration15:11
zygabrb15:12
pedronisijohnson: so maybe the first step is actually to add checks for GetCurrentBoot results in the significant places in all the TestHappy in boot_test.go15:15
mupPR snapd#8420 opened: httputil: increase testRetryStrategy max timelimit to 5s <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8420>15:18
zygare15:27
mvocan someone do a second review for 8103 please?15:35
zygamvo: I'm on it15:36
zygamid way15:36
mvozyga: \o/15:36
mvozyga: thank you15:36
mupPR snapd#8419 closed: Add libnvidia-rtcore as Nvidia library <Created by joedborg> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8419>15:38
mupPR snapd#8329 closed: interfaces: allow raw access to USB printers <Created by ogra1> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8329>15:44
mupPR 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:46
zyga7/9 files15:53
zygahttps://github.com/snapcore/snapd/pull/8103#pullrequestreview-38736799815:55
mupPR #8103: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8103>15:55
zygaI can +1 if somone will follow up15:55
zygawould +1 except for https://github.com/snapcore/snapd/pull/8103#discussion_r40310351015:56
zygamvo: ^15:56
=== hggdh is now known as hggdh_
=== hggdh_ is now known as hggdh__
=== hggdh__ is now known as hggdh-msft
ijohnsonpedronis: sorry was in meetings16:05
ijohnsonpedronis: how much of these additional tests do you want in the open PR?16:05
pedronisijohnson: should we have a quick HO?16:06
ijohnsonpedronis: sure16:06
ijohnsonSU?16:06
pedronisyes16:06
ijohnsoncool I'm there16:07
=== facubatista is now known as Teclado
=== Teclado is now known as facubatista
cmatsuokazyga: addressing that16:08
zygathanks16:08
zygaI can +1 if you do16:08
cmatsuokazyga: test "$(stat --printf='%u %a' /var/lib/snapd/device/fde/recovery-key)" = "0 600"16:12
zygaon focal this is my path16: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/bin16:12
zyganote the duplicate /snap/bin16:12
zygaand the non-ubuntu /var/lib/snapd/snap/bin16:12
zygacan anyone confirm?16:12
ijohnsonzyga: I have the same16:22
zygathanks16:22
zygaprobably one-too-many helpers16:22
ijohnsonto be clear, I have the duplicated /snap/bin and non-ubuntu /var/lib/snapd/snap/bin too16:23
zygaack16:23
pedronisit's the same on bionic fwiw16:23
zygafun16:25
zygawe must have done that earlier16:25
zygaI'll look on Monday16:25
zygaor maybe earlier16:25
zygain our job interviews we should only have one question16:25
zygaHow do you add one item to PATH16:26
zygathat will filter out everyone, including us16:26
zyga;)16:26
ijohnsonhooray16:26
zygafiled it just in case16:33
zygamvo, cmatsuoka: https://github.com/snapcore/snapd/pull/8103#pullrequestreview-38741383716:39
mupPR #8103: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8103>16:39
cmatsuokazyga: fixed as suggested, thanks16:49
zygahttps://github.com/snapcore/snapd/pull/8418/checks#step:8:916:57
mupPR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>16:57
zygagovendor cannot build on go from xenial?16:57
zygamvo: do you remember why we depend on 1.9 and not 1.10?17:05
mvozyga: centos 7 iirc17:10
zygaI see17:10
zygaoh well17:10
zygaI'll do something17:10
zygaI solved the silly go in azure17:10
zygathey pre populate GIGs of software into the image17:11
mvozyga: if we run a spread unittest task everywhere I think we are also good17:11
zygaincluding go 1.11+17:11
zygaI think we don't but I'll check17:11
mvozyga, cmatsuoka thanks for working on 8103, looks like it's ready to go once tests have run17:11
mvozyga: thanks!17:11
zygaheh17:12
zygaxenial cannot run our static checks17:12
zygait's like moving in a maze17:12
zygawithout lights17:12
zygaand knowing there are turds everywhere17:12
mvozyga: xenial cannot run our static checks? why not?17:12
zygahttps://github.com/snapcore/snapd/pull/8418/checks#step:10:1817:12
mupPR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>17:13
zygabecause shellcheck in xenial is too old17:13
zygawe never noticed17:13
mvozyga: woah, interessting17:15
mvozyga: and sad17:15
mvozyga: I thought travis would run those in xenial too - apparently not17:15
zygaeveryone does custom shit17:15
zygaat least17:15
zygaand I really have to give them credit17:16
zygagithub has excellent docs on what they do17:16
zygaand what's custom17:16
* mvo nods17:17
zygaI wonder...17:17
zyga(insert harry potter mystery music)17:18
zygamvo: check out https://github.com/snapcore/snapd/pull/8418/checks17:19
mupPR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>17:19
zygawow :)17:19
mupPR 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
zygamvo: snapd work in azure17:20
mvozyga: nice17:20
zyga*snaps17:20
mvozyga: looking17:20
zygaso shellcheck snap17:20
mvozyga: yay17:20
ijohnsonzyga: is it still a problem where "/" is owned by the vsts user or something17:20
zyga?17:21
zygavsts?17:21
zygatest17:21
zygayes17:21
zygasnap-confine will refuse to work17:21
ijohnson:-(17:21
ijohnsonbut I found someone to talk to about that on MS side17:21
zygacan I help?17:21
ijohnsonI can CC you on my email17:22
ijohnsonthe problem before was that we couldn't find anyone to talk to about who manages this on the github/microsoft side17:22
mvozyga: fun (not!) - the travis run for 8103 has not even started yet17:24
zygaheh17:25
pedronisI pushed a comment there,  might need formatting in a follow up17:33
zygait seems master is broken17:33
zygahttps://api.travis-ci.org/v3/job/670608294/log.txt17:33
zyga    - google:ubuntu-core-20-64:tests/core/writablepaths17:33
pedronishttps://github.com/snapcore/snapd/pull/8103/files#diff-13724d8f1f88168c790f24aafb4d2d13R13717:33
mupPR #8103: snap-bootstrap: store encrypted partition recovery key <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/8103>17:33
zygahttps://www.irccloud.com/pastebin/5FyicYVN/17:33
mupPR snapd#8421 opened: tests: enable unit tests on debian-sid again <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8421>17:36
zygacachio: could you please review https://github.com/snapcore/snapd/pull/841817:39
mupPR #8418: github: move static checks and spread over <Created by zyga> <https://github.com/snapcore/snapd/pull/8418>17:39
cachiozyga, sure17:40
zygathanks!17:40
zygathis will unlock more chance to improve things17:40
zygaaccelerate team-wide feedback17:40
zygaand get rid of the 50 minute limit17:41
cachiozyga, where are these going to run?17:47
cachioyour box?17:47
zygano17:47
zygathose run in azure17:47
zygain the normal gh runners17:47
cachioahh17:47
zygabtw, mvo: I got 16GB of ram into my box so we could easily run those 32 runners for extra overflow capacity17:47
zygathere's some cruft to shed17:48
zygamainly to make it faster17:48
zygabut that's all _after_ this one17:48
mupPR snapcraft#3012 opened: plugins: install required apt sources and keys to system <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3012>18:08
cachiozyga, left you some questions in the PR18:13
cachioand closed it by mistake18:14
cachiothen I reopened it18:14
zygacachio: :D18:16
zygacachio: I replied18:16
zygacachio: remember that this is a long term game:18:18
zygacachio: we get instant responsiveness18:18
zygacachio: we get faster end-to-end18:18
zygacachio: we get lower costs18:18
zygacachio: if we cut rebuilds this is even faster18:18
zygacachio: essentially boot into a system and start away18:18
cachioI 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 parallel18:19
zygayeah18:19
zygaI think this will be faster than old workflow already18:19
cachiothe good part is that if some system fails, we just restart this one18:19
zygawe...18:19
zyganot yet18:19
zygagithub has this but it's not released yet18:19
cachioahh18:20
cachiowell in the future well save that money too18:20
cachio+118:21
zygathanks!18:21
zygalet's improve it together18:21
cachioso far it worked very well18:23
cachiothat's really possitive18:23
mupPR snapd#8422 opened: tests: skip "/etc/machine-id" in "writablepaths" test <Simple 😃> <⚠ Critical> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8422>18:28
mvozyga: ^- this will unbreak master18:29
zygata18:29
zygalooking18:29
mupPR 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:29
zygahmm18:30
zygacore20 tests18:31
zygawhen do we run them18:31
zygain first boot?18:31
ijohnsonzyga: we run core20 tests during run mode after we reboot18:34
zygamvo: ^18:35
zygais this consistent with your patch?18:35
ijohnsonzyga: yes "first boot" for uc20 -> first boot run mode18:37
zygaahhh18:37
ijohnsonthere's also install mode, which is technically the "first" boot, but we don't call it that18:37
ijohnsoninstall mode boot is more like the zeroth boot18:37
mvozyga: yes18:40
mvozyga: it's what maciej also saw, next reboot is fine for strange reasons18:40
mvozyga: I think it happens when the /etc/machine-id is empty, then systemd gets confused somehow18:41
zygasystemd does have a first boot concept18:41
zyganot confuse18:41
zygait's just a mode we never see in ubuntu18:41
zygait's been normally used in fedora for a while IIRC18:41
mvozyga: right, sorry, my wording was too strong18:41
zygawell18:41
zygauntil now :)18:41
zygahttps://www.freedesktop.org/software/systemd/man/systemd-firstboot.html18:43
mvozyga: 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 seems18:43
zygago-latest? snap?18:43
mvozyga: yes18:44
mvozyga: sorry for being cryptic18:44
zygasure thing18:44
zygaplease don't apologize for anything :)18:45
mvozyga: sorry18:45
zygahaha :)18:45
mvo:P18:45
mvocouldn't resist :)18:45
zygaAfter 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
zygasystemd-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:45
mvozyga: 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-move18:46
zygait may try too early perhaps?18:46
zygaI don't know18:46
mvozyga: it still procceeds to the bind-mount step it seems18:46
mvozyga: yeah, it's not clear from the docs, I am too lazy to look at the source18:46
* mvo should probably do that still :(18:46
zygamaye not today18:46
zygaI think it's okay to EOD at 9PM on Friday18:47
mvomaybe, I still want a) master green b) recovery-key PR on master c) GH actions on master d) icecream!18:50
zygamvo: e) pony18:52
zygaor pony ice cream, as you wish18:52
mvozyga: nay, I'm vegetarian18:52
zygait's like eating a very large lump of grass anyway18:53
mvozyga: hahaha18:53
zygaI think gnome software is leaking19:12
zygait's using more memory than FF does19:12
zygathe background service I don't even see19:12
mupPR 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>19:47
mvozyga: 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 weekend20:07
* mvo waves20:07
zygao/20:07
mupPR 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:42
mupPR 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:43
King_InuYashakyrofa: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/XNKHAJAKFEJKMO46HJBSHPGMSUQLCWU5/20:44
kyrofaKing_InuYasha, yeah I saw it. That's what I get for waiting to respond I suppose20:50
kyrofaI actually resolved the needinfo ping before he sent that email, as you can see from the activity log he provided in the email :P20:51
kyrofaKing_InuYasha, I wasn't going to respond to the email though, unless I should20:52
King_InuYashakyrofa: you probably should, just be cordial and explain what's going on ;)20:53
kyrofaSo I need to respond in multiple places? Not the most efficient of processes20:53
King_InuYashakyrofa: nah, just respond in the main email and close out the bugs accordingly20:54
King_InuYashathe main email is to let the rest of the community know you're alive :)20:57
mupPR snapd#8423 opened: tests: uc20 nested suite part II <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/8423>21:11
mupPR 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>21:41
=== tai271828_ is now known as tai271828
mupPR snapd#8424 opened: cmd/snap-bootstrap/initramfs-mounts: cross-check partitions when mounting <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8424>23:51

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!