| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-17 | |||
| 17:30:23 | artom | Which isn't in the list I got | |
| 17:32:05 | mdbooth | artom: Can you note on the review that we already do that? I'll probably abandon it, and/or update my recursive test to use the existing method. | |
| 17:32:38 | artom | mdbooth, sure | |
| 17:32:45 | mdbooth | artom: Thanks | |
| 17:35:43 | artom | mdbooth, there. I didn't even -1 it ;) | |
| 17:38:52 | mriedem | cfriesen: did you happen to catch the sriov bug armax was asking about last night? | |
| 17:40:28 | armax | mriedem: btw talking to gyee_ it seems the issue happens in Newton still | |
| 17:43:56 | gyee_ | I am curious why we are using NopClaim if host is specified https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2771 | |
| 17:44:21 | gyee_ | does specifying a host implies force evacuate? | |
| 17:49:07 | gyee_ | meanwhile if SR-IOV is enabled, we are trying to get the PCI mapping via the migration context, which is only populated at rebuild_claim. https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L2462 | |
| 17:51:31 | mriedem | https://github.com/openstack/nova/commit/dc0221d7240326a2d1b467e2a367bebb7e764e61 added that code in the compute manager about the nop claim, which implies resources were already claimed, but i'd have to dig into that | |
| 17:56:36 | mriedem | yikes, that was part of a big series https://review.openstack.org/#/q/topic:bug/1417667 | |
| 17:57:37 | gyee_ | but I don't see where the resource is being claimed, if the host is specified | |
| 17:57:53 | openstackgerrit | Gage Hugo proposed openstack/nova master: Admin API Policy contingent on is_admin_project https://review.openstack.org/384148 | |
| 17:58:00 | mriedem | gyee_: i'm also looking for that | |
| 18:03:50 | gyee_ | mriedem: I don't think that's being done. The migration_context in instance_extra table is empty if host specified, regardless if SR-IOV is enabled or not | |
| 18:06:21 | mriedem | gyee_: as far as i can tell, the only way to get the noop claim is if the user specifies the host and --force | |
| 18:07:01 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4115 | |
| 18:07:20 | mriedem | if the host is specified but force is not, then the api nulls out host when calling conductor | |
| 18:07:39 | gyee_ | I didn't use the --force option. Just merely specifying the host. | |
| 18:07:54 | mriedem | which gets to this else block https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L751 | |
| 18:08:08 | mriedem | node would eventually be set here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L770 | |
| 18:08:15 | mriedem | and passed to compute as scheduled_node | |
| 18:08:43 | mriedem | which should get the rebuild_claim here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2769 | |
| 18:08:56 | mriedem | dansmith: do you remember anything about not doing a claim if the host is forced during evacuate? | |
| 18:09:41 | dansmith | mriedem: no | |
| 18:10:01 | dansmith | mriedem: I would think you'd get a new claim on the new compute node, and then drop the old one when the dead compute comes up a deletes the old instance | |
| 18:12:01 | mriedem | i'm not sure why we have this then https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2771 | |
| 18:12:31 | dansmith | mriedem: well for a regular rebuild | |
| 18:12:46 | mriedem | oh...yeah | |
| 18:12:47 | dansmith | regular rebuild is in place | |
| 18:13:01 | mriedem | original commit message says "Also - if we called the scheduler, we also know the node of the chosen host, and the limits set, and this information is needed for doing claims." | |
| 18:13:17 | mriedem | but yeah, | |
| 18:13:24 | mriedem | i suppose the noop claim is there for rebuild on the same host | |
| 18:13:25 | mriedem | not evacuate | |
| 18:13:29 | mriedem | god | |
| 18:13:29 | sdague | efried: I'm pushing in your devstack console color fix, thanks again for that | |
| 18:13:58 | mriedem | at some point we should just split the evacuate and rebuild code apart in the compute manager | |
| 18:14:53 | gyee_ | ++ | |
| 18:15:01 | mriedem | i think you can also specify a host for rebuild | |
| 18:15:50 | mriedem | oh no nvm | |
| 18:15:54 | mriedem | i was thinking of another bug, | |
| 18:16:14 | mriedem | where you change the image and the new image meta for the rebuild doesn't work with the original host, but we don't validate that | |
| 18:16:56 | mriedem | gyee_: then i think all you need to do is add an "or recreate" to this conditional https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2767 | |
| 18:17:05 | mriedem | gyee_: do you have a recreate of the bug? | |
| 18:19:01 | gyee_ | mriedem: I haven't file a bug for Nova yet | |
| 18:19:04 | gyee_ | about to do so | |
| 18:19:18 | mriedem | gyee_: oh we have a bug | |
| 18:19:20 | mriedem | well MOS does | |
| 18:19:23 | mriedem | but we can add nova to that | |
| 18:19:49 | cfriesen | question about RPC (with standard rabbitmq/kombu messaging): are incoming messages on nova-compute immediately grabbed by an eventlet? I'm seeing huge lock contention on the 'refresh_cache-%s' lock between the live migration code and external events coming in from neutron | |
| 18:19:57 | gyee_ | do I have perm to do that? Sorry I am still new to Nova. | |
| 18:20:01 | openstack | Launchpad bug 1658070 in Mirantis OpenStack "Failed SR_IOV evacuation with host" [Medium,Confirmed] | |
| 18:20:01 | mriedem | gyee_: https://bugs.launchpad.net/mos/+bug/1658070 | |
| 18:20:32 | gyee_ | oh good :-) | |
| 18:24:43 | cdent | bauzas, dansmith: sorry for beating a dead horse, but I need some help understanding https://review.openstack.org/#/c/465175/ (returning alternatives) | |
| 18:25:08 | cdent | edleafe too | |
| 18:25:12 | gyee_ | mriedem: thanks for the help! | |
| 18:25:34 | dansmith | cdent: what about it? | |
| 18:25:36 | mriedem | gyee_: np, you did the hard debug already, | |
| 18:25:39 | cdent | in the collection of hosts being returned from select_destinations there is a list of dicts | |
| 18:25:47 | cdent | in each dict is one host which is the chosen host | |
| 18:25:49 | mriedem | gyee_: if you get a recreate and have that fix handy ping me to review | |
| 18:26:04 | cdent | and then a key 'alternatives' which are other hosts that might work | |
| 18:26:56 | cdent | is each chosen host (across the entire selected_hosts) different or does every single host satisfy the request_spec? | |
| 18:27:20 | gyee_ | mriedem: definitely, just need to add unit tests before pushing it upstream | |
| 18:27:34 | cdent | or to put it another way: what is the reason for not having simply "more hosts" instead of the nested "alternatives"? | |
| 18:27:35 | dansmith | cdent: each host could satisfy the request.. meaning all the primaries and alternates | |
| 18:28:01 | dansmith | cdent: I had thought about it in terms of just a list of hosts, and not the hierarchy, however: | |
| 18:28:28 | dansmith | cdent: I think the point is that we will, for a single num_instances>1 request, not choose the same primary host for two instances, but the alternates can overlap | |
| 18:28:41 | dansmith | cdent: so the distinction is made such that we consume the primaries | |
| 18:29:00 | mriedem | gyee_: i'm still a bit confused about how this happens if that force flag isn't specified | |
| 18:29:12 | cdent | dansmith: is the additional code complexity worth the win? | |
| 18:30:16 | dansmith | cdent: well, if we don't, then we'll choose hostA, (hostB, C) for the first instance and hostB, (HostA, C) for the second, right? that's a bad decision if we fail on hostB and the first instance filled up hostA | |
| 18:30:34 | dansmith | cdent: also, I think we have to consume the hosts as primaries to keep the existing scheduler behavior, right? | |
| 18:31:01 | dansmith | the previous code did that such that num_instances>1 would not pack them all onto the same host, IIUC | |
| 18:31:49 | dansmith | I dunno, I guess I'd wait for either bauzas or johnthetubaguy to comment on that | |
| 18:32:24 | cdent | thanks dansmith, I'll have to read through it a bit more | |
| 18:32:31 | cdent | it feels like it could be simpler, but feelings may not matter | |
| 18:33:16 | dansmith | well, FWIW, when I first read that code I was kinda horrified with the nested dicting, | |
| 18:33:32 | dansmith | but I didn't complain because I think it probably makes the smallest behavioral change | |
| 18:33:52 | dansmith | it's more code, but I'm not sure it's _that_ much more complex | |
| 18:34:20 | dansmith | it's not an unreasonable opinion though, IMHO | |
| 18:34:44 | dansmith | recording it, even if it is shot down with technical reasons is probably a good idea | |
| 18:34:57 | cdent | roger that | |
| 18:35:04 | gyee_ | mriedem: maybe none != False? | |
| 18:35:06 | EmilienM | mriedem, cdent: super quick fyi: I tested again nova/wsgi and it works fine | |
| 18:35:21 | cdent | yay! | |
| 18:35:30 | EmilienM | this is our config: http://logs.openstack.org/81/462281/3/check/gate-puppet-openstack-integration-4-scenario001-tempest-centos-7/ce2e89d/logs/apache_config/10-nova_api_wsgi.conf.txt.gz | |
| 18:35:37 | EmilienM | if you have any suggestion, feel free to let me know | |
| 18:35:47 | EmilienM | the proof it works http://logs.openstack.org/81/462281/3/check/gate-puppet-openstack-integration-4-scenario001-tempest-centos-7/ce2e89d/logs/apache/nova_api_wsgi_access_ssl.txt.gz | |
| 18:36:56 | EmilienM | mwhahaha: ^ fyi also | |
| 18:37:04 | mriedem | gyee_: i think you might be right https://github.com/openstack/nova/blob/stable/newton/nova/api/openstack/compute/evacuate.py#L92 | |
| 18:37:13 | cdent | EmilienM: is cgi-bin/nova/nova-api the new wsgi script that pbr creates? | |
| 18:37:26 | EmilienM | cdent: yes, nova-api-wsgi or something | |
| 18:37:35 | mriedem | gyee_: gah, yeah, that's it | |
| 18:37:39 | EmilienM | cdent: /usr/bin/nova-api-wsgi | |
| 18:37:39 | edleafe | cdent: my understanding is that if N instances are requested, we'd pick N hosts from the list of weighed hosts. Then for each, we'd get the cell uuid and from the remaining, find enough in the same cell to satisfy the number of alternates needed | |
| 18:37:49 | gyee_ | mriedem: two bugs then | |
| 18:37:50 | mriedem | so if you're using microversion<2.29 then force is None | |