| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-30 | |||
| 11:38:12 | sean-k-mooney | i still have some todos and self notes to clean up but its mostly complete | |
| 11:38:13 | alex_xu | sean-k-mooney: nice, yea, will waiting for the gerrit | |
| 11:58:51 | openstackgerrit | Hamdy Khader proposed openstack/os-vif master: OVS DPDK port representors support https://review.opendev.org/658786 | |
| 12:17:34 | openstackgerrit | Lee Yarwood proposed openstack/nova master: block_device: Optionally recreate attachments when refreshing connection_info https://review.opendev.org/579004 | |
| 12:55:36 | efried | gibi: reading... | |
| 12:58:09 | efried | gibi: This second use case... | |
| 12:59:07 | efried | So if I had a pre-Stein instance with bandwidth request, it didn't have any bandwidth allocated in placement. Once this is implemented, I can fix up that instance, giving it appropriate allocations, by migrating it. | |
| 12:59:09 | efried | do I have that right? | |
| 13:06:13 | sean-k-mooney | presumable a rebuild could fix it too? | |
| 13:06:41 | sean-k-mooney | but a migration or resize would be the more general case | |
| 13:07:28 | efried | sean-k-mooney: This spec gel for you? | |
| 13:07:34 | efried | https://review.opendev.org/#/c/652608/ | |
| 13:07:46 | efried | you had some earlier reviews, but no votes | |
| 13:08:12 | sean-k-mooney | i kind of lost track of it. but ill review it now | |
| 13:08:21 | sean-k-mooney | at this point its proably fine | |
| 13:09:28 | efried | thanks | |
| 13:09:33 | gibi | efried: yes, migration will heal the allocation on the target host | |
| 13:09:41 | efried | thanks | |
| 13:10:01 | gibi | efried: sometimes there is no way to heal the allocation locally | |
| 13:10:32 | efried | The spec looks fine to me, though I only superficially understand some of the mechanics involved, so I'll wait for sean-k-mooney's nod, then I'll approve. | |
| 13:10:36 | gibi | efried: e.g. resource is overallocated as there was no nova support to keep it in sync | |
| 13:10:48 | efried | right, that makes sense. So the CLI will fail? | |
| 13:11:11 | gibi | efried: yes the heal_allocation CLI could fail due to overallocation | |
| 13:11:31 | gibi | as placement would reject the new allocation being created | |
| 13:11:43 | efried | yup, I get it. | |
| 13:12:15 | efried | And the overallocation was possible because nobody anywhere was tracking bw quantities | |
| 13:12:21 | shilpasd | efried: Hi, need discussion regarding 'forbidden aggregate specs' | |
| 13:12:26 | openstackgerrit | Merged openstack/nova stable/rocky: Fix retry of instance_update_and_get_original https://review.opendev.org/659318 | |
| 13:12:31 | efried | shilpasd: Sure, what's up? | |
| 13:12:35 | shilpasd | here at https://review.opendev.org/#/c/609960/8/specs/train/approved/placement-req-filter-forbidden-aggregates.rst@203 | |
| 13:12:37 | efried | shilpasd: Are you implementing it? | |
| 13:12:49 | shilpasd | yes | |
| 13:12:50 | gibi | efried: yes, there was no resource tracking | |
| 13:13:33 | shilpasd | efried: We have added code to sync traits set on aggregates for command "nova-manage placement sync_aggregates". | |
| 13:14:24 | efried | okay | |
| 13:15:06 | shilpasd | efried: further observed that 'syncing of traits set on aggregate is required for the other scenarios' like aggregate-create/delete/aggregate-remove-host/aggregate-set-metadata | |
| 13:15:18 | efried | yes, that makes sense to me. | |
| 13:15:52 | efried | though I'm not sure what we should do on delete or remove-host ... | |
| 13:16:52 | efried | perhaps we need a --flush-traits option of some sort, so the admin can choose. | |
| 13:17:09 | shilpasd | efried: for delete, will remove entry from placement.placement_aggregates and for remove-host will remove host association from aggragte | |
| 13:17:43 | efried | right, but the question is whether you should remove the aggregate's listed traits from the compute node resource providers in that aggregate | |
| 13:18:11 | efried | answer is probably: "it depends" :\ | |
| 13:18:45 | sean-k-mooney | efried: gibi three minor nits but yep that looks good to me | |
| 13:19:33 | efried | shilpasd: This would probably be a good topic for the ML. | |
| 13:19:54 | efried | gibi: I +2ed. The spelling nits probably aren't worth fixing, want me to merge? | |
| 13:21:17 | sean-k-mooney | i would eihter fix them and fast approve or just merge it as the content is correct modulo a few typos | |
| 13:22:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add heal_instance_mappings command https://review.opendev.org/655908 | |
| 13:22:50 | gibi | efried: merge it please | |
| 13:22:58 | gibi | thanks | |
| 13:23:06 | efried | gibi: done | |
| 13:23:14 | gibi | awesome | |
| 13:23:26 | efried | shilpasd: I suppose another option is to namespace the traits to be used for this feature. | |
| 13:24:46 | efried | shilpasd: e.g. if trait name contains '_AGGREGATE_ISOLATION_', automatically remove it when the agg is deleted or the host is removed. (Unless of course the host belongs to *another* aggregate which still has that trait...) | |
| 13:26:13 | efried | gibi: assume this is for train, yah? | |
| 13:26:23 | efried | (gibi: I'm doing the bp paperwork) | |
| 13:26:24 | gibi | efried: yes, it is for train | |
| 13:26:34 | efried | k | |
| 13:26:56 | gibi | I have couple of patches up already but more work is needed there which i will do in the coming days weeks | |
| 13:29:07 | efried | gibi: are the patches tracked against this bp? The "Implementation" is not showing as started. | |
| 13:29:25 | gibi | efried: I guess the bp rename did not helped | |
| 13:29:25 | shilpasd | efried: aggregate can have any trait and that we need to be sync with placement, irrespective of namespace, right? | |
| 13:29:30 | gibi | efried: let me fix it | |
| 13:29:39 | efried | (I could go look, but I'm being lazy, aka doing a bunch of other things at the same time) | |
| 13:30:08 | efried | shilpasd: When you create an aggregate or add a host, I would say yes, you should add all the traits. | |
| 13:30:20 | efried | shilpasd: I'm just trying to come up with a way to handle *deletion*. | |
| 13:30:33 | shilpasd | efried: understood, okay | |
| 13:30:53 | efried | shilpasd: For example, if you're creating an aggregate to isolate your hosts that have HW_CPU_X86_AVX, it makes zero sense to remove that trait if you destroy that aggregate. | |
| 13:30:53 | gibi | efried: I guess the implementation status will be updated if I push a new patch up targeting the renamed bp | |
| 13:31:10 | efried | gibi: yeah, it's totally no big deal, I really don't care, just happened to notice. | |
| 13:31:14 | gibi | efried: but the patches are here https://review.opendev.org/#/q/topic:bp/support-move-ops-with-qos-ports+(status:open+OR+status:merged) | |
| 13:31:19 | gibi | efried: ack | |
| 13:31:34 | efried | shilpasd: And in fact that trait would be reasserted on the host the next time the periodics run | |
| 13:32:14 | efried | shilpasd: ...but that is definitely not a behavior we should rely on to "heal" such traits. | |
| 13:33:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Change InstanceFaultRollback handling in _error_out_instance_on_exception https://review.opendev.org/633212 | |
| 13:33:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Raise InstanceFaultRollback for UnableToMigrateToSelf from _prep_resize https://review.opendev.org/633227 | |
| 13:35:17 | shilpasd | efried: ML is good suggestion to brain storm this, will generate the same, thanks | |
| 13:35:42 | openstackgerrit | Merged openstack/nova-specs master: Server move operations with ports having resource request https://review.opendev.org/652608 | |
| 13:36:10 | efried | shilpasd: Cool. To summarize the alternatives we've come up with so far, on delete/remove type operations, we could: | |
| 13:36:10 | efried | - Leave all traits alone. If they need to be removed, it would have to be manually via a separate step. | |
| 13:36:10 | efried | - Support a new option so the caller can dictate whether the operation should remove the traits. (This is all-or-none.) | |
| 13:36:10 | efried | - Define a "namespace" - a trait substring - and remove only traits in that namespace. | |
| 13:36:38 | efried | shilpasd: ...but on create/add type operations, I think we should always add the traits to the hosts. | |
| 13:45:12 | shilpasd | efried: clear, thanks | |
| 13:55:37 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Fix the server group "policy" field type in api-ref https://review.opendev.org/662224 | |
| 13:58:23 | mriedem | gibi: on https://review.opendev.org/#/c/512623/ i guess you re-ordered the series so the functional test, which depends on the fake driver global conf changes, could just be on master and the bug fix can be backported, is that right? | |
| 13:58:55 | gibi | mriedem: yes. lyarwood convinced me that it is better that way | |
| 13:59:20 | gibi | mriedem: this means no functional re-create and functional coverage on stable branches | |
| 13:59:27 | mriedem | i was hoping we could get the functional test backported as well so that's why i was pushing on slimming down the fake driver stuff, but alas i guess it wasn't possible | |
| 13:59:41 | mriedem | if we're good on master i think that's sufficient | |
| 13:59:50 | gibi | mriedem: it was hairy due to the fake driver stuff | |
| 13:59:56 | mriedem | that's a lot better than a lot of bug fixes we've had in the past which are just unit test coverage | |
| 14:00:06 | efried | nova meeting now | |
| 14:00:34 | mriedem | speaking of functional regression recreate tests https://review.opendev.org/#/c/661822/ | |
| 14:00:42 | mriedem | i could use some eyes on that since it's going all the way back to ocata | |
| 14:01:02 | gibi | mriedem: added it to my queue | |
| 14:01:12 | mriedem | thanks | |
| 14:02:30 | gibi | mriedem: btw, I'm reading your cross-cell resize review guide and I have to tell it helps me a lot. thanks for writing that up | |
| 14:03:04 | mriedem | gibi: thanks, i was about to ask if anyone had actually read it | |
| 14:03:57 | efried | mriedem: I fully intend to, if I ever get around to reviewing that series. | |
| 14:04:16 | efried | I've been kinda hoping others would have approved it by the time I got around to it. | |