Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-10
15:52:42 mriedem looks like it doesn't for separate requests
15:52:44 gibi mriedem: both the affinity and the anti-affinity case fails now
15:53:00 gibi mriedem: both uses the same helper to create the servers
15:53:50 gibi mriedem: but in case of affinity we might want to fill the compute to see NoValid host
15:54:04 mriedem that's what i was going to try with a functional test
15:54:19 mriedem to set available vcpu inventory to 1 or something
15:54:34 gibi mriedem: sure we have plenty of such functional tests already but they are not cell aware and passing
15:55:08 gibi mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py
15:55:31 openstackgerrit Matt Riedemann proposed openstack/nova master: Document differences and similaries between extra specs and hints https://review.openstack.org/581410
15:55:34 mriedem right, i plan on writing a multi-cell affinity regression functional test
15:56:18 gibi mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py#L292
15:57:27 gibi mriedem: ohh there is one case where functional is needed. multiple non-cell0 cells case
15:57:43 gibi mriedem: as in tempest we have only cell0 and cell1
15:58:02 mriedem yes, that's what i've been saying :)
15:58:14 mriedem when i say 'multi-cell' i mean multiple non-cell0 cells
15:58:15 gibi mriedem: now I understand ;)
15:58:28 mriedem i don't consider cell0 a 'real' cell since it doesn't contain compute hosts
16:00:59 gibi mriedem: OK, I will extend the tempest test tomorrow. Thanks for the explanation
16:05:41 openstackgerrit Eric Fried proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139
16:06:28 openstackgerrit karim proposed openstack/nova master: Handle rebuild of instances with image traits https://review.openstack.org/569498
16:13:50 mriedem dansmith: i found another candidate for your long_rpc_timeout https://bugs.launchpad.net/cinder/+bug/1739482
16:13:50 openstack Launchpad bug 1739482 in Cinder "test_snapshot_backup fails to build backup due to MessagingTimeout" [Medium,Confirmed]
16:14:13 dansmith mriedem: that's on the cinder side though yeah?
16:14:16 mriedem yup
16:14:21 mriedem but i'd do the exact same thing there
16:14:29 mriedem ala oslo.incubator style
16:14:59 dansmith aye
16:19:24 mriedem dansmith: so in https://review.openstack.org/#/c/563375/35/nova/objects/instance_group.py it seems weird that _rules is nullable but we default to {} if it's not set in the db
16:19:38 mriedem so i can create a group with rules=None but then what i'll get back is group.rules == {}
16:19:52 dansmith mriedem: so, the reason I left it that way is because he had the first patch passing None from the api
16:20:00 dansmith but now that I think about it with a fresh head,
16:20:06 dansmith that can and maybe should just pass {} there
16:20:41 mriedem why {} over None?
16:21:09 mriedem just so callers don't hit NoneType on access?
16:21:09 dansmith I tend to prefer things like dicts to be empty instead of None, just because it means you always have to check to see if the thing is none, then check to see if your thing is in the dict, and then check to see if the value is what you want
16:21:39 dansmith that's why I was asking for it to be {} on return, but we should be consistent either way
16:22:01 mriedem so if we just pass in {} we can drop nullable=true
16:22:19 dansmith yeah i think that was the only place
16:22:26 mriedem there is one in the request spec as well
16:22:37 mriedem https://review.openstack.org/#/c/563375/35/nova/objects/request_spec.py
16:22:39 dansmith I remember de-null-ing it but then flipping it back when the api test failed
16:22:40 dansmith okay
16:22:54 dansmith I'll pull it down and make that change and see what breaks
16:23:55 mriedem this is the only thing (later in the series) i know of that will
16:23:56 mriedem https://review.openstack.org/#/c/563401/25/doc/notification_samples/common_payloads/ServerGroupPolicyPayload.json@7
16:24:08 mriedem but that's an easy change
17:14:31 dansmith mriedem: hmm, I didn't get a failure on that
17:16:23 dansmith er, wait,
17:16:31 dansmith maybe that would manifest in the notification patch but not in the last one
17:16:45 dansmith oh, that's what you linked to
17:20:02 dansmith hrm
17:20:03 dansmith no fail
17:20:33 dansmith maybe I'll push this up and you can look/comment
17:20:49 openstackgerrit Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375
17:20:50 openstackgerrit Dan Smith proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401
17:20:51 openstackgerrit Dan Smith proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166
17:20:52 openstackgerrit Dan Smith proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465
17:20:53 openstackgerrit Dan Smith proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534
17:26:04 mriedem dansmith: did you run just unit tests?
17:26:09 mriedem the notification sample will fail functional
17:27:12 dansmith I ran functional, but maybe not on the actual notification patch, looking at my history
17:28:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Heal allocations with incomplete consumer information https://review.openstack.org/574488
17:28:41 openstackgerrit Matt Riedemann proposed openstack/nova master: Refactor _heal_instances_in_cell https://review.openstack.org/577896
17:28:42 openstackgerrit Matt Riedemann proposed openstack/nova master: Use consumer generation in _heal_allocations_for_instance https://review.openstack.org/577905
17:29:30 dansmith mriedem: yeah, I did and just re-ran with "notification" regex and all passed
17:29:40 dansmith and it's running notification sample tests...
17:30:00 dansmith nova.tests.functional.notification_sample_tests.test_server_group.TestServerGroupNotificationSample.test_server_group_add_member
17:30:33 dansmith (as an example)
17:32:38 mriedem hmm
17:33:09 mriedem oh i see why
17:33:15 mriedem https://review.openstack.org/#/c/563375/35..36/nova/objects/instance_group.py@142
17:33:27 mriedem that should be removed or changed to {}
17:33:40 dansmith oh
17:33:55 dansmith sho nuf
17:34:16 mriedem and then we can make the rules field on the payload object no longer nullable as well https://review.openstack.org/#/c/563401/26/nova/notifications/objects/server_group.py@68
17:35:38 dansmith lemme write a test for that behavior
17:36:11 mriedem yeah that group = objects.InstanceGroup(rules={}) && self.assertEqual({}, group.rules)
17:42:42 openstackgerrit Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375
17:42:43 openstackgerrit Dan Smith proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401
17:42:44 openstackgerrit Dan Smith proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166
17:42:45 openstackgerrit Dan Smith proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465
17:42:46 openstackgerrit Dan Smith proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534
17:42:51 dansmith okay here we go
17:44:07 mriedem pep8 brotha
17:45:05 dansmith come on
17:45:33 mriedem want me to tag in?
17:45:56 dansmith oh that's not my fault.. no I can just fix and push easy
17:46:02 mriedem https://review.openstack.org/#/c/563401/27/nova/notifications/objects/server_group.py@68
17:46:06 mriedem also ^
17:48:18 dansmith do I need to re-run tests after snipping that out?
17:48:23 dansmith the nullable thing
17:48:51 dansmith ah maybe hash will change?
17:50:20 melwitt gibi, mriedem: my bug fix got stalled on the functional test. I wasn't able to recreate the bug in a functional test so far
17:53:01 openstackgerrit Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375
17:53:02 openstackgerrit Dan Smith proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401
17:53:03 openstackgerrit Dan Smith proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166
17:53:04 openstackgerrit Dan Smith proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465
17:53:05 openstackgerrit Dan Smith proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534
18:00:43 mriedem1 i assume bauzas is in his futbol chamber
18:10:11 mriedem dansmith: ok i'm +2 up that stack to the point of the rest api change, reviewing that during this game

Earlier   Later