earthly icon indicating copy to clipboard operation
earthly copied to clipboard

Force output on failure

Open jamesalucas opened this issue 4 years ago β€’ 27 comments

I am running unit tests under Earthly and everything is perfect when the unit tests succeed - a junit.xml file is written via an 'SAVE ARTIFACT AS LOCAL' statement and can be picked up by our CI server. However, when a test fails the command also fails and Earthly returns a non-zero exit code back to our CI server (which is good) but no output is written, so the CI server is unable to show which tests have failed.

Is there a way to force output to be written despite the build not succeeding?

I could get the command to return zero but then I would have to find a way to fail the CI build as the CI server (Gitlab) uses exit codes to determine the success/failure of a build.

Thanks

EDIT(by @vladaionescu): See workaround in this comment.

jamesalucas avatar May 18 '21 13:05 jamesalucas

Seems like a very valid use case.

It can be achievable via the WITH TRY proposal + the UDC extension potentially: https://github.com/earthly/earthly/issues/587

Though I admit that the syntax requires putting the SAVE ARTIFACT in a UDC, which is not as readable for a simple case like this.

Maybe something like this would be nice for this use case?

TRY
  RUN <do-the-build>
FINALLY
  SAVE ARTIFACT junit.xml AS LOCAL ./
END

vladaionescu avatar May 18 '21 14:05 vladaionescu

In my case my unit test command is already within a UDC so that's not a problem, but the alternative syntax you proposed looks great and very intuitive. πŸ‘πŸ»

jamesalucas avatar May 18 '21 20:05 jamesalucas

We've recently begun experimenting with Earthly, and this proposal looks spot-on. We're running cypress integration tests in Earthly, both locally and in CI, and we're really missing the ability to SAVE ARTIFACT screenshots & videos when the tests fail.

benjadrejer avatar Jun 01 '21 13:06 benjadrejer

I'm looking this one too! Is there any estimated time when this could be implemented? A roadmap? πŸ˜ƒ

zedtux avatar Jun 28 '21 14:06 zedtux

Seems like this is requested pretty frequently. No timeline yet, but we'll prioritize it higher given the demand.

vladaionescu avatar Jun 28 '21 16:06 vladaionescu

@jamesalucas can you please tell us more about the way you did it? I'm not sure to understand how you workarounded this.

zedtux avatar Jun 29 '21 09:06 zedtux

@jamesalucas can you please tell us more about the way you did it? I'm not sure to understand how you workarounded this.

I haven't actually worked around this yet as I was waiting to see what the outcome of this was. However, I did think it would be possible to workaround it by ensuring the RUN statement always returned true, for example instead of our usual command:

RUN yarn test

it would be rewritten as:

RUN yarn test || true

This would ensure that output gets rewritten, but unfortunately Earthly would always return success even if there were test failures. In our case we would then need a way to know when to fail the CI job, which could be something like this:

test:
    FROM alpine:latest
    RUN set -e && echo 0 > exit_code && (yarn test || echo $? > exit_code)
    SAVE ARTIFACT exit_code AS LOCAL exit_code

The local exit_code file would then contain 0 if the job passed or non-zero if it failed so you could use that to pass/fail the job (e.g. grep 0 exit_code).

I'm sure there are better/other ways though!

jamesalucas avatar Jun 29 '21 11:06 jamesalucas

Thank you @jamesalucas for your comment.

zedtux avatar Jun 29 '21 11:06 zedtux

I do something like

test:
    FROM python:latest
    ....
    RUN pytest . --color=yes \
            --junit-xml=pytest.xml \
            --cov-report=xml:cov.xml || \
            echo fail > fail
    SAVE ARTIFACT --keep-ts pytest.xml /pytest/ AS LOCAL pytest.xml
    SAVE ARTIFACT --keep-ts cov.xml /pytest/ AS LOCAL cov.xml
    IF [ -f fail ]
        RUN echo "test run failed" && \
            exit 1

This successfully creates the artifacts but does not save them locally alas.

mariusvniekerk avatar Jul 01 '21 04:07 mariusvniekerk

Thank you @mariusvniekerk for your snippet! Just a warning about the missing END of the IF block.

I was able to use your way using WITH DOCKER:

#
# This target runs the Cucumber test suite.
#
# Use the following command in order to run the tests suite:
# earthly --allow-privileged +cucumber
cucumber:
    FROM earthly/dind:alpine

    COPY docker-compose.yml ./
    COPY docker-compose-earthly.yml ./

    COPY db ./db

    # --load will load the given docker image in the Eartly's Docker instance.
    # The left part of the "=" is the name that the Eartly's Docker instance
    # will have, the right part of the "=" is from where this image is loaded.
    #
    # Here we are taking the image from the "test" target and name it
    # registry.gitlab.com/mygroup/myproject:test which is the "image" name
    # defined in the docker-compose-earthly.yml file.
    #
    #
    # --pull Pull Docker images that the app depends on and cache them
    # in Earthly so that it will not re-download them again and again.
    WITH DOCKER --load registry.gitlab.com/mygroup/myproject:test=+test \
                --pull mongo:3.4.13 \
                --pull mysql:5.7.20 \
                --pull redis:4.0.5-alpine \
                --pull selenium/standalone-chrome:3.141.59-antimony
        RUN docker-compose -f docker-compose-earthly.yml run \
            --rm app \
            bash -c 'bundle exec rake db:create RAILS_ENV=test_health \
                    && bundle exec rake db:create RAILS_ENV=test \
                    && bundle exec rake db:setup RAILS_ENV=test \
                    && bundle exec rake db:migrate RAILS_ENV=test \
                    && bundle exec cucumber' || echo fail > fail
    END

    SAVE ARTIFACT ./tmp/capybara/ AS LOCAL tmp/

    IF [ -f fail ]
        RUN echo "Cucumber suite has failed." \
            && exit 1
    END

