| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-15 | |||
| 13:56:03 | gibi | bauzas: we making your job security better :) | |
| 13:56:11 | bauzas | remember, when you say "it's good to merge first and then see bugs", some people are actually doing bugs | |
| 13:56:31 | bauzas | gibi: well, no | |
| 13:56:40 | bauzas | gibi: being off upstream is bad for me :( | |
| 13:56:59 | bauzas | of course, it's nice for our customers | |
| 13:57:36 | bauzas | but tbh, I prefer to be conservative and not have bugs because we tried to merge very quickly without verifying | |
| 13:57:53 | bauzas | so customers would be happy and *I* would be happy | |
| 13:57:56 | gibi | bauzas: do you have an overall feeling that openstack produced more bugs lately than before? or what changed is that you are more involved with customer support | |
| 13:58:21 | bauzas | for placement and instance groups ? surely | |
| 13:59:05 | bauzas | what folks don't know if that most of our customers were using Newton until 6 months ago | |
| 13:59:12 | bauzas | now they use Queens | |
| 13:59:21 | cdent | so clearly any changes we are making now aren't really all that relevant | |
| 13:59:36 | bauzas | cdent: until the next year, for sure | |
| 13:59:39 | bauzas | but then ? | |
| 13:59:59 | bauzas | unless you want me going to another company... /o\ | |
| 14:01:34 | gibi | bauzas: do you see some missing test coverage what OpenStack should add to avoid some of your painfull bugs? | |
| 14:01:52 | bauzas | good question | |
| 14:02:37 | bauzas | what I know is that we have less people working on OpenStack | |
| 14:03:06 | bauzas | so, for example, if we have a problem, only a few from our team could help to fixing it | |
| 14:03:25 | bauzas | also, we have less operators testing OpenStack | |
| 14:03:44 | bauzas | so, somehow, it means that when we merge something, we discover bugs later | |
| 14:03:58 | bauzas | than previously | |
| 14:04:35 | bauzas | now, the real question is : if we were having better test coverage, would that be better ? oh yes, of course | |
| 14:04:42 | bauzas | but | |
| 14:04:50 | bauzas | we would still miss bugs | |
| 14:05:07 | bauzas | so at the end of the day, that would still impact us | |
| 14:06:24 | gibi | yeah, I feel the less people part too. Unfortunately half of the reason I'm paid to work on OpenStack is doing features, so I cannot just say no to feature work :/ | |
| 14:07:06 | bauzas | of course | |
| 14:07:15 | bauzas | we're all paid by customers | |
| 14:07:26 | bauzas | I'm not saying no to features | |
| 14:08:28 | bauzas | tbc, I just provided my opinion now because it was looking that we were not thinking about what means us quickly merging things | |
| 14:08:41 | kashyap | bauzas: Not quite; less features, more on: maintainability, stability, robustness, performance (a feature) | |
| 14:08:43 | bauzas | the reshape series is a good example | |
| 14:08:55 | bauzas | I wish I could have worked earlier on it | |
| 14:08:56 | kashyap | And ... "don't fall apart if you sneeze", so on | |
| 14:09:25 | bauzas | so, now, I'm just triying to test a lot of things | |
| 14:09:41 | bauzas | to make sure we don't have problems | |
| 14:09:55 | cdent | I reckon things could be improved quite a bit if there was a closer tie between the code being worked on now and people using it so that there were more people making sure it was okay | |
| 14:09:58 | bauzas | anyway, back to https://review.openstack.org/#/c/636591/3/nova/virt/libvirt/driver.py | |
| 14:10:02 | cdent | that time-related disconnect is a problem | |
| 14:10:09 | bauzas | cdent: I agree with you | |
| 14:10:14 | gibi | cdent: true | |
| 14:10:23 | cdent | but it is also a problem that our systems are so complex (much of the time) that they are hard to test and experiment with | |
| 14:10:27 | bauzas | it's difficult since we have some time between the merge and the usage | |
| 14:10:43 | cdent | it would be nice to figure out a way to change that | |
| 14:10:43 | bauzas | cdent: heh, Nova is 2M LOCs AFAIK | |
| 14:10:48 | cdent | ouch | |
| 14:11:01 | cdent | well I've done my part to make nova smaller, I hope someone else will too | |
| 14:11:04 | cdent | brb | |
| 14:11:06 | gibi | cdent: our downstream development switched from mitaka to pike recently and that already helped me to get actionable feedback from the downstream teams | |
| 14:11:25 | bauzas | anyway, we're diverting a lot now from real work | |
| 14:11:39 | bauzas | it's more an hallways discussion or a PTG one if you prefer | |
| 14:11:52 | bauzas | but I'm very sad to not have time to help more Nova | |
| 14:12:07 | bauzas | like for example, doing upstream bug triage | |
| 14:28:22 | mriedem | dansmith: i'm +1 on the down cell series up through the api change https://review.openstack.org/#/c/591657/ just waiting on zuul results if you want to step through those today | |
| 14:29:09 | belmoreira | mriedem: dansmith: does it make sense? https://bugs.launchpad.net/nova/+bug/1816034 or I'm missing something? | |
| 14:29:10 | openstack | Launchpad bug 1816034 in OpenStack Compute (nova) "Ironic flavor migration and default resource classes" [Undecided,New] | |
| 14:30:26 | dansmith | mriedem: cool, I meant to get to them yesterday and got distracted | |
| 14:31:15 | dansmith | belmoreira: mriedem probably remembers better than I, but there was something recently about this | |
| 14:38:38 | mriedem | belmoreira: and you have this enabled yes? https://review.openstack.org/#/c/609043/ | |
| 14:38:49 | mriedem | which is probably why the ironic driver isn't reporting standard resource class inventory | |
| 14:44:41 | mriedem | i wonder if we should just set IronicDriver.requires_allocation_refresh = not CONF.workarounds.report_ironic_standard_resource_class_inventory | |
| 14:45:07 | mriedem | because if we're not going to report the standard inventory we shouldn't try to put allocations for those standard inventory resource classes | |
| 14:46:19 | openstackgerrit | Merged openstack/nova master: Libvirt: do not set MAC when unplugging macvtap VF https://review.openstack.org/624842 | |
| 14:46:30 | belmoreira | mriedem: is set to False | |
| 14:47:05 | mriedem | yeah so i think simply setting IronicDriver.requires_allocation_refresh = not CONF.workarounds.report_ironic_standard_resource_class_inventory would fix the issue | |
| 14:47:26 | mriedem | err, | |
| 14:47:28 | mriedem | nix the 'not' | |
| 14:48:42 | mriedem | that's probably easier than mucking with the _pike_flavor_migration code in stable branches since that code is all gone in stein | |
| 14:49:45 | belmoreira | mriedem: not sure if I'm following. What you are suggesting is to disable IronicDriver.requires_allocation_refresh | |
| 14:50:06 | mriedem | correct, which is what you said you did as a workaround in the bug | |
| 14:52:01 | mriedem | unless that will regress the fix for bug 1724589 somehow | |
| 14:52:02 | openstack | bug 1724589 in OpenStack Compute (nova) pike "Unable to transition to Ironic Node Resource Classes in Pike" [High,Fix committed] https://launchpad.net/bugs/1724589 - Assigned to Matt Riedemann (mriedem) | |
| 14:54:09 | belmoreira | mriedem: we are fixing this in our instances_extra because we will need to recreate the allocations/RP. To make sure RP will not not change uuid in a sharded nova-compute setup with ironic | |
| 14:54:31 | belmoreira | otherwise allocations will not be recreated | |
| 14:54:46 | mriedem | you mean for old instances | |
| 14:54:53 | mriedem | otherwise the scheduler would create the allocations | |
| 14:55:10 | belmoreira | yes, for old instances | |
| 14:57:00 | belmoreira | because I would like to have https://review.openstack.org/#/c/571535/ | |
| 14:58:16 | mriedem | and it looks like http://git.openstack.org/cgit/openstack/nova/tree/nova/scheduler/client/report.py#n136 will do the right thing and filter out any allocations from the flavor that are 0 | |
| 14:58:40 | mriedem | so requires_allocation_refresh can remain true and only report the custom node resource class allocation | |
| 14:58:56 | mriedem | and the driver will only report custom resource class inventory since CONF.workarounds.report_ironic_standard_resource_class_inventory=False | |
| 14:59:22 | belmoreira | yes | |
| 14:59:38 | mriedem | so i guess this whole thing was missed when the code was dropped | |
| 14:59:39 | mriedem | https://github.com/openstack/nova/blob/stable/rocky/nova/virt/ironic/driver.py#L554 | |
| 14:59:45 | mriedem | "This code can be removed in Queens, and will need to be updated to also alter extra_specs to zero-out the old-style standard resource classes of VCPU, MEMORY_MB, and DISK_GB." | |
| 14:59:52 | mriedem | heh, "remove this code, but also change it" | |
| 15:00:34 | belmoreira | I guess existing ironic deployments will hit this when they upgrade to rocky. The impact depends in their expectations and config | |
| 15:01:20 | mriedem | yeah so CONF.workarounds.report_ironic_standard_resource_class_inventory=False actually causes a problem | |
| 15:01:26 | mriedem | fixes one issue, exposes another | |
| 15:03:24 | mriedem | dansmith: jaypipes: jroll: edleafe: do you remember why we didn't just zero out the standard resource classes on embedded instance.flavor for ironic instances in the first place? | |
| 15:03:29 | mriedem | i.e. https://github.com/openstack/nova/blob/stable/rocky/nova/virt/ironic/driver.py#L554 | |
| 15:03:56 | edleafe | mriedem: IIRC, it was for display purposes | |
| 15:04:14 | edleafe | the flavor would show what the user would get | |
| 15:04:39 | mriedem | but can't we still have that *and* override by setting resources:VCPU=0? | |
| 15:05:45 | mriedem | https://docs.openstack.org/nova/latest/user/flavors.html | |
| 15:05:49 | mriedem | "Custom resource classes and standard resource classes to override" | |
| 15:05:56 | mriedem | yeah | |
| 15:05:57 | mriedem | resources:CUSTOM_BAREMETAL_SMALL=1 resources:VCPU=0 | |
| 15:06:03 | edleafe | That was how it was supposed to be done. Keep the values in the flavor, and zero out in extra_specs | |