UI Improvements (#411)

pom.xml

@@ -47,7 +47,7 @@
         <gitHubRepo>jenkinsci/azure-vm-agents-plugin</gitHubRepo>
         <jenkins.version>2.361.4</jenkins.version>
         <maven.javadoc.skip>true</maven.javadoc.skip>
-        <azure-credentials.version>254.v64da_8176c83a</azure-credentials.version>
+        <hpi.compatibleSinceVersion>846</hpi.compatibleSinceVersion>
     </properties>
 
     <dependencyManagement>
@@ -71,7 +71,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
             <artifactId>azure-credentials</artifactId>
-            <version>${azure-credentials.version}</version>
+            <version>254.v64da_8176c83a</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
@@ -174,7 +174,6 @@
                 </executions>
                 <configuration>
                     <configLocation>checkstyle.xml</configLocation>
-                    <encoding>UTF-8</encoding>
                     <consoleOutput>true</consoleOutput>
                     <resourceIncludes>src/main/resources/**</resourceIncludes>
                     <!--

src/main/java/com/microsoft/azure/vmagent/AzureVMAgentCleanUpTask.java

@@ -397,8 +397,8 @@ private int getPriority(GenericResource resource) {
                 resourcesMarkedForDeletion.add(resource);
             }
 
-            LOGGER.log(getNormalLoggingLevel(), "cleanLeakedResources: %d resources marked for deletion"
-                + resourcesMarkedForDeletion.size());
+            LOGGER.log(getNormalLoggingLevel(), String.format("cleanLeakedResources: %d resources marked for deletion",
+                    resourcesMarkedForDeletion.size()));
 
             while (!resourcesMarkedForDeletion.isEmpty()) {
                 try {

src/main/java/com/microsoft/azure/vmagent/AzureVMAgentTemplate.java

@@ -35,6 +35,9 @@
 import com.microsoft.azure.vmagent.builders.BuiltInImage;
 import com.microsoft.azure.vmagent.builders.BuiltInImageBuilder;
 import com.microsoft.azure.vmagent.exceptions.AzureCloudException;
+import com.microsoft.azure.vmagent.launcher.AzureComputerLauncher;
+import com.microsoft.azure.vmagent.launcher.AzureInboundLauncher;
+import com.microsoft.azure.vmagent.launcher.AzureSSHLauncher;
 import com.microsoft.azure.vmagent.util.AzureClientHolder;
 import com.microsoft.azure.vmagent.util.AzureUtil;
 import com.microsoft.azure.vmagent.util.Constants;
@@ -55,17 +58,6 @@
 import hudson.slaves.RetentionStrategy;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
-import jenkins.model.Jenkins;
-import org.apache.commons.lang.StringUtils;
-import org.kohsuke.accmod.Restricted;
-import org.kohsuke.accmod.restrictions.NoExternalUse;
-import org.kohsuke.stapler.AncestorInPath;
-import org.kohsuke.stapler.DataBoundConstructor;
-import org.kohsuke.stapler.DataBoundSetter;
-import org.kohsuke.stapler.QueryParameter;
-import org.kohsuke.stapler.interceptor.RequirePOST;
-
-import javax.servlet.ServletException;
 import java.io.IOException;
 import java.io.Serializable;
 import java.nio.charset.StandardCharsets;
@@ -82,6 +74,17 @@
 import java.util.TreeSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.servlet.ServletException;
+import jenkins.model.Jenkins;
+import org.apache.commons.lang.StringUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.stapler.AncestorInPath;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
+import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 /**
  * This class defines the configuration of Azure instance templates.
@@ -298,11 +301,13 @@ public int hashCode() {
 
     private final String osType;
 
-    private final String agentLaunchMethod;
+    private transient String agentLaunchMethod;
+
+    private AzureComputerLauncher launcher;
 
-    private boolean preInstallSsh;
+    private transient boolean preInstallSsh;
 
-    private String sshConfig;
+    private transient String sshConfig;
 
     private final String initScript;
 
@@ -404,7 +409,7 @@ public AzureVMAgentTemplate(
             String osType,
             String imageTopLevelType,
             ImageReferenceTypeClass imageReference,
-            String agentLaunchMethod,
+            AzureComputerLauncher launcher,
             String initScript,
             String terminateScript,
             String credentialsId,
@@ -451,7 +456,7 @@ public AzureVMAgentTemplate(
         this.osType = osType;
         this.initScript = initScript;
         this.terminateScript = terminateScript;
-        this.agentLaunchMethod = agentLaunchMethod;
+        this.launcher = launcher;
         this.credentialsId = credentialsId;
         this.virtualNetworkName = virtualNetworkName;
         this.virtualNetworkResourceGroupName = virtualNetworkResourceGroupName;
@@ -676,10 +681,14 @@ public static Map<String, Object> getTemplateProperties(AzureVMAgentTemplate tem
                         : template.getImageReference().getGalleryResourceGroup());
         templateProperties.put("osType",
                 isBasic ? defaultProperties.get(Constants.DEFAULT_OS_TYPE) : template.getOsType());
+        boolean isSSH = template.getLauncher() instanceof AzureSSHLauncher;
+        String agentLaunchMethod = isSSH ? Constants.LAUNCH_METHOD_SSH : Constants.LAUNCH_METHOD_JNLP;
         templateProperties.put("agentLaunchMethod",
-                isBasic ? defaultProperties.get(Constants.DEFAULT_LAUNCH_METHOD) : template.getAgentLaunchMethod());
-        templateProperties.put("sshConfig",
-                isBasic ? "" : template.getSshConfig());
+                isBasic ? defaultProperties.get(Constants.DEFAULT_LAUNCH_METHOD) : agentLaunchMethod);
+        if (isSSH) {
+            templateProperties.put("sshConfig",
+                    isBasic ? "" : ((AzureSSHLauncher) template.getLauncher()).getSshConfig());
+        }
         templateProperties.put("initScript",
                 isBasic ? getBasicInitScript(template) : template.getInitScript());
         templateProperties.put("terminateScript",
@@ -788,6 +797,24 @@ private Object readResolve() {
             storageAccountType = SkuName.STANDARD_LRS.toString();
         }
 
+        if (StringUtils.isNotBlank(agentLaunchMethod)) {
+            if (agentLaunchMethod.equalsIgnoreCase(Constants.LAUNCH_METHOD_SSH)) {
+                AzureSSHLauncher azureSSHLauncher = new AzureSSHLauncher();
+
+                if (StringUtils.isNotBlank(sshConfig)) {
+                    azureSSHLauncher.setSshConfig(sshConfig);
+                }
+                if (preInstallSsh) {
+                    azureSSHLauncher.setPreInstallSsh(preInstallSsh);
+                }
+                launcher = azureSSHLauncher;
+
+            } else if (agentLaunchMethod.equalsIgnoreCase(Constants.LAUNCH_METHOD_JNLP)) {
+                launcher = new AzureInboundLauncher();
+            }
+            agentLaunchMethod = null;
+        }
+
         if (StringUtils.isBlank(newStorageAccountName) && StringUtils.isBlank(existingStorageAccountName)
                 && StringUtils.isNotBlank(storageAccountName)) {
             newStorageAccountName = storageAccountName;
@@ -1047,24 +1074,6 @@ public String getOsType() {
         return osType;
     }
 
-    @DataBoundSetter
-    public void setPreInstallSsh(boolean preInstallSsh) {
-        this.preInstallSsh = preInstallSsh;
-    }
-
-    public boolean isPreInstallSsh() {
-        return preInstallSsh;
-    }
-
-    public String getSshConfig() {
-        return sshConfig;
-    }
-
-    @DataBoundSetter
-    public void setSshConfig(String sshConfig) {
-        this.sshConfig = sshConfig;
-    }
-
     public String getInitScript() {
         return initScript;
     }
@@ -1149,10 +1158,6 @@ public int getNoOfParallelJobs() {
         return noOfParallelJobs;
     }
 
-    public String getAgentLaunchMethod() {
-        return agentLaunchMethod;
-    }
-
     public ProvisionStrategy getTemplateProvisionStrategy() {
         return templateProvisionStrategy;
     }
@@ -1254,12 +1259,21 @@ public String getUamiID() {
         return uamiID;
     }
 
+    public AzureComputerLauncher getLauncher() {
+        return launcher;
+    }
+
     @DataBoundSetter
     public void setDoNotUseMachineIfInitFails(boolean doNotUseMachineIfInitFails) {
         this.doNotUseMachineIfInitFails = doNotUseMachineIfInitFails;
     }
 
     public AdvancedImage getAdvancedImageInside() {
+        boolean isSSH = launcher instanceof AzureSSHLauncher;
+        boolean preInstallSshLocal = launcher != null
+                && isSSH && ((AzureSSHLauncher) launcher).isPreInstallSsh();
+        String sshConfigLocal = launcher != null
+                && isSSH ? ((AzureSSHLauncher) launcher).getSshConfig() : null;
         return new AdvancedImageBuilder()
                 .withCustomImage(imageReference.uri)
                 .withCustomManagedImage(imageReference.id)
@@ -1279,9 +1293,9 @@ public AdvancedImage getAdvancedImageInside() {
                 )
                 .withNumberOfExecutors(String.valueOf(getNoOfParallelJobs()))
                 .withOsType(getOsType())
-                .withLaunchMethod(getAgentLaunchMethod())
-                .withPreInstallSsh(isPreInstallSsh())
-                .withSshConfig(getSshConfig())
+                .withLaunchMethod(isSSH ? Constants.LAUNCH_METHOD_SSH : Constants.LAUNCH_METHOD_JNLP)
+                .withPreInstallSsh(preInstallSshLocal)
+                .withSshConfig(sshConfigLocal)
                 .withInitScript(getInitScript())
                 .withVirtualNetworkName(getVirtualNetworkName())
                 .withVirtualNetworkResourceGroupName(getVirtualNetworkResourceGroupName())
@@ -1382,8 +1396,7 @@ public List<String> verifyTemplate() throws Exception {
                 imageReference,
                 builtInImage,
                 osType,
-                agentLaunchMethod,
-                sshConfig,
+                launcher,
                 initScript,
                 credentialsId,
                 virtualNetworkName,
@@ -1474,6 +1487,12 @@ public ListBoxModel doFillLocationItems(@RelativePath("..") @QueryParameter Stri
             return model;
         }
 
+        @SuppressWarnings("unused") // Used by jelly
+        @Restricted(DoNotUse.class) // Used by jelly
+        public AzureComputerLauncher getDefaultComputerLauncher() {
+            return new AzureSSHLauncher();
+        }
+
         public ListBoxModel doFillAvailabilitySetItems(
                 @RelativePath("../..") @QueryParameter String azureCredentialsId,
                 @RelativePath("../..") @QueryParameter String resourceGroupReferenceType,
@@ -1545,14 +1564,6 @@ public ListBoxModel doFillUsageModeItems() throws IOException, ServletException
             return model;
         }
 
-        public ListBoxModel doFillAgentLaunchMethodItems() {
-            ListBoxModel model = new ListBoxModel();
-            model.add(Messages.AzureVMAgentTemplate_SSH_Agent_Launch_Method(), Constants.LAUNCH_METHOD_SSH);
-            model.add(Messages.AzureVMAgentTemplate_Inbound_Agent_Launch_Method(), Constants.LAUNCH_METHOD_JNLP);
-
-            return model;
-        }
-
         public ListBoxModel doFillExistingStorageAccountNameItems(
                 @RelativePath("..") @QueryParameter String azureCredentialsId,
                 @RelativePath("..") @QueryParameter String resourceGroupReferenceType,
@@ -1662,6 +1673,11 @@ public FormValidation doCheckTemplateName(
             List<FormValidation> errors = new ArrayList<>();
             // Check whether the template name is valid, and then check
             // whether it would be shortened on VM creation.
+
+            if (StringUtils.isBlank(value)) {
+                return FormValidation.ok();
+            }
+
             if (!AzureUtil.isValidTemplateName(value)) {
                 errors.add(FormValidation.error(Messages.Azure_GC_Template_Name_Not_Valid()));
             }
@@ -1670,7 +1686,7 @@ public FormValidation doCheckTemplateName(
                 errors.add(FormValidation.warning(Messages.Azure_GC_TemplateStatus_Warn_Msg()));
             }
 
-            if (errors.size() > 0) {
+            if (!errors.isEmpty()) {
                 return FormValidation.aggregate(errors);
             }
 
@@ -1744,8 +1760,7 @@ public FormValidation doVerifyConfiguration(
                 @QueryParameter boolean galleryImageSpecialized,
                 @QueryParameter String gallerySubscriptionId,
                 @QueryParameter String galleryResourceGroup,
-                @QueryParameter String agentLaunchMethod,
-                @QueryParameter String sshConfig,
+                @RelativePath("..") @QueryParameter String sshConfig,
                 @QueryParameter String initScript,
                 @QueryParameter String credentialsId,
                 @QueryParameter String virtualNetworkName,
@@ -1797,7 +1812,7 @@ public FormValidation doVerifyConfiguration(
                             + "offer: {17};\n\t"
                             + "sku: {18};\n\t"
                             + "version: {19};\n\t"
-                            + "agentLaunchMethod: {20};\n\t"
+                            + "sshConfig: {20};\n\t"
                             + "initScript: {21};\n\t"
                             + "credentialsId: {22};\n\t"
                             + "virtualNetworkName: {23};\n\t"
@@ -1810,8 +1825,7 @@ public FormValidation doVerifyConfiguration(
                             + "galleryImageDefinition: {30}\n\t"
                             + "galleryImageVersion: {31}\n\t"
                             + "galleryResourceGroup: {32}\n\t"
-                            + "gallerySubscriptionId: {33}\n\t"
-                            + "sshConfig: {34}",
+                            + "gallerySubscriptionId: {33}",
                     new Object[]{
                             "",
                             "",
@@ -1833,7 +1847,7 @@ public FormValidation doVerifyConfiguration(
                             imageOffer,
                             imageSku,
                             imageVersion,
-                            agentLaunchMethod,
+                            sshConfig,
                             initScript,
                             credentialsId,
                             virtualNetworkName,
@@ -1846,8 +1860,7 @@ public FormValidation doVerifyConfiguration(
                             galleryImageDefinition,
                             galleryImageVersion,
                             galleryResourceGroup,
-                            gallerySubscriptionId,
-                            sshConfig});
+                            gallerySubscriptionId});
 
             // First validate the subscription info.  If it is not correct,
             // then we can't validate the
@@ -1857,6 +1870,11 @@ public FormValidation doVerifyConfiguration(
                 return FormValidation.error(result);
             }
 
+            AzureSSHLauncher azureComputerLauncher = new AzureSSHLauncher();
+            if (StringUtils.isNotBlank(sshConfig)) {
+                azureComputerLauncher.setSshConfig(sshConfig);
+            }
+
             final List<String> errors = AzureClientHolder.getDelegate(azureCredentialsId).verifyTemplate(
                     templateName,
                     labels,
@@ -1869,8 +1887,7 @@ public FormValidation doVerifyConfiguration(
                     image,
                     builtInImage,
                     osType,
-                    agentLaunchMethod,
-                    sshConfig,
+                    azureComputerLauncher,
                     initScript,
                     credentialsId,
                     virtualNetworkName,