Allow for a custom timeout on exec ops

[mikejholly]

This is working well, but it doesn't look fantastic. It would be nice to report back to the user that the command has exceeded the max time.

/tmp+b | DOCKERHUB_AUTH=true
/tmp+b | --> RUN --no-cache sleep 10
/tmp+b | WARN /tmp/Earthfile line 11:4: The command 'RUN --no-cache sleep 10' failed: process "/bin/sh -c PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin /usr/bin/earth_debugger /bin/sh -c 'sleep 10'" did not complete successfully: exit code: 137: context deadline exceeded
Error: DeadlineExceeded: async force execution for /tmp+basic: unlazy force execution: DeadlineExceeded: process "/bin/sh -c PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin /usr/bin/earth_debugger /bin/sh -c 'sleep 10'" did not complete successfully: exit code: 137: context deadline exceeded
View logs at https://cloud.earthly.dev/builds/a455e474-35ec-453d-bbe8-9d7508cd0061

[vladaionescu]

It would be nice to report back to the user that the command has exceeded the max time.

Agreed - I think this is critical - otherwise we'll just get a ton of questions about it.

Perhaps we can do something similar to this in a defer, such that we repackage the error with something more meaningful: https://github.com/earthly/buildkit/blob/earthly-main/solver/jobs.go#L933C1-L942C4.

[alexcb]

I'm just thinking about edge cases here -- Have you tested that the process gets killed versus go returning an error while the process continues to run?

e.g. does ps show the sleep command continuing to run?

[alexcb]

side note: should we have earthly filter out error messages like "async force execution" and "unlazy force execution", and just print the underlying errors? I don't think the unlazy message is useful to anyone aside from core developers.

[alexcb]

also, is there a corresponding earthly PR for this? We require the two to stay in sync for test coverage, as outlined under https://github.com/earthly/earthly/blob/main/CONTRIBUTING.md#updates-to-buildkit-or-fsutil

[mikejholly]

@vladaionescu ok, I'll take a closer look at customizing the error message text.

[mikejholly]

I'm just thinking about edge cases here -- Have you tested that the process gets killed versus go returning an error while the process continues to run?

e.g. does ps show the sleep command continuing to run?

Not yet, but good idea.

[mikejholly]

also, is there a corresponding earthly PR for this? We require the two to stay in sync for test coverage, as outlined under https://github.com/earthly/earthly/blob/main/CONTRIBUTING.md#updates-to-buildkit-or-fsutil

I haven't had a chance yet but I will 🙂

[mikejholly]

side note: should we have earthly filter out error messages like "async force execution" and "unlazy force execution", and just print the underlying errors? I don't think the unlazy message is useful to anyone aside from core developers.

Yeah, I think async force execution is basically meaningless from a user's point of view (to me anyway!).

[mikejholly]

Good news! ps doesn't show the process after the timeout is hit.

[alexcb]

nit: squash these commits prior to merging.

[mikejholly]

@alexcb Squashed. Look good?

[idodod]

@vladaionescu ok, I'll take a closer look at customizing the error message text.

you can wrap it in a hint error and/or handle the error type in run.go

[mikejholly]

@idodod Thanks. That'll be something to potentially clean up on the Earthly side.

Here's an example with this BK version:

/tmp+basic | --> RUN --no-cache sleep 100
/tmp+basic | WARN /tmp/Earthfile line 6:4: The command 'RUN --no-cache sleep 100' failed: max execution time of 2s exceeded
Error: async force execution for /tmp+basic: unlazy force execution: max execution time of 2s exceeded
View logs at https://cloud.earthly.dev/builds/ccbb2769-ba32-4f74-a40c-4a3f69a630ff

[vladaionescu]

That'll be something to potentially clean up on the Earthly side

Agreed - on the Earthly side we might want to say a bit more explicitly something like "Unverified accounts have a limit on the duration of RUN commands. Verify your account to lift this restriction". I think that what you have now in this PR is a good string to grep for on the Earthly side.