| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-10 | |||
| 15:28:53 | dansmith | with the workaround flag you can disable the check right? | |
| 15:29:09 | mriedem | yes | |
| 15:29:54 | dansmith | although I guess that doesn't really do much, | |
| 15:29:58 | gibi | mriedem: do I understand correclty that on master server groups only work if the late check using upcall is enabled? | |
| 15:30:00 | dansmith | obviously if you disable that check it's not going to check | |
| 15:30:02 | mriedem | maybe that type of request wouldn't get past scheduling | |
| 15:30:03 | openstackgerrit | Matthew Booth proposed openstack/nova master: Remove irrelevant comment https://review.openstack.org/578821 | |
| 15:30:04 | openstackgerrit | Matthew Booth proposed openstack/nova master: Move static _get_power_off_values to compute_utils https://review.openstack.org/578822 | |
| 15:30:30 | mriedem | gibi: i don't think so... | |
| 15:30:34 | mriedem | tempest has an anti-affinity test | |
| 15:30:50 | gibi | mriedem: that is the test I changed to show that master is broken | |
| 15:30:50 | dansmith | mriedem: so, scheduler will look at group.members, get the instance.host for each, and affine to those hosts, right? | |
| 15:31:02 | mriedem | gibi: test_create_server_with_scheduler_hint_group_anti_affinity | |
| 15:31:03 | dansmith | which is lossy because those instances might be pre-scheduling | |
| 15:31:07 | dansmith | or rather, pre-build | |
| 15:31:24 | openstackgerrit | Chen proposed openstack/nova master: WIP https://review.openstack.org/581403 | |
| 15:32:06 | gibi | mriedem: that tempest test only works because it sends two server create in the same API request | |
| 15:32:17 | mriedem | right, because servers in separate requests is a known race | |
| 15:32:34 | gibi | mriedem: if you wait 10 minutes between the two requests it still doesnt work | |
| 15:32:37 | gibi | mriedem: I tried :) | |
| 15:32:46 | dansmith | gibi: really? why is that? | |
| 15:33:19 | dansmith | the filter is using a potentially stale version of server group, from the beginning of the request, I just noticed | |
| 15:33:22 | mriedem | yeah the HostState.instances dict should be populated per request | |
| 15:33:29 | dansmith | that shouldn't be a problem 10 minutes later though | |
| 15:33:36 | gibi | dansmith: this returns an empty list for the second scheduling https://github.com/openstack/nova/blob/c0350da4a1607d7aa113caceaefb5d29303c7eed/nova/objects/instance_group.py#L422 | |
| 15:33:51 | mdbooth | Just working on this patch: https://review.openstack.org/#/c/578846/ I called it out in the commit message: the cut/paste in there is horrible, but avoids a refactor. Am I going to get that landed as is, or do I need to look at refactoring the driver interface? | |
| 15:34:01 | dansmith | gibi: oh on multi-cell? | |
| 15:34:01 | mriedem | gibi: and i think that's the bug melwitt is fixing | |
| 15:34:05 | dansmith | that needs to stripe | |
| 15:34:06 | dansmith | yeah | |
| 15:34:24 | gibi | don't we deploy with multi cell _by default_ ? | |
| 15:34:39 | mdbooth | It goes without saying that I don't mind refactoring :) However, I'd also like to both land and backport the code. | |
| 15:34:55 | mriedem | gibi: in devstack yes | |
| 15:34:58 | dansmith | gibi: yeah, this would hit cell0 if your default is cell0, and always be empty | |
| 15:35:14 | mriedem | it's not really 'multi cell' so much as superconductor mode in devstack and how the services are configured | |
| 15:35:30 | gibi | let me rephrase. I our suggested deployment mode (cell_v2) the server group policies are broken | |
| 15:35:41 | gibi | that is my understanding | |
| 15:35:50 | dansmith | gibi: because your default connection is to cell0, which will never return the list of instances, yeah | |
| 15:36:03 | gibi | dansmith: thanks, that explains what I see | |
| 15:36:13 | dansmith | but striping across cells is what needs to happen, which is that bug afaik | |
| 15:36:48 | gibi | dansmith: yeah. this is why offered my help to melwitt above on the fix | |
| 15:36:59 | dansmith | gibi: are we in violent agreement? :) | |
| 15:37:06 | gibi | dansmith: yes | |
| 15:37:48 | gibi | I was not sure we are on the same page about the size of the problem. server groups are useless now | |
| 15:38:17 | dansmith | gibi: if your controllers are pointing at your main cell by default it'll be fine, fwiw | |
| 15:38:24 | dansmith | but it definitely needs changing | |
| 15:38:51 | mriedem | gibi: so, if you want to help, we need a functional test in-tree that recreates the regression | |
| 15:39:03 | mriedem | that's what i was -1 on the change for - the mock based unit tests had logic flaws in them | |
| 15:39:54 | gibi | dansmith: how can a deployer change the default cell to cell1? | |
| 15:40:08 | gibi | mriedem: ack. I offered a tempest test :) | |
| 15:40:10 | dansmith | gibi: [database]/connection :) | |
| 15:41:05 | gibi | dansmith: in the super conductor conductor conf? | |
| 15:41:17 | dansmith | gibi: in any controller config | |
| 15:41:33 | dansmith | gibi: you don't have to be running a superconductor layout with only one non-cell0 cell | |
| 15:41:58 | mriedem | gibi: yeah, but we can't use that tempest change long-term because of the otherwise known race issue | |
| 15:42:28 | gibi | dansmith: I see | |
| 15:42:32 | gibi | dansmith: thanks | |
| 15:43:18 | gibi | mriedem: is it racy even if the test waits for the first server to go to ACTIVE state before start creating the second one? | |
| 15:43:49 | mriedem | gibi: in that case maybe not | |
| 15:43:59 | mriedem | because per scheduling request, we should pull the set of instances per host | |
| 15:44:28 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L781 | |
| 15:44:49 | mriedem | that's if the compute isn't sending it's instance info to the scheduler (which it's not in default devstack b/c we disable that ability) | |
| 15:45:38 | gibi | OK, so the second scheduling will see the first instance sitting on the host | |
| 15:45:41 | mriedem | which reminds me of https://review.openstack.org/#/c/569247/ but haven't had good large scale performance testing to see if that's justified | |
| 15:45:47 | mriedem | gibi: it should yeah | |
| 15:46:09 | gibi | mriedem: then I don't see how the currently proposed tempest test be racy | |
| 15:47:28 | openstackgerrit | Jay Pipes proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139 | |
| 15:49:03 | mriedem | gibi: ok updated comments | |
| 15:50:15 | gibi | mriedem: ack, I can cover both single and multi request cases in tempest | |
| 15:50:49 | gibi | mriedem: if we have the tempest do you still require the functional test in melwitt's patch? | |
| 15:51:53 | mriedem | maybe not | |
| 15:52:06 | mriedem | does your tempest test also cover affinity? | |
| 15:52:42 | mriedem | looks like it doesn't for separate requests | |
| 15:52:44 | gibi | mriedem: both the affinity and the anti-affinity case fails now | |
| 15:53:00 | gibi | mriedem: both uses the same helper to create the servers | |
| 15:53:50 | gibi | mriedem: but in case of affinity we might want to fill the compute to see NoValid host | |
| 15:54:04 | mriedem | that's what i was going to try with a functional test | |
| 15:54:19 | mriedem | to set available vcpu inventory to 1 or something | |
| 15:54:34 | gibi | mriedem: sure we have plenty of such functional tests already but they are not cell aware and passing | |
| 15:55:08 | gibi | mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py | |
| 15:55:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Document differences and similaries between extra specs and hints https://review.openstack.org/581410 | |
| 15:55:34 | mriedem | right, i plan on writing a multi-cell affinity regression functional test | |
| 15:56:18 | gibi | mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py#L292 | |
| 15:57:27 | gibi | mriedem: ohh there is one case where functional is needed. multiple non-cell0 cells case | |
| 15:57:43 | gibi | mriedem: as in tempest we have only cell0 and cell1 | |
| 15:58:02 | mriedem | yes, that's what i've been saying :) | |
| 15:58:14 | mriedem | when i say 'multi-cell' i mean multiple non-cell0 cells | |
| 15:58:15 | gibi | mriedem: now I understand ;) | |
| 15:58:28 | mriedem | i don't consider cell0 a 'real' cell since it doesn't contain compute hosts | |
| 16:00:59 | gibi | mriedem: OK, I will extend the tempest test tomorrow. Thanks for the explanation | |
| 16:05:41 | openstackgerrit | Eric Fried proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139 | |
| 16:06:28 | openstackgerrit | karim proposed openstack/nova master: Handle rebuild of instances with image traits https://review.openstack.org/569498 | |
| 16:13:50 | openstack | Launchpad bug 1739482 in Cinder "test_snapshot_backup fails to build backup due to MessagingTimeout" [Medium,Confirmed] | |
| 16:13:50 | mriedem | dansmith: i found another candidate for your long_rpc_timeout https://bugs.launchpad.net/cinder/+bug/1739482 | |
| 16:14:13 | dansmith | mriedem: that's on the cinder side though yeah? | |
| 16:14:16 | mriedem | yup | |
| 16:14:21 | mriedem | but i'd do the exact same thing there | |
| 16:14:29 | mriedem | ala oslo.incubator style | |
| 16:14:59 | dansmith | aye | |
| 16:19:24 | mriedem | dansmith: so in https://review.openstack.org/#/c/563375/35/nova/objects/instance_group.py it seems weird that _rules is nullable but we default to {} if it's not set in the db | |