update integration tests

CONTRIBUTING.md

@@ -54,45 +54,75 @@ An essential part of getting your change through is to make sure all existing te
 * Make sure you are at the directory where pom.xml is located.
 
 ##### Unit Tests
-* Run the following:
-
-```
-mvn test
-```
+* Run the following: 
+    ```
+    mvn test
+    ```
 
 ##### Integration Tests
-* By default, the integration tests are not executed. In case you are interested in executing 
-  them, disable the `skipITs` property.
-* 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
-1. GOOGLE_SA_NAME
-
-* Run the following:
-```
-mvn verify -DskipITs=false
-```
+
+Integration tests actually provision instances in a GCP project, run pipeline, take snapshot etc.
+Therefore, they are disabled in the CI and expected to be executed by contributors in their laptop itself.
+
+Reasons for disabling integration test in CI,
+* getting provisioning GCP infra is not possible
+* even if we did get a GCP infra setup in the CI, it is risky to expose that, as someone can abuse the CI.
+
+By default, integration tests are skipped from the maven goals, need to enable using the `skipITs` property.
+
+Steps to execute integration test
+* Prepare VM images  
+  (ideally we should automate this see idea [here](https://github.com/jenkinsci/google-compute-engine-plugin/pull/492#discussion_r1892705637))  
+  The jenkins agent images need to have java installed. We have a packer script to create the image and upload to your configured GCP project.
+  The scripts are located in [testimages/linux](./testimages/linux)
+  Navigate to the directory and execute,
+  ```bash
+  bash setup-gce-image.sh
+  ```
+  If you want to execute the integration tests for `non-standard-java` location, then create non-standard-java image as,
+  ```bash
+  bash setup-gce-image.sh non-standard-java
+  ```
+  If you want to delete the images or recreate them, use the arguments `--recreate` or `--delete`.
+
+* Create a service account with relevant access - See [Refer to IAM Credentials](Home.md#iam-credentials)   
+
+* Export these mandatory environment variable   
+  ```bash
+  export GOOGLE_PROJECT_ID=your-project-id
+  export GOOGLE_CREDENTIALS=/path/to/sa-key.json
+  export GOOGLE_REGION=us-central1
+  export GOOGLE_ZONE=us-central1-a
+  export GOOGLE_SA_NAME=jenkins-agent-sa
+  ```
+* Run the integration tests as,  
+  * Run all the tests   
+    ```bash
+    mvn verify -DskipITs=false -Djenkins.test.timeout=1200
+    ```
+  * Run a specific test class  
+    ```bash
+    mvn clean test -Dtest=ComputeEngineCloudRestartPreemptedIT
+    ```
+  * Run a specific test method  
+    ```bash
+    mvn clean test -Dtest=ComputeEngineCloudRestartPreemptedIT#testIfNodeWasPreempted
+    ```
+  You can also debug the tests with surefire by passing `-Dmaven.surefire.debug=true` and in your IDE connect to remote debug port `8000`.
 
 ###### Windows Integration Test
 * 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
-```
-
-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.
-  * 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).
+  run the tests with the flag `-Dit.windows=true` like this:  
+  ```bash
+  mvn verify -Dit.windows=true
+  ```
+* You need to prepare the windows image before running the tests.  
+  * 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).  
+  * In addition to the environment variables mentioned in the previous section, also export these variables too,  
+    ```bash
+    export GOOGLE_BOOT_DISK_PROJECT_ID=your-project-id # will be the same as your project id
+    expot GOOGLE_BOOT_DISK_IMAGE_NAME=windows-image-name # will be the name of the image you created using packer in Google cloud console
+    export GOOGLE_JENKINS_PASSWORD=password # will be the password you set when creating the image with packer, used for password based ssh authentication.
+    ```

pom.xml

@@ -269,7 +269,27 @@
       <version>${powershell.version}</version>
       <scope>test</scope>
     </dependency>
-    <!-- Test Dependencies -->
+    <!--
+    The 3 plugins `workflow-cps`, `workflow-durable-task-step`, `workflow-job` are put here
+      to assist tests to also test for `Pipeline` based jobs.
+      It is good to have tests that do test for both `FreeStyle` and `Pipeline` jobs to ensure compatibility.
+      Note: FreeStyle job comes with jenkins core itself, so there is no need for additional dependency for it.
+    -->
+    <dependency>
+      <groupId>org.jenkins-ci.plugins.workflow</groupId>
+      <artifactId>workflow-cps</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins.workflow</groupId>
+      <artifactId>workflow-durable-task-step</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins.workflow</groupId>
+      <artifactId>workflow-job</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>

src/main/java/com/google/jenkins/plugins/computeengine/client/ClientUtil.java

@@ -13,6 +13,7 @@
 import com.google.jenkins.plugins.credentials.oauth.GoogleOAuth2Credentials;
 import com.google.jenkins.plugins.credentials.oauth.GoogleRobotCredentials;
 import hudson.AbortException;
+import hudson.Main;
 import hudson.model.ItemGroup;
 import hudson.security.ACL;
 import java.io.IOException;
@@ -73,6 +74,20 @@ private static GoogleRobotCredentials getRobotCredentials(
             ItemGroup itemGroup, List<DomainRequirement> domainRequirements, String credentialsId)
             throws AbortException {
 
+        /* During the integration tests, the parameter `credentialId` is equal to the `<Project-Id>` which is set
+            by the environment variable. But the actual credential created within Jenkins is having `id` as a random
+            UUID. So the `CredentialsMatchers.firstOrNull` was returning `null` due to `CredentialsMatchers.withId
+            (credentialsId)`
+        */
+        if (Main.isUnitTest) {
+            var credentialList = CredentialsProvider.lookupCredentials(
+                    GoogleOAuth2Credentials.class, itemGroup, ACL.SYSTEM, domainRequirements);
+            if (!credentialList.isEmpty()) {
+                return (GoogleRobotCredentials) credentialList.get(0);
+            }
+            return null;
+        }
+
         GoogleOAuth2Credentials credentials = CredentialsMatchers.firstOrNull(
                 CredentialsProvider.lookupCredentials(
                         GoogleOAuth2Credentials.class, itemGroup, ACL.SYSTEM, domainRequirements),

src/test/java/com/google/jenkins/plugins/computeengine/integration/ComputeClientIT.java

@@ -58,7 +58,7 @@ public static void teardown() throws IOException {
 
     @Test
     public void testGetImage() throws Exception {
-        Image image = client.getImage("debian-cloud", "debian-9-stretch-v20180820");
+        Image image = client.getImage("debian-cloud", "debian-12-bookworm-v20241210");
         assertNotNull(image);
         assertEquals("READY", image.getStatus());
     }

src/test/java/com/google/jenkins/plugins/computeengine/integration/ComputeEngineCloudMultipleMatchingConfigurationsIT.java

@@ -19,15 +19,17 @@
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.LABEL;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.NULL_TEMPLATE;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.NUM_EXECUTORS;
+import static com.google.jenkins.plugins.computeengine.integration.ITUtil.PROJECT_ID;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.TEST_TIMEOUT_MULTIPLIER;
+import static com.google.jenkins.plugins.computeengine.integration.ITUtil.ZONE;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.getLabel;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.initClient;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.initCloud;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.initCredentials;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.instanceConfigurationBuilder;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.teardownResources;
+import static org.awaitility.Awaitility.await;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import com.google.cloud.graphite.platforms.plugin.client.ComputeClient;
@@ -52,8 +54,7 @@
 
 /**
  * Integration test suite for {@link ComputeEngineCloud}. Verifies that instances can be created
- * with multiple matching {@link InstanceConfiguration} and that these instances are properly
- * provisioned.
+ * with multiple matching {@link InstanceConfiguration} and that these instances are provisioned in round-robin fashion.
  */
 public class ComputeEngineCloudMultipleMatchingConfigurationsIT {
     private static Logger log = Logger.getLogger(ComputeEngineCloudMultipleMatchingConfigurationsIT.class.getName());
@@ -105,16 +106,18 @@ public static void teardown() throws IOException {
     }
 
     @Test
-    public void testMultipleLabelsProvisionedWithLabels() {
+    public void testMultipleLabelsProvisionedWithLabels() throws IOException {
         assertEquals(2, planned.size());
 
         final Iterator<PlannedNode> iterator = planned.iterator();
         PlannedNode firstNode = iterator.next();
         PlannedNode secondNode = iterator.next();
         if (checkOneNode(firstNode, DESC_1)) {
-            assertTrue(checkOneNode(secondNode, DESC_2));
+            assertDescription(firstNode, DESC_1);
+            assertDescription(secondNode, DESC_2);
         } else if (checkOneNode(secondNode, DESC_1)) {
-            assertTrue(checkOneNode(firstNode, DESC_2));
+            assertDescription(secondNode, DESC_1);
+            assertDescription(firstNode, DESC_2);
         } else {
             fail("Nodes did not have expected values");
         }
@@ -125,4 +128,22 @@ private boolean checkOneNode(PlannedNode plannedNode, String desc) {
         Node node = jenkinsRule.jenkins.getNode(name);
         return desc.equals(node.getNodeDescription());
     }
+
+    private void assertDescription(PlannedNode plannedNode, String agentDesc) throws IOException {
+        String agentName = plannedNode.displayName;
+
+        assertEquals(
+                "Jenkins agent description is incorrect",
+                agentDesc,
+                jenkinsRule.jenkins.getNode(agentName).getNodeDescription());
+
+        // verify the corresponding instance is actually provisioned in GCP
+        // and also has matching description
+        await("Instance is in running status").timeout(2, TimeUnit.MINUTES).until(() -> "RUNNING"
+                .equals(client.getInstance(PROJECT_ID, ZONE, agentName).getStatus()));
+        assertEquals(
+                "Instance description is incorrect",
+                agentDesc,
+                client.getInstance(PROJECT_ID, ZONE, agentName).getDescription());
+    }
 }

src/test/java/com/google/jenkins/plugins/computeengine/integration/ComputeEngineCloudNoSnapshotCreatedIT.java

@@ -59,7 +59,7 @@
  * instance is terminated but no snapshot is created.
  */
 public class ComputeEngineCloudNoSnapshotCreatedIT {
-    private static Logger log = Logger.getLogger(ComputeEngineCloudNoSnapshotCreatedIT.class.getName());
+    private static final Logger log = Logger.getLogger(ComputeEngineCloudNoSnapshotCreatedIT.class.getName());
 
     @ClassRule
     public static Timeout timeout = new Timeout(7 * TEST_TIMEOUT_MULTIPLIER, TimeUnit.MINUTES);

src/test/java/com/google/jenkins/plugins/computeengine/integration/ComputeEngineCloudNonStandardJavaIT.java

@@ -16,11 +16,11 @@
 
 package com.google.jenkins.plugins.computeengine.integration;
 
+import static com.google.jenkins.plugins.computeengine.integration.ITUtil.BOOT_DISK_IMAGE_NAME;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.LABEL;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.NULL_TEMPLATE;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.NUM_EXECUTORS;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.PROJECT_ID;
-import static com.google.jenkins.plugins.computeengine.integration.ITUtil.TEST_TIMEOUT_MULTIPLIER;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.ZONE;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.getLabel;
 import static com.google.jenkins.plugins.computeengine.integration.ITUtil.initClient;
@@ -42,37 +42,24 @@
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
-import org.junit.rules.Timeout;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.recipes.WithTimeout;
 
 /**
  * Integration test suite for {@link ComputeEngineCloudNonStandardJavaIT}. Verifies that instances
  * can be created using a non-standard Java executable path.
  */
 public class ComputeEngineCloudNonStandardJavaIT {
-    private static final String NON_STANDARD_JAVA_STARTUP_SCRIPT = "#!/bin/bash\n"
-            + "sudo su -\n"
-            + "/etc/init.d/ssh stop\n"
-            + "echo \"deb http://http.debian.net/debian stretch-backports main\" >> /etc/apt/sources.list\n"
-            + "apt-get -y update\n"
-            + "apt-get -y install -t stretch-backports openjdk-8-jdk\n"
-            + "update-java-alternatives -s java-1.8.0-openjdk-amd64\n"
-            + "mv /usr/bin/java /usr/bin/non-standard-java\n"
-            + "/etc/init.d/ssh start";
 
     private static final String NON_STANDARD_JAVA_PATH = "/usr/bin/non-standard-java";
 
     private static Logger log = Logger.getLogger(ComputeEngineCloudNonStandardJavaIT.class.getName());
 
-    @ClassRule
-    public static Timeout timeout = new Timeout(5 * TEST_TIMEOUT_MULTIPLIER, TimeUnit.MINUTES);
-
     @ClassRule
     public static JenkinsRule jenkinsRule = new JenkinsRule();
 
@@ -88,9 +75,8 @@ public static void init() throws Exception {
         initCredentials(jenkinsRule);
         ComputeEngineCloud cloud = initCloud(jenkinsRule);
         client = initClient(jenkinsRule, label, log);
-
         InstanceConfiguration instanceConfiguration = instanceConfigurationBuilder()
-                .startupScript(NON_STANDARD_JAVA_STARTUP_SCRIPT)
+                .bootDiskSourceImageName(BOOT_DISK_IMAGE_NAME + "-non-standard-java")
                 .numExecutorsStr(NUM_EXECUTORS)
                 .labels(LABEL)
                 .oneShot(false)
@@ -112,11 +98,13 @@ public static void teardown() throws IOException {
         teardownResources(client, label, log);
     }
 
+    @WithTimeout(300)
     @Test
     public void testWorkerCreatedOnePlannedNode() {
         assertEquals(1, planned.size());
     }
 
+    @WithTimeout(300)
     @Test
     public void testInstanceStatusRunning() {
         assertEquals("RUNNING", instance.getStatus());