| mup | PR snapcraft#3243 opened: Add skip-keys to catkin plugin <Created by Levi-Armstrong> <https://github.com/snapcore/snapcraft/pull/3243> | 04:37 |
|---|---|---|
| mborzecki | morning | 06:08 |
| mvo | mborzecki: good morning - should I merge 9128 or is it not meant to be on master? | 06:39 |
| mborzecki | mvo: hey, hm it's probably ok to merge it, it's only adding some debug output to the test | 06:41 |
| mup | PR snapd#9126 closed: boot: introduce current_boot_assets and current_recovery_boot_assets to modeenv <Simple 😃> <UC20> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9126> | 06:45 |
| mup | PR snapd#9128 closed: tests/main/cgroup-tracking: try to collect some information about cgroups <Simple 😃> <Test Robustness> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9128> | 06:45 |
| zyga | good morning | 07:04 |
| pstolowski | morning | 07:06 |
| mborzecki | zyga: pstolowski: hey | 07:16 |
| mborzecki | zyga: left some comments under https://github.com/snapcore/snapd/pull/9128 | 07:16 |
| mup | PR #9128: tests/main/cgroup-tracking: try to collect some information about cgroups <Simple 😃> <Test Robustness> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9128> | 07:16 |
| mborzecki | zyga: some weird thing happening on 16.04 and cgroups | 07:17 |
| mvo | good morning zyga | 07:32 |
| mvo | hm, so | 07:39 |
| mvo | 2020-08-11T07:31:29.8774856Z - google:ubuntu-20.04-64:tests/main/cgroup-tracking-failure | 07:39 |
| mvo | 2020-08-11T07:31:29.8775399Z - google:ubuntu-20.04-64:tests/main/snap-user-service | 07:39 |
| mvo | that is known, right? | 07:39 |
| pedronis | mvo: I fear you'll need zyga to undersand the state of that, there's a PR open but I don't know if it's the final story on this | 07:48 |
| mvo | pedronis: that's fine, just wanted to double check | 07:49 |
| mvo | pedronis: also it was a bit of a general question, not directly targeted to you :) | 07:49 |
| mvo | pedronis: did you had a chance to think about the name for the kernel-crypto-api interface? or is the name good already? | 07:50 |
| pedronis | I didn't interpret it as targeted to me, but nobody was saying anything :) | 07:50 |
| pedronis | mvo: no, I haven't seriously thought about that PR since Friday | 07:51 |
| mvo | pedronis: heh :) thanks! | 07:51 |
| mborzecki | pedronis: hey, s/TrustedAssetsChain/TrustedAssets/? | 07:51 |
| mborzecki | pedronis: tbh i assumed that we'll glue that higher up the call stack, somewhere in boot when (re)sealing | 07:53 |
| pedronis | mborzecki: yes, as I said, I think the content matches the doc comment, but the name confused Ian | 07:53 |
| mborzecki | ok | 07:54 |
| pedronis | or so I think | 07:54 |
| pedronis | the glueing is a bit messy because it is across recovery and run bootloaders | 07:54 |
| pedronis | but we will deal | 07:55 |
| mborzecki | yeah, otoh i'm not sure one bootloader (eg. run) should know that it's being loaded by the recovery one | 07:55 |
| mup | PR snapd#9111 closed: releases: release 2.46~pre1 to groovy <Simple 😃> <Skip spread> <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/9111> | 07:55 |
| pedronis | mborzecki: I don't think that's the main problem | 07:55 |
| pedronis | mostly for the final glueing we reall need the maps from modeenv | 07:56 |
| mup | PR snapd#9131 opened: debian: release 2.46~pre2 <Simple 😃> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9131> | 08:30 |
| zyga | re | 08:47 |
| zyga | sorry, I had a small unexpected errand | 08:47 |
| zyga | back now | 08:47 |
| zyga | mborzecki: 0 is always the ID of unified cgroup | 08:47 |
| zyga | mborzecki: cachio suggested that lxd test is responsible | 08:47 |
| zyga | maybe lxd mounts the unified hierarchy somehow | 08:47 |
| mborzecki | zyga: ha, as it mounts the hierarchy and it's left behind? | 08:47 |
| zyga | mborzecki: I will try that in a moment | 08:47 |
| zyga | perhaps | 08:47 |
| zyga | that would explain it | 08:47 |
| mborzecki | zyga: and sys from the host is bind mounted to the container? | 08:47 |
| zyga | and fortunately we have a cleanup script for it | 08:48 |
| zyga | I don't fully know, there are some approaches, more than one | 08:48 |
| mborzecki | duh, how about systemd container protocol then? :) | 08:48 |
| zyga | there's a special fake cgfs as well | 08:48 |
| zyga | mborzecki: I think that's beyond my confidence zone | 08:48 |
| zyga | but we have a clue | 08:48 |
| zyga | I spent a bit too long last night trying to fix core 16 | 08:48 |
| mborzecki | if it is lxd, then stgraber will probably know more | 08:48 |
| zyga | made progress but I EOD past midnight and I was a bit tired in the morning] | 08:49 |
| zyga | mborzecki: yeah but it's quick and easy to verify this with -shell-after | 08:49 |
| zyga | I'll try that now | 08:49 |
| zyga | hey mvo, sorry for starting late | 08:49 |
| zyga | mborzecki: actually, first priority is to fix master | 08:50 |
| zyga | so give me a moment to wrap up that issue first | 08:50 |
| mborzecki | errand, back in 30 or so | 08:50 |
| zyga | ok | 08:53 |
| mvo | zyga: no worries | 08:56 |
| mborzecki | re | 09:12 |
| * zyga tries to sit in the office today | 09:16 | |
| zyga | yesterday that didn't go very well | 09:16 |
| mvo | mborzecki: any highlevel concerns/feedback about #9130? especially aobut the plan to make the mountedfilesystemwriter use the ResolvedSource (i.e. an absolute path) and stop passing in the gadget root there? | 09:17 |
| mup | PR #9130: [RFC] Support for gadget.yaml "$kernel:" style references <Created by mvo5> <https://github.com/snapcore/snapd/pull/9130> | 09:17 |
| mborzecki | mvo: haven't looked yet, i'll do it today | 09:17 |
| mvo | mborzecki: it will cause some churn so I want to somewhat be certain it's not in vain :) | 09:17 |
| mvo | mborzecki: ok, no worries | 09:17 |
| mup | PR snapd#9132 opened: o/hookstate/ctlcmd: add optional --pid argument to "snapctl is-connected" <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/9132> | 09:25 |
| mup | PR snapd#9133 opened: osutil: add CommitAs to atomic file <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9133> | 09:30 |
| mborzecki | hopefully a trivla PR ^^ | 09:32 |
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/9133#pullrequestreview-464902323 | 09:33 |
| mup | PR #9133: osutil: add CommitAs to atomic file <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9133> | 09:33 |
| zyga | yes, but with a typo :) | 09:33 |
| mborzecki | haha | 09:33 |
| mborzecki | ok, let me force push, not worth another patch | 09:33 |
| zyga | yeah | 09:33 |
| zyga | and cancel current run | 09:33 |
| zyga | save $$$ | 09:33 |
| * zyga tries another iteration of core16 fix | 09:34 | |
| mborzecki | btw. gh actions still does not support canceling a workflow when a branch is pushed to? | 09:35 |
| mborzecki | canceling a running workflow i mean | 09:35 |
| zyga | I don't know | 09:35 |
| zyga | what else is broken in master | 09:37 |
| zyga | we have systemd-logind failing often | 09:37 |
| zyga | but there was one more thing IIRC | 09:37 |
| mborzecki | zyga: cgroups on xenial | 09:38 |
| zyga | ah right | 09:38 |
| zyga | thanks | 09:38 |
| mborzecki | zyga: and unit tests using gpg fail occasionally | 09:38 |
| zyga | thanks :) | 09:38 |
| zyga | yeah | 09:38 |
| zyga | about that | 09:38 |
| zyga | it fails in azure | 09:39 |
| mborzecki | snap sign tests iirc | 09:39 |
| zyga | not in our gce tests | 09:39 |
| zyga | azure VM has all kinds of modifications that are well documented | 09:39 |
| mborzecki | but gpg? | 09:39 |
| mup | PR snapd#9095 closed: boot/bootstate20: unify commit method impls, rm bootState20MarkSuccessful <UC20> <Created by anonymouse64> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9095> | 09:40 |
| mup | PR snapd#9116 closed: tests: add system information and image information when debug info is displayed <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9116> | 09:40 |
| zyga | mborzecki: they update lots of things to more recent versions | 09:42 |
| zyga | I mean, really anything that people use | 09:43 |
| zyga | and pre-install a load of things | 09:43 |
| zyga | maybe some interaction there leaks | 09:43 |
| mup | PR snapd#9001 closed: o/snapshotstate: helpers for calculating disk space needed for an automatic snapshot (2/N) <Disk space awareness> <Needs Samuele review> <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9001> | 09:45 |
| mup | PR snapd#9079 closed: gadget/install: retrieve command lines from bootloader <Simple 😃> <UC20> <Created by cmatsuoka> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9079> | 09:45 |
| pedronis | mborzecki: should we just merge #9127 ? we can change again when it's used, if we discover we really need it to be different | 09:48 |
| mup | PR #9127: bootloader: introduce TrustedAssetsBootloader, implement for grub <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9127> | 09:48 |
| mborzecki | pedronis: i think we should land it and tweak later on | 09:48 |
| mborzecki | pedronis: this will probably be clearer when i start opening more bits | 09:48 |
| pstolowski | zyga: what's the status of https://bugs.launchpad.net/snapd/+bug/1867193 ? the test PR referenced there was merged long time ago, not a bug once "robust-mount-namespace-updates" are enabled? | 09:51 |
| mup | Bug #1867193: failure refreshing snap with content interface <snapd:In Progress by zyga> <https://launchpad.net/bugs/1867193> | 09:51 |
| pedronis | mborzecki: I'm adding this comment to the merge: Notice that this in practice splits across the divide of recovery vs run mode bootloaders, the complete chains for sealing will need to bridge that. We might use a different helper to produce that. | 09:51 |
| zyga | I'd mark it as fix released | 09:51 |
| zyga | the last comment may be a different bug | 09:51 |
| mborzecki | pedronis: sounds good | 09:52 |
| mup | PR snapd#9124 closed: gadget: introduce content update observer <UC20> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9124> | 09:55 |
| mup | PR snapd#9127 closed: bootloader: introduce TrustedAssetsBootloader, implement for grub <UC20> <Created by bboozzoo> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9127> | 09:55 |
| mborzecki | yay | 09:56 |
| * mvo hugs mborzecki | 09:56 | |
| mvo | sil2100: snapd is in beta now so we can re-enable beta image builds to pending. let me know if you prefer a mail | 10:09 |
| zyga | core16 tests are running | 10:10 |
| zyga | maybe no more typos | 10:10 |
| zyga | fingers crossed | 10:10 |
| mvo | zyga: what pr is that? | 10:12 |
| zyga | mvo: still local | 10:12 |
| zyga | there's a PR but it's only missing core16 | 10:12 |
| mvo | ok | 10:12 |
| zyga | PR number is | 10:12 |
| zyga | 9125 | 10:12 |
| zyga | if it passes then master will be better | 10:12 |
| zyga | well, locally, then I push and open the draft for review | 10:12 |
| === facundo__ is now known as facubatista | ||
| pstolowski | pedronis: #9084 is ready for review (prereq PR landed) | 10:16 |
| mup | PR #9084: o/snapstate: check disk space before creating automatic snapshot on remove (3/N) <Disk space awareness> <Needs Samuele review> <Created by stolowski> <https://github.com/snapcore/snapd/pull/9084> | 10:16 |
| zyga | tests are running, I need to stop sitting now | 10:17 |
| zyga | man standing desk is a godsend | 10:18 |
| zyga | I wish I knew that years ago | 10:18 |
| sil2100 | mvo: thanks! On it! | 10:22 |
| mup | PR snapd#9134 opened: boot, o/devicestate: TrustedAssetUpdateObserver stubs, hook up to gadget updates <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9134> | 10:26 |
| zyga | one more tweak and I think it will be good | 10:37 |
| pstolowski | pedronis: also, do you have a moment today to talk about disk space check & multi snap install/refresh? | 10:53 |
| mborzecki | mvo: can you merge https://github.com/snapcore/snapd/pull/9133 ? the test failure are unrelated | 11:12 |
| mup | PR #9133: osutil: add CommitAs to atomic file <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9133> | 11:12 |
| mvo | mborzecki: sure, done - could you please check 9131 ? | 11:28 |
| mborzecki | 2020-08-11T08:38:45.2793033Z 2020-08-11 08:38:45 Cannot allocate google:ubuntu-core-20-64: google: read JWT from JSON credentials: 'type' field is "authorized_user" (expected "service_account") | 11:29 |
| mborzecki | wow | 11:29 |
| zyga | hmm? | 11:31 |
| zyga | JWT? | 11:31 |
| mup | PR snapd#9133 closed: osutil: add CommitAs to atomic file <Simple 😃> <UC20> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9133> | 11:31 |
| zyga | javascript web token | 11:31 |
| zyga | (just making stuff up) | 11:31 |
| mborzecki | zyga: close ;) | 11:31 |
| zyga | so hot | 11:31 |
| zyga | I was outside for a sec | 11:31 |
| zyga | to move walk around the block | 11:31 |
| mborzecki | mvo: have you added skip spread tag after opening 9131? | 11:32 |
| mvo | mborzecki: yes, just now | 11:34 |
| mvo | mborzecki: actually silly that I require spread for this | 11:34 |
| mborzecki | mvo: i've closed and reopened it, the tag should be picked up now | 11:35 |
| mvo | mborzecki: aha, nice | 11:35 |
| sil2100 | mvo: dailies for beta should now be enabled! You want a new build kicked now? | 11:41 |
| mvo | sil2100: yes please | 11:41 |
| pedronis | pstolowski: we can chat after the standup if that works for you? | 11:51 |
| pedronis | pstolowski: thanks for merging master into #9084 | 11:52 |
| mup | PR #9084: o/snapstate: check disk space before creating automatic snapshot on remove (3/N) <Disk space awareness> <Needs Samuele review> <Created by stolowski> <https://github.com/snapcore/snapd/pull/9084> | 11:52 |
| mup | PR snapd#9131 closed: debian: release 2.46~pre2 <Simple 😃> <Skip spread> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9131> | 11:56 |
| zyga | I think it works now | 12:02 |
| zyga | single test passes | 12:02 |
| zyga | let me push | 12:02 |
| * mvo crosses finger for zygas pr | 12:04 | |
| zyga | yeah, I want this to be over | 12:05 |
| zyga | I'll take a quick shower and I'll start working on the other bug | 12:05 |
| zyga | (cgroups) | 12:05 |
| pstolowski | pedronis: yes, after standup is fine | 12:16 |
| zyga | looking into the cgroup + lxd thing | 12:17 |
| zyga | core16 is almost done testing, no failures | 12:24 |
| pedronis | mborzecki: some questions/comments in #9134 ? | 12:25 |
| mup | PR #9134: boot, o/devicestate: TrustedAssetUpdateObserver stubs, hook up to gadget updates <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9134> | 12:25 |
| mborzecki | pedronis: thanks for the reviews (and mvo too) | 12:26 |
| zyga | core16 is green | 12:28 |
| zyga | let's see if everything else passes now | 12:28 |
| zyga | I'm focusing on the lxd thing now | 12:28 |
| zyga | first I need to see it | 12:28 |
| zyga | the fix should not be complicated | 12:28 |
| zyga | mborzecki: yeah, that's really happening | 12:32 |
| zyga | I wonder how | 12:32 |
| mborzecki | zyga: lxd mounting a unified hierachy? | 12:32 |
| zyga | yep | 12:33 |
| mborzecki | zyga: or maybe systemd in lxd doing that | 12:33 |
| zyga | clear as day | 12:33 |
| zyga | I will run again and see which part of the test causes that | 12:33 |
| zyga | I will push a quick fix so that we're okay | 12:33 |
| mup | PR snapd#9019 closed: tests/nested/manual/minimal-smoke: run core smoke tests in a VM meeting minimal requirements <Run nested> <Created by bboozzoo> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/9019> | 12:36 |
| zyga | oh man | 12:36 |
| zyga | mborzecki: I understand now | 12:36 |
| zyga | mborzecki: that cgroupv2 is not mounted inside | 12:37 |
| pstolowski | pedronis, mvo thanks for feedback on disk space awareness | 12:37 |
| zyga | it was mounted in the container | 12:37 |
| zyga | but those are global | 12:37 |
| zyga | all processes see all cgroups | 12:37 |
| zyga | now where the hell is it mounted? | 12:38 |
| zyga | and I think this is more than tests | 12:38 |
| zyga | we need to adjust snapd code as well | 12:38 |
| zyga | mborzecki: I'm still unclear where this thing is mounted | 12:39 |
| zyga | it's not inside the container | 12:39 |
| zyga | it's not mounted in the inner container either | 12:40 |
| zyga | but the inner container is bionic | 12:41 |
| zyga | maybe it's just systemd? | 12:41 |
| zyga | hmmm | 12:48 |
| zyga | ok I'll bisect | 12:48 |
| zyga-mbp | I'll go for lunch now, ttyl | 13:50 |
| * cachio lunch | 14:54 | |
| zyga-mbp | re | 15:14 |
| mup | PR snapd#9087 closed: o/snapstate: check available disk space before downloading a snap on install (4/N) <Disk space awareness> <Needs Samuele review> <â›” Blocked> <Created by stolowski> <Closed by stolowski> <https://github.com/snapcore/snapd/pull/9087> | 15:22 |
| mup | PR snapd#9135 opened: boot: move bootloaderKernelState20 impls to separate file <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9135> | 15:42 |
| mup | PR snapd#9064 closed: .github/workflows: move snap building to test.yaml as separate cached job <Simple 😃> <Skip spread> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9064> | 15:47 |
| * zyga explores a fix for the cgroup2 thing | 16:01 | |
| zyga | it's a kernel bu | 16:01 |
| zyga | *big | 16:01 |
| zyga | *bug | 16:01 |
| zyga | or feature | 16:01 |
| zyga | depending on how you look | 16:01 |
| zyga | we can only work around it | 16:01 |
| jdstrand | kenvandine: that's weird. on focal I seem to have gnome-software installed as a deb (fine). I have the review-tools latest/stable installed so I wanted to show someone the permissions dialog, but gnome-software showed that the review-tools were not installed. I pressed the Install button and they install latest/edge | 16:13 |
| ijohnson | zyga: the unified cgroup mount after lxd container started ? | 16:17 |
| zyga | it's not just thta | 16:17 |
| zyga | just mount cgroupv2 | 16:17 |
| zyga | and unmount it | 16:17 |
| zyga | done | 16:17 |
| zyga | no containers required | 16:17 |
| jdstrand | kenvandine: I wonder if it is because I haven't done 'snap login'? (ie, polkit promted me) | 16:18 |
| ijohnson | zyga: ah interesting, maybe we just mount cgroupv2 all the time unconditionally in project prepare and deal with the consequences ? | 16:29 |
| zyga | ijohnson: nah, that's unnatural | 16:29 |
| zyga | I've adjusted both snapd and the test that's sensitive to it | 16:30 |
| zyga | just making final touches | 16:30 |
| mup | PR snapd#9136 opened: bootstate20: rm bootState20Modeenv, pass around modeenv directly <Cleanup :broom:> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9136> | 16:32 |
| ijohnson | zyga: The dark side of cgroups is a pathway to many abilities some consider ... unnatural | 16:32 |
| zyga | haha, is that a star wars quote? | 16:32 |
| ijohnson | yes haha | 16:33 |
| * ijohnson goes away now and does real things | 16:33 | |
| * ijohnson well tries too anyways | 16:33 | |
| zyga | ijohnson: I pushed more fixes to https://github.com/snapcore/snapd/pull/9125 | 16:38 |
| mup | PR #9125: tests: fix issues related to restarting systemd-logind.service <Test Robustness> <âš Critical> <Created by zyga> <https://github.com/snapcore/snapd/pull/9125> | 16:38 |
| ijohnson | zyga: nice let me take a look, would be nice to land that | 16:38 |
| zyga | I will observe if it passes and we can then either review the ensemble or review in split branches | 16:38 |
| zyga | it's still a draft but please have a look for early impression | 16:39 |
| ijohnson | zyga: yeah I was just gonna say looking at this and the title it's not obvious how much you had to change, but yes +1 to splitting off the actual Go code changes | 16:39 |
| ijohnson | I'll still submit a review for you though | 16:39 |
| zyga | ok | 16:39 |
| zyga | the go code changes are optional | 16:39 |
| zyga | we were lucky | 16:39 |
| zyga | this is just for completeness | 16:39 |
| ijohnson | ah ok, I thought the Go code was necessary to fix the test | 16:40 |
| zyga | I thought so too but again, we were lucky :) | 16:40 |
| ijohnson | :-) | 16:40 |
| zyga | look at https://github.com/snapcore/snapd/pull/9125/commits/4b3bf6cbb08302b1878a9452e8ddcf9190ab8617 | 16:40 |
| mup | PR #9125: tests: fix issues related to restarting systemd-logind.service <Test Robustness> <âš Critical> <Created by zyga> <https://github.com/snapcore/snapd/pull/9125> | 16:40 |
| ijohnson | zyga: so does this present only on 4.15 kernel or is it 4.15+ ? | 16:40 |
| zyga | it's any version really | 16:44 |
| zyga | just that xenial's systemd does not mount v2 at all | 16:44 |
| zyga | so we noticed | 16:44 |
| ijohnson | ah right that makes sense | 16:48 |
| ijohnson | I just left a review | 16:48 |
| zyga | thanks! | 16:51 |
| zyga | replied | 16:53 |
| zyga | and looking at kernel docs first | 16:53 |
| pedronis | ijohnson: I'm not entirely sure whether #9136 is or isn't on the path I described in the previous, to be clear I'm not sure the path is an improvement, we would have to try, it has the propery of making boostate20, closer to bootstate16 | 17:42 |
| mup | PR #9136: bootstate20: rm bootState20Modeenv, pass around modeenv directly <Cleanup :broom:> <â›” Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9136> | 17:42 |
| ijohnson | pedronis: mmm sorry I didn't see that you want the modeenv in the structs to go away entirely, I thought you just wanted the modeenv struct to go away | 17:46 |
| pedronis | ijohnson: yes, my plan was to have it go away completely, that's why I was suprised that not touching initramfs bits was not a problem | 17:46 |
| zyga | ijohnson: it seems to be a kernel bug | 17:47 |
| zyga | I'll report it and add the reference to the code | 17:47 |
| zyga | ijohnson: I guess no bug report | 17:55 |
| zyga | ijohnson: quote from #lxcontainers | 17:56 |
| zyga | > <zyga> do you have a reference? I will link to that next to our workaround | 17:56 |
| zyga | <stgraber> nope, that kind of upstream discussion is all private IRC and telegram ;) | 17:56 |
| zyga | I don't mind, I guess that's enough for now | 17:56 |
| zyga | tests on https://github.com/snapcore/snapd/pull/9125 are in progress | 17:56 |
| mup | PR #9125: tests: fix issues related to restarting systemd-logind.service <Test Robustness> <âš Critical> <Created by zyga> <https://github.com/snapcore/snapd/pull/9125> | 17:56 |
| zyga | I'll review the services PR and EOD | 17:56 |
| stgraber | debating whether it's an issue or not, it's unlikely to be security relevant (as no controllers are auto-loaded on cgroup2) but still trying to argue that it could confuse badly written software on the host | 17:57 |
| zyga | stgraber: yeah, I think it's more surprising than dangerous | 17:58 |
| * zyga is tired | 17:59 | |
| zyga | maybe that review needs to wait | 17:59 |
| zyga | it's 8PM | 17:59 |
| * zyga EODs, more reviews tomorrow | 18:01 | |
| zyga | have a good rest of your day friends! | 18:01 |
| ijohnson | pedronis: ok, then in this case let's just abandon this PR and do it post uc20 1.0, I really don't know how we could get rid of modeenv entirely without changing a bunch of function/interface signatures and you're about to go on holiday, so not much point to trying to hash it out now I think | 18:03 |
| ijohnson | zyga: nice find thanks for looking into it more | 18:03 |
| stgraber | zyga: consensus seems to be that it's been that way since 2016, nobody is supposed to assume that because something is listed in /proc/self/cgroup that it's mounted, you need to look at mount table too (as controllers never go away) | 18:04 |
| zyga | ack | 18:04 |
| zyga | I always found that surprising btw, | 18:04 |
| zyga | that there are those nice CG identifier numbers | 18:05 |
| zyga | that are dangling and unrelated to the filesystem | 18:05 |
| zyga | and that you need to match on the controllers and mountinfo | 18:05 |
| zyga | it's probably uninteresting as future has cgroup2 only | 18:05 |
| zyga | but v1 was somewhat messy from this point of view | 18:05 |
| pedronis | ijohnson: yes, I expected we'll need to change some signatures | 18:07 |
| ijohnson | yeah I guess I totally missed that point sorry | 18:07 |
| ijohnson | I don't think it's worth it right now to try and simplify that at all, seems quite complex | 18:07 |
| ijohnson | IMHO this PR is still a win because it's less structs and less abstractions, but I suppose it is more type casting and slightly duplicative code, so I could see it not as a win too | 18:08 |
| ijohnson | if you don't think it's worth it I will just close it and we can revisit the refactor $SOME_DAY+2 now | 18:08 |
| pedronis | ijohnson: in light of the fact that we want to do something larger it doesn't feel like a win because of your point two about the extra duplication | 18:09 |
| cachio | zyga, hey | 18:15 |
| cachio | did you fix the test tests/core/uc20-recovery at some point right? | 18:16 |
| pedronis | ijohnson: can you explain me this change though: https://github.com/snapcore/snapd/pull/9136/files#diff-bff1ced30ed4581302ca81b7b53908f9L727 is that needed for that case? | 18:16 |
| cachio | ijohnson, or you did it? | 18:16 |
| mup | PR #9136: bootstate20: rm bootState20Modeenv, pass around modeenv directly <Cleanup :broom:> <â›” Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9136> | 18:16 |
| pedronis | ijohnson: isn't that needed | 18:16 |
| pedronis | ijohnson: ah, it's done lazily by revisions in the new code? | 18:17 |
| * cachio afk | 18:21 | |
| ijohnson | sorry was at lunch | 18:24 |
| ijohnson | pedronis: it actually was never needed | 18:24 |
| ijohnson | pedronis: we already setup the modeenv when we create the bootState20Base in initramfs.go | 18:25 |
| ijohnson | cachio: I think I looked at it and I thought we fixed something there | 18:25 |
| pedronis | ijohnson: ah | 18:25 |
| ijohnson | cachio: is it failing again now ? | 18:25 |
| pedronis | I forgot about that | 18:25 |
| ijohnson | yeah that's kinda why I disliked how InitramfsRunModeSelectSnapsToMount is working where it just creates the objects and calls a method on them and kinda forgoes all the rest of the stuff in that file | 18:26 |
| ijohnson | but meh | 18:26 |
| stgraber | zyga: I'm adding a tweak to our apparmor policy to prevent mounting cgroup2 if host doesn't have it, so we'll avoid that kind of issue for now | 18:28 |
| pedronis | ijohnson: now I'm very tempted to try to see if what I have in mind is smaller or not overall (it might not be, which again would make it a bit meh) | 18:29 |
| ijohnson | pedronis: I mean hey go for it if you want, feels a bit superfluous right now but it is fun to try and write simple elegant code sometimes :-D | 18:30 |
| pedronis | ijohnson: are you blocked on something from me? | 18:34 |
| ijohnson | pedronis: if you wanted to review the last commit on my cross check PR that is a draft that would be nice, but that is blocked on systemd-mount PR, which is then blocked on x n o x | 18:35 |
| ijohnson | pedronis: that would be https://github.com/snapcore/snapd/pull/9081/commits/147e3058ed55ff88e2c3aae0b598b346b6af5576 | 18:35 |
| mup | PR #9081: secboot,cmd/snap-bootstrap: cross-check partitions before unlocking, mounting <UC20> <â›” Blocked> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9081> | 18:35 |
| pedronis | ijohnson: that last commit says it's +1711 ? | 18:36 |
| ijohnson | pedronis: no github is silly, that diff line is for the whole PR I'm pretty sure | 18:36 |
| ijohnson | which contains all the systemd-mount PR | 18:36 |
| ijohnson | pedronis: git says ` 7 files changed, 424 insertions(+), 96 deletions(-)` | 18:37 |
| pedronis | yea, that seems more reasonable | 18:37 |
| mup | PR snapd#9135 closed: boot: move bootloaderKernelState20 impls to separate file <Simple 😃> <Created by anonymouse64> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9135> | 18:38 |
| ijohnson | thanks | 18:38 |
| pedronis | ijohnson: I did a quick pass there, some comments/questions | 18:44 |
| ijohnson | thanks | 18:45 |
| pedronis | ijohnson: so I tried, it gains 30 lines, not sure it's clearer or not, the cost is a new interface. There's probably a bit more that can be tried in initramfs.go | 19:30 |
| ijohnson | pedronis: mmm interesting, but yes there's a lot more that can be done in initramfs.go | 19:32 |
| ijohnson | I need to go to the bank | 19:32 |
| * ijohnson afk for hour or two | 19:32 | |
| mup | PR snapd#9137 opened: boot: refactor such that bootStateUpdate20 mainly carries Modeenv <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9137> | 19:43 |
| cmatsuoka | ijohnson: do you see a reason for uc20 installation to be failing on sd (on the nuc, not pi)? | 20:25 |
| ijohnson | cmatsuoka: by "sd" do you mean like /dev/sda or like an actual SD card | 20:27 |
| cmatsuoka | ijohnson: I always installed to internal storage on real pc hardware, it never occured to me to boot the image and install on external media | 20:27 |
| cmatsuoka | ijohnson: sd card | 20:27 |
| cmatsuoka | ijohnson: like a giant raspberry pi | 20:27 |
| ijohnson | Hypothetically it should work | 20:27 |
| ijohnson | Did you find an issue with that setup? | 20:27 |
| cmatsuoka | yes, I think it should too, but that's the scenario that doesn't work for galem | 20:28 |
| mup | PR snapd#9138 opened: many: refactor tpm seal parameter setting <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9138> | 21:28 |
| mup | PR snapcraft#3244 opened: file utils: introduce get_host_tool_path() to find commands on host <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3244> | 21:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!