Merge remote-tracking branch 'apache/4.20' into 4.20-fix-vnf-deploy-as-is

This reverts commit 0bfc65aa00.

api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java

@@ -43,7 +43,7 @@ import java.util.List;
 public class DeployVnfApplianceCmd extends DeployVMCmd implements UserCmd {
 
     @Parameter(name = ApiConstants.VNF_CONFIGURE_MANAGEMENT, type = CommandType.BOOLEAN, required = false,
-            description = "True by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. False otherwise. " +
+            description = "False by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. True otherwise. " +
                     "Network rules are configured if management network is an isolated network or shared network with security groups.")
     private Boolean vnfConfigureManagement;
 

api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManager.java

@@ -43,9 +43,7 @@ public interface VnfTemplateManager {
 
     void updateVnfTemplate(long templateId, UpdateVnfTemplateCmd cmd);
 
-    void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds);
-
-    void validateVnfApplianceNetworksMap(VirtualMachineTemplate template, Map<Integer, Long> vmNetworkMap);
+    void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds, Map<Integer, Long> vmNetworkMap);
 
     SecurityGroup createSecurityGroupForVnfAppliance(DataCenter zone, VirtualMachineTemplate template, Account owner, DeployVnfApplianceCmd cmd);
 

api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateUtils.java

@@ -126,7 +126,7 @@ public class VnfTemplateUtils {
         if (cmd instanceof RegisterVnfTemplateCmd) {
             RegisterVnfTemplateCmd registerCmd = (RegisterVnfTemplateCmd) cmd;
             if (registerCmd.isDeployAsIs() && CollectionUtils.isNotEmpty(registerCmd.getVnfNics())) {
-                throw new InvalidParameterValueException("VNF Template cannot be registered with VNF nics as Template settings are read from OVA.");
+                throw new InvalidParameterValueException("VNF nics cannot be specified when register a deploy-as-is Template. Please wait until Template settings are read from OVA.");
             }
             validateApiCommandParams(registerCmd.getVnfDetails(), registerCmd.getVnfNics(), registerCmd.getTemplateType());
         } else if (cmd instanceof UpdateVnfTemplateCmd) {

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -6127,11 +6127,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             throw new InvalidParameterValueException("Unable to use template " + templateId);
         }
         if (TemplateType.VNF.equals(template.getTemplateType())) {
-            if (template.isDeployAsIs()) {
-                vnfTemplateManager.validateVnfApplianceNetworksMap(template, cmd.getVmNetworkMap());
-            } else {
-                vnfTemplateManager.validateVnfApplianceNics(template, cmd.getNetworkIds());
-            }
+            vnfTemplateManager.validateVnfApplianceNics(template, cmd.getNetworkIds(), cmd.getVmNetworkMap());
         } else if (cmd instanceof DeployVnfApplianceCmd) {
             throw new InvalidParameterValueException("Can't deploy VNF appliance from a non-VNF template");
         }

server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java

@@ -201,13 +201,17 @@ public class VnfTemplateManagerImpl extends ManagerBase implements VnfTemplateMa
     }
 
     @Override
