| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-15 | |||
| 20:18:08 | mriedem | efried: it's probably easiest to just look at that ironic patch | |
| 20:18:11 | mriedem | for get_inventory | |
| 20:18:22 | jaypipes | efried: basically just needs to return a dict of inventory information for each resource class the driver understands. | |
| 20:19:09 | efried | mriedem jaypipes Okay, that's where I was going to go. Guess you guys will be reviewing the change one way or another, so you'll catch me if I eff it up... | |
| 20:19:31 | mriedem | if you eff it up the resource tracker will probably give you a KeyError | |
| 20:19:46 | mriedem | it's all self testing, like most everything in nova, rock solid | |
| 20:19:53 | efried | But of course. | |
| 20:25:11 | mriedem | wtf, why is RequestSpec.forced_hosts a list when you can only provide one value in the API when creating a server? | |
| 20:25:14 | mriedem | bauzas: ^? | |
| 20:25:38 | mriedem | please oh please don't say future proofing | |
| 20:28:48 | bauzas | mriedem: because you can provide a list AFAIK | |
| 20:28:55 | bauzas | lemme verify this | |
| 20:29:28 | melwitt | yeah, from what I find I think the scheduler hint is force_hosts as a list? | |
| 20:29:30 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L579 is not a list | |
| 20:29:42 | mriedem | it's a scheduler hint? | |
| 20:30:21 | bauzas | okay, can't find a reasonable answer | |
| 20:30:22 | melwitt | I'll wait for bauzas to let us know what's the deal | |
| 20:31:04 | mriedem | it's not a scheduler hint that's checked in any in-tree filters | |
| 20:31:13 | melwitt | it seems like it coincides with the scheduler hint, but why it's baked into RequestSpec, I don't know the history on that | |
| 20:31:26 | bauzas | mriedem: melwitt: no, forced_hosts is in the host manager | |
| 20:31:28 | mriedem | the scheduler hint for groups? | |
| 20:31:40 | melwitt | yeah, forcing a host isn't part of any specific filter. it happens before filtering and will bypass filtering | |
| 20:31:55 | bauzas | yeah that | |
| 20:32:04 | mriedem | ok but there is also this https://docs.openstack.org/admin-guide/cli-nova-specify-host.html | |
| 20:32:32 | bauzas | mriedem: yeah, because it's a terrible UX | |
| 20:32:43 | mriedem | i know it's not part of a filter, but i'm trying to figure out how it becomes a list, | |
| 20:32:46 | bauzas | forced_hosts is set by the AZ flag | |
| 20:33:02 | mriedem | because the only thing i see that sets forced_hosts in the request spec comes through the REST API using the az parsing in ^ | |
| 20:33:02 | bauzas | mriedem: honestly, I don't remember why, lemme verify the change | |
| 20:33:39 | mriedem | those docs are also a bit wrong btw, they say "HOST is an optional parameter. In such cases, use the --availability-zone ZONE::NODE." - but they fail to mention that NODE is also an optional parameter | |
| 20:34:19 | bauzas | node was added later | |
| 20:34:26 | bauzas | because of $ironic :) | |
| 20:34:30 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add check for invalid inventory amounts https://review.openstack.org/428800 | |
| 20:34:31 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add check for invalid allocation amounts https://review.openstack.org/416752 | |
| 20:34:35 | edleafe | jaypipes: mriedem: updated ^^ | |
| 20:35:21 | jaypipes | cool | |
| 20:37:19 | bauzas | mriedem: so, in between PS7 and PS8, I modified force_hosts to be a list | |
| 20:37:20 | bauzas | https://review.openstack.org/#/c/145528/8..9/nova/objects/request_spec.py | |
| 20:37:39 | bauzas | I'm trying to remember the context why | |
| 20:37:47 | mriedem | 8..9, butok | |
| 20:38:40 | bauzas | mmm | |
| 20:38:55 | mriedem | it's 46 patch sets is hurting my browser | |
| 20:39:44 | bauzas | so, the only reason I see that is because filter_props was having a list | |
| 20:40:12 | bauzas | it's not what I say a reasonable answer, so lemme verify why we have a list for the legacy dict | |
| 20:40:35 | mriedem | i smell a rackspace reason here | |
| 20:41:26 | bauzas | at least the host manager waits for a list | |
| 20:41:32 | bauzas | and I haven't changed that | |
| 20:41:42 | bauzas | so somewhere we accepted a list | |
| 20:42:11 | bauzas | yeah... build_filter_properties() in nova.scheduler.utils | |
| 20:42:14 | bauzas | mriedem: ^ | |
| 20:43:00 | mriedem | https://github.com/openstack/nova/commit/90620a27e0831345864f574256b30bac1ced5843 | |
| 20:43:03 | bauzas | mriedem: https://github.com/openstack/nova/commit/9998c51f | |
| 20:43:47 | mriedem | https://github.com/openstack/nova/commit/90620a27e0831345864f574256b30bac1ced5843 goes back to havana, | |
| 20:43:52 | bauzas | mriedem: sure, but both don't explain why we want a list | |
| 20:43:55 | mriedem | and appears to introduce the idea that forced_hosts is a list | |
| 20:44:18 | mriedem | could have been a mistake if the key was named "force_hosts" so plural | |
| 20:44:53 | mriedem | https://github.com/openstack/nova/blob/90620a27e0831345864f574256b30bac1ced5843/nova/compute/api.py#L727 | |
| 20:44:55 | bauzas | we should go back where force_hosts was originally created | |
| 20:45:04 | mriedem | i'm going | |
| 20:45:31 | bauzas | yeah, like I said there are a shit ton of places where we expect a list | |
| 20:45:39 | bauzas | so I shamelessly copied that | |
| 20:46:05 | bauzas | although I originally found it was a string like I wrote in my PS8 | |
| 20:46:10 | bauzas | defeated by tech debt | |
| 20:47:41 | mriedem | https://github.com/openstack/nova/commit/7b34a2d767b60e053ac1f49d1cdd8c853b8d9133 | |
| 20:47:46 | mriedem | is about as far back as i see | |
| 20:48:27 | mriedem | oh wait it goes further | |
| 20:48:27 | bauzas | https://github.com/openstack/nova/commit/836ee3f45 | |
| 20:49:22 | jroll | don't you guys know to always blame comstud by now | |
| 20:51:00 | mriedem | https://github.com/openstack/nova/commit/16ea348a1623f055809d0d9b7fe9f046515b5dd1 | |
| 20:51:01 | melwitt | I was seeing some stuff from essex | |
| 20:51:54 | mriedem | ^ is essex | |
| 20:52:20 | bauzas | yeah I was an diablo op and I don't remember that feature | |
| 20:52:21 | melwitt | hah, nice | |
| 20:52:41 | bauzas | I just remember Essex added lots of things like that | |
| 20:52:54 | bauzas | but it's very old stories at the corner of the fireplace | |
| 20:53:41 | bauzas | soooo... to answer mriedem's question, I guess that the terrible answer he was afraid was the correct one | |
| 20:53:56 | bauzas | ie. "we planned that for the future" | |
| 20:54:31 | bauzas | just give me a license to kill that AZ hack, and I'll write a specx | |
| 20:54:45 | melwitt | it was maybe mirroring ignore_hosts, which was already there when force_hosts was added | |
| 20:55:00 | bauzas | yeah | |
| 20:55:20 | bauzas | anyway, force_hosts is a legacy from the past, and we should get rid of it | |
| 20:55:37 | mriedem | i'm not necessarily saying let's remove it, | |
| 20:55:43 | bauzas | I'm saying it :p | |
| 20:55:55 | mriedem | but it sure as shit causes a lot of confusion when you're looking at all of the code that uses it, and the API that provides the value, and you see it's not a list | |
| 20:56:00 | mriedem | and you say, wtf am i missing? | |
| 20:56:20 | bauzas | mriedem: you know that the policy differs for that, right? | |
| 20:56:25 | mriedem | yes | |
| 20:56:37 | melwitt | would have to run that by operators because afaik they use that to test new compute host before making it available for scheduling? | |
| 20:56:38 | mriedem | you mean "create:forced_host" right? | |
| 20:56:45 | bauzas | yup | |
| 20:56:54 | mriedem | yeah i thought operators were pretty keen on this, | |
| 20:56:59 | bauzas | I was | |
| 20:57:03 | mriedem | and we've baked it into the migration APIs right? | |
| 20:57:11 | bauzas | in Essex, it was terribly necessary | |
| 20:57:40 | bauzas | but now, we are in a far better place, but at least we should mimic what we merged for the move operations | |
| 20:57:53 | bauzas | ie. having a possible target with a force flag | |
| 20:58:24 | bauzas | which would then verify the filters if the target is set without force | |
| 20:58:34 | bauzas | or totally bypass the scheduler if force | |
| 20:58:55 | mriedem | so, i'm going to just start with a code comment in the request spec object | |
| 20:59:04 | mriedem | so when i look at this in a year, i won't have to do all of the detective work again | |