[05:21] morning [05:31] hey mborzecki [05:32] mvo: hey [05:35] 56 prs, nice [05:36] sweet [05:37] and 3 easy wins once samuele had a chance to look at the interface prs from jamie [06:05] PR snapd#8882 opened: tests: use the "jq" snap from the edge channel [06:23] mborzecki: hi, #8340 needs a 2nd review, and if possible also to look at #8813 again [06:23] PR #8340: boot, snap-bootstrap: move initramfs-mounts logic to boot pkg [06:23] PR #8813: gadget,cmd/snap-bootstrap: move partitioning to gadget [06:28] pedronis: hi, yes, i'll be looking at those in the morning [06:28] thank you [07:03] morning [07:40] PR snapd#8882 closed: tests: use the "jq" snap from the edge channel [07:45] hey, I'm having some problems when connecting an interface: [07:45] $ snap connect mysnap:network-control [07:45] error: cannot perform the following tasks: [07:45] - Conectar mysnap:network-control a snapd:network-control (cannot update mount namespace of snap "mysnap": cannot update preserved namespace of snap "mysnap": cannot update snap namespace: device or resource busy) [07:45] any idea on what might be going wrong? [07:45] PR snapd#8724 closed: interfaces/block_devices: add NVMe subsystem devices, support multipath paths [07:49] pedronis: heh, that isTrySnapErr does not work indeed, perhaps that's why the tests were not hitting that code path [07:50] I don't think we use new* either from the tests [07:50] pedronis: i've added a test case for that, and it wasn't handled in the right if branch [07:51] pedronis: anyways i'll push a patch there [07:51] thx [08:06] pedronis: pushed now [08:06] btw. we used to have something added to the snapd repo that showed test coverage of go code [08:11] was that codecov? [08:12] mvo: pedronis: do you think we could try enabling https://github.com/marketplace/codecov/plan/MDIyOk1hcmtldHBsYWNlTGlzdGluZ1BsYW4xNg== on snapd repo? [08:13] there's also https://github.com/marketplace/coveralls [08:14] mborzecki: we had coveralls a while ago, I would love to have coverage reports again [08:14] mborzecki: it was a bit cumbersome to setup in the past [08:16] it'd be nice to build & test the C bits too, even if the coverage is orders lower than the go code [08:22] mvo: I re-reviewed #8858, some things to tweak [08:22] PR #8858: asserts,daemon: add support for "serials" field in system-user assertion [08:25] PR snapd#8802 closed: spread.yaml: update secure boot attribute name <⛔ Blocked> [08:26] * Chipaca peers in [08:45] PR snapd#8883 opened: packaging: stop snapd early on purge [09:09] mvo: bump on https://bugs.launchpad.net/snapd/+bug/1883538 :) [09:09] Bug #1883538: Classic to strict refresh requires manual intervention [09:11] Saviq: meh! sorry, put it on my list again [09:36] pedronis: #8813 probably needs a pass from you regarding the naming of things [09:36] PR #8813: gadget,cmd/snap-bootstrap: move partitioning to gadget [09:37] otherwise, it looks quite ok to me [09:39] mborzecki: anything in particular, the new structures? [09:40] PR snapcraft#3174 opened: link_or_copy: do not try to create hardlinks to symlinks [09:40] pedronis: yes, the new structures, OnDisk* [09:45] mborzecki: I'm not completely a fan of those tbh [09:47] mvo: https://github.com/snapcore/snapd/pull/8883#discussion_r441422472 [09:47] PR #8883: packaging: stop snapd early on purge [09:49] mborzecki: right, I can go a different route, i.e. stop just snapd right before we remove the snapd dirs [09:50] mborzecki: but I think this is the reason we see the 20.04 failures, we remove the dirs while snapd is running so it will just re-addd stuff there [09:50] mborzecki: but yeah, I guess it's safer to keep it running until late [09:50] * mvo changes the PR [09:51] mvo: yeah, a middle ground is probaly the safest bet [09:52] mvo: otoh, once we start stopping the mount units and removing directories snapd could get confused, but it's a part of purge anyway [10:02] mborzecki: should I also snapd in snap-mgr or is that not needed there because snpad will not run at this point when snap-mgr purge is used? [10:04] mvo: hmm, snap-mgmt is run after snapd is stopped, so if that does not leave anything behind, maybe we could go with your initial version? [10:05] mvo: the fedora and arch packaging stops snapd (and sockets) before the purge [10:06] mborzecki: cool, I updated the PR now [10:06] mborzecki: basically I'm a bit unhappy that we need to have Structure LaidOutStructure and OnDiskStructure [10:07] I know why they each exist but is a bit much [10:10] pedronis: perhaps we can address that once we're done with snap-bootstrap -> gadget/install move [10:10] then the change should be fairly mechanical [10:11] PR snapd#8884 opened: cmd/snap: Debian does not allow $SNAP_MOUNT_DIR/bin in sudo secure_path [10:13] morning folks [10:13] hi ijohnson ! [10:13] hey pstolowski [10:13] thanks mborzecki for the patches to those PR's [10:14] * ijohnson looks at his only 3 open PR's :-D [10:15] mborzecki: if I see it correctly #8813 is mostly mechanical right? [10:15] PR #8813: gadget,cmd/snap-bootstrap: move partitioning to gadget [10:15] mborzecki: I mean I'm lookin at it, there is not subtle changes in it? [10:16] pedronis: there are some tweaks in the few recent commits [10:17] pedronis: i mean, we could very well do a single pr with renames (provided it's renames only) [10:17] in which sense? [10:17] my plan is not renames only [10:18] mborzecki: my question was really more is this simple enough that we should just try to get it in quickly, or does it need careful looking over anyway? [10:19] my plan to avoid some many struct involve more than renames [10:19] pedronis: maybe let's have a chat with cmatsuoka then? [10:20] anyway reducing the structs is not a priority [10:20] but you asked me if I had opinions about names [10:20] and it's complicated :) [10:22] haha [10:25] Saviq: can you please "snapcraft release multipass 2114 edge/snap-test-classic" for me? this way I can actually try to test your exact scenario [10:32] mvo: done, and invited you as collaborator [10:44] Saviq: thank you I think I can reproduce, let me dig into this [10:44] 8858 needs a second review, should be simple [10:44] well, not that simple but I should not say this or it won't get reviewed :P [10:46] PR snapd#8885 opened: data/sudo: drop a failed sudo secure_path workaround [11:05] ogra your mjpg streamer snap, how do you control the webcam resolution? mine is only grabbing at 640x480 [11:06] ah, there's an option for the .so [11:07] its in the config file [11:08] i have a half done branch that uses snapctl but that has been sitting for a while (it has many users, switching over the config without breaking them is a little work) [11:09] found https://community.octoprint.org/t/available-mjpg-streamer-configuration-options/1106 [11:09] mborzecki: I think zyga asked you to review #8829 ? [11:09] PR #8829: sandbox/cgroup: add tracking helpers [11:12] popey, mjpg-streamer --help ... and mjpg-streamer -i "input_uvc.so --help" [11:12] it comes with documentation builtin too 😉 [11:12] (not all controls work with all webcams though ... watch journalctl output) [11:19] pedronis: so can I merge #8340 ? [11:19] PR #8340: boot, snap-bootstrap: move initramfs-mounts logic to boot pkg [11:19] it's green now [11:21] ijohnson: yes, any concerns? [11:21] nope, just wanted to be extra suer [11:21] *sure [11:21] * ijohnson clicks the button [11:23] and boom [11:26] PR snapd#8340 closed: boot, snap-bootstrap: move initramfs-mounts logic to boot pkg [11:28] Saviq: found the issue, WIP in https://github.com/snapcore/snapd/compare/master...mvo5:classic-to-strict-is-fine?expand=1 but need to write a proper test after lunch [11:28] Saviq: but with your testcase it seems to work [11:28] pstolowski: is #8812 ready for re-review? [11:28] PR #8812: o/snapstate: service-control task handler (4/N) [11:30] pedronis: yes, thank you [11:32] pstolowski: ok, thx, putting it back in the queue === msalvatore_ is now known as msalvatore [12:11] pedronis: remind me, can *util packages import other *util packages ? [12:11] i.e. can I import strutil from osutil/disks ? [12:12] ijohnson: usually yes [12:12] I mean we should be reasonable and avoid circular deps, but there is no sense to a general rule [12:12] that they can't [12:13] pstolowski: I finally got to #8395, I didn't something is missing but maybe I'm confused (could be) [12:13] PR #8395: o/ifacestate: handle interface hooks when preseeding [12:13] pedronis: ack sounds good [12:13] thanks for the clarification [12:13] pstolowski: s/I didn't/I think/ [12:13] pedronis: thanks, will need to refresh my memory, it's been a while [12:16] pstolowski: it's not urgent for you to go back to it, don't context switch for me, but I wanted to reduce my own queue a bit more [12:16] PR snapd#8886 opened: gadget: mv encodeLabel to strutil.EncodeHexBlkIDFormat [12:18] ijohnson: ^ why strutil and not osutil itself though? [12:18] or even osutil/disks [12:18] pedronis: it seems like a string function ? [12:18] pedronis: I mean sure it can live in osutil/disks if you want that's fine [12:19] ijohnson: it's a very very specific string function [12:19] true... [12:20] pedronis: ok, thanks, i've read your comment, i'll definitely need to think deeper, so will get back to it later [12:20] pedronis: mmm ok I'll move it to osutil/disks, but it will make more messy git history in my existing PR [12:20] ijohnson: heh [12:21] ijohnson: that is not a good reason in particular [12:21] ijohnson: anyway except for Version stuff most of strutil is quite quite generic [12:22] is not a meant to be a repository of any sort of side-effect free things that produce strings [12:22] PathIterator is also a bit borderline there [12:27] apparently you can't change branch names in a PR on github which also is maybe a bit expected [12:28] names are hard (in more than one sense) [12:29] alright re-opened as 8888 [12:30] mborzecki: does #8887 supersede #8775 ? [12:30] PR #8887: bootloader: pull recovery grub config from internal assets [12:30] PR #8775: [RFC] bootloader, boot: boot scripts, edition <⛔ Blocked> [12:31] PR snapd#7570 closed: [RFC] snap-mount-dir: add mount unit for snap-mount-dir <⛔ Blocked> [12:31] PR snapd#8886 closed: gadget: mv encodeLabel to strutil.EncodeHexBlkIDFormat [12:31] PR snapd#8887 opened: bootloader: pull recovery grub config from internal assets [12:31] PR snapd#8888 opened: gadget: mv encodeLabel to osutil/disks.EncodeHexBlkIDFormat [12:32] pedronis: i think i'll close 8875 and will be open smaller bits as we go (always one less PR to remember) [12:32] mborzecki: sounds good [12:36] PR snapd#8775 closed: [RFC] bootloader, boot: boot scripts, edition <⛔ Blocked> === alan_g is now known as alan_g_ [13:26] anyone have a suggestion for what to do when kswap0 and snapd (and in this instance canonical-livepatch) lock up CPU? i've been using the default 1GB swap and have run into this a fair bit again recently. it locks up hard enough the only recourse is usually to hard power-off unless you wait long enough for OOM killer to do something, if it even does (laptop was unusable for at least 10-15 minutes in my case). [13:27] cjp256: do you have the output of "journalctl -u snapd" when this happens? and snap changes ideally? would love to know how to reproduce [13:28] * mvo is actually in a meeting [13:35] mvo: the journal https://pastebin.ubuntu.com/p/NqtJryXrwc/ shows a trace. last `change` was a few hours beforehand, not canonical-livepatch fwiw [13:45] cachio: have you seen my comment re early core20 config test? [13:46] pstolowski, no, I didnt, sorry [13:46] just got disconnected [13:47] cachio: in standup notes [13:48] cjp256: thanks, do you have a timestamp for me when the OOM happend? [13:51] cjp256: also - how many snaps are installed there? anything special about the machine this run on? [13:55] mvo: relevant journal log chunk https://pastebin.ubuntu.com/p/ynj57trvfg/ [13:55] cjp256: cool, thanks [13:55] interesting that the logs seem to be out of order :D [13:59] pstolowski, ok, I'll take a look again [14:25] ijohnson, if you make a try with uc20-recovery test please tell me [14:26] cachio: yes sorry got distracted with doing reviews, but will try that real soon [14:26] ijohnson: nitpick on #8888, feel free to apply it on a follow up or something [14:26] PR #8888: gadget: mv encodeLabel to osutil/disks.EncodeHexBlkIDFormat [14:27] pedronis: ack I think I'll take it in a followup [14:31] pedronis: small nitpicks under #8852 [14:31] PR #8852: asserts: introduce new assertion validation-set [14:33] pstolowski: reviewed 8812 for you [14:34] pstolowski: thank you [14:34] ijohnson: thanks! [14:55] ijohnson: updated [14:56] pstolowski, I run twice and both times saw the same error https://paste.ubuntu.com/p/5wfwC7kNbc/ [15:00] pstolowski: re-approved [15:01] ijohnson: thank you! [15:02] PR snapd#8888 closed: gadget: mv encodeLabel to osutil/disks.EncodeHexBlkIDFormat [15:04] cachio: i'll take a look. wasn't something like this happening when things were changing in core20 and it wouldn't boot? i saw similiar problems before when i worked on this and it would automagically work again another day after things landed [15:05] pstolowski, ahh, because it is the only test on which it fails to sdo shpass [15:07] cachio: yes, it fails in prepare already, cannot ssh into the nested system [15:07] PR snapd#8858 closed: asserts,daemon: add support for "serials" field in system-user assertion [15:07] PR snapd#8885 closed: data/sudo: drop a failed sudo secure_path workaround [15:07] in my case it could ssh [15:07] but fails trygin to create the test user [15:08] it is weird bacause all the other uc20 tests are passing [15:10] cachio: ah, wait, i think i misread the log (too many ssh attempts), indeed it seems it's test user password issue [15:10] why would it start failing, did we change something? [15:11] cachio: did it start failing last night? [15:11] pstolowski, let me check [15:13] pstolowski, 2020-06-15 passed [15:13] 2020-06-16 failed [15:14] with the same error [15:14] so, yesterday was the first fail [15:17] cachio: ok, thanks, i'll try to find out what/if something changed [15:22] pstolowski, nice, thanks [15:26] fwiw I pushed a new "test-snapd-classic-confined" to candidate, please let me know if that causes any issues, I will revert in this case (but should be fine) [15:30] cachio: mmm I see a potential issue with uc20 and self signed models, need to investigate more but it would explain your issue [15:30] cachio: can you show your model assertion you used to build the external image ? [15:30] * ijohnson needs to step out for few minutes [15:31] ijohnson, https://github.com/sergiocazzolato/validator/blob/master/images/models/pc-amd64-20.model [15:31] this is for amd64 [15:32] I'm not sure what Ian meant [15:33] ijohnson, it is hte same we use for nested tests on snapd projecy [15:33] project [15:39] ijohnson, what I see is that after reboot /home/gopath does not exist anymore [15:46] cachio I thought the issue was that you couldn't login to the device via ssh? [15:47] ijohnson, I can login [15:47] PR snapd#8889 opened: snapstate: fix autorefresh from classic->strict [15:48] ijohnson, is that the spread path is not there anymore after the reboot [15:49] ijohnson, then I see this https://paste.ubuntu.com/p/VNbk5hjCXf/ [15:49] as we said in the standup, I think the issue is that the system comes up [15:49] in recovery mode [15:49] not run mode [15:49] after the test [15:50] cachio: can you see what's in /var/lib/snapd/modeenv in the system without the path? [15:51] pedronis, mode=recover [15:51] yea, so what I said [15:52] so, why it fails here and is not failing on each PR? [15:52] something is different [15:52] but I don't know the test well [15:52] here I am testing with secboot and tpm enabled [15:52] just making sure we understand the problem the same way [15:52] Ah sorry well the issue I'm seeing is a different one then [15:55] pedronis, ok, thnaks, I'll compare the systems with the one running on gce after lunch [15:57] * cachio lunch [15:59] ah sorry I actually need to step out now for longer === ijohnson is now known as ijohnson|afk [16:03] cachio: i just run that test two times and it passed on second run. something is wonky [16:12] cmatsuoka: I did a pass on #8813 [16:12] PR #8813: gadget,cmd/snap-bootstrap: move partitioning to gadget [16:26] cmatsuoka: I marked it changes requested mostly because of the not glued up tests [16:53] pedronis: thanks, looking [17:01] PR snapcraft#3172 closed: cli: restore --target-arch with warning for LXD and Multipass [17:01] PR snapcraft#3175 opened: cli: restore --target-arch with warning for LXD and Multipass [17:11] PR snapcraft#3176 opened: tools: fix environment-setup to work on aarch64 [17:18] PR snapd#8852 closed: asserts: introduce new assertion validation-set

