Make most ITs work with both linux and windows

CONTRIBUTING.md

@@ -41,9 +41,12 @@ An essential part of getting your change through is to make sure all existing te
 
 #### Prerequisites
 * Maven to build and run tests.
-* A GCP project to test on with the compute engine API enabled with all relevant permissions enabled, especially billing. Running integration tests will also incur billing.
+* A GCP project to test on with the compute engine API enabled with all relevant permissions
+  enabled, especially billing. Running integration tests will also incur billing.
 * For development, IntelliJ is recommended.
-* **For Windows Images/VM's**, have Java and OpenSSH pre-installed. We have suggested startup-scripts for installing both if you do not want to pre-install, but pre-installing is advised.
+* **For Windows Images/VM's**, have Java and OpenSSH pre-installed. We have suggested
+  startup-scripts for installing both if you do not want to pre-install,
+  but pre-installing is advised.
 
 
 #### Running the tests
@@ -58,36 +61,35 @@ mvn test
 ```
 
 ##### Integration Tests
-* The following environment variables are required to run the integration tests. 3. and 4. are only required when running a windows integration test.
+* The following environment variables are required to run the integration tests. 5, 6, and 7 are
+  only required when running a windows integration test.
 
 1. GOOGLE_PROJECT_ID
 1. GOOGLE_CREDENTIALS
+1. GOOGLE_REGION
+1. GOOGLE_ZONE
 1. GOOGLE_BOOT_DISK_PROJECT_ID
 1. GOOGLE_BOOT_DISK_IMAGE_NAME
+1. GOOGLE_JENKINS_PASSWORD
 
 * Run the following:
 ```
 mvn verify
 ```
 
 ###### Windows Integration Test
-* By default, in the pom.xml file, exclude running the Windows Integration test unless a windows-related change was made since it will require a windows image built with Packer.
-* Exclusion is in these couple of lines between the excludes tags:
-```
-<configuration>
-    <disableXmlReport>true</disableXmlReport>
-    <useFile>false</useFile>
-    ...
-    <excludes>
-        <exclude>**/ComputeEngineCloudWindowsIT.java</exclude>
-        <exclude>**/ConfigAsCodeWindowsIT.java</exclude>
-        ...
-    </excludes>
-</configuration>
+* By default, the integration tests only use linux based agents for testing. If you make a
+  windows-related change, or otherwise want to test that a change still works for windows agents,
+  run the tests with the flag `-Dit.windows=true` like this:
+```bash
+mvn verify -Dit.windows=true
 ```
 
-* If you do make a **windows-related change**, remove the windows exclusions temporarily and run the
-integration test with mvn verify,
+Make sure you have these extra environment variables configured:
   * GOOGLE_BOOT_DISK_PROJECT_ID will be the same as your project id.
-  * GOOGLE_BOOT_DISK_IMAGE_NAME will be the name of the image you created using packer in google cloud console.
-  * More information on building your baseline windows image can be found [here](WINDOWS.md) and an example file can be found [here](windows-it-install.ps1).
+  * GOOGLE_BOOT_DISK_IMAGE_NAME will be the name of the image you created using packer in Google
+    cloud console.
+  * GOOGLE_JENKINS_PASSWORD will be the password you set when creating the image with packer, used
+    for password based ssh authentication.
+  * More information on building your baseline windows image can be found [here](WINDOWS.md) 
+    and an example powershell script for setup can be found [here](windows-it-install.ps1).

Jenkinsfile → Jenkinsfile.linux

