| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-20 | |||
| 13:20:47 | mriedem | 1. rather than cram it all into that single create() method, it would be nice to instead continue the pattern we have in here https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/helpers.py#L55 | |
| 13:21:07 | mriedem | 2. this series isn't moving the view builder code from the extensions into the actual server ViewBuilder, which is something we should do | |
| 13:21:43 | mriedem | maybe the goal for rocky is to just remove the server_create extension method and then in stein we refactor/cleanup the create() controller method to move things into https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/helpers.py#L55 and also move stuff into the view builder? | |
| 13:22:12 | mriedem | personally it's the view builder that bothers me the most, because i have to hunt all over the extensions to find what puts something into the server response body | |
| 13:22:45 | mriedem | and the view builder part is going to become very important for tssurya's change for handling a down cell | |
| 13:27:53 | mriedem | i see in https://review.openstack.org/#/c/583882/ it says "Remaining part in this BP is to merge the response extensions which is last part of this work." - is that the view builder part? if so, then i guess we can just merge what you have in rocky and do the view builder (and move stuff out of create() into helpers) in stein | |
| 13:37:07 | openstackgerrit | Andrey Volkov proposed openstack/nova master: Docs: Add more colors to Nova system architecture https://review.openstack.org/584345 | |
| 14:03:59 | dansmith | finucannot: you see that the second patch in your series is failing on pep8 right? | |
| 14:04:16 | finucannot | superdan: I did not but I'll fix now | |
| 14:19:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Start populating NUMACell.network_metadata field https://review.openstack.org/564441 | |
| 14:19:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add functional tests for numa-aware-vswitches https://review.openstack.org/583288 | |
| 14:19:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | |
| 14:19:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Assorted cleanups from numa-aware-vswitches series https://review.openstack.org/582651 | |
| 14:21:22 | smcginnis | melwitt: Morning. Do you take care of osc-placement releases? Or is that delegated to someone else? | |
| 14:21:45 | melwitt | smcginnis: I usually do, yes | |
| 14:23:05 | melwitt | smcginnis: I had thought osc-placement would go along with client library releases since it's a plugin for a client, does it actually count as a non-client library? | |
| 14:23:33 | smcginnis | melwitt: Ah - right now it is flagged as a library. | |
| 14:23:57 | melwitt | oh :( | |
| 14:24:56 | mriedem | gmann: johnthetubaguy: am i remembering correctly that there was discussion about deprecating all of these dumb legacy server extension policy rules for things like whether or not we should 'config_drive' in the server response? those are all required in the response in our docs and tempest schema validation testing, so we should deprecate those policy rules. | |
| 14:24:57 | smcginnis | melwitt: No worries. When you do the next release for it you can switch over the "type" value to be "client-library" instead of "library". | |
| 14:25:27 | smcginnis | melwitt: Just wanted to make sure we weren't missing something since I saw it had a good handful of commits that have not been released. | |
| 14:29:37 | melwitt | smcginnis: yeah, thank you for pinging me about it. if I'd known it was "non-client library" I'd have proposed a release for it yesterday, for sure | |
| 14:32:19 | openstackgerrit | Lei Zhang proposed openstack/nova master: Add method to get cpu traits https://review.openstack.org/560317 | |
| 14:33:47 | smcginnis | melwitt: Makes sense that it should be a client lib if it's an osc plugin. Just miscategorized I'd say. | |
| 14:40:21 | melwitt | smcginnis: coolness. I'll make sure to update that type value next release (next week) | |
| 15:01:24 | mriedem | finucannot: +2s on the next 2 in the series for numa stuff with comments that can be addressed in the follow up patch | |
| 15:01:28 | mriedem | especially the functional test one | |
| 15:05:03 | finucannot | mriedem: Cool, I'll stick the fixes in with https://review.openstack.org/#/c/582651/ | |
| 15:05:13 | finucannot | Thanks for tackling those too. Much obliged | |
| 15:10:19 | openstackgerrit | Eric Fried proposed openstack/nova master: Check provider generation and retry on conflict https://review.openstack.org/556669 | |
| 15:29:27 | superdan | finucannot: any chance you could clean up those unrelated bits in the functional test before you go for the week? | |
| 15:29:36 | superdan | I really hate to merge something with _that_ much noise | |
| 15:29:47 | finucannot | superdan: Fair point. Yeah, lemme tackle that now | |
| 15:29:51 | superdan | okay thanks | |
| 15:36:23 | mriedem | finucannot: comments in https://review.openstack.org/#/c/564452/23 | |
| 15:36:30 | mriedem | you're missing all of the move operation stuff | |
| 15:36:40 | mriedem | which is probably a patch that needs to come *before* the API enables this for new servers | |
| 15:42:21 | mriedem | the move stuff probably requires 2 changes | |
| 15:42:35 | mriedem | 1. model network phsynes and tunneled in nova.network.model.Network which goes into the info cache | |
| 15:42:55 | mriedem | 2. the actual stuff in conductor that populates RequestSpec.network_metadata from the instance info cache prior to calling the scheduler to find a target host | |
| 15:56:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add functional tests for numa-aware-vswitches https://review.openstack.org/583288 | |
| 15:56:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | |
| 15:56:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Assorted cleanups from numa-aware-vswitches series https://review.openstack.org/582651 | |
| 16:00:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove reference to transient domain when detaching devices https://review.openstack.org/584433 | |
| 16:21:49 | openstackgerrit | Corey Bryant proposed openstack/nova master: In Python3.7 async is a keyword [1] https://review.openstack.org/584365 | |
| 16:25:56 | openstackgerrit | Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | |
| 16:25:57 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Assorted cleanups from numa-aware-vswitches series https://review.openstack.org/582651 | |
| 16:33:42 | openstackgerrit | Pierre Blanc proposed openstack/nova master: Docs: Add guide to migrate instance with snapshot https://review.openstack.org/584442 | |
| 17:11:51 | tomtom001 | hello, does anyone know if the scheduler_default_filters=AllHostsFilter does anything? Are there any problems using it? | |
| 18:27:07 | mriedem | powerkvm ci is happy on this now https://review.openstack.org/#/c/560317/ | |
| 18:27:27 | mriedem | tomtom001: the AllHostsFilter is a noop | |
| 18:27:30 | mriedem | it accepts all hosts | |
| 18:28:25 | mriedem | superdan: on your bfv disk thing, i got the move / migrate patch up on top of that | |
| 18:28:37 | mriedem | fried_rolls also helped flush out some latent bugs in our disk usage reporting in the RT | |
| 18:29:10 | superdan | mriedem: oh awesome thanks, hadn't seen | |
| 18:29:16 | superdan | mriedem: since you're here.. on the down host thing... | |
| 18:29:35 | superdan | I was thinking -- can't we get away with the online migration without needing a marker record? | |
| 18:30:10 | superdan | we can query for max_limit records, where queued_for_delete is NULL and group_by or filter by cell to make the instance lookups more efficient | |
| 18:30:42 | superdan | we needed the marker for the initial roll-out of those records, because we didn't know where to pick up because we were creating them, | |
| 18:31:03 | superdan | but since we're just healing those that have no value set, we can use the NULL-ness to whittle that down I think | |
| 18:31:07 | mriedem | the migration is just copying how the reqspec migration was done | |
| 18:31:08 | mriedem | which uses a marker | |
| 18:31:28 | superdan | the creating of the reqspec? | |
| 18:31:30 | mriedem | yeah | |
| 18:31:33 | superdan | yeah, | |
| 18:31:40 | mriedem | i'm not sure that queued_for_delete will be NULL will it? | |
| 18:31:49 | mriedem | maybe it will, i forget how the defaults work in the new column | |
| 18:31:59 | superdan | this is different since we've already got all the records and we're just patching up | |
| 18:32:01 | superdan | it's not nullable=False, so I think it defaults to being nullable | |
| 18:32:02 | superdan | default is not server_default, | |
| 18:32:04 | superdan | so it's not in the schema AFAIK | |
| 18:32:42 | mriedem | so for every instance mapping with a NULL queued_for_delete, lookup it's instance and see if the instance is deleted (or soft deleted)? | |
| 18:33:09 | superdan | "Defaults to True unless primary_key is also True" | |
| 18:33:28 | superdan | you can still do bulk queries of each I think | |
| 18:33:47 | mriedem | by instance uuid? | |
| 18:34:05 | mriedem | so gimme 50 instance mappings with queued_for_delete=NULL, | |
| 18:34:06 | superdan | yeah, so you query out max_limit mappings, | |
| 18:34:11 | mriedem | right | |
| 18:34:17 | mriedem | and those instances might be spread across multiple cells | |
| 18:34:20 | superdan | and then do a get_by_filters({'uuid': uuids}) | |
| 18:34:28 | superdan | well, this appears to be doing one cell at a time no? | |
| 18:34:31 | superdan | it's not targeting | |
| 18:34:40 | mriedem | it's not multi-cell aware | |
| 18:34:42 | superdan | so just limit by cell when you're getting the mappings | |
| 18:34:43 | superdan | right | |
| 18:34:44 | mriedem | like the other migrations | |
| 18:35:18 | superdan | oh I see this is going by cell db first, so it's naturally scoped to a cell | |
| 18:35:27 | superdan | well anyway, | |
| 18:35:28 | mriedem | how do we know what cell we're in? | |
| 18:35:42 | mriedem | we have [database]/connection | |
| 18:35:55 | superdan | should be trivial to limit it per cell even starting from the mapping I think | |
| 18:36:37 | mriedem | would you page across cells until you've filled the limit of instances? | |
| 18:36:37 | superdan | it seems like it's worth avoiding the need for another marker-record-based migration...am I wrong? | |
| 18:37:29 | mriedem | i don't love a marker-based migration, but it works | |
| 18:37:37 | superdan | I think you'd just make this migration thing take a cell mapping, and then let nova-manage tell it what cell it's working on, and then nova-manage can take a cell_uuid or --all-cells like some of our other things | |
| 18:38:01 | mriedem | so this wouldn't be part of nova-manage db online_data_migrations then... | |
| 18:38:06 | mriedem | i don't love that | |
| 18:38:07 | superdan | no | |
| 18:38:09 | mriedem | b/c then it's another one off | |
| 18:38:10 | superdan | it should | |