| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-24 | |||
| 16:05:58 | cdent | \o/ | |
| 16:38:58 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove allocation when booting instance rescheduled or aborted https://review.openstack.org/496995 | |
| 17:16:09 | mriedem | blarg we only show faults if the instance is in ERROR or DELETED status | |
| 17:16:55 | mriedem | and when a resize/migrate fails on the dest during _prep_resize we don't set the instance to ERROR state | |
| 17:17:02 | mriedem | which i guess makes sense since it's fine on the source | |
| 17:17:17 | mriedem | but you'd wonder why the thing didn't resize or move | |
| 17:17:36 | dansmith | mriedem: I'm returning to that review for like the fifth time | |
| 17:17:45 | dansmith | need to turn off my email and chat or something | |
| 17:17:46 | mriedem | this time it's personal | |
| 17:17:51 | dansmith | hah | |
| 17:18:04 | dansmith | Review The Patch 5: Do or Die | |
| 17:20:52 | dansmith | mriedem: hmm, yeah, I guess right now if we reschedule the scheduler would actually create a doubled move allocation huh? | |
| 17:21:06 | mriedem | tripled | |
| 17:21:21 | mriedem | i think | |
| 17:21:37 | mriedem | the point of the move / double stuff in the scheduler is to not overwrite the existing allocations | |
| 17:21:37 | dansmith | well, N'd for N retries right? | |
| 17:21:40 | mriedem | yup | |
| 17:21:43 | dansmith | yeah | |
| 17:21:48 | mriedem | quintupled? | |
| 17:21:59 | dansmith | we're awesome | |
| 17:22:30 | mriedem | yeah so i was mostly worried that alex's patch was doing things in a too generic way, but i've since talked myself out of that | |
| 17:22:37 | mriedem | but left the detailed comments in there | |
| 17:22:41 | dansmith | so, this functional test has a mock in it, which is kinda contrary to the goal here right? | |
| 17:23:12 | mriedem | the mock is just to force the build failure on the source host and trigger a reschedule - i suppose you could do that by saying, boot on host A which we know won't fit a flavor or something so the claim fails | |
| 17:23:25 | dansmith | or just not make it a functional test | |
| 17:23:30 | mriedem | but you need to get by the scheduler and to the compute | |
| 17:23:45 | mriedem | we have a similar functional regression test that tests reschedule similar to this | |
| 17:23:49 | mriedem | similarly | |
| 17:24:00 | dansmith | meaning it uses a mock? | |
| 17:24:10 | mriedem | a fixture | |
| 17:24:11 | mriedem | but yeah | |
| 17:24:25 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1671648.py#L90 | |
| 17:24:40 | dansmith | okay I guess I can't keep track of our intentions here | |
| 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 | |