Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-06-04
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 ?
15:20:19 naichuans efried: Although the customer may want to configure to a new vgpu type, but we stop to support fureture allocation. bauzas: I also think it is a mistake by OpenStack operator. Fix by us, or just let scheduling fail and let operator to mange it, both OK for me, my code now take the first choice.
15:20:45 efried scheduling won't fail, though.
15:20:50 naichuans Yes
15:21:16 naichuans It would looking for another rp on a anther host
15:21:38 efried naichuans: Right now you're looping over the vgpu types. If you restart the compute service with a different configured type, you'll ignore the old one entirely and just start using the new one.
15:21:43 openstackgerrit melanie witt proposed openstack/nova stable/ocata: zuul: Move legacy jobs to project https://review.openstack.org/572132
15:22:14 bauzas efried: naichuans: so we would need to check the existing allocations when init_host() then
15:22:23 bauzas just for that
15:23:30 efried But this also dovetails with my other issue, which is that you need to be detecting and deleting resource providers for other vgpu types than the one (or in future, ones) you're currently managing.
15:25:05 bauzas efried: yup, that'z the problem
15:25:33 efried bauzas: I'm saying with the naming convention being used, you can totally do that.
15:26:42 naichuans efried: currently yes, we detect the configure changes by checking inventory changes. Like bauzas said, we also can communicate with hypervisor to check previous allocated vgpu instances
15:27:26 bauzas naichuans: we could pass the allocations to the virt driver with init_host()
15:27:36 bauzas exactly like we do for spawn() or other virt methods
15:27:47 mnaser melwitt, dansmith: https://bugs.launchpad.net/nova/+bug/1581977
15:27:48 openstack Launchpad bug 1581977 in OpenStack Compute (nova) "Invalid input for dns_name when spawning instance with .number at the end" [Undecided,Invalid]
15:27:48 bauzas naichuans: but for the moment, I think we can just document this
15:29:42 naichuans bauzas: yes, we can do it. could we use it to determin vgpu type current used? Looks difficult
15:29:59 naichuans previously used
15:30:13 bauzas naichuans: I did that for reboot :)
15:30:34 bauzas I'm just looking up the existing instances and check whether the mdev is there or not
15:30:38 bauzas and just recreate it
15:32:07 naichuans bauzas: currently, we defined vgpu type in nova.conf. these types is same with the types in hpyervisor record, so it may easier to compare with hypervisor
15:35:18 naichuans and vgpu type is transparent for placement
15:40:19 openstackgerrit Matt Riedemann proposed openstack/nova-specs master: Spec for volume multiattach enhancements https://review.openstack.org/552078

Earlier   Later