Includes job identfiers in the check to see if work can be assigned to an agent

Author: GaneshSPatil

src/main/java/cd/go/contrib/elasticagent/executors/ShouldAssignWorkRequestExecutor.java

@@ -18,18 +18,11 @@
 
 import cd.go.contrib.elasticagent.AgentInstances;
 import cd.go.contrib.elasticagent.KubernetesInstance;
-import cd.go.contrib.elasticagent.KubernetesInstanceFactory;
 import cd.go.contrib.elasticagent.RequestExecutor;
 import cd.go.contrib.elasticagent.requests.ShouldAssignWorkRequest;
 import com.thoughtworks.go.plugin.api.response.DefaultGoPluginApiResponse;
 import com.thoughtworks.go.plugin.api.response.GoPluginApiResponse;
 
-import java.util.HashMap;
-import java.util.Map;
-
-import static cd.go.contrib.elasticagent.KubernetesPlugin.LOG;
-import static org.apache.commons.lang3.StringUtils.stripToEmpty;
-
 public class ShouldAssignWorkRequestExecutor implements RequestExecutor {
     private final AgentInstances<KubernetesInstance> agentInstances;
     private final ShouldAssignWorkRequest request;
@@ -41,22 +34,13 @@ public ShouldAssignWorkRequestExecutor(ShouldAssignWorkRequest request, AgentIns
 
     @Override
     public GoPluginApiResponse execute() {
-        KubernetesInstance instance = agentInstances.find(request.agent().elasticAgentId());
+        KubernetesInstance pod = agentInstances.find(request.agent().elasticAgentId());
 
-        if (instance == null) {
+        if (pod == null) {
             return DefaultGoPluginApiResponse.success("false");
         }
 
-        boolean environmentMatches = stripToEmpty(request.environment()).equalsIgnoreCase(stripToEmpty(instance.environment()));
-
-
-        Map<String, String> containerProperties = instance.getInstanceProperties() == null ? new HashMap<>() : instance.getInstanceProperties();
-        Map<String, String> requestProperties = request.properties() == null ? new HashMap<>() : request.properties();
-
-        boolean propertiesMatch = requestProperties.equals(containerProperties);
-
-        if (environmentMatches && propertiesMatch) {
-            LOG.debug(String.format("[Should Assign Work] Assigning job[%s] to agent[%s]", request.properties(), request.agent().elasticAgentId()));
+        if (request.jobIdentifier().getJobId().equals(pod.jobId())) {
             return DefaultGoPluginApiResponse.success("true");
         }
 

src/main/java/cd/go/contrib/elasticagent/requests/ShouldAssignWorkRequest.java

@@ -21,6 +21,7 @@
 import cd.go.contrib.elasticagent.Request;
 import cd.go.contrib.elasticagent.RequestExecutor;
 import cd.go.contrib.elasticagent.executors.ShouldAssignWorkRequestExecutor;
+import cd.go.contrib.elasticagent.model.JobIdentifier;
 import com.google.gson.annotations.Expose;
 import com.google.gson.annotations.SerializedName;
 
@@ -35,20 +36,27 @@ public class ShouldAssignWorkRequest {
     @Expose
     @SerializedName("agent")
     private Agent agent;
+
     @Expose
     @SerializedName("environment")
     private String environment;
+
     @Expose
     @SerializedName("properties")
     private Map<String, String> properties;
 
-    public ShouldAssignWorkRequest(Agent agent, String environment, Map<String, String> properties) {
+    @Expose
+    @SerializedName("job_identifier")
+    private JobIdentifier jobIdentifier;
+
+    public ShouldAssignWorkRequest() {
+    }
+
+    public ShouldAssignWorkRequest(Agent agent, String environment, Map<String, String> properties, JobIdentifier jobIdentifier) {
         this.agent = agent;
         this.environment = environment;
         this.properties = properties;
-    }
-
-    public ShouldAssignWorkRequest() {
+        this.jobIdentifier = jobIdentifier;
     }
 
     public static ShouldAssignWorkRequest fromJSON(String json) {
@@ -70,4 +78,8 @@ public Map<String, String> properties() {
     public RequestExecutor executor(AgentInstances agentInstances) {
         return new ShouldAssignWorkRequestExecutor(this, agentInstances);
     }
+
+    public JobIdentifier jobIdentifier() {
+        return jobIdentifier;
+    }
 }

src/test/java/cd/go/contrib/elasticagent/executors/ShouldAssignWorkRequestExecutorTest.java

@@ -45,7 +45,6 @@
 import static org.mockito.MockitoAnnotations.initMocks;
 
 public class ShouldAssignWorkRequestExecutorTest extends BaseTest {
-    private final String environment = "production";
     @Mock
     KubernetesClientFactory factory;
     private AgentInstances<KubernetesInstance> agentInstances;
@@ -57,6 +56,7 @@ public class ShouldAssignWorkRequestExecutorTest extends BaseTest {
     private MixedOperation<Pod, PodList, DoneablePod, PodResource<Pod, DoneablePod>> mockedOperation;
     @Mock
     private NonNamespaceOperation<Pod, PodList, DoneablePod, PodResource<Pod, DoneablePod>> mockedNamespaceOperation;
+    private String environment = "QA";
 
     @Before
     public void setUp() throws Exception {
@@ -69,28 +69,23 @@ public void setUp() throws Exception {
         agentInstances = new KubernetesAgentInstances(factory);
         properties.put("foo", "bar");
         properties.put("Image", "gocdcontrib/ubuntu-docker-elastic-agent");
-        instance = agentInstances.create(new CreateAgentRequest(UUID.randomUUID().toString(), properties, environment, new JobIdentifier(1L)), createSettings(), null);
+        instance = agentInstances.create(new CreateAgentRequest(UUID.randomUUID().toString(), properties, environment, new JobIdentifier(100L)), createSettings(), null);
     }
 
     @Test
-    public void shouldAssignWorkToContainerWithMatchingEnvironmentNameAndProperties() throws Exception {
-        ShouldAssignWorkRequest request = new ShouldAssignWorkRequest(new Agent(instance.name(), null, null, null), environment, properties);
+    public void shouldAssignWorkWhenJobIdMatchesPodId() throws Exception {
+        JobIdentifier jobIdentifier = new JobIdentifier("test-pipeline", 1L, "Test Pipeline", "test-stage", "1", "test-job", 100L);
+        ShouldAssignWorkRequest request = new ShouldAssignWorkRequest(new Agent(instance.name(), null, null, null), environment, properties, jobIdentifier);
         GoPluginApiResponse response = new ShouldAssignWorkRequestExecutor(request, agentInstances).execute();
         assertThat(response.responseCode(), is(200));
         assertThat(response.responseBody(), is("true"));
     }
 
     @Test
-    public void shouldNotAssignWorkToContainerWithDifferentEnvironmentName() throws Exception {
-        ShouldAssignWorkRequest request = new ShouldAssignWorkRequest(new Agent(instance.name(), null, null, null), "FooEnv", properties);
-        GoPluginApiResponse response = new ShouldAssignWorkRequestExecutor(request, agentInstances).execute();
-        assertThat(response.responseCode(), is(200));
-        assertThat(response.responseBody(), is("false"));
-    }
-
-    @Test
-    public void shouldNotAssignWorkToContainerWithDifferentProperties() throws Exception {
-        ShouldAssignWorkRequest request = new ShouldAssignWorkRequest(new Agent(instance.name(), null, null, null), environment, null);
+    public void shouldNotAssignWorkWhenJobIdDiffersFromPodId() throws Exception {
+        long mismatchingJobId = 200L;
+        JobIdentifier jobIdentifier = new JobIdentifier("test-pipeline", 1L, "Test Pipeline", "test-stage", "1", "test-job", mismatchingJobId);
+        ShouldAssignWorkRequest request = new ShouldAssignWorkRequest(new Agent(instance.name(), null, null, null), "FooEnv", properties, jobIdentifier);
         GoPluginApiResponse response = new ShouldAssignWorkRequestExecutor(request, agentInstances).execute();
         assertThat(response.responseCode(), is(200));
         assertThat(response.responseBody(), is("false"));

src/test/java/cd/go/contrib/elasticagent/requests/ShouldAssignWorkRequestTest.java

@@ -17,13 +17,15 @@
 package cd.go.contrib.elasticagent.requests;
 
 import cd.go.contrib.elasticagent.Agent;
+import cd.go.contrib.elasticagent.model.JobIdentifier;
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 
 public class ShouldAssignWorkRequestTest {
@@ -40,6 +42,15 @@ public void shouldDeserializeFromJSON() throws Exception {
                 "  },\n" +
                 "  \"properties\": {\n" +
                 "    \"property_name\": \"property_value\"\n" +
+                "  },\n" +
+                "  \"job_identifier\": {\n" +
+                "    \"pipeline_name\": \"test-pipeline\",\n" +
+                "    \"pipeline_counter\": 1,\n" +
+                "    \"pipeline_label\": \"Test Pipeline\",\n" +
+                "    \"stage_name\": \"test-stage\",\n" +
+                "    \"stage_counter\": \"1\",\n" +
+                "    \"job_name\": \"test-job\",\n" +
+                "    \"job_id\": 100\n" +
                 "  }\n" +
                 "}";
 
@@ -49,5 +60,10 @@ public void shouldDeserializeFromJSON() throws Exception {
         HashMap<String, String> expectedProperties = new HashMap<>();
         expectedProperties.put("property_name", "property_value");
         assertThat(request.properties(), Matchers.<Map<String, String>>equalTo(expectedProperties));
+
+        JobIdentifier expectedJobIdentifier = new JobIdentifier("test-pipeline", 1L, "Test Pipeline", "test-stage", "1", "test-job", 100L);
+        JobIdentifier actualJobIdentifier = request.jobIdentifier();
+
+        assertThat(actualJobIdentifier, is(expectedJobIdentifier));
     }
 }