Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-21
18:55:07 mriedem which seems weird, i'm not sure why it would care
18:55:28 mriedem but we might have the same issue for live migration rollbacks which we don't test in the gate
18:57:35 mriedem i.e. i think we might need https://review.opendev.org/#/c/594139/
19:01:41 openstackgerrit Dustin Cowles proposed openstack/nova master: WIP: Use SDK instead of ironicclient for add/remove instance info from node https://review.opendev.org/659691
19:08:41 efried aspiers: Looks like you figured it out, but yeah, sometimes the code you're calling goes off and does things outside of the purview of the thing you're testing, and sometimes those things hit the thing you're mocking, if you're mocking something low-level like open or file existence, so you only want to use your mock in the code paths that you care about, and otherwise leave them unaffected.
19:08:47 openstackgerrit Hamdy Khader proposed openstack/os-vif master: OVS DPDK port representors support https://review.opendev.org/658786
19:15:46 aspiers efried: yeah, it hadn't occurred to me that os.path.exists() might be called on files which are actually inside the virtualenv, like placement-policy.yaml
19:16:03 aspiers efried: but when I blanket-mocked it, placement blew up in my face :)
19:16:21 aspiers efried: so I'm moving patch_exists() to nova.test.TestCase for reuse
19:19:07 efried ight
19:21:40 sean-k-mooney mriedem: ill take a look. once we have activated the dest binding it should have atomicly delete teh source binding so that makes sense
19:26:45 mriedem i thought activating the dest binding just makes the source binding automatically inactive, but not deleted
19:26:50 openstackgerrit Adam Spiers proposed openstack/nova master: Move patch_exists() to nova.test.TestCase for reuse https://review.opendev.org/660500
19:26:52 aspiers efried: ^^^^
19:30:03 sean-k-mooney mriedem: actully looking at the spec https://specs.openstack.org/openstack/neutron-specs/specs/ocata/portbinding_information_for_nova.html#usage-by-nova you are correct but i had a feeling it was different for some reason
19:30:29 mriedem looking at the ml2 plugin code that also looks correct,
19:30:43 mriedem originally i had also assumed the deleting the active binding would automatically re-activate the inactive binding, but that's not the case
19:31:00 mriedem it just deletes the active port binding resource and i think that eventually makes the vif_type=unbound
19:31:14 mriedem which is why i have https://review.opendev.org/#/c/594139/
19:32:00 openstackgerrit Merged openstack/nova stable/stein: Fix retry of instance_update_and_get_original https://review.opendev.org/659317
19:32:04 sean-k-mooney yes if there is no active port binding having it revert to vif_type=unbound make sense
19:32:07 mriedem my guess is there might be a bug in neutron when (1) delete the active binding and then (2) change the port's binding:host_id to a new host (for the inactive binding), it must try to do some cleanup of the old binding and fails b/c it doesn't exist anymore
19:32:51 sean-k-mooney well thats the think i dont think its valide to just update teh inactive binding
19:33:18 sean-k-mooney that part of why the other live migration issue was happening
19:34:01 sean-k-mooney the neutron port bidning api seems to only work if you interact with it exactly as they expect with no auto recovery if we delete or update things
19:34:39 sean-k-mooney the spec didnt say we were allowed to interact with it in thos other ways so its fair but likely it could be made more robust
19:34:47 mriedem yeah maybe, i'd accept that if i opened a neutron bug and that was the response
19:35:00 mriedem or if mlavalle said that was the case
19:36:17 sean-k-mooney i also would not expect much push back if we specified the set of addtion behavior we intuitivly expect to work and proposed them as an extention
19:36:37 mriedem i don't really care about making those edge cases work,
19:36:45 mriedem i'd just like clarity on the api, since it's not documented in the api-ref
19:37:05 mriedem so maybe that's the neutron bug is just lack of docs
19:37:32 sean-k-mooney right i more or less keep going back to the spec to determin what the expected behavior is
19:37:47 sean-k-mooney my guess is what happening is becasue of https://specs.openstack.org/openstack/neutron-specs/specs/ocata/portbinding_information_for_nova.html#effects-on-existing-apis
19:38:01 sean-k-mooney Update Port with host_id will still re-trigger port binding for a host. The difference will be update_port() will only action on the active binding.
19:38:11 mriedem "The difference will be update_port() will only action on the active binding."
19:38:12 mriedem yeah
19:38:18 mriedem and if the active binding doesn't exist....
19:38:29 mlavalle mriedem: has the src inactive binding benn deleted when you try to go back?
19:38:39 mriedem mlavalle: no
19:38:51 mriedem well, not as far as i know anyway
19:39:04 mriedem nova isn't explicitly deleting the inactive source host binding
19:39:09 sean-k-mooney the correfct thing to do based on the spec would be to reactivate the souce binding
19:39:27 mlavalle in that case, I think that whn going back, activate tje source binding
19:39:28 sean-k-mooney rather then update it
19:39:57 sean-k-mooney which is what mriedem is doing in https://review.opendev.org/#/c/594139/ correct
19:40:31 sean-k-mooney ya in setup_networks_on_host
19:41:34 mriedem ok yeah that's what i'll try (when i can push up the change)
19:42:29 sean-k-mooney mriedem: that is what you are already doing in https://review.opendev.org/#/c/594139/1/nova/network/neutronv2/api.py@423
19:42:39 sean-k-mooney instance.host should be the souce host
19:42:50 mriedem sean-k-mooney: that's not a patch in my cross-cell series though
19:42:57 mriedem it's just a related thing
19:42:57 sean-k-mooney oh ok
19:43:01 mriedem and ^ isn't merged anyway
19:44:34 sean-k-mooney i wonder if we call _update_port_binding_for_instance in teh revert flow
19:45:00 sean-k-mooney that might be why https://review.opendev.org/#/c/653506/ was fixign your cross cell issue
19:45:16 mriedem i'm not using https://review.opendev.org/#/c/653506/ in the cross-cell series
19:45:48 mriedem what fixed the cross-cell issue was i needed to activate the dest host bindings before calling migrate_instance_finish https://review.opendev.org/#/c/634832/29/nova/compute/manager.py@4625
19:46:09 mriedem same as live migration in _post_live_migration
19:47:23 sean-k-mooney actully i missed something the cross cell migration/resize is a cold migration. you choose to use the the multiple port binding flow for cross cell migrations?
19:48:14 mriedem yes
19:48:24 sean-k-mooney ah ok
19:48:43 mriedem because we want to validate early that we can create the port binding on the dest host in the target cell
19:48:50 sean-k-mooney then yes that makes sense that you would have to also add that
19:49:29 sean-k-mooney yep cross cell is proably even more imporant then live migration in that its likely more expensive to try again
19:55:26 sean-k-mooney i think the answer is no but does the super conductor ever call the cell conductor or does it always interact directly with a compute node. that a general cellsv2 question not related to your patches
19:56:14 sean-k-mooney i just realised i dont think i have ever seen code in the conductor call another conductor but i have nerver actully looked either
20:00:26 mriedem it calls the compute
20:00:48 sean-k-mooney and the compute talks to its local cell conductor as needed
20:13:05 mriedem yes
20:32:30 sean-k-mooney mriedem: thanks for the correction/comments on https://review.opendev.org/#/c/643578 ill fix that up tomorrow
20:33:02 sean-k-mooney mriedem: are you ok with the verb 'resurrect' by the way
20:34:38 mriedem from the forum session etherpad there is a note that mordred wants to talk about naming
20:34:47 mriedem https://etherpad.openstack.org/p/DEN-osc-compute-api-gaps
20:35:01 openstackgerrit Eric Fried proposed openstack/nova master: Support old & new versions of svm and avx traits https://review.opendev.org/660515
20:35:12 sean-k-mooney ill ping him and dean to review once i fix up the last few bits
20:36:55 mriedem dansmith may or may not have strong feelings about how evacuate gets named in osc
20:37:12 sean-k-mooney ill also proably submit a patch to try and swap over the nova live migration job to uses it but im not 100% certin a depens-on will work. i could add it to the requred project which should work but no sure we want to do that in general
20:37:46 mriedem you could test it with a dnm/wip patch until it's released in osc
20:38:04 sean-k-mooney ya i can try that
20:38:23 sean-k-mooney its more i dont think we want to use osc in the gate form mater in general
20:38:34 sean-k-mooney so i can juse a dnm patch that does for testing
20:38:52 sean-k-mooney then drop the source install bit after its released
20:39:55 sean-k-mooney i just dont want to adde the python-openstackclient as a required project in the final version of the gate job patch but as you siad tis fine for a dnm or wip patch
20:40:31 mriedem we already use osc in the gate
20:40:33 mriedem for devstack and grenade
20:40:52 mriedem using the things we want users to use in our testing is a good thing
20:43:13 dansmith mriedem: I dunno.. I definitely want a different name for it, but don't think we should change the API.. having it be named something different in osc seems confusing, especially since it will be a new operation that isn't one of the other three evacuates we have
20:44:34 mriedem new operation?
20:44:58 mriedem it would just be the osc version of https://docs.openstack.org/python-novaclient/latest/cli/nova.html#nova-evacuate
20:46:17 sean-k-mooney mriedem: oh im not saying we shoulding use osc in the gate i was saying we proably want to stick to released versions in the gate rather then adding to reqiured porjects which get translated in to LIBS_FORM_GIT which will install it form master
20:46:37 mriedem sean-k-mooney: yes agree
20:47:27 sean-k-mooney dansmith: my current patch just delegated internall to the nova evacuate command so its the same one that we know and hate
20:48:13 sean-k-mooney dansmith: origially i was calling the osc version "openstack server recreate" but aspiers asked me to change it to "openstack server resurrect"
20:48:22 sean-k-mooney which is what the current patch uses
20:49:02 dansmith sean-k-mooney: right and I'm saying having the command name be something new may not help with the confusion
20:49:25 sean-k-mooney dansmith: that is vaild
20:49:34 dansmith because now it will be "resurrect in osc, which is evacuate in novaclient, which is not host-evacuate or host-evacuate-live"
20:49:52 sean-k-mooney at least for osc we currently dont have the host versions
20:50:30 sean-k-mooney we did talk about that in the fourm session however but bauzas is not a fan of adding them to osc and well i dont care one way or another

Earlier   Later