Fail when cache check should have succeeded #3996
Conversation
Codecov Report
|
| color.Yellow.Fprintln(out, "Error checking cache. Rebuilding.") | ||
| needToBuild = append(needToBuild, artifact) | ||
| continue | ||
| color.Red.Fprintln(out, "Error checking cache.") |
There was a problem hiding this comment.
So it seems odd to allow caching to cause a build failure. But your claim is that cache failures are due to problems checking registries or the local daemon, which will fail anyways after building, so we may as well error out now. That seems reasonable. (I'll note that the remote check doesn't actually seem to return a failure
)If we're going to fail, shouldn't we output the error here?
There was a problem hiding this comment.
Exactly, they will fail anyways down the line (for local builders only). Why would we print the error here? We just bubble up the error like everywhere.
Here's the output that we used to have:
$ skaffold build
Generating tags...
- skaffold-example -> skaffold-example:v1.8.0-15-g992ced4a0
Checking cache...
- skaffold-example: WARN[0007] error checking cache, caching may not work as expected: getting imageID for skaffold-example:v1.8.0-15-g992ced4a0: inspecting image: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
Error checking cache. Rebuilding.
Found [docker-desktop] context, using local docker daemon.
Building [skaffold-example]...
FATA[0012] build failed: building [skaffold-example]: build artifact: docker build: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
Here's the one we have now:
$ skaffold build
Generating tags...
- skaffold-example -> skaffold-example:v1.8.0-16-gb688c4faf
Checking cache...
- skaffold-example: Error checking cache.
FATA[0015] getting imageID for skaffold-example:v1.8.0-16-gb688c4faf: inspecting image: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
For every case of failure here, there’s going to be the same issue later in the build process. We should not ignore those errors. Signed-off-by: David Gageot <[email protected]>
|
ping @briandealwis @nkubala |
For every case of failure here, there’s going to be the same issue down the line.
We should not ignore those errors.
The only case where we should ignore them is when images are not local and this is already done.
(This piece of code wasn't tested before and I couldn't find a way to test it properly)
Signed-off-by: David Gageot [email protected]