|
5 | 5 | "errors" |
6 | 6 | "fmt" |
7 | 7 | "strings" |
| 8 | + "time" |
8 | 9 |
|
9 | 10 | "github.com/labstack/onebox/internal/app" |
10 | 11 | "github.com/labstack/onebox/internal/journal" |
@@ -43,7 +44,22 @@ func (e *Engine) RunJobWithJournalID(ctx context.Context, request JobRunRequest) |
43 | 44 | if err != nil { |
44 | 45 | return operationID, nil, err |
45 | 46 | } |
46 | | - defer e.ReleaseLock(ctx) |
| 47 | + // Released unless an interrupted run left this operation's container alive. |
| 48 | + // ReleaseLock runs on its own background context, so on Ctrl-C it succeeds |
| 49 | + // while the terminal journal append — which uses the cancelled one — does |
| 50 | + // not: ownership would be dropped, immediately and silently, over a |
| 51 | + // container still changing data. |
| 52 | + holdLockForLiveContainer := false |
| 53 | + defer func() { |
| 54 | + if holdLockForLiveContainer { |
| 55 | + e.warnf("operation %s was interrupted while its container is still running; "+ |
| 56 | + "keeping the application lock so nothing else mutates alongside it. "+ |
| 57 | + "Inspect with `docker ps --filter label=%s=%s`; the lock expires on its own after %s", |
| 58 | + operationID, JobOperationLabel, operationID, e.lockTTL()) |
| 59 | + return |
| 60 | + } |
| 61 | + e.ReleaseLock(ctx) |
| 62 | + }() |
47 | 63 | if err := e.WriteFence(ctx, operationID, epoch); err != nil { |
48 | 64 | return operationID, nil, err |
49 | 65 | } |
@@ -93,7 +109,42 @@ func (e *Engine) RunJobWithJournalID(ctx context.Context, request JobRunRequest) |
93 | 109 | record.Status = "fail" |
94 | 110 | record.Detail = runErr.Error() |
95 | 111 | } |
96 | | - if journalErr := writer.Append(ctx, record); journalErr != nil { |
| 112 | + // A cancelled context is exactly when the terminal record matters most, |
| 113 | + // and exactly when appending on that context cannot work. `ob exec` |
| 114 | + // already writes its own on a bounded background context; without the |
| 115 | + // same here an interrupted job stays INCOMPLETE in `ob audit` forever, |
| 116 | + // with no record that it was ever interrupted. Append redacts Detail on |
| 117 | + // a failure, so the reason has to ride on ErrorCode. |
| 118 | + // Two independent questions. WHERE to append: a cancelled caller context |
| 119 | + // cannot carry the write, whatever the run did, so a job that finished |
| 120 | + // cleanly a moment before Ctrl-C still records its success. WHAT to |
| 121 | + // record: only a run that ended because the client went away is |
| 122 | + // interrupted — an outcome the job itself produced is its own. |
| 123 | + journalContext := ctx |
| 124 | + if ctx.Err() != nil { |
| 125 | + var cancel context.CancelFunc |
| 126 | + journalContext, cancel = context.WithTimeout(context.Background(), journalCleanupTimeout) |
| 127 | + defer cancel() |
| 128 | + } |
| 129 | + if interruptedRun(ctx, runErr) { |
| 130 | + record = journal.Record{ |
| 131 | + Phase: "job", Event: "finish", Status: "fail", ErrorCode: "interrupted", |
| 132 | + OperationKind: "job_run", Service: job, |
| 133 | + } |
| 134 | + } |
| 135 | + journalErr := writer.Append(journalContext, record) |
| 136 | + if journalErr != nil && journalContext == ctx && ctx.Err() != nil { |
| 137 | + // Cancellation can land during the write as easily as before it, and |
| 138 | + // the check above only sees a context that was already gone. Retried |
| 139 | + // only when the context died in the meantime: any other failure — |
| 140 | + // a full disk, a refused write — may have landed on the host after |
| 141 | + // reporting an error, and appending a second terminal record is |
| 142 | + // worse than reporting the first failure. |
| 143 | + retryContext, cancel := context.WithTimeout(context.Background(), journalCleanupTimeout) |
| 144 | + defer cancel() |
| 145 | + journalErr = writer.Append(retryContext, record) |
| 146 | + } |
| 147 | + if journalErr != nil { |
97 | 148 | return errors.Join(runErr, fmt.Errorf("journal job finish: %w", journalErr)) |
98 | 149 | } |
99 | 150 | return runErr |
@@ -121,10 +172,52 @@ func (e *Engine) RunJobWithJournalID(ctx context.Context, request JobRunRequest) |
121 | 172 | e.gateOpen = true |
122 | 173 | e.rollbackCovered = true |
123 | 174 | runErr := e.runJobPhase(ctx, writer, nil, remoteDir, remoteCompose, "job", []string{job}) |
| 175 | + if interruptedRun(ctx, runErr) { |
| 176 | + // Cancelling the client kills at most the wrapper shell; the container |
| 177 | + // belongs to the daemon and keeps running. |
| 178 | + holdLockForLiveContainer = e.jobContainerRunning(operationID) |
| 179 | + } |
124 | 180 | var result *journal.JobResultEvidence |
125 | 181 | if evidence, ok := e.jobResults[job]; ok { |
126 | 182 | resultCopy := evidence |
127 | 183 | result = &resultCopy |
128 | 184 | } |
129 | 185 | return operationID, result, finish(runErr) |
130 | 186 | } |
| 187 | + |
| 188 | +const journalCleanupTimeout = 5 * time.Second |
| 189 | + |
| 190 | +// interruptedRun reports a run that ended because the client went away rather |
| 191 | +// than because the job finished. |
| 192 | +// |
| 193 | +// A run that produced no error finished, whatever became of the client |
| 194 | +// afterwards — its outcome is its own and must be recorded as such. Only once |
| 195 | +// the run failed does a gone context mean the client is why. The transport does |
| 196 | +// not always surface a cancelled context as context.Canceled, so a failure |
| 197 | +// under a cancelled context counts even when the error says something else. |
| 198 | +func interruptedRun(ctx context.Context, runErr error) bool { |
| 199 | + if runErr == nil { |
| 200 | + return false |
| 201 | + } |
| 202 | + return ctx.Err() != nil || |
| 203 | + errors.Is(runErr, context.Canceled) || |
| 204 | + errors.Is(runErr, context.DeadlineExceeded) |
| 205 | +} |
| 206 | + |
| 207 | +// jobContainerRunning answers whether this operation's one-off container is |
| 208 | +// still alive, on a context of its own because the caller's is already gone. |
| 209 | +// An unreadable answer is reported as running: keeping the lock over a |
| 210 | +// container that has in fact exited costs an operator one `--break-lock`, while |
| 211 | +// releasing it over one that has not costs them concurrent writers. |
| 212 | +func (e *Engine) jobContainerRunning(operationID string) bool { |
| 213 | + ctx, cancel := context.WithTimeout(context.Background(), journalCleanupTimeout) |
| 214 | + defer cancel() |
| 215 | + res, err := e.T.Run(ctx, "docker ps -q --filter label="+q(JobOperationLabel+"="+operationID)) |
| 216 | + if err != nil { |
| 217 | + return true |
| 218 | + } |
| 219 | + if res.ExitCode != 0 { |
| 220 | + return true |
| 221 | + } |
| 222 | + return strings.TrimSpace(res.Stdout) != "" |
| 223 | +} |
0 commit comments