[05:43] morning [06:31] ehh some random failures on arch [06:31] mvo: morning [06:32] mborzecki: good morning [06:52] mvo: can you use your superpowers to merge https://github.com/snapcore/snapd/pull/9246 ? [06:52] PR #9246: boot: handle canceled update [06:53] mborzecki: sure thing [06:53] mvo: thanks! [06:53] good morning [06:53] mborzecki: done [06:53] zyga-kaveri: good morning [06:53] mvo: yay, thanks! [06:53] zyga-kaveri: hey [06:56] PR snapd#9246 closed: boot: handle canceled update [07:03] morning [07:06] PR snapd#9258 closed: devicestate: add tests around logging in RequestSystemAction [07:12] pedronis: i'll rebase #9264 on top of master now that Canceled has landed [07:12] PR #9264: [RFC] many: introduce ContentChange for tracking gadget content in observers [07:12] mborzecki: thx [07:14] mborzecki: I addressed your comments in 9210, I think this is ready for another look [07:16] PR snapd#9268 opened: daemon: add API for checking and installing available theme snaps [07:21] PR snapd#9257 closed: bootloader: retrieve boot chains from bootloader [07:21] PR snapd#9269 opened: devicestate: rename "mockLogger" to "logbuf" [07:31] PR snapd#9267 closed: many: fix partion vs partition typo [08:13] hm i think i found a subtle bug in gadget updates handling of new files on rollback when there's an unexpected reboot in between [08:16] * zyga-kaveri will really start at 11:00, but is wondering if this is a good long-term model [08:16] zyga-kaveri: probably not [08:17] mborzecki: PT interferes but fortunately I do at-home PT today [08:45] mborzecki: I re-commented in 9264 [08:46] mborzecki: also maybe, silly idea, could backupOrCheckpointFile return the ContentChange or nil directly? or doesn't it have the right info? [08:48] pedronis: hm good point, it should have all the data, and != nil means somethign will be written [08:48] yes [09:00] pedronis: pushed now [09:01] thx, I have a meeting now [09:05] heh, lucy is just now sleeping, as my wife is back [09:05] anyway [09:05] * zyga focuses on coding [09:07] hmm, some issue with spread [09:10] _hmm_ [09:13] pstolowski: offtopic, I think we have a bug in services [09:13] https://pastebin.ubuntu.com/p/mWNXbsyt2f/ [09:14] pstolowski: look at those days [09:14] dates [09:14] Undoing 2020-05-06 2020-05-06 Stop snap "lxd" services [09:15] looking [09:15] perhaps lxd's special arrangement with services makes our code ineffective [09:15] I rebooted the machine as it was stuck using lots of cpu as lxd was in a weird state [09:15] but I think this change should have progressed somehow [09:15] either by failing [09:15] or by continuing with lxd services not stopped [09:15] note that this is the undo path [09:15] so maybe it's a less prominent bug [09:16] I rebooted that device [09:17] ha [09:17] it's rebooted and in the same state?! [09:18] zyga: i don't understand what's going on, but 2020-05-13 and 2020-05-06 looks werid [09:18] https://paste.ubuntu.com/p/VVbp4k44sF/ [09:18] yeah [09:18] it's like this for a long time [09:18] look at this now [09:19] anything else I can provide? [09:20] I can refresh snapd [09:20] it's not the latest by far [09:20] but I wonder why didn't it refresh by itself [09:21] is it the lxd change in flight? [09:21] I didn't set any refresh schedules or anything like that [09:23] pstolowski: lxd is also in a funny state: https://paste.ubuntu.com/p/w69fpSx3df/ [09:23] this keeps showing up over and over [09:24] zyga@pi3-2:~$ sudo snap stop lxd [09:24] error: snap "lxd" has "auto-refresh" change in progress [09:24] hmmm [09:29] I stopped both the lxd.activate and lxd.dameon services [09:30] and the machine is no longer spinning like crazy [09:30] and now all the tasks have finished [09:30] restart-condition: on-failure [09:30] lxd will restart forever when it is broken [09:31] pstolowski: what happens when: [09:31] pstolowski: a snap has a service which has restart-condition: on-failure [09:32] pstolowski: snap has more than one service, one that talks to a socket (like activate) and causing socket-activation, and another one that is broken [09:32] pstolowski: will we ever manage to stop that? [09:35] zyga: would be good to have a simple reproducer as a spread test (without lxd), i don't know from top of my head [09:35] now store assertions service times out [09:35] - Fetch and check assertions for snap "core18" (1889) (Get https://api.snapcraft.io/api/v1/snaps/assertions/snap-revision/i_FldUJXgbUZueGCJo8M5l_yJeF-i1p9ienjEtN3SRfRmi8Wpckx_ZI8qm-r5Mdg?max-format=0: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)) [09:35] pstolowski: yeah, I agree [09:36] eh, it sucks we throw away a snap download when assertions fail [09:36] I'm re downloading core and snapd again [09:36] isn't there a cache we have? [09:36] zyga: you're probably onto something but this is a bit complicated to reason about without a test [09:36] PR snapd#9270 opened: wrappers, systemd: allow empty root dir and conditionally do not pass --root to systemctl [09:37] pstolowski: I can review that [09:37] zyga: thanks [09:37] I think we could try to fetch assertions before the download [09:37] zyga: i guess failed assertion means we cannot trust cache? [09:37] that's probably not a high cost in code [09:37] pstolowski: not really, we can keep an incomplete cache of downloads [09:37] zyga: ah, ok, it's downloading assertions [09:38] zyga: so yeah, probably [09:38] if I break network at 90% of a huge snap [09:38] it's really silly to throw that away [09:38] even before we have any assertions [09:38] zyga: i marked that PR RFC/draft [09:38] and it failed again! [09:39] pedronis: is the store assertion service broken? [09:39] - Fetch and check assertions for snap "snapd" (8792) (Get https://api.snapcraft.io/api/v1/snaps/assertions/snap-revision/eZZ56DeorGzQUHotTBqlJ_T4LLyZKnAA0IkKn9YISRHfkTjiCENUBpcH5flV-kCA?max-format=0: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)) [09:39] zyga: he is in a meeting, maybe someone from the store knows? [09:39] I see [09:43] for context, this fails when doing this: [09:43] - Fetch and check assertions for snap "snapd" (8792) (Get https://api.snapcraft.io/api/v1/snaps/assertions/snap-revision/eZZ56DeorGzQUHotTBqlJ_T4LLyZKnAA0IkKn9YISRHfkTjiCENUBpcH5flV-kCA?max-format=0: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)) [09:43] er [09:43] Consider re-refresh of "core18", "pi", "lxd", "htop", "snapd", "pi-kernel" | [09:43] it's re-refreshing a handful of snaps [09:44] and probably fails because of that [09:44] yep, failed again [09:44] and re-refresh fails the whole change [09:48] pstolowski: https://github.com/snapcore/snapd/pull/9270#pullrequestreview-481676543 [09:48] PR #9270: [RFC] wrappers, systemd: allow empty root dir and conditionally do not pass --root to systemctl [09:52] zyga: thank you, i think's a new ctor is a good idea, although i think NewInChroot is confusing [09:53] pstolowski: New vs NewChroot or something similar [09:56] spread workers will be back shortly [09:56] zyga: i would just avoid "Chroot" in the name, but yes [10:02] pstolowski: New and NewUnderRoot ? [10:03] pedronis: yes, works for me [10:04] heh, cancelling github action jobs is kind of flaky [10:04] pretty sure i canceled the right one, but a different run failed? [10:04] all in the same PR [10:08] mborzecki: spread refuses to be cancelled [10:08] mborzecki: it works if the process cooperates [10:08] mborzecki: I had to restart workers so that's expected [10:08] zyga: wouldn't the worker send SIGTERM at some point? [10:09] mborzecki: spread ignores that [10:09] mborzecki: it's a known bug [10:09] uh meant SIGKILL [10:09] mborzecki: ah, no it doesn't do that [10:09] compilers usually behave [10:09] well, you can run arbitrary commands there [10:10] do actual deployments and so on [10:10] I know but usually tools take SIGINT just fine [10:12] ok, we should have a bit more stability now, there's some extra ram for excessive spiky loads [10:16] well, you can run arbitrary commands there [10:16] duh, wrong window [10:16] ls ;) [10:25] Hi Everyone, is there any date in mind for this milestone release ? https://launchpad.net/snapd/+milestone/2.46 [10:25] the fixes has been committed for a while [10:25] mvo: ^^^ [10:31] * zyga changes hosts [10:40] zyga-kaveri: can you report a bug for that lxd services issue and collect all the info there? [10:42] PR snapd#9271 opened: boot: keep track of the original asset when observing updates [10:48] pstolowski: shouldn't most places keep New, and just very few use UnderRoot? are you not doing that because of tests? [10:51] pedronis: yes, i just wanted to limit changes to the minimum I know was problematic for services PR. i can do it all if you prefer that way? [10:52] pstolowski: I think we need to have a chat so I understand what's the final state we want [10:52] pstolowski: yes, on it [10:54] pstolowski: can we have a chat after standup? or ealier if you want? [10:54] pedronis: sure, anytime works for me [10:59] pedronis: i'll have a lunch break now, so maybe before or after standup, let me know [10:59] ok, I have a meeting noew [10:59] ok [11:06] zyga, hi [11:06] zyga, I need last review for #9098 [11:06] PR #9098: tests: new organization for nested tests [11:07] zyga, could you please take a look today? [11:10] cachio: sure [11:10] zyga-kaveri, thanks [11:51] pstolowski: I scheduled something [11:52] pedronis: thanks [12:02] PR snapd#9262 closed: configcore: rework how console-conf is disabled <⛔ Blocked> [12:08] cachio: https://github.com/snapcore/snapd/pull/9098#pullrequestreview-481741540 [12:08] PR #9098: tests: new organization for nested tests [12:10] zyga-kaveri, o makes good compression, I'll put there the values , give me 10 minutes [12:12] cachio: cool, thanks [12:12] that is not a blocker [12:12] the few issues are real blockers [12:12] once those are done we can merge and iterate [12:29] zyga-kaveri, I updated the function names [12:30] zyga-kaveri, the changes you told me about core revert test, I don't see the same thing than you [12:31] cachio: about !? [12:31] cachio: function names were not a blocker [12:31] this test has been already with the others, could be that one cached for you? [12:31] ? [12:31] cachio: cached? [12:31] are we talking about the use of negation? [12:33] zyga-kaveri, this one https://github.com/snapcore/snapd/pull/9098#discussion_r482923838 [12:33] PR #9098: tests: new organization for nested tests [12:33] for example [12:34] I didn't read your comments there [12:34] but do you understand the fundamental problem with sh, functions, set -e and if / while / !? [12:34] you cannot use all three at the same time [12:35] set -e; hello() { false; echo "blah" }; if ! hello; then echo "unreachable"; fi [12:35] false does not break execution of hello [12:36] I am not talking about the negation [12:36] cachio: replace `if` with `while/until` and `!` [12:36] ok, let me read your comments [12:37] you see variables like CORE_REFRESH_CHANNEL but I dont [12:38] that's not what I'm referring to [12:38] I know [12:38] I was only referring to the usage of functions inside control statements [12:38] so? [12:39] 1 sec, let me check github code [12:40] are we in agreement what is the problem with those functions? [12:40] I'm confused if we are talking about the same thing [12:40] I think github is doing something wrong [12:41] what? [12:42] it shows in the main page of the pr some comments but then if I go to the file I see the real one [12:42] it is like an overlap [12:43] I think it was caching here [12:43] now I see well [12:44] about the negations, we allways used the ! and not for execute_remote, in this PR I just updated the name [12:45] zyga-kaveri, [12:45] so? [12:45] it's still wrong :) [12:45] not lalalalal is true [12:45] not function_not_command is always true [12:45] that part was not testing anything real [12:46] ! function is very very fragile [12:46] unless the function is coded for it [12:46] it will misbehave [12:46] while function and if function are equally fragile [12:50] cachio: don't get me wrong, those do not have to be fixed in this pr [12:50] cachio: they are wrong though [12:51] zyga-kaveri, ok, I'll open a new pr with that [12:51] zyga-kaveri, we should organize all the changes needed for nested [12:51] yeah, let's fix the small things here and work towards fixing the rest [12:51] I am creating a todo list [12:51] I'm not sure how to fix it yet [13:02] PR snapd#9272 opened: configcore: "service.console-conf.disable" is gadget defaults only [13:59] off to pick up the kids [14:02] #9264 needs 2nd reviews [14:02] PR #9264: many: introduce ContentChange for tracking gadget content in observers [14:02] PR snapd#9273 opened: [RFC] boot: mark successful with boot assets [14:03] cmatsuoka: can you take a look at 9264 maybe? [14:03] mborzecki: sure [14:03] cmatsuoka: thanks! [14:04] * mborzecki really leaves this time to pick up the kids, bbl [14:11] mborzecki: done [14:29] cachio: have you seen this journal-state failure before ? https://pastebin.ubuntu.com/p/zqGBMpPxgF/ [14:30] * zyga-kaveri focuses on coding [14:33] PR snapd#9264 closed: many: introduce ContentChange for tracking gadget content in observers [14:33] mvo: pedronis: we can just land #9269 now and I can merge master into my PR [14:33] PR #9269: devicestate: rename "mockLogger" to "logbuf" [14:33] I need to merge master into my PR anyways [14:33] ijohnson: ok, if that works for you [14:33] yeah that's fine [14:34] I +1d it, not sure if we want a 2nd +1 on the PR, it's quite simple [14:34] ijohnson, cheking [14:35] ijohnson: I merged it [14:35] ijohnson, mmm, no [14:36] but at some point we added that retry to avoid an error [14:36] I'll try to reproduce it [14:38] PR snapd#9269 closed: devicestate: rename "mockLogger" to "logbuf" [14:39] thanks pedronis [14:39] cachio: yeah I just saw it once, I don't think it's happening all the time [14:41] ijohnson: mvo: I commented here: https://github.com/snapcore/snapd/pull/9253/files#r483024347 [14:41] PR #9253: sysconfig/cloudinit.go: add AllowCloudInit and use GadgetDir for cloud.conf [14:42] pedronis: I responded, I think returning right now is the right thing [14:42] pedronis: but eventually returning right there is not what we want [14:44] ijohnson: we are setting CloudInitSrcDir only for dangerous atm, right? [14:44] pedronis: yes [14:44] so we don't strictly need to return [14:45] we might want to add a comment about that and vs the TODO though [14:45] pedronis: do you think the existing TODO I have is sufficient or should I add another TODO ? [14:46] ah, I was confused you have already a return nil [14:47] that return nil needs its own TODO, about the fact that combining cloud.conf and renumbering / filtered seed data should be ok [14:47] can I interesst someone in a review for 9210 maybe :) ? would unblock the "snap reboot" command [14:48] mvo: I can take a look in my PM [14:48] \o/ [14:48] queued it up [14:54] ijohnson: #9273 is also something you should look (I don't know if I'll get to it myself today) [14:54] PR #9273: [RFC] boot: mark successful with boot assets [15:03] pedronis: ack I will add it to the queue, also #9253 is updated again [15:03] PR #9253: sysconfig/cloudinit.go: add AllowCloudInit and use GadgetDir for cloud.conf [15:05] ijohnson: +1 [15:06] thanks [15:10] mvo: I merged master into #8920 [15:10] PR #8920: interfaces: update cups-control and add cups for providing snaps

