[WIP/RFC] Integrate hooks into stack execution graph #722
Conversation
- Prepare for running hooks individually instead of as a group, to be used to integrate them hooks into the stack execution graph - Add extra config keys for hook execution: profile, region, requires, required_by - Add stack-level key for integrated hooks: build_hooks and destroy_hooks - Add methods to context to retrieve hooks in internal format based on the action to be run
We need to define some syntethic targets to add more features to the execution graph, so make it possible to create instances without a config def.
|
Rebased and cleaned up - removed a bunch of unnecessary changes and collapsed some commits to be teasier to follow. |
If instances of clients are created outside of the moto patching, they will be cached by boto3 and return in subsequent runs, breaking some tests. While the best fix would be to avoid that situation, disabling the caching altogether is easier and ensures the issue won't return.
This makes hooks respect the universal region and profile settings add in the config.
When running the destroy action, we reverse the order of execution compared to build, such that steps that required others are now required by them instead. Unfortunately that is not enough to express the execution requirements of hooks: they might be `required_by` a stack to run some cleanup before destroying that stack, but still need it to be deployed to lookup outputs from. Handle that by manually checking for the dependencies of lookups before executing hooks, skipping them if a required stack is not deployed (or has already been destroyed).
|
This looks good from my first pass - I like the way it's laid out. @ejholmes can you take a quick look at the plan/graph stuff, just to be sure we have another set of eyes on it? Also, it looks like this will need a bit of doc updates for the new config options, etc. One question: Do you think it's worth separating build/destroy hooks, or instead just having the hook classes themselves know how to behave in each situation (a build/destroy method)? I'm kind of on the fence - if I had a hook that created a bucket in S3 on build, not sure if I would want it to destroy that same bucket on destroy. Thanks for this @danielkza ! |
|
Any chance of getting this one merged? |
Make hooks "first-class" citizens in the Stacker execution engine, allowing them to depend on/be depended upon by stacks and targets.
This is not yet 100% complete, but I'm posting to gather some early reviews and advice on the config changes needed.
There are two new top level config. keys introduced:
build_hooksanddestroy_hooks. They contain lists of hooks specifications, much like what was already present in{pre,post}_{build,destroy}. But the hooks in there are expected to order themselves in relation to stacks using therequires/required_bykeys or lookups (which are now resolved), very much like stacks themselves.Hooks now also should retrieve a
boto3.SessionusingProvider.get_session, respecting theprofileandregionoptions they now have.Apologies for the size of the changes, but this required some deep refactorings.
Hooks andTargets have changed, hooks now have much more logic, and targets get synthetically generated to help order the "legacy" pre/post action hooks.outputsvariable ofStackthat was manually set by actions is removed, in favor of always asking theProviderfor that information. That might cause a little bit of slowdown in execution, but that can be solved in a more clean way by doing the caching transparently in theProvider.planfunction was moved to insideActions to be able to more easily use it's information.Hookclass.There are a couple of additional unrelated changes currently still in this branch. I initially made them to be able to use the AWS Account ID as part of the bucket name in some tests, but I figured out they are not strictly needed. I will refactor them out, but can still make separate PRs if they seem useful.
Joins to concatenate variable parts if needed.awsparamlookup that can interpolate CloudFormation pseudo-parameters into strings. ex:${awsparam AccountID}This depends on #714. A few tests were refactored to use pytest fixtures when appropriate (for example, to properly handle creation of boto3 clients with different regions).