| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 20:49:15 | mriedem | https://github.com/openstack/nova/blob/2c5da2212c3fa3e589c4af171486a2097fd8c54e/nova/compute/resource_tracker.py#L714 | |
| 20:49:38 | dansmith | deleting the service today which deletes the compute node will break all manner of stuff, if not deleting instances and quotas, because we'll get a new compute node uuid | |
| 20:50:09 | dansmith | but likely just placement-forward things where the uuid started to be a thing | |
| 20:50:25 | mriedem | yeah. if we block the service delete, you have options to not f*ck yourself, | |
| 20:50:29 | mriedem | like migrate the instance | |
| 20:50:30 | melwitt | tssurya: oh, yep, you're right. I forgot ... cause we have to be able to get all instances for a project/user combo and we can't know that from instance_mappings (because it lacks user_id). but we could have done a pruning by project_id based on instance_mappings | |
| 20:50:59 | tssurya | melwitt: which is what we plan to do once we have your spec in :D | |
| 20:51:05 | mriedem | this is sort of like the "should we allow AZ renames for aggregates with hosts that have instances on them" | |
| 20:51:23 | mriedem | knowing that if you do ^ you'll likely break some stuff | |
| 20:51:27 | dansmith | yeah | |
| 20:51:43 | melwitt | tssurya: heh, yeah :) | |
| 20:51:53 | mriedem | ok so is this considered a separate bug? | |
| 20:52:01 | mriedem | or do we roll it into the same fix? | |
| 20:52:02 | dansmith | it's a separate patch IMHO | |
| 20:52:09 | dansmith | which is why I was saying they're two different concerns | |
| 20:52:12 | mriedem | that's kind of what i was thinking, it would be easier that way anyway | |
| 20:52:28 | melwitt | +1 to separate patches | |
| 20:52:30 | mriedem | we can build on the same functional test base i started | |
| 20:53:00 | tssurya | sure | |
| 20:53:02 | dansmith | yeah | |
| 20:53:31 | mriedem | tssurya: you want me to open a separate bug? | |
| 20:53:58 | tssurya | mriedem: yea | |
| 20:54:04 | mriedem | also, it's 11pm there right? | |
| 20:54:15 | tssurya | I will put up a patch tomorrow | |
| 20:54:17 | mriedem | do you get to bill belmiro for overtime? | |
| 20:54:30 | tssurya | mriedem: yes its 11pm :) | |
| 20:54:33 | tssurya | haha no | |
| 20:55:04 | tssurya | this is me learning stuff from the veterans :P | |
| 21:22:40 | openstackgerrit | Merged openstack/nova master: Remove mox in tests/unit/test_utils.py https://review.openstack.org/557883 | |
| 21:52:10 | mriedem | cdent: edleafe: oh seers of http status code wisdom, hear my call, shall thou return a 400 or 409 for https://bugs.launchpad.net/nova/+bug/1763183 ? | |
| 21:52:10 | openstack | Launchpad bug 1763183 in OpenStack Compute (nova) "DELETE /os-services/{service_id} does not block for hosted instances" [High,Triaged] - Assigned to Matt Riedemann (mriedem) | |
| 21:52:41 | mriedem | i think 409 | |
| 21:53:03 | cdent | i feel like I should have some kind of a 409phone | |
| 21:53:26 | mriedem | oh and if 409, guess what, @wsgi.expected_errors((400, 404)) | |
| 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 | |