| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-11 | |||
| 20:02:05 | dansmith | ooh | |
| 20:10:03 | mriedem | so this isn't backportable b/c of the version change, but in thinking about this more - persisting the is_bfv flag is good b/c for any new instances created after this, moving them also benefits from the flag being set | |
| 20:10:13 | mriedem | the only gap is existing instances that don't have is_bfv set in the request spec, as noted in the commit message | |
| 20:10:18 | mriedem | which we can easily detect and heal during a move | |
| 20:11:19 | mriedem | there are a few comments in the bug report about how once people got to ocata the allocations for volume-backed instances were fixed, and that's because the resource tracker had the is_bfv logic to not account for flavor.root_gb, | |
| 20:11:45 | mriedem | but then in pike we stopped creating allocations in the RT and based the allocations of the request spec, so we regressed that.... | |
| 20:12:03 | mriedem | so this (re)fixes that | |
| 20:12:13 | mriedem | but we'd have a gap in pike and queens | |
| 20:12:54 | dansmith | sorry, was looking at the review window | |
| 20:12:56 | dansmith | I just replied, | |
| 20:13:14 | dansmith | but basically I didn't even consider this as backportable since it changes what we allocate and stuff and thought it wasn't worth the risk | |
| 20:13:15 | mriedem | almost seems like we could do a hybrid of https://review.openstack.org/#/c/428481/ and your new patch which could be backportable | |
| 20:13:31 | dansmith | I'd rather just roll forward and heal | |
| 20:14:05 | mriedem | i get that, | |
| 20:14:18 | mriedem | but this is a pretty high flame count bug or whatever | |
| 20:14:30 | mriedem | so backportability seems worth at least thinking through | |
| 20:14:42 | dansmith | our downstream requirement for stuff like this is that you have to be able to yum update components in any order on any machine, and having things starting to claim differently because you updated one conductor and didn't do another until a month later | |
| 20:14:59 | dansmith | even still, the scheduler is the thing that needs to know, not conductor like you said, | |
| 20:15:06 | dansmith | so I'm not sure how we could do it without an object or rpc change | |
| 20:15:49 | dansmith | if this was a regression in the last release then I'd care more I guess, but this has _always_ been this way, which makes it nearly a feature | |
| 20:16:06 | mriedem | well, it was fixed in ocata and then regressed again in pike | |
| 20:16:19 | dansmith | eh? this has never worked that I know of | |
| 20:16:20 | mriedem | i think anyway | |
| 20:16:27 | dansmith | pretty sure not, | |
| 20:16:28 | mriedem | in ocata the computes would create the allocations right? | |
| 20:16:37 | dansmith | ah, but, it has always been broken in RT :) | |
| 20:16:48 | mriedem | the RT / report client code that created the allocations in ocata took bfv into account for DISK_GB | |
| 20:16:52 | dansmith | so before we were doing allocations wrong, we were doing RT claiming wrong and failing super late | |
| 20:16:53 | mriedem | RT == claim? | |
| 20:16:56 | mriedem | well, | |
| 20:16:58 | dansmith | I mean old school claims | |
| 20:17:02 | mriedem | if you dropped the DiskFilter you wouldn't hit the claim | |
| 20:17:06 | dansmith | pfft :) | |
| 20:17:09 | mriedem | so in ocata, if you're using placement and remove DiskFilter, | |
| 20:17:10 | mriedem | fixed | |
| 20:17:39 | dansmith | where did we not allocate for volume-backed disks in RT? | |
| 20:18:44 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L169 | |
| 20:18:53 | mriedem | this comment in the bug is a pretty good one https://bugs.launchpad.net/nova/+bug/1469179/comments/37 | |
| 20:18:55 | openstack | Launchpad bug 1469179 in OpenStack Compute (nova) "instance.root_gb should be 0 for volume-backed instances" [Medium,In progress] - Assigned to Dan Smith (danms) | |
| 20:19:02 | mriedem | "After disabling the RamFilter and DiskFilter, and instead relying on the Placement API for scheduling, most of the "scheduling" aspect of this problem has been addressed. I was able to drop a local patch to handle this root_gb!=0 for EBS volumes, and for most real-life use cases it is working. The placement API is not recording disk allocations for EBS volumes." | |
| 20:19:44 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L351 | |
| 20:20:07 | dansmith | so it kinda worked almost by accident for a release? | |
| 20:20:12 | mriedem | so in ocata we wouldn't report root_gb for bfv instance allocations, you could still fail scheduling due to the old school RT disk claim if DiskFilter was enabled | |
| 20:20:40 | mriedem | i agree with the 'by accident' part | |
| 20:21:05 | mriedem | and i don't think it was until at least pike that we said to disable the DiskFilter if you're using placement and remove it from the default enabled filters | |
| 20:21:33 | dansmith | right, I was thinking ocata still required that | |
| 20:21:34 | dansmith | well, | |
| 20:21:41 | dansmith | didn't recommend placement as a complete replacement | |
| 20:22:46 | mriedem | yeah dropped CPU/Ram/DiskFilters from default enabled filters in pike | |
| 20:23:26 | mriedem | we'd been saying since at least newton that placement was the eventual fix for this problem :) that's why we kept putting off melwitt's workaround / RT patch | |
| 20:23:35 | mriedem | and sure now it is | |
| 20:23:46 | dansmith | aye | |
| 20:24:12 | dansmith | well, to be fair, we could have done this in queens at least too | |
| 20:24:32 | dansmith | meaning, it could have been the fix already | |
| 20:24:36 | mriedem | sure | |
| 20:24:51 | mriedem | and people could hack in a point-of-scheduling time fix in conductor if they need it.. | |
| 20:27:32 | melwitt | does all of the "fixed" situations require that a flavor with root_gb=0 has been used? I'm reading through the comments trying to understand | |
| 20:27:49 | dansmith | no, the right fix does not | |
| 20:28:20 | melwitt | because I thought, as of ocata, if flavor has root_gb > 0 placement would filter it out of consideration and removing DiskFilter would not help you | |
| 20:28:30 | dansmith | the middle solution requires sending a hacked flavor with root=0 to the scheduler as the signal to avoid a real signal | |
| 20:29:08 | dansmith | right I think mriedem is saying that if you align all the planets on ocata, including set root=0, you can get it to kinda work if you don't sneeze near it | |
| 20:29:16 | melwitt | got it | |
| 20:29:48 | mriedem | we actually don't want people using root_gb=0 in flavors | |
| 20:29:52 | dansmith | right | |
| 20:29:53 | mriedem | there is a security issue for that | |
| 20:30:05 | dansmith | and it's broken because it requires using specific flavors for bfv | |
| 20:30:15 | mriedem | https://github.com/openstack/nova/commit/763fd62464e9a0753e061171cc1fd826055bbc01 | |
| 20:30:22 | melwitt | yeah, just wanted to make sure I understood what the definition of "fixed" was in the convo | |
| 20:30:30 | mriedem | i have to run, be back in a few | |
| 20:30:44 | mriedem | dansmith: let's add the release note and change related-bug to partial-bug (for my ocd) and then i'm +2 | |
| 20:30:53 | dansmith | ack | |
| 20:33:57 | dansmith | mriedem: I'll remind you I added related-bug for you :) | |
| 20:36:27 | dansmith | damn, you said "link" not specifically related | |
| 20:36:30 | dansmith | I' | |
| 20:36:33 | dansmith | I'll get you next time | |
| 20:39:40 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid requesting DISK_GB allocation for root_gb on BFV instances https://review.openstack.org/580720 | |
| 21:02:25 | mriedem | muwahahaha | |
| 21:08:15 | mriedem | dansmith: see my hedge in the release note | |
| 21:08:38 | mriedem | i wonder if we should be clear that if you're not using the FilterScheduler (and/or using the DiskFilter) this is still a problem | |
| 21:08:42 | dansmith | sure makes sense | |
| 21:08:52 | mriedem | but, CachingScheduler is deprecated and DiskFilter should be disabled if using FilterScheduler | |
| 21:10:43 | dansmith | how's this: | |
| 21:10:45 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid requesting DISK_GB allocation for root_gb on BFV instances https://review.openstack.org/580720 | |
| 21:11:17 | mriedem | lgtm | |
| 21:11:20 | mriedem | pew pew | |
| 21:12:43 | mriedem | at some point we should drop the ironic flavor migration stuff, since then we can drop the remaining allocation stuff in the report client for the RT | |
| 21:13:05 | mriedem | the only driver that reports allocations in the compute is the ironic driver | |
| 21:14:29 | mriedem | # TODO(johngarbutt) we should remove this once the resource class | |
| 21:14:29 | mriedem | # migration has been completed. | |
| 21:14:29 | mriedem | requires_allocation_refresh = True | |
| 21:14:47 | mriedem | what's our signal that the resource class migration has been completed? | |
| 21:15:05 | dansmith | time and I think it's passed now | |
| 21:15:27 | mriedem | we do have a nova-status check for the ironic instance embedded flavor migration | |
| 21:15:33 | mriedem | and the online data migration | |
| 21:15:35 | mriedem | online/offline | |
| 21:15:55 | mriedem | # *always* return the custom resource class that represents the | |
| 21:15:55 | mriedem | and | |
| 21:15:55 | mriedem | # TODO(jaypipes): Completely remove the reporting of VCPU, MEMORY_MB, | |
| 21:15:55 | mriedem | # and DISK_GB resource classes in early Queens when Ironic nodes will | |
| 21:15:55 | mriedem | # baremetal node class in an atomic, singular unit. | |