Fix logrotateadditionalfiles deletion.

[baconmania]

This PR aims to fix a scenario whereby logrotateAdditionalFiles marked with deleteInExecutorCleanup would never actually be deleted. The scenario was:

• Task terminates successfully
• Executor runs the “immediate” cleanup logic
• logrotateAdditionalFiles deletion is skipped because it hasn't been 15 minutes since task termination
• Cleanup logic flows all the way through to

  Singularity/SingularityExecutor/src/main/java/com/hubspot/singularity/executor/task/SingularityExecutorTaskCleanup.java

  Line 79 in 4dea957

  return finishTaskCleanup(dockerCleanSuccess);

• Task definition file is deleted
• Scheduled executor cleanup will never be aware of this sandbox again

I’ve plumbed through some logic for log cleanup which is similar to the logic for the sandbox app/ directory cleanup. Which is to say, if it hasn’t been 15 minutes since task termination, we return a WAITING state from the “immediate” cleanup process. This way, we allow scheduled cleanup to take another look at it later, and properly delete files which we had been logrotating for the lifetime of the task.

[baconmania]

Note that there are, broadly, three things to be cleaned up:

1. app dir
2. log files uploaded by Singularity (service.log and s3UploaderAdditionalFiles)
3. log files rotated by Singularity (logrotateAdditionalFiles)

With this PR, we're set up so that 2 and 3 are either performed or skipped together. Is this what we want? Should 1 and 2 happen together, with the 15 minute delay applying only to 3?

[ssalinas]

I think I agree that the 15 minute delay should apply to both 2 and 3. In particular because the S3Uploader needs a chance to get a at the logs for 2, which the 15 minute window is nice for.

Overall PR seems fine. I think it's gonna be a matter of testing the common cases in staging. Good find on where we clean the task definition file. Couldn't remember earlier where we actually were informed about a sandbox still hanging around

[baconmania]

Tested successfully. At task termination time, app directory cleanup and logrotation happen immediately. logrotateAdditionalFiles deletion is the only piece that's subject to the 15 minute delay. The scenario described above now proceeds as follows:

• Task terminates successfully
• Executor runs the “immediate” cleanup logic
• app/ directory is cleaned
• tail_of_service.log is created by grabbing a snippet of service.log
• service.log is deleted
• All configured logfiles that are configured to be rotated, are rotated
• Deletion of logrotateAdditionalFiles is skipped because it hasn't been 15 minutes since task termination
• The immediate cleanup process returns a WAITING state and the task definition file isn't deleted
• ... 15 minutes later...
• Scheduled cleanup process discovers the task definition file
• logrotateAdditionalFiles deletion proceeds
• Task definition file is deleted
• Scheduled cleanup process returns SUCCESS

[ssalinas]

🚢