[01:34] <mup> PR core20#86 opened: .travis.yml: use stable snapcraft now <Created by anonymouse64> <https://github.com/snapcore/core20/pull/86>
[05:22] <mborzecki> morning
[05:22] <mborzecki> school run and then a quick errand, back around 9
[07:01] <pstolowski> morning
[07:03] <pedronis> zyga-kaveri: hi, we are getting google:ubuntu-16.04-64:tests/main/lxd:snapd_cgroup_just_outside failures sometimes, also cgroup-tracking on arch sometimes fails
[07:03] <pedronis> zyga-kaveri: for example, https://github.com/snapcore/snapd/pull/9325/checks?check_run_id=1100227725
[07:03] <mup> PR #9325: strutil: add SortedListsUniqueMerge <Created by pedronis> <https://github.com/snapcore/snapd/pull/9325>
[07:06] <pedronis> mborzecki: hi, should we sync?
[07:07] <mup> PR snapd#9325 closed: strutil: add SortedListsUniqueMerge <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/9325>
[07:16] <pedronis> mvo: mborzecki: hello, should we sync?
[07:17] <mvo> pedronis: I'm trying to get an overview about the open PR over the night right now
[07:18] <mvo> pedronis: but yeah, a quick sync would be good I think
[07:20] <mborzecki> re
[07:20] <mborzecki> pedronis: sorry starting only now, i can join in 5
[07:20] <pedronis> mborzecki: mvo: ok, I can be in SU in 5
[07:21] <mvo> pedronis, mborzecki call we meet at :30 ? nice round time, only slightly more than 5min :)?
[07:22] <pedronis> that's fine too
[07:23] <mborzecki> mvo:  works for me
[07:52] <mup> PR snapd#9324 closed: secboot: adjust parameters to buildPCRProtectionProfile <Simple 😃> <UC20> <Created by cmatsuoka> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9324>
[08:02] <pedronis> mborzecki: I will push some tweak to the reasel PR, it seems some code was copied before applying feedback about errors
[08:12] <mborzecki> pedronis: ok
[08:17] <mup> PR snapd#9320 closed: boot: group SealKeyModelParams by model, improve testing <Run nested> <UC20> <Created by cmatsuoka> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9320>
[08:18] <pedronis> thx
[08:22] <zyga> good morning
[08:34] <mborzecki> zyga: hey
[08:35] <zyga> long night
[08:35]  * zyga EODd at about 1AM
[08:41] <pedronis> mborzecki: mvo: I merged master and fixed/adjusted things into #9322, it's ready for 2nd/3rd reviews
[08:41] <mup> PR #9322: boot: add call to reseal an existing key <Run nested> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9322>
[08:41] <mvo> pedronis: cool, will refresh and go over it again
[08:58] <mvo> pedronis: approved 9322 as well, funny that we have very similar comments there
[09:00] <mborzecki> pedronis: InstallHostFDEDataDir is avaialble in run mode?
[09:01] <pedronis> mborzecki: for that one we really need a different var for run mode
[09:01] <pedronis> I think
[09:02] <pedronis> Int
[09:02] <pedronis> InstallHost doesn't make sense during run mode
[09:03] <pedronis> mborzecki: we might need a helper that gives a dir based on mode, depends on details
[09:11] <mvo> pstolowski: I noticed the spread tests in 9221 tend to fail, are you on this or shall I have a look ? I'm reviewing this right now and was wondering
[09:13] <pstolowski> mvo: i'm working on it right now, apart from shellcheck errors it needs to be disabled on some systems because it's too intrusive e.g. for selinux
[09:13] <pstolowski> and also for core
[09:13] <pstolowski> (as it's messing up with /var/lib/snapd)
[09:20] <mvo> pstolowski: cool, I will just wait for the update then
[09:33] <ijohnson> hey mvo, good morning, did you restart tests in #9208 at all? the most recent run I see on that PR does not have the kernel-failover test failing, just the fundamentals test and preparing generally failing
[09:33] <mup> PR #9208: tests/nested/core20/kernel-failover: add test for failed refresh of uc20 kernel <Run nested> <Test Robustness> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9208>
[09:35] <mvo> ijohnson: I didn't restart, I though master was merged
[09:36] <ijohnson> mvo: ok, sorry just trying to understand your comment there cause you said the failover test was failing in that pr
[09:38] <mvo> no worries, just restart if needed
[09:39] <ijohnson> ok, just that I haven't seen the actual kernel-failover test fail and if you saw it fail I would be very interested in logs :-)
[09:40] <zyga> good morning ijohnson :)
[09:40] <ijohnson> morning zyga
[09:48] <zyga> gosh
[09:48] <zyga> gnome-shell is stuck
[09:48] <zyga> I have a full-screen prompt
[09:48] <zyga> like polkit
[09:49] <zyga> that prompts me about ssh key of a host
[09:49] <zyga> I have no idea why
[09:49] <zyga> there are two buttons that don't do anything
[09:49] <zyga> the machine is up, I can run stuff but I cannot get that modal popup to go away
[09:49] <zyga> I guess it's save and reboot for the new kernel anyway :/
[09:49] <zyga> but one argument against modal shell popups
[09:49] <zyga> and lack of QA
[09:50] <zyga> (this is on focal)
[09:55] <zyga> it's 2020 but I can ssh -X from a mac to ubuntu and run git gui
[09:55] <zyga> I'm happy
[10:01] <mborzecki> pedronis: do i remember correctly we shoudl always reseal if the boot chain contains an unasserted kernel?
[10:01] <pedronis> mborzecki: yes, there's a todo in bootchain how to achieve that
[10:02] <pedronis> mborzecki: see predictableBootChainsEqualForReseal
[10:03] <mup> PR snapd#9327 opened: overlord: assorted typos and miscellaneous changes <Simple 😃> <Skip spread> <Created by zyga> <https://github.com/snapcore/snapd/pull/9327>
[10:08] <zyga> man after this is all done I feel you guys should do a presentation about resaling
[10:09] <zyga> it seems to have grown into this large complex system all by itself
[10:09] <zyga> not that it's any different from the reality it models
[10:11] <ijohnson> I'm actually very curious if there's another linux distro out there that does full disk encryption with automatic resealing like this, I'm inclined to think not
[10:15] <zyga> ijohnson windows ;D
[10:15] <zyga> but jokes aside, it does work for windows
[10:15] <ijohnson> haha yeah
[10:15] <zyga> which is impressive as it shipped about ~ well boatload years ago
[10:15] <zyga> when was bitlocker introduced?
[10:16] <zyga> ijohnson also macos but there it's a bit different in implementation
[10:16] <ijohnson> yeah that's not using a generic tpm 2.0 afaik
[10:16] <ijohnson> (for mac)
[10:16] <zyga> I suspect apple's T2 is more like android encryption
[10:16] <ijohnson> yeah
[10:16] <zyga> yeah
[10:16] <zyga> yeah, T2 is totally different
[10:16] <ijohnson> I remember setting up bitlocker on windows 7 in like 2010
[10:17] <zyga> but it does handshake each hardware in the system
[10:17] <zyga> right, I was wondering if I remembered win7 had bitlocker in the pro edition
[10:17] <zyga> still
[10:17] <zyga> it's one hell of a thing to bring it to linux this way
[10:17] <zyga> I would love to see this eventually arrive to classic desktop for 22.04
[10:17] <zyga> even if you have some more rigidity in result
[10:18] <ijohnson> yeah the automatic nature of it makes it super useful for installing I think, if your system has tpm 2.0 + uefi, bam fde for free w/ no passphrase, etc.
[10:19] <zyga> I think this will be very popular for corporate users
[10:27] <zyga> oh my
[10:27]  * zyga realized unexport-content should be after unlink-snap
[10:31] <pstolowski> mvo: pushed to 9221
[10:31] <mvo> pstolowski: thanks, looking
[10:32] <zyga> I need more spread tests
[10:32] <zyga> or a check in all tests
[10:33] <mup> PR snapd#9327 closed: overlord: assorted typos and miscellaneous changes <Simple 😃> <Skip spread> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9327>
[10:33] <zyga> thanks mvo
[10:38] <mup> PR snapd#9328 opened: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[10:39] <mborzecki> pedronis: ^^
[10:44] <zyga> neat, no failures
[10:44] <zyga> let's add some spread tests now
[10:46] <mvo> pstolowski: let me poke at this a bit, I have some ideas
[10:48] <pstolowski> mvo: ok, thank you. i'm playing with it as well
[10:53] <mvo> pstolowski: how does http://paste.ubuntu.com/p/ZDk8fR5Pz9/ look (untested, but should give hte idea)?
[10:55] <zyga> mvo are we measuring space in snapshots or in the parent dir?
[10:55] <pstolowski> mvo: hmm i wonder if we will get correct size in snapd if it's not a single filesystem
[10:55] <pstolowski> yeah exactly, what zyga says
[10:56] <zyga> brb, need to stretch a little
[10:58] <zyga> pstolowski I wonder if we could stop snapd, mv all of var/lib/snapd over, mount a tmpfs, copy things and resume the test
[10:58] <zyga> bonus points for cleanup that just unmounts
[11:00] <pstolowski> yeah i had that initially but hit some issues (don't remember what that was anymore, probably solvable)
[11:04] <mvo> pstolowski: it looks like we test for the actual path so that should work, I can tweak and run locally to double check
[11:05] <pstolowski> mvo: i can try that, i like how it simplifies it and avoid space calc
[11:06] <mvo> pstolowski: yeah, that's the appeal
[11:19] <mborzecki> pedronis: hmm boot.Device needs to have Model() now too right?
[11:19] <pedronis> mborzecki: yes
[11:19] <pedronis> I knew about that but forgot to mention
[11:20] <pedronis> mborzecki: I reviewed #9328, mostly a lot of nitpicks
[11:20] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[11:20] <mborzecki> pedronis: cool, thanks (and thanks to mvo too!)
[11:20] <pedronis> mborzecki: thx for it
[11:24] <pstolowski> mvo: so this gives this - Download snap "hello-world" (29) from channel "stable" (link /var/lib/snapd/snaps/hello-world_29.snap /var/lib/snapd/cache/b07bdb78e762c2e6020c75fafc92055b323a6f8da3ab42a3963da5ade386aba11f77e3c8f919b8aa23f3aa5c06c844f9: invalid cross-device link) :(
[11:24] <pstolowski> mvo: which reminded me that i tried this approach before
[11:24] <pstolowski> mvo: that's because of our hardlink cache logic
[11:24] <pedronis> mborzecki: one more comment
[11:25] <pstolowski> i need a short break and lunch
[11:25]  * pstolowski lunch
[11:27] <mvo> pstolowski: yeah, just noticed this too, slightly annoying that our code is not more robust here, I think that's a bug in itself. oh well
[11:27]  * mvo also lunches
[11:28]  * zyga is bad at taking breaks
[11:28] <zyga> taking one now for real
[11:33] <pedronis> mborzecki: should I do a small PR to complete predictableBootChainsEqualForReseal ?
[11:34] <mborzecki> pedronis: go ahead
[11:34] <mborzecki> thanks!
[11:34] <pedronis> mborzecki: made another remark
[11:58] <mup> PR snapd#9329 opened: boot: consider boot chains with unrevisioned kernels incomparable <Skip spread> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9329>
[11:58] <pedronis> mborzecki: done ^
[12:00] <ogra_> so in #9307 i added "tee" to  tests/lib/snaps/test-snapd-policy-app-consumer/meta/snap.yaml , yet the test still fails and does not seem to find my added lines ...
[12:00] <mup> PR #9307: interfaces/tee: add TEE/OPTEE interface <Needs security review> <Created by ogra1> <https://github.com/snapcore/snapd/pull/9307>
[12:00] <ogra_> https://github.com/snapcore/snapd/pull/9307/checks?check_run_id=1092075286 is just telling me to add it to that file
[12:01] <ogra_> am i missing anything (and do we perhaps have some doc where to typically turn the knobs to add the expected tests)
[12:06] <ogra_> (bah, there seems to be a missing space in the plug decl. ignore the question .. i'll ask again if it fails again)
[12:08] <pedronis> ijohnson: answered your review question
[12:10] <zyga>   hmm
[12:12] <ijohnson> pedronis: ack makes sense, but why skip spread on that PR?
[12:12] <zyga> sigh
[12:12] <pedronis> ijohnson: nothing is using that function yet
[12:12] <ijohnson> ok
[12:14] <pedronis> ijohnson: #9328 uses it, but in a helper that itself is not used yet, but ideally we should land this first then that
[12:14] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[12:14] <ijohnson> makes sense, I +1d it
[12:14] <ijohnson> I'm working through a review of 9328 now
[12:14] <ijohnson> well actually gonna make more coffee then will continue working through that
[12:14] <mborzecki> wow, seal tests are pita to set up
[12:15] <pedronis> mborzecki: yes, we might need some helper
[12:22] <ijohnson> mborzecki: hmm is it a typo in TestMakeBootable20RunMode that we use Revision: snap.Revision{N: 0}, for pc-kernel_1.snap ?
[12:22] <ijohnson> should that actually be snap.Revision{N:1}, ?
[12:22] <ijohnson> it's just unexpected to me that in the BootChain you read from the file at the end of that test we get `			KernelRevision: "",`
[12:23] <pedronis> that seems weird
[12:23] <pedronis> revisions starts at 1, 0 is unset
[12:23] <pedronis> if the goal is to make it look local/unasserted then its N: -1
[12:24] <ijohnson> yeah I think it's a typo from the previous PR
[12:24] <ijohnson> (which coincidentally I approved /o\ )
[12:26] <pstolowski> mvo: fyi my get_disk_usage helper has an issue in that spread test, i might have a fix
[12:29] <ijohnson> pedronis: I finished lookingat 9328, is there another pr I should look at before SU today?
[12:29] <ijohnson> s/today/right now/
[12:30] <pedronis> ijohnson: #9322 could use a 3rd review (given that I worked a bit on it myself)
[12:30] <mup> PR #9322: boot: add call to reseal an existing key <Run nested> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9322>
[12:30] <ijohnson> sure
[12:36] <pedronis> #9239 needs a 2nd review, it's small
[12:36] <mup> PR #9239: many: misc doc-comment changes and typo fixes <Simple 😃> <Skip spread> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9239>
[12:36] <ijohnson> oh?
[12:37] <pedronis> heh
[12:37] <pedronis> I meant #9329
[12:37] <mup> PR #9329: boot: consider boot chains with unrevisioned kernels incomparable <Skip spread> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9329>
[12:39] <ijohnson> mborzecki: are you still running arch? are you able to install the vokoscreen-ng snap via snap install ?
[12:53] <mup> PR snapd#9329 closed: boot: consider boot chains with unrevisioned kernels incomparable <Skip spread> <UC20> <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9329>
[12:58] <mup> PR snapd#9322 closed: boot: add call to reseal an existing key <Run nested> <UC20> <Created by cmatsuoka> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9322>
[13:28] <mup> PR snapd#9330 opened: boot: reseal when updating boot assets <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9330>
[13:47] <pedronis> mborzecki: I re-reviewed #9328
[13:47] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[13:50] <pstolowski> mvo: only unrelated failures in #9221 now
[13:50] <mup> PR #9221: tests: disk space awareness spread test <Disk space awareness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/9221>
[13:50]  * zyga errand 
[13:53] <mvo> pstolowski: cool, looking
[13:55] <cachio> zyga, when you have a momento could you take a look again to #9316
[13:55] <mup> PR #9316: tests: use full path to test-snapd-refresh.version binary <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9316>
[13:56] <ijohnson> cachio: I commented on your other similar PR that's the wrong way to fix this
[13:56] <ijohnson> we should figure out why /snap/bin is not on $PATH
[13:59] <cachio> ijohnson, you mean for the tests.tool right?
[13:59] <cachio> because it is on $PATH for snap run o if I just execute the snap
[14:00] <ijohnson> cachio: I mean this comment https://github.com/snapcore/snapd/pull/9315#issuecomment-690375166
[14:00] <mup> PR #9315: tests: fix snap-routime-portal-info test <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9315>
[14:04] <cachio> ijohnson, I am not sure yet but think the issue is related to the tests.session setup, but really couldn't find where
[14:05] <cachio> becuause it fails just when we run using tests.session
[14:06] <cachio> ijohnson, I mean, this works: su -c 'snap run test-snapd-refresh.version' test | MATCH v1
[14:24] <mborzecki> pedronis: cmatsuoka: hmm, current code assumes we always have a trusted assets bootloader :/
[14:24] <mborzecki> and fails otherwise
[14:24] <mborzecki> (i mean the reseal code)
[14:25] <pedronis> mborzecki: we should check that outside, if it's not trusted there is nothing to do, really
[14:25] <pedronis> we need to cleanup bootloader interface zoo
[14:25] <pedronis> but basically is like that atm
[14:26] <pedronis> mborzecki: do you need more input on this?
[14:27] <pedronis> mborzecki: well, outside, maybe inside, I don't know how you structured your helpers
[14:34]  * cachio afk 
[14:38] <zyga> cachio sure
[14:39] <zyga> cachio replied
[15:14] <mup> PR snapd#9331 opened: boot: reseal when changing kernel <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9331>
[15:16] <mborzecki> pedronis: mvo: cmatsuoka: more like an RFC really ^^
[15:17] <mvo> mborzecki: thank you!
[15:19] <cmatsuoka> mborzecki: I'll check after lunch, thanks!
[15:19] <mvo> mborzecki: why rfc? it looks pretty reasonable from a quick look
[15:19] <mvo> mborzecki: anything to watch out for?
[15:20] <mborzecki> mvo: hope i got all the places that need to be tweaked, the tests could use more work
[15:20] <mborzecki> and bootloadertest.MockTrustedBootAssetsBootloader is kind of getting in the way
[15:21]  * mvo nods
[15:21] <mborzecki> btw. the tests are spotty for calls to seal/reseal when the bootlaoder is not a trusted assets one
[15:21] <mborzecki> and probably the same if there's no trusted assets (even if bl implements the interface)
[15:24] <mborzecki> landing #9328 will make the other PRs considerably smaller
[15:24] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[15:29] <mborzecki> wth `src/github.com/snapcore/snapd/overlord/snapstate/check_snap.go:485:26: ambiguous selector deviceCtx.Model`?
[15:29] <mborzecki> but it builds just fine locally, maybe a go version change?
[15:37] <pedronis> mborzecki: IsResealNeed is not used yet, right?
[15:38] <mborzecki> pedronis: it's not
[15:42] <mborzecki> pedronis: cmatsuoka: mvo: do we need to sync? i'll need to wrap it up ~6 to take the kids to a swimming class
[15:45] <pedronis> mborzecki: I think the code is understable, anyway we need to land one piece at a time
[15:47] <pstolowski> i see recurring failures of google:ubuntu-16.04-64:tests/main/lxd:snapd_cgroup_just_inside
[15:47] <pedronis> yes, I mentioned that in the morning
[15:51] <pstolowski> right, not sure zyga saw your message
[15:52] <pedronis> there are some missing tests in 9328
[15:53] <pedronis> mmh
[16:02] <mborzecki> damn, i'm late, i'll check back in later
[16:03] <pedronis> mborzecki: sorry, my answer was a no
[16:03] <pedronis> maybe it wasn't clear
[16:13] <cmatsuoka> mborzecki: sorry, I was away, back now
[16:15] <pedronis> cmatsuoka: can we sync quickly?
[16:16] <cmatsuoka> pedronis: sure
[16:16] <pedronis> cmatsuoka: going to the standup channel
[16:32] <pedronis> cmatsuoka: https://github.com/snapcore/snapd/pull/9328#pullrequestreview-486951806
[16:32] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[16:32] <cmatsuoka> pedronis: ack
[16:34]  * pedronis dinner break
[16:55] <cachio> ijohnson, updated the PR following the zyga's recomendation
[16:55] <ijohnson> cachio: ok, which pr ?
[16:56] <cachio> ijohnson, #9315
[16:56] <mup> PR #9315: tests: fix snap-routime-portal-info test <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9315>
[16:57] <cachio> ijohnson, I think it is still open the fact that the env is not correct and needs to be fixed
[16:57] <cachio> ijohnson, there is  another one opened with that discussion #9316
[16:57] <mup> PR #9316: tests: use full path to test-snapd-refresh.version binary <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9316>
[16:57] <ijohnson> ok, as long as we know that's the issue and that should be fixed eventually I think it's fine to fix the way you did
[16:58] <ijohnson> cachio: can you add a TODO in tests.session somewhere to debug why that doesn't work?
[16:58] <ijohnson> doesn't have to be in these PR's, can be a different PR
[16:58] <cachio> ijohnson, sure, thanks!!
[17:20] <pedronis> ijohnson: nested tests failed here: https://github.com/snapcore/snapd/pull/9328 we need to understand why
[17:20] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[17:21] <ijohnson> pedronis: mmm looks like the generic problem we keep seeing where sometimes uc20 users don't get created or get created too slowly
[17:21] <pedronis> should I just try again?
[17:22] <ijohnson> just a second let me finish perusing the logs
[17:22] <ijohnson> I mean to be clear, yes there is a real problem with the nested uc20 tests somewhere and I'm doing my best to track it down
[17:23] <ijohnson> but just because they failed on this pr doesn't mean that this pr caused that
[17:25] <ijohnson> pedronis: yeah go ahead and restart that pr, there's nothing indicative in the logs
[17:26] <ijohnson> pedronis: note that the minimal-smoke test just timed out there, the other one we just never got a user login
[17:26]  * ijohnson lunches
[17:26] <pedronis> ijohnson: let's see how it goes, the PR adds writing one more file during install fwiw
[17:26] <pedronis> so it's not completely neutral
[17:34] <cachio> pedronis, hey, I am trying to fix some tests which require a model signed by canonical
[17:34] <cachio> pedronis, is it ok if I add a env var to set when the model is not signed by canonical so then we can skip some parts of the tests
[17:35] <cachio> because surrently the smapd-model test fails on beta validation because I am signing the models used for those images
[17:35] <pedronis> cachio: it depends, we have other option nowadays,  you can also put serial-authority: generic in your model
[17:36] <cachio> pedronis, ahh, right
[17:36] <pedronis> the test needs changes but not skipping full bits then
[17:36] <cachio> My idea is just to skip small parts of the test
[17:37] <cachio> I'll try adding serial-authority: generic first
[17:37] <pedronis> I understand my point is that you could set a var that says if we expect serial signed by canonical or generic etc but depends on details
[17:37] <pedronis> cachio: is it a lot of tests ? a few
[17:37] <pedronis> ?
[17:37] <cachio> pedronis, just a few
[17:38] <cachio> refresh-* and snap-model
[17:38] <pedronis> cachio: I don't have  a lot of time today, if you can make a list I could look next week?
[17:38] <cachio> pedronis, sure
[17:38] <cachio> thanks
[17:55] <mborzecki> re
[17:59] <cachio> pedronis, I think the problem is solved now, I moved all the model to canonical, I used to use my models because I needed a user assertion but now I am using cloud init in all the cases
[18:00] <cachio> so I'll just leave the model signed by me for nested tests where we test user assertions
[18:00] <cachio> pedronis, does it make sense?
[18:00] <pedronis> mborzecki: cmatsuoka: I just noticed, is not that we don't check if we have the right boot loader, we also don't check if we have a key or encrypted disk at all, in reseal? is that right?
[18:01] <cmatsuoka> hmm, this check will be needed somewhere
[18:01]  * cmatsuoka takes note
[18:01] <mborzecki> pedronis: yes, we don't
[18:02] <pedronis> ok, just double checking
[18:03] <mborzecki> heh, https://github.com/snapcore/snapd/pull/9330 panics (the unit tests are passing though)
[18:03] <mup> PR #9330: boot: reseal when updating boot assets <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9330>
[18:04] <pedronis> yes, I expect one I said is part of the problem
[18:04] <pedronis> *what I said
[18:04] <mborzecki> not sure why modeenv would be nil though there
[18:05] <pedronis> because nothing has change?
[18:05] <pedronis> maybe we have nop
[18:05] <pedronis> anyway I'm trying to look a bit around
[18:05] <pedronis> in the code
[18:05] <pedronis> to re-understand some things
[18:10] <mup> PR snapd#9332 opened: spread.yaml, tests/nested: misc changes <Run nested> <Simple 😃> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9332>
[18:15] <mup> PR snapd#9333 opened: tests/nested, fakestore: changes necessary to run nested uc20 signed/secured tests <Run nested> <Test Robustness> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9333>
[18:15] <mup> PR snapd#9334 opened: tests/nested/manual: add test for grades above signed booting with testkeys <Run nested> <Test Robustness> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9334>
[18:15] <pedronis> mborzecki: we don't track assets at install if we are not encrpyting, but we try at update
[18:15] <mborzecki> pedronis: yes, we create the observer, probably shouldn't be
[18:16] <pedronis> yes, trying to find the cleanest way to achieve that
[18:17] <mborzecki> pedronis: there's a checkEncryption helper in devicestate
[18:18] <pedronis> I know
[18:18] <pedronis> we don't want to use that
[18:18] <pedronis> it's more whether we should encrypt first time
[18:20] <mup> PR snapd#9065 closed: debug: forward systemd journald output to serial console when booting for uc20 <Precious Logs> <Run nested> <Created by anonymouse64> <Closed by anonymouse64> <https://github.com/snapcore/snapd/pull/9065>
[18:28] <pedronis> mborzecki: we can call BeforeWrite and then Cancel together right?
[18:29]  * ijohnson soft-eows
[18:31] <mborzecki> pedronis: cancel is called after before write, but it may also be called without before write (if we never got past the backup pass)
[18:32] <mborzecki> pedronis: added TODO that it makes no sense to reseal in canceled if we never resealed in before write
[18:40] <pedronis> mborzecki: this is not the end of the story, but does this change in itself make sense: https://paste.ubuntu.com/p/XrgxJSyrM9/
[18:41] <mborzecki> pedronis: got a bit more changes, i'll push them to 9330 in a bit
[18:42] <pedronis> mborzecki: changes about what?
[18:42] <mborzecki> pedronis: tests mostly
[18:42] <mborzecki> pedronis: but the change in BeforeWrite is almost the same
[18:42] <pedronis> mborzecki: sorry, I thought you were done coding
[18:42] <pedronis> for the day
[18:43] <mborzecki> pedronis: came back for a bit, high priority thing after all
[18:44] <pedronis> mborzecki: yes, but maybe we need to sync, I don't think we want to do the same things twice :)
[18:44] <cmatsuoka> I'll merge #9328
[18:44] <mup> PR #9328: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9328>
[18:45] <zyga> re
[18:45] <mup> PR snapd#9328 closed: boot: store boot chains during install, helper for checking whether reseal is needed <Run nested> <UC20> <Created by bboozzoo> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/9328>
[18:46] <mborzecki> pedronis: pushed a patch to this branch: https://github.com/snapcore/snapd/compare/master...bboozzoo:bboozzoo/uc20-reseal-with-boot-assets-wip (the last one)
[18:46] <mborzecki> pedronis: this one exactly: https://github.com/snapcore/snapd/commit/ed30e18ed284ce77893ea5031e3d871cbfca836e
[18:48] <pedronis> mborzecki: ok, better than my quick thing
[18:48] <mborzecki> pedronis: shall i push it?
[18:49] <pedronis> yes
[18:49] <mborzecki> ok
[18:49] <pedronis> code wise, that diff is all I did
[18:49] <pedronis> I'm thinking about something but haven't coded it yet
[18:50] <mborzecki> k, pushed now
[18:50] <mup> PR snapd#9316 closed: tests: use full path to test-snapd-refresh.version binary <Simple 😃> <Created by sergiocazzolato> <Merged by zyga> <https://github.com/snapcore/snapd/pull/9316>
[18:50] <mborzecki> ah, and 9328 landed, i can merge master too now
[18:51] <pedronis> yes
[18:52] <mborzecki> pushed, added 'run nested' label and closed/reopened so that we get a full run
[18:52] <mborzecki> eod for real this time, time to tuck the kids to bed
[18:55] <mup> PR snapd#9335 opened: boot: verify boot chain file in seal and reseal tests <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9335>
[19:02] <pedronis> ijohnson: could you review https://github.com/snapcore/snapd/pull/9335 ?
[19:02] <mup> PR #9335: boot: verify boot chain file in seal and reseal tests <Skip spread> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9335>
[19:13]  * cmatsuoka afk for 30min, virtual school meeting for parents
[19:18] <ijohnson> pedronis sure in a little bit
[19:19] <pedronis> thx
[19:22] <mvo> anything i can help withß
[19:22] <mvo> ?
[19:26] <ijohnson> +1d
[19:37] <pedronis> mvo: I think our reseal code has all the wrong dirs
[19:37] <mvo> pedronis: oh no!
[19:37] <mvo> ijohnson: did you see that the nested tests failed in 9334 =
[19:37] <mvo> ?
[19:38] <mvo> (sorry if I'm stating something that was already discussed)
[19:38] <mvo> pedronis: do you have more details somewhere?
[19:38] <pedronis> mvo: no, just staring at code, there's too much Install * in there
[19:39]  * mvo nods
[19:47] <mup> PR snapcraft#3283 opened: specifications: unified provider <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3283>
[19:49] <pedronis> mvo: maybe they work anyway, not sure
[19:49] <mvo> pedronis: I suspect we have the same /run/mnt mounts in run mode
[19:49] <mvo> pedronis: but I need to double check
[19:49]  * mvo checks
[19:56] <mvo> pedronis: I guess you mean  	InstallHostFDEDataDir = dirs.SnapFDEDirUnder(InstallHostWritableDir) which is  	InstallHostWritableDir = filepath.Join(InitramfsRunMntDir, "ubuntu-data", "system-data") ? we have /run/mnt/ubuntu-data also in run mode
[19:56] <mvo> (or am I misunderstanding?)
[19:56] <pedronis> yes, I mean those
[19:59] <pedronis> mvo: anyway #9330 can be reviewed (though I'm not sure is not missing some code to really work when we haven't encrypted)
[19:59] <mup> PR #9330: boot: reseal when updating boot assets <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9330>
[20:04] <ijohnson> mvo mmm no I did not I'll have a look
[20:04] <cmatsuoka> pedronis: it seems to be the case, I think it will fail if the system is not encrypted
[20:12] <mvo> pedronis, cmatsuoka should we makr 9330 as "ready" (i.e. move it out of draft?)
[20:12] <mvo> it looks reasonable to me
[20:12] <pedronis> mvo: yes
[20:15] <cmatsuoka> mvo: yes, I think so
[20:16] <mvo> ta
[20:24]  * cmatsuoka is getting confused with his local branches, time to remove a few of them
[20:24] <mvo> I merged master into 9331, it's a bit smaller now
[20:25] <pedronis> mvo: anyway I'm working on resealing only if we had sealed keys and if the boot chains are different
[20:25] <pedronis> that's a piece that no PR has atm
[20:25] <mvo> ok
[20:25] <pedronis> maybe we need to land 9330, maybe not
[20:25] <mup> PR snapd#9335 closed: boot: verify boot chain file in seal and reseal tests <Skip spread> <UC20> <Created by cmatsuoka> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/9335>
[20:26] <pedronis> sorry, I meant, maybe we need that logic already to land 9330 maybe not
[20:26] <mvo> pedronis: aha, right
[20:26] <pedronis> mvo: I looked at 9331 a bit, I think it will need a bit more work
[20:26] <pedronis> but haven't done a proper review
[20:27] <mvo> pedronis: ok, what's your hunch on what is missing?
[20:28] <pedronis> tests I think and the changes in bootstate20.go seems a bit too invasive (maybe they are really needd like that, not sure)
[20:28]  * mvo nods
[20:28] <mvo> thanks
[21:20] <ijohnson> pedronis anything for me to review before EOW ?
[21:21] <pedronis> ijohnson: I'm trying to finish something but is not quite ready yet
[21:22] <pedronis> defeated by SnapFDEDir = SnapDeviceDirUnder(rootdir)
[21:22] <pedronis> spot the error
[21:22] <ijohnson> should it have a filepath.Join somewhere?
[21:23] <pedronis> no s/Device/FDE/
[21:23] <pedronis> anyway apparently nothing is using it yet, except the test code I was writing
[21:23] <cmatsuoka> ouch
[21:24] <ijohnson> Ah
[21:27] <pedronis> now unit tests are passing, maybe I can propose this but is on top of #9330
[21:27] <mup> PR #9330: boot: reseal when updating boot assets <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9330>
[21:36]  * cmatsuoka needs to eod, but I'll check the PR at some point during the weekend
[21:36] <cmatsuoka> or he will, to be consistent
[21:39] <pedronis> ok, I proposed #9336 on top of #9330
[21:39] <mup> PR #9336: boot,many: do not reseal unless is meaningful and necessary <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9336>
[21:39] <mup> PR #9330: boot: reseal when updating boot assets <Run nested> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9330>
[21:39] <ijohnson> I'll try to take a look later tonight
[21:39] <ijohnson> Have a good weekend pedronis and cmatsuoka
[21:39] <cmatsuoka> thanks! you too
[21:40] <mup> PR snapd#9336 opened: boot,many: do not reseal unless is meaningful and necessary <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9336>
[21:42] <pedronis> seems I skipped one commit :/
[21:44] <pedronis> ijohnson: I'll have to close it and open it again, it's a bit confusing atm
[21:46] <mup> PR snapd#9336 closed: boot,many: do not reseal unless is meaningful and necessary <UC20> <Created by pedronis> <Closed by pedronis> <https://github.com/snapcore/snapd/pull/9336>
[22:05] <pedronis> reproposed as #9337
[22:05] <mup> PR #9337: boot,many: reseal only when meaningful and necessary <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9337>
[22:06] <mup> PR snapd#9337 opened: boot,many: reseal only when meaningful and necessary <Run nested> <UC20> <Created by pedronis> <https://github.com/snapcore/snapd/pull/9337>
[22:14] <ijohnson> ack I'll take a look at that one then
[22:21] <mup> PR snapd#9315 closed: tests: fix snap-routime-portal-info test <Simple 😃> <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/9315>