| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-21 | |||
| 20:12:01 | sean-k-mooney | ok so jsonutils.loads(bdm.connection_info) did not produce a dictionary | |
| 20:12:07 | mnaser | yeah, i will log it's value | |
| 20:12:21 | sean-k-mooney | it could be the empty sting | |
| 20:12:34 | sean-k-mooney | *string | |
| 20:13:48 | mnaser | LOG.debug(bdm.connection_info) => null ? | |
| 20:14:11 | sean-k-mooney | null | |
| 20:14:22 | sean-k-mooney | as in that is what it litally printed | |
| 20:14:23 | mnaser | and then json parsing null gives... None | |
| 20:14:36 | sean-k-mooney | ya that would make sense | |
| 20:14:36 | mnaser | so bdm.connection_info is the literal value 'null' | |
| 20:14:40 | mnaser | weird | |
| 20:14:56 | sean-k-mooney | well i dont know why bdm.connection_info is null | |
| 20:15:33 | mnaser | well i see it in the db equal to null too hmm | |
| 20:16:27 | sean-k-mooney | well the litral null is a valid json type | |
| 20:16:30 | sean-k-mooney | https://www.json.org/ | |
| 20:17:16 | mnaser | hmm | |
| 20:17:19 | sean-k-mooney | so id the db colume is null becuae no conection info exists for this incance becasue it has never been schduled then it makes sense that it woudl be null when we parse it there | |
| 20:17:35 | mnaser | Quota exceeded for 86bbbcfa8ad043109d2d7af530225c72, tried to create 80G volume (6080G of 6144G already consumed).: OverQuota: Quota exceeded for resources: ['gigabytes'] | |
| 20:18:05 | mnaser | so im wondering if when it tries to create bfv vol, it fails, instance tries to delete, poof | |
| 20:18:28 | sean-k-mooney | ya that sound plasible | |
| 20:18:36 | sean-k-mooney | we could jsut add a none check there | |
| 20:19:29 | sean-k-mooney | so store teh value of jsonutils.loads(bdm.connection_info) then do a "if connection_info is not None: ... | |
| 20:19:53 | mnaser | does nova get connection_info right away or does it poll till it gets it | |
| 20:19:55 | mnaser | im assuming it polls | |
| 20:20:26 | mnaser | looks like we do json loading often so it would be good for us to store {} instead of null i guess | |
| 20:20:30 | sean-k-mooney | i wont have connection info untill after it has schduled to a host | |
| 20:21:26 | sean-k-mooney | we proably dont store {} to avoid updating all old instances when we first load them | |
| 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 :) | |