[06:00] <mborzecki> morning
[06:16] <mardy> 'morning!
[06:21] <mborzecki> mardy: your services restart PR is good to go right?
[06:31] <mardy> mborzecki: yes, but if you are fine with the current version, then I'd like to squash a few commits together, because if we merge it as it is it just brings a lot of noise in the git history
[07:11] <pstolowski> morning
[07:41] <mup> PR snapd#10273 closed: c/snap: more precise message for ErrorKindSystemRestart op != reboot <Created by pedronis> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10273>
[08:15] <pedronis> mardy: hi, I have a question:  https://github.com/snapcore/snapd/pull/10251#discussion_r635011661
[08:15] <mup> PR #10251: interfaces/builtin: introduce raw-input interface <Needs Samuele review> <Created by mardy> <https://github.com/snapcore/snapd/pull/10251>
[08:22] <mborzecki> mvo: can you land #10220?
[08:22] <mup> PR #10220: seed/seedwriter: fail early when system seed directory exists <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10220>
[08:22] <mup> Bug #10220: woody upgrade troubles <cracklib2 (Ubuntu):Fix Released> <cracklib2 (Debian):Fix Released> <https://launchpad.net/bugs/10220>
[08:32] <zyga-mbp> hey guys
[08:32] <zyga-mbp> just a small information drop: I'm working with Linaro to add native support for running spread jobs in LAVA
[08:33] <zyga-mbp> we had a kick off recently and we will be doing the design and implementation in the open if anyone wants to participate
[08:33] <zyga-mbp> the goal is to allow using deployed LAVA as a target for spread
[08:34] <zyga-mbp> where spread is able to generate lava jobs natively
[08:34] <zyga-mbp> and they run with all the features and tracking in lava
[08:42] <zyga-mbp> this way spread can be used during testing locally on devices or with virtual machines and containers
[08:42] <zyga-mbp> but the same test definitions can be transformed to lava jobs and executed in the 140+ devices that lava can automate today
[08:48] <pedronis> pstolowski: doesn't https://github.com/snapcore/snapd/pull/10266  changes also "snap restart snap.svc1 snap.svc2" ?
[08:48] <mup> PR #10266: wrappers/services.go: do not restart disabled or inactive services <Needs Samuele review> <Created by mardy> <https://github.com/snapcore/snapd/pull/10266>
[08:50] <pstolowski> pedronis: yes it does
[08:50] <pedronis> pstolowski: that was not the intention though
[08:50] <pstolowski> i think i asked for a spread test for it
[08:50] <pstolowski> oh, hmm
[08:51] <pedronis> this is about strictly what happens if you restart all the services
[08:51] <pedronis> as far I remember we might not even know the difference right now
[08:51] <pedronis> but we have to
[08:52] <pedronis> https://forum.snapcraft.io/t/command-line-interface-to-manipulate-services/262/47 talks about all the variants for SNAP not single services
[08:53] <pedronis> or services explicitly listed
[08:55] <pstolowski> pedronis: indeed, we expand to appInfos very early in postApps, oh well
[08:57] <pedronis> pstolowski: that's ok but we need a flag on the task and in the options I suppose
[08:57] <pstolowski> pedronis: yes
[08:58] <pedronis> pstolowski: mardy: have a chat how to proceed on this
[08:58] <pstolowski> sorry i didn't realize we want to special-case the bahavior. will talk to mardy
[09:00] <pedronis> pstolowski: mardy: I made an explicit comment in the bug: https://bugs.launchpad.net/snapd/+bug/1803212
[09:00] <mup> Bug #1803212: snap restart starts disabled services <snapd:Triaged by mardy> <https://launchpad.net/bugs/1803212>
[09:01] <pedronis> pstolowski: also do the other things stop/start work already as described?
[09:12] <pedronis> mvo: I commented here:  https://github.com/snapcore/snapd/pull/10256#issuecomment-843909902 let me know what you think
[09:12] <mup> PR #10256: snapstate: do not allow installing the same snap revision via InstallPath <Needs Samuele review> <Created by mvo5> <https://github.com/snapcore/snapd/pull/10256>
[09:18] <mborzecki> pedronis: hmm looking a bit at snap bootrap & resealing run keys with multiple models, we'll also need to update ubuntu-boot/device/model
[09:19] <pedronis> mborzecki: yes
[09:20] <pedronis> mborzecki: we probably should update it before the state with some undo error path
[09:20] <mborzecki> the last steps feel a bit fragile though, it's going to be: 1. reseal with both models; 2. replace device/model; 3. set device in state, whereas the points 2-3 are problematic wrt. reboot i think
[09:21] <mborzecki> and we'll need a backup copy of the old device/model in case setdevice fails (it's unlike but still)
[09:21] <pedronis> mborzecki: we need to write some tests
[09:22] <pedronis> about what happens if we reboot in the middle there
[09:22] <mborzecki> pedronis: yeah, after reboot we'd reseal with model from the state, which before set-device would be the old one, but then device/model would be out of sync
[09:23] <pedronis> mborzecki: maybe we need to do this differently
[09:23] <pedronis> I mean mark something in modeenv such that if we come back with the new model then we just finish this
[09:23] <pedronis> instead of undoing
[09:24] <mborzecki> mhm, that's a good idea
[09:25] <pedronis> mborzecki: ah, notice that modeenv needs the model set as well?
[09:25] <pedronis> so maybe we kind of that already
[09:26] <pedronis> we just need to be careful about the order and what we do if we come back middle of this
[09:26] <mborzecki> hm also reversing the order of 2 and 3 could work i think, since we'd reseal for both models earlier
[09:27] <pedronis> anyway sounds we need some managers tests about these reboot scenarios
[09:28] <mborzecki> yup
[09:28] <mborzecki> and i need to run some scenarios on paper :)
[09:30] <mardy> pedronis: (about the raw-input interface) I replied to the question: those accesses do not seem to be needed, the app works fine without them
[09:31] <pstolowski> pedronis: no, start needs changing too, stop might be ok, i think this can/should be a separate PR
[09:31] <pedronis> pstolowski: yes, I think they should be separate PRs
[09:31] <pedronis> this one will already need to grow a bit
[09:31] <pedronis> mardy: thanks for the answer
[09:31] <pedronis> and the PR
[09:35] <pstolowski> mardy: let me know when you have a moment to chat about serviceds
[09:41] <mup> PR snapd#10182 closed: o/snapstate: autorefresh phase1 for refresh-control <Needs Samuele review> <Refresh control> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/10182>
[09:42] <mvo> pstolowski: congrats, nice to see this landing!
[09:44] <pstolowski> yeah! just rebased #10261
[09:44] <mup> Bug #10261: Fails to start if ipv6 disabled <postfix (Ubuntu):Fix Released by lamont> <https://launchpad.net/bugs/10261>
[09:44] <mup> PR #10261: o/snapstate: refresh control - autorefresh phase2 <Needs Samuele review> <Refresh control> <Created by stolowski> <https://github.com/snapcore/snapd/pull/10261>
[09:44] <pstolowski> pedronis ^
[09:46] <pedronis> pstolowski: thanks, queued, I might not get to it until tomorrow though
[09:46] <pstolowski> np
[09:48] <mardy> pstolowski: I do now
[10:21] <mup> PR snapd#10220 closed: seed/seedwriter: fail early when system seed directory exists <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10220>
[10:40] <pstolowski> pedronis: does it sound ok to move softCheckNothingRunningForRefresh to gate-auto-refresh hook handler's Before()? Another place would be autoRefreshPhase1() when tasks get created. the new conditional-autorefresh task is too late (after the hooks)
[10:46] <pstolowski> ah it's sligthly annoying because the hook is optional
[10:46] <pstolowski> so it may or may not run
[10:51] <pedronis> pstolowski: I think Before seems a good place, yes the hook is optional but otoh if the hook is not run the answer doesn't matter as much
[10:51] <pedronis> but it means we can't move it, anyway we also need to think about the corresponding lock
[10:51] <pstolowski> pedronis: doesn't softCheckNothingRunningForRefresh have side effects though?
[10:54] <pedronis> pstolowski: yes, it changes SnapState
[10:54] <pedronis> I think
[10:55] <pstolowski> yes exactly
[10:55] <pedronis> but it's what we need
[10:55] <pedronis> or you mean related to the fact that we will not always run it at the same place?
[10:56] <pstolowski> yes. and if we should run it only once (so if hook runs it, then doInstall skips it)
[10:57] <pedronis> as I said the bigger question is what to do with the lock
[10:57] <pedronis> because I don't think we can use softCheckNothingRunningForRefresh as is
[10:57] <pedronis> ultimately
[11:05] <pedronis> pstolowski: we can chat before or after the standup
[11:05] <pstolowski> pedronis: great, maybe after?
[11:05] <pedronis> ok
[11:09] <pedronis> mborzecki: what's the status of the blockers here: https://github.com/snapcore/snapd/pull/9647 ?
[11:09] <mup> PR #9647: vendor: upgrade to godbus v5.0.3 <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/9647>
[11:10]  * pstolowski lunch&errand
[11:10] <mborzecki> pedronis: hm fedora 32 should eol in a week or so, then we can drop it and we should be able to land the pr
[11:12] <mborzecki> pedronis: yeah, the eol should happen on 25.05, the packages in f33 and f34 are good afaict
[11:16] <pedronis> mborzecki: can you comment there?  it probably should be marked blocked until then
[11:17] <pedronis> thx
[11:17] <mborzecki> pedronis: sure
[11:27] <mup> PR snapd#10275 opened: overlord/devicestate: use system mode access helper when checking tried systems <Run nested> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10275>
[12:30] <pstolowski> re
[12:32] <mup> PR snapd#10276 opened: tests: remove tests.cleanup prepare from nested test <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10276>
[12:37] <mardy> pstolowski: hi! I'm getting more and more confused :-) It looks like the AppInto list which is constructed in daemon/api_apps.go:postApps() is thrown away and overlord/servicestate/service_control.go:doServiceControl() is rebuilding it from scratch
[12:38] <mardy> assuming that my understanding is right, what is the reason for not passing the same array around?
[12:42] <mup> PR snapcraft#3525 opened: dotnet plugin: use https for release metadata url <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/3525>
[12:44] <pstolowski> mardy: doSeviceControl is a task handler. when we create the task we resolve all names to appinfos to ensure they make sense. then we store resolved names on the task. The handler then re-creates appinfos as this is the struct used universally by wrappers
[12:46] <pstolowski> mardy: it's a bit historical because the helper that resolves names into appinfos existed already
[12:51] <mardy> pstolowski: so, my idea was to add a boolean field `ExplicitlyRequested` to the AppInfo struct. I can easily fill that in the postApps() method, but it gets lost somewhere before getting to the task handler. Looks like this approach is not feasible then…
[12:53] <pstolowski> mardy: no, AppInfo shouldn't be expanded with transient flags
[12:54] <rZr> sorry for noise but can i suggest to look about what's happening with freenode now : https://mastodon.social/@rzr/106261954890979884
[12:54] <pstolowski> mardy: AppInfo, SnapInfo etc correspond to static information about the snap, coming from its yaml
[13:07] <mup> PR snapd#10188 closed: tests: moving to tests directories snaps built locally - part 1 <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10188>
[13:13] <pstolowski> mardy: what you can do in go though is to embed AppInfo in another struct that just has extra attributes. but maybe just drop the use of appInfosFor in this particular case where we only need services?
[13:15] <pstolowski> mardy: maybe a new helper could simply return two sets of results
[13:17] <pstolowski> (individually resolved services, snap names)
[13:47] <mup> PR snapd#10277 opened: overlord/devicesate: observe snap writes when creating recovery systems <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/10277>
[14:10] <mup> PR core20#96 closed: Resurrect bash-completion <Created by xnox> <Merged by xnox> <https://github.com/snapcore/core20/pull/96>
[15:05] <mup> PR core20#103 closed: hooks: add libglib2.0-bin for the "gdbus" utility <question> <Created by jhenstridge> <Closed by jhenstridge> <https://github.com/snapcore/core20/pull/103>
[15:22] <pstolowski> mardy: regarding "...but it gets lost somewhere before getting to the task handler.." this is because you're updating in-memory structures only. Later when the task runs it can only see whatever was explicitly persisted in the state (or in the task's own data by task.Set(...) and then retrieved by task.Get(..)). In this particular case task handler just gets AppInfos from original snap yaml
[15:28] <pstolowski> mardy: if you look around, we use task's Set & Get a lot to persist and pass transient data
[15:53] <mup> PR snapd#10278 opened: o/hookstate/ctlcmd: allow system-mode for non-root <Created by pedronis> <https://github.com/snapcore/snapd/pull/10278>
[15:54] <zyga_> LAVA + spread mariage is starting: https://forum.ostc-eu.org/t/lava-and-spread-in-all-scenarios-os-ci-loop/50/2
[15:54] <zyga_> cachio, ^
[16:02] <mvo> zyga: \o/ awesome
[16:23] <mup> PR snapd#10279 opened: packaging/ubuntu-16.04/changelog: add placeholder for 2.50.1 <Skip spread> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10279>
[16:48] <zyga> mvo: yeah, it's interesting because this will also unlock one more small thing
[16:48] <zyga> mvo: we could do native shellcheck, with all the awareness of what goes where in spread and what variables are present
[18:08] <mup> PR snapd#10279 closed: packaging/ubuntu-16.04/changelog: add placeholder for 2.50.1 <Skip spread> <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10279>
[18:38] <mup> PR snapd#10280 opened: packaging: merge 2.50.1 changelog back <Skip spread> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10280>
=== oerheks1 is now known as oerheks
[21:14] <om26er> Are we hopping on the "leave freenode" bandwagon just yet ? Hope we don't. Lots of freenode/IRC memories, that I'd love to keep.
[21:20] <oerheks> For the present, freenode is still the official IRC namespace for Ubuntu until you hear otherwise from official sources.
[21:20] <oerheks> just a storm in a glass of water, AFAIK
[21:59] <mup> PR snapd#10281 opened: release-tools/changelog.py: implement script to update all the changelog files <Skip spread> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/10281>