move UnknownExitStatus to executor package from errdefs

[coryb]

resolves #1811

moved the variable from errdefs to be next to the executor interfaces

also renaming the variable since it is returned from both runc and container executor

[tonistiigi]

ci for darwin:

#25 [linux/amd64 buildctl 1/1] RUN --mount=target=. --mount=target=/root/.ca...
#25 141.5 # github.com/moby/buildkit/snapshot
#25 141.5 snapshot/localmounter_unix.go:58:38: undefined: syscall.MNT_DETACH
#25 ...

executor is not a good place for this if it is included on the client. Has a similar problem #1808 was fixing.

[coryb]

Odd, I didn't get a build failure notification, must be something with the new GHA/Travis integrations? I moved it to frontend/gateway/client since clients will likely want to access that value the error returned from ContainerProcess.Wait

[coryb]

Another option might be to keep the UnknownExitStatus with the errdefs.ExitError but move it out of solver/errdefs, maybe we create a frontend/gateway/errdefs package for ExitError?

[thaJeztah]

yes, I was struggling to find a "correct" place as well #1808 (comment)

also renaming the variable since it is returned from both runc and container executor

So I added the Containerd prefix because that's the package it originally came from. Perhaps it reveals an issue that it was just used "for convenience" in other places, but no relation with the value defined in containerd.

If so, perhaps we need separate consts; one to detect a containerd "unknown error", and one to generate our own "unknown error" ?

[coryb]

yeah, makes sense. I ended up using the continerd.UnknownExitStatus in the runc executor because there was no equivalent const there that I saw and I wanted the "unknown" exit status to be consistent between our runc and container workers. There are some assumptions in the code that the containerd.UnknownExitStatus will be consistent with the UnknownExitStatus that we copied, but to ensure that I could just add a trivial unit test to assert they are the same?

@tiborvass left a comment that solve/errdefs was likely not the right place, which I agree with, I put the ExitError there originally because I didnt know where else to put it. errdefs.ExitError is currently only used in executor and frontend/gateway packages, so I think it makes sense to create a new errdefs package under one of those. I am leaning towards executor/errdefs at the moment. And if we do this then I also think keeping the UnknownExitStatus const in the errdefs package makes sense.

Any agreement/dissent/thoughts?

[coryb]

I refactored the code to move the ExitError and UnknownExitStatus to a frontend/gateway/errdefs package.

Not sure what the codecov/patch "failure" means, it seems like a false alarm to me.