Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-14
19:36:04 dansmith I didn't -1 in case you had a good reason or excuse, but figured it was worth pointing out if you hadn't thought about it
19:49:08 mriedem replied
19:49:28 mriedem i've got some negative tests for things failing higher in the stack but not the scenario you're asking about
19:49:31 mriedem it wouldn't be hard to add though
19:53:03 sean-k-mooney mriedem: while that would be good i think its also i higher bar of testing then we have of similarly hairy failure modes.
19:54:01 sean-k-mooney i have wondered in the past if we should have a delete --force and change delete to raise a error with --force when the vm is in task state that are potentially hard to clean up
19:55:29 sean-k-mooney the issue with that is force deleteion is somehting i proably woudl want only my admin to do sicne they are the ones that woudl have to clean it up but as a use if i dont want a thing any more i should be able to say that
19:55:40 sean-k-mooney espcially since im being billed for it in most cases
20:00:17 sean-k-mooney which reminds me we shoud get around to merging this so we dont leak vifs when we delete the vm while its spwaning https://review.opendev.org/#/c/663382/
20:06:50 sean-k-mooney dansmith: melwitt could one of ye review ^ efried since you looked at that ages ago if you get a chance that would be cool too
20:07:47 efried sean-k-mooney: are you +1 at this point?
20:08:11 sean-k-mooney im rereviewing it now to determind what matt changed
20:08:24 sean-k-mooney but it think it ws just the unit tests
20:08:53 mriedem trying to catch up, was on a call
20:08:55 sean-k-mooney but it was rebased so doing eye ball diff manually
20:09:45 mriedem so (1) sean-k-mooney you're saying fault injection testing per dansmith's request is a higher bar of testing than we usually have, right? if so i agree but i also agree we should do that kind of testing while it's fresh in our minds so i don't have a problem adding it
20:10:22 mriedem and (2) https://review.opendev.org/#/c/663382/ the last time i looked at that there were questions about how you can even get into that situation - did anyone (stephen?) sort that out?
20:10:27 sean-k-mooney mriedem: yes im not saying we should not do it just we normally have not been that proactive
20:11:12 sean-k-mooney i just notice that mdbooth updated it and so maybe that is a ping to early
20:11:33 dansmith mriedem: sorry didn't see you replied.. fault injection would be a tempest-level test, I'm more looking for a test where you make the save() delete the instance and then fail like save would have to make sure you call all the cleanups you expect
20:12:34 mriedem dansmith: maybe fault injection is a bad word but that's essentially what i proposed in my reply, stub resize_claim on the dest to (through the api) delete the instance from the source, which will trigger a failure in conductor and rollback the instance from the target cell db and the test would assert the various cleanups
20:12:40 sean-k-mooney mriedem: it looks like nothing substantive has change so i need to parse all the comments again to confim if there was still any outstanding questions
20:12:56 dansmith mriedem: ack
20:14:11 mriedem sean-k-mooney: the reply from stephen is the last time i looked at it https://review.opendev.org/#/c/663382/4/nova/compute/manager.py@2506
20:14:15 mriedem "open to ideas here"
20:14:46 mriedem if there is a way to recreate that issue, then i'd say write a functional test to recreate it and then lay the fix on top to show it's fixed
20:15:09 mriedem which goes back to our historically bad level of testing which is why we have requests for fault injection and more complicated test scenarios in my cross-cell resize series above
20:15:23 mriedem *bad level of testing for non-happy path scenarios
20:15:49 sean-k-mooney mriedem: right
20:16:05 sean-k-mooney which is exeactly why this bug laid dorment for so long
20:16:41 sean-k-mooney i know how the issue happend in the customer env
20:16:59 sean-k-mooney it would be hard to recreate but maybe not imposible
20:18:11 sean-k-mooney ill try an think about how to recreate in a functionla test although mdbooth might also be looking into that
20:20:20 mriedem note that it's not against the rules to have a functional test to recreate a bug which stubs something to inject the failure mode,
20:20:35 mriedem we do that and still do it with functional tests to make sure the api / service / db interaction is what we'd expect
20:21:01 sean-k-mooney ya i was wondign if i could use a spy or mock to inject the fault at the correct time
20:21:10 mriedem e.g. the test would do something like start creating an instance, stub something to then call the api to delete the instance, and then go back to spawning which would fail and make sure we deallocaten networks properly
20:27:57 mriedem sean-k-mooney: i left a comment to that effect on the review
20:28:49 sean-k-mooney cool
20:29:02 mriedem dansmith: so on https://review.opendev.org/#/c/627890/60/nova/conductor/tasks/cross_cell_migrate.py@495 - the functional tests start later in the series and in at least one or two cases gibi has asked a "what happens if x fails" question like you are here and i've added a test later in the series before he proceeds,
20:29:08 mriedem are you ok with that here as well?
20:33:49 dansmith mriedem: with adding it later you mean?
20:36:12 mriedem dansmith: no i mean i'd add it to the series and you'd hold off on +2ing that change until it's up and shows things are cool
20:36:23 dansmith sure
20:36:23 dansmith that's what I mean
20:36:35 mriedem alright
20:36:47 mriedem that'll be my tomrrow
20:36:49 mriedem *tomorrow
20:44:34 sean-k-mooney i think its https://github.com/openstack/nova/blob/stable/queens/nova/compute/manager.py#L2164-L2167
20:46:18 sean-k-mooney if we get the delete after we call _update_instance_after_spawn but before we call instance.save(expected_task_state=task_states.SPAWNING)
20:46:29 sean-k-mooney we will raise the unexpected task state error.
20:46:52 sean-k-mooney i could simulated tat by mocking _update_instance_after_spawn to raise it but it would be a bit of a hack
20:47:23 sean-k-mooney or maybe better
20:47:47 sean-k-mooney have _update_instance_after_spawn inject the delete
20:49:05 sean-k-mooney effefctly the race only happen in the interaval betwen reciving the libvirt event to notify that the spawn has completed and updating the db with that state
20:56:09 openstackgerrit Merged openstack/nova master: Deprecate [api]auth_strategy and noauth2 https://review.opendev.org/687373
20:56:16 openstackgerrit Merged openstack/nova master: Leave brackets on Ceph IP addresses for libguestfs https://review.opendev.org/649405
20:57:06 openstackgerrit Merged openstack/nova stable/queens: Fix wrong assertions in unit tests https://review.opendev.org/677390
21:05:48 sean-k-mooney no that is not the point were it happens... we handel the exception in that case.
21:08:34 sean-k-mooney actully no it is the right spot we reraise teh expection to do the cleanup else where and that is where we dont catch it
21:26:13 mriedem so uh
21:26:24 mriedem let's just have a functional test that recreates the bug how about
21:26:35 mriedem since this spaghetti code is hard to reason about otherwise (as you've just demonstrated)
21:28:08 sean-k-mooney yes
21:28:17 sean-k-mooney i commented on the bug how to do it i think
21:28:47 sean-k-mooney mriedem: mdbooth siad he was going to add a funcitonl test to one of stephen pathces this morining
21:29:04 sean-k-mooney if it was not this one ill look into adding it later in the week
22:48:59 openstackgerrit Dustin Cowles proposed openstack/nova master: Provider Config File: Function to further validate and retrieve configs https://review.opendev.org/676029
22:48:59 openstackgerrit Dustin Cowles proposed openstack/nova master: Provider Config File: YAML file loading and schema validation https://review.opendev.org/673341
22:49:00 openstackgerrit Dustin Cowles proposed openstack/nova master: Provider Config File: Merge provider configs to provider tree https://review.opendev.org/676522
23:40:15 openstackgerrit Merged openstack/nova master: Add prep_snapshot_based_resize_at_dest compute method https://review.opendev.org/633293
23:54:54 openstackgerrit sean mooney proposed openstack/nova master: block rebuild when numa toplogy changed https://review.opendev.org/687957
#openstack-nova - 2019-10-15
01:04:51 openstackgerrit Arthur Dayne proposed openstack/nova master: libvirt:volume:Disallow AIO=native when no 'O_DIRECT' is available https://review.opendev.org/682772
02:03:16 openstackgerrit Brin Zhang proposed openstack/python-novaclient master: Add minor version [21] to test_versions https://review.opendev.org/688599
02:05:01 openstackgerrit Huachang Wang proposed openstack/nova master: cleanup to objects.fields https://review.opendev.org/688600
02:22:56 openstackgerrit Huachang Wang proposed openstack/nova master: Set instance CPU policy to 'share' when 'hw_cpu_policy==share' https://review.opendev.org/688603
02:54:19 openstackgerrit Dustin Cowles proposed openstack/nova master: Provider Config File: Function to further validate and retrieve configs https://review.opendev.org/676029
02:54:19 openstackgerrit Dustin Cowles proposed openstack/nova master: Provider Config File: YAML file loading and schema validation https://review.opendev.org/673341
02:54:20 openstackgerrit Dustin Cowles proposed openstack/nova master: Provider Config File: Merge provider configs to provider tree https://review.opendev.org/676522
05:57:31 openstackgerrit Brin Zhang proposed openstack/python-novaclient master: Microversion 2.80: Add user_id/project_id to migration-list API https://review.opendev.org/675023
06:58:52 openstackgerrit Brin Zhang proposed openstack/python-novaclient master: Add functional test for migration-list in v2.80 https://review.opendev.org/688635
07:35:55 gibi good morning nova
07:41:50 openstackgerrit Brin Zhang proposed openstack/python-novaclient master: Add functional test for migration-list in v2.80 https://review.opendev.org/688635
07:48:59 gibi stephenfin: just to be sure you also got the message I got a mail from the summit organizers that the project updates will not be recoreded. Instead they would like to get an etherpad with main points.
08:02:17 openstack Launchpad bug 1648501 in OpenStack Compute (nova) "providing different imageRef when using block_device_mapping (image -> volume) " [Low,Confirmed]
08:02:17 jkulik Hi, regarding https://bugs.launchpad.net/nova/+bug/1648501 I see a problem with code assuming instance.image_ref being None equals boot-from-volume as mentioned here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3371-L3372
08:03:46 jkulik Because even if imageRef is set to the same values as in the block-device-mapping, instance.image_ref will then be set even though it's boot-from-volume.
08:05:02 jkulik As the bug mentions, the cli already forbids providing imageRef and block-device-mapping at the same time. But the API accepts it. Shouldn't this be made consistent?
09:54:56 SonPham Hi. I'm working with nova and horizon. I traced code form Horizon on Button start-instance: Horizon call NovaClient: novaclient(request).servers.start(instance_id)
09:55:26 SonPham def start(self, server):
09:55:26 SonPham and I check python client server start is:
09:55:53 SonPham and i think it call to /nova/nova/copmute/apy.py / def start()
09:56:03 SonPham how it work?
10:05:15 openstackgerrit Stephen Finucane proposed openstack/nova master: zuul: Make functional job inherit from openstack parents https://review.opendev.org/688425
10:05:15 openstackgerrit Stephen Finucane proposed openstack/nova master: Stop testing Python 2 https://review.opendev.org/687954
10:05:15 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove unused 'nova-dsvm-base' job https://review.opendev.org/688389
10:05:59 gibi SonPham: the python client calls the nova-api service via HTTP (REST). The server create request is handed by https://github.com/openstack/nova/blob/63fb66e39a2590f00541f36d94e31372c2fe82ee/nova/api/openstack/compute/servers.py#L598
10:09:16 gibi jkulik: I agree it feels strange that you can provide an imageRef POST /servers but at that same time you are server is not booted from that image but booted from a volume with different content
10:09:46 gibi jkulik: so I think the nova-api can reject such situation
10:16:18 jkulik for the vmware driver in queens at least, we get a volume and an ephemeral disk, because it seems to only take into account instance.image_ref when creating the ephemeral one

Earlier   Later