| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-17 | |||
| 14:01:15 | efried | dansmith I'll wait to respond until I see what you have to say, then. | |
| 14:02:38 | bauzas | mriedem: sure, will look | |
| 14:03:01 | gibi | mriedem: there is another bug 1724172 with fix top of https://review.openstack.org/#/c/510938/ | |
| 14:03:02 | openstack | bug 1724172 in OpenStack Compute (nova) "Allocation of an evacuated instance is not cleaned on the source host if instance is not defined on the hypervisor" [Undecided,In progress] https://launchpad.net/bugs/1724172 - Assigned to Balazs Gibizer (balazs-gibizer) | |
| 14:03:38 | bauzas | edmondsw: sdague: does that https://bugs.launchpad.net/nova/+bug/1716344 ring a bell to you ? (tl;dr: the fact that we only lookup the public endpoint when querying the SC) | |
| 14:03:39 | openstack | Launchpad bug 1716344 in OpenStack Compute (nova) "Nova-API uses Keystone's public endpoint for project id verification" [Undecided,New] | |
| 14:04:39 | johnthetubaguy | edmondsw: very late but I found that patch you were asking me about that the PTG, at least I think you asked me: https://review.openstack.org/#/c/434870 | |
| 14:07:26 | mriedem | ildikov: reading | |
| 14:07:44 | sdague | bauzas: so, we might be missing an option there | |
| 14:07:56 | sdague | that being said, this is definitely never called in vm crate | |
| 14:07:58 | sdague | create | |
| 14:08:20 | sdague | the only places this path is called is quota updates and flavor access calls | |
| 14:09:14 | dansmith | efried: left you a suggestion about the actual method, and one about cdent's retry | |
| 14:09:35 | dansmith | efried: the retry could be a follow-on since it really isn't related to this if you use the common method | |
| 14:09:39 | ildikov | mriedem: tnx | |
| 14:09:57 | efried | dansmith Roger wilco, and thanks. | |
| 14:10:13 | ildikov | mriedem: as the spec deadline is coming up I'm trying to clean this one up so we can merge it and update later on specifics if needed | |
| 14:10:53 | efried | dansmith Did you mean report.SchedulerReportClient.get_allocations_for_instance? | |
| 14:11:04 | dansmith | efried: rebase | |
| 14:11:15 | efried | dansmith ah, beaut. | |
| 14:11:21 | dansmith | efried: https://review.openstack.org/#/c/511306/ | |
| 14:11:25 | mriedem | bauzas: replied in that bug | |
| 14:11:36 | bauzas | sdague: you mean, calling verify_project_id is just made by quota updates and flavor calls? | |
| 14:11:41 | mriedem | efried: your new ksa adapter stuff defaults to the internal interface right? | |
| 14:11:45 | mriedem | it goes internal and then public? | |
| 14:11:51 | mriedem | bauzas: correct | |
| 14:11:56 | mriedem | it wouldn't make vm create fail | |
| 14:12:12 | efried | mriedem Yes | |
| 14:12:34 | efried | mriedem nova.conf.utils.py L47 | |
| 14:12:49 | mriedem | ok, just checking. we can't rely on that for fixing this anyway. | |
| 14:12:54 | mriedem | since the fix here would have to be backported | |
| 14:13:31 | bauzas | mriedem: okay, so I'll triage it as Wontfix if that bug is punted by efried's KSA rework | |
| 14:13:46 | mriedem | bauzas: i don't think that's the right way to handle this | |
| 14:13:56 | bauzas | open to ideas :) | |
| 14:13:58 | mriedem | you can't backport the ksa fixes | |
| 14:14:13 | bauzas | so we at least need to idenfify the impact | |
| 14:14:13 | mriedem | and we effectively made a backward incompatible change in pike to those apis | |
| 14:14:29 | mriedem | you can't update quota or flavor access if nova can't access keystone's public endpoint | |
| 14:14:51 | bauzas | so it was a design decision to use the public endpoint for such calls ? | |
| 14:15:02 | bauzas | I'm having trouble understanding your sentence about ^ | |
| 14:15:13 | mriedem | yes, but the same design decision was made with placement and we later changed that when someone said it broke their deployment | |
| 14:15:18 | bauzas | I mean "There was a conscious decision to hard-code the public endpoint as the interface when this change was made, but we changed that hard-coding for nova talking to the placement endpoint so I don't see why we wouldn't also allow different endpoints for talking to keystone. " | |
| 14:15:36 | bauzas | ah gotcha | |
| 14:15:56 | bauzas | mriedem: there are 2 possibilities honestly | |
| 14:16:07 | mriedem | https://review.openstack.org/#/c/435010/6/nova/identity.py@34 | |
| 14:16:21 | bauzas | mriedem: either we say it's a bug that is backportable, and I'm tagging the bug as Confirmed | |
| 14:16:32 | edmondsw | johnthetubaguy thanks! Looking at it now | |
| 14:17:02 | sdague | mriedem: what is your concern here? | |
| 14:17:03 | bauzas | mriedem: or we just consider it's more a new design rearchitecture, and in that case, that would make the changes very difficult to backport | |
| 14:17:52 | mriedem | "Keystone's public endpoint should only visible to external clients. All internal OpenStack services should use the internalURL for authentication purposes. I think my configuration is correct. The "auth_url" point to Keystone's internal URL, whereas "auth_uri" points to Keystone's public endpoint. I want to avoid https based communication for my internal cloud services." | |
| 14:19:03 | sdague | mriedem: yes, I've read the bug. I don't understand the backwards incompatibility concern | |
| 14:20:02 | mriedem | asking for clarification in the bug | |
| 14:20:09 | mriedem | it's not clear to me if the apis fail, or if they just dump errors | |
| 14:20:47 | sdague | I think it's a real fail | |
| 14:20:58 | sdague | but not on the API call they stated | |
| 14:21:05 | mriedem | yes it's not instance create | |
| 14:21:13 | mriedem | they later said it was the flavor access project id verification | |
| 14:21:23 | mriedem | "It took some time to narrow down the problem. The issue was introduced with the Pike release, where project id verification for flavor access and quota modification got added." | |
| 14:22:22 | gongysh | hi | |
| 14:22:32 | mriedem | i don't think the api actually fails, they hit this: https://github.com/openstack/nova/blob/master/nova/api/openstack/identity.py#L57 | |
| 14:22:37 | gongysh | it seems the nested-quota is not implemented yet. right? | |
| 14:22:38 | mriedem | which dumps a stacktrace and returns True | |
| 14:22:42 | mriedem | gongysh: correct | |
| 14:23:14 | gongysh | mriedem, do you if there is alter option? | |
| 14:23:19 | sdague | mriedem: it might b | |
| 14:23:26 | mriedem | gongysh: not in nova no | |
| 14:23:45 | mriedem | gongysh: see the unified limits effort in keystone | |
| 14:23:58 | mriedem | gongysh: https://specs.openstack.org/openstack/keystone-specs/specs/keystone/ongoing/unified-limits.html | |
| 14:29:55 | mriedem | ildikov: johnthetubaguy: replied in https://review.openstack.org/#/c/499777/4 about delete_on_termination | |
| 14:30:14 | mriedem | tl;dr is that if we fail to delete a volume we handle the error and log a warning, everywhere | |
| 14:30:54 | johnthetubaguy | mriedem: so you think we just allow delete_on_termination for multi-attach volumes? | |
| 14:30:58 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 14:30:59 | mriedem | so if you boot from volume with a multi-attach volume and specify delete_on_termination=True, and that volume is attached to another instance when the first is deleted, it won't prevent the deletion on the first instance, it will just log a warning in the logs about being unable to delete the volume | |
| 14:31:32 | johnthetubaguy | I guess it would just be deleted by the last detach, probably | |
| 14:31:34 | mriedem | johnthetubaguy: i'm not sure there is a great reason to not allow it | |
| 14:31:44 | mriedem | johnthetubaguy: well, not necessarily | |
| 14:32:12 | mriedem | if instance A and B are attached to volume X, and A.delete_on_termination=True and B.delete_on_termination=False, then deleting A won't delete X and deleting B won't either | |
| 14:32:15 | johnthetubaguy | I think its a race for the delete, like it could delete it | |
| 14:32:17 | johnthetubaguy | yeah | |
| 14:32:30 | johnthetubaguy | I should have defined my "probably" bit better | |
| 14:32:51 | mriedem | if B is deleted before A, then yes X would be deleted | |
| 14:33:00 | mriedem | ultimately the lock is supposed to be on the cinder side | |
| 14:33:15 | johnthetubaguy | ++ | |
| 14:33:43 | mriedem | and i think it's just something a user would have to understand if they are attaching a multi-attach volume to an instance with delete_on_termination, that nova can't actually delete that volume if it's attached to another instance | |
| 14:33:56 | johnthetubaguy | OK, my thinking was, seems odd to delete a multi-attach volume on delete, gonna be racey, lets just not allow that flag to be set. | |
| 14:33:59 | ildikov | so the conclusion is to allow it, right? | |
| 14:34:02 | ildikov | mriedem: +1 | |
| 14:34:07 | mriedem | ildikov: i don't see a reason not to | |
| 14:34:18 | ildikov | mriedem: cool, my thinking as well | |
| 14:34:27 | johnthetubaguy | OK, so you are both thinking that way, willing to go with that | |
| 14:34:38 | ildikov | johnthetubaguy: thank you :) | |
| 14:35:25 | openstackgerrit | Eric Fried proposed openstack/nova master: Send Allocations to spawn https://review.openstack.org/511879 | |
| 14:35:52 | openstackgerrit | Dan Smith proposed openstack/nova master: Make migration uuid hold allocations for migrating instances https://review.openstack.org/506420 | |
| 14:35:52 | openstackgerrit | Dan Smith proposed openstack/nova master: Make live migration hold resources with a migration allocation https://review.openstack.org/507638 | |
| 14:56:15 | johnthetubaguy | mriedem: if you get a moment, I attempted pseudo code for the shared connection locking, that feels like the last bit of detail we need in there: https://review.openstack.org/#/c/499777/5/specs/queens/approved/cinder-volume-multi-attach.rst@53 | |
| 14:58:06 | sean-k-mooney | johnthetubaguy: cfriesen am i remebering correctly that ye were both interested in bandwidth based scheduling at the ptg | |
| 14:58:40 | johnthetubaguy | sean-k-mooney: I was probably incorrectly interested, snowed up with other things honestly | |
| 14:59:19 | sean-k-mooney | johnthetubaguy: ack ok i was just going to flag https://review.openstack.org/#/c/502306/12 to you if you cared to review | |
| 15:00:05 | johnthetubaguy | sean-k-mooney: cool, can't promise I will get to that I am afraid, chasing a few other things | |
| 15:00:26 | sean-k-mooney | johnthetubaguy: no worries | |