No delete volume action for volume with snapshots

Currently a volume with snapshots still has the "Delete Volume" menu
action.

This fix will not allow the "Delete Volume" menu action for a volume
that has snapshots. The main purpose of this change is to be consistent
with the existing behavior when the volume is not in a deletable state.

Closes-Bug: #1394015
Change-Id: If298e04952a29acbe3e8bd89e54065525ae5f30c

openstack_dashboard/dashboards/admin/volumes/tabs.py

@@ -40,7 +40,10 @@ class VolumeTab(tabs.TableTab, volumes_tabs.VolumeTableMixIn):
     def get_volumes_data(self):
         volumes = self._get_volumes(search_opts={'all_tenants': True})
         instances = self._get_instances(search_opts={'all_tenants': True})
-        self._set_attachments_string(volumes, instances)
+        volume_ids_with_snapshots = self._get_volumes_ids_with_snapshots(
+            search_opts={'all_tenants': True})
+        self._set_volume_attributes(
+            volumes, instances, volume_ids_with_snapshots)
 
         # Gather our tenants to correlate against IDs
         try:

openstack_dashboard/dashboards/project/volumes/tabs.py

@@ -49,9 +49,30 @@ class VolumeTableMixIn(object):
                                 "attachment information"))
             return []
 
-    def _set_attachments_string(self, volumes, instances):
+    def _get_volumes_ids_with_snapshots(self, search_opts=None):
+        try:
+            volume_ids = []
+            snapshots = api.cinder.volume_snapshot_list(
+                self.request, search_opts=search_opts)
+            if snapshots:
+                # extract out the volume ids
+                volume_ids = set([(s.volume_id) for s in snapshots])
+        except Exception:
+            exceptions.handle(self.request,
+                              _("Unable to retrieve snapshot list."))
+
+        return volume_ids
+
+    # set attachment string and if volume has snapshots
+    def _set_volume_attributes(self,
+                               volumes,
+                               instances,
+                               volume_ids_with_snapshots):
         instances = SortedDict([(inst.id, inst) for inst in instances])
         for volume in volumes:
+            if volume_ids_with_snapshots:
+                if volume.id in volume_ids_with_snapshots:
+                    setattr(volume, 'has_snapshot', True)
             for att in volume.attachments:
                 server_id = att.get('server_id', None)
                 att['instance'] = instances.get(server_id, None)
@@ -67,7 +88,9 @@ class VolumeTab(tabs.TableTab, VolumeTableMixIn):
     def get_volumes_data(self):
         volumes = self._get_volumes()
         instances = self._get_instances()
-        self._set_attachments_string(volumes, instances)
+        volume_ids_with_snapshots = self._get_volumes_ids_with_snapshots()
+        self._set_volume_attributes(
+            volumes, instances, volume_ids_with_snapshots)
         return volumes
 
 

openstack_dashboard/dashboards/project/volumes/test.py

@@ -45,6 +45,8 @@ class VolumeAndSnapshotsTests(test.TestCase):
             AndReturn(volumes)
         api.nova.server_list(IsA(http.HttpRequest), search_opts=None).\
             AndReturn([self.servers.list(), False])
+        api.cinder.volume_snapshot_list(
+            IsA(http.HttpRequest), search_opts=None).AndReturn(vol_snaps)
         api.cinder.volume_snapshot_list(IsA(http.HttpRequest)).\
             AndReturn(vol_snaps)
         api.cinder.volume_list(IsA(http.HttpRequest)).AndReturn(volumes)
@@ -52,8 +54,8 @@ class VolumeAndSnapshotsTests(test.TestCase):
             api.cinder.volume_backup_list(IsA(http.HttpRequest)).\
                 AndReturn(vol_backups)
             api.cinder.volume_list(IsA(http.HttpRequest)).AndReturn(volumes)
-        api.cinder.tenant_absolute_limits(IsA(http.HttpRequest)).MultipleTimes(). \
+        api.cinder.tenant_absolute_limits(IsA(http.HttpRequest)).\
-            AndReturn(self.cinder_limits['absolute'])
+            MultipleTimes().AndReturn(self.cinder_limits['absolute'])
         self.mox.ReplayAll()
 
         res = self.client.get(INDEX_URL)

