| mup | PR snapcraft#3366 opened: tools: split local environment setup from environment-setup.sh <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3366> | 00:00 |
|---|---|---|
| mup | PR snapcraft#3367 opened: requirements: pin pip to 20.1.1 for devel <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3367> | 00:00 |
| mup | PR snapd#9621 opened: many: address degraded recover mode feedback, cleanups <Simple 😃> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9621> | 02:43 |
| kinghat | anyone know of a snap permission for chromium that one can enable to make PWAs/website shortcuts function properly? | 03:53 |
| jamesh | kinghat: not at present. The current method Chrome uses is to try and install .desktop files in a place where the shell will try and launch them. Without any intermediation, that represents a pretty trivial sandbox escape. | 04:15 |
| kinghat | i was fine with it the way it was before the snap. how do you revert? | 04:47 |
| jamesh | You may be able to get some of the shortcuts working by copying desktop files from ~/snap/chromium/current/.local/share/applications (I think) to ~/.local/share/applications. I'm not sure if you'd need to fix up the Exec= line or not | 05:02 |
| mup | PR snapd#9540 closed: snap: add new `snap recovery --show-keys` option <Needs Samuele review> <Run nested> <Squash-merge> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9540> | 05:14 |
| mup | PR snapd#9619 closed: cmd/snap-bootstrap,o/devicestate: use a secret to pair data and save <Run nested> <UC20> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9619> | 05:19 |
| mup | PR snapd#9620 closed: spread.yaml: increase number of workers on 20.10 <Simple 😃> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9620> | 05:19 |
| mup | PR snapd#9622 opened: vendor: update to current secboot <Run nested> <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9622> | 08:10 |
| mup | PR snapcraft#3361 closed: extensions: use SNAP_COMMON instead of SNAP_DATA for fonts <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3361> | 09:27 |
| mup | PR snapcraft#3366 closed: tools: split local environment setup from environment-setup.sh <Created by cjp256> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3366> | 09:27 |
| mup | PR snapd#9622 closed: vendor: update to current secboot <Run nested> <Simple 😃> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9622> | 11:10 |
| pedronis | mvo: I reviewed #9621, there's a small consistency problem afaict | 11:13 |
| mup | PR #9621: many: address degraded recover mode feedback, cleanups <Simple 😃> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9621> | 11:13 |
| pedronis | mvo: I can look into it after lunch | 11:16 |
| dot-tobias | Does the order inside a uc20 model definition's snaps header define the installation order? (I have multiple snaps that plug network-manager, and judging from the serial console output, nm is installed last if declared last in the array. That is not mentioned in the docs) | 11:21 |
| mvo | pedronis: thank you | 11:31 |
| ogra | dot-tobias, the order used to rule it in UC16-18 so i'd expect the same in UC20, put NM before the snaps using it in the list | 11:32 |
| ogra | dot-tobias, if the app snaps are fully under your control and are services you can also follow what i just wrote here: https://forum.snapcraft.io/t/how-to-set-the-boot-order-for-snaps-after-reboot/13837/9 | 11:34 |
| ogra | (simply make them come up with disabled services and use hooks and "snapctl is-connected" to start the service when the interface is connected) | 11:34 |
| ogra | s/is/is getting/ | 11:35 |
| pedronis | dot-tobias: yes, for app snaps, the required-snaps/snaps order is the installation order | 11:50 |
| dot-tobias | ogra, pedronis: Thanks, booting an image with modified model atm, will see if that resolves the issue. Weird that there was no error during seed, was watching the serial console output and all hooks ran, or at least I didn't see an error … afterwards I was dropped to a login prompt (disabled console-conf), but the system-user assertion doesn't seem to be picked up. | 12:04 |
| ogra | well i generally use the hooks solution, that way you simply are not depending on the seed ordering and even if seeding of one snap fails it wont cause an avalanche of failure 😉 | 12:06 |
| mup | PR snapd#9623 opened: tests: set the opensuse tumbleweed system as manual in spread.yaml <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9623> | 12:11 |
| dot-tobias | ogra: It's less a service startup problem than it is an interface/install hook issue – and I can't defer those afaik 😅 In my snap, these hooks attempt to create network-manager connections, so I guess the nm snap needs to be … installed? 😄 I've added a similar solution with `snapctl is-connected` to work around the issue that interface hooks run before the install hook when booting a custom image, but I can't really check either | 12:12 |
| dot-tobias | since my attempts to create a local user all fail 🙈 | 12:12 |
| ogra | you can always have a while/d👋sleep loop in your interface hooks ... for debugging your last resort is always cloud-init ... that should work for creating a user, even in UC20 | 12:15 |
| ogra | (silly emoji plugin) ... "while,do,sleep" | 12:15 |
| mup | PR snapd#9624 opened: secboot: call BlockPCRProtectionPolicies even if the TPM is disabled <Created by chrisccoulson> <https://github.com/snapcore/snapd/pull/9624> | 12:16 |
| ogra | dot-tobias, a cloud-init.conf i have used before (note the ssh key is moot) https://paste.ubuntu.com/p/jMVGrg5Cfm/ | 12:17 |
| ogra | (i guess for UC20 the console-conf hack at the bottom wont work, rip that out) | 12:20 |
| dot-tobias | ogra: I meant that such a loop wouldn't let the hook finish, as they're run sequentially – so the “thing to wait for” would never happen if the respective snap wasn't installed before? Or am I missing something and things are run in parallel? | 12:20 |
| ogra | oh, right | 12:20 |
| ogra | no, you are correct, that would just hang the hook forever | 12:20 |
| dot-tobias | ogra: Phew 😄 | 12:21 |
| ogra | https://forum.snapcraft.io/t/remote-logging-on-ubuntu-core/14621 might also be an option (just seeding logsync-james, define the config in your gadget and receive the logs on a central machine running logsync-receiver), that way you'D get the logs but dont need a user | 12:23 |
| ogra | it is definitely more effort than just using cloud-init and a user though | 12:23 |
| ogra | and indeed it requires a working network ... | 12:24 |
| dot-tobias | ogra: Nice to know this setup, thanks! But the user account is intended for users to be able to SSH on their device (the resulting, pre-booted image is intended as a download to flash to your own device) | 12:25 |
| ogra | sure, i didnt mean to use it permanently but for debugging | 12:26 |
| dot-tobias | ogra: That cloud.conf looks almost identical to mine, except you use “plain_text_passwd” whereas I pass an encrypted PW. Which was working fine in a core18 model, but then again that cloud.conf was passed directly to ubuntu-image … Ian suggested that this is most likely due to a seed failure (then cloud-init won't run at all) - btw huge thanks ijohnson, very much appreciated! | 12:26 |
| dot-tobias | ogra: Ah, gotcha. | 12:27 |
| dot-tobias | it's like 99% working but I can't figure out the last % 🙃 | 12:27 |
| ogra | heh ... well, the ordering should surely help | 12:30 |
| dot-tobias | ogra: To be sure: To get a system-user via USB stick, I'd put the `auto-import.assert` from make-system-user on a thumb drive (FAT formatted?), plug that into the RasPi and then start up the freshly flashed image? When would snapd (?) pick up the assertion? | 12:31 |
| ogra | uh, i dont know when exactly it does that ... also, the USB key doesnt need to be plugged in before boot, there is a udev rule that will handle it at any point in time when you plug it in | 12:34 |
| ogra | it should tell you about importing the assertion in the logs though | 12:35 |
| dot-tobias | ogra: Re-plugging worked! I was able to log in, just to find out that all non-auto-connected interfaces weren't connected at all 🙃 So something must be wrong with my gadget, but I don't understand why … especially since the serial console showed me “connecting snap:xyz to slot-snap:xyz` … | 12:44 |
| ogra | snap changes ... | 12:46 |
| ogra | should have details | 12:47 |
| dot-tobias | … checked that, only shows “initialize device“ errors. | 12:47 |
| dot-tobias | (which is expected since I don't have a serial vault yet) | 12:47 |
| dot-tobias | correction: gadget connections for wpe-webkit-mir-kiosk *are* applied, so it's only for one snap where the gadget connections are not applied | 12:48 |
| ogra | wrong snap id or typo ? | 12:48 |
| ogra | you can work around the initialize device errors btw | 12:48 |
| dot-tobias | I sense a very dumb error on my side … to check if Ian's suggestion with is-connected in the install/interface hook works (to enable connection creation for both scenarios, manual snap install and gadget seed), I modified the downloaded snap to save some time + passed it via --snap. Which is of course not signed, so the defaults aren't applied as they pertain to the snap's store ID … | 12:50 |
| ogra | "serial-authority": [ "generic" ], | 12:50 |
| ogra | add that to your model ... | 12:50 |
| ogra | then the initialize device errors go away | 12:51 |
| dot-tobias | ogra: Cool! Is that documented anywhere 😄 | 12:51 |
| ogra | not sure 🙂 | 12:51 |
| ogra | (probably not though) | 12:53 |
| pedronis | mvo: we have a real issue with snap-bootstrap, that was hidden because the tests were mocking too much | 12:54 |
| ogra | degville, "serial-authority": [ "generic" ] should probably documented in the model assertion documentation somewhere (makes the device receive a generic assertion so it does not fail the "initialize device" setp) | 12:56 |
| degville | ogra: thanks for letting me know. I'll make sure it's added. | 12:57 |
| ogra | thanks ! | 12:57 |
| dot-tobias | degville: On that point, I opened a PR for the core20 docs https://github.com/canonical-web-and-design/ubuntu-core-docs/pull/126 → not sure if somebody was notified 😊 | 12:59 |
| mup | PR canonical-web-and-design/ubuntu-core-docs#126: Correct error in `snaps` header docs for uc20 <Created by tobias-grasse> <https://github.com/canonical-web-and-design/ubuntu-core-docs/pull/126> | 12:59 |
| dot-tobias | (or feel free to close if I misunderstood the docs) | 12:59 |
| degville | dot-tobias: thanks for letting me know. I must have missed it, but that's definitely the right place. I'll merge and rebuild. | 13:00 |
| dot-tobias | degville: Thanks! Please do review if the change makes sense 😊 Discovered that by trial and error during my model assertion upgrade. | 13:06 |
| degville | dot-tobias: will do - thank you! | 13:08 |
| mvo | pedronis: oh no, do you have more details? why did we not notice during the spread tests? | 13:39 |
| pedronis | mvo: it's about the fallback paths | 13:39 |
| pedronis | we don't have spread tests for those | 13:40 |
| pedronis | mvo: I don't know if you want to chat before the standup? | 13:40 |
| mvo | pedronis: oh, ok | 13:40 |
| mvo | pedronis: yeah, let me make a cup of tea, then I can join | 13:40 |
| pedronis | bit of a mess | 13:42 |
| pedronis | mvo: I'm in the SU | 13:48 |
| ijohnson | pedronis: should I join too ? | 13:48 |
| * ijohnson just got here | 13:48 | |
| pedronis | ijohnson: yes | 13:49 |
| ijohnson | k | 13:49 |
| ijohnson | pedronis: do you want to just push up the change you were making somewhere for me to look over? | 14:24 |
| pedronis | ijohnson: yes, but probably once I have all the tests changed | 14:25 |
| ijohnson | ok | 14:25 |
| pedronis | atm I have 2 failing tests and new failing test (no data at all) | 14:25 |
| ijohnson | right | 14:25 |
| pedronis | we might need one or more tests | 14:25 |
| pedronis | that I are not a blocker for me to push though | 14:25 |
| mup | PR snapcraft#3368 opened: ci: use github actions for cla check <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3368> | 14:27 |
| dot-tobias | Can someone confirm https://bugs.launchpad.net/snapcraft/+bug/1898877 , or am I just “holding things wrong”? | 14:28 |
| mup | Bug #1898877: [remote-build] source-type 'zip' aborts with KeyError <remote-build> <Snapcraft:New> <https://launchpad.net/bugs/1898877> | 14:28 |
| ijohnson | cjp256: ^ | 14:30 |
| mup | PR snapd#9625 opened: snap: add new "fde-setup" hooktype <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9625> | 14:41 |
| pedronis | ijohnson: https://github.com/pedronis/snappy/commit/18012b6ad1c035611b7a2833090daab5b882f97c there I get 3 failures in cmd/snap-bootstrap | 14:43 |
| ijohnson | pedronis: thanks let me have a look | 14:43 |
| pedronis | ijohnson: taking a break, we can chat afterward if needed | 14:45 |
| ijohnson | sounds good | 14:45 |
| cjp256 | dot-tobias: confirmed, we'd need to enable zip support for osx which shouldn't be a problem | 14:55 |
| cjp256 | i can put up a PR if you'll give it a test :D | 14:55 |
| * cachio lunch | 14:56 | |
| dot-tobias | cjp256: Thanks! 😊 | 15:01 |
| mup | PR snapd#9626 opened: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes <Created by chrisccoulson> <https://github.com/snapcore/snapd/pull/9626> | 15:02 |
| pedronis | ijohnson: I'm back | 15:03 |
| ijohnson | pedronis: ok, I'm working on adjusting secboot to have a DecryptedDevice and using it from initramfs-mounts | 15:03 |
| pedronis | ijohnson: mvo: we have also Chris' PRs to take into account | 15:10 |
| ijohnson | pedronis: yes I think that should be fairly easy to accomodate though, at least the one I saw this morning | 15:10 |
| ijohnson | it is fairly orthogonal to this work I think | 15:10 |
| pedronis | ijohnson: #9626 touches secboot quite a bit | 15:11 |
| mup | PR #9626: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes <Run nested> <UC20> <Created by chrisccoulson> <https://github.com/snapcore/snapd/pull/9626> | 15:11 |
| pedronis | anyway I need to review them next | 15:11 |
| ijohnson | oh I didn't see that one, I only saw 9624 | 15:11 |
| ijohnson | mmm that one is going to be a bit difficult to work with :-/ | 15:12 |
| mup | PR snapcraft#3369 opened: sources: enable 7z, bzr, hg, svn, zip for non-linux <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3369> | 15:13 |
| cjp256 | dot-toabis: https://github.com/snapcore/snapcraft/pull/3369 given the speed of travis lately, maybe we'll have a build to test by tomorrow :D | 15:13 |
| mup | PR snapcraft#3369: sources: enable 7z, bzr, hg, svn, zip for non-linux <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3369> | 15:13 |
| ijohnson | pedronis: the issue with at least one of the tests (TestInitramfsMountsRecoverModeEncryptedDegradedNoDataFoundSaveHappy) is that the wrong disk setup is being mocked, the disk we get from the ubuntu-seed mountpoint still has data in it | 15:21 |
| pedronis | ijohnson: that test is not complete, it will still fail | 15:22 |
| ijohnson | yes I know | 15:22 |
| ijohnson | just thought I would share that in case you hadn't already figured that bit out | 15:22 |
| ijohnson | anyways I'm fixing it all now | 15:22 |
| === King_InuYasha is now known as Conan_Kudo | ||
| === Conan_Kudo is now known as King_InuYasha | ||
| * ijohnson is very grateful for writing all the consistency checks on the unlockRes :-) | 15:39 | |
| pedronis | mvo: I updated 9626 | 15:44 |
| ijohnson | pedronis: do you have opinions on storing the decrypted device as "decrypted-device" inside degraded.json partitions | 15:46 |
| ijohnson | ? | 15:46 |
| ijohnson | that's what I've code and is working well I think | 15:46 |
| pedronis | ijohnson: I'm not sure, we should discuss | 15:46 |
| ijohnson | pedronis: SU ? | 15:46 |
| pedronis | yes | 15:46 |
| ijohnson | pedronis: ok, actually give me a minute I can push my changes up to a branch and we can review together | 15:47 |
| mvo | pedronis: thanks, I have a look | 15:47 |
| ijohnson | pedronis: https://github.com/anonymouse64/snapd/tree/bugfix/uc20-consistent-unlock-result-and-consequences-2 | 15:48 |
| ijohnson | pedronis: joining SU now | 15:48 |
| abeato | @ijohnson, hey, do you know if there has been any recent change in UC20 for auto imports? I use to drop a system-user assertion in the ubuntu-seed partition to get a user after first boot (so I could run tests automtaically), but it looks like it is not working anymore | 16:11 |
| ijohnson | abeato: yes support for that was dropped | 16:12 |
| abeato | ijohnson, ouch... what would be the alternative? | 16:12 |
| mup | PR snapd#9624 closed: secboot: call BlockPCRProtectionPolicies even if the TPM is disabled <UC20> <Created by chrisccoulson> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9624> | 16:17 |
| ijohnson | abeato: use console-conf in your gadget snap | 16:17 |
| ijohnson | sorry cloud-init | 16:18 |
| abeato | ijohnson, ok, is there any example around? | 16:20 |
| * cachio -> kinesiologist | 16:30 | |
| ijohnson | abeato: so you put a file in the root of your gadget snap called cloud.conf, and put content like this in it: | 16:41 |
| ijohnson | https://pastebin.ubuntu.com/p/mNCz53qjQr/ | 16:42 |
| abeato | ijohnson, I see, thanks a lot! | 16:43 |
| ijohnson | np | 16:43 |
| === asottile is now known as albertosottile | ||
| abeato | ijohnson, "# #cloud-config", is that right? | 16:43 |
| ijohnson | abeato: haha no it's not sorry | 16:43 |
| ijohnson | it should be just | 16:43 |
| abeato | :) | 16:44 |
| ijohnson | ``` | 16:44 |
| ijohnson | # cloud-config | 16:44 |
| ijohnson | ``` | 16:44 |
| abeato | ok | 16:44 |
| ijohnson | actually I don't remember how intelligent the cloud-init parser is in reading the first line and comment for that file | 16:44 |
| ijohnson | maybe it still works with the space there or without the space there | 16:44 |
| abeato | I think it prefers without the space, but not 100% sure | 16:45 |
| * ogra wonders if ijohnson has heard of the "plain_text_passwd:" key of cloud-init ... 🙂 | 16:52 | |
| ijohnson | that would be too easy | 16:52 |
| ogra | saves all the crypting | 16:52 |
| ijohnson | yeah that seems way easier! | 16:53 |
| pedronis | mvo: ijohnson: #9626 needs reviews (I fixed it and added more tests) | 17:05 |
| mup | PR #9626: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes <Run nested> <UC20> <Created by chrisccoulson> <https://github.com/snapcore/snapd/pull/9626> | 17:05 |
| ijohnson | pedronis: ack, I'm still fixing up the branch from this morning with the new degraded state things | 17:05 |
| ijohnson | pedronis: it's going ok, a bit fiddly and tedious but no complications thus far | 17:05 |
| mvo | pedronis: will do after dinner | 17:07 |
| pedronis | ijohnson: hopefully, not too fiddly, let me know if it's getting too messy | 17:12 |
| ijohnson | yeah I think it's ok, hopefully will be done with the right code soonish and then just need to update all the tests | 17:13 |
| pedronis | ijohnson: the tests shouldn't need much updating I think | 17:13 |
| pedronis | ijohnson: just the helpers | 17:13 |
| ijohnson | yes hopefully | 17:13 |
| mup | PR snapd#9627 opened: interfaces/raw_usb: allow read access to /proc/tty/drivers (LP: #1890365) <Created by oSoMoN> <https://github.com/snapcore/snapd/pull/9627> | 17:47 |
| ijohnson | pedronis: ok, I've got basically what I think is needed, one thing I didn't realize while implementing but I now remember is that you mentioned you wanted the internal "raw decrypted device" to be unset if we found the encrypted device but couldn't unlock it, is that correct ? | 18:06 |
| ijohnson | pedronis: because what I accidentally ended up implementing is that the raw decrypted device setting is now the block device if we can't unlock it | 18:07 |
| ijohnson | I can fix it to be what we agreed upon, just wanted to make sure that I should fix it that way | 18:07 |
| pedronis | ijohnson: yea, that sounds not we agreed | 18:10 |
| ijohnson | sorry, I'll fix it then | 18:10 |
| pedronis | not what we agreed | 18:10 |
| pedronis | ijohnson: but maybe it's easier to push what you have | 18:10 |
| ijohnson | pedronis: sure, should I open a PR or just send you the branch ? | 18:11 |
| pedronis | ijohnson: it shouldn't make a big difference, it still seems the right thing | 18:11 |
| pedronis | ijohnson: I'm also confused because I'm not sure why you are calling it "raw" | 18:11 |
| ijohnson | eh just the var name I used in the code since it's not the "exported" one | 18:11 |
| ijohnson | I have no personal attachment to the word "raw" here | 18:11 |
| pedronis | mmh | 18:11 |
| pedronis | ijohnson: well if it's almost PR ready, let's make a PR, you mark it draft | 18:12 |
| ijohnson | just didn't want to spend brain cycles on a better word yet | 18:12 |
| ijohnson | it is PR ready I have added the new tests we discussed and updated all the tests | 18:12 |
| ijohnson | I'll mark it as a draft then | 18:12 |
| ijohnson | one moment | 18:12 |
| mup | PR snapd#9628 opened: secboot,cmd/snap-bootstrap: fix degraded mode cases with better device handling <Needs Samuele review> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9628> | 18:18 |
| ijohnson | pedronis: ^ | 18:18 |
| ijohnson | whoops had a gofmt typo, just force pushed | 18:21 |
| === ijohnson is now known as ijohnson|lunch | ||
| mup | PR snapcraft#3370 opened: repo: address issue with fix_symlink() when pointed at directory <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3370> | 18:23 |
| mvo | pedronis: I updated the fde hooks gdoc fwiw | 18:42 |
| mvo | ijohnson|lunch: thanks for 9628! | 18:42 |
| pedronis | ijohnson|lunch: a bunch of comments in #9628, sadly it looks messier than the old code :/ so it's quite hard to review | 18:53 |
| mup | PR #9628: secboot,cmd/snap-bootstrap: fix degraded mode cases with better device handling <Needs Samuele review> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9628> | 18:53 |
| === ijohnson|lunch is now known as ijohnson | ||
| ijohnson | pedronis: well that's unfortunate | 19:12 |
| pedronis | ijohnson: please look at my comment, and let me know if you have questions, I can be around a bit more | 19:17 |
| ijohnson | yes I'm reading through them now | 19:17 |
| pedronis | ijohnson: ah, there is something off in secboot itself, that's why maybe things are confusing for me | 19:31 |
| ijohnson | what's off in secboot ? | 19:31 |
| pedronis | ijohnson: tbh this is probably simpler with chris PR | 19:32 |
| pedronis | ijohnson: if you have a purely uncrypted device you don't set DecryptedDevice | 19:32 |
| ijohnson | ... does that not make sense though ? | 19:32 |
| pedronis | ijohnson: that's what we dicussed | 19:33 |
| ijohnson | sorry I guess I missed that then | 19:33 |
| pedronis | well, it simplifies code quite a bit | 19:33 |
| pedronis | but as I said it's probably easier with chris PR then now | 19:33 |
| ijohnson | it just doesn't make sense to me to be setting DecryptedDevice for something that erm well isn't a decrypted device ? | 19:33 |
| ijohnson | if that's what is desired I think we should rename it from DecryptedDevice to something more generic like ActivatedDevice or TargetDevice or something | 19:34 |
| pedronis | ijohnson: sorry, I thought I explained this, we can argue on the name (it's internal anyway) | 19:36 |
| ijohnson | I guess I only understood you to mean to set the block device on the degraded state and not on the secboot return result | 19:37 |
| ijohnson | anyways I'm looking at 9626 now | 19:37 |
| pedronis | ijohnson: my point is that it's always correct to mount DecryptedDevice if set | 19:37 |
| ijohnson | sure I guess in this case we should just rename it to something else then | 19:38 |
| ijohnson | I wonder if we should also rename Device to BlockDevice | 19:39 |
| pedronis | ijohnson: maybe FsDevice or FilesystemDevice | 19:40 |
| ijohnson | those seem fine to me | 19:41 |
| ijohnson | yeah I see why this is simpler with Chris' PR | 19:42 |
| ijohnson | pedronis: shall I finish a review of that so we can merge that first then I'll rebase my PR as we just discussed on top of that ? | 19:42 |
| * ijohnson just does a review anyways | 19:43 | |
| mup | PR snapd#9606 closed: tests: Use systemd-run on tests part2 <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9606> | 19:43 |
| mup | PR snapd#9623 closed: tests: set the opensuse tumbleweed system as manual in spread.yaml <Simple 😃> <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9623> | 19:43 |
| pedronis | ijohnson: we should also land #9621 | 19:43 |
| mup | PR #9621: many: address degraded recover mode feedback, cleanups <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9621> | 19:43 |
| ijohnson | I guess 9621 isn't more wrong than the current state of things | 19:43 |
| pedronis | ijohnson: yea, my bit your new PR contains addresses my comment there | 19:44 |
| pedronis | ijohnson: I'm landing it | 19:44 |
| ijohnson | ok | 19:44 |
| pedronis | ijohnson: I wonder how mvo plans to backport these though | 19:45 |
| ijohnson | perhaps we should squash merge then | 19:45 |
| pedronis | or maybe he needs to just merge master back, not sure | 19:46 |
| pedronis | I fear it's going to be messy either way | 19:46 |
| ijohnson | hmm does my pr conflict with 9626 ? | 19:46 |
| pedronis | I suspect so | 19:46 |
| ijohnson | err sorry | 19:47 |
| ijohnson | does 9621 conflict with 9626 ? | 19:47 |
| pedronis | maybe, not massively though | 19:48 |
| pedronis | ijohnson: I can deal with that | 19:49 |
| ijohnson | I'm just wondering if we could land chris' pr first, maybe squash maybe not, then I could force-push a rebase of my pr on top of master so it is not conflicting | 19:49 |
| pedronis | I mean, landing 9626 | 19:49 |
| ijohnson | I'm almost done with a review, it looks good to me, I think it needs more tests, but probably fine to do in a follow-up | 19:50 |
| pedronis | ijohnson: mmh, I added quite a few tests | 19:50 |
| ijohnson | I guess not so much new tests, I just think the specific tests added are a little weird, I think other tests would make more sense | 19:51 |
| ijohnson | anyways just a minute and I will submit my review | 19:51 |
| pedronis | ijohnson: maybe | 19:51 |
| mup | PR snapd#9621 closed: many: address degraded recover mode feedback, cleanups <UC20> <Created by anonymouse64> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9621> | 19:53 |
| pedronis | ijohnson: anyway it does conflict so I'll have to sort that | 19:59 |
| ijohnson | pedronis: mmm, well I submitted my review now | 20:00 |
| ijohnson | I will rebase my newer pr on top of chris' pr locally then and wait to push anything more until that has landed | 20:01 |
| pedronis | ijohnson: TestInitramfsMountsRunModeHappy still exists afaik | 20:02 |
| ijohnson | it does ? | 20:02 |
| ijohnson | where ? | 20:02 |
| pedronis | ah | 20:02 |
| pedronis | no | 20:02 |
| pedronis | it became TestInitramfsMountsRunModeHappy | 20:03 |
| pedronis | you are right | 20:03 |
| ijohnson | I was almost doubly confused about it disappearing and then reappearing right before my eyes! | 20:04 |
| pedronis | heh, anyway I probably can fix your 2nd point | 20:07 |
| pedronis | first I need deal with the conflicts | 20:08 |
| pedronis | ijohnson: actually 9626 is a bit buggy | 20:16 |
| ijohnson | pedronis: how so? | 20:16 |
| ijohnson | in the secboot code ? | 20:17 |
| pedronis | it sets res.Device too eagerly | 20:17 |
| pedronis | at least it's a change of behavior | 20:17 |
| ijohnson | yeah I noticed that but I think it's fine though given that we will be rewriting that code immediately after in my PR to use i.e. FsDevice or whatever | 20:18 |
| pedronis | well, it's a change of behavior | 20:19 |
| pedronis | it's more likely to confuse you than anything | 20:19 |
| ijohnson | well to be fair I am easily confused so that's not saying much :-) | 20:19 |
| pedronis | sorry, that was a generic you | 20:20 |
| pedronis | it's also interesting that no test fails :/ | 20:20 |
| pedronis | either way | 20:21 |
| ijohnson | I'm starting to get highly suspicious of the tests around UnlockVolumeUsingSealedKeyIfEncrypted | 20:21 |
| ijohnson | I don't think the tests around that are really testing all the cases | 20:21 |
| ijohnson | it's testing a lot of cases, but I don't think it's testing the right things | 20:22 |
| pedronis | I added something to tests that would have failed | 20:24 |
| pedronis | ijohnson: pushed | 20:28 |
| ijohnson | ack I'll have a look | 20:28 |
| pedronis | ijohnson: the diff of the merge is readable | 20:29 |
| pedronis | I forgot to add the extra tests though | 20:31 |
| ijohnson | which extra tests ? | 20:31 |
| ijohnson | the install/recover ones? I don't think those are needed for htis pr, they can be followsup imho | 20:32 |
| ijohnson | followups | 20:32 |
| pedronis | ijohnson: actually all the NoModel tests share code, so it's very easy to add | 20:34 |
| ijohnson | nice | 20:34 |
| pedronis | pushed | 20:35 |
| pedronis | ijohnson: anything more that I can help with? | 20:38 |
| ijohnson | pedronis: if you have thoughts on what to name the *Device things on secboot.UnlockResult so that it's not quite so confusing I would appreciate input on that | 20:39 |
| pedronis | ijohnson: Part(ion)Device and FsDevice ? | 20:41 |
| ijohnson | so PartDevice will always be the partuuid one, and FsDevice will be the one that's always safe to mount? | 20:42 |
| pedronis | yes | 20:42 |
| ijohnson | got it, that sounds fine then | 20:42 |
| ijohnson | It will probably be an hour or two before I can finish that and push it up | 20:43 |
| pedronis | so for the encrypted case they are the same | 20:43 |
| ijohnson | you mean the unencrypted case they are the same ? | 20:43 |
| pedronis | sorry, unencrypted case | 20:43 |
| ijohnson | right | 20:43 |
| dot-tobias | Should a uc20 device (RasPi) have internet access via ethernet during initial boot, or is this entirely optional? (Assuming no snap attempts to do something that require internet access) | 20:47 |
| ijohnson | dot-tobias: during uc20 install mode, internet access is not required, afaik snapd shouldn't try to talk to the store during install mode at all really | 20:48 |
| ijohnson | pedronis: one final question on the pr, there is a line commented out, not sure if that should be commented out or not | 20:48 |
| ijohnson | pedronis: question on 9626 that is | 20:48 |
| dot-tobias | ijohnson: Ok thanks for clarifying, there was one point during the firstboot log on serial about failure to connect to api.snapcraft.io, but I guess that's optional. | 20:49 |
| ijohnson | dot-tobias: can you share that bit of the log? | 20:50 |
| pedronis | ijohnson: that is from the original code, I need to check | 20:51 |
| dot-tobias | ijohnson: sure, as soon as I flash and boot up the next image I'll look out for that 😄 (fear I'll be doing that at least a few more times tonight …) | 20:51 |
| ijohnson | dot-tobias: because we did somewhat recently disable refreshing the catalog in install mode, so maybe that disabling was ineffective of you just haven't gotten it in your version of snapd yet | 20:54 |
| dot-tobias | ijohnson: Good to know. Using revison 9731 (armhf) of snapd, will check the log later. Doesn't seem to have an impact on the init process, that's still stuck way later on … | 20:56 |
| pedronis | ijohnson: the secboot changes should be something like this I suppose: https://paste.ubuntu.com/p/4QKgVZ7FQM/ | 21:01 |
| ijohnson | pedronis: yes that looks about exactly like I was going to do | 21:02 |
| pedronis | ijohnson: as fas as I can tell you can drop that //err line in your PR, I'm not quite sure how that code behaved on master but it seems right as it is there | 21:14 |
| ijohnson | pedronis: ok sounds good I will drop it from my pr then | 21:15 |
| pedronis | ijohnson: feel free to use bits of that pastebin, I actually mostly wrote that because of the secboot_tpm_test.go part of it. Those tests are a bit non-obvious in how the check things | 21:16 |
| pedronis | as I said before my last push to the other PR they weren't even check some .Device related behavior | 21:17 |
| ijohnson | yes I will, it doesn't cleanly apply to my branch after I rebased it on top of Chris's branch, but I can see the relevant changes to make there | 21:17 |
| dot-tobias | ijohnson: Is there a saner way to debug/test uc20 custom images for armhf arches other than flashing them to a microSD and waiting patiently until it errs? 🙃 | 21:43 |
| ijohnson | dot-tobias: it depends on what you need to debug, are you debugging failure installing ? | 21:44 |
| dot-tobias | ijohnson: Yes, I have a hunch that the modified install hook somehow fails (though the added commands work inside a snap shell), but the serial console doesn't give me anything. Side note, does UC keep logs from the init run, and where would I find them? | 21:46 |
| pedronis | ijohnson: I suppose, it's implict, but the unexported fields now are going to be partDevice and fsDevice too? | 21:51 |
| === mup is now known as 7JTABK1M1 | ||
| ijohnson | pedronis: yes that's what I named them now | 22:25 |
| ijohnson | dot-tobias: sorry seems the netsplit lost some things | 22:25 |
| ijohnson | dot-tobias: I was going to say that we don't have logs from the specific install mode boot because the rootfs is really a tmpfs there, but there should be logs from the failed seeding in the standard journald location on the pi's SD card to read | 22:25 |
| ijohnson | dot-tobias: additionally, you could try extracting the system-data/var/lib/snapd/state.json file from the ubuntu-data partition and run `snap debug state <that-file>` on it to see `snap changes` like output, or specify `--change=1` to see `snap tasks` like output on another system | 22:26 |
| ijohnson | hopefully that helps a bit, but we are aware that we are bit lacking in this story of debugging first boot failures on uc20, it's a bit in conflict with some of our security goals around uc20 so things have gotten a bit more complicated | 22:27 |
| ijohnson | pedronis: also I guess this is obvious but I pushed up the update to 9628 now | 22:27 |
| ijohnson | that should be good to go I think | 22:27 |
| dot-tobias | ijohnson: I just realized I should add network-manager to the install hook's plugs in case the n-m operations are fired from there (instead of from the interface hook) … sometimes late night debugging helps 😄 So maybe found the cause … will rebuild and check, noting errors on the console for you. | 22:29 |
| dot-tobias | Thanks for the logging info, really invaluable at this point! | 22:29 |
| ijohnson | dot-tobias: ah yes that is the case that the install hook will need all the interfaces | 22:30 |
| ijohnson | happy to help, sorry this has been such a journey for you, but know that your struggles now will make it easier for folks in the future I think! | 22:30 |
| ijohnson | cachio_: around ? | 22:30 |
| ijohnson | cachio_: I see some nested tests with master failing like this: | 22:30 |
| ijohnson | /home/gopath/src/github.com/snapcore/snapd/tests/lib/nested.sh: line 1114: systemd_stop_and_destroy_unit: command not found | 22:30 |
| ijohnson | see https://github.com/snapcore/snapd/pull/9626/checks?check_run_id=1387265367 for example | 22:32 |
| mup | PR #9626: snap-bootstrap,secboot: call BlockPCRProtectionPolicies in all boot modes <Run nested> <UC20> <Created by chrisccoulson> <https://github.com/snapcore/snapd/pull/9626> | 22:32 |
| dot-tobias | ijohnson: Would've helped if the hook threw an error in some way I could see on the console, but in most cases OSI layer 8 strikes again 🙈 and yes, I try to contribute docs or at least naive questions along the way 😄 | 22:33 |
| ijohnson | yes I think we need to be better about surfacing errors to the console | 22:33 |
| ijohnson | well I _know_ we need to be better about that :-) | 22:34 |
| pedronis | cachio_: did you land something related to systemd that broke nested.sh ? | 22:39 |
| pedronis | cachio_: we are getting: /home/gopath/src/github.com/snapcore/snapd/tests/lib/nested.sh: line 1114: systemd_stop_and_destroy_unit: command not found | 22:40 |
| pedronis | ijohnson: ^ | 22:43 |
| ijohnson | pedronis to be fair it is just on restoring | 22:43 |
| pedronis | ijohnson: ? | 22:43 |
| ijohnson | But yes I also asked cachio_ about that a few minutes before you does | 22:43 |
| ijohnson | pedronis I just mean that failure doesn't mean we don't have test results | 22:44 |
| pedronis | if you fail to restore | 22:44 |
| pedronis | spread will skip over things | 22:44 |
| pedronis | 2020-11-11 21:22:25 Aborted tasks: 12 | 22:45 |
| ijohnson | ah | 22:45 |
| ijohnson | fair | 22:45 |
| pedronis | so we don't know what passed or not | 22:45 |
| pedronis | so this is fairly annoying | 22:45 |
| pedronis | at this juncture | 22:45 |
| ijohnson | ok, let me look at this quickly | 22:45 |
| ijohnson | maybe it's a simple thing | 22:45 |
| ijohnson | seems https://github.com/snapcore/snapd/pull/9606 is what broke it | 22:47 |
| mup | PR #9606: tests: Use systemd-run on tests part2 <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9606> | 22:47 |
| ijohnson | pedronis: can I push to chris' branch what I think fixes this nested problem we are seeing? | 22:50 |
| pedronis | ijohnson: yes | 22:51 |
| ijohnson | k, one second | 22:51 |
| pedronis | thx | 22:51 |
| ijohnson | pedronis: done, pushed to 9626, I didn't have time to test it manually, but it should be good I think | 22:54 |
| ijohnson | I also pushed the commit to 9628 | 22:55 |
| ijohnson | the commit was directly cherry-picked so it shouldn't pose a problem for conflicts when getting it back onto 2.48, however folks decide to do that | 22:56 |
| ijohnson | I need to EOD now, but will check back in a couple hours on the tests to see if there is any real nested issues here or if any silly tests fail | 22:56 |
| ijohnson | hopefully everything is green when mvo gets up | 22:56 |
| pedronis | ijohnson: I left some comments in 9628, I can take care of them in the morning I suppose, I need to give it a fresh look anyway | 23:01 |
| ijohnson | pedronis ok I can try to look quickly but probably can't address anything right now maybe later tonight | 23:03 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!