| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-27 | |||
| 12:52:10 | PapaOurs | either way, giblet +Wd your change | |
| 12:57:34 | mriedem | i do see one potential place i missed | |
| 12:59:24 | mriedem | giblet: comment inline, i'll do a follow up | |
| 13:00:36 | giblet | mriedem: OK, cool | |
| 13:01:47 | mriedem | hyperv ci failed but on unrelated tests | |
| 13:01:52 | mriedem | looks like those were failing due to ssh and timeouts | |
| 13:01:57 | mriedem | {7} tempest.api.volume.test_volumes_extend.VolumesExtendTest.test_volume_extend_when_volume_has_snapshot [365.093541s] ... FAILED | |
| 13:03:17 | mriedem | uest since it is either malformed or otherwise incorrect.\n\n JSON does not validate: {} does not have enough properties Failed validating 'minProperties' in schema['properties']['allocations']['items']['properties']['resources']: {'additionalProperties': False, 'minProperties': 1, 'patternProperties': {'^[0-9A-Z_]+$': {'minimum': 1, 'type': 'integer'}}, 'type': | |
| 13:03:17 | mriedem | huh | |
| 13:03:17 | mriedem | 2018-07-27 05:15:36.661 5060 105049744 MainThread WARNING nova.scheduler.client.report [req-640b132e-9a1b-4f75-8f8d-7ae96964af72 c329c90c52a44fe2889e0284651a21f0 82e0a447215e49079fe42481922ccd81 - default default] Failed to save allocation for 390d33d0-36e2-469e-85be-8ec10658e953. Got HTTP 400: {"errors": [{"status": 400, "request_id": "req-fc67d1c6-b641-475a-afdf-27075995c0ff", "detail": "The server could not comply with the | |
| 13:03:18 | mriedem | ect'} On instance['allocations'][0]['resources']: {} ", "title": "Bad Request"}]} | |
| 13:03:57 | mriedem | Sending updated allocation [{'resource_provider': {'uuid': u'b2979fd7-376b-4f9e-a1b9-b4c69d619cb9'}, 'resources': {}}] for instance 390d33d0-36e2-469e-85be-8ec10658e953 | |
| 13:04:32 | mriedem | 2018-07-27 05:15:36.513 5060 105049744 MainThread INFO nova.compute.manager [req-640b132e-9a1b-4f75-8f8d-7ae96964af72 c329c90c52a44fe2889e0284651a21f0 82e0a447215e49079fe42481922ccd81 - default default] [instance: 390d33d0-36e2-469e-85be-8ec10658e953] Doing legacy allocation math for migration 8221f52a-c72b-4b7b-81d9-67cb67fb37bc after instance move | |
| 13:05:06 | mriedem | i'm not sure why the hyperv ci would be hitting that in rocky | |
| 13:06:35 | mriedem | edmondsw: powervm in-tree ci took over 5 hours here and timed out https://review.openstack.org/#/c/586402/ | |
| 13:06:36 | mriedem | fyi | |
| 13:11:49 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Use placement 1.28 in scheduler report client https://review.openstack.org/583667 | |
| 13:12:55 | cdent | Is this already a known thing: http://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22Unsupported%20VIF%20type%20unbound%20convert%5C%22 | |
| 13:13:20 | cdent | oh never mind, my search on launchpad just hit | |
| 13:13:30 | mriedem | http://status.openstack.org/elastic-recheck/index.html#1783917 | |
| 13:13:30 | cdent | it didn't when I was missing a closing t | |
| 13:13:44 | mriedem | fix is in the gate | |
| 13:13:59 | cdent | cool, thanks | |
| 13:15:50 | mriedem | based on the 50 mocks i have to do in _post_live_migration, clearly that method is too big | |
| 13:16:00 | cdent | ugh | |
| 13:26:18 | edmondsw | mriedem the powervm ci is borked right now. I'm trying to help get it fixed | |
| 13:33:54 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Use placement 1.28 in scheduler report client https://review.openstack.org/583667 | |
| 13:35:38 | dansmith | efried: what should happen if I have compute nodes with MISC_SHARES (and thus no DISK_GB inventory)? Should the scheduler receive split allocations from placement with disk on the sharing provider? | |
| 13:36:02 | dansmith | because I have yet to convince it to do that in a functional test | |
| 13:57:48 | mriedem | speaking of, i think this is going to be the money patch https://review.openstack.org/#/c/586363/ | |
| 13:58:19 | mriedem | creates a shared storage provider using the DISK_GB calculated from the compute node provider, then removes the compute node provider's DISK_GB inventory before the compute service host is discovered | |
| 13:59:58 | s10 | Please check this bug: https://bugs.launchpad.net/nova/+bug/1784006 | |
| 13:59:59 | openstack | Launchpad bug 1784006 in OpenStack Compute (nova) "Instances misses neutron QoS on their ports after unrescue and soft reboot" [Undecided,New] | |
| 14:01:35 | s10 | User can easily drop QoS limitations on ports with _soft_reboot() or unrescue() for libvirt driver. | |
| 14:04:16 | mriedem | s10: i think we do plug_vifs on hard reboot now, but maybe not in pike... | |
| 14:04:29 | mriedem | or maybe only for certain types of vifs... | |
| 14:04:35 | mriedem | it's kind of a mess | |
| 14:06:17 | s10 | plug_vifs are executed on hard reboot and spawn(). Not for soft reboot, in master: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2706 | |
| 14:06:40 | mriedem | oh right, i missed the "without" here: "Execute nova reboot (without parameter --hard)" | |
| 14:07:58 | melwitt | hm, I thought for soft reboot they shouldn't have been unplugged in the first place, but the bug says a new domain is created, which I didn't think happened either. I wonder if something changed there | |
| 14:08:43 | dansmith | soft reboot will turn into a hard reboot if the guest doesn't shut down voluntarily right? | |
| 14:08:47 | melwitt | shutdown and then a create | |
| 14:08:49 | mriedem | correct | |
| 14:09:04 | dansmith | it's trivial for me to make my guest not shut down when asked | |
| 14:09:07 | mriedem | but apparently in this case soft reboot works | |
| 14:09:42 | melwitt | looking at the code, indeed it does a guest.shutdown() followed by a create. so you'd think you'd have to plug the vifs in again, I wonder how this normally works? | |
| 14:10:22 | dansmith | hmm, it doesn't do an actual reboot? | |
| 14:11:17 | melwitt | doesn't look like it? I guess I've never looked at soft reboot in detail before https://github.com/openstack/nova/blob/stable/pike/nova/virt/libvirt/driver.py#L2547 | |
| 14:11:56 | dansmith | hmm, yeah, I didn't think this was like this | |
| 14:12:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass source vifs to driver.cleanup in _post_live_migration https://review.openstack.org/586568 | |
| 14:12:14 | mriedem | giblet: ^ | |
| 14:12:21 | giblet | mriedem: looking | |
| 14:13:03 | dansmith | I thought if we were running a virt that could do real reboot, we did that and only fell back to the shutdown/restart if not | |
| 14:13:06 | dansmith | but I don't see that | |
| 14:13:56 | melwitt | mriedem: why were you thinking not to use the source vifs throughout the entire method? just wondering | |
| 14:14:26 | mriedem | no particular reason, just wanted to minimize the amount of change, | |
| 14:14:42 | mriedem | but we could just do that at the top rather than get the refreshed nw info cache | |
| 14:15:20 | mriedem | i.e. here https://review.openstack.org/#/c/586568/1/nova/compute/manager.py@6555 | |
| 14:17:00 | mriedem | i can definitely make that change if it makes more sense | |
| 14:17:39 | melwitt | yeah, I'm not 100% sure but it feels like it should be consistent throughout. but I guess that's never guaranteed anyway because neutron events in flight could change the network_info as it goes through the method anyway? | |
| 14:18:01 | mriedem | shouldn't | |
| 14:18:23 | mriedem | an event would be processed separately and shouldn't be able to modify that network_info variable by reference | |
| 14:18:31 | melwitt | oh, yeah, okay | |
| 14:18:32 | mriedem | the instance.info_cache might be updated concurrently, sure | |
| 14:18:42 | mriedem | but we're using the local variable in most places | |
| 14:18:47 | melwitt | yeah | |
| 14:18:49 | mriedem | the versioned notifications will still use instance.info_cache | |
| 14:20:10 | mriedem | left that as a comment so giblet can also ponder it | |
| 14:20:35 | mriedem | i didn't do it in https://review.openstack.org/#/c/586402/ because (1) it was late and (2) i just wanted to get the immediate fire put out | |
| 14:20:48 | melwitt | I guess I could see the rationale in only using the source vifs for the relevant actions because like I think you mentioned, maybe the notifications should reflect the state of the network info cache at the time it was queried | |
| 14:21:03 | melwitt | that's the only other thing network info is used for in that method, I assume? | |
| 14:21:20 | pooja_jadhav | mriedem: hello | |
| 14:22:37 | pooja_jadhav | sean-k-mooney : hello | |
| 14:22:59 | mriedem | and unfilter_instance in the firewall driver, | |
| 14:23:13 | mriedem | i looked at how it was used in the various drivers and it was just getting the mac address off the vifs in one case | |
| 14:23:31 | mriedem | which i don't think should change | |
| 14:23:34 | mriedem | but, | |
| 14:23:57 | mriedem | admittedly, only passing the source vifs from migrate_data to 2 spots indicates tight coupling into knowing exactly what those methods are doing with network_info | |
| 14:23:59 | giblet | mriedem, melwitt: I think having the current network infor send in the notification is the valid thing as we are notifying about current state | |
| 14:24:00 | pooja_jadhav | sean-k-mooney, mriedem: I am trying live migrate and using nfs storage, its failing for "Binding failed for port e973dde6-d68c-4aec-a70d-86dcd81fa11b and host Neha-VirtualBox." | |
| 14:24:21 | mriedem | pooja_jadhav: i can't really help you debug that right now | |
| 14:24:30 | melwitt | giblet, mriedem: I think that makes sense too, the more I think about it | |
| 14:24:40 | mriedem | pooja_jadhav: i'd suggest using something besides devstack if you want a more sophisticated deployment tool for multi-node with live migration, like openstack-ansible | |
| 14:24:57 | pooja_jadhav | mriedem: ok | |
| 14:25:00 | mriedem | melwitt: i'm totally fine with making the generic switch at the top of the method | |
| 14:25:07 | mriedem | i don't like the tight coupling that's in here really | |
| 14:25:16 | mriedem | i just wanted to reduce any exposure to regression | |
| 14:25:40 | mriedem | pooja_jadhav: or look at a nova-live-migration job config and see how it set things up | |
| 14:25:46 | mriedem | but those don't use nfs | |
| 14:25:56 | mriedem | http://logs.openstack.org/02/586402/2/check/nova-live-migration/2db7a54/ | |
| 14:26:19 | mriedem | pooja_jadhav: binding failed means something failed in neutron | |
| 14:26:28 | mriedem | so network is messed up | |
| 14:26:43 | pooja_jadhav | mriedem: Hmm | |
| 14:27:26 | melwitt | mriedem: yeah, I'm thinking I agree with giblet though, that we should leave it the way you have it. let the notifications use the fresh network info and not artificially send source vif. I think the only reason to use source vifs there is if somehow a notifications listener might want to know which vif is actually being acted upon during the actions in the method. hmm. | |
| 14:28:13 | pooja_jadhav | mriedem: But I am not able to see any error logs at neutron side.. thats the problem | |
| 14:28:39 | mriedem | melwitt: giblet: well, only the versioned notifications will use the instance.info_cache, | |
| 14:28:45 | mriedem | the legacy ones would end up using the source vifs | |
| 14:29:15 | melwitt | oh | |
| 14:29:59 | mriedem | anyway, we could always change this later i guess if it causes some other unanticipated problem | |
| 14:30:16 | melwitt | yeah | |