Exit code on container:release still 0 if release fails

vovimayhem commented 5 years ago

Current Behavior

Running the heroku container:release web release --app XXXX returns an exit code "0", even with #1233 (See #1232)

Expected Behavior

Running the heroku container:release web release --app XXXX must return an exit code "1" when the release fails.

Notes:

When I remove curl from the deployed image, heroku container:release DOES return an exit code "1" when failing.
With curl back in the deployed image, when I inspect the command output with DEBUG=*, I can see that when the release data is fetched from the API, the release is still in "pending" state:

rails aborted!
NameError: uninitialized constant ActiveRecord::ConnectionAdapters::PostgreSQLAdapter
  http --> GET /apps/xxx-xxx-xxx/releases/xxx-xxx-xxx +19s
--> GET /apps/xxx-xxx-xxx/releases/xxx-xxx-xxx
  http 
  http     accept=application/vnd.heroku+json; version=3
  http     content-type=application/json
  http     user-agent=heroku/7.26.2 linux-x64 node-v11.14.0
  http     range=id ..; max=1000
  http     host=api.heroku.com
  http     authorization=REDACTED +19s
<-- 200 OK
  http <-- undefined /apps/xxx-xxx-xxx/releases/xxx-xxx-xxx
  http {"addon_plan_names":["scout:skiloveland-db","sendgrid:starter","deployhooks:http","papertrail:choklad","cloudamqp:lemur"],"app":{"id":"1f0c038f-af43-4e2f-8680-ff7f5b6ad2a7","name":"xxx-xxx-xxx"},"created_at":"2019-07-24T04:22:29Z","description":"Deployed release (0752f548228e)","status":"pending","id":"cbe1039b-b8e3-41e7-a32b-feb771ad0fac","slug":null,"updated_at":"2019-07-24T04:22:29Z","user":{"email":"xxx-xxx-xxx","id":"49d0fae9-f9f5-4b30-b447-ddff253189af"},"version":162,"current":false,"output_stream_url":"https://release-output.heroku.com/streams/1f/1f0c038f-af43-4e2f-8680-ff7f5b6ad2a7/logs/cb/cbe1039b-b8e3-41e7-a32b-feb771ad0fac.log?AWSAccessKeyId=AKIAIO4SD3DCRO7W6IJQ&Signature=gq39r6%2B%2BQ05SnezNJVtBPS9X9Cc%3D&Expires=1564028569"} +466ms
  http 
  http     cache-control=private, no-cache
  http     content-length=765
  http     content-type=application/json
  http     date=Wed, 24 Jul 2019 04:22:49 GMT
  http     etag="5b8771e1f51a44a56bce8093dcdfad35"
  http     last-modified=Wed, 24 Jul 2019 04:22:29 GMT
  http     oauth-scope=global
  http     oauth-scope-accepted=global read-protected write-protected
  http     ratelimit-multiplier=1
  http     ratelimit-remaining=4499
  http     request-id=686d5303-67bb-403d-8e47-95bfdbdf8554,8e5e76d8-8538-06dc-214b-1167956853ee,4eb13a62-d048-3702-95f1-253f642b9195
  http     vary=Accept-Encoding
  http     via=1.1 spaces-router (c7665a42a791), 1.1 spaces-router (c7665a42a791)
  http     x-content-type-options=nosniff
  http     x-runtime=0.053183288 +466ms
<-- {"addon_plan_names":["scout:skiloveland-db","sendgrid:starter","deployhooks:http","papertrail:choklad","cloudamqp:lemur"],"app":{"id":"1f0c038f-af43-4e2f-8680-ff7f5b6ad2a7","name":"xxx-xxx-xxx"},"created_at":"2019-07-24T04:22:29Z","description":"Deployed release (0752f548228e)","status":"pending","id":"cbe1039b-b8e3-41e7-a32b-feb771ad0fac","slug":null,"updated_at":"2019-07-24T04:22:29Z","user":{"email":"xxx-xxx-xxx","id":"49d0fae9-f9f5-4b30-b447-ddff253189af"},"version":162,"current":false,"output_stream_url":"https://release-output.heroku.com/streams/1f/1f0c038f-af43-4e2f-8680-ff7f5b6ad2a7/logs/cb/cbe1039b-b8e3-41e7-a32b-feb771ad0fac.log?AWSAccessKeyId=AKIAIO4SD3DCRO7W6IJQ&Signature=gq39r6%2B%2BQ05SnezNJVtBPS9X9Cc%3D&Expires=1564028569"}

vovimayhem commented 5 years ago

So, somehow streaming the release logs may cause a delay on the status change of the release in the REST API, enough for the fix in #1233 to not work.

Maybe adding a loop until the status is not pending will do the trick?

ali-graham commented 5 years ago

👍 The behaviour of the API was a bit of an unknown for me when I wrote that PR, but we're still getting occasional instances of this happening, so it looks like this is needed.

vovimayhem commented 5 years ago

@ali-graham Yes! I do feel this is more like a problem with the platform API - the release status should display "success" or "failed" right after the release stream finishes, but that's not currently the case.

In the mean time, I think #1272 should be a workaround.

kwlockwo commented 3 years ago

The issue here is that release phase (i.e. you have defined release image) is decoupled from the heroku container:release command. The heroku container:release will not wait for your release phase to run and complete successfully and the commands exit code is not based on the status of the actual release, only if a release was created. This is why it checks /apps/<app>/releases to make sure the release version has been incremented. The status of the release would still be pending while the release phase is running, but this command doesn't care about the status.

Release phases can run up to 1 hour and you don't really want this command sitting there for this long. If you need to rely on the status of the release phase then you can check the API endpoint yourself (or use the heroku releases CLI command).

Note this works the same as git push heroku main it will not return an exit code of 1 if your release fails, its exit code is only based on if the push to our Git server or not was successful.

sfcgeorge commented 3 years ago

If running container:release doesn't actually tell you whether your container has been released then that's pretty confusing. At that point you've already pushed your containers to the Heroku registry, so this step is not even pushing just saying make it live but we won't tell you if it actually is live? Unhelpful.

I believe container:release does actually wait for the release command to finish because you can see the full release output. Which to me implies that if it's waiting then it'll wait for success or failure and exit accordingly. If it isn't supposed to be relied upon then it shouldn't wait and should just output something like Release command is running in the background, you can check release status with "heroku releases:output RELEASE_NUMBER" or on the Dashboard.

The main reason you'd use this command is in a CI script so it really should be useful for use in a CI script.

I see there's a similar issue with pipeline promotion.

Finally there was an old similar issue with one-off run Dynos which was fixed by adding a --exit-code flag. Perhaps the same or similar could be added here? Though in all cases I think exit codes would be on by default, it's a Unix CLI tool after all.

Workaround

Stick this as the next CI step. It fetches the latest 1 release status. As I believe container:release waits (unverified) then this should have the current status. JSON format for longevity. Could parse the JSON in a fancier way but grep is likely available in most CIs and should be good enough. Grep returns a non-zero exit code if it doesn't find a match, so when "failed" the below won't be found and will non-zero exit.

heroku releases --json -n 1 -a my-app-name | grep '"succeeded"'

heroku / cli