| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-26 | |||
| 15:41:32 | cdent | yes | |
| 15:42:04 | sean-k-mooney | is there a list of what is out standing beyond that or is that the final feature | |
| 15:42:14 | mriedem | https://etherpad.openstack.org/p/BER-placement-extract | |
| 15:42:22 | sean-k-mooney | mriedem: cool thanks | |
| 15:43:06 | mriedem | bauzas: did you get a functional test written that does a reshape and then schedules to the child provider inventory resource? | |
| 15:44:32 | mriedem | cdent: why were these tests removed? https://review.openstack.org/#/c/617941/21/nova/tests/unit/cmd/test_status.py | |
| 15:44:35 | mnaser | https://review.openstack.org/#/c/619349/ simple backport if anyone has a second (i'll babysit the rest) | |
| 15:46:12 | mriedem | lyarwood: ^ just consider my backport a proxy +2 there | |
| 15:46:31 | cdent | mriedem: because the tests uses the rp_objects directly | |
| 15:46:48 | cdent | the follow on patch may even remove the status command since it can no longer work | |
| 15:47:08 | cdent | there's a fixme added in cmd/status.py | |
| 15:47:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Send RP uuid in the port binding https://review.openstack.org/569459 | |
| 15:47:10 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test boot with more ports with bandwidth request https://review.openstack.org/573317 | |
| 15:47:23 | mriedem | cdent: ok but the rp objects aren't removed in that patch, so it seemed out of place | |
| 15:47:30 | mriedem | we could count rps using the placement api right? | |
| 15:47:33 | mriedem | or using the fixture | |
| 15:48:37 | cdent | I think that one can still work, but not the inventory-related one | |
| 15:48:54 | cdent | I went through so many iterations on that series of changes, I may have lost my place | |
| 15:49:12 | cdent | I assumed then (and still do) that we'll need to do some "dynamic tidying" | |
| 15:49:48 | mriedem | on that size of change i can see why something would get lost | |
| 15:49:58 | mriedem | it just stuck out to me looking at it fresh | |
| 15:52:08 | cdent | my original plan was to not remove anything from nova in the first change, and just change the tests, but it proved to confusing why debugging. | |
| 15:52:10 | mriedem | anyway, i think the command could count compute resource providers by providers with VCPU inventory as it does today | |
| 15:52:16 | mriedem | and i know you have a spec for filtering that way as well | |
| 15:52:39 | cdent | I abandoned that spec because people weren't sure that was a sufficient use case, which confused me | |
| 15:52:55 | mriedem | heh, well we have a use case right here | |
| 15:53:41 | mriedem | nova-status upgrade check already hits the placement REST API to check the version in _check_placement | |
| 15:53:45 | mriedem | anywhere | |
| 15:53:47 | mriedem | *anyway | |
| 15:54:01 | mriedem | we might want to undo https://review.openstack.org/#/c/617941/21/nova/tests/unit/cmd/test_status.py in a follow up | |
| 15:54:20 | mriedem | unless we remove that upgrade check, but that's another discussion | |
| 15:54:55 | cdent | well, we'll have to at least change the upgrade check, in which case the test will have to change so it does rather go together | |
| 15:55:11 | mriedem | sure, the test has to exist to change it though | |
| 15:55:14 | mriedem | unless you're agreeing with me | |
| 15:55:39 | cdent | I'm mostly agreeing with you, except for the part about it being another discussion | |
| 15:55:49 | cdent | if we're going to remove the command, then job done | |
| 15:56:24 | mriedem | it depends on what nova-compute does on startup - if it can't create a resource provider b/c placement isn't running or nova-compute isn't configured to talk to it, then we can probably remove the check | |
| 15:56:44 | mriedem | otherwise it's useful as a base install verification that you've got computes reporting in and resource providers for those computes | |
| 15:57:46 | mriedem | FFU makes me nervous about when we can remove these things... | |
| 15:57:57 | mriedem | b/c anyone can FFU from any release to another presumably | |
| 15:58:32 | mriedem | having said that, i'm not sure these would work for FFU'ers anyway b/c the placement api would likely be down | |
| 15:58:53 | mriedem | dansmith: is that correct? we can expect placement-api to be down during an FFU? | |
| 16:00:24 | sean-k-mooney | cdent: placement does not randomise allocation candiates by defualt correct. they are just retruned in db order? | |
| 16:00:48 | mriedem | https://docs.openstack.org/nova/latest/configuration/config.html#placement.randomize_allocation_candidates | |
| 16:02:53 | sean-k-mooney | mriedem: thanks ya its off by default. i was wondering if that could be related to this ml list post http://lists.openstack.org/pipermail/openstack-discuss/2018-November/000209.html | |
| 16:03:17 | sean-k-mooney | that said i woudl have expected the default weigher to kick in and spread the instances | |
| 16:04:08 | cdent | mriedem: yeah, I'm wondering if coupling nova-status to placement status is a good/safe idea? If they are supposed to be independently upgraded (for some value of "independent" maybe placement-status should do some kind of check? Except that we expect placement to upgrade first (usually). /me throws hands | |
| 16:05:02 | cdent | sean-k-mooney: that config item was left as not random by default so as to encoureage/allow pack, what would the weigher be doing? | |
| 16:05:15 | cdent | changing the config is certainly something they could at least try | |
| 16:05:32 | sean-k-mooney | cdent: the weighers used to spread by default | |
| 16:05:33 | cdent | my impression was that most of the system was biased towards packing to save $$ | |
| 16:05:46 | sean-k-mooney | cdent: i think they still do | |
| 16:06:46 | cdent | probably still worth trying the config setting just to see? | |
| 16:06:51 | mriedem | cdent: i left a note to self / open question in the check code so i can maybe come back to it some other time when it comes up, or is a more pressing decision that needs to be made | |
| 16:07:09 | sean-k-mooney | ya i was also gong to ask them to provide the nova.conf the schduler is useing not the compute node one | |
| 16:07:23 | cdent | good point sean-k-mooney | |
| 16:07:27 | cdent | mriedem: ✔ | |
| 16:07:55 | dansmith | mriedem: not just expect, but require | |
| 16:09:59 | mriedem | ok yeah then in that case, nova-status upgrade check will not work during FFU today | |
| 16:10:14 | mriedem | it will fail the check placement check since the API won't be up | |
| 16:10:48 | dansmith | I thought nova-status was to be db-only anyway? I guess not once placement is a different db, eh? | |
| 16:11:01 | mriedem | there is a check for the minimum placement version | |
| 16:11:10 | mriedem | otherwise it looks in the nova_api db yeah | |
| 16:11:31 | dansmith | so just have to make the api-bound check graceful I suppose | |
| 16:11:34 | mriedem | was discussing if we should change that to hit the placement API now, or just remove the placement-related checks | |
| 16:12:04 | mriedem | the upgrade checkers things were written before FFU was really a consideration i think | |
| 16:12:09 | mriedem | so i haven't put a ton of thought into it | |
| 16:12:46 | dansmith | well, I thought the plan was for that thing to be db-only anyway, so it wouldn't matter, but the scope has definitely gotten larger as we find more things for it to do | |
| 16:12:47 | mriedem | e.g. if we start saying that nova requires cinder >= 3.44 to remove our api compat code, presumably we'd want an upgrade check for that | |
| 16:12:56 | mriedem | yeah | |
| 16:13:26 | dansmith | well, if the api is up and you're not doing an ffu, it'd be nice to get that check, you just need to be super graceful I think | |
| 16:13:45 | dansmith | maybe a new category, peer to warning, error, for "manual check" or "unknown" ? | |
| 16:13:57 | dansmith | like "I would check this for you, but I can't so be sure you check it" | |
| 16:13:58 | mriedem | i was thinking it'd just be a warning | |
| 16:14:04 | mriedem | warning is already "this might be a problem, but i'm not sure" | |
| 16:14:13 | dansmith | do the deployment things hork on warning? | |
| 16:14:15 | dansmith | if not, then cool | |
| 16:14:31 | mriedem | do the deployment things run the upgrade checkers... heh | |
| 16:14:33 | mriedem | osa runs it | |
| 16:14:36 | mriedem | i don't think tripleo does | |
| 16:14:45 | dansmith | owalsh_: ? | |
| 16:14:54 | mriedem | btw, i ask this at least once every 2 weeks in here :) | |
| 16:15:08 | dansmith | you said you didn't know :) | |
| 16:15:21 | mriedem | rhetorical | |
| 16:15:28 | mriedem | i know tripleo doesn't run it | |
| 16:15:32 | dansmith | okay | |
| 16:15:53 | mriedem | http://codesearch.openstack.org/?q=nova-status&i=nope&files=&repos= | |
| 16:15:59 | mriedem | grenade, kolla-ansible and osa | |
| 16:16:27 | mriedem | so i was going to say, | |
| 16:16:48 | mriedem | unless/until someone comes along saying, "hey this doesn't work for me during FFU" i don't have much motivation to care about making it graceful in those cases | |
| 16:17:04 | dansmith | :/ | |
| 16:17:19 | dansmith | does it stack trace now? | |
| 16:17:28 | mriedem | if placement is down? | |
| 16:17:31 | mriedem | it will return a failure | |
| 16:17:33 | dansmith | yeah | |
| 16:17:43 | dansmith | oh you mean it just reports error instead of your proposed warning? | |
| 16:17:50 | mriedem | https://github.com/openstack/nova/blob/master/nova/cmd/status.py#L230 | |
| 16:17:52 | mriedem | yes | |
| 16:18:15 | mriedem | there is the guantlet of ksa exceptions, | |
| 16:18:22 | mriedem | so without testing it in devstack i'm not exactly sure | |