Earlier  
Posted Nick Remark
#openstack-nova - 2017-03-21
13:29:10 sfinucan yup, I'd completely agree...outside of '/conf'
13:29:37 sdague no, anyway, we don't want different rules in different places
13:29:46 sfinucan I want to enforce the "consistency is key" mindset in there, in the hopes that it will deliver consistency in the end result
13:29:52 sfinucan which, for the most part, it has been doing
13:30:42 sdague well, I disagree that over prescribed code indentation really impacts useful end user docs :)
13:31:01 sfinucan but it does describe people's mindsets going in there
13:32:05 sfinucan Also, I've done enough of work with that aspect of nova to surely justify the benefit of the doubt
13:33:23 bauzas jroll: mriedem: okay, gaining context :)
13:33:31 sdague I fought really hard against the whitespace wars in the past of people being overly prescriptive, I'm not really interested in bringing that mindset back for any reason
13:35:40 openstackgerrit Jim Rollenhagen proposed openstack/nova master: Ironic: hardcode min_unit for standard resources to 1 https://review.openstack.org/448098
13:36:09 jroll EmilienM: cdent: bauzas: this should definitely fix the bug, trying to figure out now if I need to change allocations code as well so we don't schedule two (undersubscribed) instances to one node
13:36:33 EmilienM jroll: would you recommend us to change something in tripleo also?
13:37:06 jroll EmilienM: well, I believe flavors should always exactly match nodes, but tripleo not doing that is a "feature", so I'd leave it alone
13:37:19 EmilienM ok
13:37:26 EmilienM jroll: thank you!
13:37:40 jroll np :)
13:37:54 EmilienM jroll: I'm going to try this one
13:38:11 jroll EmilienM: thanks
13:40:15 EmilienM jroll: https://review.openstack.org/#/c/447522/ - I'll let you know how it foes
13:40:17 EmilienM goes*
13:41:31 jroll thanks
13:41:54 bauzas EmilienM: jroll: okay, beginning to review https://review.openstack.org/#/c/448098/ FWIW
13:43:00 EmilienM bauzas: thx
13:44:02 bauzas jroll: oh, excellent catch on https://review.openstack.org/#/c/448098/1/nova/virt/ironic/driver.py
13:44:38 jroll bauzas: yeah, was fun
13:44:43 bauzas I think it's a situation where the scheduler is still needed until we implement all the features per the placement engine
13:44:47 jroll we are going to have to change how we do allocations here though :(
13:45:21 bauzas jroll: why so ? you're telling placement to accept returning RPs that have more resources
13:45:25 bauzas jroll: ah I see
13:45:33 jroll :P
13:45:41 bauzas jroll: so you would create an allocation that would eat all the left resources ?
13:45:41 jroll yeah, we don't want to leave free resources there
13:45:56 bauzas yeah, I honestly thought it was already the case
13:46:06 jroll bauzas: maybe. or hack instance.flavor to eat everything
13:46:10 bauzas an ironic allocation is free-or-none
13:46:13 cdent so does this mean we need yet more special casing?
13:46:33 mriedem jroll: did you run functional tests?
13:46:43 bauzas jroll: well, operators defined flavors that weren't really meaning this or that number of resource usage
13:46:48 mriedem jroll: i think there is a functional RT test that will fail
13:46:51 jroll bauzas: right, we have code that does this in our virt driver, but the RT decides we're wrong
13:47:05 jroll mriedem: there probably is, I haven't looked at them
13:47:13 jroll nor ran
13:47:13 bauzas jroll: we could tho tell them to add an extra spec that would signal 'eat me all the cake'
13:47:20 bauzas I mean a trait
13:47:28 mriedem that's not a trait
13:47:36 bauzas a what then ?
13:47:37 mriedem that's like a config option
13:47:41 jroll 'indivisible' is a trait :P
13:48:08 jroll cdent: yes, somewhere. unclear (to me) where
13:48:11 mriedem i guess i need to read the traits spec on the definition
13:48:15 bauzas VCPU=5&MEM_GB=1&eat_me_all_thecake ?
13:48:27 jroll mriedem: I'm mostly joking :)
13:49:12 bauzas jroll: that said I'm concerned we could merge https://review.openstack.org/#/c/448098/1/nova/virt/ironic/driver.py without modifying allocations that would allow to pick the same ironic node for two instances
13:49:42 jroll bauzas: I mean, I can WIP it, I just put it up for now so EmilienM could test and such
13:49:42 bauzas jroll: I do wonder if fixing ironic allocations shouldn't be merged first
13:50:01 bauzas jroll: well, it's a race
13:50:07 bauzas jroll: so the CI could be happy tho
13:50:24 jroll bauzas: let me be clear, I don't want this merged without fixing allocations as well
13:50:36 bauzas a-ah ok, then I agree
13:50:46 jroll bauzas: and if tripleo can boot at least one instance, it's progress
13:50:55 jroll or rather proves that was the issue
13:51:24 bauzas jroll: AFAIK, the Exact* filters are not default in config right?
13:51:32 bauzas jroll: speaking of the IronicHostManager config
13:51:39 bauzas I can verify
13:51:48 bauzas but I'm pretty sure TripleO hasn't modified that
13:52:12 jroll bauzas: it is a default, I think, if CONF.filter_scheduler.use_baremetal_filters=true
13:52:35 bauzas jroll: I know we don't have the same enabled filters if that conf opt is in use, yep
13:52:49 bauzas jroll: I just don't remember if the Exact* filters are part of those
13:52:56 jroll yeah, confirmed, it isn't default but is part of the baremetal set
13:53:04 jroll conf/scheduler.py line 298
13:54:20 bauzas jroll: so then the scheduler should get the exact same number of nodes with or without that fix you wrote
13:54:32 bauzas oops
13:54:41 bauzas the scheduler should *return*
13:55:00 bauzas but lemme see the 3O config
13:55:17 jroll bauzas: http://logs.openstack.org/22/447522/1/check-tripleo/gate-tripleo-ci-centos-7-ovb-nonha/7563947/logs/etc/nova/nova.conf.txt.gz
13:55:20 bauzas yup
13:55:21 jroll use_baremetal_filters=False
13:55:41 bauzas EmilienM: is the above the undercloud nova.conf ?
13:55:56 mriedem bauzas: jroll's fix isn't any different from how it worked before get_inventory was added
13:55:59 mriedem unless i'm missing something
13:56:07 mriedem so if you're worried about an allocation bug, it's latent
13:56:34 bauzas mriedem: jroll's fix tries to address the fact that operators could ask the scheduler to return hosts that have more space than asked
13:56:53 jroll mriedem: hm, you might be right
13:56:53 EmilienM bauzas: yes
13:57:08 bauzas okay, so the problem is there
13:57:23 bauzas lemme restate
13:57:48 mriedem jaypipes: in case you haven't seen yet, jroll found the cause of the bug with the get_inventory stuff
13:57:55 bauzas Ironic pre-jroll's fix is only accepting the default config (ie. having Exact* filters)
13:57:59 jaypipes mriedem: sorry, meeting hell...
13:58:06 jaypipes mriedem: what was the cause, jroll?
13:58:16 mriedem min_unit was equal to max_unit
13:58:19 bauzas but TripleO isn't using those specific filtrers
13:58:22 mriedem plus bm filters
13:58:22 jroll jaypipes: min_unit==max_unit, tripleo uses flavors smaller than the node
13:58:29 EmilienM scheduler_use_baremetal_filters is set to False by default in puppet-nova, I believe it's the default in Nova, but let me check
13:58:33 jroll mriedem: not related to baremetal filters
13:58:36 jroll EmilienM: that's correct
13:58:44 jaypipes jroll: ahhh.
13:59:00 jaypipes jroll: good. we can blame edleafe then. :P
13:59:00 bauzas so it hits the bug that jroll tries to fix, but which shouldn't be visible unless you modify the specific default values for the list of baremetal filters

Earlier   Later