Ensure that timed out jobsteps are marked as finished

Summary:
This change ensures that, regardless of implementation, we consistently considered
timed out job steps to be finished.
This should avoid the possibility of timed out or otherwise lost steps from things like
LXCBuildStep being canceled repeatedly without effect.

Test Plan: Light unit.

Reviewers: vishal

Reviewed By: vishal

Subscribers: changesbot, wwu

Differential Revision: https://tails.corp.dropbox.com/D91471

changes/constants.py

@@ -21,6 +21,12 @@ def __str__(self):
 
 class Result(Enum):
     unknown = 0
+    # 'aborted' indicates intentional cancellation or an unrecoverable error.
+    #
+    # Operations that time out are not considered aborted, even though the timeout
+    # may result in intentional cancellation or be the result of an infrastructural failure.
+    # This is because we don't usually know the cause of timeouts, so we assume build failure to
+    # be safe.
     aborted = 5
     passed = 1
     skipped = 3

changes/jobs/sync_job_step.py

@@ -146,6 +146,14 @@ def sync_job_step(step_id):
         if has_timed_out(step, jobplan):
             implementation.cancel_step(step=step)
 
+            # Not all implementations can actually cancel, but it's dead to us as of now
+            # so we mark it as finished.
+            step.status = Status.finished
+            step.date_finished = datetime.utcnow()
+
+            # Implementations default to marking canceled steps as aborted,
+            # but we're not canceling on good terms (it should be done by now)
+            # so we consider it a failure here.
             step.result = Result.failed
             db.session.add(step)
 

tests/changes/jobs/test_sync_job_step.py

@@ -438,6 +438,7 @@ def test_timed_out(self, get_implementation, mock_has_timed_out):
         )
 
         assert step.result == Result.failed
+        assert step.status == Status.finished
 
         assert FailureReason.query.filter(
             FailureReason.step_id == step.id,