Multi region support by ejholmes · Pull Request #551 · cloudtools/stacker

ejholmes · 2018-03-09T07:58:43Z

This implements support for provisioning stacks in multiple regions. You can target a stack to a specific region using the new per stack region option.

Example

Here's a simple pseudo stack config that shows how one might use this to create some resources in us-west-1, and reference them in us-east-1 with the output lookup.

stacks:
  - name: west/encryption-key
    stack_name: encryption-key
    region: us-west-1
    class_path: stacker_blueprints.kms.Key
    variables:
      KeyAlias: alias/backups

  # You can use the `output` lookup to use an output from a stack in one
  # region, as the input to a stack in another region:
  - name: east/db-backups
    stack_name: db-backups
    class_path: stacker.tests.fixtures.mock_blueprints.Dummy
    variables:
      # Reference the KMS key created in us-west-1!
      EncryptionKeyARN: ${output west/encryption-key}

In the long term, I'm also planning on adding multi account support, aws profiles, but I still think having a region flag on a stack is still valuable for simplicity, and everything in this PR is a pre-req to supporting aws profiles.

Depends on #550

TODO

Fix --tail (breaks because it doesn't happen within _launch_stack, so it doesn't get the stack specific region applied).
Fix info command
Fix FunctionalTests blueprint to give stacker user access to all regions.

codecov-io · 2018-03-10T00:06:52Z

Codecov Report

Merging #551 into master will decrease coverage by 0.08%.
The diff coverage is 83.5%.

@@            Coverage Diff             @@
##           master     #551      +/-   ##
==========================================
- Coverage   87.44%   87.36%   -0.09%     
==========================================
  Files          95       95              
  Lines        6127     6167      +40     
==========================================
+ Hits         5358     5388      +30     
- Misses        769      779      +10

Impacted Files	Coverage Δ
stacker/commands/stacker/destroy.py	`76.47% <ø> (ø)`	⬆️
stacker/tests/fixtures/mock_blueprints.py	`62.06% <ø> (ø)`	⬆️
stacker/commands/stacker/build.py	`84.21% <ø> (ø)`	⬆️
stacker/actions/info.py	`33.33% <0%> (-1.97%)`	⬇️
stacker/actions/diff.py	`59.28% <0%> (-0.43%)`	⬇️
stacker/commands/stacker/info.py	`71.42% <0%> (ø)`	⬆️
stacker/commands/stacker/diff.py	`73.33% <0%> (ø)`	⬆️
stacker/actions/build.py	`91.33% <100%> (+0.05%)`	⬆️
stacker/actions/destroy.py	`85.71% <100%> (+0.29%)`	⬆️
stacker/tests/actions/test_build.py	`96.85% <100%> (+0.08%)`	⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55aa8a8...42cba20. Read the comment docs.

ejholmes · 2018-03-10T10:46:42Z

                 locked=False, force=False, enabled=True, protected=False):
        self.name = definition.name
-        self.fqn = context.get_fqn(self.name)
+        self.fqn = definition.stack_name or context.get_fqn(self.name)


Curious if anyone has thoughts on this. I intentionally made stack_name not include the include the namespace, but now I think that might be confusing. Would this be better?

self.fqn = context.get_fqn(definition.stack_name or definition.name)

Made this change.

phobologic

Surprised at how little code needed to change for this. How do we deal with various s3 connections/buckets? Anything we need to worry about with hooks/lookups?

phobologic · 2018-03-12T16:46:38Z

+  since you could have multiple stacks with the same name, but in different
+  regions or accounts. (note: the namespace from the environment will be
+  prepended to this)
+**region**:


Could you detail here what happens if this is and isn't set? I'm assuming it modifies quite a bit - also, curious what the default is

I'll update the docs. The default would be whatever the global default is (e.g. AWS_DEFAULT_REGION, ~/.aws/config). If not provided, it works exactly as before.

That's what I assumed, just wanted to make sure it's immediately apparent for folks that are new to stacker in the future. Thanks.

phobologic · 2018-03-12T16:57:53Z

            return NotSubmittedStatus()

+        region = stack.region
+        provider = self.provider.build(region=region)


Can we rename self.provider to self.provider_builder/provider_factory or something similar? I think you avoided this to keep this PR small, but honestly this will probably lead to confusion in the future.

Yeah, I only did this to keep the diff small. I'll go ahead and make this change.

phobologic · 2018-03-12T16:58:33Z

            validate=True)