[15:11] pstolowski: #9245 doesn't need to target 2.46 anymore, right? [15:11] PR #9245: o/snapstate, features: add feature flags for disk space awareness [15:12] pedronis: yes, cleared, thank you [15:12] thx [15:15] PR snapcraft#3269 closed: spread tests: remove references of core16 [15:20] pstolowski: maybe I'm confused but now TestRemoveDiskSpaceForSnapshotNotCheckedWhenSnapshotsDisabled and TestRemoveDiskSpaceCheckDisabled are the same test basically? so we should keep only one? [15:20] let me check [15:21] or one is about the default? [15:23] yes, one of the two is about the default vs explicitly disabling, maybe they should share some code, and need better names [15:25] pstolowski: sorry, maybe I'm distracting you, I thought that one would be an easy win [15:30] pedronis: looking at cups in a sec [15:30] pedronis: np, i was just checking, yes, one is about not doing a snapshot becuase of ErrNothingToDo, the other is about feature flag but it has a mistake and is not really testing the flag becuase it also hits ErrNothingToDo [15:31] pedronis: thanks for spotting, i'll fix that [15:48] * cachio lunch [16:01] mvo: could you use your superpowers on #9266 ? the only failed test is on 16.04, and it's a known failure that is not related to the PR [16:01] PR #9266: tests/lib/nested.sh: reset the TPM when we create the uc20 vm [16:03] ijohnson: sure [16:03] thanks [16:03] ijohnson: done [16:03] \o/ [16:05] pedronis: done with https://github.com/snapcore/snapd/pull/9274 ; 3 tests share same code now [16:06] PR #9274: tests: simplify and fix tests for disk space checks on snap remove [16:08] PR snapd#9266 closed: tests/lib/nested.sh: reset the TPM when we create the uc20 vm [16:08] PR snapd#9274 opened: tests: simplify and fix tests for disk space checks on snap remove