From 5496e067a091bc85bec1a7e57b89e159a9dd4b53 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Mon, 3 Dec 2018 14:59:36 +0100 Subject: [PATCH 01/30] One shot instances --- .../computeengine/ComputeEngineCloud.java | 40 ++++++++++++------- .../computeengine/ComputeEngineInstance.java | 10 +++-- .../computeengine/InstanceConfiguration.java | 23 ++++++++++- .../computeengine/client/ComputeClient.java | 5 ++- .../InstanceConfiguration/config.jelly | 3 ++ .../InstanceConfiguration/help-oneShot.html | 4 ++ .../computeengine/ComputeEngineCloudIT.java | 8 ++-- .../InstanceConfigurationTest.java | 7 +--- 8 files changed, 69 insertions(+), 31 deletions(-) create mode 100644 src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/help-oneShot.html diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java index edf158be..bdaa6e23 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java @@ -26,7 +26,12 @@ import com.google.jenkins.plugins.computeengine.client.ComputeClient; import com.google.jenkins.plugins.credentials.oauth.GoogleOAuth2Credentials; import hudson.Extension; -import hudson.model.*; +import hudson.model.Computer; +import hudson.model.Descriptor; +import hudson.model.Item; +import hudson.model.Label; +import hudson.model.Node; +import hudson.model.TaskListener; import hudson.security.ACL; import hudson.slaves.AbstractCloudImpl; import hudson.slaves.Cloud; @@ -45,8 +50,14 @@ import javax.servlet.ServletException; import java.io.IOException; import java.io.PrintStream; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; @@ -162,21 +173,20 @@ public Collection provision(Label label, int excessWorkload) { public Node call() throws Exception { long startTime = System.currentTimeMillis(); LOGGER.log(Level.INFO, String.format("Waiting %dms for node %s to connect", config.getLaunchTimeoutMillis(), node.getNodeName())); - while ((System.currentTimeMillis() - startTime) < config.getLaunchTimeoutMillis()) { - LOGGER.log(Level.INFO, String.format("%dms elapsed waiting for node %s to connect", System.currentTimeMillis() - startTime, node.getNodeName())); - Computer c; - try { - c = node.toComputer(); - if (c != null) { - c.connect(false).get(); - } - } catch (Exception e) { - LOGGER.log(Level.WARNING, String.format("Exception waiting for node %s to connect", node.getNodeName()), e); + try { + Computer c = node.toComputer(); + if (c != null) { + c.connect(false).get(config.getLaunchTimeoutMillis(), TimeUnit.MILLISECONDS); + LOGGER.log(Level.INFO, String.format("%dms elapsed waiting for node %s to connect", System.currentTimeMillis() - startTime, node.getNodeName())); + return null; + } else { + LOGGER.log(Level.INFO, String.format("No computer for node %s found", node.getNodeName())); } - return node; + } catch (Exception e) { + LOGGER.log(Level.WARNING, String.format("Exception waiting for node %s to connect", node.getNodeName()), e); } - LOGGER.log(Level.WARNING, "Failed to connect to node within launch timeout"); - return node; + node.terminate(); + return null; } }), node.getNumExecutors())); excessWorkload -= 1; diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index b9c2a21d..102620b1 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -18,7 +18,6 @@ import hudson.Extension; import hudson.model.Descriptor; -import hudson.model.Node; import hudson.model.TaskListener; import hudson.slaves.*; import jenkins.model.Jenkins; @@ -32,8 +31,9 @@ public class ComputeEngineInstance extends AbstractCloudSlave { public final String zone; public final String cloudName; public final String sshUser; + public final boolean oneShot; + public boolean connected; public Integer launchTimeout; // Seconds - private Boolean connected; public ComputeEngineInstance(String cloudName, String name, @@ -42,11 +42,12 @@ public ComputeEngineInstance(String cloudName, String sshUser, String remoteFS, int numExecutors, - Node.Mode mode, + Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy, - Integer launchTimeout) + Integer launchTimeout, + boolean oneShot) throws Descriptor.FormException, IOException { super(name, nodeDescription, remoteFS, numExecutors, mode, labelString, launcher, retentionStrategy, Collections.>emptyList()); @@ -54,6 +55,7 @@ public ComputeEngineInstance(String cloudName, this.zone = zone; this.cloudName = cloudName; this.sshUser = sshUser; + this.oneShot = oneShot; } @Override diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java index c753bcc5..da324851 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java @@ -31,6 +31,7 @@ import hudson.util.ListBoxModel; import jenkins.model.Jenkins; import org.apache.commons.text.RandomStringGenerator; +import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; @@ -84,6 +85,7 @@ public class InstanceConfiguration implements Describable public final String retentionTimeMinutesStr; public final String launchTimeoutSecondsStr; public final String bootDiskSizeGbStr; + public final boolean oneShot; public Map googleLabels; public Integer numExecutors; public Integer retentionTimeMinutes; @@ -117,7 +119,8 @@ public InstanceConfiguration(String namePrefix, String launchTimeoutSecondsStr, Node.Mode mode, AcceleratorConfiguration acceleratorConfiguration, - String runAsUser) { + String runAsUser, + boolean oneShot) { this.namePrefix = namePrefix; this.region = region; this.zone = zone; @@ -127,6 +130,7 @@ public InstanceConfiguration(String namePrefix, this.preemptible = preemptible; this.minCpuPlatform = minCpuPlatform; this.numExecutors = intOrDefault(numExecutorsStr, DEFAULT_NUM_EXECUTORS); + this.oneShot = oneShot; this.numExecutorsStr = numExecutors.toString(); this.retentionTimeMinutes = intOrDefault(retentionTimeMinutesStr, DEFAULT_RETENTION_TIME_MINUTES); this.retentionTimeMinutesStr = retentionTimeMinutes.toString(); @@ -234,7 +238,22 @@ public ComputeEngineInstance provision(TaskListener listener, Label requiredLabe Instance i = instance(); Operation operation = cloud.client.insertInstance(cloud.projectId, i); logger.println("Sent insert request"); - ComputeEngineInstance instance = new ComputeEngineInstance(cloud.name, i.getName(), i.getZone(), i.getDescription(), runAsUser, "./.jenkins-slave", numExecutors, mode, requiredLabel == null ? "" : requiredLabel.getName(), new ComputeEngineLinuxLauncher(cloud.getCloudName(), operation, this.useInternalAddress), new CloudRetentionStrategy(retentionTimeMinutes), getLaunchTimeoutMillis()); + ComputeEngineComputerLauncher launcher = new ComputeEngineLinuxLauncher(cloud.getCloudName(), operation, this.useInternalAddress); + + ComputeEngineInstance instance = new ComputeEngineInstance( + cloud.name, + i.getName(), + i.getZone(), + i.getDescription(), + runAsUser, + "./.jenkins-slave", + numExecutors, + mode, + requiredLabel == null ? "" : requiredLabel.getName(), + launcher, + (oneShot ? new OnceRetentionStrategy(retentionTimeMinutes) : new CloudRetentionStrategy(retentionTimeMinutes)), + getLaunchTimeoutMillis(), + oneShot); return instance; } catch (Descriptor.FormException fe) { logger.printf("Error provisioning instance: %s", fe.getMessage()); diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java index 3e6a72c8..f38370fa 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java @@ -22,6 +22,8 @@ import java.io.IOException; import java.util.*; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Client for communicating with the Google Compute API @@ -29,6 +31,7 @@ * @see Cloud Engine */ public class ComputeClient { + private static final Logger LOGGER = Logger.getLogger(ComputeClient.class.getName()); private Compute compute; public static String zoneFromSelfLink(String zoneSelfLink) { @@ -449,7 +452,7 @@ public Operation.Error waitForOperationCompletion(String projectId, String opera if (elapsed >= timeout) { throw new InterruptedException("Timed out waiting for operation to complete"); } - System.out.println("waiting..."); + LOGGER.log(Level.FINE, "Waiting for operation " + operationId + " to complete.."); if (zone != null) { Compute.ZoneOperations.Get get = compute.zoneOperations().get(projectId, zone, operationId); operation = get.execute(); diff --git a/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/config.jelly b/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/config.jelly index 1036649f..96b97dee 100644 --- a/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/config.jelly +++ b/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/config.jelly @@ -15,6 +15,9 @@ + + + diff --git a/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/help-oneShot.html b/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/help-oneShot.html new file mode 100644 index 00000000..e65d692b --- /dev/null +++ b/src/main/resources/com/google/jenkins/plugins/computeengine/InstanceConfiguration/help-oneShot.html @@ -0,0 +1,4 @@ +
+ Create One Shot instance. + Instances like that will be created before each request and deleted after each one. +
\ No newline at end of file diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java index df22e0ab..9a7562f7 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java @@ -37,8 +37,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.*; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.logging.LogManager; import java.util.logging.Logger; import java.util.logging.SimpleFormatter; @@ -263,7 +261,8 @@ private static InstanceConfiguration validInstanceConfiguration1() { LAUNCH_TIMEOUT_SECONDS_STR, NODE_MODE, new AcceleratorConfiguration(ACCELERATOR_NAME, ACCELERATOR_COUNT), - RUN_AS_USER); + RUN_AS_USER, + false); ic.appendLabels(INTEGRATION_LABEL); return ic; } @@ -299,7 +298,8 @@ private static InstanceConfiguration invalidInstanceConfiguration1() { LAUNCH_TIMEOUT_SECONDS_STR, NODE_MODE, new AcceleratorConfiguration(ACCELERATOR_NAME, ACCELERATOR_COUNT), - RUN_AS_USER); + RUN_AS_USER, + false); ic.appendLabels(INTEGRATION_LABEL); return ic; } diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/InstanceConfigurationTest.java b/src/test/java/com/google/jenkins/plugins/computeengine/InstanceConfigurationTest.java index a6f07fbb..6f9b5256 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/InstanceConfigurationTest.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/InstanceConfigurationTest.java @@ -17,13 +17,9 @@ package com.google.jenkins.plugins.computeengine; import com.google.api.services.compute.model.*; -import com.google.jenkins.plugins.computeengine.AcceleratorConfiguration; -import com.google.jenkins.plugins.computeengine.ComputeEngineCloud; -import com.google.jenkins.plugins.computeengine.InstanceConfiguration; import com.google.jenkins.plugins.computeengine.client.ComputeClient; import hudson.model.Node; import hudson.util.FormValidation; -import jenkins.model.Jenkins; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -239,7 +235,8 @@ public static InstanceConfiguration instanceConfiguration(String minCpuPlatform LAUNCH_TIMEOUT_SECONDS_STR, NODE_MODE, new AcceleratorConfiguration(ACCELERATOR_NAME, ACCELERATOR_COUNT), - RUN_AS_USER); + RUN_AS_USER, + false); } @Test From c60051850cce03f14759411eb73f88ef0f47bb42 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Mon, 3 Dec 2018 15:13:53 +0100 Subject: [PATCH 02/30] Fix merge commit --- pom.xml | 5 +++++ .../plugins/computeengine/ComputeEngineInstance.java | 7 ++----- .../plugins/computeengine/InstanceConfiguration.java | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 98a37f84..20f3a248 100644 --- a/pom.xml +++ b/pom.xml @@ -90,6 +90,11 @@ trilead-ssh2 build-217-jenkins-11 + + org.jenkins-ci.plugins + durable-task + 1.28 + junit junit diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 102620b1..9091757a 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -31,9 +31,8 @@ public class ComputeEngineInstance extends AbstractCloudSlave { public final String zone; public final String cloudName; public final String sshUser; - public final boolean oneShot; - public boolean connected; public Integer launchTimeout; // Seconds + public Boolean connected; public ComputeEngineInstance(String cloudName, String name, @@ -46,8 +45,7 @@ public ComputeEngineInstance(String cloudName, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy, - Integer launchTimeout, - boolean oneShot) + Integer launchTimeout) throws Descriptor.FormException, IOException { super(name, nodeDescription, remoteFS, numExecutors, mode, labelString, launcher, retentionStrategy, Collections.>emptyList()); @@ -55,7 +53,6 @@ public ComputeEngineInstance(String cloudName, this.zone = zone; this.cloudName = cloudName; this.sshUser = sshUser; - this.oneShot = oneShot; } @Override diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java index da324851..939ec2a2 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java @@ -252,8 +252,8 @@ public ComputeEngineInstance provision(TaskListener listener, Label requiredLabe requiredLabel == null ? "" : requiredLabel.getName(), launcher, (oneShot ? new OnceRetentionStrategy(retentionTimeMinutes) : new CloudRetentionStrategy(retentionTimeMinutes)), - getLaunchTimeoutMillis(), - oneShot); + getLaunchTimeoutMillis() + ); return instance; } catch (Descriptor.FormException fe) { logger.printf("Error provisioning instance: %s", fe.getMessage()); From 8f877a54791e818ff3e7f3d2cff3ee34f02aeada Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sat, 22 Dec 2018 14:03:25 +0100 Subject: [PATCH 03/30] Cleaning lost nodes job --- .../computeengine/CleanLostNodesWork.java | 110 ++++++++++++++++++ .../computeengine/CloudNotFoundException.java | 2 +- .../computeengine/ComputeEngineCloud.java | 17 ++- .../computeengine/ComputeEngineComputer.java | 2 +- .../computeengine/ComputeEngineInstance.java | 6 +- .../ComputeEngineLinuxLauncher.java | 2 +- .../computeengine/CleanLostNodesWorkTest.java | 93 +++++++++++++++ .../computeengine/ComputeEngineCloudIT.java | 2 +- .../ComputeEngineCloudWindowsIT.java | 2 +- 9 files changed, 228 insertions(+), 8 deletions(-) create mode 100644 src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java create mode 100644 src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java new file mode 100644 index 00000000..d174f985 --- /dev/null +++ b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java @@ -0,0 +1,110 @@ +/* + * Copyright 2018 Google Inc. All Rights Reserved. + * + * 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. + */ + +package com.google.jenkins.plugins.computeengine; + +import com.google.api.services.compute.model.Instance; +import hudson.Extension; +import hudson.model.PeriodicWork; +import hudson.model.Slave; +import jenkins.model.Jenkins; +import org.jenkinsci.Symbol; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +import static com.google.jenkins.plugins.computeengine.ComputeEngineCloud.CLOUD_ID_LABEL_KEY; +import static java.util.Collections.emptyList; + +/** + * Periodically checks if there are no lost nodes in GCP. + * If it finds any they are deleted. + */ +@Extension +@Symbol("cleanLostNodesWork") +public class CleanLostNodesWork extends PeriodicWork { + protected final Logger logger = Logger.getLogger(getClass().getName()); + + public long getRecurrencePeriod() { + return MIN; + } + + /** + * {@inheritDoc} + */ + @SuppressWarnings("unchecked") + @Override + protected void doRun() { + logger.log(Level.FINEST, "Starting clean lost nodes worker"); + getClouds().forEach(this::cleanCloud); + } + + private void cleanCloud(ComputeEngineCloud cloud) { + logger.log(Level.FINEST, "Cleaning cloud " + cloud.getCloudName()); + List remoteInstances = findRemoteInstances(cloud); + Set localInstances = findLocalInstances(cloud); + remoteInstances.forEach(remote -> checkOneInstance(remote, localInstances, cloud)); + } + + private void checkOneInstance(Instance remote, Set localInstances, ComputeEngineCloud cloud) { + String instanceName = remote.getName(); + logger.log(Level.FINEST, "Checking instance " + instanceName); + if (!localInstances.contains(instanceName)) { + logger.log(Level.INFO, "Remote instance " + instanceName + " not found locally, removing it"); + try { + cloud.getClient().terminateInstance(cloud.getProjectId(), remote.getZone(), instanceName); + } catch (IOException ex) { + logger.log(Level.WARNING, "Error terminating remote instance " + instanceName, ex); + } + } + } + + private List getClouds() { + return Jenkins.getInstance().clouds + .stream() + .filter(cloud -> cloud instanceof ComputeEngineCloud) + .map(cloud -> (ComputeEngineCloud) cloud) + .collect(Collectors.toList()); + } + + private Set findLocalInstances(ComputeEngineCloud cloud) { + return Jenkins.getInstance() + .getNodes() + .stream() + .filter(node -> node instanceof ComputeEngineInstance) + .map(node -> (ComputeEngineInstance) node) + .filter(node -> node.getCloud().equals(cloud)) + .map(Slave::getNodeName) + .collect(Collectors.toSet()); + } + + private List findRemoteInstances(ComputeEngineCloud cloud) { + Map filterLabel = new HashMap<>(); + filterLabel.put(CLOUD_ID_LABEL_KEY, cloud.getInstanceUniqueId()); + try { + return cloud.getClient().getInstancesWithLabel(cloud.getProjectId(), filterLabel); + } catch (IOException ex) { + logger.log(Level.WARNING, "Error finding remote instances", ex); + return emptyList(); + } + } +} diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/CloudNotFoundException.java b/src/main/java/com/google/jenkins/plugins/computeengine/CloudNotFoundException.java index 147a5ca1..9a4785af 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/CloudNotFoundException.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/CloudNotFoundException.java @@ -16,7 +16,7 @@ package com.google.jenkins.plugins.computeengine; -public class CloudNotFoundException extends Exception { +public class CloudNotFoundException extends RuntimeException { public CloudNotFoundException(String message) { super(message); } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java index b4fbfb7c..7417c3e9 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java @@ -129,7 +129,7 @@ protected Object readResolve() { c.cloud = this; // Apply a label that associates an instance configuration with // this cloud provider - c.appendLabel(CLOUD_ID_LABEL_KEY, String.valueOf(this.name.hashCode())); + c.appendLabel(CLOUD_ID_LABEL_KEY, getInstanceUniqueId()); // Apply a label that identifies the name of this instance configuration c.appendLabel(CONFIG_LABEL_KEY, c.namePrefix); @@ -137,6 +137,19 @@ protected Object readResolve() { return this; } + public String getInstanceUniqueId() { + // Semi unique ID + return String.valueOf(name.hashCode()); + } + + public ComputeClient getClient() { + return client; + } + + public String getProjectId() { + return projectId; + } + public void addConfiguration(InstanceConfiguration config) { configurations.add(config); readResolve(); @@ -200,7 +213,7 @@ private synchronized Integer availableNodeCapacity() throws IOException { // We only care about instances that have a label indicating they // belong to this cloud Map filterLabel = new HashMap<>(); - filterLabel.put(CLOUD_ID_LABEL_KEY, String.valueOf(this.name.hashCode())); + filterLabel.put(CLOUD_ID_LABEL_KEY, getInstanceUniqueId()); List instances = client.getInstancesWithLabel(projectId, filterLabel); // Don't count instances that are not running (or starting up) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java index 53cb7d5c..ba67a4dd 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java @@ -100,7 +100,7 @@ private Instance _getInstance() throws IOException { } } - protected ComputeEngineCloud getCloud() throws CloudNotFoundException { + protected ComputeEngineCloud getCloud() { ComputeEngineInstance node = getNode(); if (node == null) throw new CloudNotFoundException("Could not retrieve cloud from empty node"); diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 4a8c0905..972ffa49 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -80,6 +80,10 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted } + + public String getCloudName() { + return cloudName; + } public void onConnected() { this.connected = true; @@ -93,7 +97,7 @@ public long getLaunchTimeoutMillis() { return launchTimeout * 1000L; } - public ComputeEngineCloud getCloud() throws CloudNotFoundException { + public ComputeEngineCloud getCloud() { ComputeEngineCloud cloud = (ComputeEngineCloud) Jenkins.getInstance().getCloud(cloudName); if (cloud == null) throw new CloudNotFoundException(String.format("Could not find cloud %s in Jenkins configuration", cloudName)); diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java index eb563d52..a031710a 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java @@ -146,7 +146,7 @@ private boolean testCommand(ComputeEngineComputer computer, Connection conn, Str } - private GoogleKeyPair setupSshKeys(ComputeEngineComputer computer) throws CloudNotFoundException, IOException, InterruptedException { + private GoogleKeyPair setupSshKeys(ComputeEngineComputer computer) throws IOException, InterruptedException { if (computer == null) { throw new IllegalArgumentException("A null ComputeEngineComputer was provided"); } diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java b/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java new file mode 100644 index 00000000..3a5c88ca --- /dev/null +++ b/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java @@ -0,0 +1,93 @@ +package com.google.jenkins.plugins.computeengine; + +import com.google.api.services.compute.model.Instance; +import com.google.jenkins.plugins.computeengine.client.ComputeClient; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.logging.Level; +import java.util.logging.LogManager; +import java.util.logging.Logger; + +import static com.google.common.collect.ImmutableList.of; +import static org.junit.Assert.assertNotNull; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.anyMap; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class CleanLostNodesWorkTest { + + private static final String TEST_PROJECT_ID = "test_project_id"; + + @Rule + public JenkinsRule r = new JenkinsRule(); + + @Mock + public ComputeEngineCloud cloud; + + @Mock + public ComputeClient client; + + private CleanLostNodesWork getWorker() { + return r.jenkins.getExtensionList(CleanLostNodesWork.class).get(0); + } + + @Before + public void setup() { + when(cloud.getClient()).thenReturn(client); + when(cloud.getProjectId()).thenReturn(TEST_PROJECT_ID); + when(cloud.getInstanceUniqueId()).thenReturn("234234355"); + } + + @Test + public void shouldRegisterCleanNodeWorker() { + assertNotNull(getWorker()); + } + + @Test + public void shouldRunWithoutClouds() { + getWorker().doRun(); + } + + @Test + public void shouldNotCleanAnyInstance() throws Exception { + final String instanceName = "inst-1"; + Instance remoteInstance = new Instance().setName(instanceName); + when(client.getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap())).thenReturn(of(remoteInstance)); + + ComputeEngineInstance localInstance = Mockito.mock(ComputeEngineInstance.class); + when(localInstance.getCloud()).thenReturn(cloud); + when(localInstance.getNodeName()).thenReturn(instanceName); + when(localInstance.getNumExecutors()).thenReturn(0); + + r.jenkins.clouds.add(cloud); + r.jenkins.addNode(localInstance); + + getWorker().doRun(); + verify(client).getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap()); + verifyZeroInteractions(client); + } + + @Test + public void shouldCleanLostInstance() throws Exception { + final String instanceName = "inst-2"; + final String zone = "test-zone"; + Instance remoteInstance = new Instance().setName(instanceName).setZone(zone); + when(client.getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap())).thenReturn(of(remoteInstance)); + + r.jenkins.clouds.add(cloud); + + getWorker().doRun(); + verify(client).getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap()); + verify(client).terminateInstance(eq(TEST_PROJECT_ID), eq(zone), eq(instanceName)); + } +} diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java index 36586d4c..8741bc7d 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java @@ -216,7 +216,7 @@ public void testWorkerCreated() throws Exception { // Instance should have a label with key CONFIG_LABEL_KEY and value equal to the config's name prefix assertEquals(logs(), ic.namePrefix, i.getLabels().get(ComputeEngineCloud.CONFIG_LABEL_KEY)); - assertEquals(logs(), String.valueOf(cloud.name.hashCode()), i.getLabels().get(ComputeEngineCloud.CLOUD_ID_LABEL_KEY)); + assertEquals(logs(), cloud.getInstanceUniqueId(), i.getLabels().get(ComputeEngineCloud.CLOUD_ID_LABEL_KEY)); } @Test(timeout = 300000) diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java index ec761d2e..689bfa0f 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java @@ -202,7 +202,7 @@ public void testWorkerCreated() throws Exception { // Instance should have a label with key CONFIG_LABEL_KEY and value equal to the config's name prefix assertEquals(logs(), validInstanceConfiguration1().namePrefix, i.getLabels().get(ComputeEngineCloud.CONFIG_LABEL_KEY)); // proper id label to properly count instances - assertEquals(logs(), String.valueOf(cloud.name.hashCode()), i.getLabels().get(ComputeEngineCloud.CLOUD_ID_LABEL_KEY)); + assertEquals(logs(), cloud.getInstanceUniqueId(), i.getLabels().get(ComputeEngineCloud.CLOUD_ID_LABEL_KEY)); } private static InstanceConfiguration validInstanceConfiguration1() throws Exception{ From bdb265fd27e991a549c3bb9323f3d68c8663fb10 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Mon, 24 Dec 2018 13:55:05 +0100 Subject: [PATCH 04/30] Do not try stop already stopping instances --- .../computeengine/CleanLostNodesWork.java | 12 ++++++++-- .../computeengine/CleanLostNodesWorkTest.java | 22 ++++++++++++++----- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java index d174f985..1c1d4a23 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java @@ -45,7 +45,7 @@ public class CleanLostNodesWork extends PeriodicWork { protected final Logger logger = Logger.getLogger(getClass().getName()); public long getRecurrencePeriod() { - return MIN; + return HOUR; } /** @@ -101,10 +101,18 @@ private List findRemoteInstances(ComputeEngineCloud cloud) { Map filterLabel = new HashMap<>(); filterLabel.put(CLOUD_ID_LABEL_KEY, cloud.getInstanceUniqueId()); try { - return cloud.getClient().getInstancesWithLabel(cloud.getProjectId(), filterLabel); + return cloud.getClient() + .getInstancesWithLabel(cloud.getProjectId(), filterLabel) + .stream() + .filter(instance -> shouldTerminateStatus(instance.getStatus())) + .collect(Collectors.toList()); } catch (IOException ex) { logger.log(Level.WARNING, "Error finding remote instances", ex); return emptyList(); } } + + private boolean shouldTerminateStatus(String status) { + return !status.equals("STOPPING"); + } } diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java b/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java index 3a5c88ca..939a03f5 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java @@ -11,10 +11,6 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; -import java.util.logging.Level; -import java.util.logging.LogManager; -import java.util.logging.Logger; - import static com.google.common.collect.ImmutableList.of; import static org.junit.Assert.assertNotNull; import static org.mockito.ArgumentMatchers.eq; @@ -61,7 +57,7 @@ public void shouldRunWithoutClouds() { @Test public void shouldNotCleanAnyInstance() throws Exception { final String instanceName = "inst-1"; - Instance remoteInstance = new Instance().setName(instanceName); + Instance remoteInstance = new Instance().setName(instanceName).setStatus("RUNNING"); when(client.getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap())).thenReturn(of(remoteInstance)); ComputeEngineInstance localInstance = Mockito.mock(ComputeEngineInstance.class); @@ -81,7 +77,7 @@ public void shouldNotCleanAnyInstance() throws Exception { public void shouldCleanLostInstance() throws Exception { final String instanceName = "inst-2"; final String zone = "test-zone"; - Instance remoteInstance = new Instance().setName(instanceName).setZone(zone); + Instance remoteInstance = new Instance().setName(instanceName).setZone(zone).setStatus("RUNNING"); when(client.getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap())).thenReturn(of(remoteInstance)); r.jenkins.clouds.add(cloud); @@ -90,4 +86,18 @@ public void shouldCleanLostInstance() throws Exception { verify(client).getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap()); verify(client).terminateInstance(eq(TEST_PROJECT_ID), eq(zone), eq(instanceName)); } + + @Test + public void shouldNotCleanStoppingInstance() throws Exception { + final String instanceName = "inst-2"; + final String zone = "test-zone"; + Instance remoteInstance = new Instance().setName(instanceName).setZone(zone).setStatus("STOPPING"); + when(client.getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap())).thenReturn(of(remoteInstance)); + + r.jenkins.clouds.add(cloud); + + getWorker().doRun(); + verify(client).getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap()); + verifyZeroInteractions(client); + } } From 6a7ccf82969a412ba22da3b62208a644cc6bdd40 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Thu, 27 Dec 2018 18:27:24 +0100 Subject: [PATCH 05/30] Dont kill terminated --- .../plugins/computeengine/CleanLostNodesWork.java | 2 +- .../computeengine/CleanLostNodesWorkTest.java | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java index 1c1d4a23..5706f8e2 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java @@ -113,6 +113,6 @@ private List findRemoteInstances(ComputeEngineCloud cloud) { } private boolean shouldTerminateStatus(String status) { - return !status.equals("STOPPING"); + return !status.equals("STOPPING") && !status.equals("TERMINATED"); } } diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java b/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java index 939a03f5..e4346fd2 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWorkTest.java @@ -100,4 +100,18 @@ public void shouldNotCleanStoppingInstance() throws Exception { verify(client).getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap()); verifyZeroInteractions(client); } + + @Test + public void shouldNotCleanTerminatedInstance() throws Exception { + final String instanceName = "inst-2"; + final String zone = "test-zone"; + Instance remoteInstance = new Instance().setName(instanceName).setZone(zone).setStatus("TERMINATED"); + when(client.getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap())).thenReturn(of(remoteInstance)); + + r.jenkins.clouds.add(cloud); + + getWorker().doRun(); + verify(client).getInstancesWithLabel(eq(TEST_PROJECT_ID), anyMap()); + verifyZeroInteractions(client); + } } From e5545af5a0b559ab371dffe9d0cbb24d45067bde Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 2 Jan 2019 23:11:34 +0100 Subject: [PATCH 06/30] Post review changes --- .../computeengine/CleanLostNodesWork.java | 44 ++++++++++++------- .../computeengine/ComputeEngineCloud.java | 9 ++++ .../computeengine/ComputeEngineInstance.java | 2 +- .../ComputeEngineLinuxLauncher.java | 2 +- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java index 5706f8e2..605c6c60 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/CleanLostNodesWork.java @@ -24,7 +24,6 @@ import org.jenkinsci.Symbol; import java.io.IOException; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -32,6 +31,7 @@ import java.util.logging.Logger; import java.util.stream.Collectors; +import static com.google.common.collect.ImmutableMap.of; import static com.google.jenkins.plugins.computeengine.ComputeEngineCloud.CLOUD_ID_LABEL_KEY; import static java.util.Collections.emptyList; @@ -44,6 +44,11 @@ public class CleanLostNodesWork extends PeriodicWork { protected final Logger logger = Logger.getLogger(getClass().getName()); + /** + * {@inheritDoc} + */ + @SuppressWarnings("unchecked") + @Override public long getRecurrencePeriod() { return HOUR; } @@ -59,22 +64,32 @@ protected void doRun() { } private void cleanCloud(ComputeEngineCloud cloud) { - logger.log(Level.FINEST, "Cleaning cloud " + cloud.getCloudName()); - List remoteInstances = findRemoteInstances(cloud); - Set localInstances = findLocalInstances(cloud); - remoteInstances.forEach(remote -> checkOneInstance(remote, localInstances, cloud)); + try { + logger.log(Level.FINEST, "Cleaning cloud " + cloud.getCloudName()); + List remoteInstances = findRemoteInstances(cloud); + Set localInstances = findLocalInstances(cloud); + remoteInstances + .stream() + .filter(remote -> isOrphaned(remote, localInstances)) + .forEach(remote -> terminateInstance(remote, cloud)); + } catch (Exception ex) { + logger.log(Level.WARNING, "Error cleaning cloud " + cloud.getCloudName(), ex); + } } - private void checkOneInstance(Instance remote, Set localInstances, ComputeEngineCloud cloud) { + private boolean isOrphaned(Instance remote, Set localInstances) { String instanceName = remote.getName(); logger.log(Level.FINEST, "Checking instance " + instanceName); - if (!localInstances.contains(instanceName)) { - logger.log(Level.INFO, "Remote instance " + instanceName + " not found locally, removing it"); - try { - cloud.getClient().terminateInstance(cloud.getProjectId(), remote.getZone(), instanceName); - } catch (IOException ex) { - logger.log(Level.WARNING, "Error terminating remote instance " + instanceName, ex); - } + return !localInstances.contains(instanceName); + } + + private void terminateInstance(Instance remote, ComputeEngineCloud cloud) { + String instanceName = remote.getName(); + logger.log(Level.INFO, "Remote instance " + instanceName + " not found locally, removing it"); + try { + cloud.getClient().terminateInstance(cloud.getProjectId(), remote.getZone(), instanceName); + } catch (IOException ex) { + logger.log(Level.WARNING, "Error terminating remote instance " + instanceName, ex); } } @@ -98,8 +113,7 @@ private Set findLocalInstances(ComputeEngineCloud cloud) { } private List findRemoteInstances(ComputeEngineCloud cloud) { - Map filterLabel = new HashMap<>(); - filterLabel.put(CLOUD_ID_LABEL_KEY, cloud.getInstanceUniqueId()); + Map filterLabel = of(CLOUD_ID_LABEL_KEY, cloud.getInstanceUniqueId()); try { return cloud.getClient() .getInstancesWithLabel(cloud.getProjectId(), filterLabel) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java index 7417c3e9..0311c8a8 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java @@ -137,6 +137,15 @@ protected Object readResolve() { return this; } + + /** + * Returns unique ID of that cloud instance. + * + * This ID allows us to find machines from our cloud in GCP. + * Current implementation is bit naive and generate ID based on name hashCode. + * + * @return instance unique ID + */ public String getInstanceUniqueId() { // Semi unique ID return String.valueOf(name.hashCode()); diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 972ffa49..de8150c8 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -97,7 +97,7 @@ public long getLaunchTimeoutMillis() { return launchTimeout * 1000L; } - public ComputeEngineCloud getCloud() { + public ComputeEngineCloud getCloud() throws CloudNotFoundException { ComputeEngineCloud cloud = (ComputeEngineCloud) Jenkins.getInstance().getCloud(cloudName); if (cloud == null) throw new CloudNotFoundException(String.format("Could not find cloud %s in Jenkins configuration", cloudName)); diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java index a031710a..eb563d52 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineLinuxLauncher.java @@ -146,7 +146,7 @@ private boolean testCommand(ComputeEngineComputer computer, Connection conn, Str } - private GoogleKeyPair setupSshKeys(ComputeEngineComputer computer) throws IOException, InterruptedException { + private GoogleKeyPair setupSshKeys(ComputeEngineComputer computer) throws CloudNotFoundException, IOException, InterruptedException { if (computer == null) { throw new IllegalArgumentException("A null ComputeEngineComputer was provided"); } From ade662268f3bfffeb86d1db316c37fe8206eb208 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Thu, 3 Jan 2019 21:35:32 +0100 Subject: [PATCH 07/30] Javadoc for added methods --- .../plugins/computeengine/ComputeEngineCloud.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java index 0311c8a8..c1d921c1 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java @@ -150,11 +150,21 @@ public String getInstanceUniqueId() { // Semi unique ID return String.valueOf(name.hashCode()); } - + + /** + * Returns GCP client for that cloud. + * + * @return GCP client object. + */ public ComputeClient getClient() { return client; } + /** + * Returns GCP projectId for that cloud. + * + * @return projectId + */ public String getProjectId() { return projectId; } From f3a46f26346da1a0153f573777d28f340fd80483 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 9 Jan 2019 13:57:46 +0100 Subject: [PATCH 08/30] Tests for One shot instances --- pom.xml | 8 +++- .../computeengine/ComputeEngineCloudIT.java | 48 +++++++++++++++---- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index 799d1a13..53b59278 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ 1.0.8-SNAPSHOT hpi - 2.7.3 + 2.150.1 480 8 1.8 @@ -139,6 +139,12 @@ true false + + 500 + 0.5 + 100 + 1.0 + **/ComputeEngineCloudWindowsIT.java diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java index 313eefd9..95e9555d 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java @@ -37,9 +37,13 @@ import com.google.jenkins.plugins.computeengine.client.ComputeClient; import com.google.jenkins.plugins.credentials.oauth.GoogleRobotPrivateKeyCredentials; import com.google.jenkins.plugins.credentials.oauth.ServiceAccountConfig; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; import hudson.model.Node; import hudson.model.labels.LabelAtom; import hudson.slaves.NodeProvisioner; +import hudson.tasks.Builder; +import hudson.tasks.Shell; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -294,7 +298,7 @@ public void testWorkerFailed() throws Exception { logOutput.reset(); ComputeEngineCloud cloud = (ComputeEngineCloud) r.jenkins.clouds.get(0); - cloud.addConfiguration(invalidInstanceConfiguration1()); + cloud.addConfiguration(invalidInstanceConfiguration()); // Add a new node Collection planned = cloud.provision(new LabelAtom(LABEL), 1); @@ -309,6 +313,29 @@ public void testWorkerFailed() throws Exception { assertEquals(logs(), true, logs().contains("WARNING")); } + @Test(timeout = 500000) + public void testOneShotInstances() throws Exception { + ComputeEngineCloud cloud = (ComputeEngineCloud) r.jenkins.clouds.get(0); + cloud.addConfiguration(validInstanceConfigurationWithOneShot()); + + // Assert that there is 0 nodes + assertTrue(r.jenkins.getNodes().isEmpty()); + + FreeStyleProject project = r.createFreeStyleProject(); + Builder step = new Shell("echo works"); + project.getBuildersList().add(step); + project.setAssignedLabel(new LabelAtom(LABEL)); + + // Enqueue a build of the project, wait for it to complete, and assert success + FreeStyleBuild build = r.buildAndAssertSuccess(project); + + // Assert that the console log contains the output we expect + r.assertLogContains("works", build); + + // Assert that there is 0 nodes after job finished + assertTrue(r.jenkins.getNodes().isEmpty()); + } + @Test(timeout = 300000) public void testTemplate() throws Exception { ComputeEngineCloud cloud = (ComputeEngineCloud) r.jenkins.clouds.get(0); @@ -367,31 +394,34 @@ public void testTemplate() throws Exception { private static InstanceConfiguration validInstanceConfiguration() { - return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, LABEL, NULL_TEMPLATE); + return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, LABEL, false, NULL_TEMPLATE); } private static InstanceConfiguration validInstanceConfigurationWithLabels(String labels) { - return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, labels, NULL_TEMPLATE); + return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, labels, false, NULL_TEMPLATE); } private static InstanceConfiguration validInstanceConfigurationWithExecutors(String numExecutors) { - return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, numExecutors, LABEL, NULL_TEMPLATE); + return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, numExecutors, LABEL, false, NULL_TEMPLATE); } private static InstanceConfiguration validInstanceConfigurationWithTemplate(String template) { - return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, LABEL, template); + return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, LABEL, false, template); } + private static InstanceConfiguration validInstanceConfigurationWithOneShot() { + return instanceConfiguration(DEB_JAVA_STARTUP_SCRIPT, NUM_EXECUTORS, LABEL, true, NULL_TEMPLATE); + } /** * This configuration creates an instance with no Java installed. * * @return */ - private static InstanceConfiguration invalidInstanceConfiguration1() { - return instanceConfiguration("", NUM_EXECUTORS, LABEL, NULL_TEMPLATE); + private static InstanceConfiguration invalidInstanceConfiguration() { + return instanceConfiguration("", NUM_EXECUTORS, LABEL, false, NULL_TEMPLATE); } - private static InstanceConfiguration instanceConfiguration(String startupScript, String numExecutors, String labels, String template) { + private static InstanceConfiguration instanceConfiguration(String startupScript, String numExecutors, String labels, boolean oneShot, String template) { InstanceConfiguration ic = new InstanceConfiguration( NAME_PREFIX, REGION, @@ -423,7 +453,7 @@ private static InstanceConfiguration instanceConfiguration(String startupScript, NODE_MODE, new AcceleratorConfiguration(ACCELERATOR_NAME, ACCELERATOR_COUNT), RUN_AS_USER, - false, + oneShot, template ); ic.appendLabels(INTEGRATION_LABEL); From 2fa432254ed428ba3890902f681f30780466e515 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 9 Jan 2019 23:53:06 +0100 Subject: [PATCH 09/30] Fix all tests --- pom.xml | 8 +++++- .../computeengine/ComputeEngineCloudIT.java | 27 ++++++++++++++----- .../ComputeEngineCloudWindowsIT.java | 4 ++- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index 53b59278..09725872 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ 1.0.8-SNAPSHOT hpi - 2.150.1 + 2.37 480 8 1.8 @@ -112,6 +112,12 @@ 4.12 test + + org.awaitility + awaitility + 3.0.0 + test + diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java index 95e9555d..708fcff7 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java @@ -44,6 +44,7 @@ import hudson.slaves.NodeProvisioner; import hudson.tasks.Builder; import hudson.tasks.Shell; +import org.awaitility.Awaitility; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -55,9 +56,11 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.logging.LogManager; import java.util.logging.Logger; import java.util.logging.SimpleFormatter; @@ -202,7 +205,6 @@ public static void teardown() throws Exception { public void after() { ComputeEngineCloud cloud = (ComputeEngineCloud) r.jenkins.clouds.get(0); cloud.configurations.clear(); - } @Test @@ -232,8 +234,10 @@ public void testWorkerCreated() throws Exception { // There should be a planned node assertEquals(logs(), 1, planned.size()); + String name = planned.iterator().next().displayName; + // Wait for the node creation to finish - String name = planned.iterator().next().future.get().getNodeName(); + planned.iterator().next().future.get(); // There should be no warning logs assertFalse(logs(), logs().contains("WARNING")); @@ -287,7 +291,11 @@ public void testMultipleLabelsInConfig() throws Exception { // Add a new node Collection planned = cloud.provision(new LabelAtom(LABEL), 1); - String provisionedLabels = planned.iterator().next().future.get().getLabelString(); + String name = planned.iterator().next().displayName; + + planned.iterator().next().future.get(); + + String provisionedLabels = r.jenkins.getNode(name).getLabelString(); // There should be a planned node TODO assertEquals(logs(), MULTIPLE_LABEL, provisionedLabels); } @@ -310,7 +318,7 @@ public void testWorkerFailed() throws Exception { planned.iterator().next().future.get(); // There should be warning logs - assertEquals(logs(), true, logs().contains("WARNING")); + assertTrue(logs(), logs().contains("WARNING")); } @Test(timeout = 500000) @@ -318,6 +326,8 @@ public void testOneShotInstances() throws Exception { ComputeEngineCloud cloud = (ComputeEngineCloud) r.jenkins.clouds.get(0); cloud.addConfiguration(validInstanceConfigurationWithOneShot()); + r.jenkins.getNodesObject().setNodes(Collections.emptyList()); + // Assert that there is 0 nodes assertTrue(r.jenkins.getNodes().isEmpty()); @@ -333,7 +343,7 @@ public void testOneShotInstances() throws Exception { r.assertLogContains("works", build); // Assert that there is 0 nodes after job finished - assertTrue(r.jenkins.getNodes().isEmpty()); + Awaitility.await().timeout(10, TimeUnit.SECONDS).until(() -> r.jenkins.getNodes().isEmpty()); } @Test(timeout = 300000) @@ -373,9 +383,11 @@ public void testTemplate() throws Exception { // There should be a planned node assertEquals(logs(), 1, planned.size()); - + + String name = planned.iterator().next().displayName; + // Wait for the node creation to finish - String name = planned.iterator().next().future.get().getNodeName(); + planned.iterator().next().future.get(); // There should be no warning logs assertFalse(logs(), logs().contains("WARNING")); @@ -469,6 +481,7 @@ private static void deleteIntegrationInstances(boolean waitForCompletion) throws private static void safeDelete(String instanceId, boolean waitForCompletion) { try { + System.out.printf("Deleting: " + instanceId); Operation op = client.terminateInstance(projectId, ZONE, instanceId); if (waitForCompletion) client.waitForOperationCompletion(projectId, op.getName(), op.getZone(), 3 * 60 * 1000); diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java index dc9f70ef..526864ee 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudWindowsIT.java @@ -188,8 +188,10 @@ public void testWorkerCreated() throws Exception { // There should be a planned node assertEquals(logs(), 1, planned.size()); + String name = planned.iterator().next().displayName; + // Wait for the node creation to finish - String name = planned.iterator().next().future.get().getNodeName(); + planned.iterator().next().future.get(); // There should be no warning logs assertEquals(logs(), false, logs().contains("WARNING")); From b265d5d78365803a16c45564ca7eb01850ce5735 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Thu, 10 Jan 2019 00:36:47 +0100 Subject: [PATCH 10/30] After review changes --- .../jenkins/plugins/computeengine/ComputeEngineCloud.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java index 97f5fe4d..81cdaec2 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloud.java @@ -184,8 +184,8 @@ public Node call() throws Exception { } } catch (Exception e) { LOGGER.log(Level.WARNING, String.format("Exception waiting for node %s to connect", node.getNodeName()), e); + node.terminate(); } - node.terminate(); return null; } }), node.getNumExecutors())); From 6aec8660ef7a6e2e1e391623a0eefa52d8c4b1a8 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Thu, 10 Jan 2019 13:05:11 +0100 Subject: [PATCH 11/30] Delete preemptied instances --- .../plugins/computeengine/ComputeEngineInstance.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 3b803390..1b658392 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -66,18 +66,15 @@ public AbstractCloudComputer createComputer() { } @Override - protected void _terminate(TaskListener listener) throws IOException, InterruptedException { + protected void _terminate(TaskListener listener) throws IOException { try { ComputeEngineCloud cloud = getCloud(); // If the instance is running, attempt to terminate it. This is an asynch call and we // return immediately, hoping for the best. - cloud.client.terminateInstanceWithStatus(cloud.projectId, zone, name, "RUNNING"); + cloud.getClient().terminateInstance(cloud.projectId, zone, name); } catch (CloudNotFoundException cnfe) { listener.error(cnfe.getMessage()); - return; } - - } public String getCloudName() { From 80c9be24bb0644f58e1183f199f1ac451c6894cf Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Thu, 10 Jan 2019 14:42:38 +0100 Subject: [PATCH 12/30] Add restarting tasks in case of preemption --- pom.xml | 2 +- .../computeengine/ComputeEngineInstance.java | 2 +- .../ComputeEngineRetentionStrategy.java | 77 +++++++++++++++++++ .../computeengine/InstanceConfiguration.java | 2 +- 4 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java diff --git a/pom.xml b/pom.xml index 09725872..0ce15541 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ 1.0.8-SNAPSHOT hpi - 2.37 + 2.150.1 480 8 1.8 diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 1b658392..b5019b85 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -71,7 +71,7 @@ protected void _terminate(TaskListener listener) throws IOException { ComputeEngineCloud cloud = getCloud(); // If the instance is running, attempt to terminate it. This is an asynch call and we // return immediately, hoping for the best. - cloud.getClient().terminateInstance(cloud.projectId, zone, name); + cloud.getClient().terminateInstance(cloud.getProjectId(), zone, name); } catch (CloudNotFoundException cnfe) { listener.error(cnfe.getMessage()); } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java new file mode 100644 index 00000000..17c12ae7 --- /dev/null +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -0,0 +1,77 @@ +package com.google.jenkins.plugins.computeengine; + +import hudson.model.Executor; +import hudson.model.ExecutorListener; +import hudson.model.Queue; +import hudson.slaves.RetentionStrategy; +import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; + +import javax.annotation.Nonnull; +import javax.imageio.IIOException; +import java.io.IOException; +import java.util.logging.Level; +import java.util.logging.Logger; + +public class ComputeEngineRetentionStrategy extends RetentionStrategy implements ExecutorListener { + + private static final Logger LOGGER = Logger.getLogger(ComputeEngineRetentionStrategy.class.getName()); + + private final OnceRetentionStrategy delegate; + private final boolean oneShot; + + /** + * Creates the retention strategy. + * + * @param retentionTimeMinutes number of minutes of idleness after which to kill the slave; serves a backup in case the strategy fails to detect the end of a task + * @param oneShot create one shot instance strategy + */ + ComputeEngineRetentionStrategy(int retentionTimeMinutes, boolean oneShot) { + this.oneShot = oneShot; + delegate = new OnceRetentionStrategy(retentionTimeMinutes); + } + + @Override + public long check(@Nonnull ComputeEngineComputer c) { + return delegate.check(c); + } + + @Override + public void start(@Nonnull ComputeEngineComputer c) { + delegate.start(c); + } + + @Override + public void taskAccepted(Executor executor, Queue.Task task) { + LOGGER.log(Level.INFO, "Task accepted " + task); + if (oneShot) { + delegate.taskAccepted(executor, task); + } + } + + @Override + public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { + if (oneShot) { + delegate.taskCompleted(executor, task, durationMS); + } + } + + @Override + public void taskCompletedWithProblems(Executor executor, Queue.Task task, long durationMS, Throwable problems) { + if (oneShot) { + delegate.taskCompletedWithProblems(executor, task, durationMS, problems); + } + LOGGER.log(Level.INFO, "Task completed with problem " + task, problems); + try { + LOGGER.log(Level.INFO, "Trying to rerun task"); + task.getOwnerTask().getOwnerTask().createExecutable().run(); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Task rerunning error", e); + } + } + + private void checkPreemptive(Executor executor) { + final ComputeEngineComputer computer = (ComputeEngineComputer) executor.getOwner(); +// computer.getCloud().getClient(). + } + +} diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java index dd2ef5f8..d0c0a30c 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java @@ -315,7 +315,7 @@ public ComputeEngineInstance provision(TaskListener listener) throws IOException mode, labels, launcher, - (oneShot ? new OnceRetentionStrategy(retentionTimeMinutes) : new CloudRetentionStrategy(retentionTimeMinutes)), + new ComputeEngineRetentionStrategy(retentionTimeMinutes, oneShot), getLaunchTimeoutMillis()); return computeEngineInstance; } catch (Descriptor.FormException fe) { From 70ea6c557639b50b7c2e7e18077f80fe23ccf022 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Fri, 11 Jan 2019 11:39:05 +0100 Subject: [PATCH 13/30] Not working version because of lags in operations --- pom.xml | 2 +- .../ComputeEngineRetentionStrategy.java | 41 ++++++++++++++----- .../computeengine/client/ComputeClient.java | 19 ++++++++- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index 0ce15541..09725872 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ 1.0.8-SNAPSHOT hpi - 2.150.1 + 2.37 480 8 1.8 diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index 17c12ae7..99e876ef 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -7,7 +7,6 @@ import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; import javax.annotation.Nonnull; -import javax.imageio.IIOException; import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; @@ -50,6 +49,8 @@ public void taskAccepted(Executor executor, Queue.Task task) { @Override public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { + LOGGER.log(Level.INFO, "Task completed " + task + " executor " + executor); + checkPre(executor, task); if (oneShot) { delegate.taskCompleted(executor, task, durationMS); } @@ -57,21 +58,39 @@ public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { @Override public void taskCompletedWithProblems(Executor executor, Queue.Task task, long durationMS, Throwable problems) { + LOGGER.log(Level.INFO, "Task completed with problems " + task + " executor " + executor); + checkPre(executor, task); if (oneShot) { delegate.taskCompletedWithProblems(executor, task, durationMS, problems); } - LOGGER.log(Level.INFO, "Task completed with problem " + task, problems); - try { - LOGGER.log(Level.INFO, "Trying to rerun task"); - task.getOwnerTask().getOwnerTask().createExecutable().run(); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "Task rerunning error", e); - } } - private void checkPreemptive(Executor executor) { - final ComputeEngineComputer computer = (ComputeEngineComputer) executor.getOwner(); -// computer.getCloud().getClient(). + private void checkPre(Executor executor, Queue.Task task) { + ComputeEngineComputer computer = (ComputeEngineComputer) executor.getOwner(); + final boolean preemptive = checkPreemptive(computer); + LOGGER.log(Level.INFO, "pre " + preemptive); + if (preemptive) { + try { + LOGGER.log(Level.INFO, "Trying to rerun task"); + task.getOwnerTask().getOwnerTask().createExecutable().run(); + } catch (IOException ex) { + LOGGER.log(Level.WARNING, "Task rerunning error", ex); + } + } } + //"operationType": "compute.instances.preempted", + // "targetLink": "https://www.googleapis.com/compute/v1/projects/team-saasops/zones/us-central1-a/instances/slave-ingwar-pre-rg3jbi", + // Filter [(operationType="compute.instances.preempted") AND + // (targetLink="https://www.googleapis.com/compute/v1/projects/team-saasops/zones/us-central1-a/instances/slave-ingwar-pre-rg3jbi")] + private boolean checkPreemptive(ComputeEngineComputer computer) { + ComputeEngineCloud cloud = computer.getCloud(); + boolean preempted = false; + try { + preempted = computer.getCloud().getClient().isPreempted(cloud.getProjectId(), computer.getNode().zone, computer.getInstance().getSelfLink()); + } catch (IOException ex) { + LOGGER.log(Level.WARNING, "checkPreemptive error", ex); + } + return preempted; + } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java index 98485a17..e2f4dff1 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java @@ -19,6 +19,8 @@ import com.google.api.services.compute.Compute; import com.google.api.services.compute.model.*; import com.google.common.base.Strings; +import com.google.common.collect.Collections2; +import org.apache.commons.collections.CollectionUtils; import java.io.IOException; import java.util.*; @@ -348,9 +350,9 @@ public Operation insertInstance(String projectId, Optional template, Ins return insert.execute(); } - public Operation terminateInstance(String projectId, String zone, String InstanceId) throws IOException { + public Operation terminateInstance(String projectId, String zone, String instanceId) throws IOException { zone = nameFromSelfLink(zone); - return compute.instances().delete(projectId, zone, InstanceId).execute(); + return compute.instances().delete(projectId, zone, instanceId).execute(); } public Operation terminateInstanceWithStatus(String projectId, String zone, String instanceId, String desiredStatus) throws IOException, InterruptedException { @@ -420,6 +422,19 @@ public List getTemplates(String projectId) throws IOException return instanceTemplates; } + + public boolean isPreempted(String projectId, String zone, String instanceId) throws IOException { + String filter = "(operationType=\"compute.instances.preempted\") AND (targetLink=\"" + + instanceId + "\")"; + System.out.println("Filter [" + filter + "]"); + List items = compute.zoneOperations() + .list(projectId, zone) + .setFilter(filter) + .execute() + .getItems(); + + return CollectionUtils.isNotEmpty(items); + } /** * Appends metadata to an instance. Any metadata items with existing keys will be overwritten. Otherwise, metadata From 02196690234599895cb00cda26b20aecc134d808 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sun, 13 Jan 2019 10:05:26 +0100 Subject: [PATCH 14/30] tests --- .../ComputeEngineRetentionStrategy.java | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index 99e876ef..8f8ac19e 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -1,9 +1,12 @@ package com.google.jenkins.plugins.computeengine; +import com.google.api.services.compute.model.Instance; import hudson.model.Executor; import hudson.model.ExecutorListener; import hudson.model.Queue; +import hudson.model.Run; import hudson.slaves.RetentionStrategy; +import jenkins.model.Jenkins; import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; import javax.annotation.Nonnull; @@ -11,10 +14,12 @@ import java.util.logging.Level; import java.util.logging.Logger; +import static com.google.jenkins.plugins.computeengine.client.ComputeClient.nameFromSelfLink; + public class ComputeEngineRetentionStrategy extends RetentionStrategy implements ExecutorListener { - + private static final Logger LOGGER = Logger.getLogger(ComputeEngineRetentionStrategy.class.getName()); - + private final OnceRetentionStrategy delegate; private final boolean oneShot; @@ -42,11 +47,19 @@ public void start(@Nonnull ComputeEngineComputer c) { @Override public void taskAccepted(Executor executor, Queue.Task task) { LOGGER.log(Level.INFO, "Task accepted " + task); + LOGGER.log(Level.INFO, "Task accepted owner " + getBaseTask(task)); + LOGGER.log(Level.INFO, "Task accepted sub" + task.getSubTasks()); + LOGGER.log(Level.INFO, "Task accepted res" + task.getResourceList()); + try { + LOGGER.log(Level.INFO, "Task accepted res" + ((Run)getBaseTask(task)).getResult().toString()); + } catch (Exception ex) { + LOGGER.log(Level.WARNING, "tetts error", ex); + } if (oneShot) { delegate.taskAccepted(executor, task); } } - + @Override public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { LOGGER.log(Level.INFO, "Task completed " + task + " executor " + executor); @@ -65,19 +78,31 @@ public void taskCompletedWithProblems(Executor executor, Queue.Task task, long d } } + private Queue.Task getBaseTask(Queue.Task task) { + Queue.Task parent = task.getOwnerTask(); + while (task != parent) { + task = parent; + parent = task.getOwnerTask(); + } + return parent; + } + private void checkPre(Executor executor, Queue.Task task) { ComputeEngineComputer computer = (ComputeEngineComputer) executor.getOwner(); final boolean preemptive = checkPreemptive(computer); LOGGER.log(Level.INFO, "pre " + preemptive); if (preemptive) { - try { - LOGGER.log(Level.INFO, "Trying to rerun task"); - task.getOwnerTask().getOwnerTask().createExecutable().run(); - } catch (IOException ex) { - LOGGER.log(Level.WARNING, "Task rerunning error", ex); - } + LOGGER.log(Level.INFO, "Trying to rerun task"); +// task.getOwnerTask().getOwnerTask().createExecutable().run(); +// List actions = ImmutableList.of(new CauseAction(new Cause.UpstreamCause(task)); + Jenkins.getInstance().getQueue().schedule2(task, 0); } } + + private boolean isFailed(Queue.Task task) { + return false; + } + //"operationType": "compute.instances.preempted", // "targetLink": "https://www.googleapis.com/compute/v1/projects/team-saasops/zones/us-central1-a/instances/slave-ingwar-pre-rg3jbi", // Filter [(operationType="compute.instances.preempted") AND @@ -87,7 +112,10 @@ private boolean checkPreemptive(ComputeEngineComputer computer) { ComputeEngineCloud cloud = computer.getCloud(); boolean preempted = false; try { - preempted = computer.getCloud().getClient().isPreempted(cloud.getProjectId(), computer.getNode().zone, computer.getInstance().getSelfLink()); + final Instance instance = cloud.getClient().getInstance(cloud.getProjectId(), computer.getNode().zone, nameFromSelfLink(computer.getInstance().getSelfLink())); + LOGGER.log(Level.WARNING, "instance " + instance); + preempted = instance.getStatus().equals("TERMINATED"); +// preempted = computer.getCloud().getClient().isPreempted(cloud.getProjectId(), computer.getNode().zone, computer.getInstance().getSelfLink()); } catch (IOException ex) { LOGGER.log(Level.WARNING, "checkPreemptive error", ex); } From cfd851b314d655f60df139a3b0e07f2d92a4cde5 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 15 Jan 2019 13:04:44 +0100 Subject: [PATCH 15/30] Semi working --- pom.xml | 2 +- .../computeengine/ComputeEngineComputer.java | 17 ++++ .../ComputeEngineComputerListener.java | 80 ++++++++++++++++++- .../ComputeEngineRetentionStrategy.java | 50 ++---------- .../computeengine/client/ComputeClient.java | 13 --- 5 files changed, 104 insertions(+), 58 deletions(-) diff --git a/pom.xml b/pom.xml index 09725872..ecf1cda1 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ google-compute-engine - 1.0.8-SNAPSHOT + 2.0.8-SNAPSHOT hpi 2.37 diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java index ba67a4dd..e2eb8e0a 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java @@ -27,6 +27,7 @@ public class ComputeEngineComputer extends AbstractCloudComputer { private volatile Instance instance; + private boolean preempted = false; public ComputeEngineComputer(ComputeEngineInstance slave) { super(slave); @@ -84,6 +85,14 @@ public String getInstanceStatus() throws IOException { instance = _getInstance(); return instance.getStatus(); } + + boolean getPreemptible() { + try { + return getInstance().getScheduling().getPreemptible(); + } catch (IOException | NullPointerException e) { + return false; + } + } private Instance _getInstance() throws IOException { try { @@ -124,4 +133,12 @@ public HttpResponse doDoDelete() throws IOException { } return new HttpRedirect(".."); } + + public boolean getPreempted() { + return preempted; + } + + public void setPreempted(boolean preempted) { + this.preempted = preempted; + } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java index 5e52032d..1afb6789 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java @@ -16,17 +16,93 @@ package com.google.jenkins.plugins.computeengine; +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpTransport; +import com.google.api.client.http.javanet.NetHttpTransport; import hudson.Extension; import hudson.model.Computer; import hudson.model.TaskListener; import hudson.slaves.ComputerListener; +import jenkins.model.Jenkins; +import jenkins.security.MasterToSlaveCallable; +import org.apache.commons.io.Charsets; +import org.apache.commons.io.IOUtils; + +import java.io.IOException; + @Extension public class ComputeEngineComputerListener extends ComputerListener { + final String SHUTDONW_SCRIPT = "shutdown-script"; + @Override - public void onOnline(Computer c, TaskListener listener) { + public void onOnline(Computer c, TaskListener listener) throws IOException { if (c instanceof ComputeEngineComputer) { - ((ComputeEngineComputer) c).onConnected(); + ComputeEngineComputer computer = (ComputeEngineComputer) c; + computer.onConnected(); + + if (computer.getPreemptible()) { +// if (!computer.getInstance().getMetadata().containsKey(SHUTDONW_SCRIPT)) { +// computer.getInstance().getMetadata(). +// } + Jenkins.MasterComputer.threadPoolForRemoting.submit(() -> { + printMessage("Calling 0 local"); + try { + Boolean result = computer.getChannel().call(new MasterCall(listener)); + printMessage("Calling 0 local result " + result); + computer.setPreempted(result); + } catch (IOException | InterruptedException e) { + printMessage("Calling 0.1 exception " + e.getMessage()); + } + }); + } + } + } + + + private void printMessage(String string) { + System.out.println("CC local: " + string); + } + + private static final class MasterCall extends MasterToSlaveCallable { + private static final String METADATA_SERVER_URL = "http://metadata.google.internal/computeMetadata/v1/instance/preempted?wait_for_change=true"; + + private final TaskListener listener; + + private MasterCall(TaskListener listener) { + this.listener = listener; } + + @Override + public Boolean call() throws IOException { + printMessage("Calling 111 ext"); + return runningOnComputeEngine(); + } + + private Boolean runningOnComputeEngine() throws IOException { + printMessage("Calling 112 ext"); + HttpTransport transport = new NetHttpTransport(); + printMessage("Calling 113 ext"); + GenericUrl metadata = new GenericUrl(METADATA_SERVER_URL); + HttpRequest request = transport.createRequestFactory().buildGetRequest(metadata); + request.setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google")); + request.setReadTimeout(Integer.MAX_VALUE); + printMessage("Calling 114 ext"); + HttpResponse response = request.execute(); + printMessage("Calling 2 ext response " + response.getStatusMessage()); + final String result = IOUtils.toString(response.getContent(), Charsets.UTF_8); + printMessage("Calling 2 ext result " + result); + response.disconnect(); + return "TRUE".equals(result); + } + + private void printMessage(String string) { + listener.error("CC extrernal: " + string); + System.out.println("CC extrernal: " + string); + } + } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index 8f8ac19e..bd06003d 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -1,21 +1,16 @@ package com.google.jenkins.plugins.computeengine; -import com.google.api.services.compute.model.Instance; import hudson.model.Executor; import hudson.model.ExecutorListener; import hudson.model.Queue; -import hudson.model.Run; import hudson.slaves.RetentionStrategy; import jenkins.model.Jenkins; import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; import javax.annotation.Nonnull; -import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; -import static com.google.jenkins.plugins.computeengine.client.ComputeClient.nameFromSelfLink; - public class ComputeEngineRetentionStrategy extends RetentionStrategy implements ExecutorListener { private static final Logger LOGGER = Logger.getLogger(ComputeEngineRetentionStrategy.class.getName()); @@ -48,18 +43,11 @@ public void start(@Nonnull ComputeEngineComputer c) { public void taskAccepted(Executor executor, Queue.Task task) { LOGGER.log(Level.INFO, "Task accepted " + task); LOGGER.log(Level.INFO, "Task accepted owner " + getBaseTask(task)); - LOGGER.log(Level.INFO, "Task accepted sub" + task.getSubTasks()); - LOGGER.log(Level.INFO, "Task accepted res" + task.getResourceList()); - try { - LOGGER.log(Level.INFO, "Task accepted res" + ((Run)getBaseTask(task)).getResult().toString()); - } catch (Exception ex) { - LOGGER.log(Level.WARNING, "tetts error", ex); - } if (oneShot) { delegate.taskAccepted(executor, task); } } - + @Override public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { LOGGER.log(Level.INFO, "Task completed " + task + " executor " + executor); @@ -86,39 +74,17 @@ private Queue.Task getBaseTask(Queue.Task task) { } return parent; } - + private void checkPre(Executor executor, Queue.Task task) { ComputeEngineComputer computer = (ComputeEngineComputer) executor.getOwner(); - final boolean preemptive = checkPreemptive(computer); - LOGGER.log(Level.INFO, "pre " + preemptive); - if (preemptive) { + final boolean preemptible = computer.getPreemptible(); + final boolean preempted = computer.getPreempted(); + LOGGER.log(Level.INFO, "preemptible " + preemptible + " && preempted " + preempted); + if (preemptible && preempted) { LOGGER.log(Level.INFO, "Trying to rerun task"); // task.getOwnerTask().getOwnerTask().createExecutable().run(); // List actions = ImmutableList.of(new CauseAction(new Cause.UpstreamCause(task)); - Jenkins.getInstance().getQueue().schedule2(task, 0); - } - } - - private boolean isFailed(Queue.Task task) { - return false; - } - - //"operationType": "compute.instances.preempted", - // "targetLink": "https://www.googleapis.com/compute/v1/projects/team-saasops/zones/us-central1-a/instances/slave-ingwar-pre-rg3jbi", - // Filter [(operationType="compute.instances.preempted") AND - // (targetLink="https://www.googleapis.com/compute/v1/projects/team-saasops/zones/us-central1-a/instances/slave-ingwar-pre-rg3jbi")] - - private boolean checkPreemptive(ComputeEngineComputer computer) { - ComputeEngineCloud cloud = computer.getCloud(); - boolean preempted = false; - try { - final Instance instance = cloud.getClient().getInstance(cloud.getProjectId(), computer.getNode().zone, nameFromSelfLink(computer.getInstance().getSelfLink())); - LOGGER.log(Level.WARNING, "instance " + instance); - preempted = instance.getStatus().equals("TERMINATED"); -// preempted = computer.getCloud().getClient().isPreempted(cloud.getProjectId(), computer.getNode().zone, computer.getInstance().getSelfLink()); - } catch (IOException ex) { - LOGGER.log(Level.WARNING, "checkPreemptive error", ex); + Jenkins.getInstance().getQueue().schedule2(getBaseTask(task), 0); } - return preempted; } -} +} diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java index e2f4dff1..648ed013 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java @@ -423,19 +423,6 @@ public List getTemplates(String projectId) throws IOException return instanceTemplates; } - public boolean isPreempted(String projectId, String zone, String instanceId) throws IOException { - String filter = "(operationType=\"compute.instances.preempted\") AND (targetLink=\"" + - instanceId + "\")"; - System.out.println("Filter [" + filter + "]"); - List items = compute.zoneOperations() - .list(projectId, zone) - .setFilter(filter) - .execute() - .getItems(); - - return CollectionUtils.isNotEmpty(items); - } - /** * Appends metadata to an instance. Any metadata items with existing keys will be overwritten. Otherwise, metadata * is preserved. This method blocks until the operation completes. From 7cc32925d0ff0c06647181414929162d39167434 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 15 Jan 2019 13:08:00 +0100 Subject: [PATCH 16/30] Ugly but working --- .../computeengine/client/ComputeClient.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java index 648ed013..0fdec824 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/client/ComputeClient.java @@ -19,8 +19,6 @@ import com.google.api.services.compute.Compute; import com.google.api.services.compute.model.*; import com.google.common.base.Strings; -import com.google.common.collect.Collections2; -import org.apache.commons.collections.CollectionUtils; import java.io.IOException; import java.util.*; @@ -36,7 +34,7 @@ public class ComputeClient { private static final Logger LOGGER = Logger.getLogger(ComputeClient.class.getName()); private Compute compute; - public static String nameFromSelfLink(String selfLink) { + public static String nameFromSelfLink(String selfLink) { return selfLink.substring(selfLink.lastIndexOf("/") + 1, selfLink.length()); } @@ -171,7 +169,7 @@ public List cpuPlatforms(String projectId, String zone) throws IOExcepti List cpuPlatforms = new ArrayList(); zone = nameFromSelfLink(zone); Zone zoneObject = compute.zones() - .get(projectId,zone) + .get(projectId, zone) .execute(); if (zoneObject == null) { return cpuPlatforms; @@ -389,25 +387,25 @@ public List getInstancesWithLabel(String projectId, Map getTemplates(String projectId) throws IOException { List instanceTemplates = compute.instanceTemplates() .list(projectId) @@ -422,7 +420,7 @@ public List getTemplates(String projectId) throws IOException return instanceTemplates; } - + /** * Appends metadata to an instance. Any metadata items with existing keys will be overwritten. Otherwise, metadata * is preserved. This method blocks until the operation completes. From 6708acf810d679d6ef482587eeb9b799bfc7ca10 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 15 Jan 2019 16:13:38 +0100 Subject: [PATCH 17/30] Almost almost --- .../computeengine/ComputeEngineComputer.java | 32 +++++--- .../ComputeEngineComputerListener.java | 74 +------------------ .../ComputeEngineRetentionStrategy.java | 47 +++++++++--- .../computeengine/PreemptedCheckCallable.java | 54 ++++++++++++++ 4 files changed, 112 insertions(+), 95 deletions(-) create mode 100644 src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java index e2eb8e0a..6be38bce 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java @@ -17,17 +17,23 @@ package com.google.jenkins.plugins.computeengine; import com.google.api.services.compute.model.Instance; +import hudson.model.TaskListener; import hudson.slaves.AbstractCloudComputer; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.HttpRedirect; import org.kohsuke.stapler.HttpResponse; import java.io.IOException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.logging.Level; +import java.util.logging.Logger; public class ComputeEngineComputer extends AbstractCloudComputer { + private static final Logger LOGGER = Logger.getLogger(ComputeEngineComputerListener.class.getName()); private volatile Instance instance; - private boolean preempted = false; + private Future preemptedFuture; public ComputeEngineComputer(ComputeEngineInstance slave) { super(slave); @@ -38,10 +44,15 @@ public ComputeEngineInstance getNode() { return (ComputeEngineInstance) super.getNode(); } - public void onConnected() { + void onConnected(TaskListener listener) throws IOException { ComputeEngineInstance node = getNode(); if (node != null) { node.onConnected(); + if (getPreemptible()) { + String nodeName = node.getNodeName(); + LOGGER.log(Level.INFO, "Instance " + nodeName + " is preemptible, setting up premption listener"); + preemptedFuture = getChannel().callAsync(new PreemptedCheckCallable(listener)); + } } } @@ -85,7 +96,7 @@ public String getInstanceStatus() throws IOException { instance = _getInstance(); return instance.getStatus(); } - + boolean getPreemptible() { try { return getInstance().getScheduling().getPreemptible(); @@ -93,6 +104,14 @@ boolean getPreemptible() { return false; } } + + boolean getPreempted() { + try { + return preemptedFuture != null && preemptedFuture.isDone() && preemptedFuture.get(); + } catch (InterruptedException | ExecutionException e) { + return false; + } + } private Instance _getInstance() throws IOException { try { @@ -134,11 +153,4 @@ public HttpResponse doDoDelete() throws IOException { return new HttpRedirect(".."); } - public boolean getPreempted() { - return preempted; - } - - public void setPreempted(boolean preempted) { - this.preempted = preempted; - } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java index 1afb6789..afe29e26 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java @@ -16,93 +16,21 @@ package com.google.jenkins.plugins.computeengine; -import com.google.api.client.http.GenericUrl; -import com.google.api.client.http.HttpHeaders; -import com.google.api.client.http.HttpRequest; -import com.google.api.client.http.HttpResponse; -import com.google.api.client.http.HttpTransport; -import com.google.api.client.http.javanet.NetHttpTransport; import hudson.Extension; import hudson.model.Computer; import hudson.model.TaskListener; import hudson.slaves.ComputerListener; -import jenkins.model.Jenkins; -import jenkins.security.MasterToSlaveCallable; -import org.apache.commons.io.Charsets; -import org.apache.commons.io.IOUtils; import java.io.IOException; @Extension public class ComputeEngineComputerListener extends ComputerListener { - final String SHUTDONW_SCRIPT = "shutdown-script"; - @Override public void onOnline(Computer c, TaskListener listener) throws IOException { if (c instanceof ComputeEngineComputer) { ComputeEngineComputer computer = (ComputeEngineComputer) c; - computer.onConnected(); - - if (computer.getPreemptible()) { -// if (!computer.getInstance().getMetadata().containsKey(SHUTDONW_SCRIPT)) { -// computer.getInstance().getMetadata(). -// } - Jenkins.MasterComputer.threadPoolForRemoting.submit(() -> { - printMessage("Calling 0 local"); - try { - Boolean result = computer.getChannel().call(new MasterCall(listener)); - printMessage("Calling 0 local result " + result); - computer.setPreempted(result); - } catch (IOException | InterruptedException e) { - printMessage("Calling 0.1 exception " + e.getMessage()); - } - }); - } - } - } - - - private void printMessage(String string) { - System.out.println("CC local: " + string); - } - - private static final class MasterCall extends MasterToSlaveCallable { - private static final String METADATA_SERVER_URL = "http://metadata.google.internal/computeMetadata/v1/instance/preempted?wait_for_change=true"; - - private final TaskListener listener; - - private MasterCall(TaskListener listener) { - this.listener = listener; - } - - @Override - public Boolean call() throws IOException { - printMessage("Calling 111 ext"); - return runningOnComputeEngine(); - } - - private Boolean runningOnComputeEngine() throws IOException { - printMessage("Calling 112 ext"); - HttpTransport transport = new NetHttpTransport(); - printMessage("Calling 113 ext"); - GenericUrl metadata = new GenericUrl(METADATA_SERVER_URL); - HttpRequest request = transport.createRequestFactory().buildGetRequest(metadata); - request.setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google")); - request.setReadTimeout(Integer.MAX_VALUE); - printMessage("Calling 114 ext"); - HttpResponse response = request.execute(); - printMessage("Calling 2 ext response " + response.getStatusMessage()); - final String result = IOUtils.toString(response.getContent(), Charsets.UTF_8); - printMessage("Calling 2 ext result " + result); - response.disconnect(); - return "TRUE".equals(result); - } - - private void printMessage(String string) { - listener.error("CC extrernal: " + string); - System.out.println("CC extrernal: " + string); + computer.onConnected(listener); } - } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index bd06003d..e41f763b 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -1,13 +1,19 @@ package com.google.jenkins.plugins.computeengine; +import com.google.common.collect.ImmutableList; +import hudson.model.Action; +import hudson.model.Cause; +import hudson.model.CauseAction; import hudson.model.Executor; import hudson.model.ExecutorListener; +import hudson.model.Job; import hudson.model.Queue; import hudson.slaves.RetentionStrategy; import jenkins.model.Jenkins; import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; import javax.annotation.Nonnull; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -41,8 +47,6 @@ public void start(@Nonnull ComputeEngineComputer c) { @Override public void taskAccepted(Executor executor, Queue.Task task) { - LOGGER.log(Level.INFO, "Task accepted " + task); - LOGGER.log(Level.INFO, "Task accepted owner " + getBaseTask(task)); if (oneShot) { delegate.taskAccepted(executor, task); } @@ -50,8 +54,7 @@ public void taskAccepted(Executor executor, Queue.Task task) { @Override public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { - LOGGER.log(Level.INFO, "Task completed " + task + " executor " + executor); - checkPre(executor, task); + checkPreempted(executor, task); if (oneShot) { delegate.taskCompleted(executor, task, durationMS); } @@ -59,8 +62,7 @@ public void taskCompleted(Executor executor, Queue.Task task, long durationMS) { @Override public void taskCompletedWithProblems(Executor executor, Queue.Task task, long durationMS, Throwable problems) { - LOGGER.log(Level.INFO, "Task completed with problems " + task + " executor " + executor); - checkPre(executor, task); + checkPreempted(executor, task); if (oneShot) { delegate.taskCompletedWithProblems(executor, task, durationMS, problems); } @@ -69,22 +71,43 @@ public void taskCompletedWithProblems(Executor executor, Queue.Task task, long d private Queue.Task getBaseTask(Queue.Task task) { Queue.Task parent = task.getOwnerTask(); while (task != parent) { + System.out.println("Task next: " + parent); task = parent; parent = task.getOwnerTask(); } return parent; } - private void checkPre(Executor executor, Queue.Task task) { + private void checkPreempted(Executor executor, Queue.Task task) { ComputeEngineComputer computer = (ComputeEngineComputer) executor.getOwner(); final boolean preemptible = computer.getPreemptible(); final boolean preempted = computer.getPreempted(); - LOGGER.log(Level.INFO, "preemptible " + preemptible + " && preempted " + preempted); + Queue.Task baseTask = getBaseTask(task); + try { + final List causes = ((Job) baseTask).getLastBuild().getCauses(); + System.out.println("Causes: " + causes); + } catch (Exception e) { + System.out.println("Exception for " + baseTask); + e.printStackTrace(); + } if (preemptible && preempted) { - LOGGER.log(Level.INFO, "Trying to rerun task"); -// task.getOwnerTask().getOwnerTask().createExecutable().run(); -// List actions = ImmutableList.of(new CauseAction(new Cause.UpstreamCause(task)); - Jenkins.getInstance().getQueue().schedule2(getBaseTask(task), 0); + LOGGER.log(Level.INFO, baseTask + " preemptible " + preemptible + " && preempted " + preempted); + List actions = generateActionsForTask(task); + Jenkins.getInstance().getQueue().schedule2(baseTask, 0, actions); + } else if (preemptible) { + LOGGER.log(Level.INFO, "preemptible " + preemptible + " && preempted " + preempted); } } + + private List generateActionsForTask(Queue.Task task) { + return ImmutableList.of(new CauseAction(new Cause.UserIdCause()), + new CauseAction(new Cause() { + @Override + public String getShortDescription() { + return "Rebuilding preemptied job"; + } + })); + } + + } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java new file mode 100644 index 00000000..f264ca7d --- /dev/null +++ b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java @@ -0,0 +1,54 @@ +/* + * Copyright 2019 Google Inc. All Rights Reserved. + * + * 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. + */ +package com.google.jenkins.plugins.computeengine; + +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpTransport; +import com.google.api.client.http.javanet.NetHttpTransport; +import hudson.model.TaskListener; +import jenkins.security.MasterToSlaveCallable; +import org.apache.commons.io.Charsets; +import org.apache.commons.io.IOUtils; + +import java.io.IOException; + +final class PreemptedCheckCallable extends MasterToSlaveCallable { + private static final String METADATA_SERVER_URL = "http://metadata.google.internal/computeMetadata/v1/instance/preempted?wait_for_change=true"; + + private final TaskListener listener; + + PreemptedCheckCallable(TaskListener listener) { + this.listener = listener; + } + + @Override + public Boolean call() throws IOException { + HttpTransport transport = new NetHttpTransport(); + GenericUrl metadata = new GenericUrl(METADATA_SERVER_URL); + HttpRequest request = transport.createRequestFactory().buildGetRequest(metadata); + request.setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google")); + request.setReadTimeout(Integer.MAX_VALUE); + listener.getLogger().println("Preemptible instance, listening metadata for preemption event"); + HttpResponse response = request.execute(); + final String result = IOUtils.toString(response.getContent(), Charsets.UTF_8); + listener.getLogger().println("Got preemption event " + result); + response.disconnect(); + return "TRUE".equals(result); + } +} From 9a576d4c6c3ace9e0600b07597744578ae033db0 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 15 Jan 2019 16:24:17 +0100 Subject: [PATCH 18/30] Minor fixes --- .../ComputeEngineRetentionStrategy.java | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index e41f763b..e673b162 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -1,3 +1,18 @@ +/* + * Copyright 2019 Google Inc. All Rights Reserved. + * + * 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. + */ package com.google.jenkins.plugins.computeengine; import com.google.common.collect.ImmutableList; @@ -8,6 +23,8 @@ import hudson.model.ExecutorListener; import hudson.model.Job; import hudson.model.Queue; +import hudson.security.ACL; +import hudson.security.ACLContext; import hudson.slaves.RetentionStrategy; import jenkins.model.Jenkins; import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; @@ -71,7 +88,6 @@ public void taskCompletedWithProblems(Executor executor, Queue.Task task, long d private Queue.Task getBaseTask(Queue.Task task) { Queue.Task parent = task.getOwnerTask(); while (task != parent) { - System.out.println("Task next: " + parent); task = parent; parent = task.getOwnerTask(); } @@ -83,23 +99,26 @@ private void checkPreempted(Executor executor, Queue.Task task) { final boolean preemptible = computer.getPreemptible(); final boolean preempted = computer.getPreempted(); Queue.Task baseTask = getBaseTask(task); - try { - final List causes = ((Job) baseTask).getLastBuild().getCauses(); - System.out.println("Causes: " + causes); - } catch (Exception e) { - System.out.println("Exception for " + baseTask); - e.printStackTrace(); - } if (preemptible && preempted) { - LOGGER.log(Level.INFO, baseTask + " preemptible " + preemptible + " && preempted " + preempted); + LOGGER.log(Level.INFO, baseTask + " is preemptible and was preempted"); List actions = generateActionsForTask(task); - Jenkins.getInstance().getQueue().schedule2(baseTask, 0, actions); + try (ACLContext context = ACL.as(task.getDefaultAuthentication())) { + Jenkins.getInstance().getQueue().schedule2(baseTask, 0, actions); + } } else if (preemptible) { - LOGGER.log(Level.INFO, "preemptible " + preemptible + " && preempted " + preempted); + LOGGER.log(Level.INFO, baseTask + " is preemptible and was NOT preempted"); } } private List generateActionsForTask(Queue.Task task) { + Queue.Task baseTask = getBaseTask(task); + try { + final List causes = ((Job) baseTask).getLastBuild().getCauses(); + System.out.println("Causes: " + causes); + } catch (Exception e) { + System.out.println("Exception for " + baseTask); + e.printStackTrace(); + } return ImmutableList.of(new CauseAction(new Cause.UserIdCause()), new CauseAction(new Cause() { @Override From ecf5483b66103eb6c6ebb1f63615fea1dd7cfdd7 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Fri, 18 Jan 2019 14:17:04 +0100 Subject: [PATCH 19/30] Terminating preempted jobs --- pom.xml | 2 +- .../computeengine/ComputeEngineComputer.java | 31 +++++++++++++++---- .../ComputeEngineRetentionStrategy.java | 22 +++++++------ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/pom.xml b/pom.xml index ecf1cda1..892d9280 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ google-compute-engine - 2.0.8-SNAPSHOT + 2.0.9-SNAPSHOT hpi 2.37 diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java index 6be38bce..1f957658 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java @@ -17,8 +17,12 @@ package com.google.jenkins.plugins.computeengine; import com.google.api.services.compute.model.Instance; +import hudson.model.Executor; +import hudson.model.Result; import hudson.model.TaskListener; +import hudson.remoting.Channel; import hudson.slaves.AbstractCloudComputer; +import jenkins.model.CauseOfInterruption; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.HttpRedirect; import org.kohsuke.stapler.HttpResponse; @@ -39,23 +43,38 @@ public ComputeEngineComputer(ComputeEngineInstance slave) { super(slave); } - @Override - public ComputeEngineInstance getNode() { - return (ComputeEngineInstance) super.getNode(); - } - void onConnected(TaskListener listener) throws IOException { ComputeEngineInstance node = getNode(); if (node != null) { node.onConnected(); if (getPreemptible()) { String nodeName = node.getNodeName(); - LOGGER.log(Level.INFO, "Instance " + nodeName + " is preemptible, setting up premption listener"); + LOGGER.log(Level.INFO, "Instance " + nodeName + " is preemptive, setting up preemption listener"); preemptedFuture = getChannel().callAsync(new PreemptedCheckCallable(listener)); + getChannel().addListener(new Channel.Listener() { + @Override + public void onClosed(Channel channel, IOException cause) { + LOGGER.log(Level.INFO, "Goc channel close event"); + if (getPreempted()) { + LOGGER.log(Level.INFO, "Goc channel close and its preempied"); + getExecutors().forEach(executor -> interruptExecutor(executor, nodeName)); + } + } + }); } } } + private void interruptExecutor(Executor executor, String nodeName) { + LOGGER.log(Level.INFO, "Terminating executor " + executor + " node " + nodeName); + executor.interrupt(Result.FAILURE, new CauseOfInterruption() { + @Override + public String getShortDescription() { + return "Instance " + nodeName + " was preempted"; + } + }); + } + public String getNumExecutorsStr() { return String.valueOf(super.getNumExecutors()); } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index e673b162..08abc26b 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -100,13 +100,13 @@ private void checkPreempted(Executor executor, Queue.Task task) { final boolean preempted = computer.getPreempted(); Queue.Task baseTask = getBaseTask(task); if (preemptible && preempted) { - LOGGER.log(Level.INFO, baseTask + " is preemptible and was preempted"); + LOGGER.log(Level.INFO, baseTask + " is preemptive and was preempted"); List actions = generateActionsForTask(task); try (ACLContext context = ACL.as(task.getDefaultAuthentication())) { Jenkins.getInstance().getQueue().schedule2(baseTask, 0, actions); } } else if (preemptible) { - LOGGER.log(Level.INFO, baseTask + " is preemptible and was NOT preempted"); + LOGGER.log(Level.INFO, baseTask + " is preemptive and was NOT preempted"); } } @@ -119,14 +119,16 @@ private List generateActionsForTask(Queue.Task task) { System.out.println("Exception for " + baseTask); e.printStackTrace(); } - return ImmutableList.of(new CauseAction(new Cause.UserIdCause()), - new CauseAction(new Cause() { - @Override - public String getShortDescription() { - return "Rebuilding preemptied job"; - } - })); + return ImmutableList.of( + new CauseAction(new Cause.UserIdCause()), + new CauseAction(new RebuidCause()) + ); } - + public static class RebuidCause extends Cause { + @Override + public String getShortDescription() { + return "Rebuilding preempted job"; + } + } } From 31cd6e09145704c15427a4d3d852707e9a6b5cb2 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sun, 27 Jan 2019 18:24:39 +0100 Subject: [PATCH 20/30] Nimor fixes --- .../ComputeEngineRetentionStrategy.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index 08abc26b..df34189d 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -23,6 +23,7 @@ import hudson.model.ExecutorListener; import hudson.model.Job; import hudson.model.Queue; +import hudson.model.queue.Tasks; import hudson.security.ACL; import hudson.security.ACLContext; import hudson.slaves.RetentionStrategy; @@ -64,6 +65,7 @@ public void start(@Nonnull ComputeEngineComputer c) { @Override public void taskAccepted(Executor executor, Queue.Task task) { + getBaseTask(task); if (oneShot) { delegate.taskAccepted(executor, task); } @@ -86,10 +88,12 @@ public void taskCompletedWithProblems(Executor executor, Queue.Task task, long d } private Queue.Task getBaseTask(Queue.Task task) { - Queue.Task parent = task.getOwnerTask(); + System.out.print("Task: " + task + "\n\n"); + Queue.Task parent = Tasks.getOwnerTaskOf(task); while (task != parent) { + System.out.print("Owner: " + parent + "\n\n"); task = parent; - parent = task.getOwnerTask(); + parent = Tasks.getOwnerTaskOf(task); } return parent; } @@ -113,7 +117,8 @@ private void checkPreempted(Executor executor, Queue.Task task) { private List generateActionsForTask(Queue.Task task) { Queue.Task baseTask = getBaseTask(task); try { - final List causes = ((Job) baseTask).getLastBuild().getCauses(); + final Job job = (Job) baseTask; + final List causes = job.getLastBuild().getCauses(); System.out.println("Causes: " + causes); } catch (Exception e) { System.out.println("Exception for " + baseTask); @@ -121,11 +126,11 @@ private List generateActionsForTask(Queue.Task task) { } return ImmutableList.of( new CauseAction(new Cause.UserIdCause()), - new CauseAction(new RebuidCause()) + new CauseAction(new RebuildCause()) ); } - public static class RebuidCause extends Cause { + public static class RebuildCause extends Cause { @Override public String getShortDescription() { return "Rebuilding preempted job"; From 029486e9e20319ef81702948ab2021da849c2493 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 30 Jan 2019 12:47:42 +0100 Subject: [PATCH 21/30] Merge conflicts --- .../plugins/computeengine/ComputeEngineInstance.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 00617ac9..ab4e8b9c 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -37,7 +37,7 @@ public class ComputeEngineInstance extends AbstractCloudSlave { public final String sshUser; public transient final Optional windowsConfig; public Integer launchTimeout; // Seconds - public Boolean connected; + private Boolean connected; public ComputeEngineInstance(String cloudName, String name, @@ -83,10 +83,6 @@ public String getCloudName() { return cloudName; } - public String getCloudName() { - return cloudName; - } - public void onConnected() { this.connected = true; } From bcbe1ed53e2b7614b21e96649f411b8ff60a3354 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 30 Jan 2019 14:21:03 +0100 Subject: [PATCH 22/30] Cleanup --- .../jenkins/plugins/computeengine/InstanceConfiguration.java | 4 ---- .../jenkins/plugins/computeengine/ComputeEngineCloudIT.java | 1 - 2 files changed, 5 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java index a7150d43..7309166e 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java @@ -34,13 +34,9 @@ import hudson.model.*; import hudson.model.labels.LabelAtom; import hudson.security.ACL; -import hudson.slaves.CloudRetentionStrategy; -import hudson.util.FormValidation; -import hudson.util.ListBoxModel; import jenkins.model.Jenkins; import org.apache.commons.lang.StringUtils; import org.apache.commons.text.RandomStringGenerator; -import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; diff --git a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java index 19640025..be65c6d5 100644 --- a/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java +++ b/src/test/java/com/google/jenkins/plugins/computeengine/ComputeEngineCloudIT.java @@ -506,7 +506,6 @@ private static void deleteIntegrationInstances(boolean waitForCompletion) throws private static void safeDelete(String instanceId, boolean waitForCompletion) { try { - System.out.printf("Deleting: " + instanceId); Operation op = client.terminateInstance(projectId, ZONE, instanceId); if (waitForCompletion) client.waitForOperationCompletion(projectId, op.getName(), op.getZone(), 3 * 60 * 1000); From 376a1fa4ee6e28f4b36511d32ad70c0891a46cc6 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 30 Jan 2019 22:26:59 +0100 Subject: [PATCH 23/30] Fixes after merge --- .../jenkins/plugins/computeengine/InstanceConfiguration.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java index 7309166e..6ee89ea6 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/InstanceConfiguration.java @@ -34,6 +34,8 @@ import hudson.model.*; import hudson.model.labels.LabelAtom; import hudson.security.ACL; +import hudson.util.FormValidation; +import hudson.util.ListBoxModel; import jenkins.model.Jenkins; import org.apache.commons.lang.StringUtils; import org.apache.commons.text.RandomStringGenerator; From b607d99773d0cd70b9a9c027a057dbc67664df92 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Wed, 30 Jan 2019 22:29:04 +0100 Subject: [PATCH 24/30] Remove bit of old logs --- .../plugins/computeengine/ComputeEngineRetentionStrategy.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index df34189d..af84f406 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -65,7 +65,6 @@ public void start(@Nonnull ComputeEngineComputer c) { @Override public void taskAccepted(Executor executor, Queue.Task task) { - getBaseTask(task); if (oneShot) { delegate.taskAccepted(executor, task); } @@ -88,10 +87,8 @@ public void taskCompletedWithProblems(Executor executor, Queue.Task task, long d } private Queue.Task getBaseTask(Queue.Task task) { - System.out.print("Task: " + task + "\n\n"); Queue.Task parent = Tasks.getOwnerTaskOf(task); while (task != parent) { - System.out.print("Owner: " + parent + "\n\n"); task = parent; parent = Tasks.getOwnerTaskOf(task); } From 3fb8957bb6429b3c2352a93cc823f447ba201a20 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Fri, 1 Mar 2019 15:23:25 +0100 Subject: [PATCH 25/30] Fix mirror issues --- .../plugins/computeengine/ComputeEngineComputer.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java index 166e9468..16dd8fb8 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java @@ -34,7 +34,7 @@ import java.util.logging.Logger; public class ComputeEngineComputer extends AbstractCloudComputer { - private static final Logger LOGGER = Logger.getLogger(ComputeEngineComputerListener.class.getName()); + private static final Logger LOGGER = Logger.getLogger(ComputeEngineCloud.class.getName()); private volatile Instance instance; private Future preemptedFuture; @@ -54,9 +54,9 @@ void onConnected(TaskListener listener) throws IOException { getChannel().addListener(new Channel.Listener() { @Override public void onClosed(Channel channel, IOException cause) { - LOGGER.log(Level.INFO, "Goc channel close event"); + LOGGER.log(Level.INFO, "Got channel close event"); if (getPreempted()) { - LOGGER.log(Level.INFO, "Goc channel close and its preempied"); + LOGGER.log(Level.INFO, "Goc channel close and its preempted"); getExecutors().forEach(executor -> interruptExecutor(executor, nodeName)); } } @@ -174,5 +174,4 @@ public HttpResponse doDoDelete() throws IOException { } return new HttpRedirect(".."); } - } From 9583f45e812a096b2b147ba0b720669655a27ab3 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Fri, 1 Mar 2019 15:28:11 +0100 Subject: [PATCH 26/30] Fix mirror compile issues --- .../jenkins/plugins/computeengine/ComputeEngineInstance.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 8791e61b..2a60a4b2 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -77,7 +77,7 @@ public AbstractCloudComputer createComputer() { } @Override - protected void _terminate(TaskListener listener) throws IOException { + protected void _terminate(TaskListener listener) throws IOException, InterruptedException { try { ComputeEngineCloud cloud = getCloud(); From ae8fd88a95dc530d61e868446a77feb152283bfb Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 19 Mar 2019 14:06:45 +0100 Subject: [PATCH 27/30] Review changes --- .../computeengine/ComputeEngineComputer.java | 4 +-- .../ComputeEngineComputerListener.java | 1 - .../computeengine/ComputeEngineInstance.java | 5 ++-- .../ComputeEngineRetentionStrategy.java | 29 ++++++++++--------- .../computeengine/PreemptedCheckCallable.java | 19 +++++++++++- .../plugins/computeengine/Messages.properties | 3 +- 6 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java index 16dd8fb8..5b2a74d0 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputer.java @@ -54,9 +54,9 @@ void onConnected(TaskListener listener) throws IOException { getChannel().addListener(new Channel.Listener() { @Override public void onClosed(Channel channel, IOException cause) { - LOGGER.log(Level.INFO, "Got channel close event"); + LOGGER.log(Level.FINE, "Got channel close event"); if (getPreempted()) { - LOGGER.log(Level.INFO, "Goc channel close and its preempted"); + LOGGER.log(Level.FINE, "Preempted node channel closed, terminating all executors"); getExecutors().forEach(executor -> interruptExecutor(executor, nodeName)); } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java index afe29e26..77c0838f 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineComputerListener.java @@ -23,7 +23,6 @@ import java.io.IOException; - @Extension public class ComputeEngineComputerListener extends ComputerListener { @Override diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java index 38a66730..942ad236 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineInstance.java @@ -29,9 +29,9 @@ import java.io.IOException; import java.util.Collections; +import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.Optional; public class ComputeEngineInstance extends AbstractCloudSlave { private static final long serialVersionUID = 1; @@ -100,6 +100,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted /** * Based on the instance configuration, whether to create snapshot for an instance with failed builds at deletion time. + * * @return Whether or not to create the snapshot. */ public boolean isCreateSnapshot() { @@ -109,7 +110,7 @@ public boolean isCreateSnapshot() { public String getCloudName() { return cloudName; } - + public void onConnected() { this.connected = true; } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index af84f406..d5daf88f 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -23,20 +23,22 @@ import hudson.model.ExecutorListener; import hudson.model.Job; import hudson.model.Queue; -import hudson.model.queue.Tasks; import hudson.security.ACL; import hudson.security.ACLContext; import hudson.slaves.RetentionStrategy; import jenkins.model.Jenkins; import org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy; -import javax.annotation.Nonnull; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +/** + * A strategy that allows: + * - setting one shot instances {@link OnceRetentionStrategy} + * - in case of preemption of GCP instance to restart preempted tasks + */ public class ComputeEngineRetentionStrategy extends RetentionStrategy implements ExecutorListener { - private static final Logger LOGGER = Logger.getLogger(ComputeEngineRetentionStrategy.class.getName()); private final OnceRetentionStrategy delegate; @@ -54,12 +56,12 @@ public class ComputeEngineRetentionStrategy extends RetentionStrategy actions = generateActionsForTask(task); - try (ACLContext context = ACL.as(task.getDefaultAuthentication())) { - Jenkins.getInstance().getQueue().schedule2(baseTask, 0, actions); + try (ACLContext notUsed = ACL.as(task.getDefaultAuthentication())) { + Jenkins.get().getQueue().schedule2(baseTask, 0, actions); } } else if (preemptible) { LOGGER.log(Level.INFO, baseTask + " is preemptive and was NOT preempted"); @@ -116,21 +118,20 @@ private List generateActionsForTask(Queue.Task task) { try { final Job job = (Job) baseTask; final List causes = job.getLastBuild().getCauses(); - System.out.println("Causes: " + causes); + LOGGER.log(Level.FINE, "Causes: " + causes); } catch (Exception e) { - System.out.println("Exception for " + baseTask); - e.printStackTrace(); + LOGGER.log(Level.WARNING, "Exception for " + baseTask, e); } return ImmutableList.of( new CauseAction(new Cause.UserIdCause()), new CauseAction(new RebuildCause()) ); } - + public static class RebuildCause extends Cause { @Override public String getShortDescription() { - return "Rebuilding preempted job"; + return Messages.RebuildCause_ShortDescription(); } } } diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java index f264ca7d..5da97f22 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java @@ -28,15 +28,32 @@ import java.io.IOException; +/** + * Slave callback class checking if instance was preempted. + * All of code here is serialized and executed on node. + */ final class PreemptedCheckCallable extends MasterToSlaveCallable { private static final String METADATA_SERVER_URL = "http://metadata.google.internal/computeMetadata/v1/instance/preempted?wait_for_change=true"; private final TaskListener listener; + /** + * Callback constructor. + * @param listener Node listener on which we can add extra information from slave side. + */ PreemptedCheckCallable(TaskListener listener) { this.listener = listener; } + /** + * Actual callback code, executed on node side. + * Checks in Google metadata server if instance was preempted. + * + * See https://cloud.google.com/compute/docs/instances/create-start-preemptible-instance#detecting_if_an_instance_was_preempted + * + * @return True if node was preempted. + * @throws IOException Exception when calling Google metadata API + */ @Override public Boolean call() throws IOException { HttpTransport transport = new NetHttpTransport(); @@ -44,7 +61,7 @@ public Boolean call() throws IOException { HttpRequest request = transport.createRequestFactory().buildGetRequest(metadata); request.setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google")); request.setReadTimeout(Integer.MAX_VALUE); - listener.getLogger().println("Preemptible instance, listening metadata for preemption event"); + listener.getLogger().println("Preemptive instance, listening metadata for preemption event"); HttpResponse response = request.execute(); final String result = IOUtils.toString(response.getContent(), Charsets.UTF_8); listener.getLogger().println("Got preemption event " + result); diff --git a/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties b/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties index 07d4a701..805db7f3 100644 --- a/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties +++ b/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties @@ -1,3 +1,4 @@ ComputeEngineCloud.DisplayName=Google Compute Engine ComputeEngineAgent.DisplayName=Google Compute Engine -InstanceConfiguration.SnapshotConfigError=One-shot must be enabled to create snapshots \ No newline at end of file +InstanceConfiguration.SnapshotConfigError=One-shot must be enabled to create snapshots +RebuildCause.ShortDescription=Rebuilding preempted job \ No newline at end of file From 007abb433f3ecc6e61157a651ec20c99b4b7f62d Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 19 Mar 2019 14:08:04 +0100 Subject: [PATCH 28/30] Fix message --- .../jenkins/plugins/computeengine/PreemptedCheckCallable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java index 5da97f22..fa7cdcb1 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java @@ -61,7 +61,7 @@ public Boolean call() throws IOException { HttpRequest request = transport.createRequestFactory().buildGetRequest(metadata); request.setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google")); request.setReadTimeout(Integer.MAX_VALUE); - listener.getLogger().println("Preemptive instance, listening metadata for preemption event"); + listener.getLogger().println("Preemptive instance, listening to metadata for preemption event"); HttpResponse response = request.execute(); final String result = IOUtils.toString(response.getContent(), Charsets.UTF_8); listener.getLogger().println("Got preemption event " + result); From 226970ef60de8ff5f14246923e2e4482c280dc09 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 19 Mar 2019 20:56:27 +0100 Subject: [PATCH 29/30] Fix message --- .../google/jenkins/plugins/computeengine/Messages.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties b/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties index 805db7f3..2ba598c6 100644 --- a/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties +++ b/src/main/resources/com/google/jenkins/plugins/computeengine/Messages.properties @@ -1,4 +1,4 @@ ComputeEngineCloud.DisplayName=Google Compute Engine ComputeEngineAgent.DisplayName=Google Compute Engine InstanceConfiguration.SnapshotConfigError=One-shot must be enabled to create snapshots -RebuildCause.ShortDescription=Rebuilding preempted job \ No newline at end of file +RebuildCause.ShortDescription=Rebuilding preempted job From 4df1339d00e09d8595321642237cc6f7d006bd80 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Tue, 19 Mar 2019 20:59:18 +0100 Subject: [PATCH 30/30] Fix license headers --- .../plugins/computeengine/ComputeEngineRetentionStrategy.java | 4 ++-- .../jenkins/plugins/computeengine/PreemptedCheckCallable.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java index d5daf88f..263a1863 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/ComputeEngineRetentionStrategy.java @@ -1,11 +1,11 @@ /* - * Copyright 2019 Google Inc. All Rights Reserved. + * Copyright 2019 Google LLC * * 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 + * https://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, diff --git a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java index fa7cdcb1..8d0ba6f9 100644 --- a/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java +++ b/src/main/java/com/google/jenkins/plugins/computeengine/PreemptedCheckCallable.java @@ -1,11 +1,11 @@ /* - * Copyright 2019 Google Inc. All Rights Reserved. + * Copyright 2019 Google LLC * * 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 + * https://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,