Merge pull request #568 from remind101/s3-conn-race

Fix potential race in s3_stack_push
cloudtools · Mar 20, 2018 · 031617b · 031617b
2 parents bcb5cd0 + f3e0636
commit 031617b
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 19 deletions.
diff --git a/stacker/actions/base.py b/stacker/actions/base.py
@@ -131,26 +131,17 @@ def __init__(self, context, provider_builder=None, cancel=None):
         self.provider_builder = provider_builder
         self.bucket_name = context.bucket_name
         self.cancel = cancel or threading.Event()
-        self._conn = None
-
-    @property
-    def s3_conn(self):
-        """The boto s3 connection object used for communication with S3."""
-        if not hasattr(self, "_s3_conn"):
-            # Always use the global client for s3
-            session = get_session(self.bucket_region)
-            self._s3_conn = session.client('s3')
-
-        return self._s3_conn
-
-    @property
-    def bucket_region(self):
-        return self.context.config.stacker_bucket_region \
-                or self.provider_builder.region
+        self.bucket_region = context.config.stacker_bucket_region
+        if not self.bucket_region and provider_builder:
+            self.bucket_region = provider_builder.region
+        self.s3_conn = get_session(self.bucket_region).client('s3')
 
     def ensure_cfn_bucket(self):
         """The CloudFormation bucket where templates will be stored."""
-        ensure_s3_bucket(self.s3_conn, self.bucket_name, self.bucket_region)
+        if self.bucket_name:
+            ensure_s3_bucket(self.s3_conn,
+                             self.bucket_name,
+                             self.bucket_region)
 
     def stack_template_url(self, blueprint):
         return stack_template_url(
@@ -167,7 +158,6 @@ def s3_stack_push(self, blueprint, force=False):
         """
         key_name = stack_template_key_name(blueprint)
         template_url = self.stack_template_url(blueprint)
-        self.ensure_cfn_bucket()
         try:
             template_exists = self.s3_conn.head_object(
                 Bucket=self.bucket_name, Key=key_name) is not None

diff --git a/stacker/actions/build.py b/stacker/actions/build.py
@@ -342,6 +342,7 @@ def _generate_plan(self, tail=False):
 
     def pre_run(self, outline=False, dump=False, *args, **kwargs):
         """Any steps that need to be taken prior to running the action."""
+        self.ensure_cfn_bucket()
         hooks = self.context.config.pre_build
         handle_hooks(
             "pre_build",

diff --git a/stacker/tests/actions/test_destroy.py b/stacker/tests/actions/test_destroy.py
@@ -41,7 +41,6 @@ def setUp(self):
         })
         self.context = Context(config=config)
         self.action = destroy.Action(self.context,
-                                     provider_builder=mock.MagicMock(),
                                      cancel=MockThreadingEvent())
 
     def test_generate_plan(self):