| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-10-31 | |||
| 15:57:15 | dansmith | doesn't really look like any sort of unreasonable load or kernel events around that time | |
| 16:11:35 | dansmith | mriedem: https://review.opendev.org/#/c/692402/ | |
| 16:12:14 | dansmith | mriedem: have any tricks to get a lot of grenade runs on that? | |
| 16:12:39 | dansmith | I know in the past we'd create like ten noop patches that depends-on something like that to get 10x the runs | |
| 16:12:47 | dansmith | seems irresponsible in today's climate :) | |
| 16:16:38 | mriedem | you could create a grenade patch that depends-on it | |
| 17:05:54 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP service: Do not perform version checks for deleted services https://review.opendev.org/692412 | |
| 17:06:05 | lyarwood | melwitt: ^ when you're back, I'm likely missing something obvious here. | |
| 17:11:48 | mriedem | lyarwood: didn't mel already fix that bug in the Instance object when lazy-loading services? | |
| 17:12:25 | mriedem | https://review.opendev.org/#/c/582408/ | |
| 17:14:19 | lyarwood | mriedem: that was the original bug when running through this upgrade scenario yeah but instead of ServiceNotFound we now see ServiceTooOld raised with that fix applied | |
| 17:25:06 | mdbooth | sean-k-mooney: https://review.opendev.org/#/c/687957/ | |
| 17:29:08 | sean-k-mooney | mdbooth: thanks for the review. ill go though it tommorow im just about to go have dinner | |
| 17:29:28 | mdbooth | sean-k-mooney: ack | |
| 17:30:20 | sean-k-mooney | by the way i belive the way im comparing the numa toplogy constirant should compare things like pages size too | |
| 17:30:58 | sean-k-mooney | we dont actully want to try and fit the request to a host and compare that we just want to extrat the contaritns and assert they dont change | |
| 17:31:59 | sean-k-mooney | i can add some more test cases however to test different type of constraitn changes | |
| 17:32:24 | sean-k-mooney | to ensure that change hugpeage also will be detected ectra | |
| 17:32:52 | sean-k-mooney | i assume that is what you ment by poking at different levels | |
| 17:33:21 | mdbooth | sean-k-mooney: Yeah, I wasn't 100% on that, although I *did* go look up 1) the implementation of obj_to_primitive(), 2) the implementation of dict.__eq__ in python ;) | |
| 17:33:44 | mdbooth | sean-k-mooney: I think it's ok, I was just thinking for completeness | |
| 17:34:08 | mdbooth | But about the exception, I think it's important to positively assert that your last test case is being rejected for the reason you expect | |
| 17:34:24 | sean-k-mooney | ya i can do that | |
| 17:34:24 | mdbooth | Rather than for some other, coincidental reason | |
| 17:34:30 | mdbooth | (It is, btw, I checked that too) | |
| 17:34:40 | sean-k-mooney | the self.assertRaisss returns the exception raised | |
| 17:34:46 | mdbooth | It does | |
| 17:34:52 | sean-k-mooney | so i can save it to a var and then do more asserts | |
| 17:36:35 | sean-k-mooney | im pretty sure we can do that anyway | |
| 17:37:16 | sean-k-mooney | mdbooth: https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertRaises | |
| 17:37:57 | sean-k-mooney | i know we can get teh exception via the context manager apparch but i think the non context manager form just returns the exception | |
| 17:38:06 | mdbooth | sean-k-mooney: You can do both | |
| 17:38:10 | mdbooth | You can do: | |
| 17:38:17 | mdbooth | ex = self.assertRaises(...) | |
| 17:38:22 | sean-k-mooney | yep | |
| 17:38:35 | mdbooth | Anyway, I don't care what colour the bike shed is ;) | |
| 17:38:38 | sean-k-mooney | so ill leave the exsting test tehn i can assert after on the body | |
| 17:39:09 | sean-k-mooney | well it was more that there is an easy way to be more specific for that last case | |
| 17:39:33 | sean-k-mooney | i can assert an excetion is raised and then assert what it was as you suggested | |
| 18:01:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Reset vm_state to original value if rebuild claim fails https://review.opendev.org/692185 | |
| 18:24:15 | melwitt | mdbooth, lyarwood: thanx for the fast reviews on the get_hash_str patch! | |
| 18:54:46 | efried | mriedem: flavors were previously associated with instances just by ID, right? | |
| 18:55:11 | efried | But that was problematic because when you changed a flavor, you could no longer tell what was used to create the instance... | |
| 18:55:23 | efried | so we started copying the flavor content into the instance | |
| 18:55:30 | efried | and now we... no longer even store the original flavor ID? | |
| 18:59:52 | mriedem | we saved off the flavor information for a long time...not sure when that started, but it used to be in system_metadata rather than instance_extra | |
| 19:00:08 | melwitt | I think we still do store it, as instance_type column in the instances table | |
| 19:00:17 | mriedem | yes, we do | |
| 19:00:37 | mriedem | the embedded instance flavor was migrated from system_metadata to instance_extra back in kilo - dan did that work | |
| 19:01:03 | mriedem | instances.instance_type_id is the flavor.id | |
| 19:01:09 | mriedem | it's set on create and resize | |
| 19:01:27 | efried | is it used for anything, or just there kind of as a historical artifact? | |
| 19:02:01 | mriedem | quick grok shows the simple tenant usage api uses it | |
| 19:02:04 | mriedem | but probably shouldn't | |
| 19:02:30 | efried | okay. | |
| 19:02:31 | mriedem | oh it's only used there if the instance.flavor doesn't load | |
| 19:02:34 | mriedem | that's super old compat code though | |
| 19:02:51 | mriedem | it's used for filtering servers in GET /servers | |
| 19:03:07 | mriedem | and it's in the Migration object | |
| 19:03:13 | mriedem | old/new_instance_type_id | |
| 19:03:34 | mriedem | i think it's mostly vestigial though | |
| 19:03:36 | efried | And all of this (copying the flavor rather than just referring to it) is so things like rebuild behave sanely when the flavor is changed out of band. | |
| 19:03:37 | efried | right? | |
| 19:04:03 | mriedem | the flavor doesn't change on rebuild...but you mean in case the flavor extra specs change or something? | |
| 19:04:11 | mriedem | or if the flavor is deleted after the server is created and then you rebuild the server? | |
| 19:04:14 | mriedem | if so, yes | |
| 19:04:27 | mriedem | same reason we squirrel away the image meta used to create and rebuild the server | |
| 19:04:29 | mriedem | in case the image goes away | |
| 19:04:38 | efried | right, okay, thanks for confirming. | |
| 19:05:00 | mriedem | are you asking b/c of that spec about changing how flavor extra specs work? | |
| 19:05:05 | mriedem | i don't even want to look at that spec | |
| 19:05:35 | dansmith | changing how they work how" | |
| 19:05:36 | dansmith | ? | |
| 19:05:56 | mriedem | you'd have to look... | |
| 19:06:00 | efried | Yes. IMO it's a good idea. | |
| 19:06:03 | mriedem | i couldn't get past the problem statement | |
| 19:06:04 | efried | https://review.opendev.org/#/c/663563/6/specs/backlog/approved/add-flavor-metadata-or-metadata-group.rst | |
| 19:06:29 | efried | It's basically composable flavors. | |
| 19:06:40 | dansmith | oh jesus | |
| 19:07:04 | efried | so you can have a "flavor group" that describes, say, your qos extra specs, and another that describes your numa topo, and when you build an instance, you can specify multiple groups. | |
| 19:07:08 | efried | So that you don't have skittles. | |
| 19:07:27 | efried | kind of like what we're doing for cyborg device profiles, but generically. | |
| 19:09:15 | mriedem | but we don't have dynamic flavors | |
| 19:09:22 | mriedem | and what you described is basically dynamic flavors | |
| 19:09:39 | dansmith | less useful and more complex than dynamic flavors | |
| 19:09:48 | mriedem | create me an instance with this cpu/ram/disk but gimme some qos and numa on the side, regardless of how the flavor is mapped to aggregates | |
| 19:10:41 | efried | "flavor mapped to aggregates'? | |
| 19:11:51 | mriedem | yeah, pinning flavors to a subset of hosts in the deployment | |
| 19:11:59 | mriedem | capacity planning | |
| 19:12:06 | mriedem | isolation based on hw and features | |
| 19:12:07 | mriedem | etc | |
| 19:12:09 | dansmith | composing multiple things that define super detailed requirements seems like an invitation for "I can't find any combination of flavors that boot me an instance" | |
| 19:12:16 | efried | you mean by having a member_of in the flavor? | |
| 19:12:55 | efried | this idea isn't suggesting removing the ability to put extra specs into the flavor afaict | |
| 19:13:21 | efried | anyway, I've got no skin in this, I'm just reviewing someone's spec. If you guys have major objections, put 'em in the review. | |
| 19:20:58 | mriedem | i've gotta run to my kids halloween parade thing at the school, be back later | |
| 19:46:55 | openstackgerrit | Kobi Samoray proposed openstack/nova master: Avoid fetching metadata when no subnets found https://review.opendev.org/679247 | |
| 20:10:42 | openstackgerrit | Merged openstack/nova stable/ocata: Add functional regression test for bug 1669054 https://review.opendev.org/649419 | |
| 20:10:42 | openstack | bug 1669054 in OpenStack Compute (nova) ocata "RequestSpec.ignore_hosts from resize is reused in subsequent evacuate" [Medium,In progress] https://launchpad.net/bugs/1669054 - Assigned to Matt Riedemann (mriedem) | |
| 20:10:44 | openstackgerrit | Merged openstack/nova stable/ocata: Do not persist RequestSpec.ignore_hosts https://review.opendev.org/649420 | |