| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-20 | |||
| 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, | |
| 17:50:06 | mriedem | that went back to newton upstream, | |
| 17:50:08 | mriedem | and further downstream | |
| 17:50:14 | mriedem | depending on how long you support old relases | |
| 17:50:16 | mriedem | *releases | |
| 17:50:19 | mriedem | it's the cve that keeps on giving | |
| 17:50:32 | mriedem | i guess the only difference with pike would be placement.. | |
| 17:50:36 | mriedem | in the scheduler i mean | |
| 17:52:16 | openstackgerrit | Chris Dent proposed openstack/nova-specs master: Provide error codes for placement API https://review.openstack.org/418393 | |
| 17:53:15 | cfriesen | mriedem: right, just saw your comment. will have to see if we've backported the bug. | |
| 17:53:34 | mriedem | i'm going to go eat my feelings | |
| 17:54:02 | sean-k-mooney | anyone know if there is documentation on what constitutes a minimal legal virt driver implementation? | |
| 17:54:29 | penick | As well you should. | |
| 17:55:05 | openstack | Launchpad bug 1750623 in OpenStack Compute (nova) "rebuild to same host with different image shouldn't check with placement" [Undecided,New] | |
| 17:55:05 | cfriesen | mriedem: just for info, the placement issue is https://bugs.launchpad.net/nova/+bug/1750623 | |
| 17:56:00 | sean-k-mooney | cfriesen: rebuilds on the same host are disabled by default correct so that bug will only be present if they are enabled? | |
| 17:57:42 | cfriesen | sean-k-mooney: are you thinking resize? | |
| 17:57:51 | sean-k-mooney | cfriesen: yes i am | |
| 17:58:09 | sean-k-mooney | specifcially https://docs.openstack.org/nova/queens/configuration/config.html#DEFAULT.allow_resize_to_same_host | |
| 17:58:20 | sean-k-mooney | but i guess there is not the same option for rebuild | |
| 17:58:41 | cfriesen | "rebuild" is by definition on same host, otherwise it's an evacuate | |
| 17:59:14 | sean-k-mooney | cfriesen: evacuate is a livemigration but yes rebild make sense on the same host as less work to do | |
| 17:59:35 | sean-k-mooney | a rebuild to another host would still be valid | |
| 18:00:03 | cfriesen | "nova evacuate" is not a migration, the host is already dead. really it should be called "resuscitate" | |
| 18:00:38 | sean-k-mooney | cfriesen: no you can evacuate hosts that are not dead for maintneance | |
| 18:00:53 | sean-k-mooney | at least via horizon | |
| 18:02:06 | cfriesen | sean-k-mooney: that's not the nova evacuate API though. | |
| 18:02:59 | cfriesen | sean-k-mooney: that's "nova host-evacuate" or "nova host-evacuate-live", which is done on the client side just to confuse things. | |
| 18:03:07 | sean-k-mooney | cfriesen: are you sure because i was pretty sure evacuate was ment to livemigrate off all instances on a singel host | |
| 18:03:12 | dansmith | sean-k-mooney: yes | |
| 18:03:25 | dansmith | sean-k-mooney: http://www.danplanet.com/blog/2016/03/03/evacuate-in-nova-one-command-to-confuse-us-all/ | |
| 18:04:07 | cfriesen | so dan, when do we rename evacuate to "resuscitate"? | |
| 18:04:10 | dansmith | if horizon has an evacuate button, I don't know what it does, but I would have assumed it is actually evacuate | |
| 18:04:24 | sean-k-mooney | dansmith: looks like the have renamed the horizon option to migrate host | |
| 18:04:40 | dansmith | cfriesen: the complication is that if you have instances on shared storage, evacuate is not resuscitate | |
| 18:04:42 | dansmith | or recreate | |
| 18:05:00 | dansmith | cfriesen: evacuate is the worst possible name, but changing it also brings confusion, IMHO | |
| 18:06:39 | cfriesen | dansmith: we're bringing instances back from the dead, so I think resuscitate works. if they've got shared storage they keep what they had, if not they get new rootfs | |
| 18:07:02 | sean-k-mooney | dansmith: so the evacuate option in old horizon now migrate host is only available when you mark a host as down but that does not mean nova-compute and all the vms on that host are dead. it just means the api introduces for the opnfv guest to aministativly set the host down has been called | |