Switch volume delete to use the ID

At the moment, the current code that's used when deleting volumes
uses a function that attempts to pull by ID, and then falls back
to name.

This is potentially dangerous because it might match more than one
volume and therefore potentially delete the wrong volume.  This
patch revises the code to enforce retrieving the volume based on
the ID instead which is globally unique.

The tools that leverage this inside releng-global-jjb already
provide an ID at the moment[1].

[1]: https://github.com/lfit/releng-global-jjb/blob/d22d069117b64cee73be1165861013de9e79ab19/shell/openstack-cleanup-orphaned-volumes.sh#L18

Change-Id: I078c8c4447ac308c66678e86ae31e8947f40340e
Signed-off-by: Mohammed Naser <mnaser@vexxhost.com>

diff --git a/lftools/openstack/cmd.py b/lftools/openstack/cmd.py
index 5e22dc1..cc3dde4 100644 (file)
--- a/lftools/openstack/cmd.py
+++ b/lftools/openstack/cmd.py
@@ -276,16 +276,16 @@ def list(ctx, days):
 
 
 @click.command()
-@click.argument('volume')
+@click.argument('volume_id')
 @click.option(
     '--minutes', type=int, default=0,
     help='Delete volumes if older than x minutes.')
 @click.pass_context
-def remove(ctx, volume, minutes):
+def remove(ctx, volume_id, minutes):
     """Remove volumes."""
     os_volume.remove(
         ctx.obj['os_cloud'],
-        volume_name=volume,
+        volume_id=volume_id,
         minutes=minutes)
 
 

diff --git a/lftools/openstack/volume.py b/lftools/openstack/volume.py
index a3da966..61dba3c 100644 (file)
--- a/lftools/openstack/volume.py
+++ b/lftools/openstack/volume.py
@@ -73,14 +73,15 @@ def cleanup(os_cloud, days=0):
     _remove_volumes_from_cloud(filtered_volumes, cloud)
 
 
-def remove(os_cloud, volume_name, minutes=0):
+def remove(os_cloud, volume_id, minutes=0):
     """Remove a volume from cloud.
 
     :arg str os_cloud: Cloud name as defined in OpenStack clouds.yaml.
+    :arg str volume_id: Volume ID to delete
     :arg int minutes: Only delete volume if it is older than number of minutes.
     """
     cloud = shade.openstack_cloud(cloud=os_cloud)
-    volume = cloud.get_volume(volume_name)
+    volume = cloud.get_volume_by_id(volume_id)
 
     if not volume:
         print("ERROR: volume not found.")
@@ -91,4 +92,4 @@ def remove(os_cloud, volume_name, minutes=0):
         print('WARN: volume "{}" is not older than {} minutes.'.format(
             volume.name, minutes))
     else:
-        cloud.delete_volume(volume.name)
+        cloud.delete_volume(volume.id)