[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3794: [WIP] create Volume Access Groups per cluster instead of CloudStack-RandomUUID()

[GitBox <gi...@apache.org>]

DaanHoogland commented on a change in pull request #3794: [WIP] create Volume Access Groups per cluster instead of CloudStack-RandomUUID()
URL: https://github.com/apache/cloudstack/pull/3794#discussion_r372335887
 
 

 ##########
 File path: plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
 ##########
 @@ -692,39 +699,63 @@ public static void placeVolumeInVolumeAccessGroups(SolidFireConnection sfConnect
             throw new CloudRuntimeException(errMsg);
         }
 
+        // Get list of VAGs from solidfire
         List<SolidFireUtil.SolidFireVag> sfVags = SolidFireUtil.getAllVags(sfConnection);
 
-        Map<SolidFireUtil.SolidFireVag, List<String>> sfVagToIqnsMap = new HashMap<>();
+        // Populate vag to iqn(host) map
+        Map<SolidFireUtil.SolidFireVag, List<String>> sfVagToIqnsMap = buildVagtoIQNMap(hosts, sfVags);
 
-        for (HostVO hostVO : hosts) {
-            String iqn = hostVO.getStorageUrl();
+        // throw exception if more than 4 VAGs
+        if (sfVagToIqnsMap.size() > MAX_NUM_VAGS_PER_VOLUME) {
+            throw new CloudRuntimeException("A SolidFire volume can be in at most four volume access groups simultaneously.");
+        }
 
-            SolidFireUtil.SolidFireVag sfVag = getVolumeAccessGroup(iqn, sfVags);
+        if (sfVagToIqnsMap.containsKey(null)) {
 
-            List<String> iqnsInVag = sfVagToIqnsMap.computeIfAbsent(sfVag, k -> new ArrayList<>());
+            //Check if cluster VAG exists
+            SolidFireVag sfVagMatchingClusterId = sfVags.stream().filter(vag -> vag.getName().equals("CloudStack-"+clusterUuId)).findFirst().orElse(null);
 
-            iqnsInVag.add(iqn);
-        }
+            if (sfVagMatchingClusterId == null) {
+                // Create cluster VAG if doesnt exist
+                LOGGER.info("Creating volume access group CloudStack-"+clusterUuId);
+                SolidFireUtil.createVag(sfConnection, "CloudStack-"+clusterUuId, sfVagToIqnsMap.get(null).toArray(new String[0]), new long[] { sfVolumeId });
+                sfVags = SolidFireUtil.getAllVags(sfConnection);
+                sfVagMatchingClusterId = sfVags.stream().filter(vag -> vag.getName().equals("CloudStack-"+clusterUuId)).findFirst().orElse(null);
+            }
 
-        if (sfVagToIqnsMap.size() > MAX_NUM_VAGS_PER_VOLUME) {
-            throw new CloudRuntimeException("A SolidFire volume can be in at most four volume access groups simultaneously.");
+            //update null entry in vag to IQN Map
+            sfVagToIqnsMap.put(sfVagMatchingClusterId,sfVagToIqnsMap.get(null));
+            sfVagToIqnsMap.remove(null);
         }
 
+
+        // add volumeId to each VAG if not already present
 
 Review comment:
   This comment is a good reason to factor the next bit code out for cleanup.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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