| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-29 | |||
| 08:44:33 | gibi | thanks | |
| 08:49:24 | gibi | brinzhang_: did you hit the archiving case described in the comment https://opendev.org/openstack/nova/src/branch/master/nova/compute/resource_tracker.py#L1499-L1500 ? | |
| 09:03:58 | brinzhang_ | gibi: yes | |
| 09:04:00 | brinzhang_ | gibi: we meet the case that the RequestSpec data of instance isnot exist, but the allocations data still exist, maybe it is migration operation causes this scence, but i am not sure. | |
| 09:09:06 | openstackgerrit | Merged openstack/nova-specs master: Support volume local cache https://review.opendev.org/728778 | |
| 09:13:15 | gibi | brinzhang_: do you have a reproduction? if yes then please file a bug | |
| 09:15:03 | brinzhang_ | gibi: I am sorry, I cannot provide exact steps :( | |
| 09:15:26 | brinzhang_ | gibi: Can you think of the anomalies that will occur here? | |
| 09:15:58 | brinzhang_ | whatever you can think | |
| 09:17:34 | openstackgerrit | Merged openstack/nova-specs master: Update the testing section of the rbd-glance-multistore spec https://review.opendev.org/735192 | |
| 09:19:28 | gibi | brinzhang_: so by the fact that not even the RequestSpec of the instance exists in the db you concluded that this instance was deleted + archived. | |
| 09:20:12 | gibi | brinzhang_: if that the case then there is no way to check instance.state == BUILDING | |
| 09:22:25 | gibi | brinzhang_: also you cannot simply check the existence of the RequestSpec from the compute as RequestSpec is in the api database | |
| 09:27:52 | brinzhang_ | gibi: I discussed it with my colleagues. In the following situation, the resource cannot be cleaned up: | |
| 09:27:59 | brinzhang_ | Steps to reproduce: | |
| 09:28:07 | brinzhang_ | 3. Timed task update_available_resource() fails to synchronize resource occupancy to placement | |
| 09:28:07 | brinzhang_ | 2. Delete instance A, the placement API is abnormal and cannot synchronize resource information. RequestSPEC will store instance A resource allocation information | |
| 09:28:07 | brinzhang_ | 1. Failed to create instance A, instance A exist in cell0 | |
| 09:30:27 | gibi | brinzhang_: i think what you need is "nova-manage placement audit | |
| 09:30:28 | gibi | " | |
| 09:31:03 | gibi | brinzhang_: and later on https://specs.openstack.org/openstack/nova-specs/specs/victoria/approved/nova-audit.html | |
| 09:31:10 | gibi | when it is implemented | |
| 09:34:17 | brinzhang_ | gibi: later I will review this spec | |
| 09:36:10 | brinzhang_ | gibi: in Lin1487, we just get the instance from nova cell1 db, but cannot get from cell0(store the error instances) https://opendev.org/openstack/nova/src/branch/master/nova/compute/resource_tracker.py#L1487 | |
| 09:37:13 | gibi | I don't think you can talk to the cell0 db from a compute host | |
| 09:37:44 | brinzhang_ | I know | |
| 09:37:50 | gibi | but placement audit runs on the top therefore it can inspect every cell db | |
| 09:38:38 | brinzhang_ | if so, I think it can resolve my question | |
| 11:02:19 | openstackgerrit | Merged openstack/nova stable/stein: Make greande jobs n-v for EM and oldest stable https://review.opendev.org/737332 | |
| 11:05:13 | openstackgerrit | Johannes Kulik proposed openstack/nova master: vmware: Handle exception in destroy with attached volumes https://review.opendev.org/738416 | |
| 12:00:48 | openstackgerrit | Wenping Song proposed openstack/nova master: Add instance project_id for cyborg arq https://review.opendev.org/738428 | |
| 12:37:02 | openstackgerrit | renminmin proposed openstack/nova master: SRIOV: Fix instance with macvtap port live migration failed https://review.opendev.org/738432 | |
| 13:06:34 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Update keypairs in saving an instance object https://review.opendev.org/683043 | |
| 13:13:58 | stephenfin | gibi: I've reproposed the use-pcpu-and-vcpu-in-one-instance BP work for a runway. What was approved has merged (thanks for the reviews!), and now there are four patches at the top good to go and huaquiang is reworking the remainder at the moment | |
| 13:14:15 | stephenfin | Will ping alex_xu when he's around and ask for reviews | |
| 13:14:19 | gibi | stephenfin: cool | |
| 13:14:27 | gibi | stephenfin: I will move it to the last slot then | |
| 13:14:36 | stephenfin | \o/ | |
| 13:15:54 | stephenfin | Also, I took a look through Gerrit on Friday trying to find more stuff to add. No luck. melwitt and johnthetubaguy both have stuff that could conceivably go in (nova-audit and oslo.limit integration, respectively) but they both have open comments :( | |
| 13:16:37 | johnthetubaguy | snowed under at the moment sadly | |
| 13:16:42 | stephenfin | johnthetubaguy: I'll review if/when you do | |
| 13:16:50 | stephenfin | The provider.yaml file work looks stalled as well :( | |
| 13:17:06 | gibi | stephenfin: thanks for looking. | |
| 13:17:13 | johnthetubaguy | stephenfin: thanks, I will try get to that ASAP | |
| 13:17:16 | gibi | yeah I noticed that provider.yaml is stopped | |
| 13:17:26 | gibi | stephenfin: what is the resolution of https://review.opendev.org/#/c/737026/2/nova/virt/hardware.py@1708 ? | |
| 13:17:35 | johnthetubaguy | I wanted to reboot that, but right now don't have bandwidth | |
| 13:17:52 | johnthetubaguy | (on the OpenDev call into) | |
| 13:19:13 | stephenfin | I mean, I could take it over but I don't know who'll be available to review. I've already got the vTPM stuff and mixed CPU types work against me too | |
| 13:19:36 | stephenfin | gibi: Oh, I've a patch for that somewhere. Let me try find it | |
| 13:19:59 | stephenfin | gibi: nvm, pushed it. https://review.opendev.org/#/c/737026/3/nova/virt/libvirt/driver.py | |
| 13:20:48 | gibi | stephenfin: thanks, will look | |
| 13:23:04 | stephenfin | gibi: Sweet. Last thing, it's not ready but I've started work on adding type hints for objects. It's kind of ugly and will probably need a mypy plugin to make it not ugly, but it should fix the gap you were seeing with types not being checked for stuff from nova.objects https://review.opendev.org/#/c/738020/2 | |
| 13:23:25 | stephenfin | To be clear, obviously not ready for review yet | |
| 13:24:42 | gibi | stephenfin: interesting. so basically that mypy plugin could look at the ovo field declaration directly and generate the type hints on the fly for ovos | |
| 13:25:52 | stephenfin | gibi: Yup. It seems that's what they do for Django, which has similar magic behavior for things like models https://github.com/typeddjango/django-stubs/tree/master/mypy_django_plugin | |
| 13:26:04 | stephenfin | I just need to figure out how mypy works internally first :( | |
| 13:27:21 | gibi | just opened the code of that plugin and got scared by the internals :) | |
| 13:27:51 | gibi | this feel heavy lifting for me right now | |
| 13:28:29 | stephenfin | Yeah, it's pretty low priority. If I can get the ugly but effective thing working for now, I'll do that | |
| 13:29:01 | stephenfin | and if I did a plugin working, it would live in o.vo itself so nothing you'd have to review | |
| 13:31:02 | gibi | don't misunderstand me I'm happy to indulge into mypy I just need some pre-learning first. so it is scary now as it is unknown | |
| 13:31:10 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Extend is_ipv6_supported() to cover more error cases https://review.opendev.org/736167 | |
| 13:31:17 | gibi | stephenfin: btw, fixed up ^^ | |
| 13:37:55 | gibi | stephenfin: thanks | |
| 13:40:08 | sean-k-mooney | gibi: when you say the provider.yaml stuff has stalled to you mean the code or the reivew | |
| 13:40:16 | stephenfin | sean-k-mooney: the code | |
| 13:40:40 | stephenfin | sean-k-mooney: https://review.opendev.org/#/c/673341/ | |
| 13:40:45 | sean-k-mooney | ah ok well i guess i could looks tat that again or we can figure something out | |
| 13:41:33 | sean-k-mooney | were there pending change sstill need after v47 | |
| 13:42:07 | stephenfin | I don't see anything, but I haven't been involved until now | |
| 13:43:01 | sean-k-mooney | if more changes can be done via followups i would personally prefer to start merging the code and adress it that way gibi johnthetubaguy how would you feel about that | |
| 13:43:50 | sean-k-mooney | i think the code was perfectly resonable to merge even at the end of last cycle but im sure we can still tighten the schema definitions and testing | |
| 13:44:14 | sean-k-mooney | i just dont want perfect to be the enemy of good enough and delay this again | |
| 13:44:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Update keypairs in saving an instance object https://review.opendev.org/683043 | |
| 13:49:36 | stephenfin | dansmith: Can you look at https://review.opendev.org/#/c/683043/ again today? | |
| 13:50:18 | gibi | sean-k-mooney: I have to re-review the patch to see if everything is resolved or not | |
| 13:51:38 | gibi | sean-k-mooney: restricting the schema later is a backward incompatible change which would be expensive so I'm on the side to do something that is pretty solid | |
| 13:51:43 | gibi | it is like an API | |
| 13:52:06 | sean-k-mooney | sure but until we have a release with this im not sure we need to be as strict | |
| 13:52:32 | sean-k-mooney | e.g. we coudl treat the scema as a 0.X and then bump to 1.0 when we are happy and release with that | |
| 13:53:12 | gibi | sean-k-mooney: 0.x could work, indicating that it is beta and can change in a backward incompatible way | |
| 13:53:43 | dansmith | stephenfin: I just glanced at it and he didn't do what I prescribed (you said he did), but I haven't looked closely at why he thinks that will work | |
| 13:54:07 | dansmith | just FYI in case you didn't examine it closely | |
| 13:55:34 | stephenfin | You'd suggested calling '_save_extra_generic' and 'self.obj_reset_changes' from '_save_keypairs', and he's just put 'self.obj_reset_changes' into '_save_extra_generic'. That looked functionally equivalent to me | |
| 13:57:07 | dansmith | but keypairs isn't an extra field | |
| 13:57:37 | dansmith | I said | |
| 13:58:07 | dansmith | "the generic save handler" but "save_extra_generic" is specifically for fields in instance_extra, which I think keypairs is not in, no? | |
| 13:58:46 | dansmith | or maybe I'm confusing keypairS with keypair | |
| 13:59:09 | stephenfin | I think it is | |
| 13:59:21 | stephenfin | Yeah, it's part of _INSTANCE_EXTRA_FIELDS | |
| 14:01:12 | dansmith | right, okay.. Instance has its own (key_name, key_data) from when we could only have one | |
| 14:02:31 | stephenfin | TIL (that those fields existed) | |
| 14:02:58 | stephenfin | anyway, since it's in _INSTANCE_EXTRA_FIELDS we'll trigger the correct code path https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L788-L790 | |
| 14:03:24 | dansmith | aye | |
| 14:03:43 | stephenfin | and calling 'self.obj_reset_changes' on a nested object field would presumably always be the correct thing to do in that path | |
| 14:04:53 | dansmith | I'm not sure about that, I need to check something.. because there are cases where we do and don't delegate that to sub-objects | |
| 14:05:01 | dansmith | since originally all sub-objects would have their own save handler, | |
| 14:05:13 | dansmith | we initially (at least) didn't reset through like that | |
| 14:05:57 | dansmith | I'mma pull it down and look | |