| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-13 | |||
| 17:51:16 | bauzas | rm -rf ? | |
| 17:51:19 | melwitt | tzumainn: ok, there might be a bug there. I can do some digging later to confirm and check if we have any open bugs or patches about it already | |
| 17:51:46 | mriedem | bauzas: if i need a clean slate i create a new devstack vm | |
| 17:51:54 | bauzas | lucky you :p | |
| 17:52:09 | mriedem | lemme guess, you're running devstack on baremetal with gpus | |
| 17:52:12 | bauzas | here I'm talking of some internal server that I got by accident | |
| 17:52:37 | bauzas | mriedem: yeah, I want to test the reshape series on real hardware | |
| 17:53:11 | bauzas | -where we have RHEL7.5... - | |
| 17:53:22 | bauzas | but anyway, will continue to look | |
| 17:53:35 | tzumainn | melwitt, thanks! I didn't see an existing bug, but looking at https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/hypervisors.py#L167-L168 maybe what I'm seeing is intended behavior, and it's matching based on the hypervisor host_ip (which is the same for all four baremetal nodes)? | |
| 17:55:09 | mriedem | tzumainn: it's a fuzzy search on hypervisor hostname https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L680 | |
| 17:55:42 | mriedem | for ironic nodes that should actually be a uuid | |
| 17:55:47 | tzumainn | mriedem, yep, that's what I'm using | |
| 17:56:47 | mriedem | and the API code is taking that compute node record and getting all instances on that compute_nodes.host | |
| 17:56:50 | mriedem | which is the same for all of your nodes | |
| 17:57:05 | mriedem | compute_nodes.host == nova-compute service hostname == instances.host | |
| 17:57:22 | mriedem | so it's working as expected, i.e. that API is not ironic-aware | |
| 17:57:25 | mriedem | nor is most of the compute API | |
| 17:57:27 | mriedem | hence MOGAN | |
| 17:57:30 | melwitt | tzumainn: yeah, internally in nova we have hypervisor "hostname" which is associated with the nova-compute service and we have "nodename" which is the ironic nodename. if you're not using ironic hostname == nodename. else, it will be different and to get the right answer (better answer?) for ironic, we'd need to do a lookup based on nodename, not "hostname" | |
| 17:57:44 | openstackgerrit | Adrian Chiris proposed openstack/nova master: Libvirt: do not set MAC when unplugging macvtap VF https://review.openstack.org/624842 | |
| 17:57:45 | openstackgerrit | Adrian Chiris proposed openstack/nova master: Add free for claimed, allocated devices https://review.openstack.org/616120 | |
| 17:57:46 | openstackgerrit | Adrian Chiris proposed openstack/nova master: SR-IOV Live migration indirect port support https://review.openstack.org/620115 | |
| 17:57:46 | openstackgerrit | Adrian Chiris proposed openstack/nova master: Add get_instance_pci_request_from_vif https://review.openstack.org/619929 | |
| 17:57:46 | openstackgerrit | Adrian Chiris proposed openstack/nova master: Allow per-port modification of vnic_type and profile https://review.openstack.org/607365 | |
| 17:57:47 | openstackgerrit | Adrian Chiris proposed openstack/nova master: libvirt: auto detach/attach sriov ports on migration https://review.openstack.org/629589 | |
| 17:59:54 | tzumainn | mriedem, melwitt gotcha - okay, thanks! | |
| 18:00:16 | mriedem | tzumainn: without an api change the best we can do right now is document the wrinkle in the api reference | |
| 18:01:11 | tzumainn | mriedem, would an API change be possible in the longer-term, or would you consider this a tragic wrinkle in how nova/ironic interact? | |
| 18:01:42 | mriedem | tzumainn: i think it could be a possible change on a new microversion - if the resulting compute node we found is of type ironic, we have to lookup instances by nodename, not host | |
| 18:02:05 | melwitt | ++ | |
| 18:02:36 | mriedem | tzumainn: feel free to file a bug so it could at least be documented and then a blueprint, if someone wants to work on that, could be written based on the bug | |
| 18:02:40 | mriedem | jaypipes might like that | |
| 18:02:49 | tzumainn | mriedem, I'll do that - thanks very much! | |
| 18:02:54 | mriedem | yw | |
| 18:03:13 | melwitt | would also be good for a newer contributor bc it seems like it should be pretty small change | |
| 18:03:16 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: ironic: check fresh data when sync_power_state doesn't line up https://review.openstack.org/636699 | |
| 18:03:35 | mriedem | until they have to write functional api samples tests | |
| 18:03:39 | mriedem | and their head explodes | |
| 18:04:05 | melwitt | ah, I didn't think of that part | |
| 18:07:13 | cdent | but that's not why I'm here, I have an actual question | |
| 18:07:42 | cdent | it seems that most of the volume related methods in compute manager are synchronized on instance uuid, but detach volume is not. Is there a reason it is not? | |
| 18:09:22 | sean-k-mooney | instnace uuid or volume uuid | |
| 18:10:12 | sean-k-mooney | if its instance could there be issues with multiattach volumes? | |
| 18:10:33 | sean-k-mooney | cdent: also i dont know | |
| 18:10:37 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: ironic: check fresh data when sync_power_state doesn't line up https://review.openstack.org/636699 | |
| 18:11:06 | cdent | sean-k-mooney: yeah, I was confused by the choice of lock id | |
| 18:11:10 | cdent | (too) | |
| 18:14:32 | sean-k-mooney | cdent: i have not looked at the code but is your requestion related to the use of the uuid for synconisation or is detach ungaurded and your asking should it be | |
| 18:15:31 | cdent | sean-k-mooney: I'm asking if it should be. In k8s-on-openstack environment there are issues happening with volume attachements getting into a weird state | |
| 18:15:32 | openstackgerrit | garyk proposed openstack/nova master: Lock detach_volume https://review.openstack.org/636701 | |
| 18:15:44 | cdent | that's ^ the proposed fix | |
| 18:16:58 | sean-k-mooney | cdent: lyarwood and mdbooth are usually who i ask when it comes to volumn/storage things | |
| 18:17:26 | cdent | yeah, me too, but I'm hoping they've already stepped awy, keeping reasonable hours for this timezone, unlike you and me | |
| 18:17:44 | lyarwood | I'm around, just | |
| 18:18:04 | lyarwood | we already lock within the libvirt driver, I assume this is for VMWare? | |
| 18:18:26 | cdent | lyarwood: in the compute manager itself | |
| 18:18:31 | cdent | not the virtdrivers | |
| 18:18:35 | cdent | see the patch | |
| 18:18:57 | lyarwood | cdent: right, and I assume the issues you're seeing are with the underlying detach/attach on the virt drivers? | |
| 18:19:08 | lyarwood | cdent: or is it with the attachments themselves in c-api? | |
| 18:19:43 | cdent | lyarwood: I'm short on details. At the moment I'm just feeling around trying to gather info. I wasn't present at the discovery of the issue. | |
| 18:19:54 | lyarwood | kk np | |
| 18:20:36 | openstackgerrit | garyk proposed openstack/nova master: Lock detach_volume https://review.openstack.org/636701 | |
| 18:22:09 | cdent | lyarwood: as far as I can tell whatever the client is (some k8s thing) is going async on the volume delete and instance delete | |
| 18:22:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: mark os-cells as deprecated https://review.openstack.org/636708 | |
| 18:23:13 | lyarwood | cdent: yeah and as vmware isn't using os-brick you don't get any of the locking within it, adding a lock in the compute layer for detach seems reasonable tbh. | |
| 18:23:13 | mriedem | cdent: i seem to remember mdbooth having a change to lock detach_volume on instance.uuid | |
| 18:23:44 | cdent | mriedem: you mean in progress? | |
| 18:23:49 | mriedem | might be abandoned by now | |
| 18:24:56 | mriedem | https://review.openstack.org/#/c/430213/ | |
| 18:24:57 | mriedem | bingo | |
| 18:25:01 | sean-k-mooney | lyarwood: well as cdent pointed out we already lock in the compute manager for attach | |
| 18:25:29 | mriedem | cdent: there you go - want me to restore and you can rebase? | |
| 18:25:39 | lyarwood | sean-k-mooney: right, I'm agreeing | |
| 18:26:16 | sean-k-mooney | lyarwood: yep i know. | |
| 18:26:30 | sean-k-mooney | mriedem: is there any reason to restor it vs https://review.openstack.org/#/c/636701/2 | |
| 18:26:47 | sean-k-mooney | the two are identical | |
| 18:26:52 | mriedem | how about because it came first? | |
| 18:27:06 | cdent | thanks mriedem I looked, but not that far back | |
| 18:27:50 | sean-k-mooney | ok i gues garyk could take over rebaseing the old patch | |
| 18:29:00 | cdent | since it is in merge conflict, how about we just booth as a co-author on the clean one, and add the older bug too? | |
| 18:29:42 | mriedem | that works too... | |
| 18:29:47 | mriedem | i've duplicated the bug already | |
| 18:29:55 | mriedem | sec | |
| 18:30:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Lock detach_volume https://review.openstack.org/636701 | |
| 18:32:42 | mriedem | comment inline | |
| 18:32:47 | mriedem | i'm not sure we need to hold the lock while doing the db query | |
| 18:32:47 | cdent | damn dood, I think I'm just gonna let you do everything ;) | |
| 18:32:53 | cdent | yeah | |
| 18:33:43 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Lock detach_volume https://review.openstack.org/636701 | |
| 18:34:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Lock detach_volume https://review.openstack.org/636701 | |
| 18:34:21 | mriedem | there god | |
| 18:34:23 | mriedem | coding in gerrit ui | |
| 18:35:21 | cdent | Is there any chance that get_by_volume_and_instance itself can race? | |
| 18:35:48 | openstackgerrit | Vlad Gusev proposed openstack/nova stable/queens: Handle unicode characters in migration params https://review.openstack.org/636678 | |
| 18:37:02 | mriedem | worst case is you'd get a not found error from the db query | |
| 18:53:00 | openstackgerrit | Merged openstack/nova master: Change nova-next job to run with python3 https://review.openstack.org/634739 | |
| 18:53:14 | openstackgerrit | Merged openstack/nova master: Adding cross refs for config options in scheduler filter guide https://review.openstack.org/636635 | |
| 19:01:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove cells v1 (for the most part) from the docs https://review.openstack.org/636718 | |