mgerdts | thanks blackboxsw. I'll try to get through all of your suggestions today or tomorrow. | 12:24 |
---|---|---|
robjo | smoser: Q about ds-identify/cloud-init-generator | 14:42 |
smoser | ok. | 14:43 |
robjo | We've run into a case where ds-identify fails because blkid cannot be found | 14:43 |
smoser | as in not present ? or not in path | 14:43 |
robjo | blkid lives in /sbin, but it also lives there in Ubuntu | 14:43 |
smoser | cloud-init assumes that the system is set up to have a sane path | 14:44 |
robjo | now if I modify ds-identify to be /sbin/blkid then the error goes away | 14:44 |
smoser | isnt it just more reasonable to have PATH set up to have utilities in it than to have every tool hard code where it thinks the right place for that is ? | 14:44 |
robjo | but the generator run in the context, i.e. the environment of systemd, i.e. PID 1 and that context nothing is supposed to be assumed | 14:45 |
robjo | I agree with your reasoning but for some reason systemd early execution environment doesn't appear to be the same | 14:46 |
smoser | well you're suggesting i assume that blkid is /sbin/blkid | 14:46 |
smoser | aren't you? | 14:46 |
robjo | I do not know if we decided to fiddle with it or the Ubuntu systemd maintainers | 14:46 |
smoser | https://coreos.com/os/docs/latest/using-environment-variables-in-systemd-units.html | 14:46 |
robjo | no, where I am actually going with this is a question about he generator itself and why it should exist | 14:47 |
smoser | maybe htts not the right link. | 14:47 |
smoser | but surely some way you can change the PATH environment of PID1 | 14:47 |
robjo | but this is not a unit file, it's the generator | 14:47 |
robjo | https://www.freedesktop.org/wiki/Software/systemd/Generators/ | 14:47 |
smoser | yeah. | 14:51 |
smoser | https://www.freedesktop.org/software/systemd/man/systemd.environment-generator.html maybe ? | 14:51 |
smoser | i dont know how the path gets set on ubuntu. | 14:51 |
smoser | i'm not entirely opposed to adjusting PATH but it just seems like the wrong way to do things. | 14:52 |
smoser | and i am pretty solidly opposed to cloud-init having to set PATH in other places. | 14:53 |
robjo | Well I can chase the PATH issue by asking our systemd maintainers and when I have a better understanding we can maybe revisit if needed | 14:54 |
robjo | but what I really want to do is drop the generator from my package all together | 14:54 |
robjo | based on me looking through the code ds-identify tries to do what each data source is doing already | 14:55 |
robjo | so why probe twice? | 14:56 |
robjo | if ds-identify doesn't run the Python code still finds the datasource and gets it's data | 14:56 |
smoser | correct. it is supposed to operate without ds-identify. | 14:56 |
smoser | the thing that it can't do without ds-identify is completely disable itself. | 14:57 |
smoser | ie, if ds-identify doesn't find anything then cloud-init.target will not be targetted. | 14:58 |
smoser | and no bottlenecks imposed as a result of that. | 14:58 |
smoser | and no python code run at all in boot | 14:58 |
smoser | ie, you can install cloud-init on your ubuntu desktop and its completely inert (other than the generator running). | 14:58 |
smoser | https://mail.python.org/pipermail/python-dev/2018-May/153296.html | 14:59 |
robjo | fair, but if I impose cloud-init should be used/installed and services enabled only in an environment where it is actually useful then the "check if I should run" becomes kind of unnecessary | 14:59 |
smoser | that is true. bu the value of ds-identify also comes from trimming the DataSource list. | 15:03 |
smoser | so less code runs in python too | 15:03 |
smoser | but more code in shell | 15:03 |
smoser | so... | 15:03 |
robjo | problem is if it fails the Python code, which in this environment then actually works, never gets a chance | 15:04 |
robjo | in a test where we set the full path ds-identify still failed, i.e. no data source was found | 15:05 |
smoser | yeah. | 15:05 |
smoser | i'm fine with simply PATH=$PATH:/sbin:/usr/sbin:/bin:/sbin | 15:05 |
smoser | but honestly you should chase having a sane environment | 15:06 |
robjo | but when forced in that environment, i.e. ds-identify returned true, then cloud-init found a datasource and configuration worked | 15:06 |
smoser | because i'm not the only person that assumes sane environment | 15:06 |
robjo | I will chase that, composing an e-mail this minute | 15:06 |
smoser | i'm pretty confident that ds-identify as it is riht now will improve boot speed | 15:09 |
robjo | but we run the risk of not configuring the system at all | 15:11 |
smoser | well, thats a bug. we can fix it. you can just hard code the PATH. | 15:23 |
smoser | when using software, you do run the risk of bugs. | 15:23 |
smoser | but suggesting you should not use software because of that is not really an option | 15:24 |
smoser | robjo: fyi | 15:32 |
smoser | $ sudo cat /proc/1/environ | tr '\0' ' ' ; echo | 15:32 |
smoser | HOME=/ init=/sbin/init recovery= TERM=linux drop_caps= BOOT_IMAGE=/boot/vmlinuz-4.15.0-20-generic PATH=/sbin:/usr/sbin:/bin:/usr/bin PWD=/ rootmnt=/root | 15:32 |
smoser | i'd be interested in seeing what you have there. | 15:33 |
smoser | interesting. but in a container: | 15:34 |
smoser | $ lxc exec x1 cat /proc/1/environ | tr '\0' ' '; echo | 15:34 |
smoser | container=lxc | 15:34 |
smoser | thats *it* no PATH there. i'm not sure what ds-identify has for path there. i'll check. | 15:34 |
smoser | even in that container environment i get | 15:36 |
smoser | PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin | 15:37 |
smoser | and... robjo seems like it comes from | 15:38 |
smoser | # strings /lib/systemd/systemd | grep PATH= | 15:38 |
smoser | PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin | 15:38 |
robjo | cat /proc/1/environ | tr '\0' ' ' ; echo produces a very long list of things, weeding through it | 15:39 |
smoser | thats odd. | 15:39 |
robjo | PATH=/sbin:/bin:/usr/sbin:/usr/bin | 15:40 |
smoser | in systemd: | 15:41 |
smoser | src/basic/path-util.h | 15:41 |
robjo | which does make sense because in the images we build there is no issue with ds-identify or I would have found this a while ago | 15:41 |
smoser | #define DEFAULT_PATH_NORMAL "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin" | 15:41 |
robjo | so that makes me think that the party complaining is also fiddling with stuff they shouldn't | 15:42 |
smoser | https://git.launchpad.net/ubuntu/+source/systemd/tree/src/basic/path-util.h#n31 | 15:45 |
robjo | smoser: thanks for digging all of this up much appreciated | 15:46 |
smoser | upstream trunk much harder to read | 15:47 |
smoser | https://github.com/systemd/systemd/blob/master/src/basic/path-util.h | 15:47 |
=== blackboxsw is now known as blackboxs | ||
=== blackboxs is now known as blackboxsw | ||
smoser | blackboxsw: https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/343738 | 17:09 |
smoser | updated | 17:09 |
blackboxsw | smoser: approved the above, parting comment for us to chat about some standup | 19:19 |
blackboxsw | just confirmed this yaml.load issue in cloud-init for utf-8 https://bugs.launchpad.net/cloud-init/+bug/1768600 seems fairly signification for someone presenting their own runcmd info that could contain printable utf-8 chars. | 19:21 |
ubot5 | Ubuntu bug 1768600 in cloud-init "UTF-8 support in User Data (text/x-shellscript) is broken" [Medium,Confirmed] | 19:21 |
smoser | :-( | 19:48 |
* blackboxsw is working up a quick unit test for robjo | 20:25 | |
blackboxsw | thx for the branches | 20:25 |
robjo | blackboxsw: thanks | 20:26 |
robjo | there didn't appear to be a distinct place where we test the config for stage handling | 20:27 |
robjo | and the tests that run through stages.py confused me :( | 20:27 |
blackboxsw | yeah robjo that stuff (init/stages/and cli main) are pains and not very testable. I started some of it in cloudinit/cmd/tests/test_main.py which I'm adding to now for your branch. | 20:28 |
blackboxsw | so it's hard to tell people they should add tests there because those tests and ugly/mock hells | 20:28 |
blackboxsw | yeah I want cloudinit/tests/test_stages.py, but again haven't yet done that overhaul to pull all tests from tests/unittests to the appropriate tests subdir next to each cloudinit module | 20:29 |
blackboxsw | it's going to be a long manual crank on that one | 20:29 |
dpb1 | blackboxsw: for the metadata unification.. do we have the 6-10 fields that we want to name? | 22:00 |
dpb1 | instance_id, etc | 22:00 |
blackboxsw | dpb1: currently we have only | 22:13 |
blackboxsw | "v1": { | 22:13 |
blackboxsw | "availability-zone": null, | 22:13 |
blackboxsw | "cloud-name": "nocloud", | 22:13 |
blackboxsw | "instance-id": "myb1", | 22:13 |
blackboxsw | "local-hostname": "myb1", | 22:13 |
blackboxsw | "region": null | 22:13 |
blackboxsw | } | 22:13 |
dpb1 | ok | 22:13 |
dpb1 | thx | 22:13 |
dpb1 | is that in a card somewhere? | 22:14 |
blackboxsw | I think we are missing probably around 5 fields from the initial cut I took. I'll draw up a doc and card (as I have to refresh my memory on the cloud-instance data we see nowadays | 22:14 |
blackboxsw | I only spiked in NYC and left it to undocumented as I initially hit some datasource generalization issues that'd have to be resolved first | 22:15 |
blackboxsw | dpb1: I did capture std instance-data on a few clouds here https://forum.snapcraft.io/t/feeding-cloud-init-data-to-snapd/3474/21 because as you know, if it's not on snapforums it doesn't exist ;) | 22:16 |
blackboxsw | I'll do a quick review of those to see what we can pull out of the 'ds' related keys provided by the datasources | 22:17 |
* blackboxsw was thinking instance.type, network related information as initial attributes | 22:18 | |
blackboxsw | but there will be bit of work first to pull that content out of certain datasources , like OpenStack's network_data.json needs to be represented in our instance-data.json format | 22:21 |
dpb1 | the snapcraft forum? | 22:30 |
dpb1 | what | 22:30 |
dpb1 | blackboxsw: your pastebin's there are meant to be your example of what *would* show up, right? | 22:30 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!