Ddev: Add config.yaml option to fail an action when hooks fail

[AronNovak]

Is your feature request related to a problem? Please describe.

hooks:
  post-start:
    - exec: exit 1
    # Private files directory.
    - exec: mkdir /var/www/private

    # Install profile
    - exec: drush site-install server -y --existing-config

    # Compile theme
    - exec-host: ddev robo theme:compile

    # Generate one-time login link
    - exec: drush uli

Let's say we have a failing post-start command in our custom configuration.
Right now there's no trivial way to know if all the commands succeeded or not, as ddev start exits with code 0 even if something in the hooks failed.

Describe the solution you'd like
We could stop on the first failure for post-start and propagate the received exit code for the caller of ddev start.

Describe alternatives you've considered
As it might break existing configs, it could be a config option, like stop_on_hook_failure: true , with a default value false, it would ensure the backwards compatibility.

[rfay]

ddev used to exit on failure of a hook and too many people complained about it and wanted it to keep going :)

[AronNovak]

@rfay How does it sound to make it configurable?

[AronNovak]

I see that it originates from https://github.com/drud/ddev/pull/1860

A failed task does not fail the entire operation.

[rfay]

How about your travis recipe just doing a ddev exec to check to make sure that everything is set up properly after the ddev start ?

[AronNovak]

@rfay That could work, but then, we cannot use ddev config YML as a source of truth for the things we'd execute, it would be essentially the same on local. But that's an obvious workaround, thanks for the suggestion!

[rfay]

DDEV-Local tries to be optimized for what helps people doing local development, so that's the reason for the trade-off here.

[amitaibu]

How about your travis recipe just doing a ddev exec to check to make sure that everything is set up properly

I'm not sure if it's the right approach in general. I can think of some commands that if not executed, we'd like it to blow. That is, if some encryption/ sensitive data related command didn't work - we'd like the entire flow to stop.

[amitaibu]

I mean, in our case we can surely check that CSS file was compiled (which is currently our use case); but I think in general stopping on error can be a good thing.

[majamee]

I think even if the exit codes don't lead to a "ddev start" failing anymore, it would be more than welcome to have the option to achieve that if a critical hook is failing the ddev start gets interrupted as it was before.

Possible solutions:

• Having at least 1 error code which still uses the old behavior (interrupting ddev start)
• Having a range of error codes for interrupting ddev start, e.g. starting with 4 digit error codes
• Having a command which could be called from within a ddev start hook which causes the 'ddev start' to be interrupted

It would be highly appreciated to give any lead how to interrupt a "ddev start" on purpose. Thank you very much for your efforts.
@rfay Maybe one of the above suggestions is considered feasible? :)

[majamee]

@rfay Could you please provide a short update about this issue? Would be highly appreciated. Thank you for your efforts & have a great day :bow:

[rfay]

Hi @majamee - This issue is not currently slotted for work in v1.17. PRs are welcome. The more people that show their interest in the issue, the sooner it makes it onto the board. But this one is tricky because it used to work this way, and it seems more people are happy with the way it works now. A config option could make both work of course.

[majamee]

@rfay could you please show me the commit which changed that behavior? That way I would at least have a solid starting point to work on such a PR :-)

[rfay]

The PR that changed it was https://github.com/drud/ddev/pull/1860

The code in question is https://github.com/drud/ddev/blob/41cf849948e250f0971290b84d376216671a8b94/pkg/ddevapp/ddevapp.go#L807-L810

You'd also have to add config, which is slightly harder, but lots of examples.

[majamee]

@rfay I tried my best and you can find the according PR here:
https://github.com/drud/ddev/pull/2652

Maybe it is possible now to have this new option to be included for the next ddev version (v1.17) 😄

P.S.: I did read the CONTRIBUTING.md and followed it closely, please bare in mind that this is my first development work in "Go" though. So please be indulgent. 🙂

[rfay]

Awesome, looking forward to looking at it. Congrats! I imagine on the contributing.md you mean https://github.com/drud/ddev/blob/master/docs/developers/building-contributing.md ?

[majamee]

Actually https://github.com/drud/ddev/blob/master/CONTRIBUTING.md

[rfay]

OK, just pointing out that https://github.com/drud/ddev/blob/master/docs/developers/building-contributing.md will be useful to you.

[majamee]

Thank you very much for pointing that out, I just was reading through the Makefile and its comments instead, I think I should have covered anything important (maybe not the branch name, but it is pretty much self-explanatory though) 😉

[rfay]

The branch name isn't important. It's just the way I do it so I can distinguish old branches from current ones in git branch (and get decent descriptions for the branches)