@@ -28,7 +28,8 @@ pipeline {
         GOOGLE_ZONE = "${GCE_IT_ZONE}"
         GOOGLE_SA_NAME = "${GCE_IT_SA}"
         BUILD_ARTIFACTS_BUCKET = "${GCE_IT_BUCKET}"
-        BUILD_ARTIFACTS = "build-${BRANCH_NAME}-${BUILD_ID}.tar.gz"
+        CLEAN_BRANCH_NAME = "${BRANCH_NAME}".replaceAll("[/&;<>|\\]]", "_")
+        BUILD_ARTIFACTS = "build-${CLEAN_BRANCH_NAME}-${BUILD_ID}.tar.gz"
     }
 
     stages {

Jenkinsfile.windows

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2019 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Windows Agent IT pipeline, run manually.
+pipeline {
+    agent {
+        kubernetes {
+            cloud 'kubernetes'
+            label 'maven-pod'
+            yamlFile 'jenkins/maven-pod.yaml'
+        }
+    }
+
+    environment {
+        GOOGLE_PROJECT_ID = "${GCE_IT_PROJECT_ID}"
+        GOOGLE_REGION = "${GCE_IT_REGION}"
+        GOOGLE_ZONE = "${GCE_IT_ZONE}"
+        GOOGLE_SA_NAME = "${GCE_IT_SA}"
+        GOOGLE_BOOT_DISK_PROJECT_ID = "${GCE_WINDOWS_IT_IMAGE_PROJECT}"
+        GOOGLE_BOOT_DISK_IMAGE_NAME = "${GCE_WINDOWS_IT_IMAGE_NAME}"
+        BUILD_ARTIFACTS_BUCKET = "${GCE_IT_BUCKET}"
+        CLEAN_BRANCH_NAME = "${BRANCH_NAME}".replaceAll("[/&;<>|\\]]", "_")
+        BUILD_ARTIFACTS = "windows-${CLEAN_BRANCH_NAME}-${BUILD_ID}.tar.gz"
+    }
+
+    stages {
+        stage("Build and test") {
+            steps {
+                container('maven') {
+                    withCredentials([[$class: 'StringBinding', credentialsId: env.GCE_IT_CRED_ID, variable: 'GOOGLE_CREDENTIALS'],
+                                     [$class: 'StringBinding', credentialsId: env.GCE_WINDOWS_IT_JENKINS_PASSWORD, variable: 'GOOGLE_JENKINS_PASSWORD']]) {
+                        catchError {
+                            // build
+                            sh "mvn clean package -ntp"
+
+                            // run tests
+                            sh "mvn verify -ntp -Dit.windows=true"
+                        }
+
+                        sh "jenkins/saveAndCompress.sh"
+                        step([$class: 'ClassicUploadStep', credentialsId: env.GCE_BUCKET_CRED_ID, bucket: "gs://${BUILD_ARTIFACTS_BUCKET}", pattern: env.BUILD_ARTIFACTS])
+                    }
+                }
+            }
+        }
+    }
+}

pom.xml

@@ -73,8 +73,11 @@
     <maven.compiler.source>1.8</maven.compiler.source>
     <maven.compiler.target>1.8</maven.compiler.target>
     <skip.surefire.tests>${skipTests}</skip.surefire.tests>
+    <it.windows>false</it.windows>
     <configuration-as-code.version>1.14</configuration-as-code.version>
+    <powershell.version>1.3</powershell.version>
     <concurrency>10</concurrency>
+    <it.runOrder>balanced</it.runOrder>
   </properties>
 
   <dependencies>
@@ -172,6 +175,12 @@
       <version>${configuration-as-code.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>powershell</artifactId>
+      <version>${powershell.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <repositories>
@@ -262,14 +271,13 @@
             <hudson.model.LoadStatistics.decay>0.5</hudson.model.LoadStatistics.decay>
             <hudson.agents.NodeProvisioner.MARGIN>100</hudson.agents.NodeProvisioner.MARGIN>
             <hudson.agents.NodeProvisioner.MARGIN0>1.0</hudson.agents.NodeProvisioner.MARGIN0>
+            <com.google.jenkins.plugins.computeengine.integration.ITUtil.windows>${it.windows}</com.google.jenkins.plugins.computeengine.integration.ITUtil.windows>
           </systemPropertyVariables>
           <excludes>
-            <exclude>**/ComputeEngineCloudWindowsIT.java</exclude>
-            <exclude>**/ConfigAsCodeWindowsIT.java</exclude>
             <exclude>**/ITUtil.java</exclude>
           </excludes>
           <argLine>-Djenkins.test.timeout=${jenkinsRuleTimeout}</argLine>
-          <runOrder>balanced</runOrder>
+          <runOrder>${it.runOrder}</runOrder>
         </configuration>
       </plugin>
       <plugin>
@@ -332,5 +340,18 @@
         <concurrency>4</concurrency>
       </properties>
     </profile>
+    <profile>
+      <id>windows-agents</id>
+      <activation>
+        <property>
+          <name>it.windows</name>
+          <value>true</value>
+        </property>
+      </activation>
+      <properties>
+        <concurrency>8</concurrency>
+        <it.runOrder>reversealphabetical</it.runOrder>
+      </properties>
+    </profile>
   </profiles>
 </project>

src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java

@@ -19,14 +19,12 @@
 import com.google.api.services.compute.model.Instance;
 import com.google.api.services.compute.model.Scheduling;
 import hudson.model.Executor;
-import hudson.model.Result;
 import hudson.model.TaskListener;
 import hudson.slaves.AbstractCloudComputer;
 import java.io.IOException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.logging.Level;
-import jenkins.model.CauseOfInterruption;
 import lombok.extern.java.Log;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.HttpRedirect;
@@ -76,14 +74,7 @@ private Boolean getPreemptedStatus(TaskListener listener, String nodeName) {
 
   private void interruptExecutor(Executor executor, String nodeName) {
     log.log(Level.INFO, "Terminating executor " + executor + " node " + nodeName);
-    executor.interrupt(
-        Result.FAILURE,
-        new CauseOfInterruption() {
-          @Override
-          public String getShortDescription() {
-            return "Instance " + nodeName + " was preempted";
-          }
-        });
+    executor.abortResult();
   }
 
   /**

src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java

@@ -51,27 +51,18 @@ protected Optional<Connection> setupConnection(
       return Optional.empty();
     }
 
-    Connection cleanupConn;
     GoogleKeyPair kp = node.getSSHKeyPair().get();
-    boolean isBootstrapped = bootstrap(kp, computer, listener);
-    if (isBootstrapped) {
-      // connect fresh as ROOT
-      logInfo(computer, listener, "connect fresh as root");
-      cleanupConn = connectToSsh(computer, listener);
-      if (!cleanupConn.authenticateWithPublicKey(
-          node.getSshUser(), kp.getPrivateKey().toCharArray(), "")) {
-        logWarning(computer, listener, "Authentication failed");
-        return Optional.empty(); // failed to connect
-      }
-    } else {
+    Optional<Connection> bootstrapConn = bootstrap(kp, computer, listener);
+    if (!bootstrapConn.isPresent()) {
       logWarning(computer, listener, "bootstrapresult failed");
       return Optional.empty();
     }
 
-    return Optional.of(cleanupConn);
+    return bootstrapConn;
   }
 
-  private boolean bootstrap(GoogleKeyPair kp, ComputeEngineComputer computer, TaskListener listener)
+  private Optional<Connection> bootstrap(
+      GoogleKeyPair kp, ComputeEngineComputer computer, TaskListener listener)
       throws IOException, Exception { // TODO: better exceptions
     logInfo(computer, listener, "bootstrap");
     ComputeEngineInstance node = computer.getNode();
@@ -105,14 +96,16 @@ private boolean bootstrap(GoogleKeyPair kp, ComputeEngineComputer computer, Task
       }
       if (!isAuthenticated) {
         logWarning(computer, listener, "Authentication failed");
-        return false;
+        return Optional.empty();
       }
-    } finally {
+    } catch (Exception e) {
+      logException(computer, listener, "Failed to authenticate with exception: ", e);
       if (bootstrapConn != null) {
         bootstrapConn.close();
       }
+      return Optional.empty();
     }
-    return true;
+    return Optional.of(bootstrapConn);
   }
 
   @Override

src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineWindowsLauncher.java

@@ -16,6 +16,7 @@
 
 import com.cloudbees.jenkins.plugins.sshcredentials.SSHAuthenticator;
 import com.google.api.services.compute.model.Operation;
+import com.google.common.base.Preconditions;
 import com.trilead.ssh2.Connection;
 import hudson.model.TaskListener;
 import java.io.IOException;
@@ -51,21 +52,12 @@ protected Optional<Connection> setupConnection(
       logWarning(computer, listener, "Non-windows node provided");
       return Optional.empty();
     }
-    boolean isBootstrapped = bootstrap(computer, listener);
-    if (!isBootstrapped) {
+    Optional<Connection> bootstrapConn = bootstrap(computer, listener);
+    if (!bootstrapConn.isPresent()) {
       logWarning(computer, listener, "bootstrapresult failed");
       return Optional.empty();
     }
-
-    // connect fresh as ROOT
-    logInfo(computer, listener, "connect fresh as root");
-    Connection cleanupConn = connectToSsh(computer, listener);
-    if (!authenticateSSH(node.getSshUser(), node.getWindowsConfig(), cleanupConn, listener)) {
-      logWarning(computer, listener, "Authentication failed");
-      return Optional.empty(); // failed to connect
-    }
-
-    return Optional.of(cleanupConn);
+    return bootstrapConn;
   }
 
   private boolean authenticateSSH(
@@ -87,11 +79,8 @@ private boolean authenticateSSH(
     return isAuthenticated;
   }
 
-  private boolean bootstrap(ComputeEngineComputer computer, TaskListener listener)
-      throws IOException, Exception { // TODO(evanbrown): better exceptions
-    if (computer == null) {
-      throw new IllegalArgumentException("A null ComputeEngineComputer was provided");
-    }
+  private Optional<Connection> bootstrap(ComputeEngineComputer computer, TaskListener listener) {
+    Preconditions.checkNotNull(computer, "A null ComputeEngineComputer was provided");
     logInfo(computer, listener, "bootstrap");
 
     ComputeEngineInstance node = computer.getNode();
@@ -125,14 +114,16 @@ private boolean bootstrap(ComputeEngineComputer computer, TaskListener listener)
       }
       if (!isAuthenticated) {
         logWarning(computer, listener, "Authentication failed");
-        return false;
+        return Optional.empty();
       }
-    } finally {
+    } catch (Exception e) {
+      logException(computer, listener, "Failed to authenticate with exception: ", e);
       if (bootstrapConn != null) {
         bootstrapConn.close();
       }
+      return Optional.empty();
     }
-    return true;
+    return Optional.ofNullable(bootstrapConn);
   }
 
   @Override