| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-21 | |||
| 19:41:12 | dansmith | eeeeeverybody wants into the prefilter game | |
| 19:41:40 | cdent | some people can follow trends but only dansmith can make them possible | |
| 19:41:49 | mriedem | psh i was talking about this way back in https://review.openstack.org/#/c/538498/ | |
| 19:42:41 | cdent | now that efried is ptl do we have to roll to resolve disputes? | |
| 19:42:43 | mriedem | gonna go for a walk with the mrs while (1) it's nice out and (2) the neighbors and their annoying kids aren't outside mingling | |
| 19:47:31 | openstackgerrit | Merged openstack/python-novaclient master: Remove deprecated options https://review.openstack.org/642312 | |
| 19:52:36 | efried | I've long thought the center of the square of tables would make a good "cage". | |
| 19:55:27 | mnaser | should _local_delete() be called with instance.host == None ? | |
| 19:56:01 | mnaser | "instance's host None is down, deleting from database" with "Ignoring volume cleanup failure due to 'NoneType' object has no attribute 'get'" afterwards, i'm assuming that volume clean up has to do with the fact that host is None | |
| 19:57:06 | mnaser | https://github.com/openstack/nova/blob/5ca858eaa72acd0513e27a4c9518980b769f5d6e/nova/compute/api.py#L1967 | |
| 19:57:19 | mnaser | i can only imagine that not possibly being evaluated properly because its hitting the local delete path | |
| 19:57:24 | sean-k-mooney | mnaser: proably not | |
| 19:57:50 | sean-k-mooney | instance.host==None would only happen if the inscate was in cell0 right | |
| 19:57:57 | mnaser | or not scheduled yet | |
| 19:58:32 | mnaser | if not instance.host and not may_have_ports_or_volumes <= the first part evaluates to True (as per the obvious warnings) | |
| 19:58:57 | mnaser | may_have_ports_or_volumes = compute_utils.may_have_ports_or_volumes(instance) is probably evaluating the true (say this is a bfv instance) | |
| 19:59:31 | sean-k-mooney | ya just read the code around it for context | |
| 20:00:14 | mnaser | so it may seem that _local_delete() assumes that a host exists | |
| 20:01:45 | sean-k-mooney | i might be blind but wher is _local_delete() called in that function | |
| 20:02:17 | mnaser | https://github.com/openstack/nova/blob/5ca858eaa72acd0513e27a4c9518980b769f5d6e/nova/compute/api.py#L2128-L2129 | |
| 20:02:37 | mnaser | the NoneType get happens inside here somewhere https://github.com/openstack/nova/blob/5ca858eaa72acd0513e27a4c9518980b769f5d6e/nova/compute/api.py#L2174-L2199 | |
| 20:02:39 | sean-k-mooney | ah that funciton is a lot longer then i tought it was | |
| 20:05:26 | sean-k-mooney | proably form compute_utils.get_stashed_volume_connector | |
| 20:06:22 | mnaser | i think so | |
| 20:06:32 | sean-k-mooney | yep https://github.com/openstack/nova/blob/a5e3054e1d6df248fc4c00b9abd7289dde160393/nova/compute/utils.py#L1262 | |
| 20:06:34 | mnaser | will LOG.error() print out the traceback or whats the best way to do it | |
| 20:06:59 | mnaser | sean-k-mooney: yeah but there is 'if connector:' before that | |
| 20:07:16 | mnaser | which if connector is None then it should evaluate to false right off the bat) | |
| 20:07:38 | sean-k-mooney | i think there is a log.exception() prints a nice traceback | |
| 20:11:12 | mnaser | connector = jsonutils.loads(bdm.connection_info).get('connector') | |
| 20:11:16 | mnaser | AttributeError: 'NoneType' object has no attribute 'get' | |
| 20:12:00 | mnaser | the interesting thing is there is an if statement `bdm.connection_info is not None` | |
| 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 | |