Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-21
20:22:45 sean-k-mooney in anycase we proably should not be doing random json loads in the code like that
20:22:58 mnaser its happening a tons tho :X
20:23:34 sean-k-mooney sure but that does not mean its a good thing :)
20:24:01 mnaser ++ yeah i agree
20:24:08 mnaser https://github.com/openstack/nova/blob/6efa3861a5a829ba5883ff191e2552b063028bb0/nova/volume/cinder.py#L575-L605
20:25:37 edmondsw I don't think openstack-tox-lower-constraints is working properly... I just had to put up a fix for nova-powervm that would appear to be needed for nova as well based on the lower-constraints.txt, but that check is passing...
20:25:38 sean-k-mooney if
20:25:57 mnaser https://github.com/openstack/nova/blob/67ac57b3258d4d7476fa544b3dfef7296e220ad2/nova/virt/block_device.py#L677-L687
20:26:04 edmondsw and it looks like it's passing because the job is actually using a newer version of psycopg2 (2.7.7) than what the lc txt file indicates (2.6.2)
20:26:31 sean-k-mooney edmondsw: cdent fixed some things in lower contratins a week or two ago
20:26:44 edmondsw here's the nova-powervm fix for reference: https://review.openstack.org/#/c/645330
20:26:47 sean-k-mooney edmondsw: it may be using the system installed version
20:26:59 edmondsw whatever it's doing, it's not right
20:27:13 mnaser sean-k-mooney: from oslo_serialization import jsonutils; jsonutils.dumps(None) => 'null'
20:27:23 efried edmondsw: This may be a better question for #openstack-requirements, but I think whatever you have in l-c is not necessarily what the lower-constraints job will install, as it has to take the highest-low from all the projects it's installing.
20:27:45 sean-k-mooney mnaser: ya mapping None to 'null' is what i would expect
20:28:04 mnaser so what we should do is make sure that we do something like
20:28:11 mnaser self.get('connection_info', {}) instead
20:28:15 efried edmondsw: iow if nova-powervm has l-c for foo==1.5 and bar==X and bar at X has l-c for foo==1.7, you're gonna get 1.7
20:28:16 edmondsw efried this is just an FYI... do with it what you will
20:30:32 sean-k-mooney edmondsw: if the issue is the one i think it is related to postgree it could also be donw to the desto the lower constratits job is using
20:30:57 edmondsw and whatever you do, you may want to bump to at least 2.7 for psycopg2 in nova's lc file, since 2.6.2 isn't going to work if it ever was to get used
20:31:30 sean-k-mooney edmondsw: well it will if you are not useing postgres
20:31:45 edmondsw if you're not using postgres then you don't need psycopg2 :)
20:31:57 sean-k-mooney well that was basically my point
20:32:11 edmondsw but anyway... this is just a public service announcement
20:32:35 sean-k-mooney im not sure we can raise our lower constrants at this point can we?
20:32:51 efried for stein? It would take an emergency.
20:33:18 sean-k-mooney edmondsw: we have see this issue downstream by the way too but we just used 2.7 instead
20:33:29 mnaser https://bugs.launchpad.net/nova/+bug/1821244 im gonna work on this now. it should be a simple fix.
20:33:31 openstack Launchpad bug 1821244 in OpenStack Compute (nova) "Failed volume creation can result in invalid `connection_info` field" [Undecided,New]
20:33:49 edmondsw sean-k-mooney negating the purpose of lower-constraints
20:34:12 sean-k-mooney edmondsw: well this show up in older release too
20:34:59 sean-k-mooney the root casue was the presence of postgres 10.X that broke the compatiblity
20:35:17 sean-k-mooney if you use an older postgres it also works
20:35:47 sean-k-mooney but ya we should bump it but tommorow
20:38:46 sean-k-mooney efried: the lower constratins job should use nova lower constratin as the upper constratin
20:39:15 efried sean-k-mooney: even if it installs some other dep that has a higher lower constraint? That doesn't make sense.
20:39:21 sean-k-mooney pip transitive depencies via requirement.txt will not look at there upper or lower constratints
20:40:34 sean-k-mooney efried: it will fail to install if our constratin file say 2.0.0 and they request >= 3.0.0
20:41:59 sean-k-mooney upper/lower constratins.txt is an openstack convention so pip/setuptools does not know about them
20:42:36 sean-k-mooney pip just looks ate the requiremets.txt files
20:42:41 efried sure, but I would expect the l-c job to know about them.
20:42:56 efried ...about the lower constraints for the projects that have lower constraints.
20:43:10 sean-k-mooney the lower constratin job does pip install -r requriements.txt -c lower-constratints.txt
20:43:40 sean-k-mooney only one constratint file. nova is passed when installing the deps
20:43:51 efried mm, okay, then yeah, I guess it should only be observing my own l-c huh
20:46:14 sean-k-mooney for what its worth the job does look broken
20:46:21 sean-k-mooney http://logs.openstack.org/46/621646/20/check/openstack-tox-lower-constraints/8b49dc9/tox/lower-constraints-1.log
20:46:59 sean-k-mooney we are passing bot an upper-constratins.txt and lower-constraints.txt to pip
20:48:30 sean-k-mooney ok i see what going on
20:48:39 sean-k-mooney ill see if i can fix that
20:49:03 sean-k-mooney our install command include upper-constratints
20:51:24 efried sean-k-mooney: you mean L18 of tox.ini?
20:51:43 sean-k-mooney yes
20:52:09 sean-k-mooney im overriding hte install_command local to not do that in the lower-constraints env and ill see what hapens
20:52:20 sean-k-mooney i can see it endup using 2.7.7 in the gate
20:52:31 sean-k-mooney it should have been using 2.6.8 or whatever
20:53:13 sean-k-mooney well that failed instantly
20:53:16 efried so there's nothing tox-magical about the "lower-constraints" name like there is for pyXX that would be smart enough to use a fresh install_command?
20:53:41 efried failed instantly because a l-c is too low?
20:53:42 sean-k-mooney no lower-constraints is an openstack thing
20:53:49 efried okay.
20:54:06 sean-k-mooney it faild on the postgressql issue
20:54:10 efried so we've been borked for a while and real l-cs have been creeping past us.
20:54:29 efried edmondsw: Thanks for the find there!
20:54:33 sean-k-mooney ya for basicly ever i think looking at the git blame
20:54:50 sean-k-mooney the install command predate adding lower constratints i think
20:54:50 efried well, we've done quite a bit to tox.ini in the last release or two.
20:55:01 efried ...but not sure if those have been touched.
20:55:19 sean-k-mooney ill bump the lower constatin to 2.7 and push a patch shortly
20:55:30 efried sean-k-mooney: Well, that might not be the only transgressor
20:55:35 efried but I guess you'll find out :)
20:55:51 sean-k-mooney i was trying not to say that out loud but yes
20:57:20 sean-k-mooney i should proably file a bug for this. ya the next issue is the zvm sdk
20:57:57 sean-k-mooney i think i fixed this error actully it complingin about the python version support
20:58:45 sean-k-mooney i shoudl proably file a bug for this. shoudl this be a RC2 thing
21:00:57 openstackgerrit Merged openstack/nova-specs master: Re-propose count quota usage from placement https://review.openstack.org/645302
21:04:36 mnaser im struggling to trace the path of failure but i have a fix for this :\
21:07:02 openstackgerrit Mohammed Naser proposed openstack/nova master: wip: bdm: store empty object as connection_info by default https://review.openstack.org/645352
21:07:27 mnaser if someone has a few cycles to spare, i'm going to keep digging but i don't know the code that well to know why it's being saved with 'null'
21:08:23 sean-k-mooney mnaser: if the value is ever set to None it will be serialised to 'null'
21:08:55 sean-k-mooney i can try and take a look at it in a bit but i also dont know the bdm code that well
21:08:56 mnaser sean-k-mooney: yeah.. but i am trying to figure out why it would either be a) set to None or b) .get('connection_info') fails and returns None
21:09:30 mnaser oh interesting
21:09:34 sean-k-mooney if its trying to delete it before the block device mappings are constructed then that might be the issue
21:10:03 mnaser https://github.com/openstack/nova/blob/67ac57b3258d4d7476fa544b3dfef7296e220ad2/nova/virt/block_device.py#L284-L288
21:10:14 mnaser by default, the connection_info field defaults to 'NULL'
21:10:23 mnaser so that means self['connection_info'] = None ..
21:10:40 mriedem isn't the instance.host set before that though?
21:10:46 mriedem the instance_claim sets the instance.host
21:10:47 sean-k-mooney ya which just porpagates that default
21:10:51 mriedem so you shouldn't get a local delete flow in the API
21:11:09 mnaser mriedem: this is a case where instance.host = None so i think what happens is that the build aborts a few times on the compute host
21:11:17 mnaser and then when it fails to build it, instance.host goes to None
21:11:31 mnaser because cinder blows up when it tries to create a volume and you're out of quota
21:11:57 mnaser this isnt a case of instance.host = None because we're in cell0, it's because we failed to build the instance N times and gave up
21:12:12 mriedem anything failing in _prep_block_device automatically aborts the build
21:12:14 mriedem there is no reschedule
21:12:38 mriedem https://github.com/openstack/nova/blob/67ac57b3258d4d7476fa544b3dfef7296e220ad2/nova/compute/manager.py#L2317

Earlier   Later