[06:05] <zyga> o/
[06:07] <zyga> mvo: good morning
[06:07] <zyga> mvo: I will start focused work late; Lucy is still sleeping today (yesterday she woke at 6 AM)
[06:26] <mvo> zyga: sure, that's fine. if you want you can review 9450, I would love to land this today and samuele is mostly happy
[06:26] <zyga> mvo: sure, on it
[06:26] <mvo> zyga: at least happy enough to not want to do a review anymore :)
[06:26] <mvo> zyga: I will pop out for 10min to get my glasses fixed, they are misaligned :/
[06:27] <zyga> oh
[06:27] <zyga> sure, good luck
[06:36] <zyga> https://github.com/snapcore/snapd/pull/9450#pullrequestreview-500884377
[06:46] <mvo> zyga: cool, thanks! will address the feedback right away
[07:00] <pstolowski> morning
[07:00] <mvo> good morning pstolowski
[07:01] <mvo> pstolowski: review for 9450 would be great, I'm working on the feedback from zyga right now, I hope to land this today and that should make the backpart much smaller/easier
[07:01] <zyga> mvo: today I will work on feedback from samuele but I would love to land the notifications PR
[07:03] <mvo> zyga: yeah, will look at it today, a tiny bit of unit testing would be great there, maybe via some mocks, not sure yet, need to look at the details
[07:03] <mvo> zyga: but definitely on my list for today too
[07:04] <pstolowski> mvo: yes, sure
[07:04]  * zyga nods
[07:13] <mvo> zyga: updated my PR
[07:13] <zyga> looking
[07:14] <zyga> mvo: I think this is still a good question: https://github.com/snapcore/snapd/pull/9450#discussion_r498637999
[07:15] <mvo> zyga: I replied, did I misunderstood the question?
[07:16] <mvo> zyga: i.e. if x.clientSnapshotImport() returned successfully we know the snapshot is saved with the returend id
[07:17] <zyga> mvo: I mean we do this
[07:17] <zyga> x.client.SnapshotImport
[07:18] <zyga> which sends the snapshot to snapd
[07:18] <zyga> right?
[07:18] <zyga> that's persistent
[07:18] <zyga> then we do another command? savedCmd
[07:18] <zyga> what is that for?
[07:18] <zyga> I was asking to understand what is the effect if y.Execute() doesn't run at all
[07:21] <mvo> zyga: aha, sorry. snap saved is just the equivalent of "snap show-snapshot-details"
[07:21] <zyga> ah
[07:21] <mvo> The saved command displays a list of snapshots that have been created
[07:21] <mvo> previously with the 'save' command.
[07:21] <zyga> just informative?
[07:21] <mvo> correct
[07:21] <zyga> great, that was my concern
[07:21] <mvo> maybe I should add a comment :)
[07:22] <mvo> the name is a bit misleading without more context
[07:22] <zyga> :)
[07:22] <zyga> +1 on both ideas
[07:23] <mvo> zyga: what was the second idea :) ?
[07:23] <zyga> merging it
[07:23] <mvo> zyga: hahaha
[07:24] <mvo> zyga: ok
[07:24] <mvo> zyga: I have the changes for the constant for the contenttype ready so will do a followup with those right away
[07:24] <zyga> Lucy woke up
[07:24] <mvo> zyga: take care of her, see you :)
[07:28]  * mvo hugs zyga
[07:38] <mvo> pstolowski: I replied to your question in 9450 :)
[07:38] <mvo> (hope it makes sense)
[07:40] <pstolowski> ty, +1
[07:40] <pstolowski> (+1 to the reply, still reviewing ;))
[07:49] <mvo> pstolowski: haha, ok
[07:49] <mvo> pstolowski: no worries :)
[07:58] <pstolowski> mvo: +1
[07:59] <mvo> ta
[08:08] <zyga> back in the office
[08:08] <zyga> Lucy slept for so long she nearly won't notice mom is at work
[08:08] <zyga> Fridays are easier and my wife will be home soon
[08:08] <zyga> she has such a good mood today :-)
[08:08] <zyga> anyway
[08:08] <zyga> on to coding
[08:14] <mvo> lovely!
[08:40] <mvo> pstolowski: just FYI, I'm deconflicting 9036 right now, it's a bit of work :)
[08:40] <pstolowski> mvo: i pushed export-import roundrip test
[08:41] <pstolowski> on real files
[08:41] <mvo> pstolowski: \o/ thanks so much
[08:42] <pstolowski> i'm thinking about adding one more for checksum error
[08:42] <mvo> pstolowski: go for it
[08:47] <pstolowski> mvo: yeah i imagine deconflicting is fun now.. thank you
[08:50] <mvo> pstolowski: I deconflicted and pushed, I see one unit  error in backend that I did not touch, I can have a look in a wee bit
[08:51] <mvo> pstolowski: meh, some artifacts left from the merge, fixing now
[08:52] <pstolowski> mvo: are you ok to address cleanup of interrupted imports separately?
[08:52] <mvo> pstolowski: I think so
[08:53] <pstolowski> i think this would an isolated change, so good for a new PR
[08:53] <pstolowski> ok
[08:53] <mvo> pstolowski: I think that's a bit of a bigger task anyway (maybe not so bad)
[08:53] <mvo> pstolowski: yes
[08:55] <mvo> pstolowski: 9036 diff after merge looks correct now
[09:04] <pstolowski> mvo: great. i'll work on a better spread test, a new one (instead of growing snapshot-basic test)
[09:06] <pstolowski> but it will be a followup
[09:10] <mvo> pstolowski: great
[09:11] <mvo> pstolowski: the snapshot test is now failing so I think I broke something, I'm looking now
[09:12] <pstolowski> mvo: hmm which one exactly (i haven't pull your changes yet, passing locally)
[09:12] <mvo> pstolowski: ahah, I think it's just the test
[09:12] <mvo> pstolowski: it was parsing output from "snap import" and that changed (at least that's what it looks like)
[09:12] <mvo> pstolowski: I will fix it, no worries
[09:12] <pstolowski> ty
[09:20] <mvo> pstolowski: fixed with my latest push
[09:21] <pstolowski> great, ty
[09:31] <mvo> pstolowski: I'm doing anohter review of 9036, I will probbly just to the tweaks myself, again, just fyi :)
[09:31] <pstolowski> mvo: ok, thanks, i'm fighting with a new unit test
[09:32] <mvo> pstolowski: yeah, nothing in my comments is deep, all about small style tweaks etc (so far)
[09:32] <mvo> pstolowski: it's funny, this one will be one of the best reviewed features we had in a while :)
[09:51] <abeato> hey, where is the initramfs shipped on UC20? is it in the core snap? I do not see it anymore
[09:56] <zyga> abeato: kernel perhaps?
[09:57] <ijohnson> abeato: hi :-)
[09:58] <abeato> zyga, ah, wait, I think it was now embedded in kernel.eigi?
[09:58] <abeato> efi
[09:58] <ijohnson> abeato: yes it is in the kernel snap as zyga mentions
[09:58] <abeato> ijohnson, o/
[09:58] <ijohnson> abeato: yes it's in the kernel.efi, it comes from the new snappy-dev/image ppa package ubuntu-core-initramfs
[09:59] <abeato> ijohnson, got it, thanks
[09:59] <ijohnson> np
[10:45] <pstolowski> mvo: i've a unit test for corrupted zip inside snapshot import, but will make it a followup, it's a bit heavy
[10:47] <mvo> pstolowski: yeah, I'm also working in this area right now, we are actually testing for corrupted zips already indirectly via testImportCheckError, I'm tweaking the MockCreateExportFIles so that it can actually create real zip and we can avoid the entire open mocking (which is a bit confusing to me at least)
[10:47] <pstolowski> mvo: hmm i have something along these lines, let me push a new PR
[10:48] <pstolowski> but as i said it's a bit heavy, maybe we can have something simpler
[10:48] <mvo> pstolowski: yeah, please push
[10:48] <mvo> pstolowski: I have a look, I only digged into this because there is the unit test TestImportCheckerError broken in 9036 right now
[10:49] <mvo> pstolowski: does your new code create a meta.json? I think I need this
[10:51] <pstolowski> mvo: no, i corrupt archive.tgz
[10:51] <pstolowski> mvo: pushed #9457
[10:51] <mvo> pstolowski: thank you!
[10:52] <mvo> pstolowski: it's the last commit, right?
[10:52] <mvo> pstolowski: that's cool, thanks ! I think we are good, no duplication I think
[10:53] <mvo> pstolowski: I keep poking at this and will push my update very soon that fixes the other side
[10:53] <pstolowski> mvo: yes, last commit in the new PR
[10:54] <pstolowski> i don't know, maybe this test is too much, it's easier for spread test
[10:59] <pstolowski> mvo: actually the spread test we have looks fine, i  didn't notice it grew a quite a bit wrt import/export
[11:03] <zyga> today is not my day
[11:03] <zyga> everything is slow or hard
[11:03] <mvo> pstolowski: it's okay I think either way
[11:06] <zyga> I wanted to be able to have an action that closes the given app
[11:06] <zyga> by asking the window to close at the protocol level
[11:06] <zyga> and obviously there is nothing like that, or nothing documented at least
[11:06] <zyga> one one hand side we have a pid
[11:06] <zyga> on the other side we may have a wayland server or an x server
[11:07] <zyga> anyway, I won't rant
[11:07] <zyga> (more than I already did)
[11:41] <mvo> pstolowski: pushed some tweaks and lots of comments to 9036, I see what I can do after lunch but right now it needs quite a bit of love I think
[11:42] <mvo> pstolowski: but maybe I'm overdoing it, not sure, I try to be super careful which is maybe too much
[11:42]  * mvo lunches first
[11:44] <pstolowski> mvo: ack, enjoy
[11:59]  * ijohnson re-caffeinates
[12:03] <Saviq> jdstrand: hey, thanks for the heads-up, so recommendation for right now would be to bring `/etc/apparmor` and lay it out in the multipass snap?
[12:13] <zyga> Saviq yes
[12:13] <zyga> Saviq until we can fix it at the interface level
[12:13] <zyga> Saviq it's more likely /etc/apparmor* that you need to handle
[12:13] <zyga> IIRC apparmor.d
[12:13] <zyga> and perhaps other files
[12:14] <zyga> a layout means you need to bundle apparmor
[12:14] <zyga> you cannot use it to present apparmor from the base snap
[12:16] <ijohnson> zyga: or, can't they upgrade to `base: core20` and then the focal apparmor_parser doesn't die and just complains?
[12:16] <ijohnson> maybe I misunderstood that aspect
[12:22] <jdstrand> ijohnson: the policy loads. it may not do exactly what we want if moving to core20
[12:22] <ijohnson> jdstrand: ah ok, sorry that wasn't clear to me, is it understood whether that works or not? or do we need to do testing before suggesting that as an alternative workaround
[12:26] <jdstrand> ijohnson: jjohansen could comment more fully. if probably works ok for the policy multipass is using
[12:26] <ijohnson> jdstrand: what about docker ?
[12:26] <ijohnson> for docker, we just use the docker-default policy
[12:26] <jdstrand> I don't think recommending moving to core20 is a good workaround
[12:26] <ijohnson> which is pretty "simple"
[12:26] <ijohnson> ok, fair enough
[12:26]  * ijohnson was hoping it was that easy
[12:27] <jdstrand> snapcraft does all kinds of different things with core20 and that is a heavy lift
[12:27] <jdstrand> it has been for all of my snaps for example
[12:27] <jdstrand> docker's policy is simple too, yes
[12:27] <ijohnson> well I think docker should be fairly easy to port since it's mostly go code iirc
[12:27] <ijohnson> but yes there is a cost due to snapcraft
[12:29] <jdstrand> ijohnson: note, amurray has already started on this: https://github.com/snapcore/snapd/compare/master...alexmurray:fix-docker-support-apparmor-parser (he hasn't proposed it yet of course)
[12:29] <ijohnson> ack
[12:35] <jdstrand> amurray: fyi, that seems pretty reasonable at first glance. I can't dive in right now, but perhaps zyga will want to. otoh, I would be curious if that works correctly on classic and on ubuntu core. I'm glad to see that etc/apparmor and etc/apparmor.d are present in those snaps (verified locally). not sure why I thought they were not
[13:04] <zyga> amurray: I'd prefer a snap-update-ns approach instead, ideally tied to interfaces as we discussed
[13:04] <zyga> snap-confine changes are not incremental and cannot be applied until the host reboots, in most practical way, or causes the mount namespace to be discarded
=== grumble is now known as Spooktober
[14:29] <cachio> ijohnson, https://trello-attachments.s3.amazonaws.com/5da8bc830df86851446e9d4e/5f741e434ea3af8ec75cac6a/6245aa7ee335beb7b9a7d25d73fc7a0e/snapd_2.47_(9607)_pi4-20_arm64.log
[14:29] <cachio> this is the log for the uc20-recovery test
=== philroche_ is now known as philroche
[15:11] <ijohnson> cachio: thanks looking now
[15:28] <cachio> ijohnson, nice
[15:28]  * cachio having lunch now 
[18:20] <cachio> ijohnson, hey
[18:20] <cachio> any idea about uc20-recovery test?
[18:21] <cachio> I saw also an error in snapd-faiolver test for pi4 https://paste.ubuntu.com/p/n57MV6T5zW/
[18:21] <cachio> not sure if it could be related
[18:21] <ijohnson> cachio: sorry I meant to reach out earlier but forgot
[18:22] <cachio> ijohnson, np
[18:22] <ijohnson> cachio: it's unclear to me if that uc20-recovery test is failing in the same way
[18:22] <ijohnson> cachio: that was on a pi ?
[18:22] <ijohnson> on arm64 specifically?
[18:22] <cachio> just to want to make sure it is not an issue before promoting it to candidate
[18:22] <ijohnson> cachio: also let me quick look at the snapd-failover test
[18:23] <cachio> Write failed because Read-only file system
[18:23] <cachio> it is really weird
[18:23] <ijohnson> cachio: that error is extremely weird
[18:24] <ijohnson> cachio: I have no idea what could be causing that but I don't think it's related at all
[18:24] <ijohnson> cachio: at least it is probably not related to the uc20-recovery failure
[18:24] <ijohnson> cachio: also on the uc20-recovery failure is it 100% reproducible ?
[18:25] <cachio> ijohnson, run twice and failed twice
[18:25] <cachio> so far is 100%
[18:25] <ijohnson> same way each time ?
[18:25] <cachio> yes
[18:25] <ijohnson> ok
[18:25] <ijohnson> let me have a look at the kernel there, perhaps the kernel for the pi is lagging behind the pc-kernel
[18:25] <ijohnson> cachio: the uc20-recovery for this beta test is passing on amd64 though right?
[18:28] <cachio> ijohnson, it failed to execute the tests
[18:29] <ijohnson> cachio: the pi-kernel on arm64 at revision 210 is new enough to pass the uc20-recovery test
[18:29] <ijohnson> well it is supposed to be, I can see the new initramfs pkg in the kernel snap there
[18:29] <ijohnson> so either it's some other bug or we didn't really implement the feature as described for arm64
[18:29] <cachio> pi3 also failed
[18:29] <cachio> all tests pass but uc20-recovery failed
[18:29] <ijohnson> cachio: can you share failure log for amd64 for that uc20-recovery ?
[18:30] <cachio> ijohnson, on amd64 I wasn't able to run any test so far for uc20
[18:30] <ijohnson> cachio: I will try tonight to flash my pi4 with a uc20 fresh image and have a look around
[18:30] <cachio> because a problem with the base image
[18:30] <ijohnson> cachio: what was the problem ?
[18:30] <cachio> I'll try here with a local vm
[18:31] <cachio> the focal image that we used as base sometimes gets stuck
[18:31] <ijohnson> ah right the debian package of snapd fails to remove properly?
[18:31] <cachio> no
[18:32] <cachio> It gets stuck downloading things
[18:32] <ijohnson> cachio: ah ok
[18:32] <cachio> with wget ..
[18:32] <cachio> I need to debug it
[18:32] <ijohnson> cachio: I need to step out for lunch and will not be back for a bit to run some errands as well but I will look at uc20 on the pi tonight
[18:32] <cachio> sure
[18:32] <ijohnson> cachio: did uc20-recovery pass anywhere you have tested yet?
[18:32] <cachio> ijohnson, thanks
[18:33] <cachio> ijohnson, no
[18:33] <ijohnson> ok
[18:33] <ijohnson> thanks for confirming
[18:33]  * ijohnson afk for a bit
[18:33] <cachio> I run this test in 3 places
[18:33] <cachio> pi3 failed pi4 failed amd64 not executed yer
[18:33] <cachio> yet
[19:43] <cachio> ijohnson, I need to go now
[19:44] <cachio> with amd64 image also failed
[19:44] <cachio> ijohnson, https://paste.ubuntu.com/p/QfJCyHhyfN/
[19:44] <cachio> iti s a beta image
[19:44] <cachio> ijohnson, I'll be back in 1hr aprox
[19:44]  * cachio afk
[19:56] <cmatsuoka> It's 5PM and 35 Celsius here