server: fix duplicate tag exception as CloudRuntimeException (apache#3348)

See apache#3339: a runtime exception is thrown but it should be converted to an error return. Wrapping it in a CloudRuntimeException should do the trick.

Fixes apache#3339

Author: DaanHoogland

server/src/com/cloud/tags/TaggedResourceManagerImpl.java

@@ -85,6 +85,7 @@
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
+import javax.persistence.EntityExistsException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -310,7 +311,11 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                         }
 
                         ResourceTagVO resourceTag = new ResourceTagVO(key, value, accountDomainPair.first(), accountDomainPair.second(), id, resourceType, customer, resourceUuid);
-                        resourceTag = _resourceTagDao.persist(resourceTag);
+                        try {
+                            resourceTag = _resourceTagDao.persist(resourceTag);
+                        } catch (EntityExistsException e) {
+                            throw new CloudRuntimeException(String.format("tag %s already on %s with id %s", resourceTag.getKey(), resourceType.toString(), resourceId),e);
+                        }
                         resourceTags.add(resourceTag);
                     }
                 }

test/integration/component/test_tags.py

@@ -2974,3 +2974,83 @@ def test_32_user_a_doesnt_have_access_to_user_b_tags(self):
             self.fail("User1 has access to create tags for User2.")
 
         return
+
+    @attr(tags=["advanced", "basic"], required_hardware="false")
+    def test_33_duplicate_vm_tag(self):
+        """
+         Test creation of a duplicate tag on UserVM and verify error return.
+         cleanup by deleting
+        """
+        # Validate the following
+        # 1. Create  a tag on VM using createTags API
+        # 2. Create the same tag on VM using createTags API
+        # 3. check the return for the right error message
+
+        tag_key = 'scope'
+        tag_value = 'test_33_duplicate_vm_tag'
+
+        self.debug("Creating a tag for user VM")
+        # use vm_2 as vm_1 is deleted in other tests :(
+        tag = Tag.create(
+            self.apiclient,
+            resourceIds=self.vm_2.id,
+            resourceType='userVM',
+            tags={tag_key: tag_value}
+        )
+        self.debug("Tag created: %s" % tag.__dict__)
+
+        self.debug("Trying second tag witgh the same key for user VM")
+        try:
+            erronousTag = Tag.create(
+                self.apiclient,
+                resourceIds=self.vm_2.id,
+                resourceType='userVM',
+                tags={tag_key: tag_value}
+            )
+        except Exception as e:
+            # verify e.message
+            assert "tag scope already on UserVm with id" in e.message, \
+                "neat error message missing from error result"
+            pass
+
+
+        # we should still find the tag
+        vms = VirtualMachine.list(
+            self.apiclient,
+            listall=True,
+            key=tag_key,
+            value=tag_value
+        )
+
+        self.assertEqual(
+            isinstance(vms, list),
+            True,
+            "Tag based VMs listing failed")
+
+        self.debug("Deleting the created tag..")
+        try:
+            Tag.delete(
+                self.apiclient,
+                resourceIds=self.vm_2.id,
+                resourceType='userVM',
+                tags={tag_key: tag_value}
+            )
+        except Exception as e:
+            self.fail("Failed to delete the tag - %s" % e)
+
+        self.debug("Verifying if tag is actually deleted!")
+        tags = Tag.list(
+            self.apiclient,
+            listall=True,
+            resourceType='userVM',
+            account=self.account.name,
+            domainid=self.account.domainid,
+            key=tag_key,
+            value=tag_value
+        )
+        self.assertEqual(
+            tags,
+            None,
+            "List tags should return empty response"
+        )
+        return