| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-15 | |||
| 14:36:45 | mriedem | tssurya: it wouldn't, | |
| 14:36:50 | mriedem | https://github.com/openstack/nova/blob/872a823d9a02f31e266882bfb499673c51fb4075/nova/compute/manager.py#L4977 | |
| 14:36:51 | tssurya | yea it doesn't | |
| 14:36:57 | mriedem | it would record a failure b/c of NotImplementedError | |
| 14:37:02 | tssurya | yea | |
| 14:38:00 | tssurya | dansmith, mriedem: if you are both ok with the whole success and return thing I can do that | |
| 14:38:16 | tssurya | but if you feel its confusing for the users I can reraise and log and ignore raise | |
| 14:38:34 | dansmith | tssurya: I dunno, that I have a strong feeling about which is right or not | |
| 14:39:23 | tssurya | dansmith: ack | |
| 14:39:38 | mriedem | gmann: then why have these unit tests still? https://review.opendev.org/#/c/676275/4/novaclient/tests/unit/v2/test_shell.py | |
| 14:39:53 | mriedem | they are just asserting what the stub is doing https://review.opendev.org/#/c/676275/4/novaclient/tests/unit/v2/fakes.py | |
| 14:39:59 | mriedem | which might be wrong if there is a bug in the stub | |
| 14:41:11 | mriedem | idk either - the user is going to see a stop or start action with a compute_power_update event, they likely won't know what that event means. does the user know that someone powered on or off their bm instance out-of-band? | |
| 14:41:57 | mriedem | and since this isn't an action initiated by the user (at least directly in the compute api) but by ironic, is it just best effort and we shouldn't record a failed event if that operatoin lost a race? | |
| 14:42:32 | mriedem | if it's best effort and we don't consider losing a race a failure, then we shouldn't record the overall action as failed, but just "processed" | |
| 14:42:59 | dansmith | mriedem: yeah, that's a valid way to look at it I think | |
| 14:43:14 | dansmith | it's best effort, a hint so to speak, so failure isn't really failure | |
| 14:43:19 | tssurya | fair enough | |
| 14:43:36 | tssurya | and its easier that way for me :) | |
| 14:43:56 | dansmith | just put "it's easier for tssurya" in the spec and then you're good ;) | |
| 14:43:59 | tssurya | I'll stick to what we discussed yesterday then | |
| 14:44:06 | tssurya | dansmith: hehe | |
| 14:44:09 | mriedem | thinking about volume-extended events, we definitely want those to be failed so the user sees because it's initiated by the user resizing their volume in the cinder api | |
| 14:44:25 | tssurya | mriedem: this was a another doubt I had | |
| 14:44:33 | tssurya | that means we block the rest of the events in the queue no ? | |
| 14:44:42 | tssurya | because one volume-extend event broke ? | |
| 14:44:50 | tssurya | and you raise | |
| 14:45:02 | mriedem | we wouldn't get a volume-extended and power-update event in the same request | |
| 14:45:10 | mriedem | unless someone sends that manually | |
| 14:45:34 | mriedem | there might be a bug in the volume-extended event handling in compute that's not handling that properly, but that's a separate bug | |
| 14:45:50 | tssurya | mriedem: got it | |
| 14:46:51 | mriedem | tssurya: how much longer are you going to be around today? | |
| 14:46:54 | tssurya | mriedem: do you have a sample functional test that you have written for such races | |
| 14:46:56 | tssurya | ? | |
| 14:47:13 | tssurya | I couldn't find a way to hack the task_state in between my code | |
| 14:47:16 | tssurya | for the functional teste | |
| 14:47:35 | tssurya | mriedem: i'll be around only for 2 more hours | |
| 14:47:36 | mriedem | you'd have to stub/mock something in between to change the task_state after the compute manager sets it and the driver code gets it | |
| 14:47:50 | mriedem | i'm not sure we need the functional test to cover that though | |
| 14:48:04 | mriedem | that's pretty unit-y test level to me | |
| 14:48:10 | tssurya | but its ok, I'll be online and if you have comments will update them during vacation | |
| 14:48:45 | tssurya | mriedem: ok then I'll just do a unit test | |
| 14:49:16 | dansmith | yep, unit test makes sense to me | |
| 14:49:23 | dansmith | just to make sure we catch it and do the right thing | |
| 14:49:51 | tssurya | dansmith: yea I'll do that | |
| 14:51:19 | mriedem | tssurya: but i won't be around to review the updates until monday... and people are held up on this microversion, so i'll review it in detail now before you make another push | |
| 14:52:08 | tssurya | mriedem: ok sounds good | |
| 14:52:46 | tssurya | even if I am on vacation I do intend to work on this patch to get it in, so I hope to not block everyone | |
| 14:53:08 | tssurya | s/not block/unblock soon | |
| 14:55:36 | atmark | hello, I have mix setup of local disk computes and rbd backed computes. In nova.conf of local disk computes, I have images_type=raw and in the rbd backed compute image_type=rbd. I can spin up VMs fine in both setup however the when RESIZE a local disk VM, I get 'boot failed: not a bootable disk' from the VM's console | |
| 14:56:26 | atmark | resizing a RBD VM works. Resizing local disk VMs fails | |
| 15:12:56 | mriedem | tssurya: doesn't test_external_instance_event_power_update_unexpected_task_state already cover the race you were asking about testing? | |
| 15:13:15 | tssurya | mriedem: yes | |
| 15:13:23 | tssurya | that's the unit test I have | |
| 15:13:30 | mriedem | then we don't need a functional test wrinkle for that | |
| 15:13:37 | tssurya | I didn't have a functional test hence asked | |
| 15:13:39 | tssurya | ah ok | |
| 15:13:42 | tssurya | cool | |
| 15:14:57 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test heal port allocations in nova-next https://review.opendev.org/669879 | |
| 15:18:15 | mriedem | tssurya: dansmith: comments posted https://review.opendev.org/#/c/645611/ | |
| 15:18:24 | tssurya | thanks | |
| 15:20:20 | mriedem | if you run out of time, post what you have and i'll fix the rest and +2 | |
| 15:39:08 | openstackgerrit | Eric Fried proposed openstack/nova master: Introduces SDK to IronicDriver and uses for node.get https://review.opendev.org/642899 | |
| 15:39:09 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for node.list https://review.opendev.org/656027 | |
| 15:39:09 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for validating instance and node https://review.opendev.org/656028 | |
| 15:39:10 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for setting instance id https://review.opendev.org/659690 | |
| 15:39:10 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for add/remove instance info from node https://review.opendev.org/659691 | |
| 15:39:11 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 15:39:26 | efried | dustinc: Rebased and with requirements bump to sdk 0.34.0 ^ | |
| 15:39:48 | dustinc | thanks | |
| 15:39:49 | efried | that should fix `fields`. I'm hoping it also fixes the remaining patches | |
| 15:40:00 | dustinc | 🤞 | |
| 15:40:03 | efried | cause I also added a dep to the devstack patch | |
| 15:40:05 | efried | hmm | |
| 15:40:20 | efried | Will the second patch still observe the dep from the first? | |
| 15:42:09 | mriedem | yes | |
| 15:44:21 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Limit get_sdk_adapter to requested service type https://review.opendev.org/674678 | |
| 15:45:09 | efried | cool | |
| 15:48:20 | mriedem | i guess forbidden aggregates is never getting out of a runway slot | |
| 15:51:17 | mriedem | sean-k-mooney: if you can get a new patch set for https://review.opendev.org/#/c/671338/ today it would be good as well, since it was pretty close | |
| 15:51:45 | openstackgerrit | Mohammed Naser proposed openstack/nova master: config: remove deprecated checksum options https://review.opendev.org/676539 | |
| 15:51:46 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Remove nova.image.download.modules extension point https://review.opendev.org/676540 | |
| 15:51:46 | openstackgerrit | Mohammed Naser proposed openstack/nova master: requirements: move to using psycopg2-binary https://review.opendev.org/676684 | |
| 15:52:05 | sean-k-mooney | ok i was doing som sriov testing but ill priorities that and get it updated today | |
| 15:52:27 | sean-k-mooney | mriedem: ill ping you when its done | |
| 15:53:51 | openstackgerrit | Mohammed Naser proposed openstack/nova master: constraints: bump psycopg2 to 2.8 https://review.opendev.org/676725 | |
| 15:54:13 | mriedem | brinzhang: i'm going to move https://blueprints.launchpad.net/nova/+spec/support-specifying-az-when-restore-shelved-server into the open runway slot though you'll likely have to rebase it again once 2.76 is taken by the power-update change | |
| 15:59:30 | melwitt | tssurya: what do you think about multi-cell archive deleted rows skipping disabled cells? or would you rather expect it to try each cell and move on if can't connect instead | |
| 15:59:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Mock libvirt'y things globally https://review.opendev.org/676727 | |
| 16:00:04 | tssurya | melwitt: you mean down cells ? | |
| 16:00:17 | tssurya | disabled cells should be archived without distinction | |
| 16:00:17 | mriedem | disabled | |
| 16:00:36 | mriedem | wouldn't one want to disable a cell if it's down? | |
| 16:00:44 | mriedem | so the scheduler doesn't try to pull computes from it and timeout? | |
| 16:01:04 | tssurya | yea but its only a workaround for scheduling right ? | |
| 16:01:14 | tssurya | people could also have disabled the cells since its new | |
| 16:01:19 | melwitt | tssurya: ok. that's kinda what I was thinking, not to assume because disabled to skip. because otherwise you (operator) would have to enable any disabled cells in order to archive and I thought maybe that would be annoying | |
| 16:01:21 | tssurya | not necessarily down | |
| 16:01:33 | mriedem | melwitt: well, you could just archive them directly | |
| 16:01:37 | mriedem | like you do today | |