On 3/30/2016 9:14 PM, GHANSHYAM MANN wrote:
On Thu, Mar 31, 2016 at 10:39 AM, Matt Riedemann
<[email protected]> wrote:
On 3/30/2016 8:20 PM, Matt Riedemann wrote:
On 3/30/2016 7:56 PM, Matt Riedemann wrote:
On 3/30/2016 7:38 PM, Matt Riedemann wrote:
On 2/25/2016 5:31 AM, Takashi Natsume wrote:
Hi Nova and Cinder developers.
As I reported in a bug report [1], nova swap volume
(updating an attached volume) fuction does not work
in the case of non admin users by default.
(Volumes are stuck.)
Before I was working for fixing another swap volume bug [2][3].
But Ryan fixed it on the Cinder side [4].
As a result, admin users can execute swap volume function,
but it was not fixed in the case of non admin users.
So I reported the bug report [1].
In the patch[5], I tried to change the default cinder's policy
to allow non admin users to execute migrate_volume_completion API.
But it was rejected by the cinder project ('-2' was voted).
In the patch[5], it was suggested to make the swap volume API admin
only
on the Nova side.
But IMO, the swap volume function should be allowed to non admin users
because attaching a volume and detaching a volume can be performed
by non admin users.
I agree with this. DuncanT said in IRC that he didn't think non-admin
users should be using the swap-volume API in nova because it can be
problematic, but I'm not sure why, is there more history or detail
there? I'd think it shouldn't be any worse than doing a detach/attach in
quick succession (like in a CI test for example).
If migrate_volume_completion is only allowed to admin users
by default on the Cinder side, attaching a new volume and
detaching an old volume should be performed on the Nova side
when swapping volumes.
My understanding of the problem is as follows:
1. Admin-initiated volume migration in Cinder calls off to Nova to
perform the swap-volume, and then Nova calls back to Cinder's
migrate_volume_completion API. This is fine since it's an admin that
initiated this series of operations on the Cinder side (that's by
default, however, this is broken if the policy file for Cinder is change
to allow non-admins to migrate volumes).
2. A non-admin swap-volume API call in Nova fails because Nova blindly
makes the migrate_volume_completion call to Cinder which fails with a
403 because the Cinder API policy has that as an admin action by
default.
I don't know the history around when the swap-volume API was added to
Nova, was it specifically for this volume migration scenario in Cinder?
Are there other use cases? Knowing those would be good to determine
if Nova should change it's default policy for swap-volume, although,
again, that's only a default and can be changed per deployment so we
probably shouldn't rely on it.
Ideally we would have implemented this like the nova/neutron server
events callback API in Nova during vif plugging (nova does the vif plug
on the host then waits for neutron to update it's database for the port
status and sends an event (API call) to nova to continue booting the
server). That server events API in nova is admin-only by default and
neutron is configured with admin credentials for nova to use it.
Another option would be for Nova to handle a 403 response when calling
Cinder's migrate_volume_completion API and ignore it if we don't have an
admin context. This is pretty hacky though. It assumes that it's a
non-admin user initiating the swap-volume operation. It wouldn't be a
problem for the volume migration operation initiated in Cinder since by
default that's admin-only, so nova shouldn't get a 403 when calling
migrate_volume_completion. The trap would be if the cinder policy for
volume migration was changed to allow non-admins, but if someone did
that, they should also change the policy for migrate_volume_completion
to allow non-admin too.
If you have a good idea, please let me know it.
[1] Cinder volumes are stuck when non admin user executes nova swap
volume API
https://bugs.launchpad.net/cinder/+bug/1522705
[2] Cinder volume stuck in swap_volume
https://bugs.launchpad.net/nova/+bug/1471098
[3] Fix cinder volume stuck in swap_volume
https://review.openstack.org/#/c/207385/
[4] Fix swap_volume for case without migration
https://review.openstack.org/#/c/247767/
[5] Enable volume owners to execute migrate_volume_completion
https://review.openstack.org/#/c/253363/
Regards,
Takashi Natsume
NTT Software Innovation Center
E-mail: [email protected]
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe:
[email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
I also just checked Tempest and apparently we have no coverage for the
swap-volume API in Nova, we should fix that as part of this.
I've done some more digging. The swap-volume functionality was added to
nova here [1]. The cinder use of it for volume migration was added here
[2].
Looking at the cinder volume API for migrate_volume_completion, it
expects the source (old) volume to have migration_status set [3].
So, I think we can easily fix this in Nova by simply not calling
volume_migration_completion if old_volume['migration_status'] is None.
[1]
https://github.com/openstack/nova/commit/8f51b120b430c7c21399256f37e1d8f75d030484
[2]
https://github.com/openstack/nova/commit/0e4bd7f93b9bfadcc2bb6dfaeae7bb5ee00c194b
[3]
http://git.openstack.org/cgit/openstack/cinder/tree/cinder/volume/api.py#n1358
Of course that had to be too easy to be true. The volume 'migration_status'
is only returned for the volume details if you're calling with an admin
context [1].
I think we can still use this, we just can't expect
volume['migration_status'] to be in the response from the volume GET. If
it's not there, we can assume we're not doing a migration and we're not an
admin anyway, so we can't call migrate_volume_completion.
So in that case, we need to attach, detach volume on nova side right?
I mean if migrate_volume_completion is not being called then
new volume attachment and old volume detachment should be initiated explicitly.
Can we make Nova default policy same as cinder, i mean swap volume
allowed only for admin? Because if there is simple swap initiated from
nova(not cinder migration), Nova allow that operation for non-admin
user and get stuck to attaching/detaching status.
[1]
http://git.openstack.org/cgit/openstack/cinder/tree/cinder/api/v2/views/volumes.py#n82-L84
--
Thanks,
Matt Riedemann
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Good question, I'm assuming the virt driver does the attach/detach on
the nova side, but I haven't dug into it or verified the fix myself. I
wanted to write a Tempest test for the non-admin swap-volume case in Nova.
Changing the nova policy to be admin-only by default for swap-volume was
another easy option, I just wasn't sure if it was the right thing to do
since this was not initially added as an admin-only operation, and it
seems the only thing that's made it become that is the cinder volume
migration callback case.
--
Thanks,
Matt Riedemann
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: [email protected]?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
