Skip to content

Commit

Permalink
fix not in list ValueError
Browse files Browse the repository at this point in the history
This error occured due to `cwlmain` within calrissian/main.py
returning before all the threads were finished. So the delete_pods
cleanup method was called then one of the threads processed
a message about a cleaned up pod. Not sure of the underlying
reason why cwlmain returned early. Perhaps there is another error
that this bug is masking.
  • Loading branch information
johnbradley committed Aug 20, 2019
1 parent ab0246f commit a249143
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
7 changes: 5 additions & 2 deletions calrissian/k8s.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,12 @@ def add(self, pod):
PodMonitor.pod_names.append(pod.metadata.name)

def remove(self, pod):
log.info('PodMonitor removing {}'.format(pod.metadata.name))
# This has to look up the pod by something unique
PodMonitor.pod_names.remove(pod.metadata.name)
if pod.metadata.name in PodMonitor.pod_names:
log.info('PodMonitor removing {}'.format(pod.metadata.name))
PodMonitor.pod_names.remove(pod.metadata.name)
else:
log.warning('PodMonitor {} has already been removed'.format(pod.metadata.name))

@staticmethod
def cleanup():
Expand Down
8 changes: 7 additions & 1 deletion tests/test_k8s.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,14 +366,20 @@ def test_delete_pods_calls_podmonitor(self, mock_pod_monitor):
self.assertTrue(mock_pod_monitor.cleanup.called)

@patch('calrissian.k8s.KubernetesClient')
def test_remove_after_cleanup(self, mock_client):
@patch('calrissian.k8s.log')
def test_remove_after_cleanup(self, mock_log, mock_client):
# Depending upon timing cleanup may get called before we receive a remove pod event
pod = self.make_mock_pod('pod-123')
with PodMonitor() as monitor:
monitor.add(pod)
PodMonitor.cleanup()
with PodMonitor() as monitor:
monitor.remove(pod)
mock_log.info.assert_has_calls([
call('PodMonitor adding pod-123'),
call('PodMonitor deleting pod pod-123'),
])
mock_log.warning.assert_called_with('PodMonitor pod-123 has already been removed')


class CompletionResultTestCase(TestCase):
Expand Down

0 comments on commit a249143

Please sign in to comment.