Follow name_or_id pattern on domain operations

On pretty much all resources you can pass either a name or an ID
on CRUD operations.
This change does exactly that for domains.

Change-Id: I88da81a6a62839f4eedc362fd15cb38149b6efd8

releasenotes/notes/domain_operations_name_or_id-baba4cac5b67234d.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Added name_or_id parameter to domain operations, allowing
+    an admin to update/delete/get by domain name.

shade/operatorcloud.py

@@ -1077,7 +1077,20 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         return _utils.normalize_domains([domain])[0]
 
     def update_domain(
-            self, domain_id, name=None, description=None, enabled=None):
+            self, domain_id=None, name=None, description=None,
+            enabled=None, name_or_id=None):
+        if domain_id is None:
+            if name_or_id is None:
+                raise OpenStackCloudException(
+                    "You must pass either domain_id or name_or_id value"
+                )
+            dom = self.get_domain(None, name_or_id)
+            if dom is None:
+                raise OpenStackCloudException(
+                    "Domain {0} not found for updating".format(name_or_id)
+                )
+            domain_id = dom['id']
+
         with _utils.shade_exceptions(
                 "Error in updating domain {domain}".format(domain=domain_id)):
             domain = self.manager.submitTask(_tasks.DomainUpdate(
@@ -1085,23 +1098,37 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
                 enabled=enabled))
         return _utils.normalize_domains([domain])[0]
 
-    def delete_domain(self, domain_id):
+    def delete_domain(self, domain_id=None, name_or_id=None):
         """Delete a Keystone domain.
 
         :param domain_id: ID of the domain to delete.
+        :param name_or_id: Name or ID of the domain to delete.
 
-        :returns: None
+        :returns: True if delete succeeded, False otherwise.
 
         :raises: ``OpenStackCloudException`` if something goes wrong during
             the openstack API call.
         """
-        with _utils.shade_exceptions("Failed to delete domain {id}".format(
-                id=domain_id)):
+        if domain_id is None:
+            if name_or_id is None:
+                raise OpenStackCloudException(
+                    "You must pass either domain_id or name_or_id value"
+                )
+            dom = self.get_domain(None, name_or_id)
+            if dom is None:
+                self.log.debug(
+                    "Domain {0} not found for deleting".format(name_or_id))
+                return False
+            domain_id = dom['id']
+
+        with _utils.shade_exceptions(
+            "Failed to delete domain {id}".format(id=domain_id)):
             # Deleting a domain is expensive, so disabling it first increases
             # the changes of success
             domain = self.update_domain(domain_id, enabled=False)
-            self.manager.submitTask(_tasks.DomainDelete(
-                domain=domain['id']))
+            self.manager.submitTask(_tasks.DomainDelete(domain=domain['id']))
+
+        return True
 
     def list_domains(self):
         """List Keystone domains.
@@ -1115,9 +1142,10 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
             domains = self.manager.submitTask(_tasks.DomainList())
         return _utils.normalize_domains(domains)
 
-    def search_domains(self, filters=None):
+    def search_domains(self, filters=None, name_or_id=None):
         """Search Keystone domains.
 
+        :param name_or_id: domain name or id
         :param dict filters: A dict containing additional filters to use.
              Keys to search on are id, name, enabled and description.
 
@@ -1130,15 +1158,22 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :raises: ``OpenStackCloudException``: if something goes wrong during
             the openstack API call.
         """
+        if name_or_id is not None:
+            domains = self.list_domains()
+            return _utils._filter_list(domains, name_or_id, filters)
+        else:
             with _utils.shade_exceptions("Failed to list domains"):
                 domains = self.manager.submitTask(
                     _tasks.DomainList(**filters))
             return _utils.normalize_domains(domains)
 
-    def get_domain(self, domain_id):
+    def get_domain(self, domain_id=None, name_or_id=None, filters=None):
         """Get exactly one Keystone domain.
 
         :param domain_id: domain id.
+        :param name_or_id: domain name or id.
+        :param dict filters: A dict containing additional filters to use.
+             Keys to search on are id, name, enabled and description.
 
         :returns: a dict containing the domain description, or None if not
             found. Each dict contains the following attributes::
