Moti Asayag has uploaded a new change for review.

Change subject: engine: Display network must have boot protocol
......................................................................

engine: Display network must have boot protocol

The scheduler should not select a host which the
display network is configured on without a proper
settings for IP address (i.e. only DHCP or Static
boot protocol are allowed).

Change-Id: I14e5a0edae8524a7334609e58934880a722ca87f
Bug-Url: https://bugzilla.redhat.com/955429
Signed-off-by: Moti Asayag <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
M 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java
M 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
M 
frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
M 
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
7 files changed, 104 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/28/25528/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
index 5cbd09f..dd9788c 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
@@ -1,15 +1,19 @@
 package org.ovirt.engine.core.bll.scheduling.policyunits;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.commons.lang.StringUtils;
+import org.ovirt.engine.core.bll.ValidationResult;
 import org.ovirt.engine.core.bll.scheduling.PolicyUnitImpl;
 import org.ovirt.engine.core.common.businessentities.Entities;
 import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.network.Network;
+import 
org.ovirt.engine.core.common.businessentities.network.NetworkBootProtocol;
+import 
org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface;
 import 
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
@@ -20,51 +24,44 @@
 import org.ovirt.engine.core.dao.network.InterfaceDao;
 import org.ovirt.engine.core.dao.network.NetworkDao;
 import org.ovirt.engine.core.dao.network.VmNetworkInterfaceDao;
