| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-19 | |||
| 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 | fried_rice | that's... pretty weird. | |
| 19:25:03 | eandersson | ironic.client.get_client(...) returns the endpoint | |
| 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 | |
| 20:00:36 | fried_rice | mriedem: Must be. | |
| 20:00:49 | fried_rice | mriedem: seems to have cleared up now | |
| 20:01:25 | eandersson | I'll create one in a bit | |
| 20:01:59 | fried_rice | thanks | |
| 20:02:35 | fried_rice | mriedem: I was on the second of those patches, 'Soft delete virtual_interfaces when instance is destroyed' -- test passes when fix is reverted. | |
| 20:03:17 | mriedem | ok i'll look in a bit | |
| 20:12:11 | fried_rice | eandersson: So this https://github.com/openstack-dev/devstack/blob/master/lib/nova_plugins/hypervisor-ironic#L44 isn't setting api_endpoint. Which means it still ought to be going through the code path that does get_ksa_adapter with the bogus min_version and max_version kwargs. Which should still be returning None. Which should still be attempting to call get_client with endpoint=None. So I don't get how adding region_name to | |
| 20:13:01 | fried_rice | because region_name isn't in the kwargs sent to get_client | |
| 20:15:31 | fried_rice | get_endpoint with min/max but no region in the conf somehow magically works, but adding region_name in the conf makes it not work until you remove the min/max version?? /me confused. | |
| 20:28:05 | eandersson | Yea - not sure why it works without the region_name | |
| 20:30:17 | mriedem | fried_rice: test_delete_virtual_interfaces_on_instance_destroy fails for me locally if i remove the fix | |
| 20:42:12 | eandersson | I wrote a bug report but... launchpad error'd out :'( | |
| 20:45:21 | fried_rice | eandersson: Yeah, I won't be able to post the fix until all the things come back anyway. | |
| 20:45:30 | fried_rice | eandersson: One other data point that would be interesting: | |
| 20:46:13 | fried_rice | The failure setup, i.e. with region_name and min_version and max_version, but change min_version from IRONIC_API_VERSION to the tuple (1, 0) | |
| 20:49:41 | eandersson | Yea that seems to work | |
| 20:50:44 | fried_rice | eandersson: Okay, good, theory confirmed. Soon as the dark, dark night is over, we can have this fix pushed. | |
| 20:51:08 | fried_rice | though it's still a mystery why it doesn't break until you specify region_name. mordred, you around or chasing bunnies? | |
| 20:51:48 | mordred | chasing finishing this opendev rollout | |
| 20:52:01 | fried_rice | oh, you're involved in that? I'm sorry. | |
| 20:52:04 | fried_rice | Please do carry on. | |
| 20:52:15 | mordred | sorry - I saw the note from eandersson earlier -just havne't gotten to it. | |
| 20:52:19 | mordred | we're having all the fun | |
| 20:53:01 | fried_rice | yeah, I don't want to distract you from that for sure. | |
| 20:53:05 | eandersson | btw fried_rice would be nice if we could error out if endpoint is None | |
| 20:53:30 | fried_rice | eandersson: Well, if you look at the comment there, I deliberately didn't do that | |
| 20:53:32 | eandersson | because at the moment if this gets fixed, and nova is misconfigured it will look like it's working | |
| 20:53:52 | eandersson | Yea - saw your comment on that | |
| 20:53:59 | eandersson | At the very least I believe we should log it | |
| 20:54:04 | fried_rice | I was basically paranoid about changing the nature of legacy broken behavior. | |
| 20:54:09 | fried_rice | Yeah, I can get behind a log for sure. | |
| 20:54:11 | eandersson | because it can lead to some bad stuff | |
| 20:54:30 | eandersson | or maybe we can pass on region to the ironic client | |
| 20:55:03 | eandersson | because we are essentially telling the ironic client, hey if I fail, just make the best judgment you can.... but I will not give you all the info you need | |
| 20:55:06 | fried_rice | eandersson: Well, I would rather rip out ironicclient altogether and use the sdk. | |
| 20:55:29 | eandersson | I do like that idea :p | |
| 20:56:07 | fried_rice | eandersson: https://blueprints.launchpad.net/nova/+spec/openstacksdk-in-nova | |
| 20:56:19 | fried_rice | (I wasn't being entirely hypothetical) | |
| 20:56:33 | eandersson | Yep - already commented on the very initial implementation :P | |
| 20:56:59 | fried_rice | cool. I would have known that if I could have referenced the gerrit part of my extended brain. | |
| 21:00:07 | eandersson | https://github.com/openstack/python-ironicclient/blob/master/ironicclient/client.py#L111 | |
| 21:00:32 | eandersson | I still think we should pass on region_name to the client if we keep the current behavior with a possible endpoint = None | |
| 21:01:13 | eandersson | That way at the least the ironicclient can make an educated decision | |
| 21:02:50 | fried_rice | That can be done. I think I would prefer it to be a separate change though. It's clearly a different bug from the bogus min_version thing. | |
| 21:02:57 | eandersson | For sure | |
| 21:03:48 | fried_rice | ugh, this is such a mess | |
| 21:04:35 | fried_rice | ironicclient is doing all this preliminary stuff that's almost (but not quite, and incorrectly) duplicating a subset of the logic in ksa itself. | |
| 21:05:03 | eandersson | Yea - this took me hours to figure out | |
| 21:05:12 | eandersson | Because there was actually 3 separate bugs | |
| 21:05:17 | fried_rice | and for that matter, nova's client_wrapper is *also* doing more of that | |
| 21:05:24 | fried_rice | mainly for backward compatibility | |
| 21:07:55 | eandersson | https://github.com/openstack/python-ironicclient/commit/466be3b6568b643605d826e5aa26d9a344cc74ae | |
| 21:08:01 | eandersson | This bug added A LOT of confusion for me | |
| 21:08:06 | fried_rice | mriedem: Confirmed the test fails when the fix is removed. Something bizarre happening with cached .pyc files maybe, would explain a couple of the weird things that have befallen me today. So I'll be +A on that guy when the world comes back up. | |
| 21:08:44 | fried_rice | eandersson: Oh yeah, that one had a knock-on effect resulting in several other bugs. | |
| 21:09:13 | melwitt | mriedem: re: tripleo, I think it supports neither deploy nor upgrade with extracted placement based on the last I heard from lyarwood and EmelienM. this isn't going to work right now but the relevant patches are https://review.openstack.org/#/c/630644/ and https://review.openstack.org/#/q/topic:tripleo-placement-extraction+status:open and https://review.openstack.org/#/q/topic:tripleo-placement-upgrade-from-nova-placement+status:open | |