| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-20 | |||
| 16:59:04 | mriedem | 3rd bullet under #2 | |
| 16:59:50 | mriedem | if i'm reading that correctly, it's saying bring down the control plane services, with nova-api last (ok), but then the next bullet is talking about rolling through the computes | |
| 17:00:10 | dansmith | mriedem: "This results in a condition where both old (N) and new (N+1) nova-compute services co-exist for a certain time period." | |
| 17:00:44 | dansmith | that's all that promises | |
| 17:00:53 | dansmith | but, if it's not clear, let's be clear about it | |
| 17:01:30 | dansmith | but I really don't want to get into the habit of trying to call out problematic releases more than others so that people think if it's not called out for a given release then they should expect it to work | |
| 17:01:53 | mriedem | that's fine, | |
| 17:02:11 | mriedem | but then i need to understand if what some of the api compat code was doing via the compute rpcapi client was wrong, so we don't continue that pattern | |
| 17:02:18 | mriedem | because it's in several different compute rpcapi methods | |
| 17:02:53 | dansmith | mriedem: I don't think it's wrong, | |
| 17:03:03 | dansmith | mriedem: at some point the api needs to assume it can make a call and it will work | |
| 17:03:31 | mriedem | ok, so that means, any of these compat checks in compute rpcapi made *before* queens are OK to remove | |
| 17:03:33 | dansmith | if the operator has set up something wrong or created some unsupportable scenario, a 5xx error code from the api is correct rihgt? | |
| 17:03:54 | dansmith | I think any that were made *in* queens are good to remove | |
| 17:04:08 | dansmith | we can't detect or honor any of them made for scenarios before 5.0 | |
| 17:04:22 | dansmith | (in or before, in case it's not clear) | |
| 17:04:44 | mriedem | yeah, so then what the api could gracefully handle in queens, it cannot in rocky | |
| 17:04:54 | dansmith | right | |
| 17:04:58 | mriedem | but, | |
| 17:05:03 | cfriesen | In stable/pike if we do a rebuild-to-same-node with a new image, it results in ComputeManager.rebuild_instance() being called with "scheduled_node=<hostname>" and "recreate=False". This results in a new Claim, which seems wrong since we're not changing the flavor and that claim could fail if the compute node is already full. | |
| 17:05:05 | mriedem | we don't explicitly say the api can do that | |
| 17:05:19 | mriedem | is what you're saying, right? | |
| 17:05:47 | mriedem | you can have N control services and N and N-1 compute services, fin. | |
| 17:05:55 | dansmith | mriedem: we've said the api can be one rev newer than the computes, so our api code needs to handle the case where some computes don't support a function because they're N-1, and we do | |
| 17:06:01 | mriedem | no guarantees that the API will support the N-1 computes | |
| 17:06:08 | dansmith | as of this moment, api code for rocky can't have any computes that don't support things | |
| 17:06:44 | dansmith | no, we guarantee the api will support N-1 computes | |
| 17:06:45 | dansmith | but not N-2 | |
| 17:06:46 | dansmith | and at the moment, N-1 computes can do all the things of the N ones, so no checks are needed | |
| 17:12:22 | mriedem | cfriesen: that's a regression | |
| 17:12:25 | mriedem | yet another one | |
| 17:12:45 | dansmith | gawd | |
| 17:13:27 | mriedem | i don't know why this isn't just "if recreate: | |
| 17:13:28 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2866 | |
| 17:13:36 | mriedem | recreate == evacuate == always do a claim | |
| 17:14:15 | mriedem | this is stale: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2854 | |
| 17:14:25 | mriedem | since the new crap to consult the scheduler if the image changes | |
| 17:15:30 | mriedem | cfriesen: open a bug | |
| 17:16:03 | mriedem | it's a regression from I11746d1ea996a0f18b7c54b4c9c21df58cc4714b | |
| 17:16:20 | cfriesen | mriedem: will do | |
| 17:20:40 | openstack | Launchpad bug 1750618 in OpenStack Compute (nova) "rebuild to same host with a different image results in erroneously doing a Claim" [Undecided,New] | |
| 17:20:40 | cfriesen | mriedem: https://bugs.launchpad.net/nova/+bug/1750618 | |
| 17:21:00 | mriedem | cfriesen: are you going to post a patch? | |
| 17:21:05 | mriedem | i think it's just changing that one line | |
| 17:22:52 | cfriesen | just change it to only check "recreate" and ignore "scheduled_node"? yeah, I can post that | |
| 17:23:07 | cfriesen | I guess we'd need a new unit test? | |
| 17:23:29 | cfriesen | or a tweak to one of the existing ones to check if we're doing a real claim | |
| 17:23:37 | mriedem | i guess, there might already be one that has to change, | |
| 17:23:45 | mriedem | the problem here isn't in unit tests, | |
| 17:24:06 | mriedem | the problem is the fact that rebuild/evacuate is so tightly coupled between api, conductor and compute, and what the assumptions are between all of them about what the other is doing | |
| 17:24:21 | cfriesen | fair enough | |
| 17:24:22 | mriedem | so changing anything in one breaks the others | |
| 17:24:43 | mriedem | a solid test would be a functional rebuild test with a new image that asserts that we never do a claim | |
| 17:24:54 | mriedem | i know we have functional tests that do a rebuild with a new image, | |
| 17:25:04 | mriedem | but it's using the fake driver so likely the claim stuff was never a problem | |
| 17:25:15 | mriedem | there might be a way to poison the claim such that if it's called, it will fail | |
| 17:25:35 | openstackgerrit | Merged openstack/nova master: Check quota before creating volume snapshots https://review.openstack.org/520158 | |
| 17:27:43 | mriedem | cfriesen: functional test 'test_rebuild_with_new_image' should probably be tweaked | |
| 17:27:56 | mriedem | such that if nova.compute.resource_tracker.ResourceTracker.rebuild_claim is called, it blows up | |
| 17:28:53 | cfriesen | will take a look. | |
| 17:30:13 | mriedem | bauzas: melwitt was +2 on https://review.openstack.org/#/c/545132/ before i updated the commit message, can you just proxy her +2 and approve so we can do backports? | |
| 17:33:15 | dansmith | jaypipes: cdent which of you was going to work on the placement api change to allow passing aggregates? | |
| 17:33:40 | cfriesen | mriedem: for the rebuild-on-same-host-with-different-image case, if it runs through the scheduler could it fail due to placement if there isn't room for another one of itself on its current compute node? | |
| 17:35:10 | mriedem | cfriesen: we don't claim in that case | |
| 17:35:27 | cdent | dansmith: I probably _can_ do it, but I haven't made plans to do it | |
| 17:35:53 | mriedem | although i guess it's possible that placement could return 0 allocation candidates and we'd get a novalidhost | |
| 17:36:01 | cfriesen | mriedem: yeah, that's what I was thinking | |
| 17:36:01 | dansmith | cdent: _would_ you do it? :) | |
| 17:36:32 | cfriesen | mriedem: we need to validate the image properties of the new image against the current host though | |
| 17:37:13 | cdent | dansmith: I'm on the phone, so don't want to make promises, but if it needs someone it can probalby be me | |
| 17:37:56 | edleafe | dansmith: cdent: I may also have bandwidth for that | |
| 17:38:20 | dansmith | edleafe: that'd be cool | |
| 17:40:19 | mriedem | cfriesen: as far as i can tell, we don't do any manipulation of the request spec for rebuild with a new image such that we try to fake out placement to not try and double allocate the vcpu/memory_mb/disk_gb | |
| 17:40:48 | mriedem | we do tell the scheduler exactly which host/node we want, and we tell some filters to not run on it again, | |
| 17:41:10 | mriedem | but if placement doesn't return the source host then we'll have novalidhost and we can't rebuild with the new image | |
| 17:42:52 | mriedem | if placement does return the node we want, we don't double the allocations in placement, but we do attempt a claim on the compute as you found out | |
| 17:43:24 | mriedem | if the node is full, we likely won't even get to the compute to attempt the faulty claim... | |
| 17:43:37 | mriedem | b/c placement would have filtered it out | |
| 17:44:30 | cfriesen | before placement did the "_nova_check_type" flag tell us to skip over the resource filters? | |
| 17:44:34 | cdent | dansmith, edleafe: sorry wasn't really paying attention, which thing is this that needs a doer? | |
| 17:44:58 | mriedem | cfriesen: that flag was added long after placement was used in the scheduler, | |
| 17:45:07 | cdent | but am off phone now | |
| 17:45:08 | cfriesen | ah, okay | |
| 17:45:09 | mriedem | but yes that flag is used to tell which scheduler filters to run, and to not make a claim in placement | |
| 17:45:19 | mriedem | but it doesn't say 'don't use placement at all' | |
| 17:45:39 | cfriesen | open another bug? | |
| 17:45:50 | mriedem | request_is_rebuild could probably be used to completely bypass placement and just pull the compute node record from the db | |
| 17:46:05 | mriedem | cfriesen: idk | |
| 17:46:05 | dansmith | cdent: the member_of aggregate add to allocation_candidates api | |
| 17:46:06 | jaypipes | doude: is this something that only recently started failing? I'm struggling to pinpoint when the privsep (which from what I can tell has used msgpack for a while) started failing | |
| 17:46:33 | mriedem | cfriesen: after working on and breaking and regressing and trying to fix this rebuild with new image + scheduler thing for like the last 4 months, i don't really have any stomach for it anymore | |
| 17:47:12 | mriedem | cfriesen: if you want to report a bug just so it's tracked as a known issue, sure | |
| 17:47:19 | cdent | dansmith: thanks. edleafe and I will wrestle for it in dublin | |
| 17:47:51 | dansmith | jaypipes: it'd be after mikal privsep'd the libvirt driver recently | |
| 17:47:56 | dansmith | cdent: rock on | |
| 17:48:00 | cfriesen | mriedem: yeah, that's what I was thinking. we have a customer that'll probably end up complaining about it in a year or so when they get to pike | |
| 17:48:44 | dansmith | jaypipes: I'm sure we have no real tests for that code and that driver, so it popped up when we released queens and someone tried it | |
| 17:49:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Drop API compat handling for old compute error cases https://review.openstack.org/546256 | |
| 17:49:50 | mriedem | cfriesen: it's not just pike | |
| 17:50:02 | mriedem | cfriesen: that original regression was for a cve, | |