zedtux avatar Jul 01 '21 09:07 zedtux

I actually have a weird error with the IF block:

      +cucumber | --> FROM earthly/dind:alpine
      +cucumber | [β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆ] resolve docker.io/earthly/dind:alpine@sha256:03ca247da4f32bb2c14719d83edc5e4d1f013af5af07ab9b9b6e7e9307374c37 ... 100%
       internal | --> docker tar context +test 165cc66b5bb1821266cd5d73a7bcb12789ad706ed9c6e4a134b9a3ceaab1e9e4
       internal | WARN: (docker tar context +test 165cc66b5bb1821266cd5d73a7bcb12789ad706ed9c6e4a134b9a3ceaab1e9e4) rpc error: code = NotFound desc = no access allowed to dir "registry.gitlab.com/mygroup/myproject:test-0b4ce4981d1e0a51c93096ddeda59c64fc81dc136273c149ef9b3d644468e122"
Error: no access allowed to dir "registry.gitlab.com/mygroup/myproject:test-0b4ce4981d1e0a51c93096ddeda59c64fc81dc136273c149ef9b3d644468e122"

I can't really figure out what's wrong ... when I remove the block, it works. Then I put back the IF block and it works too. :thinking:

zedtux avatar Jul 01 '21 19:07 zedtux

I don't know the Earthly version you're using, but with 0.5.17 I can't get my files out of Earthly until the final exit code is 0.

Maybe when using WITH DOCKER Earthly works differently?

zedtux avatar Jul 03 '21 16:07 zedtux

Yeah.. Earthly doesn't yet output anything unless it ends in success. We'll be tackling this very soon though - thanks for the patience - we've been super busy lately.

vladaionescu avatar Jul 03 '21 16:07 vladaionescu

The last version of Earthly (v0.5.24) fixed the issue reported in my comment but the artifacts aren't saved since the target fails as @mariusvniekerk.

Until this proposal is implemented, I'll try the @jamesalucas' way ...

zedtux avatar Oct 04 '21 09:10 zedtux

BTW we've prioritized this for Oct. It's unintuitively hard though - it may take a while until it's ready.

vladaionescu avatar Oct 04 '21 18:10 vladaionescu

I have the same ask, for the use case of gitlab-ci where we need the junit xml for https://docs.gitlab.com/ee/ci/unit_test_reports.html

Related slack comment: https://earthlycommunity.slack.com/archives/C01DL2928RM/p1636929954128600

Alphasite avatar Nov 15 '21 17:11 Alphasite

BTW the @jamesalucas' way works fine: I'm saving the exit code in a file, and then from the .gitlab-ci.yml file I'm doing:

  script:
  - earthly --allow-privileged
            --build-arg COMPOSE_HTTP_TIMEOUT=$COMPOSE_HTTP_TIMEOUT
            --build-arg TAG=$DOCKER_IMAGE_TAG
            --ci
            --config $EARTHLY_CONFIG_PATH
            --output
            --remote-cache $CI_REGISTRY_IMAGE:$DOCKER_IMAGE_TAG-cache
            +cucumber
  - >
    if [[ -f ${CI_PROJECT_DIR}/exit_code ]]; then
      exit $(cat ${CI_PROJECT_DIR}/exit_code)
    else
      echo "'${CI_PROJECT_DIR}/exit_code' file missing, something failed in previous command"
      exit 1
    fi

zedtux avatar Nov 28 '21 13:11 zedtux

Stumbled upon this issue myself today.

Earthly doesn't yet output anything unless it ends in success.

This makes things extra painful. I'm hoping to make my Earthfile copy the "test-results" directory only when Playwright tests fail, and then have the outer GH action infer success/failure by the presence of that directory (rather than the exit code) but it feels pretty janky.

I described the awkward workaround steps required to account for this in https://github.com/replayio/devtools/pull/7286

bvaughn avatar Jun 28 '22 16:06 bvaughn

I realize that this issue is currently under development, so this comment may be moot, but I wanted to suggest an implementation that I, as an earthly newbie, would find intuitive -- adding support for a flag like --immediate or --always for both SAVE ARTIFACT and SAVE IMAGE.

jschnitter avatar Aug 09 '22 18:08 jschnitter

I suspect it’s been erroneously marked as under development, since it was moved into that state almost 1 year ago.

Alphasite avatar Aug 10 '22 01:08 Alphasite

