| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-04 | |||
| 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? | |
| 15:00:11 | dansmith | unless we shouldn't be passing the display name | |
| 15:00:14 | melwitt | we do this kind of sanitizing https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1416 | |
| 15:00:36 | efried | naichuans: Yes, I'm not arguing with that part. When we need to "disable" a vgpu, we set reserved = total. What I'm trying to figure out is, under what circumstances do we decide we *need* to disable a vgpu? Am I on the right track with 1 and 2 above? | |
| 15:00:59 | melwitt | https://github.com/openstack/nova/blob/master/nova/utils.py#L485 | |
| 15:01:25 | dansmith | right, but DNS validation shouldn't be part of the sanitization | |
| 15:02:01 | melwitt | yeah, it isn't | |
| 15:02:35 | naichuans | efried: I see. In truth each vgput type consume different resource, the `total` = `whole resource` / `resource for one vgpu of certain type`. so change the vgpu type, you will change the `total` | |
| 15:02:54 | efried | not necessarily? | |
| 15:02:55 | dansmith | mnaser: surely there's some configuration of neutron that doesn't require valid TLDs right? | |
| 15:03:08 | efried | naichuans: Surely there are some "types" that will have the same `total`? | |
| 15:03:21 | naichuans | efried: it should not. | |
| 15:03:40 | naichuans | especially on a same pgpu group | |
| 15:03:42 | efried | naichuans: But different types can have the same group ID? | |
| 15:04:13 | jaypipes | bauzas: so, I note that you didn't respond to any of my review comments on your multi-gpu-types spec... | |
| 15:04:44 | naichuans | efried: right, one pgug has one id, but it would support more than one vgpu type, each of them has different resource amount | |
| 15:04:50 | bauzas | jaypipes: wait? while I haven't replied to your comments, I think I fixed given your opinions :) | |
| 15:05:25 | bauzas | jaypipes: https://review.openstack.org/#/c/557065/5..6/specs/rocky/approved/vgpu-rocky.rst ;) | |
| 15:06:24 | jaypipes | bauzas: ok. I was just expecting you to come back at me with words :) | |
| 15:07:04 | bauzas | jaypipes: heh, sometimes I'm just revisiting my patches if the comment is good for me :) | |
| 15:08:22 | efried | naichuans: I'm having a hard time understanding that. But okay, so the code we're looking at is meant to handle #2. What are we doing to handle #1? | |
| 15:08:38 | jaypipes | bauzas: good to know! :) | |
| 15:09:55 | bauzas | jaypipes: sorry about the confusion :) | |
| 15:10:38 | jaypipes | bauzas: no worries! | |
| 15:12:03 | bauzas | jaypipes: cdent: efried: btw. not sure when the hangout will be ? | |
| 15:12:11 | bauzas | for the upgrade issues ? | |
| 15:12:16 | efried | bauzas: I don't think we discussed it further. | |
| 15:12:24 | bauzas | k | |
| 15:12:31 | naichuans | efried: Eric, I can't catch your minde... in truth we have only one way to configure vgpu type on OpenStack side, change `CONF.devices.enabled_vgpu_type` by modify nova.conf. So I think #1 and #2 comes from the same reason, nova.conf changed. could you explain more? | |
| 15:12:31 | efried | I think tomorrow morning (for some U.S. value of "morning") | |
| 15:12:48 | bauzas | lemme look at my agenda | |
| 15:12:56 | cdent | that's my understanding as well. we were going to figure it out here. hopefully with some input from edleafe | |
| 15:13:28 | efried | naichuans: Okay, enabled_vgpu_types=foo. Create an instance with a vGPU. Change enabled_vgpu_types=bar, restart compute service. Try to create an instance with a vGPU. It should fail. Where's the code that makes it fail? | |
| 15:14:14 | bauzas | efried: FWIW about the discussion we had with naichuans, I think we agreed on saying that if an operator modifies nova.conf for changing the types, then meh, it's their responsibility to make sure there is not existing instances | |
| 15:14:20 | bauzas | efried: we can document it | |
| 15:14:46 | efried | bauzas: Hm, okay. It's certainly possible to detect this and throw an error. | |
| 15:15:05 | naichuans | No, it would not failed, we changed the `reserve` when you restart(after modify conf), the schduling to this rp would be impossile | |
| 15:15:52 | naichuans | impossible | |
| 15:17:19 | openstackgerrit | melanie witt proposed openstack/nova stable/pike: zuul: Move legacy jobs to project https://review.openstack.org/572130 | |
| 15:17:27 | bauzas | naichuans: you mean, we *should* do that ? | |