| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 21:53:30 | mriedem | we get to have a microversion debate | |
| 21:53:40 | mriedem | alex_xu: ^ | |
| 21:54:16 | mriedem | although, it's an admin api... | |
| 21:54:40 | edleafe | mriedem: 400 usually means that you can make the call succeed by fixing the parameters, etc | |
| 21:54:44 | cdent | mriedem: yes 409 | |
| 21:54:53 | cdent | because of what edleafe just said | |
| 21:55:06 | cdent | the server has to change to make things work, not the request | |
| 21:55:17 | cdent | but if the server does change, then things might work when you try later | |
| 21:55:47 | mriedem | yeah 409 is correct, but need to figure out if i can get by the existing expected errors decorator | |
| 21:55:57 | mriedem | but i think with the legal caveat in https://docs.openstack.org/nova/latest/contributor/microversions.html#when-a-microversion-is-not-needed 3rd bullet | |
| 21:55:58 | mriedem | we maybe can | |
| 22:08:47 | mriedem | oh my | |
| 22:08:53 | mriedem | this is the issue that keeps on giving | |
| 22:10:22 | mriedem | https://bugs.launchpad.net/nova/+bug/1679750 | |
| 22:10:22 | openstack | Launchpad bug 1679750 in OpenStack Compute (nova) queens "Allocations are not cleaned up in placement for instance 'local delete' case" [Medium,Confirmed] | |
| 22:17:46 | mriedem | efried: seems delete_resource_provider should fail if delete_allocation_for_instance returns False... | |
| 22:18:03 | efried | ... | |
| 22:19:11 | efried | mriedem: 404 means there were no allocations for that consumer. Which is fine in the context of delete_resource_provider. Am I misunderstanding something? | |
| 22:19:18 | mriedem | getting a 409 | |
| 22:19:20 | mriedem | and now i know why | |
| 22:19:37 | efried | 409 is gonna mean generation conflict. | |
| 22:19:44 | mriedem | no it's not that | |
| 22:19:45 | efried | And sorry, I missed that we return False for any 4xx | |
| 22:20:21 | efried | mriedem: I think 404 should return True (or somehow otherwise be ignorable) and any other 4xx should be a failure in delete_resource_provider - agreed. | |
| 22:20:22 | mriedem | so i'm not actually calling this https://github.com/openstack/nova/blob/fe976dcc559d059589a9ccf953a28e855abf50fb/nova/scheduler/client/report.py#L1920 | |
| 22:20:49 | mriedem | because the list of instances is empty, because the compute service is down so when i delete the instance, the api removes it 'locally' but doesn't cleanup allocations | |
| 22:20:55 | mriedem | because of bug 1679750 | |
| 22:20:55 | openstack | bug 1679750 in OpenStack Compute (nova) queens "Allocations are not cleaned up in placement for instance 'local delete' case" [Medium,Confirmed] https://launchpad.net/bugs/1679750 | |
| 22:21:07 | mriedem | then https://github.com/openstack/nova/blob/fe976dcc559d059589a9ccf953a28e855abf50fb/nova/scheduler/client/report.py#L1922 fails with ResourceProviderInUse and we pass | |
| 22:21:41 | efried | noyce | |
| 22:22:11 | efried | mriedem: Me, I would rewrite a cascading RP deletion routine that doesn't rely on nova-isms. | |
| 22:22:30 | efried | You pass it a provider name/UUID and it goes after all related records, period. | |
| 22:22:42 | efried | I thought we had that somewhere already. | |
| 22:22:55 | mriedem | i just need to fix that bug | |
| 22:22:58 | mriedem | which has been around forever | |
| 22:23:30 | mriedem | i'm actually happy my test fails here, because the cascade silent fail means we're not doing something right | |
| 22:23:50 | efried | mriedem: Do you need help writing a real cascading delete in report client? | |
| 22:23:54 | mriedem | no | |
| 22:24:11 | mriedem | i need to fix that bug in the api local delete case to cleanup allocations in placement when the compute service is down | |
| 22:24:42 | mriedem | will be more clear when i post this code | |
| 22:24:44 | efried | Which you're going to need to do by getting placement to delete allocations based on the resource providers, not the instances. | |
| 22:24:53 | mriedem | no | |
| 22:24:56 | mriedem | we're deleting the instance | |
| 22:24:58 | mriedem | we know the consume | |
| 22:25:03 | mriedem | *consumer, so we get the allocations to delete | |
| 22:25:23 | mriedem | https://developer.openstack.org/api-ref/placement/#delete-allocations | |
| 22:25:46 | cfriesen | when doing a "git cherry-pick -x" do we generally preserve any conflicts or remove them from the commit message? | |
| 22:25:53 | cfriesen | for backporting | |
| 22:26:01 | efried | cfriesen: preserve (and explain if necessary) | |
| 22:26:11 | mriedem | cfriesen: i leave the conflicts in the commit message and make a note about what they are to help reviewers | |
| 22:26:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional test for deleting a compute service https://review.openstack.org/560626 | |
| 22:26:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Cleanup RP and HM records while deleting a compute service. https://review.openstack.org/554920 | |
| 22:26:25 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Block deleting compute services which are hosting instances https://review.openstack.org/560674 | |
| 22:26:31 | mriedem | efried: ^ should make it more clear | |
| 22:26:42 | efried | mriedem: ack, will look mañana | |
| 22:26:59 | mriedem | manana iguana? https://www.amazon.com/Manana-Iguana-Ann-Whitford-Paul/dp/0823419800 | |
| 22:27:08 | mriedem | 2 childrens books references in one day | |
| 22:27:36 | mriedem | those are for dansmith and jaypipes btw | |
| 22:35:23 | jaypipes | ewww | |
| 22:36:00 | mriedem | there goes a gate reset so that functional test fix is going to be awhile yet | |
| 23:01:42 | openstackgerrit | Eric Fried proposed openstack/nova master: test.stub_object https://review.openstack.org/560683 | |
| 23:30:46 | openstackgerrit | Chris Friesen proposed openstack/nova stable/pike: libvirt: disconnect volume from host during detach https://review.openstack.org/560690 | |
| 23:42:32 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: api-ref: Example verification for servers.inc https://review.openstack.org/529520 | |
| 23:50:17 | gmann | sdague: can you feedback on this comment (one of TODO from you) - https://review.openstack.org/#/c/529520/18/api-ref/source/servers.inc@765 | |
| 23:51:18 | gmann | sdague: i feel single latest sample response file is good enough instead of separate sample files per request attribute for update | |
| 23:51:36 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in test_neutron_security_groups.py https://review.openstack.org/560267 | |
| #openstack-nova - 2018-04-12 | |||
| 00:04:01 | mnaser | hey | |
| 00:04:03 | mnaser | fun bug time | |
| 00:05:04 | mnaser | if a user builds an instance that if boot from volume (where nova creates the volume) and nova-compute fails to create the volume, the vm build fails, and it actually counts as a 'consecutive build failures' | |
| 00:05:29 | mnaser | so if i'm $bad_user and i try to launch 40 instances but keep hitting quota limits but keep trying again, i will effectively slowly but surely disable every compute node in the cloud | |
| 00:06:14 | mnaser | so while that feature is nice, i think we have to find a way to identify the type of failure it is | |
| 00:08:10 | mnaser | i mean we could disable that feature but that feels dirty | |
| 00:14:36 | mriedem | mnaser: yeah https://bugs.launchpad.net/nova/+bug/1742102 | |
| 00:14:37 | openstack | Launchpad bug 1742102 in OpenStack Compute (nova) "Simple user can disable compute" [Undecided,Confirmed] - Assigned to jichenjc (jichenjc) | |
| 00:14:40 | mriedem | there is an ops ML thread about similar issues | |
| 00:14:59 | mriedem | i think we likely need to consider a whitelist of acceptable, not threshold inducing exceptions | |
| 00:15:30 | mriedem | the latest comments in there from jichen are actually the exact issue you're describing | |
| 00:17:07 | mriedem | but we can also be smarter and check quota before trying to create volumes | |
| 00:17:09 | mriedem | like we do for ports | |
| 00:17:27 | mnaser | mriedem: yeah, i think that's better, do we check quota at api layer for ports? | |
| 00:17:29 | mriedem | could do that in conductor so we don't block the API response | |
| 00:17:33 | mriedem | mnaser: yes | |
| 00:17:40 | mriedem | it's the validate_networks call | |
| 00:17:50 | mnaser | that sounds reasonable and backport-able too i think | |
| 00:17:52 | mriedem | based on the number of requested networks and instances, we check quota | |
| 00:18:22 | mnaser | the thing is if i have a quota of 10 volumes, and i launch a 100 instances (one by one), i might still run into that issue i guess | |
| 00:18:23 | mriedem | yeah - either way you'd fail after the 202 you get from the API | |
| 00:18:31 | mriedem | you either fail in conductor or you fail in compute | |
| 00:18:45 | mriedem | sure but we don't disable your compute :) | |
| 00:18:54 | mnaser | because of the race as conductor still sees you using 0 volumes but as computes start creating volumes | |
| 00:19:05 | mriedem | oh, sure | |
| 00:19:23 | mriedem | we could also just handle volume quota issues in the compute as a whitelist of things to not disable the compute | |
| 00:19:29 | mriedem | multi-part fixes | |
| 00:19:49 | mnaser | i think both out-of-ports and out-of-volumes both seem like reasonable 'skip' failures | |
| 00:20:01 | mriedem | yeah | |
| 00:20:11 | mriedem | the bug above started as a port / fixed ips quota issue | |
| 00:20:15 | mriedem | but also extends to volumes | |
| 00:20:21 | mnaser | i think any api exception should be skipped tbh | |
| 00:20:29 | mnaser | because that means the compute node is fine | |
| 00:20:51 | mnaser | sure it sounds vague but if your cinder is not having a good time then you're slowly disabling all compute nodes | |