Add support for node properties to template (#570)

Co-authored-by: Tim Jacomb <[email protected]>

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

@@ -33,16 +33,18 @@
 import hudson.slaves.EnvironmentVariablesNodeProperty;
 import hudson.slaves.JNLPLauncher;
 import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
 import hudson.slaves.OfflineCause;
 import hudson.slaves.RetentionStrategy;
 import hudson.slaves.SlaveComputer;
+import hudson.util.DescribableList;
 import hudson.util.FormValidation;
 import hudson.util.LogTaskListener;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.nio.charset.StandardCharsets;
-import java.util.Collections;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -151,7 +153,7 @@ public AzureVMAgent(
             String label,
             ComputerLauncher launcher,
             RetentionStrategy<AzureVMComputer> retentionStrategy,
-            List<? extends NodeProperty<?>> nodeProperties,
+            String fqdn,
             String cloudName,
             String vmCredentialsId,
             String sshPrivateKey,
@@ -181,7 +183,13 @@ public AzureVMAgent(
             String remotingOptions,
             AzureVMAgentTemplate template) throws FormException, IOException {
 
-        super(name, nodeDescription, remoteFS, numExecutors, mode, label, launcher, retentionStrategy, nodeProperties);
+        super(name, remoteFS, launcher);
+        setNodeDescription(nodeDescription);
+        setNumExecutors(numExecutors);
+        setMode(mode);
+        setLabelString(label);
+        setRetentionStrategy(retentionStrategy);
+        setNodeProperties(getNodeProperties(fqdn, template));
 
         this.cloudName = cloudName;
         this.templateName = templateName;
@@ -220,6 +228,24 @@ public AzureVMAgent(
         this.creationTime = System.currentTimeMillis();
     }
 
+    private static DescribableList<NodeProperty<?>, NodePropertyDescriptor> getNodeProperties(
+            String fqdn, AzureVMAgentTemplate template
+    ) {
+        DescribableList<NodeProperty<?>, NodePropertyDescriptor> nodeProperties = template.getNodeProperties();
+        EnvironmentVariablesNodeProperty envVarsNodeProperty = nodeProperties
+                .get(EnvironmentVariablesNodeProperty.class);
+
+        List<EnvironmentVariablesNodeProperty.Entry> newEntries = new ArrayList<>();
+        if (envVarsNodeProperty != null) {
+            nodeProperties.remove(envVarsNodeProperty);
+            newEntries.addAll(envVarsNodeProperty.getEnv());
+        }
+        newEntries.add(new EnvironmentVariablesNodeProperty.Entry("FQDN", fqdn));
+        nodeProperties.add(new EnvironmentVariablesNodeProperty(newEntries));
+
+        return nodeProperties;
+    }
+
     public AzureVMAgent(
             ProvisioningActivity.Id id,
             String name,
@@ -259,7 +285,6 @@ public AzureVMAgent(
             String javaPath,
             String remotingOptions
     ) throws FormException, IOException {
-
         this(name,
                 templateName,
                 nodeDescription,
@@ -271,9 +296,7 @@ public AzureVMAgent(
                 agentLaunchMethod.equalsIgnoreCase("SSH")
                         ? new AzureVMAgentSSHLauncher() : new JNLPLauncher(),
                 retentionStrategy,
-                Collections.singletonList(new EnvironmentVariablesNodeProperty(
-                        new EnvironmentVariablesNodeProperty.Entry("FQDN", fqdn)
-                )),
+                fqdn,
                 cloudName,
                 vmCredentialsId,
                 sshPrivateKey,

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

@@ -44,18 +44,23 @@
 import com.microsoft.azure.vmagent.util.FailureStage;
 import com.microsoft.jenkins.credentials.AzureResourceManagerCache;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Extension;
 import hudson.RelativePath;
 import hudson.Util;
 import hudson.model.Describable;
 import hudson.model.Descriptor;
 import hudson.model.Label;
 import hudson.model.Node;
+import hudson.model.Saveable;
 import hudson.model.TaskListener;
 import hudson.model.labels.LabelAtom;
 import hudson.security.ACL;
 import hudson.slaves.Cloud;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
 import hudson.slaves.RetentionStrategy;
+import hudson.util.DescribableList;
 import hudson.util.FormApply;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
@@ -384,6 +389,10 @@ public int hashCode() {
 
     private String licenseType;
 
+    @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "They are serializable in practice")
+    private DescribableList<NodeProperty<?>, NodePropertyDescriptor> nodeProperties =
+            new DescribableList<>(Saveable.NOOP);
+
     // deprecated fields
     private transient boolean isInstallDocker;
     private transient boolean isInstallMaven;
@@ -492,6 +501,18 @@ public AzureVMAgentTemplate(
         this.tags = new ArrayList<>();
     }
 
+    public DescribableList<NodeProperty<?>, NodePropertyDescriptor> getNodeProperties() {
+        return nodeProperties;
+    }
+
+    @DataBoundSetter
+    public void setNodeProperties(List<? extends NodeProperty<?>> nodeProperties) throws IOException {
+        if (nodeProperties == null) {
+            this.nodeProperties = new DescribableList<>(Saveable.NOOP);
+        }
+        this.nodeProperties.replaceBy(nodeProperties);
+    }
+
     @DataBoundSetter
     public void setBuiltInImage(String builtInImage) {
         this.builtInImage = builtInImage;
@@ -827,6 +848,10 @@ private Object readResolve() {
             storageAccountType = SkuName.STANDARD_LRS.toString();
         }
 
+        if (nodeProperties == null) {
+            nodeProperties = new DescribableList<>(Saveable.NOOP);
+        }
+
         if (StringUtils.isNotBlank(agentLaunchMethod)) {
             if (agentLaunchMethod.equalsIgnoreCase(Constants.LAUNCH_METHOD_SSH)) {
                 AzureSSHLauncher azureSSHLauncher = new AzureSSHLauncher();
@@ -1912,6 +1937,11 @@ public FormValidation doCheckJvmOptions(@QueryParameter String value) {
             return FormValidation.ok();
         }
 
+        // called by stapler
+        public List<NodePropertyDescriptor> getNodePropertyDescriptors() {
+            return NodePropertyDescriptor.for_(NodeProperty.all(), AzureVMAgent.class);
+        }
+
         @POST
         public FormValidation doVerifyConfiguration(
                 @QueryParameter String cloudName,

src/main/resources/com/microsoft/azure/vmagent/AzureVMAgentTemplate/config.jelly

@@ -179,6 +179,8 @@
       </f:section>
     </f:radioBlock>
 
+    <f:descriptorList title="${%Node Properties}" field="nodeProperties" descriptors="${descriptor.nodePropertyDescriptors}" />
+
     <f:radioBlock name="imageTopLevelType" value="advanced" title="${%Advanced_Image}"
                   checked="${instance.isTopLevelType('advanced')}" inline="true">
       <f:dropdownList name="imageReference">

src/main/resources/com/microsoft/azure/vmagent/AzureVMComputer/configure.jelly

@@ -3,22 +3,23 @@
   <l:layout permission="${it.EXTENDED_READ}" title="${it.name} Configuration">
     <st:include page="sidepanel.jelly"/>
     <l:main-panel>
+      <j:set var="instance" value="${it.node}" />
+      <j:set var="descriptor" value="${instance.descriptor}" />
+      <j:set var="readOnlyMode" value="true" />
       <l:app-bar title="${it.name} Configuration" />
-      <f:entry title="${%Name}">
-        <f:readOnlyTextbox value="${it.name}"/>
-      </f:entry>
+      <p><a href="${it.azurePortalLink}">View on Azure Portal</a></p>
       <f:entry title="${%FQDN}">
-        <f:readOnlyTextbox value="${it.publicDNSName}"/>
+        <f:textbox value="${it.publicDNSName}"/>
       </f:entry>
       <j:if test="${!empty(it.publicIP)}">
         <f:entry title="${%Public_IP}">
-          <f:readOnlyTextbox value="${it.publicIP}"/>
+          <f:textbox value="${it.publicIP}"/>
         </f:entry>
       </j:if>
       <f:entry title="${%Private_IP}">
-        <f:readOnlyTextbox value="${it.privateIP}"/>
+        <f:textbox value="${it.privateIP}"/>
       </f:entry>
-      <a href="${it.azurePortalLink}">View on Azure Portal</a>
+      <f:descriptorList title="${%Node Properties}" descriptors="${h.getNodePropertyDescriptors(descriptor.clazz)}" field="nodeProperties" />
     </l:main-panel>
   </l:layout>
 </j:jelly>

src/test/java/com/microsoft/azure/vmagent/test/jcasc/BasicConfigAsCodeTest.java

@@ -5,7 +5,10 @@
 import com.microsoft.azure.vmagent.AzureVMCloud;
 import com.microsoft.azure.vmagent.AzureVMCloudRetensionStrategy;
 import com.microsoft.azure.vmagent.launcher.AzureSSHLauncher;
+import hudson.EnvVars;
 import hudson.model.Node;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.NodeProperty;
 import io.jenkins.plugins.casc.ConfigurationContext;
 import io.jenkins.plugins.casc.ConfiguratorRegistry;
 import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
@@ -17,12 +20,15 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 
+import static hudson.Functions.getEnvVars;
 import static io.jenkins.plugins.casc.misc.Util.getJenkinsRoot;
 import static io.jenkins.plugins.casc.misc.Util.toYamlString;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
 public class BasicConfigAsCodeTest {
 
@@ -90,6 +96,12 @@ public void importBasicConfiguration() {
         assertThat(template.getUsePrivateIP(), is(false));
 
         assertThat(template.getVirtualMachineSize(), is("Standard_DS2_v2"));
+
+        assertNotNull(template.getNodeProperties());
+        EnvironmentVariablesNodeProperty property = template.getNodeProperties().get(EnvironmentVariablesNodeProperty.class);
+        assertNotNull("The EnvironmentVariablesNodeProperty should not be null", property);
+        assertTrue("The environment variable FOO should exist", property.getEnvVars().containsKey("FOO"));
+
     }
 
     @Test

src/test/resources/com/microsoft/azure/vmagent/test/jcasc/basic.yaml

@@ -43,4 +43,9 @@ jenkins:
             templateName: "ubuntu"
             usageMode: "NORMAL"
             usePrivateIP: false
-            virtualMachineSize: "Standard_DS2_v2"
+            virtualMachineSize: "Standard_DS2_v2"
+            nodeProperties:
+              - envVars:
+                  env:
+                    - key: "FOO"
+                      value: "BAR"

src/test/resources/com/microsoft/azure/vmagent/test/jcasc/expectedBasic.yaml

@@ -23,6 +23,11 @@
       location: "East US"
       newStorageAccountName: "agent-storage"
       noOfParallelJobs: 1
+      nodeProperties:
+      - envVars:
+          env:
+          - key: "FOO"
+            value: "BAR"
       osType: "Linux"
       retentionStrategy:
         azureVMCloudRetentionStrategy: