zygagood morning07:02
zygamvo: I've shut down half of the workers now07:02
zygalet me know if there is any disruption07:02
mborzeckizyga: hey07:06
mvohey pstolowski08:01
mvozyga: thank you, excellent08:02
mupPR snapd#9724 closed: boot: observe successful command line update, provide a default  <Run nested> <UC20> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9724>08:05
mupPR snapd#9718 closed: secboot,devicestate: add scaffoling for "fde-reveal-key" support <Needs Samuele review> <Squash-merge> <UC20> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9718>08:10
mborzeckipstolowski: mvo; hey08:18
mvomborzecki: good morning!08:29
pedronis#9730 needs 2nd reviews08:55
mupPR #9730: hookstate: refactor around EphemeralRunHook <Squash-merge> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9730>08:55
mborzeckithe apparmor support situation on opensuse leap is confusing, on the one hand we downgrade the policy template, on the other snap-confine is built without apparmor support, so why bother with any support at all?09:17
mborzeckiand there's a special case for tw, where it's built with apparmor support and no policy downgrade09:18
mborzeckiso on leap we generate some profiles, but never transition to them09:19
mupBug #1906621 opened: System doesn't do FDE when installing with secure boot enabled <Snappy:New> <https://launchpad.net/bugs/1906621>09:27
mupBug #1906621 changed: System doesn't do FDE when installing with secure boot enabled <Snappy:New> <https://launchpad.net/bugs/1906621>09:30
mupBug #1906621 opened: System doesn't do FDE when installing with secure boot enabled <Snappy:New> <https://launchpad.net/bugs/1906621>09:36
mupBug #1906621 changed: System doesn't do FDE when installing with secure boot enabled <Snappy:New> <https://launchpad.net/bugs/1906621>09:42
jameshpedronis: bug filed here: https://bugs.launchpad.net/snapd/+bug/190662209:49
mupBug #1906622: UC20 system fails to seed when model contains snaps requiring experimental.user-daemons or dbus-activation <snapd:New> <https://launchpad.net/bugs/1906622>09:49
* mvo hugs mborzecki for reviewing 969509:50
mupBug #1906621 opened: System doesn't do FDE when installing with secure boot enabled <Snappy:New> <https://launchpad.net/bugs/1906621>09:51
pedronisjamesh: thx10:01
pstolowskipedronis: hey, does this structure look sensible? https://pastebin.ubuntu.com/p/tCSc7wznby/10:44
mupPR snapd#9738 opened: secboot: use `fde-reveal-key` if available to unseal key <Squash-merge> <UC20> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9738>10:56
pedronismborzecki: do you remember why we have EffectiveRole and EffectiveLabel, they don't to be used very consistently (especially for new UC20 stuff) ?10:58
pedronis*dont' seem10:58
mborzeckipedronis: iirc it was some backward compatibity hacks where things were left unspecified ing adget yaml11:00
pedronismborzecki: it's a bit of a mess11:00
pedronisnew code seems ignore them11:00
pedroniswhich seems not to provoke immediate problems, but long term is not a good state of things11:01
mborzeckipedronis: it should be used wherever we check Role, but i agree that it's not used and should be fixed11:02
mborzeckisame holds for label11:02
pedronisor we should set things to the value11:02
pedronishaving Label and EffectiveLabel is just asking to get it wrong11:02
pedronisor have only the accessor11:03
pedronisanyway bit of a tangle atm11:03
* pstolowski going to the dentist11:03
mborzeckithough there will probably be some weird undocumented cases where it's supposed to be explicit11:03
mborzeckipedronis: i can put it my queue11:03
pedronismborzecki: well, I'm looking into it right now because of the issue from yesterday11:03
pedronismborzecki: I'll see how far I can get11:03
mborzeckipedronis: ok11:03
pedronispstolowski: let's chat this afternoon, sorry, I was chasing something else11:06
mupPR snapd#9739 opened: interfaces/apparmor: fix generating of extended s-c AppArmor profiles with /usr/libexec <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9739>11:31
mupPR snapd#9740 opened: packaging/opensuse: enable AppArmor on Leap <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9740>11:41
zygamborzecki at the time this was a single decision11:51
zygamborzecki we should probably just build s-c with apparmor now11:51
mborzeckizyga: yup, there's 9740 for that11:51
mupPR snapd#9409 closed: cmd/snap: implement 'snap validate' command <validation-sets :white_check_mark:> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/9409>13:57
* zyga looks at export manager now14:14
zygamborzecki https://github.com/snapcore/snapd/pull/9740#pullrequestreview-54399673014:18
mupPR #9740: packaging/opensuse: enable AppArmor on Leap <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9740>14:18
om26erMy snap has a need to access `/usr/lib/libGLESv2.so.2` from the host, I have the opengl interface connected. However it seems that my snap's `/usr/lib` view is actually of the core20 snap and *not* the host's rootfs, which actually has the nvidia's libGL* libraries.14:19
mborzeckizyga: well, that requires a differnt fix then, because right now snapd acts silly when there's no s-c apparmor profile, and is completely oblivious to s-c not supporting aa at all14:21
mborzeckiom26er: can you run the app with SNAP_CONFINE_DEBUG=1 SNAPD_DEBUG=1 and post the logs somewhere? btw. is it a tegra device?14:21
zygaom26er try /var/lib/snapd/lib/gl14:21
zygategra is not supported AFAIK14:22
zygaas in, no gl support for tegra14:22
zygavia snapd14:22
om26ermborzecki, yes its a Nvidia Jetson Xavier NX, running a yocto based image.14:22
zygamborzecki why does snapd get confused when s-c has no apparmor enabled?14:23
zygaom26er no snapd support for nvidia on that platform14:23
zygaom26er if you want to know more I can explain but it's a long road to fix t hat14:23
mborzeckizyga: use has home on nfs: https://forum.snapcraft.io/t/snapd-doesnt-start-on-opensuse-leap-15-1/13710/10 and snapd tries to update s-c profile, which does not exist, so it just fails and exits14:23
zygamborzecki that's different14:24
zygawe should have the profile14:24
zygaand not use it14:24
om26erzyga I was able to "fix" that by copying all the required libGL* files to `/var/lib/snapd/lib/gl`, maybe a patch to snapd would fix?14:24
zygaIMO that's safe14:24
mborzeckizyga: why have it when s-c has no aa?14:24
om26erthat of course is a hack14:24
zygaom26er a dirty hack might work but that's not the right way14:24
zygamborzecki for consistency and simplicity14:24
zygamborzecki it's one to not use aa14:24
zygamborzecki: it's something else to special case lots of places14:24
zygaom26er there are the following concerns:14:25
zygaom26er: hosts provides some files for "GL" support, those differ across hardware and across OSes for the same hardware14:25
mborzeckizyga: shipping apparmor profile without aa is a special case on its own14:25
zygamborzecki I disagree, many packages ship unused profiles14:25
zygaand offer admins to enable them14:25
zygaI see it as lower cost than special fixes in code14:25
zygaom26er: apps need to load those libraries and find appropriate support files14:26
zygaom26er that's the general problem14:26
zyganow for nvidia specifically14:26
zygawe have two pieces of code, compile time choice, of how apparmor is "provided"14:26
zygaone is assuming everything is in a directory that can be bind mounted14:26
zygathe other assumes that the files are in /usr/lib or similar, and a symlink farm is constructed14:26
zygathose differ in sandbox description required to use them14:27
zygabefore I left Canonical I wanted to build a different way that's no longer ugly14:27
zygaand also allow snaps to provide those libraries on some systems14:27
zygaeither from snap to host or from snap to other snaps14:27
zygabut also allow the host to describe the files and allow classic host to provide the libraries to snaps14:28
zygathere's some work towards that14:28
zyganotably the export manager is nearly everything required to make that possible, there's still more work required after the manger is merged14:28
zygabut it gives you a way to tell that the host can just export the libraries and the snaps consume them14:28
zygathen snap-confine code can go away14:28
zygaand systems with other packaging, like debian even, can be made to work by adjusting packaging in the os (in debian) without patching snapd14:29
zygathe idea is that the export manager creates a tree of libraries14:29
zygaand the provided no longer matters14:29
zygait could be provided from snaps later on14:29
zygabut can, most importantly, provided by the host in a way that is not hard-coded in snapd14:30
zygaom26er let me know if you are interested in pursuing that14:30
om26erIn our case all the required libraries do live in /usr/lib, these are the one's that I had to copy https://gist.github.com/om26er/b4aaf190f5ce3629067bd18b5ff502f214:30
zygaom26er you probably want to build snap-confine with different config option then14:30
zygait's an acceptable stop-gap even though the code was never tested on tegra14:30
zygaor !x86 "fork" of nvidia14:31
zygasome arm builds are more like x8614:31
zygategra is not IIRC14:31
om26erzyga since I "build" the image, the do have the option to copy the required so files to a location from where snapd could read them. All in all we are looking for a solution that wouldn't break easily14:32
zygawell, as long as you upgrade everything in lockstep that's possble14:32
zygaI would still call it a hack though14:32
om26erSo how would a change to snap-confine look like ?14:32
zygaom26er there's a binary choice of nvidia support mode14:32
zygause the other one14:32
zygait looks for libs in /usr/lib14:33
zygathe first one bind mounts the whole /usr/lib/nvidiasomething directory14:33
zygahow do you configure snap-confine today?14:33
om26erzyga nothing special, here is the build code that we stole from morphis's project https://github.com/crossbario/meta-snappy/blob/master/recipes-support/snapd/snapd_2.47.1.bb14:34
zygaI forgot which value is default14:34
zygayou're not explicitly using any14:34
zygahttps://github.com/crossbario/meta-snappy/blob/master/recipes-support/snapd/snapd_2.47.1.bb#L68 you need more services14:35
pstolowskipedronis_: would you like to discuss validation set error i pasted earlier?14:39
om26erI think I should probably look at snapd' deb packaging ?14:39
pstolowskipedronis_: or shall I just push to the PR  for re-review?14:39
om26eralso apart from /usr/lib, the libglvnd vendor directory is also needs to be "read", namely `/etc/glvnd/egl_vendor.d`14:40
=== pedronis_ is now known as pedronis
pedronispstolowski: if you have time now, yes I would like to discuss a couple of things about it14:41
om26erand `/etc/egl/egl_external_platform.d/`14:41
pedronispstolowski: they relate also to local vs store snaps14:41
om26erzyga re more services: Which one's do you have in mind ?14:42
mborzeckiom26er: reminds me i have this outstanding pr to meta-snappy to review :/14:43
pstolowskipedronis: ok, coming to standup HO14:43
om26ermborzecki I was planning to pursue that. Especially now that we have apparmor kernel patches for full confinement for multiple platforms.14:44
om26erqemu, linux-raspberrypi-5.4 and linux-tegra-4.914:45
om26er^ of course, those patches are 99% Canonical's, we only fixed minor conflicts etc ;-)14:46
mborzeckiom26er: let me take a look at that tomorrow morning and we can thing about taking that further14:46
mupPR snapd#9741 opened: boot: add sealKeyToModeenvUsingFdeSetupHook() <Squash-merge> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9741>14:47
om26ersure, sounds good :+1:14:47
mborzeckizyga: kida meh https://paste.ubuntu.com/p/cnRQ7xMgpJ/ i would like to avoid having another configure switch14:48
mborzeckizyga: so in theory distro could decide to not ship the file at all, but then snapd behaves silly again if it finds sufficient kernel & userspace support14:49
mborzeckii think those 2 should be separate, if there's no aa profile for s-c snapd should not try to fix it even if aa is supported by userspace and the kernel14:50
mborzeckiheh, type=AVC msg=audit(1607007092.399:196): apparmor="DENIED" operation="exec" profile="/usr/lib/snapd/snap-confine" name="/usr/lib/snapd/snap-update-ns" pid=24802 comm="snap-confine" requested_mask="x" denied_mask="x" fsuid=0 ouid=014:53
mborzeckizyga: there's no x in s-c profile for s-u-n :/ so one canno really ship the profile for s-c without enabling other bits14:55
* cachio lunch14:59
om26erI wonder what would happen if I add /var/lib/snapd/hostfs/usr/lib/ to LD_LIBRARY_PATH ? The opengl interface already allow "read" access to quite a few so files there.15:00
zygaom26er tip, use SNAP_LIBRARY_PATH15:01
ijohnsonpedronis: thinking about it, for that cloud-init thing, we probably want to _add_ that config key to existing zzzz_snapd.cfg's right? or do you think it's sufficient to just add it to new ones given that this new feature from multipass is the only entity that seems to require it enough to file a bug for us?15:02
pedronisijohnson: let's start with a fix for new install and then see15:03
ijohnsonI think I should be able to also write a spread test for this case too given what they have in multipass is for a VM15:03
om26erzyga that kind of "worked". There are still a few so files that the opengl interface does not allow:15:13
om26erlibnvdc.so  libnvimp.so  libnvos.so  libnvrm.so  libnvrm_gpu.so  libnvrm_graphics.so15:13
om26erWould a PR for opengl interface be acceptable ?15:13
zygaom26er that's another aspect why this is hacky and annoying15:13
zygaom26er perhaps, I dont' see why not15:13
zygait's just not scalable in general15:13
om26erYeah, I understand it's not scalable, so would definitely love to have a real fix and even help where I can. Wouldn't be fun to have our UI break because Nvidia decided there is now a new .so file required as well15:16
om26erbtw: That confined snap, would run on Weston's Kiosk Shell as full screen. (It kind of already works, with a few minor hacks)15:17
ograerm ... nvidia libs are mapped to /var/lib/snapd/lib/gl usually (and fully accessible by the opengl interface this way) ... you dont need to access hostfs at all15:20
zygamvo FYI: all workers are shut down,I'll kill the instancenow15:20
zygaogra not exactly right15:20
zygait depends15:20
ograon what ?15:20
zygaon build config15:20
zygathere are two separate nvidia support versions15:21
zygaone does depend on hostfs15:21
ograthe desktop launchers definitely map these libdirs for you automatically15:21
ograand that usually works very well for desktop and kiosk apps15:21
zygano, that's not true either15:21
zygaI mean15:21
zygasnapd provides the libraries15:21
zygahelpers just use them15:21
ograright and i.e. the gnome extension auto-maps them into the library path15:21
mvocachio: zyga shut down the other instances now15:22
ograwith its desktop-launch script15:22
zygaogra that's correct15:22
ograthe only thing i found missing yet was VDPAU in some exotic setups15:22
ogra(interestingly intel ... not nvidia actually ... )15:23
cachiomvo, nice, I'll start adding more in some minutes15:23
zygahw support bits are a bit missing in snapd15:24
zygaI hope the export manager unlocks that15:24
zygaas custom hacks cannot scale15:24
zygamborzecki interesting15:25
zygayeah, sadly apparmor is used in two ways15:25
zygaand explicitly15:25
zygathe explicit part we can control15:26
zygathe implicit one we cannot15:26
zygacan you compile snap-confine without apparmor support15:26
zygaso that when snap-update-ns runs, it is not confined either?15:26
zygamborzecki looking at your diff15:27
zygado not load profiles15:27
zygaif disabled, just keep them off15:27
zygaotherwise snap-confine _is_ confined anyway15:27
zygaand we don't want it to15:27
zygaout of our set, snap-confine is the only program that uses implicit path-based profile association15:27
pedronismvo: ijohnson: mborzecki: proposed https://github.com/snapcore/snapd/pull/974215:28
mupPR #9742: gadget,o/devicestate: hybrid 18->20 ready volume setups should be valid <Created by pedronis> <https://github.com/snapcore/snapd/pull/9742>15:28
ijohnsonpedronis: ack thanks I'll have a look today15:29
pedronisit's just the fix plus some gadget update tests15:29
pedronisto be clear15:29
pedronisbut it's a baseline to then try to untangle things15:29
mborzeckizyga: apparmor.service will load the profile when restarted anyway15:32
mupPR snapd#9742 opened: gadget,o/devicestate: hybrid 18->20 ready volume setups should be valid <Created by pedronis> <https://github.com/snapcore/snapd/pull/9742>15:32
mupPR snapd#9743 opened: daemon: split unsupported buy implementation to its own api_*.go files <Cleanup :broom:> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9743>15:32
pedronisijohnson: btw let me know if you have questions about my lk comments16:16
ijohnsonpedronis: yes I was going through them, I will ask questions in a couple minutes16:16
ijohnsonpedronis: instead of Go build tags what do you think of trying to type cast *os.PathError to Unwrap() and if it returns nil, then we do the compat thing, and if it does return non-nil just return it as-is ?16:18
pedronisijohnson: that also works16:19
ijohnsonessentially what Go source is doing here: https://golang.org/src/errors/wrap.go?s=372:400#L416:19
ijohnsonok, I think that is more managable16:19
ijohnsonit just feels a bit heavy to have an entire set of multiple files per go versions that just have one method16:19
pedronismvo: cachio: I'm seeing the cla check fail sometimes like this: https://github.com/snapcore/snapd/pull/9736/checks?check_run_id=1493790248 are we missing deps on some agents?16:20
mupPR #9736: o/devicestate: save model with serial in the device save db <Run nested> <Squash-merge> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9736>16:20
cachiopedronis, let me check16:20
cachiopedronis, this ran in canonistack16:21
cachiothe old env16:21
cachioI'll monitor the other action jobs to see where it is happening16:22
cachiothanks for the ifno16:22
pedronismvo: #9730 can be squash merged16:34
mupPR #9730: hookstate: refactor around EphemeralRunHook <Skip spread> <Squash-merge> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9730>16:34
pedronis#9705 needs 2nd reviews16:35
mupPR #9705: devicestate: add runFDESetupHook() helper <Squash-merge> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9705>16:35
pstolowskipedronis: the error struct we discussed still needs to be slightly sophisticated, reason is that AFAIU  a snap may be required by multiple validation sets (if they are not conflicting); so, something like map[string]map[string]bool is needed, or map[string][]string ?16:35
pstolowski(for each of: missing, required, wrongrevision)16:36
pedronis[]string seems ok16:36
pedronispstolowski: ^16:40
pstolowskipedronis: yes, ok16:40
ijohnsonpedronis: ok, so I think I have most of your comments handled, but you said something about "maybe we need two level of helpers", that bit is unclear to me, I eliminated backupEnvFile and removed the `if` from partLabelForRoleAndTime so it's now just partLabelForRole and the callers of that function do the appending of ".bin" themselves17:12
pedronisijohnson: the two level helper is because of my wondering about envFileForPartName, it seems to be used for the actually boot partitions as well17:16
pedronisso the name is off17:16
ijohnsonah I see so that relates to your comment about using envFileForPartName being used in ExtractKernelAssets17:16
pedronisyes, but I had probably a different org in mind, you said you removed backupEnvFile which I didn't quite expect17:17
pedronisbut that's ok17:17
pedronisso I don't know if we need two helper or not17:17
pedronisbut that name is off17:17
pedronisgiven usage17:17
ijohnsonpedronis: what about devFileForPartName ?17:17
pedronisijohnson: that sounds ok in principle, I just wonder a bit how envFile looks like atm17:18
cachiomvo, today is comming 2.48.1 right?17:19
ijohnsonenvFileForPartName would become maybe devFileForPartName17:19
ijohnsonpedronis: thoughts ?17:23
mvocachio: yes, to beta in ~1h for snapd17:24
mvocachio: I'm not sure we will do a core release but probably for symetry17:24
mvocachio: but less urgent17:24
cachiomvo, nice17:24
* cachio afk 20mins17:26
pedronisijohnson: that looks ok, the bool doesn't read great though17:26
ijohnsonyeah I have constants for that though17:26
ijohnsonconst (17:26
ijohnsonbackupEnvFile  = true17:26
ijohnsonprimaryEnvFile = false17:26
ijohnsonbut agreed it's clunky17:26
pedronisdevPathForPartName ?17:27
pedronisI think we might also want to acknowledge the reality that envFile is not always/primarely a file17:28
pedronisnow that is not exposed anymore because we have Present17:28
pedronismight that should be called envBackstore()17:28
pedronisijohnson: ^17:30
ijohnsonmmm that sounds better17:30
ijohnsonlet me add that17:30
mupPR snapd#9744 opened: OpenGL interface: Support more Tegra libs <Created by om26er> <https://github.com/snapcore/snapd/pull/9744>17:43
ijohnsonpedronis: oh wait actually I have more more question, you mentioned `validate()` in one of your last comments about checking if PrepareImageTime is true that Role is Sole or Recovery, but we don't have a validate() method?17:46
ijohnsondid you mean to return an internal error from Present()17:46
om26erfyi @zyga https://github.com/snapcore/snapd/pull/974417:48
mupPR #9744: OpenGL interface: Support more Tegra libs <Created by om26er> <https://github.com/snapcore/snapd/pull/9744>17:48
zygaom26er replied17:49
pedronisijohnson: we do on opts17:51
ijohnsonoh right I see at that level17:51
pedronisI think it would be always correct, is just that only lk cares17:52
pedronisunless I'm confused17:52
mupPR snapd#9745 opened: [RFC] seed: enable uc20 devmode snaps in dangerous models <Bug> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9745>17:53
zygaom26er look again please :)17:57
om26ersure ;-)17:58
mupPR snapd#9746 opened: bootloader: add check for prepare-image time and more tests validating options <Simple 😃> <Skip spread> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9746>18:08
ijohnsoncachio: CLA check failed again https://github.com/snapcore/snapd/pull/9695/checks?check_run_id=149439885618:10
mupPR #9695: bootloader/lk: add support for UC20 lk bootloader with V2 lkenv structs <Squash-merge> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9695>18:10
=== ijohnson is now known as ijohnson|lunch
mupPR snapd#9730 closed: hookstate: refactor around EphemeralRunHook <Skip spread> <Squash-merge> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9730>18:13
pedronisijohnson|lunch: I'm seeing core20/degraded failing on one of my PRs but it passed before on it and nothing related to it changed, is it flaky?19:01
cachioijohnson|lunch, updated all the canonistack and prodstack agents and installed the cla dependency19:32
cachioit is really weird it didn't fail before19:33
=== ijohnson|lunch is now known as ijohnson
ijohnsonpedronis: hmm I haven't seen it fail, I don't know that it is any more flaky than the nested tests are in general20:08
ijohnsonif you have logs that would be good to look at20:08
ijohnsoncachio: thanks, I'll let you know if I see it fail anywhere else again20:09
cachioijohnson, nice ,thanks20:13
mupPR snapd#9747 opened: interfaces/builtin/log_observe.go: allow controlling apparmor audit levels <Needs security review> <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9747>21:59
pedronisijohnson: it failed here:  https://github.com/snapcore/snapd/pull/9736/checks?check_run_id=1494221544 it's the saving serial one22:03
mupPR #9736: o/devicestate: save model with serial in the device save db <Run nested> <Squash-merge> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9736>22:03
ijohnsonpedronis: thanks I'll have a look22:03
pedronisijohnson: thx22:08

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