| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-12 | |||
| 14:11:39 | sean-k-mooney | add --os-compute-api-version 2.21 to the request | |
| 14:11:50 | sean-k-mooney | *command | |
| 14:16:03 | rafaelweingartne | something like? "openstack server event list 9b82ae8f-ac7e-47e9-819c-24105631df80 --os-compute-api-version 2.21" | |
| 14:16:39 | rafaelweingartne | I just checked the code, and aren't the instances removed from "instance_mappings" table when they get deleted? | |
| 14:29:04 | efried | sean-k-mooney: First blush, adding the allocations from placement to the Instance object sounds like a solid idea. | |
| 14:34:28 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Fix double word hacking test https://review.opendev.org/664940 | |
| 14:34:43 | stephenfin | efried: f*** me that was a rabbit hole and half ^ | |
| 14:35:08 | efried | stephenfin: no doubt. Looking... | |
| 14:35:23 | stephenfin | tl;dr: I can't decide if either the Python patch or pycodestyle is broken, but it's the edgiest of edge cases that we'd only really see in tests, so we can work around it | |
| 14:37:51 | mriedem | rafaelweingartne: no, | |
| 14:37:58 | rafaelweingartne | hmm | |
| 14:37:59 | mriedem | the instance mappings are cleaned up when you archive | |
| 14:38:07 | mriedem | nova-manage db archive_deleted_rows | |
| 14:38:35 | mriedem | rafaelweingartne: 2.21 is about instance actions / events | |
| 14:39:07 | mriedem | rafaelweingartne: https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server-event.html | |
| 14:39:08 | gibi | stephenfin: what a detective work! | |
| 14:42:46 | dansmith | mriedem: can you quickly weigh in on this? I thought you might care about the use of a sentinel value in nova-manage. Otherwise I'm good with it: https://review.opendev.org/#/c/662383/5 | |
| 14:45:40 | mriedem | ew | |
| 14:47:11 | dansmith | before you shame him, note that it was my idea :) | |
| 14:48:53 | mriedem | i'm just trying to think of alternatives, | |
| 14:49:19 | mriedem | i saw melwitt pointed out https://review.opendev.org/#/c/603998/ but i'm not sure what that change is doing, | |
| 14:49:27 | mriedem | with takashi's change, | |
| 14:50:10 | mriedem | if you specify one but not the other it's an error, | |
| 14:50:38 | mriedem | i was thinking, what if we change the logic such that if you specify one but not the other, we just don't read from the config for the other (don't set/update the other) | |
| 14:50:48 | mriedem | if you don't specify either, we use config, | |
| 14:50:54 | mriedem | if you specify both, we set/update both, | |
| 14:50:56 | dansmith | that just seems confusing to me | |
| 14:50:59 | mriedem | if you only provide one, we only use one | |
| 14:51:14 | mriedem | i think that's more in line with how osc set commands work | |
| 14:51:27 | dansmith | I was actually expecting to have a --transport_url=(keep|config|[url]) sort of thing, but.. | |
| 14:52:02 | dansmith | the confusing bit is that we do the magic config thing if we don't specify either.. if we specify one, then we do a totally different set of things | |
| 14:52:32 | mriedem | i'm assumine when this was written, we expected people were either using all of the command options or none to let config handle it | |
| 14:52:33 | mriedem | not a mix | |
| 14:52:39 | dansmith | this has tripped up more than one person who was surprised by one or the other behaviors | |
| 14:52:54 | dansmith | well, | |
| 14:53:06 | dansmith | I think it actually came from create, where that makes more sense | |
| 14:53:14 | dansmith | and we imported the same behavior for update where it does not | |
| 14:54:39 | mriedem | yeah ok i can see that, | |
| 14:54:57 | mriedem | so in that case i think it makes more sense to only update the field provided via the option if only one option is provided, and ignore the other (don't use config) | |
| 14:55:46 | dansmith | le sigh | |
| 14:57:58 | mriedem | hey, you asked me | |
| 14:58:03 | mriedem | anyway, i commented | |
| 14:58:16 | mriedem | if we need a tie breaker i vote that we ask dean to weigh in | |
| 14:58:37 | mriedem | mean dean okerlund | |
| 14:58:40 | mriedem | from WWF fame | |
| 15:07:42 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Fix double word hacking test https://review.opendev.org/664940 | |
| 15:08:43 | bauzas | sean-k-mooney: hey | |
| 15:08:54 | bauzas | sean-k-mooney: yeah I tested it and it worked | |
| 15:09:05 | bauzas | ie. stopping the instance and then restarting it | |
| 15:09:08 | stephenfin | efried: I need to rebase this series now, right? https://review.opendev.org/#/c/651311/ | |
| 15:09:19 | stephenfin | to pick up the fix to test_hacking.py | |
| 15:09:37 | sean-k-mooney | bauzas: ok looking at the code im not sure where we figureout the mdevs in that case but i also didnt look | |
| 15:09:41 | sean-k-mooney | to far | |
| 15:09:49 | bauzas | sean-k-mooney: I provided a change for that | |
| 15:09:52 | bauzas | sec | |
| 15:10:09 | sean-k-mooney | my concern was we had to hit the placmenet api to look up the allcoation again | |
| 15:10:17 | sean-k-mooney | which i dont think we shoudl have to do. | |
| 15:10:57 | efried | stephenfin: I wouldn't think you need to rebase; I think zuul automatically rebases you against tip of master before starting tests | |
| 15:11:04 | bauzas | sean-k-mooney: https://review.opendev.org/#/c/564257/ | |
| 15:11:29 | bauzas | and https://review.opendev.org/#/c/533642/ | |
| 15:11:41 | sean-k-mooney | bauzas: that wont fix it | |
| 15:12:00 | sean-k-mooney | the second on might | |
| 15:12:09 | stephenfin | efried: Yeah, I'm not sure. That said, I think I'm going to have to recheck every one of them again anyway so maybe a rebase would be the surer thing | |
| 15:12:30 | stephenfin | Yeah, they're all -2 | |
| 15:12:49 | efried | stephenfin: Yeah, that would be an easy way to get them all back in the queue. Feel free to re+W the ones that were already +A. | |
| 15:12:58 | sean-k-mooney | bauzas: quickly looking i dont think either of those will fix the edgecase i am thinking of | |
| 15:13:09 | bauzas | sean-k-mooney: mmm N | |
| 15:13:10 | bauzas | ? | |
| 15:13:39 | sean-k-mooney | bauzas: if the instance is not found on the host in libvirt you retrun {] | |
| 15:13:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove cells v1 parameter from 'ComputeTaskAPI.resize_instance' https://review.opendev.org/651311 | |
| 15:13:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop passing 'kwargs' to 'rebuild_instance' https://review.opendev.org/651312 | |
| 15:13:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop passing 'delete_type' to 'terminate_instance' https://review.opendev.org/651313 | |
| 15:13:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: filters: Stop handling cells v1 https://review.opendev.org/651314 | |
| 15:13:41 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove nova.compute.*API() shims https://review.opendev.org/660527 | |
| 15:13:41 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reno for removed cells v1 policies https://review.opendev.org/662031 | |
| 15:13:42 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'InstanceUnknownCell' exception https://review.opendev.org/662411 | |
| 15:13:42 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Ensure controllers all call super https://review.opendev.org/660950 | |
| 15:13:47 | sean-k-mooney | bauzas: we can talk after standup | |
| 15:18:08 | sean-k-mooney | bauzas: basically if we stop the instance then we undefine the domain so if start it when we call _get_all_assigned_mediated_devices | |
| 15:18:18 | sean-k-mooney | bauzas: we retrun {} | |
| 15:18:41 | bauzas | I'm pretty sure I tested it | |
| 15:18:44 | stephenfin | mriedem: Are you okay for me to rebase https://review.opendev.org/#/c/621061/ onto the cells v1 removal series (all but one +W'd now). It conflicts with that series and I'd like to use it as a new base for https://review.opendev.org/#/c/651315/ | |
| 15:22:02 | mriedem | sure | |
| 15:22:19 | mriedem | maybe you want to re-apply your +2 when doing so | |
| 15:39:57 | Sundar | sean-k-mooney: Re. tempest CI with a fake driver/device, I have some questions | |
| 15:41:21 | Sundar | sean-k-mooney: When there is no real device, the VM bringup is going to fail. Not only that, there would be no VM in 'virsh list', so we can't do 'virsh dumpxml ...' to see if the right device(s) got attached. | |
| 15:41:37 | Sundar | So, how would would one do any testing? | |
| 15:42:45 | sean-k-mooney | Sundar: there are a few ways. one we can use dummy devices | |
| 15:43:10 | sean-k-mooney | but the intent is to test teh end to end workflow not assert a spcific device exits | |
| 15:43:35 | Sundar | What would be the success metric for the test? | |
| 15:46:43 | mriedem | Sundar: tempest isn't going to be doing virsh list anyway | |
| 15:46:57 | mriedem | tempest tests should be hypervisor agnostic, like the compute api should be hypervisor agnostic | |
| 15:47:27 | mriedem | Sundar: the basic success metric for tempest tests are going to be that the API behaves as expected, | |
| 15:47:49 | mriedem | i.e. if i create a server with a flavor configured for an ARQ device profile, that once the server is ACTIVE there is an attached ARQ in cyborg | |
| 15:48:02 | mriedem | and when the server is deleted, the corresponding ARQ resource is also deleted | |
| 15:49:08 | mriedem | Sundar: whitebox integration testing is something that would likely be done in a tempest plugin / tests that are configured to only run when there is real hardware in a 3rd party CI env | |
| 15:50:32 | Sundar | mriedem: "once the server is ACTIVE" -- with a fake device, it never becomes active. It will go to error state. | |
| 15:51:20 | mriedem | Sundar: because of the event not being sent from cyborg, or because libvirt will fail to start the domain? | |
| 15:51:26 | Sundar | We need to ensure the device gets attached to the VM | |
| 15:51:28 | sean-k-mooney | Sundar: not if we add a bus type for dummy devices and handel that in nova | |