[cloudstack] branch 4.17 updated: server: Check for null poolid (#6879)

[ro...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.17
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.17 by this push:
     new dbc20320770 server: Check for null poolid (#6879)
dbc20320770 is described below

commit dbc20320770efa5a62b67aacc226bb2ecb72105e
Author: Craig Squire <cr...@gmail.com>
AuthorDate: Wed Nov 30 01:15:35 2022 -0600

    server: Check for null poolid (#6879)
    
    Extract retrieveDatastore method
    
    Add unit test for null poolId
    
    Fixes #6878
    
    Co-authored-by: Craig Squire <cr...@ticketmaster.com>
    Co-authored-by: Stephan Krug <st...@icloud.com>
---
 .../main/java/com/cloud/tags/TaggedResourceManagerImpl.java   | 11 ++++++++++-
 .../java/com/cloud/tags/TaggedResourceManagerImplTest.java    |  7 +++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java
index c855c9efa30..60ded224a21 100644
--- a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java
+++ b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java
@@ -318,8 +318,10 @@ public class TaggedResourceManagerImpl extends ManagerBase implements TaggedReso
     private void informStoragePoolForVmTags(long vmId, String key, String value) {
         List<VolumeVO> volumeVos = volumeDao.findByInstance(vmId);
         for (VolumeVO volume : volumeVos) {
-            DataStore dataStore = dataStoreMgr.getDataStore(volume.getPoolId(), DataStoreRole.Primary);
+            Long poolId = volume.getPoolId();
+            DataStore dataStore = retrieveDatastore(poolId);
             if (dataStore == null || !(dataStore.getDriver() instanceof PrimaryDataStoreDriver)) {
+                s_logger.info(String.format("No data store found for VM %d with pool ID %d.", vmId, poolId));
                 continue;
             }
             PrimaryDataStoreDriver dataStoreDriver = (PrimaryDataStoreDriver) dataStore.getDriver();
@@ -328,4 +330,11 @@ public class TaggedResourceManagerImpl extends ManagerBase implements TaggedReso
             }
         }
     }
+
+    protected DataStore retrieveDatastore(Long poolId) {
+        if (poolId == null) {
+            return null;
+        }
+        return dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
+    }
 }
diff --git a/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java b/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java
index 6d5b314263c..d25f2feccb5 100644
--- a/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java
+++ b/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java
@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -123,4 +124,10 @@ public class TaggedResourceManagerImplTest extends TestCase {
         Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(caller, null, false, owner);
         taggedResourceManagerImplSpy.checkTagsDeletePermission(List.of(resourceTag1, resourceTag2), caller);
     }
+
+    @Test
+    public void testRetrieveDataStoreNullPoolId() {
+        DataStore dataStore = taggedResourceManagerImplSpy.retrieveDatastore(null);
+        Assert.assertNull(dataStore);
+    }
 }