[17:20] pedronis: added missing glue and todo notes, left some of the changes to follow-ups [17:35] cmatsuoka: thanks, probably deploy.go => install.go and other things still called deploy should be called install, like deployMountpoint [17:36] pedronis: yes, adjusting that [17:37] some test names as well [17:37] pedronis: the mountpoint itself looks a bit strange, do you have a suggestion for a better name? [17:39] hmm, renaming to install.go now could be a problem because there's a file with the same name in the next PR [17:39] cmatsuoka: then call it content.go I suppose [17:39] pedronis: content.go is good, thanks [17:45] cmatsuoka: about the mountpoint, maybe /run/snapd/gadget-install [17:45] /run/snapd is dirs.SnapRunDir [17:45] yes, good, I'll use it, thanks [17:48] * cachio at kinesiologist [17:48] cmatsuoka: if you do something like what with do for FreezerCgroupDir in sandbox/cgroup/freezer.go, maybe you don't need the Mock but dirs.SetRootDir is enough, you'll have to see [17:49] ah, it's only local anyway [17:49] so doesn't matter [17:51] pedronis: so can I just declare an uninitialized var and initialize it on init() to use dirs.SnapRunDir? [17:51] yes, and if you setup a callback you can also have if follow to root in dirs.go [17:52] interesting solution [17:53] as I said, you might not need it here [17:53] I thought you were exporting the Mock but is only in export_test.go [17:54] yes. well, I'll use a simple initialization and then if need arises we can use the callback [17:56] cmatsuoka: you lost deploy_test.go somehow instead of it getting commited as content_test.go now [17:56] https://github.com/snapcore/snapd/pull/8813/commits/6245b309270ee00b818ccfea45a84522c3f11499 [17:56] PR #8813: gadget,cmd/snap-bootstrap: move partitioning to gadget [17:57] that's... strange, let me see here what's happening and I'll push a fix along with the content mountpoint change [17:58] PR snapd#8884 closed: cmd/snap: Debian does not allow $SNAP_MOUNT_DIR/bin in sudo secure_path [18:03] pedronis: pushed. I think the missing test was caused by mistakingly using mv instead of git mv [18:11] cmatsuoka: reviewed [18:11]