It turns out this is really hard to implement due to BuildKit having never been designed to do anything like this. We've been working on this on-and-off. Currently, we're tackling a construct that gets us closer to the goal as part of WAIT - it allows a build to output an artifact before it finishes (among other things). More needs to be done after that though.

We don't have a clear timeline but know that this is high on @alexcb's list. As I said before, implementing this is unintuitively hard.

vladaionescu avatar Aug 10 '22 01:08 vladaionescu

I propose my workaround for the very same issue, just a bit more compact than what what proposed earlier.

gitlab-ci.yaml:

test:
  image:
    name: registry.gitlab.com/xdev-tech/build/earthly:3.2
  script:
    - earthly-daemonless --ci --push --output +test-and-save-results
    - exit $(cat exit-code)
  artifacts:
    when: always
    paths:
      - report.json
      - test-results/

Eartfile:

test-and-save-results:
    FROM +deps
    COPY ./ ./
    RUN --no-cache poetry run pytest -v --cucumberjson=report.json && echo 0 > exit-code || echo 1 > exit-code
    SAVE ARTIFACT exit-code AS LOCAL exit-code
    SAVE ARTIFACT report.json AS LOCAL report.json
    SAVE ARTIFACT --if-exists test-results AS LOCAL test-results

I think all the cases are covered with this code. Not as clean as the TRY-FINALLY-END proposal, but it does the job :)

glehmann avatar Aug 19 '22 21:08 glehmann

I'm keenly following this thread - being able to SAVE ARTIFACT after a failed step still seems like an elegant option.

The drawback that we've experienced with the exit-code-style workarounds is if you have a flaky test, then its failure can up end getting cached because the RUN step was "successful". I'm sure with the addition of more complexity (perhaps an ARG) we could ensure that our pytest RUN step always runs without busting the entire earthly cache.

edsharp avatar Aug 23 '22 07:08 edsharp

The new --wait-block feature introduces a new WAIT / END block, which allows one to mix commands in a fan-out-fan-in pattern.

Under v0.6.23, it is now possible to run:

VERSION --wait-block 0.6

test:
    FROM ...
    COPY my-test .
    RUN --no-cache ./my-test; echo $? > exit_code
    WAIT
        SAVE ARTIFACT junit.xml AS LOCAL ./
    END
    RUN test "$(cat exit_code)" = "0"

which will allow outputting an artifact (such as junit.xml), before testing the exit code.

(we still want to implement a TRY / CATCH; however, we wanted to share this as an improved work-around)

alexcb avatar Sep 07 '22 00:09 alexcb

(we still want to implement a TRY / CATCH; however, we wanted to share this as an improved work-around)

Thanks for sharing!

edsharp avatar Sep 08 '22 07:09 edsharp

@alexcb Your new workaround works for me, except that the remote cache is not saved anymore in case of test failure.

My Earthfile looks something like:

VERSION --wait-block 0.6

deps:
   FROM ubuntu:22.04
   RUN install test dependencies…
   SAVE IMAGE --cache-hint

ci:
    BUILD +deps
    FROM +deps
    COPY . ./
    RUN --no-cache pytest --junitxml=report.xml ; echo $? > exit-code
    WAIT
        SAVE ARTIFACT report.xml AS LOCAL report.xml
    END
    RUN exit $(cat exit-code)

Is it the expected behavior? Is it possible to tell earthly to save the cache anyway?

I tried to put the SAVE IMAGE --cache-hint in a WAIT/END block, without much success.

glehmann avatar Sep 12 '22 09:09 glehmann

except that the remote cache is not saved anymore in case of test failure.

That's a great point! That's not something I considered while working on WAIT/END; I'll take a closer look to figure out how to fix that (and write a test for this). I created #2178 to track this issue specifically.

alexcb avatar Sep 12 '22 16:09 alexcb

mostly a copy/paste from https://github.com/earthly/earthly/issues/587#issuecomment-1255633387 :

v0.6.24 introduces a solution for this:

VERSION --try 0.6

example:
    FROM ...
    TRY
        # only a single RUN command is currently supported; no BUILD, or other commands allowed
        RUN ./test
    FINALLY
        # only SAVE ARTIFACT commands are supported here
        SAVE ARTIFACT junit.xml AS LOCAL ./
        SAVE ARTIFACT .... AS LOCAL ./
   END

Note that it only supports a single RUN command, and only implements saving artifacts in the FINALLY.

alexcb avatar Sep 22 '22 23:09 alexcb

Fantastic, thanks! πŸ‘

Note that it only supports a single RUN command, and only implements saving artifacts in the FINALLY.

Is it also a limitation that it doesn't support directory artifacts? With Earthly 0.6.30 I'm getting:

+xxx *failed* | failed to save xxx: read xxx: is a directory

Earthfile:

xxx:
  RUN mkdir xxx && echo hi > xxx/hello.txt
  TRY
    RUN cat asdlkajd
  FINALLY
    SAVE ARTIFACT xxx AS LOCAL ./xxx
  END

darabos avatar Nov 25 '22 15:11 darabos

closing this ticket in favour of #2452 which documents a bug that affects saving large files.

alexcb avatar Mar 06 '23 20:03 alexcb