| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-04 | |||
| 14:10:45 | mriedem | "If left disabled, image signature verification can still | |
| 14:10:45 | mriedem | occur but the end user will not have any assurance that the signing | |
| 14:10:45 | mriedem | certificate used to generate the image signature is still trustworthy." | |
| 14:10:49 | openstackgerrit | MultipleCrashes proposed openstack/nova master: Retry decorator fix for autoscale delete https://review.openstack.org/563418 | |
| 14:11:20 | mriedem | the end user isn't going to have any idea if the cloud is configured to validate image certs | |
| 14:11:51 | mriedem | i guess if you just aren't going to support trusted certs in your cloud at all, you should disable the feature by policy so the user can't pass those into server create and rebuild? | |
| 14:12:38 | dansmith | yeah I mean I dunno what else to tell them to do.. maybe we log a warning if the config and policy don't match? | |
| 14:12:48 | dansmith | so they know they're advertising a bad setup? | |
| 14:13:18 | alex_xu_ | mriedem: replied | |
| 14:13:34 | mriedem | alex_xu_: thanks | |
| 14:14:29 | sahid | melwitt: yep you can mark it as completed | |
| 14:14:37 | alex_xu_ | np | |
| 14:14:49 | sahid | melwitt: there is also the vf-trusted one | |
| 14:15:06 | mriedem | sahid: i completed that on friday | |
| 14:15:19 | sahid | ah ok i did not notice | |
| 14:16:11 | melwitt | sahid: cool, thanks for confirming | |
| 14:22:25 | jangutter | Is it possible I could add a spec to the review queue for tomorrow? | |
| 14:22:56 | naichuans | efried: Hi, Eric | |
| 14:23:09 | efried | naichuans: hello | |
| 14:24:30 | mriedem | dansmith: ok there is a bug in the libvirt plumbing patch too, and it looks like the barbican plugin test for this certs stuff isn't enabling cert validation either, which is probably how it was passing CI https://review.openstack.org/#/c/515210/ | |
| 14:24:36 | mriedem | so :( | |
| 14:27:01 | naichuans | efried: I do the modification when we discuss the case: customers modified nova.conf, changed the vgpu type. If the gpu already has a vgpu with the previous vgpu type, we can't allocate new gpu type instance(becase we only support one vgpu type per gpug), so we set `reserve` = `total` to prevent scheduling. | |
| 14:28:08 | efried | naichuans: So help me understand which part of the code is detecting old vs new vgpu type. That's the part I'm missing. | |
| 14:28:52 | naichuans | efried: Now we check inventory change to determine if there is vgpu configure changes | |
| 14:29:08 | efried | but inventory changes when you allocate a vgpu | |
| 14:29:18 | naichuans | No | |
| 14:29:22 | efried | no??? | |
| 14:30:17 | naichuans | inventory only contains the vgpu type, total, such kinds of features, no resource count | |
| 14:31:27 | naichuans | resouce amount related information is include in allocation(about used), and the real time host states from hypervisor | |
| 14:31:35 | melwitt | mriedem: it looks like the -W can be lifted from this patch now https://review.openstack.org/568382 and the rest of the series be reviewed for the ocata release https://review.openstack.org/571522 | |
| 14:35:44 | naichuans | efried: check inventory define in the patch, we only have `total` field, no `remain` | |
| 14:36:07 | naichuans | fields.ResourceClass.VGPU: { | |
| 14:36:08 | naichuans | 'total': grp_value['total'], | |
| 14:36:08 | naichuans | 'min_unit': 1, | |
| 14:36:08 | naichuans | 'max_unit': 1, | |
| 14:36:08 | naichuans | 'step_size': 1, | |
| 14:36:09 | naichuans | } | |
| 14:36:11 | naichuans | } | |
| 14:36:46 | efried | naichuans: I understand that 'remaining' comes from the hypervisor. | |
| 14:37:32 | naichuans | vGPU type changes would effect total and display heads, I will add traits detecting code in the following patches | |
| 14:38:21 | naichuans | effect -> affect. sorry | |
| 14:38:48 | efried | (wow, that's one English speakers have trouble with :) | |
| 14:39:19 | efried | naichuans: Okay, so the key here is has_inventory_changed - meaning, is the inventory in the provider tree different from the inventory you expect... | |
| 14:39:28 | efried | let me stare at this for a sec... | |
| 14:40:19 | naichuans | efried: almost | |
| 14:40:51 | efried | naichuans: So you're using has_inventory_changed to detect that the vgpu type is different? That's the part that doesn't make sense to me. | |
| 14:41:17 | efried | Surely there are vgpu types that will have the same total as each other. | |
| 14:41:26 | efried | so that check won't do anything. | |
| 14:41:49 | efried | I would expect if you're trying to detect a changed vgpu type, you would be looking for the grp_id to be different. | |
| 14:42:46 | efried | see, at the moment, your condition says "the resource provider for the expected grp_id already exists, but its inventory total isn't what I expected. Let's see if anything is allocated. Okay, it is - let's disable this GPU." | |
| 14:42:54 | naichuans | The check is for the gpug with same uuid | |
| 14:43:14 | efried | does that correspond to a distinct gpu type? | |
| 14:43:21 | efried | or vgpu type? | |
| 14:44:07 | naichuans | almost means it is the same gpug, but there is configure changes on it. we can't support vgpu type checking, jianghua and jay has reject this field | |
| 14:45:05 | mriedem | melwitt: we don't run the nova-lvm job in ocata, so not sure if we should hold and backport/verify that? if even in a WIP patch. note that we also merged the pike backports w/o having nova-lvm running in pike either. | |
| 14:45:28 | efried | naichuans: Same group, but config has changed. Okay, I think I can buy that. But why does that mean you have to disable the gpu? | |
| 14:47:03 | efried | ...and how did the configuration change if there are existing allocations? | |
| 14:47:19 | melwitt | mriedem: oh, it's -W just waiting for the higher branches to merge | |
| 14:47:23 | naichuans | efried: we can't support multi-type vgpu on one gpug. so if the old type instance still on the gpug, we can't allocate new type on it. So we want to disable it. At first I suggest to set `total` to 0, but Jay suggests use `reserved` | |
| 14:47:52 | naichuans | change nova.conf to modify vgpu type white list | |
| 14:48:07 | mriedem | melwitt: what is? | |
| 14:48:07 | naichuans | we only need a service resart after that. | |
| 14:48:18 | melwitt | mriedem: this https://review.openstack.org/568382 | |
| 14:48:35 | mriedem | melwitt: that was waiting for https://review.openstack.org/#/c/571433/ | |
| 14:48:38 | mriedem | but i'm saying, | |
| 14:48:47 | mriedem | https://review.openstack.org/#/c/571433/ is to fix a nova-lvm regression, | |
| 14:48:52 | mriedem | and we don't run the nova-lvm job in pike or ocata, | |
| 14:49:06 | mriedem | so should we hold until we verify nova-lvm works with that change, or just...punt | |
| 14:49:15 | efried | naichuans: But I thought "changing vgpu type" and "changing the configuration (which leads to a different 'total')" were two different things. | |
| 14:50:08 | naichuans | You mean change the `total` without change the conf? | |
| 14:51:26 | melwitt | mriedem: oh, I thought it was waiting for the higher branches version of it to merge (and they have). I was thinking to just punt ... we verified it worked on master, so tbh I wasn't thinking of doing it per branch even though maybe we should have. we don't have access to the old experimental jobs on older branches do we? we'd have to do something like upload a change that adds a new nova-lvm job for ocata, for example? | |
| 14:52:22 | mriedem | we would have to backport https://github.com/openstack/nova/commit/0db7b77b4615f25217ad646b2e4082994e8dfb73#diff-7415f5ff7beee2cdf9ffe31e12e4c086 to pike and then ocata | |
| 14:52:39 | melwitt | okay, I see | |
| 14:52:54 | mriedem | https://review.openstack.org/568382 was waiting for https://review.openstack.org/#/c/571433/ because https://review.openstack.org/568382 regressed lvm | |
| 14:53:38 | mriedem | given my current lack of energy, i'm ok with punting | |
| 14:53:38 | melwitt | oh, right. I didn't realize that was that change | |
| 14:54:43 | naichuans | efried: as I know, we don't supoort hot plugin/plugout, so that case should not happen. | |
| 14:55:52 | efried | naichuans: I'm still trying to wrap my head around all of this, but what I think I've gathered so far is that there are two scenarios we need to be worried about: | |
| 14:55:52 | efried | 1) User reconfigures which vgpu type they want to use. They do this by changing CONF.devices.enabled_vgpu_types. | |
| 14:55:52 | efried | 2) User somehow changes the configuration of their vgpu so it now has a different 'total' than it did before. <== how does this happen? | |
| 14:57:09 | efried | naichuans: In the case of 1), if there are existing allocations of the old type, we need to block any subsequent allocations of the new type because we only support one type at a time. Right? | |
| 14:57:23 | efried | And I don't understand which part of the code is doing ^ this. | |
| 14:57:23 | mnaser | monday morning fun | |
| 14:57:35 | mnaser | nova boot foo.123 fails because | |
| 14:57:37 | mnaser | .123 isn't a tld | |
| 14:57:50 | mnaser | and it doesn't even fail in the api layer, it gets rescheduled | |
| 14:58:28 | mnaser | RescheduledException: Build of instance e142e418-7c47-409f-8892-542d5e86e442 was re-scheduled: Invalid input for dns_name. Reason: 'foo.123' not a valid PQDN or FQDN. Reason: TLD '123' must not be all numeric.\nNeutron server returns request_ids: ['req-3ddffd11-e41e-4be2-98a8-d2c22b8c515c | |
| 14:58:38 | mnaser | well i guess you could say it's a neutron bug, i dunno what'd you'd classify this at | |
| 14:59:02 | dansmith | I wasn't aware that we assigned any special significance to names with dots | |
| 14:59:08 | mnaser | if nova team feels like this is a nova bug, ill file and start looking into fixes | |
| 14:59:17 | naichuans | efried: we set `reserve` = `total`, to avoid scheduling for new vgpu | |
| 14:59:19 | mnaser | i think neutron does validation and we might not be doing proper sanitization when creating a port | |
| 14:59:21 | melwitt | I feel like I've seen that before. because we use display_name as the hostname | |
| 14:59:28 | dansmith | mnaser: I wonder if we're just passing the display name to neutron and it's inferring some signficance? | |
| 14:59:38 | mnaser | i think that's exactly whats happening | |
| 14:59:39 | dansmith | melwitt: ah, that'd make sense | |
| 14:59:44 | melwitt | dansmith: I think that's why | |
| 14:59:46 | dansmith | so I don't think that's an us thing | |
| 14:59:46 | mnaser | it creates a port with dns_name=display_name and poof | |
| 15:00:09 | mnaser | well if nova is giving a non-valid dns_name, i'd say it's okay for neutron to complain? | |