| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-08 | |||
| 11:27:20 | stephenfin | bauwser: Whoops | |
| 11:58:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: API: Remove evacuate/live-migrate 'force' parameter https://review.openstack.org/634600 | |
| 12:22:43 | openstackgerrit | Merged openstack/nova master: Transfer port.resource_request to the scheduler https://review.openstack.org/567268 | |
| 12:25:29 | openstackgerrit | Merged openstack/nova-specs master: AMD SEV: address final feedback received prior to merge https://review.openstack.org/628252 | |
| 12:42:37 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Avoid redundant initialize_connection on source post live migration https://review.openstack.org/551302 | |
| 12:46:56 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing required in servers ViewBuilder to construct partial results https://review.openstack.org/635146 | |
| 12:46:57 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells https://review.openstack.org/591657 | |
| 12:46:57 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells Documentation https://review.openstack.org/635147 | |
| 12:56:30 | openstackgerrit | Merged openstack/os-traits master: Add HW_CPU_AMD_SEV trait https://review.openstack.org/635608 | |
| 14:00:43 | mriedem | tssurya: i think we're just missing a unit test on https://review.openstack.org/#/c/635121/ and then i'd be +2 | |
| 14:04:14 | tssurya | mriedem: on it thanks | |
| 14:08:12 | mriedem | dansmith: another good example of how brittle everything is https://bugs.launchpad.net/nova/+bug/1815153 | |
| 14:08:14 | openstack | Launchpad bug 1815153 in OpenStack Compute (nova) "Requested host during cold migrate is ignored if server created before Rocky" [High,In progress] - Assigned to Takashi NATSUME (natsume-takashi) | |
| 14:09:30 | mriedem | tssurya: if you're respinning i had some nits in https://review.openstack.org/#/c/635145/ as well | |
| 14:09:57 | tssurya | mriedem: yea saw them, I'll fix them too | |
| 14:16:56 | bauwser | mriedem: thanks for the detailed bug report on https://bugs.launchpad.net/nova/+bug/1815153 | |
| 14:16:57 | openstack | Launchpad bug 1815153 in OpenStack Compute (nova) "Requested host during cold migrate is ignored if server created before Rocky" [High,In progress] - Assigned to Takashi NATSUME (natsume-takashi) | |
| 14:17:18 | bauwser | mriedem: tbc, requested_destination wasn't intended to be persisted | |
| 14:26:04 | dansmith | mriedem: just in general? | |
| 14:26:21 | mriedem | just another example of how i feel that everything i touch breaks something else somewhere | |
| 14:26:33 | dansmith | ah yep | |
| 14:26:38 | mriedem | bauwser: sure, we fixed that, | |
| 14:26:46 | mriedem | and in doing so broke something else inadvertently later | |
| 14:32:20 | bauwser | mriedem: ergh ok | |
| 14:37:54 | mriedem | i'm also not sure why, | |
| 14:37:56 | mriedem | because of https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/compute/api.py#L3329 | |
| 14:38:18 | mriedem | we should blow up here https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/conductor/manager.py#L1009 with an old request spec | |
| 14:38:26 | mriedem | but i modified a functional test that hits that code and it doesn't blow pu | |
| 14:38:27 | mriedem | *up | |
| 14:38:56 | mriedem | oh i see why | |
| 14:39:02 | mriedem | https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/compute/api.py#L3329 has no effect on save | |
| 14:40:13 | mriedem | https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/objects/request_spec.py#L619. | |
| 14:46:05 | bauwser | mriedem: yeah, most of the problems with the persisted fields in the ReqSpec is because we late call .save() sometimes or not | |
| 14:46:43 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing for ignoring list_records_by_skipping_down_cells https://review.openstack.org/635121 | |
| 14:46:44 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing for allowing the all-tenants filter with down cells https://review.openstack.org/635145 | |
| 14:46:44 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing required in servers ViewBuilder to construct partial results https://review.openstack.org/635146 | |
| 14:46:45 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells https://review.openstack.org/591657 | |
| 14:46:45 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells Documentation https://review.openstack.org/635147 | |
| 14:47:02 | bauwser | mriedem: so the only way to make sure to not persist them is to verify if save() does it or not | |
| 14:47:04 | openstackgerrit | Mohammed Naser proposed openstack/nova master: zuul: add cross-repo jobs for openstack-ansible https://review.openstack.org/635852 | |
| 14:47:48 | mriedem | i would argue the root of our problems with the request spec is its dual nature - it's both a record of the original server create request (and modified on rebuild + new image and resize + new flavor), but also used as a parameter data bag to pass stuff to the scheduler for per-request operatoins like migrations | |
| 14:47:54 | mriedem | and we don't deal with ^ very well | |
| 14:48:13 | bauwser | yeah I agree with you | |
| 14:48:24 | mriedem | if we would have just passed things like requested_destination through to the scheduler as a specific parameter we could have saved some trouble | |
| 14:48:37 | bauwser | well | |
| 14:48:39 | bauwser | sure | |
| 14:48:50 | cdent | change it? | |
| 14:48:54 | bauwser | but we had a consensus about not doing this 2 years ago ;) | |
| 14:49:06 | bauwser | we preferred to pass o.vo objects rathre :) | |
| 14:49:07 | mriedem | sure, because 2 years ago no one anticipated the shit loads of bugs | |
| 14:49:25 | mriedem | i'm not advocating a big overhaul since that will yield yet more bugs | |
| 14:49:48 | mriedem | i just bemoan blowing off a two every time i try to stitch a finger back on | |
| 14:49:53 | mriedem | *toe | |
| 14:49:57 | bauwser | well, maybe the analysis could be to say which fields should be persisted once and which ones not | |
| 14:50:06 | mriedem | bauwser: we already have that in the request spec | |
| 14:50:12 | mriedem | and have been adding to it over time | |
| 14:50:20 | bauwser | right | |
| 14:50:27 | mriedem | https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/objects/request_spec.py#L587 | |
| 14:50:30 | mriedem | https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/objects/request_spec.py#L593 | |
| 14:50:50 | mriedem | gibi is already handling requested_resources so they aren't overwritten on save https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/objects/request_spec.py#L517 | |
| 14:51:00 | mriedem | we need to do the same for those other fields probably | |
| 14:51:00 | bauwser | ok | |
| 14:51:06 | bauwser | yeah I guess | |
| 14:51:14 | bauwser | like the flavor | |
| 14:51:26 | bauwser | when should we persist it? | |
| 14:51:39 | bauwser | for example, for resize | |
| 14:51:43 | mriedem | flavor isn't in that list | |
| 14:51:56 | bauwser | should we persist the new flavor in the original ReqSpec ? | |
| 14:51:59 | bauwser | I know | |
| 14:52:05 | mriedem | we already do that here yeah https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/conductor/manager.py#L361 | |
| 14:52:06 | bauwser | I guess so | |
| 14:52:09 | bauwser | so | |
| 14:52:43 | bauwser | what I'm trying to explain is that we basically fixed problems by each bug | |
| 14:52:52 | bauwser | and then we modified the object | |
| 14:53:07 | bauwser | maybe we should just take some time to discuss about *all* the fields | |
| 14:53:09 | mriedem | oh yeah and that flavor thing introduced a bug that was latent for a long time https://github.com/openstack/nova/blob/a6963fa6858289d048e4d27ce8e61637cd023f4c/nova/compute/api.py#L3403 | |
| 14:53:23 | bauwser | right | |
| 14:53:32 | bauwser | I'm just giving examples | |
| 14:53:38 | mriedem | and i'm just venting | |
| 14:53:41 | bauwser | so, honestly, the more I'm thinking | |
| 14:53:58 | bauwser | the more I wonder if we should have more than one RequestSpec for an instance :) | |
| 14:54:10 | bauwser | ie. the original record for create | |
| 14:54:16 | bauwser | and then a new record for a move | |
| 14:54:31 | mriedem | the more i think about request spec, the less i want to think about request spec | |
| 14:54:36 | mriedem | i want to get this bug fixed and move on | |
| 14:54:38 | bauwser | hah | |
| 14:55:05 | bauwser | anyway, I need to go to a meeting | |
| 14:55:10 | bauwser | :/ | |
| 15:21:25 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Trim fake_deserialize_context in test_conductor https://review.openstack.org/635859 | |
| 15:21:27 | mriedem | another subunit parser thing ^ based on http://logs.openstack.org/27/619527/16/gate/openstack-tox-lower-constraints/cd2889d/job-output.txt.gz#_2019-02-08_12_32_26_104934 | |
| 15:26:34 | mriedem | dansmith: so wrt upgrades we just assert that conductor always goes before everything else yeah? and api is last. which is why if we're making changes to both api and conductor at the same time w/o changing rpc method interfaces, but like through fields on the RequestSpec (behavior changes), we aren't doing any strict version checking in the API to make sure conductor is new enough to handle that behavior change | |
| 15:26:41 | mriedem | we just assume that conductor will be upgraded before api | |
| 15:27:06 | dansmith | mriedem: api isn't last, compute is last | |
| 15:27:08 | mriedem | unlike how the api will check min versions on computes for behavior | |
| 15:27:19 | dansmith | mriedem: we pretty much have to assume that conductor, scheduler, and api all go at the same time | |
| 15:27:19 | mriedem | ok but conductor is first | |
| 15:27:27 | mriedem | yeah | |
| 15:27:53 | mriedem | was just thinking about https://review.openstack.org/#/c/635668/2/nova/conductor/tasks/migrate.py@200 | |
| 15:28:07 | mriedem | which is changes to conductor based on changes the api will make to the request spec, which is all assumed to be upgraded at the same time | |
| 15:28:30 | dansmith | yeah | |