| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-04-30 | |||
| 16:42:03 | dansmith | what is the actual problem? | |
| 16:42:21 | sean-k-mooney | we should not need that | |
| 16:42:21 | gibi | sqlite does not support altering constarint on a table without recreate | |
| 16:42:31 | sean-k-mooney | although i know ironic does some sqlite specific tweaks | |
| 16:42:50 | dansmith | the create_table includes mysql-specific stuff too | |
| 16:42:51 | sean-k-mooney | gibi: as in adding a foringkey | |
| 16:43:45 | dansmith | is this because we merged the migration before the implementation and then decided that we needed something different? | |
| 16:43:47 | sean-k-mooney | so this is the share mapping table | |
| 16:43:51 | sean-k-mooney | which does not exist right | |
| 16:44:03 | sean-k-mooney | so why do we need this drop table and recreate | |
| 16:44:10 | sean-k-mooney | did that merge last cycle? | |
| 16:44:16 | gibi | dansmith: we needed an exta constraint to prevent creating two sharemapping with the same share id and instance id | |
| 16:44:28 | gibi | sean-k-mooney: we merged the db code | |
| 16:44:37 | dansmith | ugh, that's very unfortunate | |
| 16:44:41 | sean-k-mooney | ... ok this partly why i didint want to do that | |
| 16:44:46 | dansmith | yeah seriously | |
| 16:45:13 | dansmith | but, if it's not used currently, can we just drop and recreate it for all backends? should be very non-impactful for an empty mysql table I think | |
| 16:45:34 | sean-k-mooney | ya i think i prefer ^ | |
| 16:45:35 | gibi | dansmith: correct, the api code is not landed, so the table is empty | |
| 16:46:01 | dansmith | yeah, I'd prefer that, but.. also prefer not landing db code before implementation is ready for exactly this reason :( | |
| 16:46:35 | gibi | OK | |
| 16:46:52 | sean-k-mooney | we dont alwasy do this but my prerfered ordering is "driver -> object -> rpc ->db -> api -> docs" | |
| 16:47:28 | gibi | just a side note that in the other hand people are not happy with 30 patches in a series. | |
| 16:47:46 | gibi | driver uses object code I think | |
| 16:47:50 | gibi | object used db backing | |
| 16:48:22 | sean-k-mooney | it didnt need too but lets not rehash that | |
| 16:48:29 | gibi | anyhow thanks for the feedback, then this migration will be a drop / recreate for all backends | |
| 16:48:44 | gibi | Uggla_: ^^ | |
| 16:48:57 | opendevreview | Rajesh Tailor proposed openstack/nova master: Fix KeyError on assisted snapshot call https://review.opendev.org/c/openstack/nova/+/900783 | |
| 16:49:20 | gibi | I need to drop now. see you on thrusday | |
| 16:49:36 | sean-k-mooney | enjoy o/ | |
| 16:49:39 | Uggla_ | gibi, seen | |
| 16:49:57 | Uggla_ | I will update to drop / recreate the table | |
| 17:20:52 | frct1 | Hello, could anybody from nova team review change to nova-api ? https://review.opendev.org/c/openstack/nova/+/917276 this fix #cloud-config provide over vendor_data2.json which is broken for now. | |
| 17:25:56 | sean-k-mooney | frct1: so any change shoudl really have test coverage to validate it | |
| 17:26:16 | sean-k-mooney | also this seams like a new feature | |
| 17:26:30 | sean-k-mooney | adding cloud init supprot to dymic veder data | |
| 17:27:32 | frct1 | Not a new feature, since its support declared by cloud-init (https://cloudinit.readthedocs.io/en/latest/reference/datasources/openstack.html#vendor-data), there is proposal to fix https://blueprints.launchpad.net/nova/+spec/dynamicjson-vendordata-cloud-config | |
| 17:27:47 | sean-k-mooney | in json format | |
| 17:28:01 | frct1 | Current DynamicJSON implementation strongly limit developers to return JSON encoded response from provider | |
| 17:28:09 | sean-k-mooney | right | |
| 17:28:13 | sean-k-mooney | we dont supprot any other format | |
| 17:28:20 | sean-k-mooney | only json | |
| 17:28:27 | sean-k-mooney | so that is the new feature your addign | |
| 17:28:43 | sean-k-mooney | allowign the raw respocne to be passed | |
| 17:28:43 | frct1 | I have to implement another vendor data provider ? | |
| 17:29:02 | sean-k-mooney | which also changes the returnd formate form a dict to a string | |
| 17:29:50 | sean-k-mooney | so the vendor data feature only supprot talking to rest api that return a json responce | |
| 17:30:29 | bauzas | folks, I'll be off tomorrow, see you on Thur | |
| 17:31:59 | frct1 | What are ways to solve it ? New vendor data provider ? It is possible is over-engineering to implement new one because cloud-init is only thing that expects cloud-init key in vendor_data2.json | |
| 17:44:19 | sean-k-mooney | frct1: the first step woudl really be to create a repoducer test case for the issue | |
| 17:45:07 | sean-k-mooney | do you have an example fo a valid respocne that has a cloud-init key? | |
| 17:52:47 | frct1 | yep: https://paste.openstack.org/show/bNmVjYg2Fmq3wpxtny9y/ | |
| 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 | |