| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-24 | |||
| 17:24:59 | mriedem | given there are 4 services involved with a reschedule i like having the functional test | |
| 17:25:48 | dansmith | yeah, just having a fake driver that fails the first spawn or something seems like less unit-test-esque interaction, but whatever | |
| 17:26:13 | mriedem | could do that too - would be simpler | |
| 17:26:18 | mriedem | later cleanup i suppose | |
| 17:26:31 | mriedem | or in this one, whatever | |
| 17:28:00 | mriedem | things get complicated with a special compute driver i think b/c you have to set that in config before starting the compute service you're using, which is done in setUp, unless you make it a standalone test class, or start a new 3rd compute and boot directly to that host | |
| 17:28:02 | dansmith | mriedem: and we're saying this belongs in ServerMovingTests because why? it's a reschedule? | |
| 17:28:22 | dansmith | sure, but it's more cleanerer I think | |
| 17:28:28 | mriedem | i assume he threw it there because there are other tests checking allocation stuff in there | |
| 17:28:34 | mriedem | but it's not a move, yeah | |
| 17:28:45 | mriedem | didn't think abou that | |
| 17:29:05 | dansmith | yeah we could probably rename that test class at this point to be TestAllTheAllocationThingsWeEffedUpKthx | |
| 17:29:09 | mriedem | so maybe we split that out to a separate test class with it's own fake driver in a follow up? | |
| 17:29:21 | dansmith | okay | |
| 17:32:41 | dansmith | okay +Wd with those comments | |
| 17:34:50 | mriedem | cool. i'm working on the regression test for the migrate + reschedule one, which is turning out to be non-trivial | |
| 17:35:32 | dansmith | awesome | |
| 17:35:45 | mriedem | just hard to poll for the failure | |
| 17:35:55 | mriedem | have to check action events or listen for notifications | |
| 17:36:04 | mriedem | ^ super usability for end api users when a resize / migrate fails | |
| 17:39:01 | dansmith | whatever, users love a challenge, amirite? | |
| 17:39:13 | mriedem | playing hard to get works in dating | |
| 17:39:16 | mriedem | why can't it work in software? | |
| 17:40:07 | artom | Because it only works for girls, and there are no girls in software | |
| 17:40:55 | mriedem | enjoy your twitter doom | |
| 17:41:06 | artom | I'm not on Twitter | |
| 17:43:10 | mriedem | ok ok, well, i'm sure mel's boot will find you | |
| 17:44:16 | artom | Mel is clearly a figment of our imagination, because of my previous assertion | |
| 17:46:16 | rabel | sean-k-mooney: it seems that "Intel PCI CI"s pci-test is not non-voting in https://review.openstack.org/#/c/494169/4 . is that correct? can you tell me how to rerun it? | |
| 17:46:33 | mriedem | rabel: it is non-voting | |
| 17:46:41 | mriedem | otherwise there would be a -1 next to 'Verified' | |
| 17:46:51 | mriedem | don't let the red FAILURE on the actual job status trick you | |
| 17:47:07 | rabel | mriedem: ok, thanks. i was confused by having (non-voting) on other tests, but not this one | |
| 17:47:32 | rabel | so no -1 means i'm fine? :) | |
| 17:47:34 | mriedem | ah i don't know how other 3rd party CIs do that | |
| 17:47:44 | mriedem | well, jenkins is the vote you care about | |
| 17:47:45 | mriedem | for CI | |
| 17:47:50 | mriedem | and the vmware CI | |
| 17:48:09 | mriedem | but the latter is on the fritz | |
| 17:48:24 | mriedem | cdent has to call down to larry in the basement to kick it | |
| 17:48:37 | rabel | :D | |
| 17:48:44 | openstackgerrit | Merged openstack/nova master: api-ref: fix key_name note formatting https://review.openstack.org/496718 | |
| 17:48:53 | artom | To be fair, if your patch actually touched on anything PCI, it might be a good idea to see why the Intel CI failed | |
| 17:48:59 | artom | But since it doesn't, you can ignore it | |
| 17:49:17 | openstackgerrit | Merged openstack/nova master: Remove VMware driver _get_vm_ref_from_uuid method https://review.openstack.org/444959 | |
| 17:49:18 | rabel | will it help to write vmware-recheck-patch from time to time if it does not run? | |
| 17:54:39 | openstackgerrit | Evgeny Antyshev proposed openstack/nova master: No scsi unit information in instance XML https://review.openstack.org/495756 | |
| 17:58:35 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM Driver: config drive https://review.openstack.org/409404 | |
| 18:04:11 | openstackgerrit | Merged openstack/nova master: Move common definition into common layer https://review.openstack.org/489491 | |
| 18:04:47 | openstackgerrit | Merged openstack/nova master: Remove RamFilter and DiskFilter in default filter https://review.openstack.org/492765 | |
| 18:05:28 | openstackgerrit | Octave Orgeron proposed openstack/nova master: Enables MySQL Cluster Support for Nova https://review.openstack.org/446643 | |
| 18:07:00 | mriedem | dansmith: per your comment in alex's change for another test, i think that's covered in https://review.openstack.org/#/c/470578/ | |
| 18:07:25 | dansmith | oh okay I hadn't seen that yeah | |
| 18:07:55 | mriedem | starting the compute runs update_available_resource which does the thing to remove the now deleted allocation | |
| 18:07:57 | mriedem | so yay | |
| 18:23:35 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional test for rescheduling during a migration https://review.openstack.org/497541 | |
| 18:24:01 | mriedem | dansmith et al ^ is the functional recreate test for the reschedule + migrate bug | |
| 18:31:28 | mriedem | ugh, you know what, don't we merge allocations for resize to same host in the scheduler | |
| 18:32:15 | mriedem | so i can't remove the allocations for the dest node blindly in prep_resize b/c dest node might == source node for resize to same host, but we also shouldn't leave the allocations as the doubled up ones | |
| 18:33:19 | dansmith | we don't? we have a test for that | |
| 18:33:58 | mriedem | this is where prep_resize fails on the dest host, before we ever confirm/revert | |
| 18:34:07 | mriedem | i think our existing resize to same host tests are all driven on confirm/revert | |
| 18:34:25 | dansmith | right but the scheduler is the thing that doubles | |
| 18:34:31 | mriedem | i know | |
| 18:34:32 | dansmith | that move_claim thingy | |
| 18:34:42 | mriedem | and the compute is what's cleaning up | |
| 18:35:03 | mriedem | in the before times, if this happened, the RT periodic would just overwrite the allocations for the instance based on what it's currently consuming | |
| 18:35:10 | mriedem | are we still doing that? | |
| 18:35:50 | dansmith | what is the thing you think we're not doing? handling the case where prep_resize fails? or the initial doubling before we get that far? | |
| 18:36:21 | mriedem | yeah so before we did the has_ocata_computes thing, the periodic would overwrite the doubled allocation using _update_usage_from_instance | |
| 18:36:26 | mriedem | self.reportclient.update_instance_allocation | |
| 18:36:33 | dansmith | right | |
| 18:36:36 | mriedem | if you don't have ocata computes, that won't correct this now | |
| 18:36:38 | dansmith | which won't happen in pike land | |
| 18:36:49 | mriedem | we're not handling the case that prep_resize fails | |
| 18:36:57 | mriedem | and cleaning up the allocation | |
| 18:36:59 | mriedem | that the scheduler created | |
| 18:37:01 | dansmith | yeah, so you're talking about the case where we've doubled things in the scheduler and don't undouble them if we fail in prep right? | |
| 18:37:07 | mriedem | yup | |
| 18:37:38 | mriedem | so maybe this falls under the same bug i reported for when live migration fails and we don't cleanup | |
| 18:37:56 | dansmith | I would like to point out that if we were doing the allocation thing in the conductor instead of the scheduler, we'd have this all in an auto-cleanup context manager that would roll back the doubling if we failed to kick off a thing | |
| 18:37:58 | mriedem | this is essentially the same kind of fix probably, a periodic checking for failed migrations and cleaning up allocations related to them | |
| 18:38:20 | dansmith | well, we should clean up allocations any time we have a solid failure and know where the instance remains, | |
| 18:38:30 | dansmith | and a failure in prep is that case, right? we know we didn't move anything | |
| 18:39:06 | mriedem | prep_resize is a cast from conductor so i'm not sure how that would auto-cleanup in this case | |
| 18:39:24 | dansmith | it's a cast from conductor to compute? | |
| 18:39:34 | mriedem | yeah | |
| 18:39:41 | dansmith | ah, yeah, I see | |
| 18:39:57 | mriedem | so, remove the dest node allocation when not resizing to same host is simple | |
| 18:40:00 | dansmith | that's legacy from when the api was doing it I think, but.. | |
| 18:40:22 | mriedem | the resize to same host cleanup is shittier, since we basically need to overwrite the allocation back to the original flavor | |
| 18:40:37 | mriedem | which is basically just doing our RT overwrite stuff again, but in a different place | |
| 18:40:38 | dansmith | well, | |
| 18:40:51 | dansmith | we can just subtract what the new flavor would have had in it right? | |
| 18:41:07 | dansmith | not just regenerate, but subtract the new_flavor from our allocation if it's same host | |
| 18:41:17 | dansmith | merge_resources() with new_flavor and -1 as the sign | |
| 18:41:25 | dansmith | that will avoid trampling on shared things | |
| 18:41:30 | mriedem | sure | |
| 18:42:00 | mriedem | god i should probably have a test for the resize to same host case then too... | |
| 18:42:08 | mriedem | and it's nearly 2pm here | |