[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6845: [WIP] plugins: Add non strict affinity groups

[GitBox]

DaanHoogland commented on code in PR #6845:
URL: https://github.com/apache/cloudstack/pull/6845#discussion_r1006757930


##########
api/src/main/java/com/cloud/deploy/DataCenterDeployment.java:
##########
@@ -124,4 +127,30 @@ public String toString() {
                 "migrationPlan");
     }
 
+    @Override
+    public void addHostPriority(Long hostId, HostPriority priority) {
+        Integer currentPriority = hostPriorities.get(hostId);
+        if (currentPriority == null) {
+            currentPriority = DEFAULT_HOST_PRIORITY;
+        }
+        if (HostPriority.HIGH.equals(priority)) {
+            hostPriorities.put(hostId, currentPriority + 1);

Review Comment:
   same here actually: if the currentPriority equals LOW and HIGH is requested 
it becomes DEFAULT_HOST_PRIORITY?



##########
api/src/main/java/com/cloud/deploy/DataCenterDeployment.java:
##########
@@ -124,4 +127,30 @@ public String toString() {
                 "migrationPlan");
     }
 
+    @Override
+    public void addHostPriority(Long hostId, HostPriority priority) {
+        Integer currentPriority = hostPriorities.get(hostId);
+        if (currentPriority == null) {
+            currentPriority = DEFAULT_HOST_PRIORITY;
+        }
+        if (HostPriority.HIGH.equals(priority)) {
+            hostPriorities.put(hostId, currentPriority + 1);
+        } else if (HostPriority.LOW.equals(priority)) {
+            hostPriorities.put(hostId, currentPriority - 1);

Review Comment:
   so if the currentPriority equals HIGH and LOW is requested it becomes 
DEFAULT_HOST_PRIORITY?



##########
api/src/main/java/com/cloud/deploy/DeploymentPlan.java:
##########
@@ -20,10 +20,21 @@
 import com.cloud.vm.ReservationContext;
 
 import java.util.List;
+import java.util.Map;
 
 /**
  */
 public interface DeploymentPlan {
+
+    Integer DEFAULT_HOST_PRIORITY = 0;
+
+    enum HostPriority {
+        HIGH,
+        DEFAULT,
+        LOW,
+        PROHIBITED
+    }

Review Comment:
   aren't ordinals to be specified here?
   ```suggestion
       enum HostPriority {
           HIGH(-1),
           DEFAULT(0),
           LOW(1),
           PROHIBITED(Integer.MIN_VALUE)
       }
   ```
   I think it will not work out of the box like this but without it i think it 
will fail as well. you'd need to add a field for the value and implement some 
fromInt or fromString methods



-- 
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.

To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org