| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-19 | |||
| 16:11:57 | dansmith | mriedem: this is getting really old | |
| 16:12:04 | dansmith | can't you do something positive to help the project? | |
| 16:12:22 | mriedem | would you like auto-formatted code style? | |
| 16:12:33 | mriedem | it will bring the new contribs | |
| 16:12:57 | dansmith | yes please! | |
| 16:13:48 | dansmith | mriedem: I'm highly allergic to changing the host/node assignment timing | |
| 16:14:08 | dansmith | not because what we do today is good or right, but just because I'm afraid of breaking tons of other things that encode those assumptuons | |
| 16:14:11 | dansmith | *assumptions | |
| 16:14:51 | mriedem | dansmith: i.e. "I think this is probably not really an option because finish_resize has never done this on failure and we don't really know what state the instance is in" | |
| 16:15:09 | dansmith | sure | |
| 16:15:41 | mriedem | i don't think that's really a good option either, i just listed it for completeness | |
| 16:16:46 | mriedem | an auto-revert is appealing but i very much doubt the revert code is graceful enough to handle it, b/c of baked in assumptions about the state of the world when a revert is started | |
| 16:17:14 | mriedem | e.g. "cleaning up networking...wtf there is no networking on this host KABOOM!!!" | |
| 16:22:47 | fried_rice | mriedem: Did you already do the e-r thing for bug 1825435? | |
| 16:22:48 | openstack | bug 1825435 in OpenStack Compute (nova) "TestRPC unit tests intermittently fail with "'>' not supported between instances of 'NoneType' and 'datetime.datetime'" - maybe due to "Fatal Python error: Cannot recover from stack overflow."" [High,Confirmed] https://launchpad.net/bugs/1825435 | |
| 16:22:54 | mriedem | yeah | |
| 16:22:58 | fried_rice | k | |
| 16:23:33 | mriedem | http://status.openstack.org/elastic-recheck/#1825435 | |
| 16:28:04 | mriedem | dansmith: ok option 1 fix is quick and easy and already done locally | |
| 16:28:08 | mriedem | it's essentially just doing what confirm_resize does | |
| 16:28:12 | dansmith | cool | |
| 18:00:39 | eandersson | Sweet mriedem_lunch | |
| 18:01:02 | eandersson | fried_rice, We have this in Rocky as well. I don't think it ever worked. | |
| 18:01:58 | eandersson | https://github.com/openstack-dev/devstack/blob/master/lib/nova_plugins/hypervisor-ironic#L44 | |
| 18:02:13 | eandersson | We don't add a region_name, so that path was never really tested. | |
| 18:43:07 | fried_rice | eandersson: Okay. It's nearly impossible to test with all the permutations of ksa opts, times all the permutations of service catalog and endpoint setups. Inevitable that we missed things. | |
| 18:43:20 | eandersson | Yep | |
| 18:43:36 | eandersson | For sure. | |
| 18:43:47 | eandersson | I opened a PR to have it added. | |
| 18:44:12 | fried_rice | eandersson: IIUC this is only reproducible when there's a real ironicclient listening on the other end? | |
| 18:44:38 | eandersson | The problem is really that the real ironicclient hides this. | |
| 18:44:41 | fried_rice | right | |
| 18:44:50 | fried_rice | oh | |
| 18:44:57 | fried_rice | wait, you mean the *real* real ironicclient? | |
| 18:45:03 | eandersson | Since when you pass on a None value, the ironicclient tries to figure it out. | |
| 18:45:19 | eandersson | Yep | |
| 18:45:35 | fried_rice | so... where is it a problem? | |
| 18:46:09 | eandersson | Well the problem is that ironicclient is smart, but not smart enough | |
| 18:46:22 | eandersson | when you pass on a None endpoint it just gets the first endpoint in the catalog | |
| 18:47:15 | eandersson | and does not take region into account | |
| 18:48:47 | eandersson | There has been multiple bugs with this code pass, so been difficult to know exactly where things are failing | |
| 18:49:12 | eandersson | e.g. https://github.com/openstack/python-ironicclient/commit/466be3b6568b643605d826e5aa26d9a344cc74ae | |
| 18:49:40 | eandersson | *code path | |
| 18:50:48 | fried_rice | I don't actually see where we're taking region into account at all. | |
| 18:51:04 | eandersson | Yea - that was my first assumption. | |
| 18:51:11 | fried_rice | oh, there it is. | |
| 18:51:21 | eandersson | But it actually looks like it works. | |
| 18:51:50 | eandersson | I assumed that > get_endpoint(region_name=bla) | |
| 18:51:52 | fried_rice | eandersson: I'm still not understanding exactly what environment and setup it takes to make it not work, and also how it fails. | |
| 18:51:52 | eandersson | had to look like this | |
| 18:52:22 | eandersson | fried_rice, if you add region_name get_endpoint(..) in nova will always return None | |
| 18:52:23 | fried_rice | no, by the time we hit get_endpoint(), region_name has already been consumed from the conf. That happens on load_adapter_from_conf_options. | |
| 18:52:50 | eandersson | http://logs.openstack.org/39/653839/2/check/ironic-tempest-ipa-wholedisk-bios-agent_ipmitool-tinyipa/63a832a/controller/logs/screen-n-cpu.txt.gz#_Apr_19_03_21_25_243777 | |
| 18:53:12 | eandersson | I just adding if not endpoint into the Nova code here and ran the test with region_name=regionOne set | |
| 18:53:18 | eandersson | *added | |
| 18:53:52 | fried_rice | And your service catalog has your ironic endpoint in regionOne? | |
| 18:57:02 | eandersson | Yep | |
| 18:57:20 | eandersson | Once review.openstack is up again I'll link you the changes I did for testing | |
| 18:59:58 | fried_rice | Okay. I don't have a way to do ironic stuff locally. Is there a way to spin up the service with no nodes? | |
| 19:01:41 | eandersson | They have a noop driver you can probably use. | |
| 19:07:48 | eandersson | I was just doing something like this for testing http://paste.openstack.org/show/uANO7kLnU4NcTJrB2ywE/ | |
| 19:08:25 | eandersson | and ironic can be set up with a super basic config to provide api only | |
| 19:09:14 | eandersson | Since you don't actually need to create nodes to test this | |
| 19:09:42 | eandersson | http://logs.openstack.org/39/653839/2/check/ironic-tempest-ipa-wholedisk-bios-agent_ipmitool-tinyipa/63a832a/controller/logs/etc/ironic/ironic_conf.txt.gz | |
| 19:15:33 | fried_rice | eandersson: But you're not actually even getting to ironicclient by the time you hit the problem. | |
| 19:15:54 | eandersson | Sorry, that was for a different issue. | |
| 19:16:01 | fried_rice | I think you're loading the ksa auth, session, and adapter from conf... | |
| 19:16:08 | eandersson | Just an example. | |
| 19:16:09 | fried_rice | and then asking the adapter to get_endpoint()... | |
| 19:16:24 | fried_rice | which is going to the service catalog looking for a matching endpoint | |
| 19:16:48 | fried_rice | Either it doesn't find it there, or it finds it and then goes and queries that endpoint - the ironic API itself - to do the discovery. | |
| 19:16:56 | fried_rice | One of those two steps is failing to yield resulds. | |
| 19:16:58 | fried_rice | results. | |
| 19:20:51 | fried_rice | eandersson: You have an env where you're able to reproduce this, or just in zuul? | |
| 19:21:00 | eandersson | I do have an env yea | |
| 19:21:18 | eandersson | I did it locally first, but as the way I have it set up is very specific to our env I wanted to confirm in zuul as well. | |
| 19:21:41 | fried_rice | okay, can you throw some debug logs into... | |
| 19:21:46 | eandersson | Sure | |
| 19:22:43 | fried_rice | https://github.com/openstack/nova/blob/master/nova/virt/ironic/client_wrapper.py#L122-L133 | |
| 19:24:03 | fried_rice | Specifically, after L122 to see if that's reached; vs at L127 to see if get_endpoint raised an exception - should be one or the other - and then after L130 to see what get_client returns. | |
| 19:24:08 | fried_rice | eandersson: ^ | |
| 19:24:35 | eandersson | So that is what I added into the Zuul logs | |
| 19:24:52 | eandersson | ksa_adap.get_endpoint() returns None | |
| 19:25:03 | eandersson | ironic.client.get_client(...) returns the endpoint | |
| 19:25:03 | fried_rice | that's... pretty weird. | |
| 19:25:25 | eandersson | If I remove line 120-121 it get_endpoint() returns the endpoint | |
| 19:25:57 | fried_rice | What does the version document at your ironic endpoint say? | |
| 19:26:08 | fried_rice | like if you just curl the root endpoint | |
| 19:31:19 | eandersson | http://paste.openstack.org/show/eXVi99OZVrjJ6V9C496P/ | |
| 19:40:07 | fried_rice | eandersson: Oh. Ahem. That min_version/max_version thing is just wrong. No wonder it's borked. | |
| 19:40:26 | fried_rice | Those are supposed to be major version numbers, not microversions. | |
| 19:40:36 | fried_rice | yeah, those should just be removed entirely. | |
| 19:40:48 | fried_rice | ye gods, who *wrote* this crap?? | |
| 19:41:19 | fried_rice | eandersson: Do you have a bug for this? | |
| 19:56:42 | fried_rice | mriedem: I thought I could get rid of, "There is no script for 63 version" by rebuilding venv, wth? | |
| 19:58:39 | mriedem | is that an api db version? | |
| 19:59:08 | mriedem | do you have stale pycs or something? | |
| 20:00:02 | eandersson | no bug yet fried_rice | |
| 20:00:15 | fried_rice | eandersson: Okay, we're going to need one | |
| 20:00:24 | fried_rice | eandersson: Because this fix will want to be backported | |