-        options.provider = default.Provider(
+        options.provider = default.ProviderBuilder(


options.provider -> options.provider_builder, etc, based on my comment above

phobologic · 2018-03-12T16:59:21Z

+        self.kwargs = kwargs
+
+    def build(self, region=None):
+        # TODO(ejholmes): This class _could_ cache built providers, however,


Honestly, I don't think we need to cache Providers. My guess is they're pretty cheap to build/throw away at the scale we usually operate.

phobologic · 2018-03-12T16:59:43Z

+        session = get_session(region=region)
+        return Provider(session, **self.kwargs)
+
+    def tail_stack(self, stack, cancel, retries=0, **kwargs):


Curious why you brought this in here, rather than modifying the tail code elsewhere?

Good question. I'm definitely not happy with it, but it's a little tricky because of where we pass the tail_stack function.

I could change that line to something like:

tail=self._tail_stack

And then:

def _tail_stack(...) provider = self.build_provider(stack) return provider.tail_stack(...)

Which would definitely be better, so we can share and re-use a build_provider method.

phobologic · 2018-03-12T17:09:15Z

Also - should log messages be updated? Particularly debug logs, to include the region? Maybe not super necessary if we print the stack names themselves.

ejholmes · 2018-03-12T23:21:13Z

How do we deal with various s3 connections/buckets

We already talked about this offline, but just to mention it here: since this PR only deals with multi-region, there's no special considerations that need to be taken into account for the stacker bucket; stacker will continue to provision the bucket in the default region.

Anything we need to worry about with hooks/lookups?

Hooks: We could eventually add region support to this in the future, but since we're talking about re-working hooks to be a part of the graph, probably best to hold off on that for now.
Lookups: output just works between multi-region stacks, but there are some special considerations to take into account when using something like xref; if a stack is provisioned in us-west-1, then the xref lookup will also look for the stack in us-west-1.

Also - should log messages be updated? Particularly debug logs, to include the region? Maybe not super necessary if we print the stack names themselves.

This is definitely something I wanted to talk about. At the moment, when we log a step, we log the stack's FQN:

https://github.com/remind101/stacker/blob/b58e01c7281b925311bfad95a20fc68432b57635/stacker/plan.py#L63-L64

This has worked ok for now, since stack FQN's have been unique within a config, but in multi-region that's no longer true. I think we should change it so that the "node name" in the graph is logged:

    def __str__(self):
-        return self.stack.fqn
+        return self.stack.name

That's somewhat of a breaking change (it's just logging to stderr, so not too breaking), but probably necessary.

phobologic · 2018-03-13T16:22:58Z

That's somewhat of a breaking change (it's just logging to stderr, so not too breaking), but probably necessary.

I'm fine with this changing. We can share on the channel/make sure it's outlined in the CHANGELOG (maybe update that now so we don't lose it).

This allows the stack name to be overriden. The main thing that this allows, is for multiple "stacks" using the same stack name, which will be important in multi account/region.

Multi region support

ejholmes requested a review from a team March 9, 2018 07:58

ejholmes force-pushed the multi-region branch from 0d979b9 to 0719dc7 Compare March 10, 2018 00:04

ejholmes force-pushed the node-names branch from ad6979d to c8cf7d6 Compare March 10, 2018 00:05

ejholmes mentioned this pull request Mar 10, 2018

Cross account support #553

Merged

2 tasks

ejholmes force-pushed the multi-region branch from 0719dc7 to 44288ac Compare March 10, 2018 02:59

ejholmes changed the base branch from node-names to master March 10, 2018 03:00

ejholmes force-pushed the multi-region branch 2 times, most recently from c8a48ae to dab7779 Compare March 10, 2018 06:57

ejholmes commented Mar 10, 2018

View reviewed changes

ejholmes force-pushed the multi-region branch 4 times, most recently from dd01f19 to c1ee087 Compare March 12, 2018 08:03

phobologic requested changes Mar 12, 2018

View reviewed changes

ejholmes force-pushed the multi-region branch 3 times, most recently from d785bce to 42cba20 Compare March 13, 2018 02:21

Add stack_name field to stacker.config.Stack

6625724

This allows the stack name to be overriden. The main thing that this allows, is for multiple "stacks" using the same stack name, which will be important in multi account/region.

ejholmes force-pushed the multi-region branch from 42cba20 to af7f87a Compare March 14, 2018 02:37

Add support for specifying stack region

3385536

ejholmes force-pushed the multi-region branch from af7f87a to 3385536 Compare March 14, 2018 02:54

ejholmes merged commit f8f83c3 into master Mar 14, 2018

This was referenced Mar 14, 2018

Configure region outside of the CLI? #277

Closed

Log step name, instead of stack fqn #559

Merged

ejholmes mentioned this pull request Apr 11, 2018

Add a safety check on stack ARN when building #581

Open

phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018

Merge pull request cloudtools#551 from remind101/multi-region

bc5c14c

Multi region support

Conversation

ejholmes commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phobologic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phobologic commented Mar 12, 2018

Uh oh!

ejholmes commented Mar 12, 2018

Uh oh!

phobologic commented Mar 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ejholmes commented Mar 9, 2018 •

edited

Loading

codecov-io commented Mar 10, 2018 •

edited

Loading