/srv/irclogs.ubuntu.com/2019/08/13/#snappy.txt

mborzeckimorning05:05
=== vicamo_ is now known as vicamo
zygagood morning06:24
zygahey mborzecki :)06:24
zygahow was flock?06:24
mborzeckizyga: hey06:24
mborzeckizyga: flock was nice an interesting06:25
mborzeckizyga: i'm slowly finishing the report, should send it out soon06:27
zygacool, looking forward to it06:28
zygamborzecki: how was yesterday with regards to master stability?06:34
mborzeckizyga: still haunted by http2 protocol_error06:35
zygawhat about sbuild?06:35
zygais that magically better or fixed?06:35
mborzeckizyga: afaik it occasionally hits timeouts06:37
mborzeckizyga: also, mvo added a pr that retries when there's a http2 protocol error, it ought to be better now06:37
zygaoh, that's good06:37
mborzeckizyga: renewed my rhel developer subscription ;)06:55
zygamborzecki: cannog go wrong with buying IBM ;-)06:55
mborzeckizyga: btw. need to investigate but snapd doesn't build on rhel806:56
zygadeps or tests?06:57
mborzeckizyga: it looks like the go rpm macros are a problem06:57
zygaof course :)06:58
zygahttps://github.com/snapcore/spread gives me 50406:58
zygais that just me?06:58
mborzeckizyga: things like %gobuild_static are not expanded07:00
mborzecki504 Not Worthy ;)07:00
zygathat's typical when the macro does not exist07:00
mborzeckizyga: same here07:00
zygamborzecki: did you post the spread bugfix?07:00
mborzeckizyga: not yet07:00
zygaOK07:00
=== pstolowski|afk is now known as pstolowski
pstolowskimorning07:09
zygahey Pawel07:14
pstolowskizyga: hey, what's the plan with the per-user-mount-ns PRs?07:25
zygapstolowski: it's all sad, I'm stuck behind layers of other bugs07:27
zygapstolowski: I will try to re-enable the mount measurement test now07:27
pstolowskizyga: uhm i see :(07:28
zygapstolowski: at least on ubuntu where I think, it no longer fails07:28
zygawe need spread fixes07:28
zygawe need little test fixes (still quite a few)07:28
zygahmm07:31
zygasbuild test uses fastly07:31
zygaI: Updating chroot.07:31
zygaHit:1 http://cdn-fastly.deb.debian.org/debian sid InRelease07:31
zygamaybe it's all connected? :)07:31
zygabrb07:33
* zyga fights more broken tests08:04
zygaeh08:04
* zyga does reviews while tests churn08:07
zygaspread output is unreadable08:13
zygathere, I said it08:13
zygapstolowski: https://github.com/snapcore/snapd/pull/723408:16
pstolowskizyga: huh08:16
=== Girtablulu is now known as Girtablulu|Away
jpeWondering where I can find info about how the isolation in snappy actually works? Google isn't really being helpful.08:46
zygajpe: hey08:51
zygajpe: I don't have any good one point of information08:52
zygajpe: if you want to look at the code I can give you pointers08:52
zygajpe: if you want to know the high-level picture I can tell you about that as well08:52
jpeWell the code is a lot08:52
jpeI see it uses apparmour somehow08:53
jpe*armor08:53
jpeIt uses system image overlays similar to docker. So when you install an application you get the overlay for that application which sits on top of the core image. Then access to the system is controlled via apparmour?08:55
zygajpe: we're not using image overlays (not sure what that is), we use mount namespaces08:57
zygajpe: we use cgroups for device (block/char) control08:57
zygajpe: we use apparmor for file level control as well as some misc things, most notably dbus method access08:58
zygajpe: we use seccomp for syscall control08:58
zygajpe: the mount namespaces are used for reliablity mostly, they are not a method of control08:58
zyga(much)08:58
jpeI see so it's a combination of those three tings08:59
zygayes08:59
mborzeckiok, report sent08:59
zygamborzecki: cool08:59
mborzeckiback to normal work08:59
zygamborzecki: I'm chasing flaky tests08:59
jpeby image overlays I meant that you have an imutable 'disk image' like file which is mounted under the mountpoint namespaces.08:59
mborzeckizyga: which ones?08:59
zygajpe: yes but in our case it's much different from what docker does09:00
zygamborzecki: how many fingers do you have :)09:00
zygamborzecki: I lost track, chasing one by one09:00
zygaeverything loves to leave mount points behind (in our test suite)09:00
zygamborzecki: then the usual random suspects09:01
zygamborzecki: protocol error on http2, sbuild, random timing sensitive test09:01
jpezyga: thanks i guess thats enough to get me started09:02
mborzeckizyga: heh, the usual fun :)09:03
pstolowskipedronis: hey, can I land https://github.com/snapcore/snapd/pull/7209 (it has 2 reviews) or would you like to take a look?09:03
pedronispstolowski: I would like to look at it quickly, wil do so today09:04
pstolowskipedronis: great, thanks. i've also re-requested your re-reviews or some other PRs but i'm sure you have a lot to catch up with this week09:06
pstolowskis/or/on/09:06
mborzeckipedronis: hi, will you be able to take a look at https://github.com/snapcore/snapd/pull/7087 ? this is the last one09:07
pedronismborzecki: yes, but need to see in which order, my main task today is to go over PRs09:12
mborzeckipedronis: sure, thank you!09:12
* zyga looks at sbuild test09:30
zygait's so annoying that it fails with no useful output09:30
zygasometimes it doesn't fail with "kill timeout reached" but just, sbuild failed, without any extra detail attached09:32
mborzeckizyga: ha, yeah, so there's a way to extend the debug section and tail -<some-lines> of the log file09:33
mborzeckizyga: if you have a shell, once the build completes, there's a log file in the test directory09:34
zygaah, thanks09:34
zygaI'll check that out and include it09:34
pedronisyes, we do need that, I remember once chasing a real failure there and being very confused09:35
pedronishad to run it manually and stare at that log09:35
pedronis(which is confusing in its own, so big)09:35
ChipacaIDEA: close non-essential PRs until we get our PR count down09:38
Chipacaalso: close stale PRs09:38
Chipacaalso also: close all the PRs09:38
popey_mark them WIP?09:38
zygaChipaca: idea, also holidays for everyone ;)09:38
Chipacazyga: yay09:39
Chipacapopey_: nah, that doesn't help, they're all WIP one way or another09:39
Chipacapopey_: we've got 85 WIP PRs09:39
popey_merge them all then :)09:39
zygaChipaca: I would like to see a way to switch master into "emergency" mode where other PRs don't even get tested09:39
zygaChipaca: but yeah, I share the sentiment09:39
Chipacazyga: per-user mount namespace work09:39
Chipacazyga: from januhary09:39
Chipacajanuhairy09:39
zygaChipaca: I can close it, it's not going anywhere sadly,09:40
Chipacazyga: they're all our children, killing them might hurt a little bit09:40
* Chipaca eyes his children09:40
Chipacait would make things … simpler09:40
zygadone09:41
Chipacazyga: what's the status of #6714 ?09:44
Chipacazyga: do we need to request a re-review from j[d]strand?09:44
zygaI think I need to implement a fallback case09:44
Chipacazyga: ah, i see he responded to my prior query there09:45
zygaI didn't want to but there was a disagreement between people and that's what it is at09:45
Chipacazyga:  i mean, https://github.com/snapcore/snapd/pull/6714#issuecomment-49530251309:45
zygathough I think this will break more things09:45
zygaI need to return there without firefighter mode09:45
Chipacazyga: and #6891?09:47
Chipaca(yes i'm going up PRs per person, backwards, meaning you got to go first)09:47
zygaChipaca: blocked by bugs elsewhere09:47
zygaChipaca: mainly tests leaking random cruft09:48
Chipacazyga: aren't those now fixed?09:48
zygaChipaca: snaps, system changes, et09:48
zyganope, we've found more and we learned about lxd more at the last sprint09:48
zyga(we need to reboot after lxd test)09:48
Chipacazyga: ah, and that one was proving tricky?09:48
zygayeah, needs a patch to spread that mborzecki has09:48
* zyga finds aborting spread test broken09:51
zygahow do I do that again?09:51
zygactrl-c?09:51
zygactrl-d09:51
zygakillall -9 spread09:51
Chipacazyga: ^C will eventually shut it down09:51
Chipacait waits for the current test to finish iirc09:51
Chipacazyga: re #7205, should you tag who you want to comment?09:51
Chipacazyga: re #7225 can we close until the number of PRs is under control again?09:55
zygaChipaca: it runs more tests, I want to abort a test reliably, there seems to be no command to do so09:57
zygaChipaca: 7225 is in response to a PR from sergio, which I think makes our problems worse,09:58
zygaChipaca: number of PRs is not the problem09:58
zygainability to land them effectively is09:58
zygaChipaca: 7205 is something I want to discuss with pedronis and mvo tomorrow09:59
mborzeckizyga: c-\09:59
Chipacazyga: 7225, why isn't that info in the PR?09:59
mborzeckizyga: aka ^\09:59
zygaChipaca: because it was made at a sprint and I didn't include it, it's a draft, I don't think those need to be reviewed10:00
Chipacazyga: 7225 is not a draft10:00
Chipacazyga: ah, you mean 720510:00
zygaChipaca: I was responding about 720510:00
zyga7225 references the other PR10:00
Chipacazyga: just throwing a PR up when they're this many is not enough, you need to actively chase people10:01
zygayeah10:01
zygaChipaca: actively chasing people is not always possible (holidays/timezone), I agree but not sure what the outcome is10:01
zygaclose all the things does not help really10:01
Chipacazyga: 7225, yes but then it's set to blocked after a meeting, because it's not going to be merged for a while (when?), so... why leave it open at all10:01
zygaif we want to review only stuff that helps with the stabilization effort let's say so, let's make a tag or project or whatever10:02
zygaagree not to restart other PRs10:02
Chipacazyga: I don't want us to only review one flavour of stuff, I want us to review all the stuff10:06
zygaChipaca: IMO review is not the problem10:06
zygaChipaca: why cannot we land a typo fix?10:06
zygaChipaca: not because of reviews10:07
zygaChipaca: I think we fundamentally have issue with testing in the project10:07
Chipacazyga: the number of open PRs without two reviews disagrees with you10:07
zygaChipaca: high number is not a problem10:07
zygaChipaca: it's like saying we need to close bugs because we have two pages10:07
zygaChipaca: acting on them is what matters10:07
zygaChipaca: if we cannot act because everything takes forever to iterate we get into this kind of situation10:08
zygaChipaca: I don't disagree we need to focus reviews10:08
zygabut I think open PRs is not the real problem10:08
Chipacazyga: let's just add more PRs then10:09
zygaChipaca: I think we both want the same thing, effective progress10:09
zygaif you want we can HO and discuss10:10
zygaChipaca: can10:12
zyga you look at -- https://github.com/snapcore/snapd/pull/7242  -- it might help us to see what fails in sbuild10:12
pedroniszyga: too many PRs is a problem, it indicates in most cases a lack of focus, both on the opening of, and reviewing side of them10:13
zygapedronis: do you think this is our actual problem now?10:14
pedronisat some point is also a issue of being able to keep state on the open PRs at a personal/mental level10:14
pedroniszyga: yes, more on the reviewing side of this equation but yes10:14
mborzeck1quick errand, back in 3010:15
pedroniszyga: the problem also with saying open as many PRs as you like, is no pressure on the authors on chasing reviews/conclusion on them10:15
zygaI think it depends on how we approach reviews; IMO putting specific barriers in place just discourages contribution10:18
pedroniszyga: barrier on what?10:19
zygaon opening a PR10:19
pedroniszyga: we are not blocking other people10:19
zygaI meant ourselves10:20
pedroniszyga: I don't really know were you are trying to go here10:20
zygapedronis: I was explaining my POV to chipaca about how I feel about the project10:21
pedroniszyga: my main very rough suggestions stay the same, people on the team should try not to have more than 5 PRs open at a time, they should probably also not work on more than 2 main topics at a time10:21
zygapedronis: I have opened 4 PRs today, apart from drafts I have 6 other PRs that are mainly blocked by failing tests or by something else being broken preventing them from going in10:23
pedroniszyga: if tests are failing then the team should fix the failing tests10:23
pedronisnot open PRs10:23
zygawhile I could close the other PRs (and the conversations there) until situation improves I think this is not really a meaningful improvement to our situation10:23
zygapedronis: that's what I am doing10:23
pedroniszyga: to be honest I didn't suggest to close random PRs10:23
pedronisthat came from Chipaca10:23
zygapedronis: how do you suppose we fix anything without opening PR?10:23
Chipacathat was all me :)10:24
pedroniszyga: you are being silly now10:24
Chipacazyga: every PR open uses up the team's finite resources of attention and review ability. While right now we have issues with test, at least half of our open PRs predate that10:24
zygawe do tend to get more revies for the stuff up top so that helps10:24
zygapedronis: that was a joke, but I'm not sure your comment was spot on, we are doing that I think10:24
pedroniszyga: sorry, not really for joke, as long you believe that opening PRs is always good, we have a problem here10:25
zygapedronis: not always good, but should be a lightweight process because they land fast too10:26
zygaI think that's the real issue10:26
pedroniszyga: sorry, you just closed some month old PRs, they weren't being stuck by tests problems10:26
zygayes but I'm not disputing those10:27
zygacachio, Chipaca: https://github.com/snapcore/snapd/pull/724310:48
zygaanother small test usability improvement10:48
zygamborzeck1: can you propose your fix to spread please?10:53
Chipacazyga: why do you say we don't remove base:core18 snaps? i recall no such logic10:56
zygaChipaca: on core systems we routinely don't remove snaps10:57
zygaChipaca: I was looking from the point of view of mounted filesystems, runs on ubuntu-core-16-64 left over pretty much all the snaps that had base:core1810:58
Chipacazyga: that's a bug in our reset.sh then I suspect, because we do reset the state afaik11:00
zygaamazon has systemd 219, hmm11:00
Chipacazyga: so the way to fix that is in reset.sh's reset_all_snap, imho11:01
zygaChipaca: ah, indeed11:03
zygaChipaca: and I think I see the bug now11:03
Chipacazyga: unless the ones you're seeing are test-snapd-rsync or test-snapd-rsync-core1811:03
zygaChipaca: hold on11:03
* Chipaca onholds11:03
zygahttps://www.irccloud.com/pastebin/yd7O7ve9/11:03
mborzeckizyga: opening a PR in a bit, doing some final testing11:03
zygamborzecki: thanks!11:04
zygaChipaca: we never removed them correctly11:04
diddledandocument.getElementById('chipaca').addEventListener('hold', e => 'firm grasp!');11:04
diddledan:-)11:04
diddledanjavascript for the evil!11:04
* Chipaca rejects outright any rumours about being evil, and casts the defamatory individuals into the Pit11:06
diddledandangit, not again11:06
zygacachio, Chipaca https://github.com/snapcore/snapd/pull/724411:06
* Chipaca remembers to remove diddledan's rope ladder this time11:07
Chipacazyga: I'd refactor that to use an array, but maybe not today11:07
zygaChipaca: yeah, one at a time :)11:08
diddledanI can reliably reproduce layouts breaking with my mycroft snap but can't seem to build a simple testcase that is as reliable https://www.irccloud.com/pastebin/AhbwBUUa/11:08
pedronisdoes that mean we can close 7241 ?11:08
mborzeckizyga: https://github.com/snapcore/spread/pull/85 enjoy11:08
zygapedronis: yeah, I just commented there11:09
zygaclosed now11:09
pedroniszyga: how are you tracking to reopen 6714 ?11:09
pedronisis there a trello card? or something else?11:10
mborzeckiChipaca: pinged you in the spread PR too :)11:10
Chipacasaw it11:10
zygapedronis: there's still a bug for it, I will check if there's a card11:11
mborzeckihmmm, spread PR queue has grown11:11
pedronisin general I'm not a fan about closing PRs that are not just nice to have, just for the sake of PR count11:11
cachiozyga, checking11:13
Chipacazyga, pedronis, FWIW is:closed is:unmerged is:pr author:zyga helps11:19
pedronisChipaca: except it's very large11:20
ograjamesh, hah, you rock, how did you manager to get the idea he looks for ant-media-server ? that would have never striked me ...11:28
diddledanhere we go - reproducer: https://github.com/diddlesnaps/layout-fail11:28
diddledanoh, it looks like I've already filed it: https://bugs.launchpad.net/snapd/+bug/180882111:30
diddledanassigned to zyga :-)11:31
zygadiddledan: how does it fail?11:32
diddledanzyga: I've updated the bug11:36
zygadiddledan: I see now11:37
zygahmmm11:37
zygasbuild failed again11:42
pedronisflaky unit test?11:43
pedronisor something else11:43
=== ricab is now known as ricab|lunch
zygatests/main/spread, kill timeout11:43
zygano further information :/11:43
pedroniswe should ask cachio to debug, whether is just taking long to start with11:44
pedronisor some unit test blocks sometimes?11:45
* diddledan rocks out to popeycore music11:45
diddledan"music"11:45
cachiopedronis, zyga I am working with sbuild right now11:46
Chipacadiddledan: no, popeycore is a dress style, like dadcore but more extreme11:46
* Chipaca goes to have lunch before he gets himself in trouble11:46
diddledan:-p11:47
cachiozyga, pedronis still not possible to reproduce the error running here11:47
diddledanChipaca: you're as bad as me. and I'm worse :-p11:47
popey_popeycore is a core laptop, no sound :)11:50
mborzeckipstolowski: really simple selinux fix https://github.com/snapcore/snapd/pull/724511:50
diddledanaah, art nouveau music, popey_ ?11:50
popey_hmm, are there any command line (curses) music creation apps?11:50
mborzeckididdledan: speaking of art nouveau, couple years back, you could listen to your hard disks by running dd if=/dev/hda of=/dev/dsp11:52
pstolowskimborzecki: +111:53
diddledan:-o11:53
mborzeckipstolowski: thanks!11:54
mborzeckipstolowski: i'll force push that little tweak11:54
mborzeckipstolowski: or not, there's more places that could use a similar format change11:58
pstolowskiis there? Ok, nvm then12:06
popey_https://github.com/danfrz/PLEBTracker  oooh12:10
pedronis#6950 needs a 2nd review12:15
cmatsuokaChipaca: I see in master that create-user is still using the v2/create-user api, how should user removal be handled now?12:19
cmatsuokaChipaca: is there a new api that covers all user creation/removal operations?12:19
Chipacacmatsuoka: last I heard we were blocked waiting for field input12:20
cmatsuokaChipaca: ah ok, so I'll wait as well, thanks12:20
Chipacacmatsuoka: mvo might know more12:22
popey_What exactly needs to happen (beyond editing a seed file) for tmux (which is in main) to be added to ubuntu core images?12:51
zygadiddledan: debugged now12:57
Chipacapopey_: you need to make the case that it's worth it, i guess?12:58
Chipacapopey_: one place to start would be with ogra, wrt size impact13:01
ograme ?13:03
ograi doubt i still have a say :)13:03
ograi'd vote for screen instead of tmux though13:03
popey_I spoke to jdstrand about it at the summit, he "voted" for tmux over screen13:03
popey_(upstream screen is dead AIUI)13:04
ograhe never uses serial terminals :P13:04
popey_that's a different use case13:04
popey_I'm talking about a terminal multiplexor here, not a serial console13:04
ograwell, it is a usecase that screen includes13:04
Chipacaogra: how does 'cu' compare to screen?13:05
ogra(and i use serial terminals from core to talk to attached devices...)13:05
popey_what do you use now?13:05
ogradunno, i think i only have used cu once in my life several years ago13:06
=== ricab|lunch is now known as ricab
ograpopey_, screen from a chroot, classic snap or lxd13:06
jdstrandI did advocate for tmux, yes for a multiplexer. a multiplexer does seem like something that should be in core. if screen addresses other use cases, I would not be opposed to screen13:07
popey_Ok, I personally dont care too much either way. However we have had this conversation multiple times over 2 years.13:08
popey_I wondered who I have to nail down to get it13:08
jdstrandwe even had verbal agreement it should be in core :)13:08
Chipacaooh13:09
Chipacamaybe it's just a PR in core then?13:09
jdstrandiirc, mvo liked the idea, but this was before core18 was heavily pruned, and I wasn't part of those coversations13:09
* jdstrand notes that screen is and will continue to be in main for the foreseeable future13:10
popey_despite not having a release for 2 years?13:10
jdstrandoh yes13:11
jdstrandinertia13:11
ograit is "stable" !13:11
ogra(though i recently had actual issues with it ... new rockchip boards use hilarious serial speeds that screen doesnt support OOTB)13:12
ogra(1,500,000 baud by default)13:13
jdstrandI mean, I strongly prefer tmux for maintenance, but it is hard to imagine screen going away. if screen is meeting real world use cases for core that tmux does not, I'm just saying it can be there from an Ubuntu maintenance perspective13:14
ograwell, i'm probably a special case ... but it helps to debug stuff attached via serial to core13:15
jdstrandthat said, popey_ and I are talking about a good multiplexer that a lot of users have asked for and something a snap cannot ship. ogra is talking about serial consoles-- perhaps that continues to be a chroot/lxd/classic snap thing13:16
ogranot sure many people would do it that way13:16
* jdstrand doesn't know13:16
popey_I'm currenly using screen on my core laptop, which I copied out of the deb into ~/bin which is messy13:16
popey_well, if screen is still maintained, size-comparable and can hit two birds with one stone, I don't see why not?13:16
ograusing it is messy ? or the way you got it running ?13:16
ogra(does it work correctly just copying the bin ?)13:17
jdstrandit would feel slightly weird to put something so ancient as screen into core, but then, it isn't like glibc is that fresh (it is very well maintained though)13:17
ograwell, flip a coin :) i'm not pushing that hard for it, it just would make more sense imho13:18
jdstrandit is true that tmux does not support serial console. it would seem to me an unusual use for a core device to need a serial console program since I would expect one to use such a program to connect to the serial console of the device instead of the other way around13:23
jdstrandboth screen and tmux are part of ubuntu-server, but no where else13:23
popey_ogra: it works like that, yes13:23
jdstrandtmux is a modern multiplexer with healthy upstream whereas as screen is a very mature application13:24
popey_that was my main reason for choosing tmux13:24
jdstrandI very strongly advocate for a multiplexer in core. if I had to vote, I would vote tmux13:24
popey_do a pr against the seed and we poke people until it lands ? :)13:25
jdstrandis it just a seed? I mean, I could just do it...13:26
jdstrandbut no, I won't do that without mvo or pedronis saying it is ok :)13:26
jdstrandoh, and I have used cu in the past with mixed results as a serial console. screen has always worked better. I'm not sure if that is a pebkak thing, a docs thing or a software thing13:27
ograjust dont forget that 90% of core installs are headless/userless anyway13:27
ograwe are some special breed being developers that log in to it13:27
popey_Sure, but it's also useful during development13:28
jdstrandthe problem is that it can't be a snap13:28
popey_When migrating from ubuntu server to ubuntu core13:28
pedronisit seems a case where one would install the classic snap and use it from there?13:28
ogranote also that nearly all relevant developer tools were dropped on the switch to core1813:28
popey_you can't install classic snaps on core pedronis13:29
jdstrandpedronis: that is what people do, but that doesn't work well to use it as part of your regular workflow13:29
ograso i'm not sure what adding a multiplexer means while you cant even touch the bootloader config anymore and such13:29
pedronispopey_: I mean the classic snap,13:29
pedronisnot classic snaps13:29
ograheh13:29
popey_NAMING!13:29
ograwe really need a new name at some point13:29
jdstrandthe classic snap (or an lxd container) puts you in a box, not on the host13:29
ograright13:29
popey_it's additional overhead too13:29
jdstrandand people want a multiplexer for working on the host13:30
ograsure, buit given all the removals in core18 we dont really encourage development on the host anymore anyway13:30
jdstrandtmux is 500k13:30
ograthats not actually small13:30
ogra(not really big either though)13:31
* jdstrand wasn't using it as justification13:31
ograscreen is 425k it seems13:31
ograso not much difference here13:31
jdstrandbut it has supporting files13:31
popey_they'll also be compressed13:31
jdstrandidk how much they are needed13:32
ograwell, popey_ said he got along with only copying /usr/bin/screen13:32
ogranot sure if thats also true for tmux13:32
jdstrandthe tmux package has no supporting files. I didn't look at deps13:32
jdstrandtmux could be a snap if we created the (gasp) classic *interface*13:33
popey_well, it was a case of installing classic, jumping into classic, installing screen and finding the binary and copying it out13:33
popey_not just copying /usr/bin/screen13:33
jdstrandwhich has been discussed from time to time13:33
ogradoes it really need full classic access ? not just some files that would justify a "tmux-core" interface or so ?13:34
popey_it needs to launch arbitrary binaries13:35
ograoh, right13:35
jdstrandit needs to be able to run anything on the system. plus there is a setuid component13:35
ogranot just shells13:35
ograyeah, i fogot that the shell you start indeed runs inside tmux13:36
jdstrandso, if we wanted a tmux-support interface, I could *perhaps* make it work without modifying tmux13:36
jdstrandit would need an installation constraint. the idea would be that I let it do what it needs to do, then use a 'ux' rule to the shell13:37
jdstrandthis is hand-wavy13:37
jdstrandI still maintain that a multiplexer should be in core. tmux isn't only for development. admins use it and we provide ssh for admins13:38
ograanyway, i think the decision process is somewhere between pedronis and foundations ... though if we add it, the question is, shopuldnt we also add keymaps, locales etc etc ... since we turn it into an actual developer image13:38
ogra... where do we draw the line13:38
popey_I don't see that we're making it a developer image13:38
popey_it's just a useful tool13:38
jdstrandas mentioned, not for devel, for admins. devs get a nice tool in the process as a side effect13:39
pedronisjdstrand: is #6767 on your list?13:40
jdstrandthat's right, it is setgidness (utemper)13:40
Chipacabrb, need to reboot13:40
ograjdstrand, what admins ?13:41
ograyou usually control core from some central mgmt tool via snapd-control13:41
ograand dont usually even have an account on actual products13:41
jdstrandogra: there are more than devs connecting to ubuntu core via ssh. those people13:41
ograwell, talking from a product/sales perspective here ...13:42
jdstrandsure, but that doesn't mean that others don't create that user. we know of customers that demanded certain user features13:42
jdstrandand they even had a management snap13:42
jdstrandsome want ssh as an escape hatch13:42
ograi havent seen a single sale in the recent months where it was actualy not requested to *avoid* logins13:43
jdstrandI am not saying it is a primary use case, I'm agreeing with popey_ that it is a useful tool13:43
ograbut indeed, brand-store owners can sping own images and add system users at will13:43
ogra*spin13:43
ograjdstrand, but so is nano for many people ...13:44
ograshoudl we add it too13:44
ogra... and typing on a non-us keyboard13:44
jdstrandogra: but there is no multiplexer in core. there is an editor13:44
popey_(I have nano in ~/bin too :( )13:44
ograhahaha13:45
ijohnsonpopey_: snap install nano --devmode13:45
* ijohnson hides13:45
ograyeah13:45
jdstrandyours is an argument to have tmux and screen when there is only tmux. mine is pick something :)13:45
ograwell, my question is if we need it urgent enough to add it :)13:45
ijohnsonfwiw I would love to have tmux just for terminal multiplexing13:45
ogra(either of them)13:45
jdstrandpedronis: re 6767, that and whole bunch of others13:45
pedronisjdstrand: ok, just double checking, it's a target for 2.4113:46
ograbut as i said, not really my decision anymore anyway13:46
jdstrandpedronis: I'll take a looked at the milestoned 2.41 PRs and do them first13:46
jdstrandlook*13:47
ograi'd personally take screen is what i said ... thats all ... for multiplexing on logout we have nohup installed13:47
pedronisjdstrand: thx13:47
ografor multiplexing sessions there is indeed nothing13:47
jdstrandpedronis: I'm going to do a push to get through as much as I can today/tomorrow, then get back to k8s/pulse13:47
pedronisjdstrand: I was back from holiday yesterday, I will go over your deamon user PRs in the next couple of days13:48
ogra(though i bet you can use nohup as session multiplexer when doing some scriptery)13:49
ograbut that would most likely become tmux re-implemented in shell :)13:49
jdstrandpedronis: thanks. fyi, while not terribly urgent, I redid PR 6436 some time ago based on your feedback. it is ready to re-review13:53
jdstrand(system-backup)13:53
jdstrandpopey_: I took 30 minutes to explore a tmux-support interface. I think it is possible: set seccomp to unrestricted, opt out of the devide cgroup, apparmor allows transitioning/transitions to unconfined, ship utmpter as setgid root. this gives everything except the hosts /tmp14:15
* cachio afk14:18
jdstrandpopey_: actually, no, it doesn't allow running snap commands14:20
jdstrandthis is seriously, really best as a part of core14:20
* zyga breaks14:37
* ogra looks for the glue14:43
abeatojdstrand, afaik one thing that screen does and tmux not is access to serial devices (/dev/ttyUSB* and the like), which is incredibly useful if you are connected to a serial console or something like accepts AT commands14:55
ograyeah, on-device GSM modems and such14:56
abeatoor bluetooth devices too14:56
ograah, indeed14:56
abeatozigbee, etc.14:57
ograthat sounds like another point for screen over tmux14:57
pedronisijohnson: Chipaca: I did another pass over #7149, there is a bit of confusion about how error kinds work there, Chipaca can also help in that area14:57
ijohnsonthanks pedronis I'll take a look now14:57
pstolowskiijohnson: hey, a small hint re hotplug wrt to the forum topic about serial port14:58
pstolowskiijohnson: i think you looked at serial_port - HotplugDeviceMethod and concluded id vendor/model are the required attributes; this aspect may be confusing - you should be looking at hotplug.go - defaultDeviceKey() and attrGroups list. there are 4 groups of attributes we look into, and we require >=2 attributes from those groups to be present15:01
ograpstolowski, interesting ... how would you actually do GPIO via hotplug ... they dont exists until you "echo $gpionum  >/sys/class/gpio/export" which is what only the gpio interface does today once you connect to it actively ... (so the device doesnt show up until the interface gets connected, no udev  stuff going on afaik)15:02
ogra(i'd love if we could do them more dynamic ... but the concept doesnt sound like it could work)15:03
ogra(referring to your forum answer ... i dont want to bloat the thread with offtopic chatter)15:04
pstolowskiogra: ok, maybe i said something silly.. i think i remember zyga suggesting gpio discovery like this, but that was long time ago in early days of hotplug, maybe i misremembered it15:04
ijohnsonpstolowski: thanks for the pointer, I had missed that part15:05
ograwell, i'd love if we could find a way to use hotplug for them ...15:05
ograbut i dont thing tieing it to hotplug will work for this... that might need a new mechanism15:05
ogras/hotplug/udev/ (sorry)15:05
ijohnsonpstolowski, what if in the HotplugDeviceDetected method for the serial-port interface we also checked to see if ID_BUS was "pci", that would probably work for this specific device they have15:06
ograpstolowski, something like reading from the devicetree (via /sys/firmware/devicetree/base/) could work for them though ...15:07
ograyeah ... that would likely work: https://paste.ubuntu.com/p/bwxppNgtp6/15:10
ogra(would need some filtering to find gpio-only interfaces, i.e. not spi or i2c)15:10
pstolowskiijohnson: yes, absolutely, although we should look at device key compuitation and possibly make pci path a part of the key for such cases (based on ID_BUS?). every hotplug interface can define HotplugKey method to override how unique key is computed15:13
ijohnsonpstolowski, for this specific case, it already would qualify with ID_VENDOR_ID=0x8086 and ID_VENDOR_FROM_DATABASE=Intel Corporation for the vendor attr group15:14
ijohnsonso I think all that would be needed to unblock this specifically is that check that the ID_BUS matches usb15:15
zygacachio: can you disable sbuild test until we know what's wrong15:19
zygacachio: it's blocking all activity15:19
pstolowskiijohnson: it needs 2 attributes from different groups, so vendor + model from the other group. yes. i'd extend this with path though for static devies to make sure we handle multiple devices with same vendor+model (in the absence of serial#). and yes we have a problem there afaict if serial is not present15:19
pstolowskiin that multiple instances of same device with no serial will not be distinguished15:20
pedronispstolowski: I made a comment in 7209, my plan is much more complicated though :/, let me know what you think15:21
pstolowskilooking15:22
ijohnsonpstolowski, oh I see what you're saying sorry I misunderstood you before I thought it needed 2 from the same group15:22
ijohnsonpstolowski: when you say extend it with path for static devices, do you mean DEVPATH which in the code is di.DeviceName()? that's already set in the proposed slot attributes returned from HotplugDeviceDetected method for serial-port interface15:31
ijohnsonerr wait I read wrong - currently the code uses DEVNAME as di.DeviceName(), are you saying it should be extended to instead use "path" attribute in the proposed slot as the value of DEVPATH15:32
ijohnsoni.e. `"path": di.Attribute("DEVPATH")` or thereabouts15:33
pstolowskiijohnson: yep, it should use whatever attribute makes that static device unique in the system (DEVPATH looks ok); it only makes sense for static devices. it must be made part of the hotplug-key which is crucial for hotplug logic and tracking of devices15:35
ijohnsonpstolowski: so is the hotplug-key determined from the attributes which are returned from HotplugDeviceDetected in the hotplug.ProposedSlot?15:36
pstolowskiijohnson: and note it's strictly about hotplug key which is internal to snapd, it is not about what we expose as attributes of a slot to the user15:36
pstolowskiijohnson: no. it's computed by defaultDeviceKey() method in hotplug.go from the predefined groups of attributes - OR - by optional HotplugKey(..) method of the interface, if defined.15:39
ijohnsonahh I see that HotplugKey isn't defined for the serial-port interface so that's why I didn't see it15:39
pstolowskiijohnson: so if we know a specific interface needs specific logic for some devices - such as serial port & static ports with pci bus, we can override key logic just for serial port15:39
pstolowskiijohnson: so far have just serial port interface and no such use cases, so it's a bit hard to follow ;)15:41
pstolowski*we have15:41
ijohnsonpstolowski: so can an implementation decide in the HotplugKey method to instead use the defaults instead of always providing it's own?15:41
ijohnsonthen for serial-port we could have a check, if it's pci then the interface will provide it's own including DEVPATH which will be unique, if it's usb then just use the default from defaultDeviceKey()15:41
ijohnsonit looks like if HotplugKey returns deviceKey as "", then it will fall through and use the default one15:43
pstolowskiijohnson: yes. serial-port can implement HotplugKey method and return an empty key, in which case we fall back to default computation - see https://pastebin.ubuntu.com/p/zPzqVM5xDX/15:43
ijohnsonso I think that would work15:43
ijohnsonyeah haha I just found that function15:43
ijohnsongood that I'm on the right path15:43
pstolowskiijohnson: what's a bit undefined and missing in the enitre story is versioning of hotplug keys in case they are returned by HotplugKey of an interface. we will need to address this at some point15:46
pstolowskiijohnson: versioning may be needed in case we need to change how keys are computed (e.g. new attributes added, affecting how key looks for existing devices)15:47
ijohnsonhmm yes that does look complicated15:47
ijohnsonpstolowski, pedronis: do you think versioning keys from the HotplugKey method of a hotplug implementation would block adjusting serial-port to work for this pci case?15:56
pstolowskiijohnson: imho no, i don't think so - on the basis that this is still experimental16:07
ijohnsonokay, cool I'll give it a try and push something up to try and unblock this person's use case16:08
pstolowskiijohnson: we should probably use 0 as version, like we do with default keys for now16:08
ijohnsonthat sounds good16:08
pstolowskiijohnson: fyi we have a nested vm test for hotplug & serial-port16:11
pstolowskipedronis: commented on #720916:12
ijohnsonright I'll try to look at that as well16:13
pstolowskiijohnson: i don't think you'll be able to extend this for new use case, but at least it can prove no regressions16:14
pstolowskiijohnson: i think pedronis or mvo should ack this change, so we don't commit to providing something we need to remove later (although it looks relatively uncontroversial to me)16:16
ijohnsonyes I was going to request reviews from them as well16:16
pstolowskicool16:16
=== pstolowski is now known as pstolowski|afk
cachiozyga, #724616:26
cachioplase approve :)16:27
ijohnsoncachio: if it's any help I approved that too16:28
cachioijohnson, sure, thanks!!16:29
* cachio afk18:52
ijohnsonjdstrand: did you see my question in the PR description of #7214?20:55
ijohnson(thanks for the review BTW)20:55
jdstrandijohnson: oh, I didn't, let me look21:30

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