zygamvo: good morning06:07
zygamvo: I will start focused work late; Lucy is still sleeping today (yesterday she woke at 6 AM)06:07
mvozyga: sure, that's fine. if you want you can review 9450, I would love to land this today and samuele is mostly happy06:26
zygamvo: sure, on it06:26
mvozyga: at least happy enough to not want to do a review anymore :)06:26
mvozyga: I will pop out for 10min to get my glasses fixed, they are misaligned :/06:26
zygasure, good luck06:27
mvozyga: cool, thanks! will address the feedback right away06:46
mvogood morning pstolowski07:00
mvopstolowski: 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/easier07:01
zygamvo: today I will work on feedback from samuele but I would love to land the notifications PR07:01
mvozyga: 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 details07:03
mvozyga: but definitely on my list for today too07:03
pstolowskimvo: yes, sure07:04
* zyga nods07:04
mvozyga: updated my PR07:13
zygamvo: I think this is still a good question: https://github.com/snapcore/snapd/pull/9450#discussion_r49863799907:14
mvozyga: I replied, did I misunderstood the question?07:15
mvozyga: i.e. if x.clientSnapshotImport() returned successfully we know the snapshot is saved with the returend id07:16
zygamvo: I mean we do this07:17
zygawhich sends the snapshot to snapd07:18
zygathat's persistent07:18
zygathen we do another command? savedCmd07:18
zygawhat is that for?07:18
zygaI was asking to understand what is the effect if y.Execute() doesn't run at all07:18
mvozyga: aha, sorry. snap saved is just the equivalent of "snap show-snapshot-details"07:21
mvoThe saved command displays a list of snapshots that have been created07:21
mvopreviously with the 'save' command.07:21
zygajust informative?07:21
zygagreat, that was my concern07:21
mvomaybe I should add a comment :)07:21
mvothe name is a bit misleading without more context07:22
zyga+1 on both ideas07:22
mvozyga: what was the second idea :) ?07:23
zygamerging it07:23
mvozyga: hahaha07:23
mvozyga: ok07:24
mvozyga: I have the changes for the constant for the contenttype ready so will do a followup with those right away07:24
zygaLucy woke up07:24
mvozyga: take care of her, see you :)07:24
* mvo hugs zyga07:28
mvopstolowski: I replied to your question in 9450 :)07:38
mvo(hope it makes sense)07:38
pstolowskity, +107:40
pstolowski(+1 to the reply, still reviewing ;))07:40
mvopstolowski: haha, ok07:49
mvopstolowski: no worries :)07:49
pstolowskimvo: +107:58
zygaback in the office08:08
zygaLucy slept for so long she nearly won't notice mom is at work08:08
zygaFridays are easier and my wife will be home soon08:08
zygashe has such a good mood today :-)08:08
zygaon to coding08:08
mvopstolowski: just FYI, I'm deconflicting 9036 right now, it's a bit of work :)08:40
pstolowskimvo: i pushed export-import roundrip test08:40
pstolowskion real files08:41
mvopstolowski: \o/ thanks so much08:41
pstolowskii'm thinking about adding one more for checksum error08:42
mvopstolowski: go for it08:42
pstolowskimvo: yeah i imagine deconflicting is fun now.. thank you08:47
mvopstolowski: I deconflicted and pushed, I see one unit  error in backend that I did not touch, I can have a look in a wee bit08:50
mvopstolowski: meh, some artifacts left from the merge, fixing now08:51
pstolowskimvo: are you ok to address cleanup of interrupted imports separately?08:52
mvopstolowski: I think so08:52
pstolowskii think this would an isolated change, so good for a new PR08:53
mvopstolowski: I think that's a bit of a bigger task anyway (maybe not so bad)08:53
mvopstolowski: yes08:53
mvopstolowski: 9036 diff after merge looks correct now08:55
pstolowskimvo: great. i'll work on a better spread test, a new one (instead of growing snapshot-basic test)09:04
pstolowskibut it will be a followup09:06
mvopstolowski: great09:10
mvopstolowski: the snapshot test is now failing so I think I broke something, I'm looking now09:11
pstolowskimvo: hmm which one exactly (i haven't pull your changes yet, passing locally)09:12
mvopstolowski: ahah, I think it's just the test09:12
mvopstolowski: it was parsing output from "snap import" and that changed (at least that's what it looks like)09:12
mvopstolowski: I will fix it, no worries09:12
mvopstolowski: fixed with my latest push09:20
pstolowskigreat, ty09:21
mvopstolowski: I'm doing anohter review of 9036, I will probbly just to the tweaks myself, again, just fyi :)09:31
pstolowskimvo: ok, thanks, i'm fighting with a new unit test09:31
mvopstolowski: yeah, nothing in my comments is deep, all about small style tweaks etc (so far)09:32
mvopstolowski: it's funny, this one will be one of the best reviewed features we had in a while :)09:32
abeatohey, where is the initramfs shipped on UC20? is it in the core snap? I do not see it anymore09:51
zygaabeato: kernel perhaps?09:56
ijohnsonabeato: hi :-)09:57
abeatozyga, ah, wait, I think it was now embedded in kernel.eigi?09:58
ijohnsonabeato: yes it is in the kernel snap as zyga mentions09:58
abeatoijohnson, o/09:58
ijohnsonabeato: yes it's in the kernel.efi, it comes from the new snappy-dev/image ppa package ubuntu-core-initramfs09:58
abeatoijohnson, got it, thanks09:59
pstolowskimvo: i've a unit test for corrupted zip inside snapshot import, but will make it a followup, it's a bit heavy10:45
mvopstolowski: 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
pstolowskimvo: hmm i have something along these lines, let me push a new PR10:47
pstolowskibut as i said it's a bit heavy, maybe we can have something simpler10:48
mvopstolowski: yeah, please push10:48
mvopstolowski: I have a look, I only digged into this because there is the unit test TestImportCheckerError broken in 9036 right now10:48
mvopstolowski: does your new code create a meta.json? I think I need this10:49
pstolowskimvo: no, i corrupt archive.tgz10:51
pstolowskimvo: pushed #945710:51
mvopstolowski: thank you!10:51
mvopstolowski: it's the last commit, right?10:52
mvopstolowski: that's cool, thanks ! I think we are good, no duplication I think10:52
mvopstolowski: I keep poking at this and will push my update very soon that fixes the other side10:53
pstolowskimvo: yes, last commit in the new PR10:53
pstolowskii don't know, maybe this test is too much, it's easier for spread test10:54
pstolowskimvo: actually the spread test we have looks fine, i  didn't notice it grew a quite a bit wrt import/export10:59
zygatoday is not my day11:03
zygaeverything is slow or hard11:03
mvopstolowski: it's okay I think either way11:03
zygaI wanted to be able to have an action that closes the given app11:06
zygaby asking the window to close at the protocol level11:06
zygaand obviously there is nothing like that, or nothing documented at least11:06
zygaone one hand side we have a pid11:06
zygaon the other side we may have a wayland server or an x server11:06
zygaanyway, I won't rant11:07
zyga(more than I already did)11:07
mvopstolowski: 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 think11:41
mvopstolowski: but maybe I'm overdoing it, not sure, I try to be super careful which is maybe too much11:42
* mvo lunches first11:42
pstolowskimvo: ack, enjoy11:44
* ijohnson re-caffeinates11:59
Saviqjdstrand: 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:03
zygaSaviq yes12:13
zygaSaviq until we can fix it at the interface level12:13
zygaSaviq it's more likely /etc/apparmor* that you need to handle12:13
zygaIIRC apparmor.d12:13
zygaand perhaps other files12:13
zygaa layout means you need to bundle apparmor12:14
zygayou cannot use it to present apparmor from the base snap12:14
ijohnsonzyga: or, can't they upgrade to `base: core20` and then the focal apparmor_parser doesn't die and just complains?12:16
ijohnsonmaybe I misunderstood that aspect12:16
jdstrandijohnson: the policy loads. it may not do exactly what we want if moving to core2012:22
ijohnsonjdstrand: 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 workaround12:22
jdstrandijohnson: jjohansen could comment more fully. if probably works ok for the policy multipass is using12:26
ijohnsonjdstrand: what about docker ?12:26
ijohnsonfor docker, we just use the docker-default policy12:26
jdstrandI don't think recommending moving to core20 is a good workaround12:26
ijohnsonwhich is pretty "simple"12:26
ijohnsonok, fair enough12:26
* ijohnson was hoping it was that easy12:26
jdstrandsnapcraft does all kinds of different things with core20 and that is a heavy lift12:27
jdstrandit has been for all of my snaps for example12:27
jdstranddocker's policy is simple too, yes12:27
ijohnsonwell I think docker should be fairly easy to port since it's mostly go code iirc12:27
ijohnsonbut yes there is a cost due to snapcraft12:27
jdstrandijohnson: 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
jdstrandamurray: 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 not12:35
zygaamurray: I'd prefer a snap-update-ns approach instead, ideally tied to interfaces as we discussed13:04
zygasnap-confine changes are not incremental and cannot be applied until the host reboots, in most practical way, or causes the mount namespace to be discarded13:04
=== grumble is now known as Spooktober
cachioijohnson, https://trello-attachments.s3.amazonaws.com/5da8bc830df86851446e9d4e/5f741e434ea3af8ec75cac6a/6245aa7ee335beb7b9a7d25d73fc7a0e/snapd_2.47_(9607)_pi4-20_arm64.log14:29
cachiothis is the log for the uc20-recovery test14:29
=== philroche_ is now known as philroche
ijohnsoncachio: thanks looking now15:11
cachioijohnson, nice15:28
* cachio having lunch now 15:28
cachioijohnson, hey18:20
cachioany idea about uc20-recovery test?18:20
cachioI saw also an error in snapd-faiolver test for pi4 https://paste.ubuntu.com/p/n57MV6T5zW/18:21
cachionot sure if it could be related18:21
ijohnsoncachio: sorry I meant to reach out earlier but forgot18:21
cachioijohnson, np18:22
ijohnsoncachio: it's unclear to me if that uc20-recovery test is failing in the same way18:22
ijohnsoncachio: that was on a pi ?18:22
ijohnsonon arm64 specifically?18:22
cachiojust to want to make sure it is not an issue before promoting it to candidate18:22
ijohnsoncachio: also let me quick look at the snapd-failover test18:22
cachioWrite failed because Read-only file system18:23
cachioit is really weird18:23
ijohnsoncachio: that error is extremely weird18:23
ijohnsoncachio: I have no idea what could be causing that but I don't think it's related at all18:24
ijohnsoncachio: at least it is probably not related to the uc20-recovery failure18:24
ijohnsoncachio: also on the uc20-recovery failure is it 100% reproducible ?18:24
cachioijohnson, run twice and failed twice18:25
cachioso far is 100%18:25
ijohnsonsame way each time ?18:25
ijohnsonlet me have a look at the kernel there, perhaps the kernel for the pi is lagging behind the pc-kernel18:25
ijohnsoncachio: the uc20-recovery for this beta test is passing on amd64 though right?18:25
cachioijohnson, it failed to execute the tests18:28
ijohnsoncachio: the pi-kernel on arm64 at revision 210 is new enough to pass the uc20-recovery test18:29
ijohnsonwell it is supposed to be, I can see the new initramfs pkg in the kernel snap there18:29
ijohnsonso either it's some other bug or we didn't really implement the feature as described for arm6418:29
cachiopi3 also failed18:29
cachioall tests pass but uc20-recovery failed18:29
ijohnsoncachio: can you share failure log for amd64 for that uc20-recovery ?18:29
cachioijohnson, on amd64 I wasn't able to run any test so far for uc2018:30
ijohnsoncachio: I will try tonight to flash my pi4 with a uc20 fresh image and have a look around18:30
cachiobecause a problem with the base image18:30
ijohnsoncachio: what was the problem ?18:30
cachioI'll try here with a local vm18:30
cachiothe focal image that we used as base sometimes gets stuck18:31
ijohnsonah right the debian package of snapd fails to remove properly?18:31
cachioIt gets stuck downloading things18:32
ijohnsoncachio: ah ok18:32
cachiowith wget ..18:32
cachioI need to debug it18:32
ijohnsoncachio: 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 tonight18:32
ijohnsoncachio: did uc20-recovery pass anywhere you have tested yet?18:32
cachioijohnson, thanks18:32
cachioijohnson, no18:33
ijohnsonthanks for confirming18:33
* ijohnson afk for a bit18:33
cachioI run this test in 3 places18:33
cachiopi3 failed pi4 failed amd64 not executed yer18:33
cachioijohnson, I need to go now19:43
cachiowith amd64 image also failed19:44
cachioijohnson, https://paste.ubuntu.com/p/QfJCyHhyfN/19:44
cachioiti s a beta image19:44
cachioijohnson, I'll be back in 1hr aprox19:44
* cachio afk19:44
cmatsuokaIt's 5PM and 35 Celsius here19:56

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