+import org.ovirt.engine.core.utils.NetworkUtils;
 
 public class NetworkPolicyUnit extends PolicyUnitImpl {
     public NetworkPolicyUnit(PolicyUnit policyUnit) {
         super(policyUnit);
     }
 
-    private boolean displayNetworkInitialized;
-    private Network displayNetwork;
-
     @Override
     public List<VDS> filter(List<VDS> hosts, VM vm, Map<String, String> 
parameters, List<String> messages) {
-        List<VDS> toRemoveHostList = new ArrayList<VDS>();
-        List<VmNetworkInterface> vmNICs;
-        List<Network> clusterNetworks;
-        Map<String, Network> networksByName;
-        Guid clusterId = null;
-        if (hosts != null && hosts.size() > 0) {
-            clusterId = hosts.get(0).getVdsGroupId();
-            clusterNetworks = getNetworkDAO().getAllForCluster(clusterId);
-            networksByName = Entities.entitiesByName(clusterNetworks);
-            vmNICs = getVmNetworkInterfaceDao().getAllForVm(vm.getId());
-        } else {
+        if (hosts == null || hosts.isEmpty()) {
             return null;
         }
-        Map<Guid, List<String>> hostNics =
-                getInterfaceDAO().getHostNetworksByCluster(clusterId);
+
+        List<VDS> toRemoveHostList = new ArrayList<VDS>();
+        List<VmNetworkInterface> vmNICs = 
getVmNetworkInterfaceDao().getAllForVm(vm.getId());
+        Guid clusterId = hosts.get(0).getVdsGroupId();
+        List<Network> clusterNetworks = 
getNetworkDAO().getAllForCluster(clusterId);
+        Map<String, Network> networksByName = 
Entities.entitiesByName(clusterNetworks);
+        Map<Guid, List<String>> hostNics = 
getInterfaceDAO().getHostNetworksByCluster(clusterId);
+        Network displayNetwork = 
NetworkUtils.getDisplayNetwork(clusterNetworks);
+        Map<Guid, VdsNetworkInterface> hostDisplayNics = 
getDisplayNics(displayNetwork);
+
         for (VDS host : hosts) {
-            VdcBllMessages returnValue =
+            ValidationResult result =
                     validateRequiredNetworksAvailable(host,
                             vm,
                             vmNICs,
-                            clusterNetworks,
+                            displayNetwork,
                             networksByName,
-                            hostNics.get(host.getId()));
-            if (VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_NETWORKS == 
returnValue) {
-                StringBuilder sbBuilder = new StringBuilder();
-                
sbBuilder.append(Entities.vmInterfacesByNetworkName(vmNICs).keySet());
-                log.debugFormat("host {0} is missing networks required by VM 
nics {1}",
-                        host.getName(),
-                        sbBuilder.toString());
-            } else if 
(VdcBllMessages.ACTION_TYPE_FAILED_MISSING_DISPLAY_NETWORK == returnValue) {
-                log.debugFormat("host {0} is missing the cluster's display 
network", host.getName());
-            }
-            if (returnValue != null) {
-                messages.add(returnValue.toString());
+                            hostNics.get(host.getId()),
+                            hostDisplayNics.get(host.getId()));
+
+            if (!result.isValid()) {
+                messages.add(result.getMessage().name());
+                if (result.getVariableReplacements() != null) {
+                    messages.addAll(result.getVariableReplacements());
+                }
+
                 toRemoveHostList.add(host);
             }
         }
@@ -72,32 +69,44 @@
         return hosts;
     }
 
+    public Map<Guid, VdsNetworkInterface> getDisplayNics(Network 
displayNetwork) {
+        Map<Guid, VdsNetworkInterface> displayNics = new HashMap<>();
+        if (displayNetwork != null) {
+            List<VdsNetworkInterface> nics = 
getInterfaceDAO().getVdsInterfacesByNetworkId(displayNetwork.getId());
+            for (VdsNetworkInterface nic : nics) {
+                displayNics.put(nic.getVdsId(), nic);
+            }
+        }
+
+        return displayNics;
+    }
+
     /**
      * Determine whether all required Networks are attached to the Host's 
Nics. A required Network, depending on
      * ConfigValue.OnlyRequiredNetworksMandatoryForVdsSelection, is defined 
as: 1. false: any network that is defined on
      * an Active vNic of the VM or the cluster's display network. 2. true: a 
Cluster-Required Network that is defined on
      * an Active vNic of the VM.
+     *
      * @param vds
-     *          the Host
+     *            the Host
      * @param vm
-     *          the VM
+     *            the VM
      * @param vmNICs
-     * @param clusterNetworks
+     * @param displayNetwork
      * @param networksByName
      * @param hostNetworks
-     *          the Host network names
-     * @return <code>VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_NETWORKS</code> 
if a required network on an active vnic is
-     *         not attached to the host.<br>
-     *         
<code>VdcBllMessages.ACTION_TYPE_FAILED_MISSING_DISPLAY_NETWORK</code> if the 
cluster's display network
-     *         is required and not attached to the host.<br>
-     *         Otherwise, <code>null</code>.
+     *            the Host network names
+     * @param displayNic
+     *            the interface on top the display network is configured
+     * @return the result of network compatibility check
      */
-    private VdcBllMessages validateRequiredNetworksAvailable(VDS vds,
+    private ValidationResult validateRequiredNetworksAvailable(VDS vds,
             VM vm,
             List<VmNetworkInterface> vmNICs,
-            List<Network> clusterNetworks,
+            Network displayNetwork,
             Map<String, Network> networksByName,
-            List<String> hostNetworks) {
+            List<String> hostNetworks,
+            VdsNetworkInterface displayNic) {
 
         boolean onlyRequiredNetworks =
                 Config.<Boolean> 
getValue(ConfigValues.OnlyRequiredNetworksMandatoryForVdsSelection);
@@ -116,15 +125,16 @@
                 }
             }
             if (!found) {
-                return VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_NETWORKS;
+                StringBuilder sbBuilder = new StringBuilder();
+                
sbBuilder.append(Entities.vmInterfacesByNetworkName(vmNICs).keySet());
+                log.debugFormat("host {0} is missing networks required by VM 
nics {1}",
+                        vds.getName(),
+                        sbBuilder.toString());
+                return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_NETWORKS);
             }
         }
 
-        if (!isDisplayNetworkAvailable(vds, onlyRequiredNetworks, 
hostNetworks, clusterNetworks)) {
-            return VdcBllMessages.ACTION_TYPE_FAILED_MISSING_DISPLAY_NETWORK;
-        }
-
-        return null;
+        return validateDisplayNetworkAvailability(vds, onlyRequiredNetworks, 
displayNic, displayNetwork);
     }
 
     private boolean networkRequiredOnVds(VmNetworkInterface vmIface,
@@ -149,48 +159,40 @@
      *            the host
      * @param onlyRequiredNetworks
      *            should be false, in order the method to be non-trivial.
-     * @param hostNetworks
-     *            list of hosts networks names
-     * @param allNetworksInCluster
-     * @return <c>true</c> if the cluster's display network is defined on the 
host or
-     *         ConfigValue.OnlyRequiredNetworksMandatoryForVdsSelection is 
true; otherwise, <c>false</c>.
+     * @param displayNic
+     *            the interface on top the display network is configured
+     * @param displayNetwork
+     *            the cluster's display network
+     * @return the result of the display network validity check on the given 
host
      */
-    private boolean isDisplayNetworkAvailable(VDS host,
+    private ValidationResult validateDisplayNetworkAvailability(VDS host,
             boolean onlyRequiredNetworks,
-            List<String> hostNetworks,
-            List<Network> allNetworksInCluster) {
+            VdsNetworkInterface displayNic,
+            Network displayNetwork) {
         if (onlyRequiredNetworks) {
-            return true;
-        }
-
-        if (!displayNetworkInitialized) {
-            resolveClusterDisplayNetwork(host, allNetworksInCluster);
+            return ValidationResult.VALID;
         }
 
         if (displayNetwork == null) {
-            return true;
+            return ValidationResult.VALID;
         }
 
-        // Check if display network attached to host
-        for (String networkName : hostNetworks) {
-            if (displayNetwork.getName().equals(networkName)) {
-                return true;
-            }
+        // Check if display network attached to host and has a proper boot 
protocol
+        if (displayNic == null) {
+            log.debugFormat("host {0} is missing the cluster's display 
network", host.getName());
+            return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_MISSING_DISPLAY_NETWORK);
         }
 
-        return false;
-    }
-
-    private void resolveClusterDisplayNetwork(VDS host, List<Network> 
allNetworksInCluster) {
-        // Find the cluster's display network
-        for (Network tempNetwork : allNetworksInCluster) {
-            if (tempNetwork.getCluster().isDisplay()) {
-                displayNetwork = tempNetwork;
-                break;
-            }
+        if (displayNic.getBootProtocol() == NetworkBootProtocol.NONE) {
+            log.debugFormat("Host {0} has the display network {1} configured 
with improper boot protocol on interface {2}.",
+                    host.getName(),
+                    displayNetwork.getName(),
+                    displayNic.getName());
+            return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISPLAY_NETWORK_HAS_NO_BOOT_PROTOCOL,
+                    String.format("$DisplayNetwork %s", 
displayNetwork.getName()));
         }
 
-        displayNetworkInitialized = true;
+        return ValidationResult.VALID;
     }
 
     private VmNetworkInterfaceDao getVmNetworkInterfaceDao() {
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
index 310e13d..30f1da5 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
@@ -498,6 +498,7 @@
     EXTERNAL_NETWORK_CANNOT_BE_PROVISIONED(ErrorType.NOT_SUPPORTED),
     NETWORK_LABEL_FORMAT_INVALID(ErrorType.BAD_PARAMETERS),
     
ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC(ErrorType.CONFLICT),
+    
ACTION_TYPE_FAILED_DISPLAY_NETWORK_HAS_NO_BOOT_PROTOCOL(ErrorType.BAD_PARAMETERS),
     IMPROPER_INTERFACE_IS_LABELED(ErrorType.BAD_PARAMETERS),
     IMPROPER_BOND_IS_LABELED(ErrorType.BAD_PARAMETERS),
     INTERFACE_ALREADY_LABELED(ErrorType.CONFLICT),
diff --git 
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties 
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
index 2f60a9f..421eb18 100644
--- 
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++ 
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -489,6 +489,7 @@
 NETWORK_LABEL_FORMAT_INVALID=Network label must be formed only from: English 
letters, numbers, hyphen or underscore.
 ACTION_TYPE_FAILED_NETWORK_ALREADY_LABELED=Cannot ${action} ${type}. The 
specified network is already labeled.
 ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC=Cannot ${action} 
${type}. The following networks cannot be removed from the network interface 
since they are managed by the label: 
${ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC_LIST}. Please 
remove the label from the network interface in order to remove the network.
+ACTION_TYPE_FAILED_DISPLAY_NETWORK_HAS_NO_BOOT_PROTOCOL=Cannot ${action} 
${type}. The display network ${DisplayNetwork} must have a DHCP or Static boot 
protocol when conifgured on a host.
 LABELED_NETWORK_ATTACHED_TO_WRONG_INTERFACE=Cannot ${action} ${type}. The 
following networks are already attached to a different interface on the host: 
${AssignedNetworks}. Please remove the networks in order to label the interface.
 OTHER_INTERFACE_ALREADY_LABELED=Cannot ${action} ${type}. The label is already 
defined on other interface ${LabeledNic} on the host.
 ERROR_CANNOT_RECOVERY_STORAGE_POOL_THERE_IS_ACTIVE_DATA_DOMAINS=Cannot recover 
Data Center with active Data Storage Domain in Data Center.
diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java
index e3fdff0..ef75c6b 100644
--- 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java
@@ -1,6 +1,7 @@
 package org.ovirt.engine.core.utils;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -281,4 +282,20 @@
     public static String getVlanDeviceName(VdsNetworkInterface underlyingNic, 
Network network) {
         return underlyingNic.getName() + "." + network.getVlanId();
     }
+
+    /**
+     * Returns the cluster's display network
+     */
+    public static Network getDisplayNetwork(Collection<Network> 
clusterNetworks) {
+        Network displayNetwork = null;
+
+        for (Network network : clusterNetworks) {
+            if (network.getCluster().isDisplay()) {
+                displayNetwork = network;
+                break;
+            }
+        }
+
+        return displayNetwork;
+    }
 }
diff --git 
a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
 
b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
index 6de8d0a..378905f 100644
--- 
a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
+++ 
b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
@@ -1357,6 +1357,9 @@
     @DefaultStringValue("Cannot ${action} ${type}. The following networks 
cannot be removed from the network interface since they are managed by the 
label: ${ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC_LIST}. 
Please remove the label from the network interface in order to remove the 
network.")
     String ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC();
 
+    @DefaultStringValue("Cannot ${action} ${type}. The display network 
${DisplayNetwork} must have a DHCP or Static boot protocol when conifgured on a 
host.")
+    String ACTION_TYPE_FAILED_DISPLAY_NETWORK_HAS_NO_BOOT_PROTOCOL();
+
     @DefaultStringValue("Cannot ${action} ${type}. The following networks are 
already attached to a different interface: ${AssignedNetworks}. Please remove 
the networks in order to label the interface.")
     String LABELED_NETWORK_ATTACHED_TO_WRONG_INTERFACE();
 
diff --git 
a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
 
b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
index f9b8ab2..5fd729e 100644
--- 
a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
+++ 
b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -468,6 +468,7 @@
 NETWORK_LABEL_FORMAT_INVALID=Network label must be formed only from: English 
letters, numbers, hyphen or underscore.
 ACTION_TYPE_FAILED_NETWORK_ALREADY_LABELED=Cannot ${action} ${type}. The 
specified network is already labeled.
 ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC=Cannot ${action} 
${type}. The following networks cannot be removed from the network interface 
since they are managed by the label: 
${ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC_LIST}. Please 
remove the label from the network interface in order to remove the network.
+ACTION_TYPE_FAILED_DISPLAY_NETWORK_HAS_NO_BOOT_PROTOCOL=Cannot ${action} 
${type}. Please contact your system administrator.
 LABELED_NETWORK_ATTACHED_TO_WRONG_INTERFACE=Cannot ${action} ${type}. The 
following networks are already attached to a different interface: 
${AssignedNetworks}. Please remove the networks in order to label the interface.
 OTHER_INTERFACE_ALREADY_LABELED=Cannot ${action} ${type}. The label is already 
defined on other interface ${LabeledNic} on the host.
 ERROR_CANNOT_RECOVERY_STORAGE_POOL_THERE_IS_ACTIVE_DATA_DOMAINS=Cannot recover 
Data Center with active Data Storage Domain in Data Center.
diff --git 
a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
 
b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
index 352e9a2..90be940 100644
--- 
a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
+++ 
b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -493,6 +493,7 @@
 NETWORK_LABEL_FORMAT_INVALID=Network label must be formed only from: English 
letters, numbers, hyphen or underscore.
 ACTION_TYPE_FAILED_NETWORK_ALREADY_LABELED=Cannot ${action} ${type}. The 
specified network is already labeled.
 ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC=Cannot ${action} 
${type}. The following networks cannot be removed from the network interface 
since they are managed by the label: 
${ACTION_TYPE_FAILED_CANNOT_REMOVE_LABELED_NETWORK_FROM_NIC_LIST}. Please 
remove the label from the network interface in order to remove the network.
+ACTION_TYPE_FAILED_DISPLAY_NETWORK_HAS_NO_BOOT_PROTOCOL=Cannot ${action} 
${type}. The display network ${DisplayNetwork} must have a DHCP or Static boot 
protocol when conifgured on a host.
 LABELED_NETWORK_ATTACHED_TO_WRONG_INTERFACE=Cannot ${action} ${type}. The 
following networks are already attached to a different interface: 
${AssignedNetworks}. Please remove the networks in order to label the interface.
 OTHER_INTERFACE_ALREADY_LABELED=Cannot ${action} ${type}. The label is already 
defined on other interface ${LabeledNic} on the host.
 ERROR_CANNOT_RECOVERY_STORAGE_POOL_THERE_IS_ACTIVE_DATA_DOMAINS=Cannot recover 
Data Center with active Data Storage Domain in Data Center.


-- 
To view, visit http://gerrit.ovirt.org/25528
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I14e5a0edae8524a7334609e58934880a722ca87f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Moti Asayag <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to