You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/01/09 08:27:00 UTC

[GitHub] yvsubhash closed pull request #905: BUG-ID: CLOUDSTACK-8922: Unable to delete IP tag

yvsubhash closed pull request #905: BUG-ID: CLOUDSTACK-8922:  Unable to delete IP tag
URL: https://github.com/apache/cloudstack/pull/905
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/.travis.yml b/.travis.yml
index b4749c05e31..b7b358c2e18 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -91,6 +91,7 @@ env:
              smoke/test_staticroles
              smoke/test_templates
              smoke/test_usage
+             smoke/test_tags
              smoke/test_usage_events
              smoke/test_vm_life_cycle
              smoke/test_vm_snapshots
@@ -144,8 +145,6 @@ env:
              component/test_stopped_vm"
 
     - TESTS="component/test_resource_limits"
-
-    - TESTS="component/test_tags
              component/test_templates
              component/test_update_vm
              component/test_volumes"
diff --git a/server/src/com/cloud/tags/TaggedResourceManagerImpl.java b/server/src/com/cloud/tags/TaggedResourceManagerImpl.java
index 7528d6861dc..752adb517cd 100644
--- a/server/src/com/cloud/tags/TaggedResourceManagerImpl.java
+++ b/server/src/com/cloud/tags/TaggedResourceManagerImpl.java
@@ -299,21 +299,9 @@ public String getUuid(String resourceId, ResourceObjectType resourceType) {
     @DB
     @ActionEvent(eventType = EventTypes.EVENT_TAGS_DELETE, eventDescription = "deleting resource tags")
     public boolean deleteTags(List<String> resourceIds, ResourceObjectType resourceType, Map<String, String> tags) {
-        Account caller = CallContext.current().getCallingAccount();
-
-        SearchBuilder<ResourceTagVO> sb = _resourceTagDao.createSearchBuilder();
-        sb.and().op("resourceId", sb.entity().getResourceId(), SearchCriteria.Op.IN);
-        sb.or("resourceUuid", sb.entity().getResourceUuid(), SearchCriteria.Op.IN);
-        sb.cp();
-        sb.and("resourceType", sb.entity().getResourceType(), SearchCriteria.Op.EQ);
-
-        SearchCriteria<ResourceTagVO> sc = sb.create();
-        sc.setParameters("resourceId", resourceIds.toArray());
-        sc.setParameters("resourceUuid", resourceIds.toArray());
-        sc.setParameters("resourceType", resourceType);
 
-        List<? extends ResourceTag> resourceTags = _resourceTagDao.search(sc, null);
-        ;
+        Account caller = CallContext.current().getCallingAccount();
+        List<? extends ResourceTag> resourceTags = getResourceTags(resourceIds, resourceType);
         final List<ResourceTag> tagsToRemove = new ArrayList<ResourceTag>();
 
         // Finalize which tags should be removed
@@ -363,6 +351,48 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
         return true;
     }
 
+    private List<? extends ResourceTag>  getResourceTags(final List<String> resourceIds, ResourceObjectType resourceType) {
+
+        List<String> uuids = new ArrayList<String>();
+        List<String> internalIds = new ArrayList<String>();
+        for(String resourceId : resourceIds){
+            if(resourceId.matches("^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$")){
+                uuids.add(resourceId);
+            }else{
+                try {
+                    Long.parseLong(resourceId);
+                } catch (final NumberFormatException e) {
+                    throw new InvalidParameterValueException("Invalid resourceId");
+                }
+                internalIds.add(resourceId);
+            }
+        }
+
+        SearchBuilder<ResourceTagVO> sb = _resourceTagDao.createSearchBuilder();
+
+        if(!uuids.isEmpty() && !internalIds.isEmpty()){
+            throw new InvalidParameterValueException("Expecting only uuids or Ids");
+        }else if (!uuids.isEmpty()){
+            sb.and("resourceUuid", sb.entity().getResourceUuid(), SearchCriteria.Op.IN);
+        }else if (!internalIds.isEmpty()){
+            sb.and("resourceId", sb.entity().getResourceId(), SearchCriteria.Op.IN);
+        }
+
+        sb.and("resourceType", sb.entity().getResourceType(), SearchCriteria.Op.EQ);
+
+        SearchCriteria<ResourceTagVO> sc = sb.create();
+
+        if (!uuids.isEmpty()){
+            sc.setParameters("resourceUuid", resourceIds.toArray());
+        }else if (!internalIds.isEmpty()){
+            sc.setParameters("resourceId", resourceIds.toArray());
+        }
+
+        sc.setParameters("resourceType", resourceType);
+        return _resourceTagDao.search(sc, null);
+
+    }
+
     @Override
     public List<? extends ResourceTag> listByResourceTypeAndId(ResourceObjectType type, long resourceId) {
         return _resourceTagDao.listBy(resourceId, type);
diff --git a/test/integration/component/test_tags.py b/test/integration/smoke/test_tags.py
similarity index 89%
rename from test/integration/component/test_tags.py
rename to test/integration/smoke/test_tags.py
index f9b0655016d..8bfebb87984 100644
--- a/test/integration/component/test_tags.py
+++ b/test/integration/smoke/test_tags.py
@@ -2543,3 +2543,258 @@ def test_24_public_IP_tag(self):
             "List tags should return empty response"
         )
         return
+    @attr(tags=["advanced"], required_hardware="false")
+    def test_25_IP_Address_tag(self):
+        """ Testcreation, adding and removing tag on public IP address
+        """
+        # Validate the following
+        # 1. Create a domain and 2 user accounts under that domain
+        # 2. Create network in both accounts and acquire public IP address
+        # 3. Create a tag on acquired public IP address using createTags API
+        # 4. Delete above created both tag using deleteTags API
+
+        self.debug("Creating a sub-domain under: %s" % self.domain.name)
+        self.child_domain = Domain.create(
+            self.apiclient,
+            services=self.services["domain"],
+            parentdomainid=self.domain.id
+        )
+        self.admin_acc_1 = Account.create(
+            self.apiclient,
+            self.services["account"],
+            admin=False,
+            domainid=self.child_domain.id
+        )
+        # Cleanup the resources created at end of test
+        self.cleanup.append(self.admin_acc_1)
+        self.dom_admin_api_client1 = self.testClient.getUserApiClient(
+            UserName=self.admin_acc_1.name,
+            DomainName=self.admin_acc_1.domain
+        )
+        result = createEnabledNetworkOffering(
+            self.apiclient,
+            self.services["network_offering"]
+        )
+        assert result[0] == PASS, \
+            "Network offering create/enable failed with error %s" % result[2]
+        self.network_offering = result[1]
+        self.network = Network.create(
+            self.dom_admin_api_client1,
+            self.services["network"],
+            networkofferingid=self.network_offering.id,
+            accountid=self.admin_acc_1.name,
+            domainid=self.admin_acc_1.domainid,
+            zoneid=self.zone.id
+        )
+        self.debug("Fetching the network details for account: %s" %
+                   self.admin_acc_1.name
+                   )
+        networks = Network.list(
+            self.dom_admin_api_client1,
+            account=self.admin_acc_1.name,
+            domainid=self.admin_acc_1.domainid,
+            listall=True
+        )
+        self.assertEqual(
+            isinstance(networks, list),
+            True,
+            "List networks should not return an empty response"
+        )
+        network = networks[0]
+        self.debug("Network for the account: %s is %s" %
+                   (self.admin_acc_1.name, network.name)
+                   )
+        self.debug("Associating public IP for network: %s" % network.id)
+        public_ip1 = PublicIPAddress.create(
+            self.dom_admin_api_client1,
+            accountid=self.admin_acc_1.name,
+            zoneid=self.zone.id,
+            domainid=self.admin_acc_1.domainid,
+            networkid=network.id
+        )
+        self.debug("Creating a tag for Public IP")
+        tag1 = Tag.create(
+            self.dom_admin_api_client1,
+            resourceIds=public_ip1.ipaddress.id,
+            resourceType='PublicIpAddress',
+            tags={'region': 'India'}
+        )
+        self.debug("Tag created: %s" % tag1.__dict__)
+
+        tags = Tag.list(
+            self.dom_admin_api_client1,
+            listall=True,
+            resourceType='PublicIpAddress',
+            account=self.admin_acc_1.name,
+            domainid=self.admin_acc_1.domainid,
+            key='region',
+            value='India'
+        )
+        self.assertEqual(
+            isinstance(tags, list),
+            True,
+            "List tags should not return empty response"
+        )
+        self.assertEqual(
+            tags[0].value,
+            'India',
+            'The tag should have original value'
+        )
+        publicIps = PublicIPAddress.list(
+            self.dom_admin_api_client1,
+            account=self.admin_acc_1.name,
+            domainid=self.admin_acc_1.domainid,
+            listall=True,
+            key='region',
+            value='India'
+        )
+        self.assertEqual(
+            isinstance(publicIps, list),
+            True,
+            "List Public IPs should not return an empty response"
+        )
+
+        # Setting up second user account.
+        self.admin_acc_2 = Account.create(
+            self.apiclient,
+            self.services["account"],
+            admin=False,
+            domainid=self.child_domain.id
+        )
+        # Cleanup the resources created at end of test
+        self.cleanup.append(self.admin_acc_2)
+        self.cleanup.append(self.child_domain)
+        self.dom_admin_api_client2 = self.testClient.getUserApiClient(
+            UserName=self.admin_acc_2.name,
+            DomainName=self.admin_acc_2.domain
+        )
+        result = createEnabledNetworkOffering(
+            self.apiclient,
+            self.services["network_offering"]
+        )
+        assert result[0] == PASS, \
+            "Network offering create/enable failed with error %s" % result[2]
+        self.network_offering = result[1]
+        self.network = Network.create(
+            self.dom_admin_api_client2,
+            self.services["network"],
+            networkofferingid=self.network_offering.id,
+            accountid=self.admin_acc_2.name,
+            domainid=self.admin_acc_2.domainid,
+            zoneid=self.zone.id
+        )
+        self.debug("Fetching the network details for account: %s" %
+                   self.admin_acc_2.name
+                   )
+        networks = Network.list(
+            self.dom_admin_api_client2,
+            account=self.admin_acc_2.name,
+            domainid=self.admin_acc_2.domainid,
+            listall=True
+        )
+        self.assertEqual(
+            isinstance(networks, list),
+            True,
+            "List networks should not return an empty response"
+        )
+        network = networks[0]
+        self.debug("Network for the account: %s is %s" %
+                   (self.admin_acc_2.name, network.name)
+                   )
+        self.debug("Associating public IP for network: %s" % network.id)
+        public_ip2 = PublicIPAddress.create(
+            self.dom_admin_api_client2,
+            accountid=self.admin_acc_2.name,
+            zoneid=self.zone.id,
+            domainid=self.admin_acc_2.domainid,
+            networkid=network.id
+        )
+        self.debug("Creating a tag for Public IP")
+        tag2 = Tag.create(
+            self.dom_admin_api_client2,
+            resourceIds=public_ip2.ipaddress.id,
+            resourceType='PublicIpAddress',
+            tags={'region': 'Pune'}
+        )
+        self.debug("Tag created: %s" % tag2.__dict__)
+
+        tags = Tag.list(
+            self.dom_admin_api_client2,
+            listall=True,
+            resourceType='PublicIpAddress',
+            account=self.admin_acc_2.name,
+            domainid=self.admin_acc_2.domainid,
+            key='region',
+            value='Pune'
+        )
+        self.assertEqual(
+            isinstance(tags, list),
+            True,
+            "List tags should not return empty response"
+        )
+        self.assertEqual(
+            tags[0].value,
+            'Pune',
+            'The tag should have original value'
+        )
+        publicIps = PublicIPAddress.list(
+            self.dom_admin_api_client2,
+            account=self.admin_acc_2.name,
+            domainid=self.admin_acc_2.domainid,
+            listall=True,
+            key='region',
+            value='Pune'
+        )
+        self.assertEqual(
+            isinstance(publicIps, list),
+            True,
+            "List Public IPs should not return an empty response"
+        )
+
+        idset = self.dbclient.execute( "SELECT resource_uuid from resource_tags WHERE `key` = 'region' AND `value` = 'India'");
+        self.assertNotEqual(
+            len(idset),
+            0,
+            "Check DB Query result set"
+        )
+        iresult = idset[0]
+        rid = iresult[0]
+
+        self.dbclient.execute( "UPDATE resource_tags SET resource_id = CAST( '%s' AS SIGNED) WHERE `key` = 'region' AND `value` = 'Pune'" % rid);
+
+        try:
+            tag1.delete(
+                self.dom_admin_api_client1,
+                resourceIds=public_ip1.ipaddress.id,
+                resourceType='PublicIpAddress',
+                tags={'region': 'India'}
+            )
+        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.dom_admin_api_client1,
+            listall=True,
+            resourceType='PublicIpAddress',
+            account=self.admin_acc_1.name,
+            domainid=self.admin_acc_1.domainid,
+            key='region',
+            value='India'
+        )
+        self.assertEqual(
+            tags,
+            None,
+            "List tags should return empty response"
+        )
+        try:
+            tag2.delete(
+                self.dom_admin_api_client2,
+                resourceIds=public_ip2.ipaddress.id,
+                resourceType='PublicIpAddress',
+                tags={'region': 'Pune'}
+            )
+        except Exception as e:
+            self.fail("Failed to delete the tag - %s" % e)
+        return
+


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services