Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Fixes #3661: Abort deploy if Git repo is inaccessible. #3662

Merged
merged 1 commit into from
May 22, 2019

Conversation

danepowell
Copy link
Contributor

Fixes #3661

Changes proposed

  • Instead of blindly fetching and merging from a Git remote branch, make sure that a remote branch actually exists first
  • Fail the deploy task if the fetch and merge fail.
  • Also fail the deploy task if the later checkout fails.

Steps to replicate the issue

  1. Add a bogus remote to blt.yml so that git fetches fail.
  2. Run blt artifact:deploy --ignore-dirty --dry-run

Previous (bad) behavior, before applying PR

Deploy task finishes with exit code 0

Expected behavior, after applying PR and re-running test steps

Deploy task fails.

Additional details

In searching for other places where the deploy task might pass even though subcommands fail, I found one other instance (the git checkout -b step). That was originally allowed to fail in #373 . There's not much context there, I suspect it was specific to an older version of Git. With lots of testing on git 2.20.1, I could never get git checkout -b to fail even if the target branch already existed.

@dimdiden can you please review and test?

@dimdiden
Copy link

@danepowell Thank you for this fix! I have tested it and now it works as expected.

However, I've noticed another bugs with the same behavior:
I'm running source code inside a php docker container with the volume bound to this source code

  1. After the stage [ExecStack] git ls-remote --exit-code --heads... I see the following Exit code:
Warning: Permanently added the RSA host key for IP address  to the list of known hosts.
[ExecStack]  Exit code 2  Time 2.323s

and it doesn't stop deploy process. I believe the correct behavior would be to return no errors as adding RSA key is not an error.

  1. On the stage [ExecStack] rsync -a --no-g --delete --delete-excluded... I have rsync issue (probably because of running inside docker container):
rsync: failed to set permissions on "/home/wodby/app/deploy/vendor/acquia/blt/docs/drush.md": Not supported (95)
rsync: failed to set permissions on "/home/wodby/app/deploy/vendor/acquia/blt/docs/git-hooks.md": Not supported (95)

which ends up with the following error:

rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1189) [sender=3.1.3]
[ExecStack]  Exit code 23  Time 10:42

But this doesn't stop the deploy process and it finishes without issues.

Let me know if you need any additional details on that matter. Thank you.

@danepowell
Copy link
Contributor Author

The first error is expected and only indicates that the branch you are pushing to doesn't exist remotely. I don't think it has anything to do with the fingerprint warning. The error shouldn't appear unless you are using very verbose logging, and it's not something we can squelch entirely due to limitations in the Robo result logger.

The second error seems more related to #3670

@danepowell danepowell merged commit 9a2d9df into acquia:10.x May 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BLT deploy does not stop on error event
4 participants