Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-14
20:52:56 melwitt I know we've talked before about whether we should except-continue for things that raise during update_available_resource. maybe this would be an acceptable one. for anything else, we let it halt
20:53:19 openstackgerrit Eric Fried proposed openstack/nova master: WIP/PoC: Bypass ironicclient for node.get https://review.openstack.org/642899
20:53:28 dansmith it of course depends on the context of the issue
20:53:38 dansmith if it's unable to do something fundamental, it should abort
20:54:05 dansmith if it just can't expose support for some thing because of a new libvirt (which is sounds like in this case) then of course it should just not break on forward compat
20:54:07 melwitt yeah, I mean in update_available_resource, if InvalidArchitectureName is raised
20:55:02 dansmith it's some virt specific place it's crashing I imagine, so it would be in there, not directly in update_available_resource()
20:55:17 dansmith you surely don't want the resource tracker ignoring something like ArchNotSupported
20:55:32 dansmith but the libvirt driver can know if it's okay to skip that
20:55:45 melwitt oh
20:56:00 dansmith _get_instance_capabilities()
20:56:11 dansmith just needs to catch and ignore that
20:56:13 openstackgerrit Mark Goddard proposed openstack/nova master: Add armv6l to Architecture enum https://review.openstack.org/643455
20:56:15 mriedem this is why it breaks https://review.openstack.org/#/c/356067/
20:56:45 mriedem i mean, there is no major reason for that change except "use the enums we have"
20:56:50 mriedem w/o considering side effects
20:57:39 mgoddard I'll leave this one in your capable hands - it's long past home time here. Thanks for your input
20:57:45 dansmith mriedem: one reason (even if presumptive) so that each driver doesn't call the arches something slightly different causing the api to return inconsistent results
20:57:46 mriedem it would blow up before that as well https://review.openstack.org/#/c/356067/12/nova/compute/arch.py
20:57:57 mriedem so this is super latent behavior
20:58:26 mriedem dansmith: like the 50 different ways to describe power
20:58:32 dansmith point being, the bug (aside from the missing arch) is just in the libvirt driver being stupid
20:59:39 dansmith we need a catch and ignore right here, is all: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L5942-L5945
20:59:42 melwitt mgoddard: ok, thanks for letting us know. I can open a bug for that and propose a patch, if no one else wants to
21:00:23 dansmith melwitt: I'm already rage typing it
21:00:34 mriedem i would not be surprised if that value is used elsewhere though
21:00:39 melwitt dansmith: cool
21:00:39 mriedem and could blow up elsewhere
21:01:08 dansmith mriedem: could be, but it's clearly wrong for that loop to not continue
21:01:32 mriedem yeah sure
21:01:39 mriedem just exclude that one with a warning i guess
21:02:12 dansmith I don't thin it should even be that, otherwise we'll log that over and over for every instance and every run of the periodic
21:02:28 mriedem oh right this is the periodic
21:02:44 mriedem well you could log once and set a flag in the driver instance, or are you just going to exit?
21:03:16 dansmith no, not exit that makes no sense, just continue the loop
21:03:26 dansmith nova should expose whatever arches libvirt reports that it supports, that's really all
21:03:35 dansmith this code is intending to do that, assuming it knows about everything
21:11:38 openstackgerrit Dan Smith proposed openstack/nova master: Avoid crashing while getting libvirt capabilities with unknown arch names https://review.openstack.org/643458
21:14:42 openstackgerrit Dan Smith proposed openstack/nova master: Avoid crashing while getting libvirt capabilities with unknown arch names https://review.openstack.org/643458
21:26:53 mriedem jroll: TheJulia: fyi https://bugs.launchpad.net/nova/+bug/1815799
21:26:53 openstack Launchpad bug 1815799 in OpenStack Compute (nova) "Attempting to delete a baremetal server places server into ERROR state" [Low,Confirmed]
21:27:03 mriedem maybe a simple retry loop in the ironic driver on node destroy would solve that
21:28:03 TheJulia mriedem_away: we should be able to just pass on it if if we've already started the process of deleting the node in ironic
21:36:45 openstackgerrit Takashi NATSUME proposed openstack/nova master: Remove mox in virt/test_block_device.py https://review.openstack.org/566153
21:37:29 openstackgerrit Takashi NATSUME proposed openstack/nova master: Fix cleaning up console tokens https://review.openstack.org/637716
21:44:22 openstackgerrit Marcin Juszkiewicz proposed openstack/nova master: virt/arch: use 'armv6l' for ARMV6 architecture https://review.openstack.org/643467
21:46:33 hrw that ^^ change may fix Kolla CI error
21:57:17 melwitt dansmith ^
22:02:54 hrw and sorry for writing 3rd patch to what looks the same
22:07:12 hrw dansmith: after reading https://review.openstack.org/#/c/643458 and https://review.openstack.org/#/c/643455 I think that 643355 is wrong, your patch is good (left comment) and mine https://review.openstack.org/643467 just fixes to comply with capabilities contents
22:07:30 hrw but that's me - random guy from the internet D:
22:13:24 openstackgerrit Merged openstack/nova master: Customize irrelevant-files for nova-live-migration job https://review.openstack.org/643346
22:13:43 openstackgerrit Merged openstack/nova master: Re-enable Ceph in live migration testing https://review.openstack.org/643150
22:21:35 openstackgerrit Eric Fried proposed openstack/nova master: WIP/PoC: Bypass ironicclient for node.get https://review.openstack.org/642899
22:26:57 dansmith melwitt: what are you pointing me to?
22:27:25 melwitt dansmith: hrw's patch https://review.openstack.org/643467
22:28:04 melwitt it's related to the unknown arch blowup, sort of
22:28:28 dansmith yeah, I'm not sure which is the official arch name and/or if they're both legit as I'm not an arm specialist
22:28:52 dansmith but my patch is still something we should do as (as hrw said)
22:29:12 hrw dansmith: armv6 armv6l depends on kernel.
22:29:13 melwitt hrw: if you think https://review.openstack.org/643455 is wrong, it would be helpful to put your comment on it about why. I'm curious anyway
22:29:23 dansmith hrw: my concern would be it might also depend on libvirt behavior
22:29:40 melwitt dansmith: yeah, not saying your patch isn't something we should do. just that along that line, if you knew anything about changing that enum, hrw's patch was there
22:29:52 dansmith hrw: because if the l just means long or something, then I would expect both or either could be emulatable
22:30:51 dansmith melwitt: hopefully it's obvious that hrw's patch to change the value of the enum is not okay :)
22:31:29 melwitt dansmith: gah, I thought maybe you could comment on the review
22:31:40 melwitt nevermind me
22:31:55 hrw dansmith: libvirt mentions only armv6l and armv7l.
22:31:59 dansmith melwitt: I just did
22:32:05 melwitt thanks
22:32:14 dansmith hrw: on your system you mean right?
22:32:17 hrw https://gitlab.com/search?utf8=%E2%9C%93&snippets=&scope=&search=VIR_ARCH_ARMV6L&project_id=192693#L30
22:32:23 hrw in libvirt code
22:33:29 dansmith hrw: okay, like I said, I'm not really able to say whether armv6 or armv6l (or both) are right
22:33:49 dansmith but we can't just make the change you've proposed because it has effects to versioned objects and will break communication between services
22:33:55 hrw ok.
22:34:03 hrw perfectly fine for me.
22:34:30 dansmith hrw: I will say, if you're right that it's always armv6l, then we definitely need a variant of your patch over mgoddard's
22:34:55 hrw dansmith: let me discuss it tomorrow with libvirt guys, ok?
22:34:58 hrw 23:34 here
22:35:13 dansmith sure
22:36:05 hrw as nova/virt/libvirt gets capabilities from libvirt so is it armv6 or armv6l is more or less their decision. if they say 'no, it was always armv6z' then we both are wrong
22:36:57 dansmith yeah
22:36:59 hrw the good point is that armv6 is not able to be a host so it is only guest issue
22:38:07 openstackgerrit Kevin Rasmussen proposed openstack/nova master: created AggregateSubnetAvailabilityFilter https://review.openstack.org/633178
22:38:17 dansmith I'm not sure that's relevant to the thing that nova has to do in this case, but.. okay :)
22:38:32 hrw nothing in this case ;D
22:42:11 hrw ok, went, asked, go to sleep. wake up in ~6h
22:42:27 dansmith go :)
22:44:21 hrw and I promised myself to not play with nova for at least a year... ;d
22:44:43 hrw have a nice day
23:03:00 openstackgerrit Eric Fried proposed openstack/nova master: Remove [ironic]api_endpoint option https://review.openstack.org/643483
23:03:27 openstackgerrit Julia Kreger proposed openstack/nova master: [ironic] ignore conflict upon cleaning up node https://review.openstack.org/643484
23:03:36 TheJulia mriedem_away: melwitt: ^^^
23:06:11 melwitt TheJulia: commented. I had thought jroll or someone had said we shouldn't do that in _cleanup_deploy unilaterally
23:07:21 TheJulia I really don't know what to do then
23:10:56 melwitt TheJulia: well, I just mean that try-except would move to the destroy() method
23:11:13 TheJulia I'm checking to see if we can do that
23:11:25 melwitt if I'm remembering correctly that it should be done for all clean deploys. I might be wrong
23:11:32 melwitt *shouldn't

Earlier   Later