[01:41] PR snapd#5826 opened: tests: refactor for nested suite and tests fixed [05:01] morning [06:19] Yawn, hey hey [06:22] zyga: hey hey [06:25] hey zyga and mborzecki [06:28] any interesting reviews to look at? === pstolowski|afk is now known as pstolowski [07:02] heyas [07:02] pstolowski: good morning [07:02] Issue core18#4 closed: Crash qt gui aplication (build with core 18 on ubuntu 18.04(LXD)) [07:04] there's going to be an interesting change [07:04] to seccomp [07:04] hopefully it won't be painful [07:08] zyga: what kind of change? [07:08] something in upstream? [07:09] a "one liner" [07:09] we need to support statx [07:09] I'm on it [07:22] ack [07:59] mborzecki: I reviewed #5778 a bit closer, is not super clear that all removes and undo cases have been replaced, basically I tried looked at all the call paths to the removed os.Remove and seen if they grew a matching call to some new remove, it might also mean we miss some tests [07:59] PR #5778: snap-update-ns: remove deadcode [08:00] mborzecki: sorry, I meant #5758 [08:00] PR #5758: overlord/snapstate, snap: handle shared snap directories when installing/remove snaps with instance key [08:01] pedronis: thanks [08:02] pedronis: the removes in undosetupsnap had to go one level higher to the caller, because you don't know whether it's ok to remove the directory in the cleanup code path [08:02] mborzecki: I know but there are other cases, removeDirs that you changed was called from a lot of places for example [08:02] not super clear they are all covered now [08:03] including undo cases [08:26] pedronis: i see, so undoCopySnapData and doCopySnapData need a further fix, i'm not sure about cleanupSnapData, is cleanup executed only in success path? [08:28] PR snapd#5827 opened: ifacestate/autoconnect: do not self-conflict on setup-profiles if core-phase-2 [08:30] pedronis: ^ i didn't manage to recreate the case with spread (tried coming from snapd 2.0.2, 2.33.1, 2.23.9). i'm pretty sure the conflict check is the culrpit as discussed, but there might be a factor in reproducing that I missed [08:48] PR snapd#5765 closed: store: move download tests into downloadSuite [08:53] pedronis, any idea why the assertion importer talks aout a model that doesnt exist in https://forum.snapcraft.io/t/porting-ubuntu-core-to-imx6-am335x-issue/7089/20 ? [08:54] (the model name of the image seems to be "advantech" not "rsb4220") [08:55] *about [08:57] zyga, mborzecki : responded to your comments to https://github.com/snapcore/snapd/pull/5807 and fixed gofmt oddities [08:57] PR #5807: interfaces: extra argument for static attrs in NewConnectedPlug/NewConnectedSlot [08:59] mvo: would you mind taking a look at https://github.com/snapcore/snapd/pull/5762 ? [09:00] PR #5762: ifacestate: don't initialize udev monitor until we have a system snap [09:00] pstolowski: sure, will do in a bit [09:01] mvo: also, your https://github.com/snapcore/snapd/pull/5813 could land i think? [09:01] PR #5813: image: warn on missing default-providers [09:08] pstolowski: yeah, I think its good - was wonder if samuele wanted to peek at it but its probably fine as is [09:17] PR snapd#5818 closed: cmd/snap: handle "snap interfaces core" better [09:17] thanks! [09:28] thursday before the sprint, and me without a haircut yet [09:28] morning all [09:33] Chipaca: hey! yeah, I'm scrambling to get osme things sorted as well [09:34] mvo: I don't know what osme is, but it sounds serious [09:34] mvo: today's my dad's birthday, as well. And I've got an interview with people from one of the boys' potential schools. [09:34] and, I've got tests to write [09:34] :-) [09:34] Chipaca: sounds like a full day! [09:35] Chipaca: *cough* osme -> some :( [09:35] and a washing day! because the sun is out [09:35] Chipaca: clear sign that I need to get better at typing [09:35] mvo: left a couple of non-blocking comments on #5813 for you to consider [09:35] PR #5813: image: warn on missing default-providers [09:35] pedronis: \o/ thank you [09:35] oh right, haircut /o\, thanks for reminder! [09:36] getting repeated failures today on google:ubuntu-18.04-64:tests/main/lxd [09:36] pstolowski: gave a +1 to the fix PR but it's red atm [09:36] pedronis: yes everything is terrible atm [09:36] pedronis: and thanks btw [09:37] actuall, this thing is already nearly 900 lines. I'll split it, and then write the tests [09:37] zyga, mvo: I think we should do a bit better for that fix [09:37] y [09:38] But good mornings, first :) [09:38] niemeyer: what fix in particular? and good morning :) [09:38] mvo: The one about interfaces core [09:38] ok [09:39] niemeyer: hello [09:39] what kind of improvement are you looking for? [09:39] zyga: Heya [09:40] zyga: It would be nice if it is possible, anyway.. let's see [09:40] good morning niemeyer! [09:40] zyga: Do we ever send the snap name to the server end? [09:40] no [09:40] mvo: thanks for the review of wait-for-core pr [09:40] zyga: Hmm :/ [09:41] niemeyer: otherwise I would have implemented a server side fix [09:41] zyga: and I guess we don't actually print the snap name either, when it's system/core [09:41] that's correct [09:41] zyga: Ok, sorry, the fix is great then, thanks [09:42] zyga: The only other alternative I can think of is reverting the change, making it "core" again, and adding an explicit "nicknames" field [09:42] I had a branch that does the revert, if we add the nicknames field we would indeed be 100% backwards compatible [09:43] we could do that, that's an interesting idea [09:43] let me check one thing really fast [09:43] niemeyer: so in all cases we _can_ add a field [09:44] so yeah, we could do that [09:44] zyga: The thing that bothers me a bit about that approach is that we have a significant number of connections to core [09:44] obviously [09:44] zyga: So this will become super repetitive [09:44] indeed :/ [09:44] but this is the best we can do without breaking the compatibility [09:44] zyga: Perhaps we could add some metadata at the outer level [09:44] zyga: So it stops being repetitive [09:44] zyga: "nicknames": {"system": "core"} [09:44] phone [09:45] zyga: Not sure.. "phone" doesn't feel like a great nickname in this case [09:46] i m on z call [09:46] zyga: z ? [09:46] I'm on a call [09:46] heh :) [09:46] ok sorry [09:48] mvo: He's not getting jokes today :P [09:49] niemeyer: heh [09:49] * mvo hands zyga some more coffee [09:49] "I'm on z call" feels so german somehow [09:50] re [09:50] ok [09:50] so yeah, we could do that [09:51] give me an hour to wrap this up [09:51] I just got a reply to my flight re-request saying "Sorry for not responding earlier, we work 10:00-18:00.." .. the request was on Monday... [09:51] I can reopen my branch, add the nickname there and see what we get [09:51] zyga: There where? [09:51] to the top-level of the response [09:52] because we have space for that [09:52] as in, the types are set up so that it is not hard to do [09:52] niemeyer: we may even use this to fix another issue [09:52] niemeyer: that the client doesn't know the type of the snap [09:52] zyga: Yeah, I think we have a place specific for metadata in the response already [09:52] to know that "core" or "snapd" are special [09:52] and can be abbreviated [09:53] and we could do it all in a way that is 100% backwards compatible [09:53] off to pick up the kids [09:53] zyga: Yeah [09:54] mborzecki: o/ [09:55] PR snapcraft#2267 closed: build providers: refresh packages on bring up [09:59] "building microservices with go" ebook available for free today at www.packtpub.com today (follow 'Free learning...' button at the bottom) [10:01] PR snapcraft#2266 closed: integration tests: disable tests using snaps in bionic container [10:29] mvo: hey, i've added real device samples to https://github.com/snapcore/snapd/pull/5759 per your comment [10:29] PR #5759: ifacestate: implementation of defaultDeviceKey function for hotplug [10:32] everything fails terribly on lxd spread test now (18.04). seems to be something fundamental - /var/snap/lxd/common/lxd/unix.socket is missing [10:33] pstolowski: lxd is socket activated now [10:33] perhaps the socket has moved? [10:33] zyga, it isnt anymore [10:33] no? [10:33] (while it gets switched to the snap by default) [10:33] can you tell me more? [10:34] https://lists.ubuntu.com/archives/ubuntu-devel/2018-September/040487.html [10:34] Note that at this time, the LXD snap isn't using socket activation yet. [10:34] We have code in place for that in the edge channel but want to do more [10:34] tests on it before we roll it out to all our users. This means that [10:34] starting on Monday, LXD will be starting up unconditionally on 18.10 [10:34] images. This is a temporary situation and will be corrected by release. [10:35] so better ping stephane, that might be a transition bug or some such [10:36] (since i dont know if it should actually apply to 18.04 too) [10:36] zyga: should we get rid of wait_for_lxd? [10:37] pstolowski: I'm not very familiar with the details [10:37] me neither [10:38] we basically keep polling for /var/snap/lxd/common/lxd/unix.socket before actual tests [10:38] pstolowski: cool [10:40] re [10:41] pstolowski: I'm deep in another task, can we discuss this in some time once I'm done [10:42] zyga: sure, in the meantime i'm experimenting with this test a little bit [11:06] pstolowski: nice, the updated tests give me a much better idea what it will look like in the real world [11:35] pedronis: niemeyer: I've got three options, and they're three somewhat unpalatable, and I thought maybe you guys might help me decide (or find a fourth) [11:35] if you recall, every successful command needs to check the warnings summary and report about it [11:35] the three options I see: 1. have everything from client return a ResultInfo, that contains the wanring summary, and have every command check that once it succeeds. This is the one I wrote, but it has two problems: it's a massive diff :-) and it's very easy to forget to check for warnings on success of a command [11:35] 2. have the client hang on to the relevant info, and pass the client into the checker. Smaller diff, just as easy to forget to do [11:35] 3. in cmd/snap have a single client instace instead of a Client() method, and have main itself check the warnings. Small diff, single point checking (as long as no command calls os.Exit directly on success, which should be easier to catch), but it's a global. Globals are bad, right? [11:35] 3b. like 3, but change all commands so the client is passed in. [11:35] pedronis, niemeyer: thoughts? [11:56] Chipaca: the part about the client keeping state is what we did for restarts fwiw, but there were fewer places that cared [11:57] Chipaca: so on that expect there is precedent, see Client.Maintenance [11:57] yep [11:58] pedronis: but here, every single command cares [11:58] (with two exceptions: the warnings-related commands themselves don't care) [11:59] Chipaca: how would you pass in a client btw ? [12:00] Execute is part of go-flags contract [12:00] ahoi, is there a possibility for minikube to get the source recipe? there is a very old version of minikube installed, i'd like to try to update it on the current version [12:00] pedronis: ah, drat, you're right [12:00] cmars: ^ see menace's q [12:01] Chipaca: same with the output [12:02] pedronis: the output? [12:02] pstolowski: A few comments in #5782... nitpicks mostly (sorry) [12:02] PR #5782: ifacestate: helpers for generating slot names for hotplug [12:02] I mean you can't change either input or output of that [12:02] pedronis: if this is 3b, that'd be every command doing checkWarnings(client) on success [12:03] ah no [12:03] pedronis: no it wasn't [12:03] pedronis: so i don't follow, no [12:03] pedronis: ah, maybe this: idea was you pass the client in, and then run does the checkWarnings(client) (client would presumably have warning info after the command was done with it) [12:03] Chipaca: As pedronis mentioned, it sounds analogous to Maintenance [12:04] pedronis, Chipaca: I think our Execute is already higher-level [12:04] nop [12:04] Or that's what my faulty mind reminds [12:04] or yep [12:05] niemeyer: no, the Execute from commands isn't run directly by us, but via flags.Parse() [12:06] niemeyer: it's similar, except Maintenance is only checked in two places (in wait, ie for all async commands, and in cmd_changes) [12:06] maybe it's time to think of a cheap way to sit in between go-flags and Execute, we do have some control in addCommand [12:06] so we could do something, the question is how cheap/clean that could be [12:07] it's high time we thought about moving on from flags, but that's a longer-term thing [12:07] i have to go, i will come back later... [12:08] re [12:08] sorry. [12:08] i have still time to the next track [12:08] pedronis: ah, i see what you mean, yes [12:08] did anyone answer my minikube source recipe question? :> [12:08] Deknos: you were menace a moment ago [12:09] Deknos: the snap author is in TX, it's 7am there, check back in a few hours === Deknos is now known as menace [12:09] ah, okay, thanks [12:09] menace: is the thing about it being old also true about the edge version? [12:09] Deknos is just an older handle which i used on other network.. because of that i want to use it here now, too === menace is now known as Deknos [12:10] how can i choose the edge channel/version? [12:10] i am new to snap =) [12:10] Deknos: ah. Look at 'snap info minikube' to see more [12:11] Chipaca: there should be a way to have ExecuteExt and some wrapping that would expose an Execture calling ExecuteExt [12:11] Deknos: and 'snap install --edge minikube' if you want that [12:11] Deknos: (note edge is typically unstable) [12:12] pedronis: you'd rather this to approach, then? [12:12] should be straightforward, but I'd do it in two steps [12:12] how can i check which version is in the snap channel without installing it? [12:12] Chipaca: I doubt is straightforward [12:12] Deknos: did you look at 'snap info' [12:12] but possible [12:12] ah, edge is 0.24.1, but current is 0.28 [12:12] Chipaca: you need to keep go-flags reflection happy [12:13] pedronis: I have a code axe. Everything is straightforward with enough violence. [12:13] then my question still stands :) [12:13] Chipaca: said that it still make sense the other path, make warnings state on client [12:13] Client [12:13] s/path/part/ [12:13] pedronis: on client the package, not client.Client? [12:13] mborzecki: suggested the same [12:14] Chipaca: ? [12:14] Chipaca: like Maintenance so client.Client [12:14] ah, right [12:14] not global in client [12:14] pedronis: and have the check be in each command? [12:14] please not that [12:14] Chipaca: no have check in Execute wrapping ExecuteExt [12:14] ah [12:14] mmkay [12:15] maybe [12:15] you need to write [12:15] if we had go 1.7 it'd be super easy [12:15] and ssee how it looks [12:15] i can build structs using reflect, there :-) [12:15] heh [12:15] metaprogramming \o/ [12:16] anyway. I'll go make lunch and think. [12:16] pedronis: thanks [12:16] niemeyer: and you [12:16] o/ [12:16] Chipaca: Btw, I think it's easy to wrap around the builder() [12:17] Chipaca: But you probably already figured as muc [12:17] h [12:24] niemeyer: thanks for the review! [12:27] pstolowski: np [12:36] * pstolowski quick lunch [13:08] jamesh: what is the status of backporting x-d-p in bionic? [13:10] (kpom [13:10] arg, sry [13:17] hi menace, the source to minikube is here: https://github.com/cmars/minikube-pkg [13:18] note the unsupported note there -- microk8s is a far better solution for my needs, uses less resources, so I've dropped it [13:18] menace: however, if you want to take it over, i'm happy to assign the snap ownership over to you [13:24] cmars, well first i want to be sure i am able to do such stuff [13:25] i'll look at the repository :) if i am able to, i can maintain it. [13:30] PR snapd#5828 opened: snap-confine: make /lib/modules optional [13:35] alexlarsson: while James is probably away now in 18.10 we have 1.0.2-1 [13:35] alexlarsson: not sure if that helps [13:35] Well, that is kinda expected [13:36] The issue is about the bionic flatpak ppa [13:36] I don't know the status of the back porting effort though [13:36] should i update x-d-p in it [13:36] I waited with that a bit, but people are starting to complain [13:37] I don't know [13:37] perhaps just email James [13:37] yeah, will do [13:37] cmars, do i understand that right that microk8s does not need a virtual machine, but it could be run within a virtualmachine? [13:38] Deknos: yes, that's correct [13:38] Deknos: i'm currently running a microk8s cluster in a multipass bionic VM, works great [13:39] .. nice. i have windows and macos developers.. and though vagrant/virtualbox works there, minikube is a pain in the ass.. having the local developer kubernetes cluster in a virtual machine with ubuntu would help massively... [13:40] nonetheless first i have to look into your minikube repo. [13:41] cmars, when you built the last minikube version (0.23) did you build it for ubuntu 16.04 or ubuntu 18.04? because i do not know to see the base dependency in snap for your edge package :) [13:42] Deknos: i think it was just 16.04 [13:44] since i am reading your readme and it mentions that libvirt interface was not released with snap... is that still the issue or should i try to convert it to a libvirt interface if i understood the normal packaging and able to recreate it? [13:46] Deknos: ah, the readme is stale. the libvirt interface landed in snapd, so the KVM driver binary should work with that plug [13:48] Deknos: so this minikube snap is designed to run minikube on linux.. that's probably not going to work out too well for your windows/macos users [13:48] Deknos: the minikube snap will use docker-machine-kvm2 to create another VM. a VM in a VM is not great [13:48] https://www.irccloud.com/pastebin/NR6enGKq/ [13:49] mvo: ^ re lxd test, that's the issue i'm getting now [13:49] pstolowski: yeah, thats what I see [13:49] pstolowski: I pushed a small PR to test this [13:49] pstolowski: 5828 [13:49] Deknos: https://microk8s.io is probably going to be a better fit for your needs [13:53] i know that miniknube snap will not work on windows/macos. but i hate to do the curl-calls. and i suppose, others, too. therefore, if it is really easier than deb packaging i would want to maintain minikube for snap [13:53] microk8s is then another tool for my toolbox. but people already know minikube (on linux) [13:55] gotcha [13:56] mvo: ah, nice, thanks. i wonder about socket check though [14:04] pstolowski: the socket check I'm not sure about, it looks like this works (after some time) on master but if there is a nicer way by all means, we should switch to the nicer way [14:04] mvo: okay, i'll see if my change works on older systems [14:13] mvo: you PR passed [14:14] *your [14:16] pstolowski: yay [14:16] pstolowski: did your PR with fake backend appends land? [14:17] mborzecki: no, because of lxd test failures [14:17] aah [14:17] PR snapcraft#2269 closed: Provider decides on the image [14:17] pstolowski: cause i'm running a test with UpdateMany() and then every some runs i get bs data in fakebackend ops [14:18] mborzecki: right.. yeah i hope to land my fix very soon [14:25] zyga: can you ack mvo's #5828 ? [14:25] PR #5828: snap-confine: make /lib/modules optional [14:26] it's green (it's refreshing to see green after all-day red ;)) [14:29] pstolowski: \o/ [14:29] zyga: I reported the https://github.com/seccomp/libseccomp/issues/129 [14:29] zyga: seccomp issue [14:33] PR snapd#5828 closed: snap-confine: make /lib/modules optional [14:43] pstolowski: eh found where the bug was, fakestore is collecting data by snap-id :/ [14:43] ok, 'm off to that meeting [14:43] wish me luck :-) [14:43] Chipaca: good luck [14:44] mborzecki: so is it another problem, or the actual problem (and we don't need locking)? [14:44] pedronis: turns out mine was not locking related after all [14:44] pstolowski: ^^ [14:44] mborzecki: re https://github.com/seccomp/libseccomp/issues/129> nice report and idea. looking forward to the response [14:44] pstolowski: but caused by map iteration, so randomish hence giving bs results [14:44] mborzecki: k [14:45] mborzecki: i see [14:45] mborzecki: multi snap tests are hard in snapstate (or you need to be flexible checking results) [14:45] because order is not guaranteed by various pieces not can they [14:46] pedronis: yeah, i was pulling my hair out on this [14:47] PR snapd#5829 opened: tests: use lxd's waitready instead of custom polling around lxd s… [14:56] mvo, snapd has some sort of system health check these days, doesn't it? Is this system sane/can it run snaps type thing? [14:56] mvo, is that something an external caller (say, snapcraft) could use? [14:58] kyrofa: it has a mechanism, yes. its not very sophisticated though. its currently a) kernel version too old b) squashfs mountable c) appamror usable (lxd) [14:58] kyrofa: its not available via a call currently but that would be trivial to do [15:00] mvo, that would be quite useful. If you point me in the right direction I'd be happy to do it [15:00] kyrofa: the selftest/ pkg [15:00] kyrofa: in snapd, you could just wire it to a (hidden) command [15:01] kyrofa: to a hidden snap command - selftest.Run() error iirc is the external API [15:01] mvo, e.g. snap selftest? [15:01] cmars, you forked the docker_machine_kvm repository? is there anything in there which is still not in the repository of dhiltgen? [15:03] mvo, or a new binary not in the standard PATH? [15:07] cmars: at least i see no pull from you to him... [15:07] bbl [15:08] mvo, I'll work toward `snap selftest`, let me know if you prefer something different [15:13] kyrofa: thats fine [15:14] kyrofa: yes, snap selftest and it should error differently when not run as root [15:14] kyrofa: the mount selftest needs to be root [15:16] re [15:16] cmars, did you answer my docker-machine-kvm repo question? had to reboot because of bios update [15:17] menace: yes. i think i had a PR into that project, but it may have never landed [15:18] menace: i think it was something to do with libvirt version compatibility, but it's been awhile. [15:19] dhiltgen might have since updated docker machine with the reqd changes [15:19] gotta go afk for now [15:21] i will look into it. dhiltgen and others put the project under a new umbrella, but not much activity. thanks for your help [15:34] Wimpress: hello [15:36] Wimpress: this needs you review https://github.com/snapcrafters/android-studio/pull/28 -- Its inspired by your prior work on sublime-text [15:37]