stacker icon indicating copy to clipboard operation
stacker copied to clipboard

[WIP/RFC] Integrate hooks into stack execution graph

Open danielkza opened this issue 6 years ago • 3 comments

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_hooks and destroy_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 the requires / required_by keys or lookups (which are now resolved), very much like stacks themselves.

Hooks now also should retrieve a boto3.Session using Provider.get_session, respecting the profile and region options they now have.


Apologies for the size of the changes, but this required some deep refactorings.

  • The internal representation of Hooks and Targets have changed, hooks now have much more logic, and targets get synthetically generated to help order the "legacy" pre/post action hooks.
  • The outputs variable of Stack that was manually set by actions is removed, in favor of always asking the Provider for 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 the Provider.
  • The plan function was moved to inside Actions to be able to more easily use it's information.
  • We no longer run hooks in "batches" with a single function. Now each hooks has it's own instance, and the logic for configuring and executing them is contained in the Hook class.

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.

  1. Make lookups be able to return Troposphere objects, and properly generate Joins to concatenate variable parts if needed.
  2. Using 1, implement an awsparam lookup 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).

danielkza avatar Mar 20 '19 01:03 danielkza

Rebased and cleaned up - removed a bunch of unnecessary changes and collapsed some commits to be teasier to follow.

danielkza avatar Apr 08 '19 02:04 danielkza

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 !

phobologic avatar May 11 '19 23:05 phobologic

Any chance of getting this one merged?

Amir-Ahmad avatar Nov 13 '20 07:11 Amir-Ahmad