| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-04-30 | |||
| 17:53:27 | sean-k-mooney | ok well that not a valid respocne the content of the data return by the service has to be a valid json object | |
| 17:54:53 | frct1 | Yes, but in this case cloud-init will ignore cloud-config if it is presented in nested json even if cloud-init key present. There is discrepancy between cloud-init and nova implementation, because cloud-init support to run cloud-config provided in vendor data (https://cloudinit.readthedocs.io/en/latest/reference/datasources/openstack.html#vendor-data) | |
| 17:54:54 | sean-k-mooney | it should be something like this https://paste.openstack.org/show/bua9OEI0FpYb24kTIjHj/ | |
| 17:55:24 | sean-k-mooney | frct1: right and nova does not supprot that so adding supprot would be a new feature | |
| 17:55:47 | frct1 | I've found that there is some issues reported to cloud-init, (https://github.com/canonical/cloud-init/issues/3433 and https://github.com/canonical/cloud-init/pull/777) that has been ignored | |
| 17:56:09 | sean-k-mooney | sure the cloud init doc you linked is incorrect | |
| 17:56:12 | sean-k-mooney | that is not supproted | |
| 17:56:52 | frct1 | I've tried to modify nova-api code to return plain response if service_name equals to "cloud-init" and it actually works | |
| 17:57:20 | sean-k-mooney | ok but that is not valid | |
| 17:58:05 | sean-k-mooney | we could enable that | |
| 17:58:20 | sean-k-mooney | but its not somethign that was orgianlly uspported when we added this feature | |
| 17:58:50 | frct1 | Exactly. That;s why i started to investigate on how to solve it. If it is will be implemented as new feature, should it have it's own configuration ? | |
| 17:59:18 | sean-k-mooney | is the comment in the respocne | |
| 17:59:22 | sean-k-mooney | https://paste.openstack.org/show/bNmVjYg2Fmq3wpxtny9y/ | |
| 18:00:07 | frct1 | No, there is no any comments in response, it was added as note | |
| 18:00:19 | sean-k-mooney | https://paste.openstack.org/show/bb6tjYB8z4Zc2xrcVBXy/ | |
| 18:00:32 | sean-k-mooney | ok this ^ would be valid json but the other poast is not | |
| 18:02:06 | sean-k-mooney | so the expecation is that content would be put into a file called somethign like openstack/2016-10-06/vendor_data2.json | |
| 18:03:22 | frct1 | Yes, Or openstack/latest/vendor_data2.json, not sure what path cloud-init looks for, but it checks that `cloud-init` key is presented and is string type and then process it | |
| 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 | |