| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-16 | |||
| 17:51:29 | mdbooth | But I thought for safety we should just leave it as is | |
| 17:51:33 | johnthetubaguy | the real error happen in _driver_detach_volume | |
| 17:51:38 | mdbooth | And change it deliberately at another time if that's what we want | |
| 17:51:43 | johnthetubaguy | so I think the tweak seems good | |
| 17:51:54 | johnthetubaguy | its tempting to do it after the BDM clean up I supose | |
| 17:52:18 | lyarwood | I'm working on this now so it's not an issue to kick it out | |
| 17:52:19 | johnthetubaguy | oh, wait, we can't we need to send that info as part of the instance | |
| 17:52:56 | johnthetubaguy | so here is a reason to go lyarwood's way | |
| 17:53:03 | johnthetubaguy | in the new API there is only one cinder API call | |
| 17:55:58 | johnthetubaguy | lyarwood: I was thinking the fake BDMs should probably have attachment_id = None, so it matches the current BDMs generated by the code: https://review.openstack.org/#/c/437665/11/nova/tests/unit/objects/test_block_device.py@36 | |
| 17:56:45 | lyarwood | johnthetubaguy: true, I was thinking ahead to testing detach but you're right that doesn't make sense yet. | |
| 17:57:37 | johnthetubaguy | lyarwood: I would keep the uuids for the object compat one though, that seems legit | |
| 17:57:51 | lyarwood | johnthetubaguy: kk | |
| 17:58:56 | lyarwood | johnthetubaguy: and re get_by_attachment_id , I just assumed we would want that around tbh, if you can't think of a case where we would want that over get_by_instance* then I'll drop it | |
| 18:00:20 | johnthetubaguy | lyarwood: I think we will always fetch using volume-uuid and instance-uuid, because of API flow. Its easy to add when we need it though, best to leave it out for now I think. | |
| 18:00:32 | johnthetubaguy | lyarwood: I will probably regret that tomorrow, but it seems the best way to go | |
| 18:00:45 | lyarwood | johnthetubaguy: kk, should I leave the index on attachment_id? | |
| 18:00:59 | mriedem | probably don't need the index if we don't query by attachment_id right? | |
| 18:01:01 | johnthetubaguy | lyarwood: good question, I guess we can leave that off too | |
| 18:01:07 | johnthetubaguy | yeah, what mriedem said | |
| 18:01:14 | lyarwood | right, just checking before I pull it | |
| 18:01:17 | lyarwood | thanks | |
| 18:01:30 | johnthetubaguy | index will slow down write performance, so worth dropping tht | |
| 18:08:04 | mriedem | gibi: have you ever seen this? http://logs.openstack.org/08/445308/3/check/gate-tempest-dsvm-py35-ubuntu-xenial/7bf0d72/logs/screen-n-api.txt.gz#_2017-03-16_05_31_09_399 | |
| 18:08:13 | mriedem | ValueError: Circular reference detected | |
| 18:08:17 | mriedem | during send_notification | |
| 18:08:40 | openstackgerrit | Ken'ichi Ohmichi proposed openstack/nova master: Clarify os-stop API description https://review.openstack.org/446264 | |
| 18:13:00 | mriedem | oomichi: left a question in ^ | |
| 18:13:01 | mriedem | for os-start | |
| 18:15:11 | oomichi | mriedem: oh, nice catch. I am a fun for removing it also | |
| 18:17:40 | mdbooth | mriedem: I've also seen Circular reference detected recently | |
| 18:17:52 | mdbooth | After an error | |
| 18:17:59 | openstackgerrit | Ken'ichi Ohmichi proposed openstack/nova master: Clarify os-stop API description https://review.openstack.org/446264 | |
| 18:18:14 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Remove check_detach https://review.openstack.org/446671 | |
| 18:18:33 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/446672 | |
| 18:18:34 | oomichi | mriedem: ^^^ thanks, done | |
| 18:18:42 | mdbooth | mriedem: http://logs.openstack.org/42/445142/3/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/07e282a/logs/screen-n-api.txt.gz?level=ERROR#_2017-03-15_10_18_17_459 | |
| 18:18:57 | mdbooth | Is that similar? | |
| 18:19:09 | mriedem | mdbooth: yes | |
| 18:19:19 | openstack | Launchpad bug 1673375 in oslo.messaging ""ValueError: Circular reference detected" in send_notification" [Undecided,New] | |
| 18:19:19 | mriedem | mdbooth: https://bugs.launchpad.net/nova/+bug/1673375/comments/3 | |
| 18:19:32 | mriedem | the notification is coming from the wrap_exception decorator here https://github.com/openstack/nova/blob/2380659e358770a3f36253b93a112b9779a23958/nova/compute/api.py#L4602 | |
| 18:19:38 | mriedem | but i don't know what the circular reference is | |
| 18:20:55 | mdbooth | I expect if we knew that and decoded the big chunk of text above we'd discover it | |
| 18:21:53 | mriedem | something is messed up in that change of yours | |
| 18:22:01 | mriedem | TypeError: get_by_instance_mapping_list() got an unexpected keyword argument 'expected_addrs' | |
| 18:22:07 | openstackgerrit | OpenStack Proposal Bot proposed openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/446674 | |
| 18:22:18 | mdbooth | mriedem: Yeah, I've fixed it since | |
| 18:22:22 | mriedem | ah ok | |
| 18:22:31 | mdbooth | It was just a typo | |
| 18:22:49 | mriedem | johnthetubaguy: https://review.openstack.org/#/c/446672/ | |
| 18:22:50 | mriedem | yup | |
| 18:22:52 | mriedem | addrs/attrs | |
| 18:23:01 | mriedem | johnthetubaguy: ^ is the cinderclient min version bump | |
| 18:23:20 | mdbooth | mriedem: Now *you* saw that straight away. I stared at it so long I became blind to it. | |
| 18:23:45 | mriedem | pair programming | |
| 18:23:54 | mriedem | the ibm execs were right! | |
| 18:24:52 | mriedem | we were told that spotify did everything correctly, and so we needed to do everything they did | |
| 18:24:53 | mriedem | like, | |
| 18:25:05 | mriedem | pair programming, squads, kanban, agile, | |
| 18:25:11 | mriedem | work around a single table in a room | |
| 18:25:14 | mriedem | eat together | |
| 18:25:17 | mriedem | sleep together | |
| 18:25:18 | mriedem | etc etc | |
| 18:25:22 | mriedem | out pops innovation | |
| 18:25:39 | mriedem | yada yada yada, 18 months later, i left ibm | |
| 18:25:41 | mriedem | ") | |
| 18:25:43 | mriedem | :) | |
| 18:26:08 | mdbooth | mriedem: So, looking at that circular reference thing, there are a few python objects on those dicts | |
| 18:26:33 | ildikov | mriedem: sounds like a long time to sleep together with the whole team :) | |
| 18:26:42 | mriedem | ildikov: you wouldn't believe the VD | |
| 18:26:44 | mdbooth | If the json dumper is trying to serialise them, and there's a circular reference in there | |
| 18:26:52 | mdbooth | Then... that would be a circular reference | |
| 18:26:57 | mdbooth | There are plenty to choose from | |
| 18:27:17 | ildikov | mriedem: VD? | |
| 18:27:24 | mdbooth | There's probably stuff in there it's just not useful to serialise, tbh | |
| 18:27:37 | mriedem | venereal disease | |
| 18:27:46 | mriedem | ildikov: a joke | |
| 18:28:21 | ildikov | mriedem: I knew it was a joke, just had no idea what VD is, LOL | |
| 18:28:43 | ildikov | mriedem: I could've guessed though :) | |
| 18:30:37 | openstackgerrit | Sujitha proposed openstack/nova master: Add helper method to add additional data about policy rule. https://review.openstack.org/434842 | |
| 18:35:01 | openstack | bug 1671648 in OpenStack Compute (nova) ocata "Instances are not rescheduled after deploy fails" [High,In progress] https://launchpad.net/bugs/1671648 - Assigned to Matt Riedemann (mriedem) | |
| 18:35:01 | openstackgerrit | melanie witt proposed openstack/nova master: Fix functional regression/recreate test for bug 1671648 https://review.openstack.org/446685 | |
| 18:35:38 | melwitt | mriedem: while I was working on making it consider two hosts while scheduling, I discovered a race. so I have put up a fix ^ | |
| 18:35:58 | melwitt | the regression test, that is | |
| 18:36:51 | mriedem | melwitt: ok, cool | |
| 18:37:20 | mriedem | i had backported the functional regression test and the retry fix to ocata already, they aren't merged, but we could hold them up for the race fix too | |
| 18:37:21 | mriedem | in the test | |
| 18:38:26 | melwitt | okay | |
| 18:40:26 | melwitt | mriedem: the "host" on the instance server attributes is the host from the start_service('compute', host=) but the "hypervisor_hostname" is the nodename, and the nodename for both hosts was the same "fake-mini" so that's how they got collapsed into only one host for scheduling | |
| 18:41:03 | dansmith | and nodes bites us in the ass once again | |
| 18:41:26 | melwitt | there's a fake.set_nodes() that lets you set the hostnames to return and you have to do it before starting each compute service fixture | |
| 18:41:38 | melwitt | yeah :( node ass-biters | |
| 18:41:58 | mriedem | ah ok | |
| 18:41:59 | mriedem | ouch | |
| 18:42:19 | mriedem | also, can you all please clean it up please, | |
| 18:42:27 | mriedem | i'd like to keep this channel classy | |
| 18:42:48 | melwitt | after I set the nodes, the race reared its head, sometimes I got a Claim, sometimes I got a NopClaim | |
| 18:43:57 | mriedem | alright, my kid just threw up in class so i'm going to the school, back in a bit | |
| 18:44:32 | mriedem | probably all those stories i told here about nodes and local delete and quotas | |