Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-03
15:47:23 efried jroll: Okay, but the tempest tests presumably use some kind of API to create/delete the nodes. Because they're ending up registered in placement somehow.
15:47:28 jroll I think it's handled in update_available_resource or something
15:47:49 sambetts they are just created in ironic, then nova syncs with ironic and picks up the new nodes
15:47:55 jroll efried: yes, POST ironic-host/v1/nodes, then the ..... yes that
15:48:31 efried oho
15:48:42 efried it's happening *after* _update_available_resource_for_node
15:49:32 efried sambetts: How reproducible is this thang?
15:51:00 openstackgerrit Stephen Finucane proposed openstack/nova master: network: Always retrieve network information if available https://review.openstack.org/564444
15:51:01 openstackgerrit Stephen Finucane proposed openstack/nova master: network: Retrieve tunneled status in '_get_physnet_info' https://review.openstack.org/564445
15:51:02 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Add NUMACell.network_metadata https://review.openstack.org/564439
15:51:03 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Add NUMATopologyLimits.networks https://review.openstack.org/575486
15:51:04 openstackgerrit Stephen Finucane proposed openstack/nova master: hardware: Start accounting for networks in NUMA placement https://review.openstack.org/564448
15:51:05 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Add RequestSpec.network_metadata https://review.openstack.org/564442
15:51:06 openstackgerrit Stephen Finucane proposed openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452
15:51:07 openstackgerrit Stephen Finucane proposed openstack/nova master: conf: Add '[neutron] physnets' and related options https://review.openstack.org/564440
15:51:08 sambetts every single one of my CI runs hits it at the moment, I expect the reason the upstream ironic CI isn't hitting it is because setting tempest concurrency=1
15:51:08 openstackgerrit Stephen Finucane proposed openstack/nova master: libvirt: Start populating NUMACell.network_metadata field https://review.openstack.org/564441
16:08:57 openstack Launchpad bug 1739325 in OpenStack Compute (nova) "Server operations fail to complete with versioned notifications if payload contains unset non-nullable fields" [Medium,Confirmed] - Assigned to Balazs Gibizer (balazs-gibizer)
16:08:57 mriedem gibi: so i'm looking at https://bugs.launchpad.net/nova/+bug/1739325 again and Flavor.disabled isn't something you can set from the API as far as i can tell
16:11:21 gibi mriedem: but then why we even have a disabled field?
16:11:43 mriedem idk, i'm not seeing in git history where it was added, or if there is a way to ever set it via cli or something
16:11:47 mriedem dansmith: do you remember?
16:13:15 openstack Launchpad bug 1259262 in python-novaclient "there is no api or cli to enable/disable a flavor" [Wishlist,Opinion]
16:13:15 gibi mriedem: https://bugs.launchpad.net/nova/+bug/1259262
16:13:48 gibi pretty old whishlist bug to make the disabled flag changeable
16:14:06 mriedem https://github.com/openstack/nova/commit/f371198
16:16:20 gibi mriedem: nice finding. Even this patch uses direct db access to created a disabled instance_type https://github.com/openstack/nova/commit/f371198#diff-59404890aec6423ece734730c153addbR752
16:17:08 gibi I think we can conclude that there is no way to set it outside of the API
16:17:21 mriedem so i think the issue might be related to https://github.com/openstack/nova/blob/master/nova/compute/flavors.py#L52
16:17:35 mriedem and how we used to store the embedded flavor in the instance based on the instance system_metadata
16:17:41 mriedem which didn't include the 'disabled' key
16:18:27 mriedem so likely any really old instance didn't have the instance_type_disabled key in it's system_metadata
16:18:58 mriedem which might explain why we're hitting "Field `disabled' cannot be None" when extracting these things and loading them into Flavor objects
16:19:37 gibi this very well be the explanation
16:20:27 gibi dansmith: would this ^^ be enough explanation to revive https://review.openstack.org/#/c/529194/ ?
16:21:09 gibi dansmith: you were -1 on that patch as we did not know why can be the instance.flavor.disabled undefined
16:21:46 gibi mriedem: thanks for the investigation I was not able to find this by myself
16:22:40 dansmith gibi: I'd really rather not embed that behavior in the flavor object.. either we should set those properties when we load an older flavor from the instance_extra, or (better) just be graceful about it when we're emitting the notification
16:26:11 mriedem left a comment here about how i think it all tied together https://bugs.launchpad.net/nova/+bug/1739325/comments/9
16:26:12 openstack Launchpad bug 1739325 in OpenStack Compute (nova) "Server operations fail to complete with versioned notifications if payload contains unset non-nullable fields" [Medium,Confirmed] - Assigned to Balazs Gibizer (balazs-gibizer)
16:26:29 gibi mriedem: thanks
16:27:58 gibi dansmith: do we still load those old flavors from the system_metadata or we loaded once and copied into the instance without the disabled field?
16:28:17 dansmith no, we don't, they've all been migrated to extra now
16:29:02 gibi dansmith: then the first option is not a real solution
16:29:40 dansmith gibi: no, the first option I meant was when we're in the context of the instance, loading a flavor from instance_extra
16:29:43 mriedem wouldn't we just implement obj_load_attr (lazy-load) for Flavor.disabled?
16:30:14 dansmith we could, but that also seems obscure to me,
16:30:28 dansmith because the flavor could be coming from the instance, or from the actual db
16:30:40 dansmith and we wouldn't want the disabled default to affect the latter
16:30:54 dansmith it seems like we should patch it up if/when we're loading from extra and notice that field is gone
16:30:54 mriedem so then add default=False to 'disabled': fields.BooleanField(), ?
16:31:29 dansmith that's basically the same as the lazy-load, although slightly less icky, and one of the things I suggested in the original patch
16:32:22 mriedem ok so maybe handle in _flavor_from_db
16:32:25 mriedem Instance._flavor_from_db
16:32:31 mriedem if 'disabled' not in flavor_info
16:32:39 mriedem flavor_info['disabled'] = False
16:32:41 dansmith exactly
16:32:45 mriedem ok i'll push that up
16:32:46 gibi mriedem, dansmith I got it
16:32:48 dansmith because there we know what we're dealing with
16:33:00 dansmith and a comment about old instance flavors will be quite relevant
16:33:47 dansmith _load_flavor will get it implicitly if we're actually lazy-loading instance.flavor too, so that is good
16:34:35 zzzeek nova peeps, dansmith / mriedem , what happens when someone in Newton sets "workers=0" in nova.conf, does that mean default to number of CPUs ?
16:35:00 gibi mriedem, dansmith: thanks for the help
16:36:49 mriedem dansmith: gibi: pre-tests https://gist.github.com/mriedem/3f1560a7e9697ade633ab11b6aab8c62
16:37:01 mriedem zzzeek: yes
16:37:05 zzzeek mriedem: thanks!
16:37:10 dansmith mriedem: sure
16:37:54 gibi mriedem: LGTM
16:49:39 efried sambetts: Sorry, it's been a day. Can you tell me where/how to run this thing?
16:50:16 efried If I push a nova patch, do I have to push something in ironic with a Depends-On? Looks like an experimental job, needs special incantation?
16:51:30 efried sambetts: Can I mod the commit message on https://review.openstack.org/#/c/514312/ to add the Depends-On?
16:52:26 openstackgerrit Jay Pipes proposed openstack/nova master: move lookup of provider from _new_allocations() https://review.openstack.org/579920
16:52:27 openstackgerrit Jay Pipes proposed openstack/nova master: placement: delete auto-created consumers on fail https://review.openstack.org/579921
16:55:05 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Delete orphan nodes before updating resources https://review.openstack.org/579922
16:57:14 sambetts efried: you can mod the commit message to add the Depends-On, then you need to leave a "cisco-experimental" comment to trigger our experimental job queue which has some other fixes for our ironic job
16:57:30 efried sambetts: Roger wilco, it's on the way...
16:57:39 sambetts efried: thanks so much for looking into this!
16:57:43 efried sambetts: the experimental fix is ---^ btw
16:58:25 sambetts efried: oh nice thats quite an eligant fix
16:59:15 efried sambetts: I think it's probably going to work, but I don't think that's the end of it. We're seeing similar races in other places as well (though not as consistently), so I think I really need to implement that retry loop I mentioned earlier.
17:01:15 sambetts yeah I feel like the whole compute claiming thing (particularly with ironic) is really racey, I still think we have the race to do with a claim being released on an ironic node that then goes into cleaning, doesn't get removed from the resource tracker before a new claim can be made on it
17:01:39 sambetts but thats another story
17:01:53 dansmith sambetts: there's no compute claiming anymore
17:03:38 sambetts might be using the wrong words, still a race between the scheduler and the resource tracker
17:03:39 efried dansmith: TL;DR we're running into races now because rt._update is run both during periodic and from instance_claim. Both of them hit update_[from_]provider_tree asynchronously, and freak each other out.
17:03:45 efried Yeah, what sambetts said.
17:04:15 dansmith there should be no changes coming from resource tracker
17:04:35 efried The periodic is designed to be self-healing, but if the race makes the claim side fail, that's bad.
17:04:41 dansmith or do you mean a race between an allocation and inventory changes/
17:05:24 efried dansmith: Yeah, or aggregate changes, which are happening a relatively lot now that we're mirroring host aggs to placement.
17:05:46 openstackgerrit Matt Riedemann proposed openstack/nova master: Default embedded instance.flavor.disabled attribute https://review.openstack.org/579925
17:06:03 mriedem dansmith: gibi: ^
17:06:13 dansmith well, I guess I see that as a major improvement over the races we used to have between scheduling and claiming (on the compute)
17:06:29 dansmith but yeah, "compute claiming" is the wrong term since that's not what happens now
17:06:31 efried dansmith: What we're seeing in tempest envs is aggs being added/removed as hosts are brought up & down in concurrent tests.
17:06:46 dansmith hosts aren't brought up and down in tempest
17:06:50 efried dansmith: I've been thinking the real solution will be a retry loop around this chunk: https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L883-L914
17:06:53 dansmith but tempest may be creating/destroying aggregates
17:07:51 efried dansmith: Here's an example: http://logs.openstack.org/69/556669/11/check/nova-multiattach/b3195af/job-output.txt.gz#_2018-06-22_14_50_02_095506

Earlier   Later