Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-04-30
16:41:33 dansmith and it's a table drop/recreate? that seems incredibly unfortunate to me
16:41:42 gibi it is for sqlite
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" ?

Earlier   Later