| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-08 | |||
| 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 | |
| 15:28:48 | mriedem | i must be having some sort of existential crisis this morning | |
| 15:29:22 | dansmith | mriedem: go to your happy place | |
| 15:29:40 | spatel | sean-k-mooney: morning | |
| 15:30:33 | sean-k-mooney | spatel: o/ | |
| 15:30:43 | spatel | openstack hypervisor stats show command " count | 179" is this number of hypervisor right? | |
| 15:31:09 | melwitt | mriedem: looking at https://review.openstack.org/635859, is it that the assertions are constantly failing and being swallowed? is it bad that the assertions were failing? | |
| 15:32:29 | spatel | why hypervisor stats showing count 179 & "openstack hypervisor list" showing 193 | |
| 15:32:42 | spatel | which one i should consider right? | |
| 15:32:44 | spatel | sean-k-mooney: ^ | |
| 15:33:25 | spatel | http://paste.openstack.org/show/744746/ | |
| 15:36:49 | mriedem | melwitt: yes and idk | |
| 15:37:01 | mriedem | i do'nt know where the 'fake_user' is coming from b/c it's not coming from anything in test_conductor.py that i can tell | |
| 15:40:08 | mriedem | i assume that any test which really cares about the context being serialized/deserialized is checking that explicitly, otherwise we use fake user/project ids all the time | |
| 15:41:49 | sean-k-mooney | spatel: i know you had that issue in the past but i can rememebr what it was again | |
| 15:42:29 | melwitt | mriedem: yeah, fair enough | |
| 15:59:22 | spatel | i think stats output doing something which list not doing.. that is why its difference | |
| 16:08:01 | sean-k-mooney | spatel: the hypervior stats i think only show running vms | |
| 16:08:33 | sean-k-mooney | nova list will also show shelved instnaces and ones in error or power off state | |
| 16:08:47 | spatel | if that is true then it make sense | |
| 16:09:14 | spatel | let me drill and see | |
| 16:11:04 | sean-k-mooney | oh wait you are not asking about nova list | |