@@ -1149,6 +1184,9 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :raises: ``OpenStackCloudException``: if something goes wrong during
             the openstack API call.
         """
+        if domain_id is None:
+            return _utils._get_entity(self.search_domains, filters, name_or_id)
+        else:
             with _utils.shade_exceptions(
                 "Failed to get domain "
                 "{domain_id}".format(domain_id=domain_id)

shade/tests/functional/test_domain.py

@@ -66,6 +66,11 @@ class TestDomain(base.BaseFunctionalTestCase):
         self.assertEqual(1, len(results))
         self.assertEqual(domain_name, results[0]['name'])
 
+        # Now we search by name with name_or_id, should find only new domain
+        results = self.operator_cloud.search_domains(name_or_id=domain_name)
+        self.assertEqual(1, len(results))
+        self.assertEqual(domain_name, results[0]['name'])
+
     def test_update_domain(self):
         domain = self.operator_cloud.create_domain(
             self.domain_prefix, 'description')
@@ -78,3 +83,42 @@ class TestDomain(base.BaseFunctionalTestCase):
         self.assertEqual('updated name', updated['name'])
         self.assertEqual('updated description', updated['description'])
         self.assertFalse(updated['enabled'])
+
+        # Now we update domain by name with name_or_id
+        updated = self.operator_cloud.update_domain(
+            None,
+            name_or_id='updated name',
+            name='updated name 2',
+            description='updated description 2',
+            enabled=True)
+        self.assertEqual('updated name 2', updated['name'])
+        self.assertEqual('updated description 2', updated['description'])
+        self.assertTrue(updated['enabled'])
+
+    def test_delete_domain(self):
+        domain = self.operator_cloud.create_domain(self.domain_prefix,
+                                                   'description')
+        self.assertEqual(self.domain_prefix, domain['name'])
+        self.assertEqual('description', domain['description'])
+        self.assertTrue(domain['enabled'])
+        deleted = self.operator_cloud.delete_domain(domain['id'])
+        self.assertTrue(deleted)
+
+        # Now we delete domain by name with name_or_id
+        domain = self.operator_cloud.create_domain(
+            self.domain_prefix, 'description')
+        self.assertEqual(self.domain_prefix, domain['name'])
+        self.assertEqual('description', domain['description'])
+        self.assertTrue(domain['enabled'])
+        deleted = self.operator_cloud.delete_domain(None, domain['name'])
+        self.assertTrue(deleted)
+
+        # Finally, we assert we get False from delete_domain if domain does
+        # not exist
+        domain = self.operator_cloud.create_domain(
+            self.domain_prefix, 'description')
+        self.assertEqual(self.domain_prefix, domain['name'])
+        self.assertEqual('description', domain['description'])
+        self.assertTrue(domain['enabled'])
+        deleted = self.operator_cloud.delete_domain(None, 'bogus_domain')
+        self.assertFalse(deleted)

shade/tests/unit/test_domains.py

@@ -49,6 +49,13 @@ class TestDomains(base.TestCase):
         self.assertTrue(mock_keystone.domains.get.called)
         self.assertEqual(domain['name'], 'a-domain')
 
+    @mock.patch.object(shade._utils, '_get_entity')
+    @mock.patch.object(shade.OpenStackCloud, 'keystone_client')
+    def test_get_domain_with_name_or_id(self, mock_keystone, mock_get):
+        self.cloud.get_domain(name_or_id='1234')
+        mock_get.assert_called_once_with(mock.ANY,
+                                         None, '1234')
+
     @mock.patch.object(shade._utils, 'normalize_domains')
     @mock.patch.object(shade.OpenStackCloud, 'keystone_client')
     def test_create_domain(self, mock_keystone, mock_normalize):
@@ -78,6 +85,15 @@ class TestDomains(base.TestCase):
         mock_keystone.domains.delete.assert_called_once_with(
             domain='update_domain_id')
 
+    @mock.patch.object(shade.OperatorCloud, 'get_domain')
+    @mock.patch.object(shade.OpenStackCloud, 'keystone_client')
+    def test_delete_domain_name_or_id(self, mock_keystone, mock_get):
+        self.cloud.update_domain(name_or_id='a-domain',
+                                 name='new name',
+                                 description='new description',
+                                 enabled=False)
+        mock_get.assert_called_once_with(None, 'a-domain')
+
     @mock.patch.object(shade.OperatorCloud, 'update_domain')
     @mock.patch.object(shade.OpenStackCloud, 'keystone_client')
     def test_delete_domain_exception(self, mock_keystone, mock_update):
@@ -102,6 +118,15 @@ class TestDomains(base.TestCase):
         mock_normalize.assert_called_once_with(
             [meta.obj_to_dict(domain_obj)])
 
+    @mock.patch.object(shade.OperatorCloud, 'get_domain')
+    @mock.patch.object(shade.OpenStackCloud, 'keystone_client')
+    def test_update_domain_name_or_id(self, mock_keystone, mock_get):
+        self.cloud.update_domain(name_or_id='a-domain',
+                                 name='new name',
+                                 description='new description',
+                                 enabled=False)
+        mock_get.assert_called_once_with(None, 'a-domain')
+
     @mock.patch.object(shade.OpenStackCloud, 'keystone_client')
     def test_update_domain_exception(self, mock_keystone):
         mock_keystone.domains.update.side_effect = Exception()