Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-14
15:52:10 tssurya mriedem: and yea the downside point is valid
15:52:13 mriedem dansmith: i think so, but if the driver raises the nthe compute manager code has to handle it and not barf for the other events in the same request
15:52:53 dansmith mriedem: I think the driver should just not raise, I think the driver should handle all of this, because it's only one
15:53:29 mriedem so if the driver gets UnexpectedTaskStateError, it just logs and returns?
15:53:34 dansmith yeah
15:53:38 mriedem wfm
15:53:48 dansmith it will be contextually relevant,
15:53:52 tssurya dansmith, mriedem: wait the instance.save(expected_Task_state=None) is happening in the manager no ?
15:53:56 dansmith where compute manager won't know what the eff it means
15:54:00 tssurya and the instance.save(expected_task_state=task_states.POWERING_ON)
15:54:05 tssurya will happen in the driver
15:54:13 mriedem tssurya: no, your code is doing that in the driver
15:54:14 dansmith tssurya: I don't think so
15:54:21 mriedem it's like the only thing the driver does
15:54:30 mriedem https://review.opendev.org/#/c/645611/12/nova/virt/ironic/driver.py
15:54:38 tssurya ok so we record an instance action start before setting the task_state to POWERING_ON ?
15:54:55 mriedem the instance actoin stuff in the api doesn't change
15:54:55 mriedem imo
15:55:10 dansmith I dunno
15:55:13 mriedem that's how you know from the api if the event failed or not
15:55:14 tssurya so isn't it confusing to the user that the task_state hasn't changed
15:55:22 tssurya but there is an instance-action record/notification
15:55:25 tssurya emitted
15:55:26 dansmith mriedem: if I send this event for libvirt instance, it will record an action but nothing really happened
15:55:45 mriedem dansmith: and i'll say, you shouldn't really do that
15:55:57 mriedem same with trying to swap volumes on anything other than libvirt
15:56:29 dansmith :/
15:57:06 mriedem or extend an attached volume, or do host-assised snapshot, or anything with pmems :)
15:58:13 mriedem so https://review.opendev.org/#/c/645611/12/nova/compute/api.py@250 would move to the compute manager, or the ironic driver?
15:58:25 dansmith yeah, well, this is a minor detail
15:58:32 mriedem i'm cool with that moving to the driver too
15:58:40 mriedem until some other driver needs this same thing for whatever reason
15:58:52 dansmith I would think it moves to the driver, and really doesn't need to be all that detailed I would think, but yeah
15:59:02 dansmith I mean, I guess it could stay, I don't really care that much
15:59:04 tssurya ok I can move that whole chunk with the no-op
15:59:09 tssurya all into the driver
15:59:27 tssurya only the instance action and notification stuff remain in place in the api and manager right ?
15:59:49 mriedem the state checking in the api could save you some rpc traffic i guess, but you could be racing either way
16:00:01 tssurya but then again, is it ok for the task_state to still be None when we emit the state notification ?
16:00:01 dansmith the thing I think makes the biggest difference, is changing the power state before we know if we're going to do anything
16:00:07 tssurya start*
16:00:45 dansmith I don't know the semantics of any expectations around that
16:01:10 tssurya ok yea then maybe its fine :)
16:01:12 dansmith I didn't really think notifications are required to be realtime and synchronous, so I don't think it'd matter
16:01:35 mriedem tssurya: you can set the task_state on the instance in the compute manager before calling the driver method if we care - but then you have to handle UnexpectedTaskStateError there as well
16:01:49 dansmith but if we do that,
16:01:50 dansmith we
16:02:01 dansmith are changing it for non-ironic instances,
16:02:12 tssurya hmm yea
16:02:13 dansmith then catch the NotImplementedError, and then change it right back, yeah?
16:02:18 mriedem which won't actually power off or on the guest
16:02:33 mriedem @reverts_task_state would set the task_state back to None yes
16:03:08 mriedem but i'm not too worried about non-ironic instances because this is admin-only api stuff and if someone is abusing it for non-ironic instances and it doesn't work as expected....meh?
16:03:18 dansmith okay
16:03:45 tssurya so what't it going to be: manager or driver ?
16:03:55 tssurya at this point I don't mind either
16:04:12 dansmith "at this point" .. translation: "at this point I'm about to shoot both of you guys"
16:04:13 mriedem i forgot the question
16:04:26 tssurya dansmith: ;)
16:04:35 dansmith tssurya: I think it doesn't matter that much, figure it out when you move the code and it's probably okay
16:04:44 tssurya mriedem: meaning we move the task state and https://review.opendev.org/#/c/645611/12/nova/compute/api.py@250
16:04:46 dansmith tssurya: also when are you actually leaving?
16:05:13 mriedem right now :)
16:05:32 dansmith lol
16:05:36 dansmith #ragequit
16:05:37 mriedem (11:04:47 AM) dansmith: tssurya: also when are you actually leaving?
16:05:38 tssurya sorry bad connection
16:05:49 tssurya I leave on Friday
16:05:53 dansmith okay
16:05:56 tssurya so I can work on it this evening and tomorrow
16:06:02 dansmith okay cool
16:06:24 dansmith tssurya: so I said I think manager vs. driver probably doesn't matter that much, you choose while moving the code and it'll probably be okay
16:06:37 dansmith I don't have a strong opinion without seeing it, so maybe just have to pick one and see
16:06:41 tssurya ok awesome thanks
16:06:48 dansmith mriedem: sound okay?
16:06:49 tssurya so first I'll try with the manager and then we can see
16:06:50 mriedem i'll leave a couple of comments
16:07:22 tssurya thanks a lot dansmith and mriedem :D
16:10:48 mriedem done https://review.opendev.org/#/c/645611/12
16:13:10 tssurya ty
16:13:49 dansmith mriedem: yeah, so assuming your "don't do this for libvirt" assertion I think what you said in there is fine
16:13:49 openstackgerrit Merged openstack/nova stable/stein: Fix misuse of nova.objects.base.obj_equal_prims https://review.opendev.org/676289
16:42:22 alex_xu dansmith: mriedem sean-k-mooney anyway thanks for your time today, it is indeed I use a lot today :)
16:43:17 dansmith alex_xu: it's okay, and I think sean-k-mooney has more than 24 hours in his days, so he has plenty to spare :)
16:43:59 mriedem no problem
16:44:39 dansmith mars has 25 hour long days, so maybe sean-k-mooney is from mars
16:45:05 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Hook resource_tracker to remove stale node information https://review.opendev.org/676461
16:52:11 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: rt: only map compute node if we created it https://review.opendev.org/676463
16:54:51 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Hook resource_tracker to remove stale node information https://review.opendev.org/676461
16:55:04 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: rt: only map compute node if we created it https://review.opendev.org/676463
17:00:52 efried dustinc: Ima rebase the sdk series and bump ksa/sdk requirements and get it tracking against this devstack patch https://review.opendev.org/#/c/676268/
17:01:02 efried as soon as sdk 0.34.0 lands in u-c
17:01:36 efried when I say rebase - it's currently based on a really old nova commit, so I'm going to bring it up to current master.
17:05:03 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Hook resource_tracker to remove stale node information https://review.opendev.org/676467
17:12:37 rouk is there a particular reason why OS-FLV-DISABLED:disabled is not exposed via api client? is it intended to be manual api queries only?
17:15:28 rouk i see an old proposed patch from... a long time ago, but it was abandoned.
17:18:43 dansmith efried: on this:
17:18:44 dansmith https://review.opendev.org/#/c/675705/2/nova/tests/functional/regressions/test_bug_1839560.py
17:19:20 efried yah
17:19:22 dansmith efried: you're just noting that in real life we have a service with hostname "host1" but would normally get uuids for the node names, is that right?

Earlier   Later