-    public void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds) {
+    public void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds, Map<Integer, Long> vmNetworkMap) {
+        if (template.isDeployAsIs()) {
+            if (CollectionUtils.isNotEmpty(networkIds)) {
+                throw new InvalidParameterValueException("VNF nics mappings should be empty for deploy-as-is templates");
+            }
+            validateVnfApplianceNetworksMap(template, vmNetworkMap);
+            return;
+        }
         if (CollectionUtils.isEmpty(networkIds)) {
             throw new InvalidParameterValueException("VNF nics list is empty");
         }
-        if (CollectionUtils.isNotEmpty(networkIds) && template.isDeployAsIs()) {
-            throw new InvalidParameterValueException("VNF nics list should be empty for deploy-as-is templates");
-        }
         List<VnfTemplateNicVO> vnfNics = vnfTemplateNicDao.listByTemplateId(template.getId());
         for (VnfTemplateNicVO vnfNic : vnfNics) {
             if (vnfNic.isRequired() && networkIds.size() <= vnfNic.getDeviceId()) {
@@ -216,8 +220,7 @@ public class VnfTemplateManagerImpl extends ManagerBase implements VnfTemplateMa
         }
     }
 
-    @Override
-    public void validateVnfApplianceNetworksMap(VirtualMachineTemplate template, Map<Integer, Long> vmNetworkMap) {
+    private void validateVnfApplianceNetworksMap(VirtualMachineTemplate template, Map<Integer, Long> vmNetworkMap) {
         if (MapUtils.isEmpty(vmNetworkMap)) {
             throw new InvalidParameterValueException("VNF networks map is empty");
         }

ui/public/locales/en.json

@@ -2527,8 +2527,8 @@
 "label.vnf.app.action.reinstall": "Reinstall VNF Appliance",
 "label.vnf.cidr.list": "CIDR from which access to the VNF appliance's Management interface should be allowed from",
 "label.vnf.cidr.list.tooltip": "the CIDR list to forward traffic from to the VNF management interface. Multiple entries must be separated by a single comma character (,). The default value is 0.0.0.0/0.",
-"label.vnf.configure.management": "Configure Firewall and Port Forwarding rules for VNF's management interfaces",
-"label.vnf.configure.management.tooltip": "True by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. False otherwise. Learn what rules are configured at http://docs.cloudstack.apache.org/en/latest/adminguide/networking/vnf_templates_appliances.html#deploying-vnf-appliances",
+"label.vnf.configure.management": "Configure network rules for VNF's management interfaces",
+"label.vnf.configure.management.tooltip": "False by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. True otherwise. Learn what rules are configured at http://docs.cloudstack.apache.org/en/latest/adminguide/networking/vnf_templates_appliances.html#deploying-vnf-appliances",
 "label.vnf.detail.add": "Add VNF detail",
 "label.vnf.detail.remove": "Remove VNF detail",
 "label.vnf.details": "VNF Details",

ui/src/config/section/network.js

@@ -356,7 +356,10 @@ export default {
       permission: ['listVnfAppliances'],
       resourceType: 'UserVm',
       params: () => {
-        return { details: 'servoff,tmpl,nics', isvnf: true }
+        return {
+          details: 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp,backoff,vnfnics',
+          isvnf: true
+        }
       },
       columns: () => {
         const fields = ['name', 'state', 'ipaddress']

ui/src/views/compute/DeployVnfAppliance.vue

@@ -1312,7 +1312,7 @@ export default {
         }
         if (this.vnfNicNetworks && this.vnfNicNetworks[deviceId] &&
           ((this.vnfNicNetworks[deviceId].type === 'Isolated' && this.vnfNicNetworks[deviceId].vpcid === undefined) ||
-            (this.vnfNicNetworks[deviceId].type === 'Shared' && this.zone.securitygroupsenabled))) {
+            (this.vnfNicNetworks[deviceId].type === 'Shared' && this.vnfNicNetworks[deviceId].service.filter(svc => svc.name === 'SecurityGroupProvider')))) {
           return true
         }
       }

ui/src/views/compute/wizard/VnfNicsSelection.vue

@@ -50,7 +50,7 @@
       <template #network="{ record }">
         <a-form-item style="display: block" :name="'nic-' + record.deviceid">
           <a-select
-            disabled="templateNics || templateNics.length === 0"
+            disabled="templateNics && templateNics.length > 0"
             @change="updateNicNetworkValue($event, record.deviceid)"
             optionFilterProp="label"
             :filterOption="(input, option) => {

ui/src/views/network/VnfAppliancesTab.vue

@@ -120,7 +120,7 @@ export default {
   methods: {
     fetchData () {
       var params = {
-        details: 'servoff,tmpl,nics',
+        details: 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp,backoff,vnfnics',
         isVnf: true,
         listAll: true
       }

utils/src/main/java/com/cloud/utils/script/Script.java

@@ -40,9 +40,11 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -708,13 +710,31 @@ public class Script implements Callable<String> {
         return executeCommandForExitValue(0, command);
     }
 
+    private static void cleanupProcesses(AtomicReference<List<Process>> processesRef) {
+        List<Process> processes = processesRef.get();
+        if (CollectionUtils.isNotEmpty(processes)) {
+            for (Process process : processes) {
+                if (process == null) {
+                    continue;
+                }
+                LOGGER.trace(String.format("Cleaning up process [%s] from piped commands.", process.pid()));
+                IOUtils.closeQuietly(process.getErrorStream());
+                IOUtils.closeQuietly(process.getOutputStream());
+                IOUtils.closeQuietly(process.getInputStream());
+                process.destroyForcibly();
+            }
+        }
+    }
+
     public static Pair<Integer, String> executePipedCommands(List<String[]> commands, long timeout) {
         if (timeout <= 0) {
             timeout = DEFAULT_TIMEOUT;
         }
+        final AtomicReference<List<Process>> processesRef = new AtomicReference<>();
         Callable<Pair<Integer, String>> commandRunner = () -> {
             List<ProcessBuilder> builders = commands.stream().map(ProcessBuilder::new).collect(Collectors.toList());
             List<Process> processes = ProcessBuilder.startPipeline(builders);
+            processesRef.set(processes);
             Process last = processes.get(processes.size()-1);
             try (BufferedReader reader = new BufferedReader(new InputStreamReader(last.getInputStream()))) {
                 String line;
@@ -741,6 +761,8 @@ public class Script implements Callable<String> {
             result.second(ERR_TIMEOUT);
         } catch (InterruptedException | ExecutionException e) {
             LOGGER.error("Error executing piped commands", e);
+        } finally {
+            cleanupProcesses(processesRef);
         }
         return result;
     }