| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-20 | |||
| 13:42:37 | mnaser | let me double check that doesnt exist | |
| 13:43:43 | mnaser | nope | |
| 13:43:46 | mnaser | that's probably it | |
| 13:43:58 | mnaser | ill mark my bug as duplicate of that one | |
| 13:44:04 | mriedem | yeah so the RP is still in placement, and it gave it back to the scheduler, and the scheduler somehow looked it up | |
| 13:44:23 | mriedem | i'm curious as to why the scheduler is looking up deleted compute nodes though | |
| 13:44:40 | mnaser | yeah, that doesn't seem right i guess | |
| 13:44:56 | dansmith | um, guys, obvi | |
| 13:45:00 | dansmith | it's scheduling a deleted instances | |
| 13:45:05 | mnaser | objects.ComputeNodeList.get_all(cctxt) | |
| 13:45:44 | mriedem | i see the bug | |
| 13:45:51 | mriedem | 1756179 | |
| 13:45:53 | mriedem | oops | |
| 13:45:59 | mriedem | return db_computes | |
| 13:45:59 | mriedem | models.ComputeNode.uuid.in_(compute_uuids)).all() | |
| 13:45:59 | mriedem | db_computes = context.session.query(models.ComputeNode).filter( | |
| 13:45:59 | mriedem | def _db_compute_node_get_all_by_uuids(context, compute_uuids): | |
| 13:46:08 | mriedem | isn't using model_query so it's not filtering out soft deleted records | |
| 13:46:14 | tssurya | yikes | |
| 13:47:02 | mnaser | do i want to mark my bug as duplicate and create another one for that ^ or use the same one to cover that | |
| 13:47:32 | dansmith | hmm, I wonder why we did that | |
| 13:47:39 | dansmith | instead of writing it in sqla/api.py | |
| 13:47:57 | mriedem | because the new jazz is to write db stuff in the objects | |
| 13:48:08 | mriedem | mnaser: we'll use yours for the query thing | |
| 13:48:12 | mriedem | i'll un-invalidate it | |
| 13:48:31 | dansmith | mriedem: only for api objects | |
| 13:48:54 | mriedem | oh, then idk | |
| 13:48:55 | dansmith | or at least, I've been still putting main stuff in there | |
| 13:49:19 | dansmith | IIRC we agreed to not put api stuff int here, but keep doing it for the main stuff, but | |
| 13:49:48 | mriedem | you'd have to ask papa bear https://review.openstack.org/#/c/418134/ | |
| 13:50:00 | openstack | Launchpad bug 1793533 in OpenStack Compute (nova) "Scheduler doesn't filter out deleted compute node records based on placement RP UUIDs" [Medium,Triaged] | |
| 13:50:00 | mriedem | dansmith: https://bugs.launchpad.net/nova/+bug/1793533 | |
| 13:50:05 | bauzas | I'm back | |
| 13:50:12 | bauzas | PapaOurs is here | |
| 13:50:32 | mriedem | dansmith: so you got this one? | |
| 13:50:33 | bauzas | any regression I made, mmm ? | |
| 13:50:55 | tssurya | Q: placement should not even return deleted RPs right ? the list of compute_uuids coming from placement should be legit ? or its okay because the post-scheduler compute filter is supposed to check for these conditions like it doesn for enabled/disabled ? | |
| 13:51:04 | dansmith | mriedem: but the real problem is not in placement but that we keep finding the compute node in our search yeah? | |
| 13:51:11 | bauzas | tssurya: we have a filter for this | |
| 13:51:13 | mriedem | tssurya: mnaser didn't have https://review.openstack.org/#/c/563698/ | |
| 13:51:20 | mriedem | tssurya: so yes this is a side effect that shouldn't otherwise happen | |
| 13:51:33 | bauzas | we have the ComputeFilter | |
| 13:51:44 | tssurya | (I understand that beore this fix, the RPs were not deleted so it they are returned, but in current situation it should be okay right ?) | |
| 13:51:49 | tssurya | bauzas: yea that filter | |
| 13:51:49 | dansmith | mriedem: I got this one what? you want me to fix it up? | |
| 13:52:02 | mriedem | dansmith: i assumed you were asking for the bug in order to push a change | |
| 13:52:21 | mnaser | if we get a fix it'd be nice to get a dot release for nova, last queens release was end of may | |
| 13:52:25 | dansmith | no, I was just curious what the symptom was.. but I can fix it up, sure | |
| 13:52:29 | jroll | TIL papa bear is not a name for comstud | |
| 13:52:35 | dansmith | mnaser: ++ | |
| 13:52:40 | bauzas | so I think we agreed maybe in Portland that the Placement API wouldn't check whether the node is active or not | |
| 13:52:49 | mriedem | mnaser: we know - i've been watching rocky/queens/pike/ocata approved changes sit in the gate for 3 days | |
| 13:52:59 | mnaser | :( | |
| 13:53:03 | cdent | bauzas: correct | |
| 13:53:08 | mriedem | http://lists.openstack.org/pipermail/openstack-dev/2018-September/134867.html | |
| 13:53:14 | mnaser | rough times out here | |
| 13:53:18 | tssurya | ack | |
| 13:53:27 | bauzas | jroll: https://scontent-cdt1-1.xx.fbcdn.net/v/t1.15752-9/41984667_323237498424760_7346301711654322176_n.jpg?_nc_cat=0&oh=a19da45f6d7b7b799d8f831e7f3c4a05&oe=5C1954CF | |
| 13:53:34 | mriedem | bauzas: tssurya: cdent: dansmith: mnaser: in true form, we're all talking past each other | |
| 13:53:40 | mriedem | does someone want me to summarize the issue here? | |
| 13:53:57 | jroll | bauzas: add a guitar and you can come work on ironic :) | |
| 13:54:01 | dansmith | seems clear, | |
| 13:54:08 | cdent | mriedem: I think i was following along okay, was just trying to pause bauzas | |
| 13:54:09 | gibi | nova meeting stars in 6 minutes on #openstack-meeting | |
| 13:54:09 | tssurya | I have understood this to be something needed as a regression fix for the code before this patch | |
| 13:54:13 | tssurya | so its clear to me too | |
| 13:54:18 | bauzas | wait wait | |
| 13:54:22 | mriedem | so bauzas is the odd man out | |
| 13:54:25 | dansmith | we are not filtering the list of placement results against compute nodes that exist | |
| 13:54:38 | mnaser | yes, we're getting *all* including deleted ones | |
| 13:54:40 | bauzas | can someone explains me what's the problem ? | |
| 13:54:45 | tssurya | yep yep | |
| 13:54:49 | mriedem | dansmith: yes, but we were only given those uuids for deleted records because mnaser doesn't have https://review.openstack.org/#/c/563698/ | |
| 13:54:49 | dansmith | bauzas: wait for my bug fix | |
| 13:54:51 | dansmith | it'll be clear then | |
| 13:54:52 | bauzas | see my BBIAB 30 mins before :) | |
| 13:54:56 | bauzas | dansmith: ok cool | |
| 13:54:57 | dansmith | mriedem: right | |
| 13:55:01 | mriedem | which would have deleted the RP in placement when the compute service was deleted | |
| 13:55:04 | mriedem | it's just a simple side effect | |
| 13:55:33 | mnaser | bauzas: if a compute service is deleted (but still somehow in placement and has host_mappings, something which a bug has fixed not long ago), then nova ComputeNodeList.get_all_by_uuids returns that, it shouldn't | |
| 13:55:37 | dansmith | mriedem: yeah, but we could need to delete a compute node when placement is down or something, so definitely should fix this issue | |
| 13:55:39 | bauzas | jroll: meh, I do have a guitar as well at home... | |
| 13:56:02 | jroll | ++ | |
| 13:56:03 | mnaser | bauzas: and the reason was because get_all_by_uuids() doesn't use model_query so it doesn't filter against soft deleted stuff | |
| 13:56:17 | mnaser | that's what i got out of it and it makes sense enough for me :p | |
| 13:56:24 | mriedem | dansmith: sure i agree | |
| 13:56:42 | bauzas | mnaser: we don't have services in placement, just compute nodes, but okay I understand you :) | |
| 13:56:53 | mnaser | haha | |
| 13:56:53 | mnaser | i'm trying here | |
| 13:57:03 | bauzas | 3 mins dude | |
| 13:57:06 | bauzas | too early | |
| 13:57:08 | mnaser | one last thing: is someone pushing up a patch for this or is it on me? | |
| 13:57:13 | mnaser | (i dont mind) | |
| 13:57:13 | mriedem | dan is | |
| 13:57:16 | mnaser | ok cool | |
| 13:57:18 | bauzas | i think dansmith said | |
| 13:57:20 | mnaser | thanks dansmith | |
| 13:57:21 | bauzas | yeah that | |
| 13:57:29 | mriedem | *head explodes* | |