openstack_dashboard/dashboards/project/volumes/volumes/tables.py

@@ -80,19 +80,12 @@ class DeleteVolume(VolumePolicyTargetMixin, tables.DeleteAction):
     policy_rules = (("volume", "volume:delete"),)
 
     def delete(self, request, obj_id):
-        obj = self.table.get_object_by_id(obj_id)
+        cinder.volume_delete(request, obj_id)
-        name = self.table.get_object_display(obj)
-        try:
-            cinder.volume_delete(request, obj_id)
-        except Exception:
-            msg = _('Unable to delete volume "%s". One or more snapshots '
-                    'depend on it.')
-            exceptions.check_message(["snapshots", "dependent"], msg % name)
-            raise
 
     def allowed(self, request, volume=None):
         if volume:
-            return volume.status in DELETABLE_STATES
+            return (volume.status in DELETABLE_STATES and
+                    not getattr(volume, 'has_snapshot', False))
         return True
 
 

openstack_dashboard/dashboards/project/volumes/volumes/tests.py

@@ -819,41 +819,28 @@ class VolumeViewTests(test.TestCase):
         self.assertIn("Scheduled deletion of Volume: Volume name",
                       [m.message for m in res.context['messages']])
 
-    @test.create_stubs({cinder: ('tenant_absolute_limits',
+    @test.create_stubs({cinder: ('volume_get',
-                                 'volume_list',
+                                 'tenant_absolute_limits')})
-                                 'volume_backup_supported',
+    def test_delete_volume_with_snap_no_action_item(self):
-                                 'volume_delete',),
+        volume = self.cinder_volumes.get(name='Volume name')
-                        api.nova: ('server_list',)})
+        setattr(volume, 'has_snapshot', True)
-    def test_delete_volume_error_existing_snapshot(self):
+        limits = self.cinder_limits['absolute']
-        volume = self.cinder_volumes.first()
+
-        volumes = self.cinder_volumes.list()
+        cinder.volume_get(IsA(http.HttpRequest), volume.id).AndReturn(volume)
-        formData = {'action':
+        cinder.tenant_absolute_limits(IsA(http.HttpRequest)). \
-                    'volumes__delete__%s' % volume.id}
+            MultipleTimes('limits').AndReturn(limits)
-        exc = self.exceptions.cinder.__class__(400,
-                                               "error: dependent snapshots")
 
-        cinder.volume_backup_supported(IsA(http.HttpRequest)). \
-            MultipleTimes().AndReturn(True)
-        cinder.volume_list(IsA(http.HttpRequest), search_opts=None).\
-            AndReturn(volumes)
-        cinder.volume_delete(IsA(http.HttpRequest), volume.id).\
-            AndRaise(exc)
-        api.nova.server_list(IsA(http.HttpRequest), search_opts=None).\
-            AndReturn([self.servers.list(), False])
-        cinder.volume_list(IsA(http.HttpRequest), search_opts=None).\
-            AndReturn(volumes)
-        api.nova.server_list(IsA(http.HttpRequest), search_opts=None).\
-            AndReturn([self.servers.list(), False])
-        cinder.tenant_absolute_limits(IsA(http.HttpRequest)).MultipleTimes().\
-            AndReturn(self.cinder_limits['absolute'])
         self.mox.ReplayAll()
 
-        url = VOLUME_INDEX_URL
+        url = (VOLUME_INDEX_URL +
-        res = self.client.post(url, formData, follow=True)
+               "?action=row_update&table=volumes&obj_id=" + volume.id)
-        self.assertEqual(list(res.context['messages'])[0].message,
+
-                         u'Unable to delete volume "%s". '
+        res = self.client.get(url, {}, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
-                         u'One or more snapshots depend on it.' %
+
-                         volume.name)
+        self.assertEqual(res.status_code, 200)
+
+        self.assertNotContains(res, 'Delete Volume')
+        self.assertNotContains(res, 'delete')
 
     @test.create_stubs({cinder: ('volume_get',), api.nova: ('server_list',)})
     @override_settings(OPENSTACK_HYPERVISOR_FEATURES={'can_set_mount_point':