Merge pull request #37 from bdpiparva/avoid-terminated-pod

Avoid already terminated pods while checking for unregistered pods.
gocd · Mar 16, 2018 · 7daecf3 · 7daecf3
2 parents faa7579 + 63c64b0
commit 7daecf3
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 4 deletions.
diff --git a/src/main/java/cd/go/contrib/elasticagent/KubernetesAgentInstances.java b/src/main/java/cd/go/contrib/elasticagent/KubernetesAgentInstances.java
@@ -127,8 +127,8 @@ public void terminateUnregisteredInstances(PluginSettings settings, Agents agent
         }
 
         LOG.warn(format("Terminating instances that did not register {0}.", toTerminate.instances.keySet()));
-        for (KubernetesInstance container : toTerminate.instances.values()) {
-            terminate(container.name(), settings);
+        for (String podName : toTerminate.instances.keySet()) {
+            terminate(podName, settings);
         }
     }
 
@@ -154,6 +154,7 @@ public void refreshAll(PluginRequest pluginRequest) {
         KubernetesClient client = factory.client(pluginRequest.getPluginSettings());
         PodList list = client.pods().list();
 
+        instances.clear();
         for (Pod pod : list.getItems()) {
             Map<String, String> podLabels = pod.getMetadata().getLabels();
             if (podLabels != null) {
@@ -162,6 +163,8 @@ public void refreshAll(PluginRequest pluginRequest) {
                 }
             }
         }
+
+        LOG.info(String.format("[refresh-pod-state] Pod information successfully synced. All(Running/Pending) pod count is %d.", instances.size()));
     }
 
     @Override
@@ -182,7 +185,13 @@ private KubernetesAgentInstances unregisteredAfterTimeout(PluginSettings setting
             if (knownAgents.containsAgentWithId(instanceName)) {
                 continue;
             }
-            Pod pod = client.pods().withName(instanceName).get();
+
+            Pod pod = getPod(client, instanceName);
+            if (pod == null) {
+                LOG.debug(String.format("[server-ping] Pod with name %s is already deleted.", instanceName));
+                continue;
+            }
+
             Date createdAt = getSimpleDateFormat().parse(pod.getMetadata().getCreationTimestamp());
             DateTime dateTimeCreated = new DateTime(createdAt);
 
@@ -193,6 +202,15 @@ private KubernetesAgentInstances unregisteredAfterTimeout(PluginSettings setting
         return unregisteredInstances;
     }
 
+    private Pod getPod(KubernetesClient client, String instanceName) {
+        try {
+            return client.pods().withName(instanceName).get();
+        } catch (Exception e) {
+            LOG.warn(String.format("[server-ping] Failed to fetch pod[%s] information:", instanceName), e);
+            return null;
+        }
+    }
+
     public boolean instanceExists(KubernetesInstance instance) {
         return instances.contains(instance);
     }

diff --git a/src/test/java/cd/go/contrib/elasticagent/KubernetesAgentInstancesTest.java b/src/test/java/cd/go/contrib/elasticagent/KubernetesAgentInstancesTest.java
@@ -19,6 +19,7 @@
 import cd.go.contrib.elasticagent.model.JobIdentifier;
 import cd.go.contrib.elasticagent.requests.CreateAgentRequest;
 import io.fabric8.kubernetes.api.model.DoneablePod;
+import io.fabric8.kubernetes.api.model.ObjectMeta;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodList;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -30,9 +31,12 @@
 import org.mockito.InOrder;
 import org.mockito.Mock;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Map;
 
+import static cd.go.contrib.elasticagent.Constants.JOB_ID_LABEL_KEY;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;
@@ -60,6 +64,8 @@ public class KubernetesAgentInstancesTest {
     @Mock
     private MixedOperation<Pod, PodList, DoneablePod, PodResource<Pod, DoneablePod>> mockedOperation;
 
+    @Mock
+    private PodList podList;
     private HashMap<String, String> testProperties;
 
     @Before
@@ -72,7 +78,6 @@ public void setUp() {
         JobIdentifier jobId = new JobIdentifier("test", 1L, "Test pipeline", "test name", "1", "test job", 100L);
         when(mockCreateAgentRequest.jobIdentifier()).thenReturn(jobId);
 
-        final PodList podList = mock(PodList.class);
         when(mockKubernetesClient.pods()).thenReturn(mockedOperation);
         when(mockPluginRequest.getPluginSettings()).thenReturn(mockPluginSettings);
         when(mockedOperation.list()).thenReturn(podList);
@@ -117,6 +122,18 @@ public void shouldNotCreatePodWhenOutstandingRequestsExistForJobs() {
         verify(mockKubernetesInstanceFactory, times(1)).create(any(), any(), any(), any(), any());
         reset(mockKubernetesInstanceFactory);
 
+        final Map<String, String> labels = new HashMap<>();
+        labels.put(JOB_ID_LABEL_KEY, jobId.getJobId().toString());
+        labels.put(Constants.KUBERNETES_POD_KIND_LABEL_KEY, Constants.KUBERNETES_POD_KIND_LABEL_VALUE);
+
+        final Pod pod = mock(Pod.class);
+        final ObjectMeta objectMeta = mock(ObjectMeta.class);
+        when(pod.getMetadata()).thenReturn(objectMeta);
+        when(objectMeta.getLabels()).thenReturn(labels);
+        when(objectMeta.getName()).thenReturn("test-agent");
+        when(podList.getItems()).thenReturn(Arrays.asList(pod));
+        when(mockKubernetesInstanceFactory.fromKubernetesPod(pod)).thenReturn(kubernetesInstance);
+
         agentInstances.create(mockCreateAgentRequest, mockPluginSettings, mockPluginRequest);
         verify(mockKubernetesInstanceFactory, times(0)).create(any(), any(), any(), any(), any());
     }
@@ -140,6 +157,18 @@ public void shouldNotCreatePodsWhenOutstandingLimitOfPendingKubernetesPodsHasRea
         verify(mockKubernetesInstanceFactory, times(1)).create(any(), any(), any(), any(), any());
         reset(mockKubernetesInstanceFactory);
 
+        final Map<String, String> labels = new HashMap<>();
+        labels.put(JOB_ID_LABEL_KEY, jobId.getJobId().toString());
+        labels.put(Constants.KUBERNETES_POD_KIND_LABEL_KEY, Constants.KUBERNETES_POD_KIND_LABEL_VALUE);
+
+        final Pod pod = mock(Pod.class);
+        final ObjectMeta objectMeta = mock(ObjectMeta.class);
+        when(pod.getMetadata()).thenReturn(objectMeta);
+        when(objectMeta.getLabels()).thenReturn(labels);
+        when(objectMeta.getName()).thenReturn("test-agent");
+        when(podList.getItems()).thenReturn(Arrays.asList(pod));
+        when(mockKubernetesInstanceFactory.fromKubernetesPod(pod)).thenReturn(kubernetesInstance);
+
         //second create agent request
         agentInstances.create(mockCreateAgentRequest, mockPluginSettings, mockPluginRequest);
         verify(mockKubernetesInstanceFactory, times(0)).create(any(), any(), any(), any(), any());