| mup | PR snapd#7272 opened: [PATCH] interfaces/bluez: enable communication between bluetoothd and meshd via dbus <Created by jrigling> <https://github.com/snapcore/snapd/pull/7272> | 01:18 |
|---|---|---|
| mborzecki | morning | 04:56 |
| mborzecki | brb, new kernel | 05:22 |
| mborzecki | re | 05:23 |
| zyga | Hejka | 06:11 |
| mborzecki | zyga: hey hey | 06:18 |
| mborzecki | mvo: hey | 06:42 |
| mvo | hey mborzecki | 06:43 |
| mborzecki | mvo: anything intersting happened on thu/fri last week? | 06:43 |
| mborzecki | mvo: https://github.com/snapcore/snapd/pull/7257 failed on 14.04 | 06:44 |
| mup | PR #7257: packaging: fix symlink for snapd.session-agent.socket <Created by mvo5> <https://github.com/snapcore/snapd/pull/7257> | 06:45 |
| pedronis | mvo: hi, did you see my fix for the FindU/Gid spread problem, it was a silly issue | 06:46 |
| mvo | pedronis: aha, thanks! I noticed it failing but then had to leave and was a bit puzzled, thanks for sorting it out | 06:52 |
| mvo | pedronis: oh fun! now I see the fix, silly indeed | 06:53 |
| mvo | pedronis: thanks :) | 06:54 |
| mvo | mborzecki: I run it in spread with -debug now | 06:56 |
| zyga | re | 06:58 |
| * zyga looks after PRs from last week | 06:58 | |
| zyga | thank you for iterating on https://github.com/snapcore/snapd/pull/7254 everyone! | 07:01 |
| mup | PR #7254: cmd/snap-update-ns: fix pair of bugs affecting refresh of snap with layouts <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7254> | 07:01 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | morning | 07:15 |
| zyga | hey :) | 07:15 |
| mvo | hey zyga and pstolowski ! | 07:16 |
| mborzecki | pstolowski: hey | 07:17 |
| mborzecki | mvo: can you cherry-pick e864cd99affea16b8742a9f3b5257dfb5f65065f to 2.40? it's blocking https://github.com/snapcore/snapd/pull/7264 | 07:19 |
| mup | PR #7264: packaging/fedora: build on RHEL8 (2.40) <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7264> | 07:19 |
| mvo | mborzecki: sure | 07:21 |
| mvo | mborzecki: done | 07:22 |
| mborzecki | mvo: thank you | 07:22 |
| pedronis | mvo: I merged master into #7124 and gave my +1, but I have a question there, that maybe you can answer too | 07:22 |
| mup | PR #7124: many: create system-usernames user/group if both don't exist <Created by jdstrand> <https://github.com/snapcore/snapd/pull/7124> | 07:22 |
| mvo | pedronis: looking now | 07:23 |
| mup | PR snapd#7263 closed: snap: cleanup some tests, clarify some errors <Simple 😃> <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7263> | 07:31 |
| mvo | pedronis: I will work on your question about 7124 and push a fix (I think we need one but I double check now) | 07:32 |
| pedronis | thx | 07:33 |
| pstolowski | zyga: can you finish the review of #7081? would be great to land it | 07:33 |
| mup | PR #7081: ifacestate: optimize auto-connect by setting profiles once after all connects <Needs Samuele review> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7081> | 07:33 |
| zyga | pstolowski: sure, on it | 07:33 |
| pstolowski | thanks! | 07:35 |
| mvo | pedronis: I think we also need to delete the group, i'm checking now | 07:51 |
| mvo | pedronis: aha, no, nevermind | 07:53 |
| mup | PR snapd#7267 closed: many: simpler access to snap-seccomp version-info <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7267> | 08:26 |
| === mborzeck1 is now known as mborzecki | ||
| mborzecki | this is new: 2019-08-19 07:56:22 Cannot allocate google:ubuntu-18.10-64: cannot allocate new Google server for ubuntu-18.10-64: the resource 'projects/ubuntu-os-cloud/global/images/family/ubuntu-1810' was not found | 08:55 |
| mborzecki | images are gone or what? | 08:56 |
| Chipaca | 1804 just worked for me fwiw | 08:58 |
| Chipaca | bah, 09:38:37, 20 minutes ago | 08:58 |
| jamesh | 18.10 reached end of life at the end of July | 08:58 |
| jamesh | maybe the official images got pulled to encourage upgrades? | 08:59 |
| mborzecki | duh | 08:59 |
| mborzecki | thought we copy those images to our project :/ | 08:59 |
| Chipaca | mborzecki: but we don't have 18.10 in google in our spread.yaml | 09:00 |
| mborzecki | Chipaca: that's a job for 2.40 branch | 09:00 |
| Chipaca | ah | 09:00 |
| mborzecki | hm an update neded then? :) | 09:00 |
| jamesh | It's going to be interesting to see what GitHub Actions does to the CI service landscape in the next year | 09:01 |
| Chipaca | mborzecki: pull #7127 into 2.40 | 09:01 |
| mup | PR #7127: tests: removing support for ubuntu cosmic on spread test suite <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7127> | 09:01 |
| Chipaca | mborzecki: that's f2fa5ce91609d31b8263c92f5f46488d2eedd1f3 if it makes it any easier :) | 09:02 |
| mborzecki | mvo: can you cherry-pick f2fa5ce91609d31b8263c92f5f46488d2eedd1f3 ? | 09:02 |
| Chipaca | hehe | 09:02 |
| mborzecki | Chipaca: thanks! | 09:02 |
| zyga | mvo: did you see that snapd-master cannot be uploaded? | 09:23 |
| pstolowski | zyga: happy birthday! 🍺 🎁 | 09:24 |
| zyga | pstolowski: thank you :) | 09:24 |
| zyga | more gray hair to grow I guess :D | 09:24 |
| pstolowski | :) | 09:25 |
| mvo | mborzecki: sure, done | 09:25 |
| mborzecki | mvo: thank you | 09:25 |
| mvo | zyga: yes, the socket symlink fix will fix this | 09:25 |
| mvo | zyga: onces spread is green I think we can merge it | 09:26 |
| mvo | zyga: oh? I had no idea - HAPPY BIRTHDAY :-D | 09:26 |
| zyga | thank you everyone | 09:26 |
| mborzecki | zyga: 37? | 09:28 |
| zyga | yep | 09:28 |
| pstolowski | oh boy, you're young | 09:29 |
| mborzecki | zyga: happy almost-40 :) | 09:29 |
| zyga | pstolowski: are you older? | 09:29 |
| pstolowski | zyga: 42 | 09:30 |
| zyga | pstolowski: you look like 30 | 09:30 |
| pstolowski | :D | 09:43 |
| mborzecki | pedronis: hi, the renames we talked about https://github.com/snapcore/snapd/pull/7273 | 09:45 |
| mup | PR snapd#7273 opened: gadget, overlord/devicestate: rename Position/Layout <Gadget update> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7273> | 09:45 |
| mup | PR #7273: gadget, overlord/devicestate: rename Position/Layout <Gadget update> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7273> | 09:45 |
| pedronis | mborzecki: thx, will look | 09:47 |
| zyga | mvo: I found a simpler way to solve the lxd problem https://github.com/snapcore/snapd/pull/7274 | 10:03 |
| mup | PR snapd#7274 opened: tests: change cgroups so that LXD doesn't have to <Created by zyga> <https://github.com/snapcore/snapd/pull/7274> | 10:03 |
| mup | PR #7274: tests: change cgroups so that LXD doesn't have to <Created by zyga> <https://github.com/snapcore/snapd/pull/7274> | 10:03 |
| mborzecki | does the store support searching for snaps by free/non-free license? | 10:04 |
| mvo | zyga: nice! good job | 10:04 |
| Chipaca | mvo: silly question: where is grub-editenv supposed to be in a core18 system? | 10:05 |
| mvo | Chipaca: we may not have it there, we don't need it for snapd, it handles all of this via the internal grubenv code | 10:06 |
| mvo | Chipaca: do you need it for tests? | 10:06 |
| Chipaca | ahh | 10:06 |
| Chipaca | mvo: maybe not | 10:06 |
| Chipaca | maybe i was just being lazy :) | 10:06 |
| mvo | heh, ok | 10:07 |
| zyga | pstolowski: I'm still going through | 10:15 |
| zyga | need a coffee, brb | 10:15 |
| Chipaca | how did one stop initramfs before it pivoted into the real thing? | 10:57 |
| Chipaca | something-bottom | 10:57 |
| zyga | Chipaca: break=bottom or break=top | 10:57 |
| zyga | Chipaca: in the kernel command line | 10:57 |
| Chipaca | thanks | 10:57 |
| Chipaca | my core18 is coming up mounted weird :) | 10:57 |
| * Chipaca debugs | 10:57 | |
| mborzecki | Chipaca: weird how? | 10:58 |
| Chipaca | mborzecki: it's something i broke with my boot changes | 10:58 |
| Chipaca | weird as in not mounted? :) | 10:59 |
| Chipaca | but, mounted | 10:59 |
| Chipaca | no home | 10:59 |
| mborzecki | heisenmount | 10:59 |
| pedronis | are we setting snap_core wrong ? | 10:59 |
| Chipaca | these are the questions i'm asking myself :) | 11:00 |
| Chipaca | there's some weird dependency that i'm not seeing, so i'm going to go back to plan C and redo this bit by bit | 11:11 |
| Chipaca | but first, a wild lunch appears! | 11:12 |
| * Chipaca is in the tall grass | 11:12 | |
| mvo | pedronis: I added some comments to 7124, I think we want some more tests for check_snap.go otherwise just nitpicks. probably fine in a followup. I also broke the spread test - it looks like on arch groupdel is not actually working :/ at least that is what the error indicates | 11:18 |
| mborzecki | mvo: do you have more details about the problem on arch? | 11:24 |
| mvo | mborzecki: working on getting details, maybe just some different default | 11:24 |
| mvo | mborzecki: USERGROUPS_ENAB seems to be false on e.g. tumbleweed (zyga can you confirm) ? this is set in login.defs | 11:25 |
| mborzecki | mvo: 196:USERGROUPS_ENAB yes | 11:27 |
| mvo | mborzecki: ok, I look further | 11:27 |
| mborzecki | mvo: which spread test is that? | 11:27 |
| zyga | mvo: checking | 11:30 |
| zyga | mvo: USERGROUPS_ENAB is not an option listed in /proc/config.gz | 11:30 |
| zyga | perhaps the configuration has changed and is no longer there? | 11:30 |
| mvo | mborzecki: it was in 7124 but I think I made it robust enough now | 11:31 |
| zyga | this is on 5.2.5 | 11:31 |
| mborzecki | zyga: it's in /etc/login.defs | 11:31 |
| mvo | zyga: login.defs | 11:31 |
| zyga | ah, sorry | 11:32 |
| zyga | it's indeed set to "no" | 11:33 |
| mvo | zyga: thanks! I updated the test | 11:42 |
| mborzecki | btw. do user ids still start at 500 by default in opensuse? | 11:51 |
| mborzecki | zyga: ^^ | 11:51 |
| zyga | mborzecki: I don't think so | 11:56 |
| zyga | my id is 1000 | 11:56 |
| mup | PR snapd#7269 closed: tests: add test for services disabled during refresh hook <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7269> | 11:56 |
| zyga | mborzecki: UID_MIN is 1000 on TW | 11:57 |
| mup | PR snapd#7268 closed: tests: add /var/cache/snapd to the snapd state to prevent error on the store <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7268> | 11:57 |
| mup | PR snapd#7236 closed: packaging/ubuntu-16.04: provide a way to reset snapd without purging <Created by mwhudson> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/7236> | 11:59 |
| mborzecki | mvo: shall we land https://github.com/snapcore/snapd/pull/7109 ? | 12:07 |
| mup | PR #7109: [RFC] snap-confine: fallback gracefully on a cgroup v2 only system <Created by mvo5> <https://github.com/snapcore/snapd/pull/7109> | 12:07 |
| mvo | mborzecki: if it works well enough for fc31 then yes | 12:10 |
| mvo | (IMHO) | 12:10 |
| mborzecki | mvo: with this branch, the list of failing tests is here: https://forum.snapcraft.io/t/snapd-with-unified-cgroup-hierarchy/12528/2 | 12:13 |
| === ricab is now known as ricab|lunch | ||
| zyga | Chipaca: what is 408? | 12:40 |
| zyga | https://www.irccloud.com/pastebin/2uUquJFw/ | 12:40 |
| Chipaca | zyga: request timeout | 12:47 |
| Chipaca | zyga: https://httpstatusdogs.com/408-request-timeout | 12:48 |
| Chipaca | er | 12:48 |
| Chipaca | zyga: https://httpstatusdogs.com/408 | 12:48 |
| Chipaca | zyga: note it's a _client_ error | 12:53 |
| Chipaca | these days using GPRS you'll hit a lot of 408s | 12:54 |
| zyga | 2fa | 13:01 |
| zyga | mborzecki: hmm, one test exploded on me on fedora | 13:22 |
| zyga | https://www.irccloud.com/pastebin/Q9PySGal/ | 13:22 |
| zyga | it's pretty odd, seems like random corruption and failure | 13:22 |
| zyga | (it didn't fail on the same code before) | 13:22 |
| zyga | sudo: unable to execute ./hightest: Permission denied | 13:22 |
| zyga | may be something I broke after all, please ignore me | 13:24 |
| mborzecki | zyga: iirc hightest only checks user id's in high values range | 13:24 |
| jdstrand | mvo: hey, thanks for the updates for 7124, shall I take over now (note, groupdel --force isn't available until newer releases, but I can fix that) | 13:34 |
| ogra | jdstrand, https://paste.ubuntu.com/p/TpynTdQv8Z/ ... adding time-control (which is a bit funny given the app doesnt have anything to do with time) makes the environment error go away due to teh explicit denial you added there ... wouldnt it make more sense to put that and the /proc/1/sched denial into some generic place instead ? | 13:37 |
| ogra | i assume we never want any snap app to read either anyway | 13:38 |
| mvo | jdstrand: lets have a quick sync with samuele - in a meeting right now | 13:38 |
| mvo | jdstrand: most of my comments are really just nitpicks | 13:38 |
| jdstrand | mvo: we need to fix the groupdel issue, so I'll fix that and small nits | 13:43 |
| mvo | jdstrand: \o/ thank you | 13:44 |
| jdstrand | mvo: and wait for you for other stuff | 13:44 |
| mvo | jdstrand: sorry, was not aware that --foce is not there :/ | 13:44 |
| === ricab|lunch is now known as ricab | ||
| mborzecki | zyga: isn't that a default umask or sth? | 13:49 |
| zyga | AFK for cooking | 13:54 |
| mvo | jdstrand: ok, I just wait for your push and then we can do more stuff in a followup. I am happy to help if you want | 13:56 |
| mvo | ijohnson: hey! 7214 is green and has two +1 - can I merge it ? | 14:08 |
| * ijohnson looks | 14:08 | |
| ijohnson | mvo: I made the changes that jdstrand requested, but I'm not sure if we need jdstrand to re-approve the changes before merging | 14:09 |
| jdstrand | ijohnson: I just now approved (for the security policy bits) | 14:19 |
| ijohnson | thanks jdstrand, then mvo yes I think it's good to be merged | 14:21 |
| mvo | ta | 14:22 |
| mup | PR snapd#7214 closed: interfaces/network-setup-control: allow dbus netplan apply messages <Created by anonymouse64> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7214> | 14:24 |
| ogra | zyga, i think i found a layouts bug :( | 14:34 |
| ogra | zyga, a snap that ises a layout can be installed fine (and teh layout works as expected) ... but if i try to upgrade it i get https://paste.ubuntu.com/p/KnbskvpwQ7/ | 14:36 |
| ogra | (and note that there is no /etc/libblockdev anywhere on the host, so the error is bogus) | 14:36 |
| ogra | s/ises/uses/ | 14:37 |
| ogra | note that snap remove/install works fine as well | 14:40 |
| zyga | ogra: hey | 14:50 |
| zyga | ogra: it was recently fixed, can you check edge | 14:50 |
| ogra | i will ... good to know it is known then | 14:50 |
| zyga | ogra: it was bug .... | 14:50 |
| ogra | yeah | 14:50 |
| zyga | https://bugs.launchpad.net/snapd/+bug/1808821 | 14:51 |
| mup | Bug #1808821: snap with layout cannot be updated multiple times <snapd:Fix Committed by zyga> <https://launchpad.net/bugs/1808821> | 14:51 |
| ogra | btw, is there any possible way to layout something in /run ? | 14:51 |
| zyga | ogra: no, not to /run AFAIR | 14:51 |
| zyga | we NACK that for security reasons | 14:51 |
| ogra | i'm trying to package udisks2 and it tries to write to /run/mount/utab | 14:51 |
| zyga | there's an interface for that | 14:51 |
| zyga | but udisks and utab is a bit of a special case | 14:52 |
| zyga | it's complex to get right, I think utab is a bit of a lost case :/ | 14:52 |
| ogra | well, the snap uses the udisks interface | 14:52 |
| ogra | yeah | 14:52 |
| zyga | I can explain later after lunch, bbl | 14:52 |
| ogra | awe dropped the official udisks2 snap and i want to make at least a demo snap for people relying on auto-mounting on core ... so i'm trying my luck here :) | 14:53 |
| ogra | it automounts fine already | 14:53 |
| ogra | but sadly it never cleans up /media/root but instead adds dir after dir if utab isnt accessible | 14:53 |
| ogra | (one new one for each plug of a disk) | 14:53 |
| Chipaca | return fmt.Errorf("no not remove kernel assets: %s", err) | 15:00 |
| Chipaca | "no anything but that" | 15:00 |
| mvo | Chipaca: heh | 15:00 |
| mvo | Chipaca: where does this one comes from? | 15:00 |
| Chipaca | mvo: that's in the old (current master) boot/kernel_os.go's RemoveKernelAssets | 15:01 |
| pedronis | #7135 is ready for reviews | 15:03 |
| mup | PR #7135: image: support prepare-image --classic for snapd snap only images <Created by pedronis> <https://github.com/snapcore/snapd/pull/7135> | 15:03 |
| mvo | stgraber: hey, is there any in-cloud lxd images mirror? in our spread tests for snapd (that run in gce) I saw that pulling an image sometimes gets ~60kb/sec download speeds only | 15:05 |
| jdstrand | mvo: just finishing up some unit tests | 15:05 |
| stgraber | mvo: using ubuntu: as image remote? If so, we've been requesting it be served by more than a single machine in London for years... | 15:07 |
| stgraber | mvo: you could use images:ubuntu/bionic which is our community image server that unlike the main Ubuntu one does have multiple distributed servers backing it | 15:08 |
| pedronis | ijohnson: I commented briefly on 7270 | 15:14 |
| ijohnson | pedronis: but what about for the disable case? if we only save the info in the change and the change gets garbage collected we would lose that info when someone goes to enable a snap | 15:15 |
| pedronis | ijohnson: ? | 15:15 |
| ijohnson | say someone disables a snap, we save the list of services in the change, they wait for xyz days and the change in state.json disappears | 15:16 |
| ijohnson | don't changes in state.json disappear? | 15:16 |
| pedronis | ijohnson: I'm missing something here | 15:16 |
| pedronis | what worries you? | 15:16 |
| ijohnson | (sorry responding to your comment on #7270, figured it would be quicker over IRC) | 15:16 |
| mup | PR #7270: [RFC] overlord: save disabled snap svcs on unlink snap tasks <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7270> | 15:16 |
| ijohnson | I'm worried about saving the list of disabled services inside a chance since changes aren't persistent | 15:17 |
| ijohnson | ... afaik | 15:17 |
| pedronis | ijohnson: you are thinking of code that is not there yet | 15:17 |
| pedronis | in the code is all about the change, no? | 15:17 |
| pedronis | do you have a case were we need to remember across changes? | 15:17 |
| ijohnson | snap install xyz, snap stop --disable xyz.svc; snap disable xyz; sleep 10000000000; snap enable xyz | 15:18 |
| ijohnson | the enable and disable of the entire snap are different changes, no? | 15:18 |
| pedronis | yes | 15:19 |
| ijohnson | so in that case if we saved the disabled services in the change we would lose it when we disable the entire snap and re-enable the snap | 15:19 |
| ijohnson | it would specifically not be available when we do the enable of the snap | 15:20 |
| jdstrand | mvo: ok, I made the adjustments where it was clear to me what was wanted. I didn't do the bit flags/options struct or invert error message | 15:22 |
| pedronis | ijohnson: I see, problem is we don't do anything like that so far | 15:23 |
| pedronis | and there are two approaches to this | 15:23 |
| jdstrand | mvo: I think this is what you had in mind for testing 'passing as expected': https://github.com/snapcore/snapd/pull/7124/commits/047c7bc64062d0938f1d7b152c540170d7e2f710 | 15:24 |
| mup | PR #7124: many: create system-usernames user/group if both don't exist <Created by jdstrand> <https://github.com/snapcore/snapd/pull/7124> | 15:24 |
| pedronis | ijohnson: we could remember the state as an "intention" when we actually get the enable/disable command | 15:24 |
| pedronis | ignore what is really happening on disk | 15:24 |
| pedronis | sorry I mean the enable/disable command for services (not for the snap) | 15:24 |
| jdstrand | mvo: please let me know what else you need | 15:25 |
| jdstrand | arg, a conflict | 15:25 |
| * jdstrand fixes | 15:25 | |
| pedronis | ijohnson: that would be more similar to what we do in general | 15:25 |
| pedronis | it means tough we could stomp on the user doing things outside snapd | 15:26 |
| pedronis | we don't really promise to always support that tough | 15:26 |
| pedronis | ijohnson: 2nd approach, we can do what your code is doing, we need to think how to make clear that that state is different from all other state we keep tough | 15:26 |
| pedronis | which is the part that confused everybody | 15:26 |
| pedronis | because as I said we don't have state like that | 15:27 |
| pedronis | so far | 15:27 |
| ijohnson | pedronis: hmm so in your first option would we really be "enabling"/"disabling" the service not in the systemd state, but rather inside snapd? | 15:27 |
| ijohnson | so the systemd unit would always show up as "enabled", but if the user disabled it with --disable, then `snap run the-svc` would never really run the service? | 15:28 |
| pedronis | ijohnson: isn't that true with your code as well? | 15:29 |
| pedronis | we remember exactly what the user did | 15:29 |
| jdstrand | mvo: ok, resolved conflict and merged | 15:29 |
| ijohnson | no that's different from how I was imagining it, in my case the state of the service is always in systemd, and just while the snap is inactive between revisions we save the disabled services | 15:30 |
| ijohnson | then on "link-snap", after generating the systemd services we go through that list and disable them and everything is normal | 15:30 |
| ijohnson | s/everything/everything else/ | 15:30 |
| pedronis | ijohnson: yes, I understand | 15:30 |
| pedronis | not sure how it relates to your point tough | 15:30 |
| ijohnson | do you think we need a meeting to discuss? I can explain my reasoning in a forum post if that would be helpful | 15:31 |
| mvo | jdstrand: I love your commit message "don't use --force with groupdel since it is only documented, not implemented" :) | 15:32 |
| pedronis | ijohnson: no I think I understand you what you are saying, nothing stop us to show that snapd state and reality have diverged | 15:32 |
| pedronis | though | 15:32 |
| zyga | re | 15:32 |
| pedronis | anyway I'm just trying to make clear that there isn't only one way | 15:32 |
| zyga | sorry, kitchen emergency | 15:32 |
| pedronis | to attack this | 15:32 |
| pedronis | having just one place with state is appealing | 15:33 |
| ijohnson | hmm | 15:33 |
| jdstrand | mvo: I know right? I had to revert --force in one of my commits on this PR so I learned the hard way | 15:33 |
| pedronis | but the current code in the PR is confusing | 15:33 |
| ijohnson | well from a user perspective I think this would be more confusing because now `snap run the-snap.the-svc` doesn't always actually run the snap | 15:34 |
| pedronis | snap run doesn't work for service like that | 15:34 |
| pedronis | anyway | 15:34 |
| pedronis | ijohnson: I mean confusing code wise | 15:34 |
| pedronis | 3 people have read and didn't get what was supposed to be going on | 15:35 |
| ijohnson | I understand that the code proposal was probably not along the same lines of what has done before, I can clean it up, but in the back of my mind it keeps nagging at me that systemd and snapd are no longer in sync with state | 15:36 |
| jdstrand | mvo: so, on later releases, --force doesn't work, but -f does but on earlier releases neither are documented or implemented. anyhoo, no --force for us :) | 15:37 |
| pedronis | ijohnson: as I said, right now I'm holding both possiblities as viable | 15:37 |
| jdstrand | mvo: it is surprising with how mature those utilities are, how there are still changes and bugs for simple stuff | 15:38 |
| pedronis | ijohnson: the problem with the code is generally SnapState is an absolute truth or intention about the snap | 15:38 |
| pedronis | and the new field you added is neither | 15:38 |
| ijohnson | well in my mind, the new field is an absolute truth, but perhaps the name is wrong | 15:38 |
| ijohnson | the field is an absolute statement about the state of the services when the snap was unlinked, to be used when we re-link | 15:39 |
| pedronis | ijohnson: that's the problem, as named you would think it's true also while that snap is active | 15:39 |
| pedronis | but that's not true | 15:39 |
| pedronis | it's not even reset | 15:40 |
| pedronis | atm | 15:40 |
| ijohnson | I have more changes in the link-snap that would reset it, but I hadn't decoupled that from the change that actually fixes the bug with this state | 15:40 |
| ijohnson | I suppose I could add that change in link-snap to this PR and then rename it to be clear as to what state is being staved in SnapState | 15:41 |
| pedronis | so DisabledServicesAsLastActive might be a better name | 15:41 |
| pedronis | also it likely belongs to SnapState | 15:41 |
| pedronis | and not the one of the SideInfos | 15:41 |
| pedronis | we don't let mess much with a disable snap anyway | 15:42 |
| ijohnson | yeah I wasn't sure which one to put it in, SideInfo was the path of least resistance for me | 15:42 |
| ijohnson | okay, thanks pedronis I think I'm unblocked on this then I will refactor to make it clearer | 15:43 |
| pedronis | ijohnson: basically it belongs close to the Active flag | 15:45 |
| ijohnson | pedronis: is SnapState saved per-revision? it seems like SnapState is saved only once per snap instance, when the state I need to track is per-revision | 15:51 |
| pedronis | ijohnson: is per snap | 15:51 |
| pedronis | why do you think you need to track this per revision? | 15:52 |
| ijohnson | well, imagine you have a snap that gets refreshed multiple times and you want to revert back to one of the previous revisions | 15:52 |
| ijohnson | each revision could hypothetically have different snap service names | 15:53 |
| ijohnson | that also at least handles service renames on revert, since if on rev 1 you have "svc1" and on rev 2 it gets renamed to "svc2", when you get to revision 3 you will only have saved "svc2" and lost the name of "svc1" | 15:54 |
| pedronis | yes, but the service picture is only for the current one | 15:54 |
| ijohnson | what do you mean current one? I thought this flag was meant for only non-current snaps? | 15:54 |
| ijohnson | s/snaps/snap revisions/ | 15:54 |
| pedronis | I mean you install a snap with svc1 and svc2 | 15:55 |
| pedronis | you refresh it | 15:55 |
| pedronis | same services | 15:55 |
| pedronis | you disable svc2 | 15:55 |
| pedronis | you revert | 15:55 |
| pedronis | you want to keep svc2 disabled | 15:55 |
| ijohnson | I guess I didn't think that was a use case we cared about | 15:55 |
| ijohnson | I was more worried about: you install a snap with svc1 and svc2, svc2 doesn't work well, so you disable it, you refresh and now svc2 works better, you enable it, but then you revert and don't want to re-enable svc2 because it was broken in the previous revision | 15:57 |
| ijohnson | hmm I guess I'm not sure we can handle both cases automatically without choosing one use case over the other or having some kind of flag | 15:57 |
| pedronis | I think, thinking of disable services as per revision | 15:58 |
| pedronis | is a bit "too much" | 15:58 |
| ijohnson | I guess from the use case we had on the field team with edgex, it was more important for usability to not automatically re-enable services from previous revisions that were disabled | 15:58 |
| ijohnson | because for example on low power hardware you don't want some of those services running because they are resource intensive | 15:59 |
| pedronis | well, disable again and revert after | 15:59 |
| ijohnson | hmm | 16:00 |
| ijohnson | how would you handle service renames in that situation? I don't think you could | 16:00 |
| pedronis | if there renames is all up to the snap | 16:01 |
| pedronis | or the admin | 16:01 |
| pedronis | notice that if there is not admin | 16:01 |
| pedronis | there is likely no disabled services either | 16:01 |
| zyga | I think for renames the refresh hook should have a chance to control services | 16:01 |
| ijohnson | at least in the per-revision version, you handle svc renames going backwards in time | 16:01 |
| ijohnson | zyga: yes the snap will always have post-refresh hook to control services | 16:02 |
| pedronis | the problem is that is unclear that people expect that behavior | 16:02 |
| ijohnson | zyga: I just added a spread test to make sure that works/continues to work :-) | 16:02 |
| pedronis | ijohnson: to explain, we have similar problem with connections | 16:02 |
| pedronis | but connections are also not per revision | 16:02 |
| pedronis | slot and plugs can come and go | 16:02 |
| ijohnson | hmm right I can see that | 16:02 |
| pedronis | and be renamed | 16:02 |
| ijohnson | I guess my expectation is that the snap's hooks can handle renames going forward in time, but snapd should handle renames going backwards in time | 16:03 |
| ijohnson | but that's just my expectation | 16:03 |
| ijohnson | what do we do with connections on disable/enable? | 16:04 |
| pedronis | they stay | 16:04 |
| pedronis | ijohnson: anywway, what we can do is not reset any disable that has no current matching service | 16:05 |
| pedronis | (that's a bit similar to what we do with manual disconnects) | 16:05 |
| ijohnson | yes in my implementation all service names that don't match a currently existing service it's just ignored and goes on | 16:05 |
| ijohnson | was thinking about adding a warning or something | 16:05 |
| pedronis | ijohnson: yes, but my point is that we also want to reset the list | 16:05 |
| pedronis | but we could keep in it | 16:06 |
| pedronis | those services that didn't match | 16:06 |
| pedronis | so they would be disbaled if we go back to an impl | 16:06 |
| pedronis | that has them | 16:06 |
| pedronis | need to be a bit careful though | 16:06 |
| ijohnson | I guess from a consistency standpoint, services should act like connections | 16:07 |
| ijohnson | so if that's what's wanted I can make it per-snap, and also handle non-matching service names not disappearing somehow | 16:07 |
| pedronis | yes | 16:08 |
| pedronis | as I said, it would be odd for services and connections to have vastly different | 16:08 |
| pedronis | semantics vs revisions | 16:08 |
| ijohnson | pedronis: so we currently store connections separate from snaps in data.conns in state.json | 16:09 |
| pedronis | we do | 16:10 |
| ijohnson | should I try to match that more closely with storing services, or should I still try to fit the service states inside SnapState ? | 16:10 |
| pedronis | no strong preferences | 16:11 |
| pedronis | conns are also always there, and they are intentions | 16:11 |
| pedronis | in your case, state is more state that we cannot keep/pass to systemd momentarily | 16:11 |
| ijohnson | right, yeah I guess connections is also something that snapd explicitly controls | 16:12 |
| ijohnson | I think I will stick to putting it inside SnapState then | 16:12 |
| pedronis | ijohnson: basically, you can think this way, if there's a unit systemd is authoritative about the state, we keep our state when that cannot be | 16:12 |
| ijohnson | yes | 16:13 |
| === pstolowski is now known as pstolowski|afk | ||
| * ijohnson lunches | 17:25 | |
| popey_ | Since when did the output of "snap info" change to no longer show the channel map? | 17:46 |
| popey_ | wait, it shows for some but not all my snaps. how odd | 17:48 |
| popey_ | oh, it's confusing because I have a file in the current directory the same name as a snap, so it's doing "snap info folder/" | 17:49 |
| popey_ | :( | 17:49 |
| popey_ | that's gonna break a bunch of snapcrafters builds, as they use "snap info (snapname)" to interrogate the store | 17:50 |
| zyga | popey_: this has been like that for extremely long now | 18:38 |
| zyga | popey_: perhaps it should be tweaked in two ways | 18:38 |
| zyga | popey_: it could say "local file in big bright green text" | 18:38 |
| zyga | popey_: or it could require snap info ./foo to trigger that semantics | 18:39 |
| popey_ | zyga: yeah, or perhaps some way to override and say "No, check the store please!" | 19:11 |
| Aavar | so, where does a snap keep it's config files? For example if I install something from apt it creates a config in a .file, but it seems a snap does not do that. | 20:36 |
| ijohnson | Aavar, it depends on the snap, some snaps keep their configuration files in the installation directory $SNAP which resolves to /snap/<name-of-snap>/current/, some keep it in $SNAP_DATA, which is usually /var/snap/<name-of-snap>/current, some keep it in $SNAP_USER_DATA which is $HOME/snap/<name-of-snap>/current | 20:38 |
| Aavar | ijohnson, Do you know if it will be purged on removal of the snap? | 20:48 |
| ijohnson | Aavar, $SNAP and $SNAP_DATA will be removed if you do `snap remove <name-of-snap>` (but note you could restore it if you have snapshots enabled) while $SNAP_USER_DATA will persist after removal | 20:49 |
| Aavar | ijohnson, thank you :) | 20:50 |
| ijohnson | Aavar you're welcome :-) | 20:52 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!