Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-13
14:35:33 dansmith even if we can disable the ironic side, the nova side needs to not do that
14:36:25 dansmith tbh, the api setting the task state has kinda rubbed me the wrong way the whole time anyway,
14:36:39 dansmith as it seems like we should just forward this down to the compute and let it do its thing if needed
14:36:57 mriedem i mentioned in ealier review that the API could check the compute service version, but because volume-extended was merged w/o doing that and has the same issue, it was going to slide here as well
14:37:19 dansmith setting task state is to help with races to take action on events, and if we can't do the power state short-circuit because something else has started, it's fine to fail/log late
14:37:32 stephenfin mriedem: Question on https://review.opendev.org/#/c/631581/44/nova/conductor/tasks/cross_cell_migrate.py
14:37:57 mriedem dansmith: that was another thing i pointed out, the API code isn't handling UnexpectedTaskStateError if we lose a race so the API would 500 and fail for all events
14:38:10 mriedem *all events in that request
14:39:02 dansmith mriedem: yeah, so I think I'd use this as an argument to not make this something more complex on the api side
14:41:37 mriedem stephenfin: replied
14:45:05 mriedem stephenfin: i realized late that the cinder 3.44 check in there can be removed now https://review.opendev.org/#/c/649759/
14:45:15 mriedem i wrote the cross-cell patch before we bumped the min required cinder,
14:45:18 mriedem so i'll remove that in a follow up
14:45:33 stephenfin I also missed that. Yeah, follow-up would be good (y)
15:09:56 dansmith mriedem: I commented on tssurya's patch about the api task_state involvement, and also thought of another problem with that
15:26:18 mriedem dansmith: i don't think the libvirt thing is an issue if you're compute is train+
15:26:37 mriedem b/c the driver will raise NotImplementedError and @reverts_task_state will set the task_state to None
15:26:37 dansmith no?
15:26:54 dansmith is that a call?
15:26:56 mriedem i commented on that in one of the unit tests - as in that scenario is not asserted
15:27:09 mriedem @reverts_task_state is a decorator on the compute managermethod
15:27:27 dansmith ah
15:28:30 mriedem so i think we're covered there, it's just not tested :(
15:28:47 dansmith okay I see, she's got it on the helper method
15:28:54 dansmith I was pretty sure it was not on the event handler function itself
15:32:44 mriedem dansmith: efried: fyi i'll be out on friday, going camping up north in the bOuNdArY wAtErS; if i'm not back next monday it's either because a bear ate me or i illegally emigrated to canada
15:33:02 efried ack
15:33:34 efried you're going with someone you can outrun, right?
15:33:47 mriedem until then i'll be planning on how to live w/o running water and on hot dogs for 6 straight meals
15:33:53 mriedem oh of course
15:51:10 openstackgerrit Merged openstack/nova stable/ocata: fix unshelve notification test instability https://review.opendev.org/674907
15:52:36 openstackgerrit Matt Riedemann proposed openstack/nova master: FUP to I30916d8d10d70ce25523fa4961007cedbdfe8ad7 https://review.opendev.org/676228
15:55:49 openstackgerrit Shilpa Devharakar proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074
15:55:49 openstackgerrit Shilpa Devharakar proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
15:57:20 openstackgerrit Shilpa Devharakar proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
15:58:08 openstackgerrit Shilpa Devharakar proposed openstack/nova master: Enable request filter isolate_aggregates https://review.opendev.org/667952
15:59:25 openstackgerrit Takashi NATSUME proposed openstack/python-novaclient master: Microversion 2.75 - Multiple API cleanup changes https://review.opendev.org/676230
16:08:49 openstackgerrit Matt Riedemann proposed openstack/nova master: Add CrossCellMigrationTask https://review.opendev.org/631581
16:08:50 openstackgerrit Matt Riedemann proposed openstack/nova master: FUP to I4d181b44494f3b0b04537d5798537831c8fdf400 https://review.opendev.org/676231
16:09:21 mriedem wtf
16:09:40 mriedem oh sob
16:10:35 efried is mriedem wishing he had rebase=false in his .gitconfig?
16:10:48 mriedem no, i did git review -R, that's not the problem
16:11:25 mriedem the problem is i rebased HEAD~41, FUP to 41, git rebase --continue, FUP to 40, but that was not part of my main series so now it's diverged
16:11:49 openstackgerrit Matt Riedemann proposed openstack/nova master: Add CrossCellMigrationTask https://review.opendev.org/631581
16:12:00 sean-k-mooney efried: im not sure how people end up with git reveiw rebasing there patches without them asking. i dont have rebase=false in my .gitconfig but i dont think i have ever had that proablem and dont then to use -R
16:13:06 sean-k-mooney at least in my normal workflow of "git review -d <gerrit number>; edit, git commit --amend, git review" that has never been a issue i have had
16:13:07 openstackgerrit Matt Riedemann proposed openstack/nova master: FUP to I4d181b44494f3b0b04537d5798537831c8fdf400 https://review.opendev.org/676231
16:13:36 openstackgerrit Matt Riedemann proposed openstack/nova master: Execute TargetDBSetupTask https://review.opendev.org/633853
16:15:16 mriedem i'll just wait for the bottom things to merge and then do a proper rebase
16:16:04 efried mriedem: I don't think I understand the rolling upgrade with the ironic event thing.
16:16:43 efried If ironic sends the event to a nova that can't understand it, it'll just bounce, and the behavior will be as if we didn't have the feature, and the nova periodic will force the instance state like it does today -- right??
16:16:53 mriedem no
16:17:11 mriedem traditional upgrade sequence for nova and ironic is ironic first because the driver didn't do version discovery,
16:17:20 mriedem i think that's changed in recent releases but not sure how robust it is,
16:17:33 mriedem so you upgrade ironic, then the nova control plane, and you can have Stein computes for rolling upgrades, right?
16:17:42 mriedem so your nova api is train exposing 2.76
16:18:00 dansmith yup
16:18:08 mriedem ironic default to send the events and if ironic is configured for talking to nova,
16:18:08 efried ah, so this is really a coordination problem where we need to tolerate n-api > n-cpu
16:18:10 mriedem it will send the events
16:18:14 mriedem yes
16:18:24 efried it really has little to do with the ironic side sending events or not.
16:18:29 mriedem this is why the api does min compute service version checking in places
16:18:36 mriedem correct
16:18:38 dansmith correct, which is why I said the ironic tunable is not a solution
16:18:44 mriedem if the nova api was smarter ironic wouldn't care
16:18:56 efried okay, I'm up to speed, thanks.
16:19:03 dansmith even if the power state sync will fix it (not sure it will), ironic will lock up the instance for some period of time until it unlocks, which is not okay
16:19:22 mriedem the power state sync task will ignore the instance if the task_state is not None
16:19:36 mriedem so if the api gets an event, sets the task_state, then casts off to stein compute that ignores it, the sync task won't fix it
16:19:46 dansmith yeah, I thought it wouldn't fix it up, but I can never remember the million or so conditionals int here
16:19:55 dansmith no response from tssurya right?
16:20:03 mriedem if it's thursday and you're using vcenter you get a free vm
16:20:06 mriedem correct
16:31:42 cdent mriedem: nothing ever comes for free from vcenter
16:33:40 mriedem heh
17:40:28 openstackgerrit Matt Riedemann proposed openstack/nova stable/stein: Convert HostMapping.cells to a dict https://review.opendev.org/676256
17:40:28 openstackgerrit Matt Riedemann proposed openstack/nova stable/stein: Delete unused get_all_host_states method https://review.opendev.org/676255
17:40:29 openstackgerrit Matt Riedemann proposed openstack/nova stable/stein: Cache host to cell mapping in HostManager https://review.opendev.org/676257
17:48:04 openstack Launchpad bug 1840068 in OpenStack Compute (nova) "(lxc) Instance failed to spawn: TypeError: object of type 'filter' has no len()" [High,Confirmed]
17:48:04 mriedem the nova-lxc job finds it's first bug https://bugs.launchpad.net/nova/+bug/1840068
17:50:36 sean-k-mooney oh you/someone got an lxc job running? cool
17:51:41 sean-k-mooney looks like its more or less in teh same nbd mount region of the code that the other issue was in
17:51:52 mriedem this is a py3 specific failure
17:53:16 sean-k-mooney oh that is becasue filter is nolonger a keyword
17:53:26 sean-k-mooney and not you have to import it form itertools
17:53:40 sean-k-mooney so we need to use six
17:53:56 sean-k-mooney s/keywork/built in fucntion/
18:01:24 mriedem that implies the problem would actually be in py27 rather than 3
18:01:25 mriedem https://github.com/openstack/nova/blob/e3a55493ba0cb405f14b9090282d93eb230c5715/nova/virt/disk/mount/nbd.py#L44
18:01:28 mriedem since it's using filter()
18:01:44 mriedem i think the problem is passing an iterable from filter() to random.shuffle
18:02:00 sean-k-mooney well filter is a builtin in python 2
18:02:08 sean-k-mooney in python 3 its not and you have to import it
18:02:39 sean-k-mooney so i was about to push what i think is a fix
18:02:40 mriedem TypeError: object of type 'filter' has no len()
18:02:40 mriedem ok i'm not sure what was used then, because we
18:02:59 sean-k-mooney it might be something else
18:03:15 sean-k-mooney does this show up in unit tests

Earlier   Later