Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure the CLI to exit non-zero on failures. #1313

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

emmceemoore
Copy link
Contributor

I noticed that kamal wasn't exiting non-zero for me in certain scenarios where I expected it to.

$ kamal deploy -d staging --some-option-that-doesnt-exist
ERROR: "kamal deploy" was called with arguments ["--some-option-that-doesnt-exist"]
Usage: "kamal deploy"

$ echo $?
0

This caught me by surprise in my CI/CD when my staging deployment "succeeded" super quickly and my changes started deploying to production. 😅

I'm not very familiar with Ruby but I believe that this is a good change to make.

(Looks like this was previously set to true but was changed to false by @djmb here.)

Testing

  • With the proposed change in place, I manually ran bundle exec ./bin/kamal deploy -d staging --some-option-that-doesnt-exist and got the expected behavior (non-zero exit code).
  • I tried running bundle exec ./bin/tests but I'm getting several toomanyrequests errors [1] from docker. The number of errors (10) seems to remain consistent with and without this change so I have some confidence that it shouldn't cause any regressions but I'm not certain.
[1] - Error Message Example
...

  ERROR (SSHKit::Command::Failed): Exception while executing on host vm1: docker exit status: 125
docker stdout: Nothing written
docker stderr: Error response from daemon: No such container: kamal-proxy
Error: failed to start containers: kamal-proxy
cat: .kamal/proxy/options: No such file or directory
Unable to find image 'basecamp/kamal-proxy:v0.8.4' locally
docker: Error response from daemon: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit.
See 'docker run --help'.

...

I went ahead and created a Docker account. I did a docker login (which indicated success) and tried again and still get the error. Looking at the "Usage" dashboard in Docker doesn't seem to indicate an issue. Perhaps my anonymous usage before logging in is causing my IP to be throttled? Another theory is that maybe the way the tests are run aren't making use of my logged in account. 🤷

I'm not really interested in upgrading to any sort of paid account at this time.

Any suggestions on how to work around this issue would be greatly appreciated!

@emmceemoore
Copy link
Contributor Author

Looks like a test is failing: 😞

Error:
MainTest#test_setup_and_remove:
RuntimeError: Command `TEST_ID=93f72bc2b0a41038ecd9498bc1edfd70 docker compose exec --workdir /app_with_roles deployer kamal proxy set_config --publish=false --options=label=traefik.http.services.kamal_proxy.loadbalancer.server.scheme=http label=traefik.http.routers.kamal_proxy.rule=PathPrefix\(\`/\`\) label=traefik.http.routers.kamal_proxy.priority=2` failed with error code `pid 63353 exit 1`, and output:

    test/integration/integration_test.rb:34:in `docker_compose'
    test/integration/integration_test.rb:40:in `deployer_exec'
    test/integration/integration_test.rb:44:in `kamal'
    test/integration/main_test.rb:93:in `block in <class:MainTest>'

bin/test /home/runner/work/kamal/kamal/test/integration/main_test.rb:90

Seems like the test should be updated/removed (since set_config no longer appears to be an option).

@emmceemoore
Copy link
Contributor Author

And now another:

Error:
AppTest#test_stop,_start,_boot,_logs,_images,_containers,_exec,_remove:
RuntimeError: Command `TEST_ID=ab6d82077231b99d1df5bc116d848a37 docker compose exec --workdir /app deployer kamal deploy` failed with error code `pid 90106 exit 1`, and output:

    test/integration/integration_test.rb:34:in `docker_compose'
    test/integration/integration_test.rb:40:in `deployer_exec'
    test/integration/integration_test.rb:44:in `kamal'
    test/integration/app_test.rb:5:in `block in <class:AppTest>'

bin/test /home/runner/work/kamal/kamal/test/integration/app_test.rb:4

BTW, it looks like the runners are also subject to the toomanyrequests errors (example).

@emmceemoore
Copy link
Contributor Author

With the recent changes I made the tests are now passing. I believe things are now ready to be reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant