Skip to content

Commit

Permalink
fix: remove job using region
Browse files Browse the repository at this point in the history 
Jobs created in other region than `global` are not removed when worker
is stopped.

This fix doesn't handle multi region yet.
  • Loading branch information
@jfroche
jfroche committed May 1, 2022
1 parent 0c2cc15 commit d67391d
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 23 deletions.
60 changes: 47 additions & 13 deletions src/main/java/org/jenkinsci/plugins/nomad/NomadApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
import java.util.logging.Level;
import java.security.GeneralSecurityException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.logging.Logger;
import java.util.Map;

import hudson.util.FormValidation;
import hudson.util.Secret;
import okhttp3.Call;
import okhttp3.HttpUrl;
import okhttp3.MediaType;
import okhttp3.OkHttpClient;
import okhttp3.Request;
Expand All @@ -46,7 +49,7 @@ public final class NomadApi {
* @return FormValidation object with kind = OK or ERROR and a message.
*/
public FormValidation checkConnection() {
Request request = createRequestBuilder("/v1/agent/self")
Request request = createRequestBuilder("/v1/agent/self", null)
.build();

try (Response response = executeRequest(request)) {
Expand All @@ -72,7 +75,7 @@ public FormValidation checkConnection() {
public FormValidation validateTemplate(NomadWorkerTemplate template) {
String id = UUID.randomUUID().toString();

Request request = createRequestBuilder("/v1/job/" + id + "/plan")
Request request = createRequestBuilder("/v1/job/" + id + "/plan", null)
.post(RequestBody.create(buildWorkerJob(id, "", template), JSON))
.build();

Expand Down Expand Up @@ -104,7 +107,7 @@ public String startWorker(String workerName, String jnlpSecret, NomadWorkerTempl

LOGGER.log(Level.FINE, workerJob);

Request request = createRequestBuilder("/v1/jobs")
Request request = createRequestBuilder("/v1/jobs", null)
.put(RequestBody.create(workerJob, JSON))
.build();

Expand All @@ -117,13 +120,16 @@ public String startWorker(String workerName, String jnlpSecret, NomadWorkerTempl
* or not.
* @param workerName Name of the corresponding {@link NomadWorker} (e.g. jenkins-1234)
* @param namespace Name of the nomad namespace where job is running
* @param region Name of the region where job is running
*/
public void stopWorker(String workerName, String namespace) {
String nsParameter = "";
public void stopWorker(String workerName, String namespace, String region) {
Map<String,String> params = new HashMap<>();
if (namespace != null)
nsParameter = "?namespace=" + namespace;
params.put("namespace", namespace);
if (region != null && !region.equals("global"))
params.put("region", region);

Request request = createRequestBuilder("/v1/job/" + workerName + nsParameter)
Request request = createRequestBuilder("/v1/job/" + workerName, params)
.delete()
.build();

Expand All @@ -137,14 +143,34 @@ public void stopWorker(String workerName, String namespace) {
* @return Array of {@link JobInfo} objects or an empty list if there are no Jobs at all or when something was wrong
*/
public JobInfo[] getRunningWorkers(String prefix) {
Map<String,String> params = new HashMap<>();
params.put("namespace", "*");
params.put("prefix", prefix);

Request request = createRequestBuilder("/v1/jobs?namespace=*&prefix=" + prefix)
Request request = createRequestBuilder("/v1/jobs", params)
.get()
.build();
String body = checkResponseAndGetBody(request);
return new Gson().fromJson(body, JobInfo[].class);
}

/**
* Get all job specifications and status
* @param jobID Id of the job
* @param namespace Name of the nomad namespace where job is running
* @return {@link JSONObject} object
*/
public JSONObject getRunningWorker(String jobID, String namespace) {
Map<String,String> params = new HashMap<>();
if (namespace != null)
params.put("namespace", namespace);
Request request = createRequestBuilder("/v1/jobs/" + jobID, params)
.get()
.build();
String body = checkResponseAndGetBody(request);
return new JSONObject(body);
}

/**
* Creates from a given job template a Nomad Job which can be sent to Nomad.
* @param name Name of the Nomad Job (e.g. jenkins-1234)
Expand Down Expand Up @@ -179,7 +205,7 @@ private String normalizeJobTemplate(String jobTemplate) {
JsonObject jobHCL = new JsonObject();
jobHCL.addProperty("JobHCL", jobTemplate);

Request request = createRequestBuilder("/v1/jobs/parse")
Request request = createRequestBuilder("/v1/jobs/parse", null)
.post(RequestBody.create(gson.toJson(jobHCL), JSON))
.build();

Expand Down Expand Up @@ -264,10 +290,18 @@ private Response executeRequest(Request request) throws IOException {
* Provides a new request builder with a fresh Nomad token (if necessary).
* @param path Relative path to a Nomad resource (e.g. /v1/agent/self)
*/
private Request.Builder createRequestBuilder(String path) {
Request.Builder builder = new Request.Builder()
.url(cloud.getNomadUrl()+path);

private Request.Builder createRequestBuilder(String path, Map<String,String> params) {
Request.Builder builder = new Request.Builder();
HttpUrl httpUrl = HttpUrl.parse(cloud.getNomadUrl()+path);
if (httpUrl == null)
return builder;
HttpUrl.Builder httpBuilder = httpUrl.newBuilder();
if (params != null) {
for(Map.Entry<String, String> param : params.entrySet()) {
httpBuilder.addQueryParameter(param.getKey(),param.getValue());
}
}
builder.url(httpBuilder.build());
String nomadToken = cloud.getNomadACL();
if (StringUtils.isNotEmpty(nomadToken)) {
builder = builder.addHeader("X-Nomad-Token", nomadToken);
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/org/jenkinsci/plugins/nomad/NomadCloud.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.logging.Logger;

import org.jenkinsci.plugins.nomad.Api.JobInfo;
import org.jenkinsci.plugins.nomad.Api.JobSummary;
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
import org.json.JSONObject;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -163,8 +164,12 @@ private void pruneOrphanedWorkers(NomadWorkerTemplate template) {
Node node = Jenkins.get().getNode(worker.getName());

if (node == null) {
LOGGER.log(Level.FINE, "Found Orphaned Node: " + worker.getID() + " in namespace " + worker.getJobSummary().getNamespace());
this.nomad.stopWorker(worker.getID(), worker.getJobSummary().getNamespace());
JobSummary jobSummary = worker.getJobSummary();
String jobNamespace = worker.getJobSummary().getNamespace();
JSONObject job = this.nomad.getRunningWorker(jobSummary.getJobID(), jobNamespace);
String jobRegion = job.getString("Region");
this.nomad.stopWorker(worker.getID(), jobNamespace, jobRegion);
LOGGER.log(Level.FINE, "Found Orphaned Node: " + worker.getID() + " in namespace " + jobNamespace + " in region " + jobRegion);
}
}
}
Expand Down Expand Up @@ -344,11 +349,12 @@ public Node call() throws Exception {
LOGGER.log(Level.INFO, "Asking Nomad to schedule new Jenkins worker");

String workerJob = nomad.startWorker(workerName, jnlpSecret, template);
JSONObject workerJobJSON = new JSONObject(workerJob);
String namespace = workerJobJSON.getJSONObject("Job").optString("Namespace");
JSONObject workerJobJSON = new JSONObject(workerJob).getJSONObject("Job");
String namespace = workerJobJSON.optString("Namespace");
if (!namespace.equals("")) {
worker.setNamespace(namespace);
}
worker.setRegion(workerJobJSON.optString("Region"));

// Check scheduling success
Callable<Boolean> callableTask = () -> {
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/org/jenkinsci/plugins/nomad/NomadWorker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class NomadWorker extends AbstractCloudSlave implements EphemeralNode {
private final String cloudName;
private final int idleTerminationInMinutes;
private String namespace;
private String region;

@DataBoundConstructor
public NomadWorker(String name, String cloudName, String labelString, int numExecutors, int idleTerminationInMinutes,
Expand All @@ -38,6 +39,7 @@ public NomadWorker(String name, String cloudName, String labelString, int numExe
this.reusable = reusable;
this.idleTerminationInMinutes = idleTerminationInMinutes;
this.namespace = null;
this.region = null;
}

@Override
Expand All @@ -53,7 +55,7 @@ public AbstractCloudComputer<NomadWorker> createComputer() {
@Override
protected void _terminate(TaskListener listener) {
LOGGER.log(Level.INFO, "Asking Nomad to deregister worker '" + getNodeName() + "' in namespace '" + getNamespace() + "'");
getCloud().nomad().stopWorker(getNodeName(), getNamespace());
getCloud().nomad().stopWorker(getNodeName(), getNamespace(), getRegion());
}

public NomadCloud getCloud() {
Expand Down Expand Up @@ -100,4 +102,12 @@ public void setNamespace(String namespace) {
this.namespace = namespace;
}

public String getRegion() {
return this.region;
}

public void setRegion(String region) {
this.region = region;
}

}
54 changes: 49 additions & 5 deletions src/test/java/org/jenkinsci/plugins/nomad/NomadApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,57 @@ public void testStopWorker() {
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

// WHEN
api.stopWorker(workerName, null);
api.stopWorker(workerName, null, null);

// THEN
verify(deleteRequestedFor(urlEqualTo("/v1/job/" + workerName)));
}

@Test
public void testStopWorkerWithNS() {
// GIVEN
String workerName = UUID.randomUUID().toString();
stubFor(delete(urlEqualTo("/v1/job/" + workerName + "?namespace=ns1"))
.willReturn(ok()));
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

// WHEN
api.stopWorker(workerName, "ns1", null);

// THEN
verify(deleteRequestedFor(urlEqualTo("/v1/job/" + workerName + "?namespace=ns1")));
}

@Test
public void testStopWorkerWithNSAndRegion() {
// GIVEN
String workerName = UUID.randomUUID().toString();
stubFor(delete(urlEqualTo("/v1/job/" + workerName + "?namespace=ns1&region=regionA"))
.willReturn(ok()));
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

// WHEN
api.stopWorker(workerName, "ns1", "regionA");

// THEN
verify(deleteRequestedFor(urlEqualTo("/v1/job/" + workerName + "?namespace=ns1&region=regionA")));
}

@Test
public void testStopWorkerWithNSAndGlobalRegion() {
// GIVEN
String workerName = UUID.randomUUID().toString();
stubFor(delete(urlEqualTo("/v1/job/" + workerName + "?namespace=ns1"))
.willReturn(ok()));
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

// WHEN
api.stopWorker(workerName, "ns1", "global");

// THEN
verify(deleteRequestedFor(urlEqualTo("/v1/job/" + workerName + "?namespace=ns1")));
}

@Test
public void testValidateTemplateJSON() {
// GIVEN
Expand Down Expand Up @@ -167,7 +212,7 @@ public void testValidateTemplateHCL() {
@Test
public void testGetJobs() {
// GIVEN
stubFor(get(urlMatching("/v1/jobs\\?namespace=\\*&prefix=(.*)"))
stubFor(get(urlMatching("/v1/jobs\\?prefix=jenkins&namespace=\\*"))
.willReturn(ok("[{\"ID\":\"jenkins-A\",\"Name\":\"jenkins-A\",\"Priority\":50,\"Status\":\"pending\"}]")));
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

Expand All @@ -185,7 +230,7 @@ public void testGetJobs() {
@Test
public void testGetJobsWithNamespace() {
// GIVEN
stubFor(get(urlMatching("/v1/jobs\\?namespace=\\*&prefix=(.*)"))
stubFor(get(urlMatching("/v1/jobs\\?prefix=jenkins&namespace=\\*"))
.willReturn(ok("[{\"ID\":\"jenkins-A\",\"Name\":\"jenkins-A\",\"Priority\":50,\"Status\":\"pending\", \"JobSummary\": { \"JobID\": \"example\", \"Namespace\": \"ns1\" } }]")));
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

Expand All @@ -204,11 +249,10 @@ public void testGetJobsWithNamespace() {
assertThat(jobSummary.getNamespace(), is("ns1"));
}


@Test
public void testGetJobsIsEmpty() {
// GIVEN
stubFor(get(urlMatching("/v1/jobs\\?namespace=\\*&prefix=(.*)"))
stubFor(get(urlMatching("/v1/jobs\\?prefix=jenkins&namespace=\\*"))
.willReturn(ok("[]")));
when(cloud.getNomadUrl()).thenReturn(wireMockRule.baseUrl());

Expand Down

0 comments on commit d67391d

Please sign in to comment.