| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-04-30 | |||
| 18:03:47 | sean-k-mooney | ok but it should be presented like that today | |
| 18:04:31 | frct1 | If there is cloud-config provided in vendor_data2.json and user-data is provided then cloud-init will merge both of them (user-data will override config options presented in vendor_data cloud-config) | |
| 18:05:37 | frct1 | How do you think what is more preferred way to implement it ? Adding cloud-init as whitelisted is easiest workaround but probably violates idea of DynamicJSON provider | |
| 18:06:01 | sean-k-mooney | personally i dont want to see use special case at all | |
| 18:07:30 | sean-k-mooney | im trying to reconsile what cloud-init expect with what openstack supprots | |
| 18:12:20 | frct1 | Since DynamicJSON combine responses from all providers. May be we can use some sort of suffix to vendor data provider name like "testing@10.10.10.10:1337,cloud-init:string@10.10.13.37:8000" to make it more declarative to let provider know that response should be string. | |
| 18:12:43 | sean-k-mooney | frct1: so our functional tests assert that each backend is a nested resouce contianing a dict repsponce | |
| 18:12:46 | sean-k-mooney | https://github.com/openstack/nova/blob/fa678870b24d450df6d5abce57db2e6eb83703c5/nova/tests/functional/test_metadata.py#L97-L119 | |
| 18:13:20 | sean-k-mooney | so based on that this is a bug in cloud init | |
| 18:13:46 | sean-k-mooney | it shouldl be loopting over the keys and looking for a backend with a nested "cloud-init" key | |
| 18:14:28 | sean-k-mooney | frct1: have you condierd filing a bug with cloud-init for this | |
| 18:14:53 | frct1 | I did it couple days ago: https://github.com/canonical/cloud-init/issues/5221 | |
| 18:16:06 | frct1 | What if multiple vendor data provider responses will have cloud-iniit key ? It is big question on how to solve it | |
| 18:16:27 | sean-k-mooney | we explcitly say that that is not supproted in our docs | |
| 18:17:04 | sean-k-mooney | at lest in our olde rdocs | |
| 18:17:20 | sean-k-mooney | cloud init woudl be free to merge the section si guess if that was the case | |
| 18:21:30 | frct1 | ¯\_(ツ)_/¯ Re-implementing at cloud-init side would require from operators building brand new images with new cloud-init version that contain fixes that could take months to propagate over repos. | |
| 18:22:30 | frct1 | When it is fixed on nova side it could be easily update and all "relatively" recent images will work as expected with new feature | |
| 18:23:11 | sean-k-mooney | but its not broke on the nova side | |
| 18:23:21 | sean-k-mooney | to do what you want would actully have sideffect | |
| 18:23:48 | sean-k-mooney | the first sideefect is obvioulsy that cloud-init | |
| 18:23:52 | sean-k-mooney | is currenlty a valid backend name | |
| 18:24:22 | sean-k-mooney | i.e vendordata_dynamic_targets=['cloud-init@http://127.0.0.1:123'] | |
| 18:24:27 | sean-k-mooney | is currently valid | |
| 18:24:58 | sean-k-mooney | second this is a breaking change to the format provided by openstack | |
| 18:25:06 | frct1 | What do you think on implementing new vendor data provider that operator will be able to configure how response type should look ? For example vendordata_dynamic_targets="cloud-init:striing@http://127.0.0.1:123,testing:json@http://127.0.0.1:124" ? | |
| 18:25:20 | sean-k-mooney | meaning this could only be done in a new version and we woudl have to kept it for all the old version | |
| 18:26:23 | sean-k-mooney | frct1: thats more palitable but we likely need to have a nova-spec for that | |
| 18:26:31 | sean-k-mooney | to ensure there are no upgrade impacts to this | |
| 18:26:48 | frct1 | So old DynamicJSON provider will continue to work as before, but if operator would like to provide some sort of "common" cloud-config he can enable new implementation that covers old DynamicJSON and new one that allows response type explicitly defined | |
| 18:26:50 | sean-k-mooney | we would likely need a new version | |
| 18:26:59 | sean-k-mooney | and only include the string providers in the latest version | |
| 18:28:15 | sean-k-mooney | right now clinet of the vendor data file should expect the fiile to be {<provider>:<data>} string to dict | |
| 18:28:40 | sean-k-mooney | but in the new format they would have to support parsing string to (dict or string) | |
| 18:30:07 | frct1 | So yeah, this is really not a bad option to explicitly define response type. Is there any other options for types to be supported like number or string ? Is it really needed ? | |
| 18:30:29 | sean-k-mooney | we only supprot json today as you saw | |
| 18:30:57 | sean-k-mooney | so each provider is expected to provide a json dict | |
| 18:31:30 | sean-k-mooney | but the content of that dict can technially be any valid json so the nest dict can have strings more dicts number ectra | |
| 18:35:25 | frct1 | I'm talking not about current implementation, but new one provider with options to explicitly define provider response type (json or string) | |
| 18:36:45 | frct1 | I will able to describe proposal for new provider as well as new vendor data provider code covered with tests | |
| 18:39:50 | sean-k-mooney | i dont think this would be a new provider | |
| 18:40:10 | sean-k-mooney | well it depend on what you mean by provider | |
| 18:40:37 | sean-k-mooney | it would not be a new VendorDataDriver but an enhacment to the existing VendorDataDriver | |
| 18:41:39 | frct1 | Now i'm not sure how it should look like... Do you have possible example for configuring it ? | |
| 18:43:06 | sean-k-mooney | maybe somethign like this vendordata_dynamic_targets=['cloud-init:string@http://127.0.0.1:123'] which is what you were suggesting | |
| 18:43:15 | sean-k-mooney | can you try somethign else first | |
| 18:43:50 | sean-k-mooney | technially https://paste.openstack.org/show/bYyFYXCJ5GCPrW45iYEC/ is valid json | |
| 18:44:19 | sean-k-mooney | if you neamed the provider "vendordata_dynamic_targets=['cloud-init@http://127.0.0.1:123']" today | |
| 18:44:30 | sean-k-mooney | and it provide jus the quoted string as the respocne | |
| 18:44:32 | sean-k-mooney | i think it would work | |
| 18:47:07 | frct1 | Nope, just tried one more time. https://paste.openstack.org/show/bRonCys8hzLUKA6FyQr0/ | |
| 18:47:24 | frct1 | https://opendev.org/openstack/nova/src/branch/master/nova/api/metadata/vendordata_dynamic.py#L98 this line raise exception | |
| 18:48:19 | sean-k-mooney | this should be jsonutils.loads('"#cloud-config\nhostname: ocdvworks"') | |
| 18:49:10 | frct1 | Nope https://paste.openstack.org/show/bQYbqkIHPTeoDaJtuXge/ | |
| 18:49:51 | sean-k-mooney | its not happy with the \n | |
| 18:50:22 | frct1 | Uhm, with double slashes it works | |
| 18:50:23 | frct1 | >>> jsonutils.loads('"#cloud-config\\nhostname: ocdvworks"') '#cloud-config\nhostname: ocdvworks' | |
| 18:50:39 | sean-k-mooney | ok ya you need to secape it so that it actlly \n in the data | |
| 18:51:00 | sean-k-mooney | but thats because of how you are typing it | |
| 18:51:45 | sean-k-mooney | i have commented here https://github.com/canonical/cloud-init/issues/5221#issuecomment-2086560281 | |
| 18:53:19 | frct1 | So response would be like https://paste.openstack.org/show/byDvpUrAI61jMVAUMRfY/ ? | |
| 18:54:41 | frct1 | nah, ""#cloud-config\\nhostname: ocdvworks"" not works at all, raise exception, somehow it have to be escaped | |
| 18:54:44 | sean-k-mooney | i do not think it will render as that but rther the file will contain https://paste.openstack.org/show/bXFAMO5uOlegBy4KJmbP/ | |
| 18:56:49 | frct1 | If provider will return "#cloud-config\nhostname: ocdvworks" it will raise the same JSON exception, i would not dig in it if it would work | |
| 18:58:37 | sean-k-mooney | from where | |
| 18:58:39 | sean-k-mooney | https://paste.openstack.org/show/bcayBBbJaZjiagsjcVdu/ | |
| 18:58:55 | sean-k-mooney | im using a raw sting in that version to avoid any escaping | |
| 18:59:58 | sean-k-mooney | i guess you mean when this is used by nova | |
| 19:01:02 | frct1 | So somehow provider have to return escaped string, it is ? | |
| 19:01:52 | sean-k-mooney | the provider need to return a vaild json sring containing something that can be parsed by the client | |
| 19:02:28 | sean-k-mooney | cloud init is documeitnign that it shoudl have a srting with newlines excaped as \n | |
| 19:02:31 | sean-k-mooney | https://cloudinit.readthedocs.io/en/latest/reference/datasources/openstack.html#vendor-data | |
| 19:03:13 | sean-k-mooney | so the server litrally need to return "#cloud-config\npackage_upgrade: True\npackages:\n - htop" | |
| 19:03:21 | sean-k-mooney | including the " | |
| 19:04:26 | sean-k-mooney | my guess is internaly cloud-init is reading the content of that string directly adn parsing it in text mode | |
| 19:06:09 | sean-k-mooney | that or writign it to a file first and then reading it back | |
| 19:07:29 | sean-k-mooney | i think its doign this here ? https://github.com/canonical/cloud-init/blob/main/cloudinit/sources/DataSourceOpenStack.py#L193-L195 | |
| 19:08:21 | sean-k-mooney | so we are going to take this path https://github.com/canonical/cloud-init/blob/main/cloudinit/sources/__init__.py#L1154 | |
| 19:10:03 | sean-k-mooney | im not familar with this code but i think tis hten used here https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/stages.py#L562-L564 | |
| 19:11:08 | sean-k-mooney | which calls json_dumps on the string contaiing the cloud config | |
| 19:11:11 | sean-k-mooney | https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/stages.py#L604-L605 | |
| 19:11:45 | sean-k-mooney | and it finaly write it to a file | |
| 19:11:48 | sean-k-mooney | https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/stages.py#L590-L594 | |
| 19:12:14 | sean-k-mooney | which has mode wb https://github.com/canonical/cloud-init/blob/7eb5a1df0769fb88bbe3fc357ab44a378770043b/cloudinit/util.py#L2249 | |
| 19:12:37 | sean-k-mooney | so its doign a binary write of the raw data to a file | |
| 19:13:16 | sean-k-mooney | frct1: so the respocne form the backend woudl have to be a raw binary sring | |
| 19:13:31 | sean-k-mooney | contiing the cloud-init file content | |
| 19:14:14 | frct1 | Yep, i'm trying to implement it somehow | |
| 19:14:28 | frct1 | The main thing is to make vendor data provider return escaped data | |
| 19:28:41 | frct1 | Uhm, it works, lol https://paste.openstack.org/show/bShqKlZLG9hjmix2wCmc/ | |
| 19:29:55 | frct1 | https://paste.openstack.org/show/bV1JTGhzjKuRdsHGGS5B/, i will try to launch cloud-init shortly | |
| 19:33:24 | sean-k-mooney | so two thing. one your obviously in unsupproted terrirotry using implemenation specific knowldage | |
| 19:33:33 | sean-k-mooney | two if you get it work ing we have a few optiosn | |
| 19:33:48 | sean-k-mooney | first we could docuemnt and add test coverage for this hack | |
| 19:34:04 | sean-k-mooney | second we could still consider the new feature to do this properly | |
| 19:37:10 | frct1 | Actually i'm really inclined to add support for DynamicJSON provider's response typings like mentioned earlier "cloud-init:string@10.10.13.37:123,testing@10.10.13.37:124", so cloud-init key will be explicitly encoded as string, but testing will be presented as json | |
| 19:37:35 | frct1 | I will specs to provide details about observed hack to docs | |
| 19:37:45 | frct1 | I will add* | |
| 19:41:32 | frct1 | Wrapping everything up to update vendor data provider used with Consul and then move to compose docs update | |
| 19:42:55 | frct1 | What we have to add to test coverage for this case ? Something like check that string is probably valid escaped JSON string, i cant find any RFC that covers that | |