Multi region support #551
Multi region support #551
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
c8a48ae to
dab7779
Compare
stacker/stack.py
Outdated
| @@ -62,7 +62,8 @@ class Stack(object): | |||
| def __init__(self, definition, context, variables=None, mappings=None, | |||
| 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) | |||
There was a problem hiding this comment.
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)dd01f19 to
c1ee087
Compare
| 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**: |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
stacker/actions/build.py
Outdated
| @@ -219,8 +219,11 @@ def _launch_stack(self, stack, **kwargs): | |||
| if not should_submit(stack): | |||
| return NotSubmittedStatus() | |||
|
|
|||
| region = stack.region | |||
| provider = self.provider.build(region=region) | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yeah, I only did this to keep the diff small. I'll go ahead and make this change.
stacker/commands/stacker/__init__.py
Outdated
| @@ -30,7 +30,7 @@ def configure(self, options, **kwargs): | |||
| environment=options.environment, | |||
| validate=True) | |||
|
|
|||
| options.provider = default.Provider( | |||
| options.provider = default.ProviderBuilder( | |||
There was a problem hiding this comment.
options.provider -> options.provider_builder, etc, based on my comment above
| self.kwargs = kwargs | ||
|
|
||
| def build(self, region=None): | ||
| # TODO(ejholmes): This class _could_ cache built providers, however, |
There was a problem hiding this comment.
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.
stacker/providers/aws/default.py
Outdated
| session = get_session(region=region) | ||
| return Provider(session, **self.kwargs) | ||
|
|
||
| def tail_stack(self, stack, cancel, retries=0, **kwargs): |
There was a problem hiding this comment.
Curious why you brought this in here, rather than modifying the tail code elsewhere?
There was a problem hiding this comment.
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_stackAnd 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.
|
Also - should log messages be updated? Particularly debug logs, to include the region? Maybe not super necessary if we print the stack names themselves. |
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.
This is definitely something I wanted to talk about. At the moment, when we log a step, we log the stack's FQN: 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.nameThat's somewhat of a breaking change (it's just logging to stderr, so not too breaking), but probably necessary. |
d785bce to
42cba20
Compare
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
This implements support for provisioning stacks in multiple regions. You can target a stack to a specific region using the new per stack
regionoption.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
outputlookup.In the long term, I'm also planning on adding multi account support, aws profiles, but I still think having a
regionflag 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
--tail(breaks because it doesn't happen within_launch_stack, so it doesn't get the stack specific region applied).FunctionalTestsblueprint to give stacker user access to all regions.