diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b1a71bdd97..b5c59b164d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -229,6 +229,7 @@ integration: - python setup_gitlab_ssh.py - chmod 400 /root/.ssh/id_rsa - make test tests=src/toil/test/jobStores/jobStoreTest.py + - make test tests=src/toil/test/lib/aws/test_s3.py provisioner_integration: stage: integration diff --git a/src/toil/jobStores/aws/jobStore.py b/src/toil/jobStores/aws/jobStore.py index d42dc7750c..0c111dedbc 100644 --- a/src/toil/jobStores/aws/jobStore.py +++ b/src/toil/jobStores/aws/jobStore.py @@ -57,6 +57,7 @@ from toil.jobStores.utils import (ReadablePipe, ReadableTransformingPipe, WritablePipe) +from toil.lib.aws.utils import create_s3_bucket from toil.lib.compatibility import compat_bytes from toil.lib.ec2 import establish_boto3_session from toil.lib.ec2nodes import EC2Regions @@ -744,11 +745,9 @@ def bucket_creation_pending(error): bucketExisted = False logger.debug("Bucket '%s' does not exist.", bucket_name) if create: - logger.debug("Creating bucket '%s'.", bucket_name) - location = region_to_bucket_location(self.region) - bucket = self.s3_resource.create_bucket( - Bucket=bucket_name, - CreateBucketConfiguration={'LocationConstraint': location}) + bucket = create_s3_bucket( + self.s3_resource, bucket_name, self.region + ) # Wait until the bucket exists before checking the region and adding tags bucket.wait_until_exists() @@ -757,7 +756,9 @@ def bucket_creation_pending(error): # produce an S3ResponseError with code # NoSuchBucket. We let that kick us back up to the # main retry loop. - assert self.getBucketRegion(bucket_name) == self.region + assert ( + self.getBucketRegion(bucket_name) == self.region + ), f"bucket_name: {bucket_name}, {self.getBucketRegion(bucket_name)} != {self.region}" owner_tag = os.environ.get('TOIL_OWNER_TAG') if owner_tag: @@ -1637,7 +1638,8 @@ def _delete_domain(self, domain): if not no_such_sdb_domain(e): raise - def _delete_bucket(self, bucket): + @staticmethod + def _delete_bucket(bucket): """ :param bucket: S3.Bucket """ diff --git a/src/toil/jobStores/aws/utils.py b/src/toil/jobStores/aws/utils.py index b90b959655..f58891762c 100644 --- a/src/toil/jobStores/aws/utils.py +++ b/src/toil/jobStores/aws/utils.py @@ -441,4 +441,4 @@ def region_to_bucket_location(region): def bucket_location_to_region(location): - return 'us-east-1' if location == '' else location + return "us-east-1" if location == "" or location is None else location diff --git a/src/toil/lib/aws/utils.py b/src/toil/lib/aws/utils.py index 6bde8a8095..65d851eb45 100644 --- a/src/toil/lib/aws/utils.py +++ b/src/toil/lib/aws/utils.py @@ -20,8 +20,10 @@ try: from boto.exception import BotoServerError + from boto3.resources.factory import s3 except ImportError: BotoServerError = None # AWS/boto extra is not installed + s3 = None logger = logging.getLogger(__name__) @@ -78,3 +80,17 @@ def delete_s3_bucket(bucket: str, region: Optional[str], quiet: bool = True): s3_client.delete_object(Bucket=bucket, Key=version['Key'], VersionId=version['VersionId']) s3_resource.Bucket(bucket).delete() printq(f'\n * Deleted s3 bucket successfully: {bucket}\n\n', quiet) + + +def create_s3_bucket( + s3_session: "s3.ServiceResource", bucket_name: str, region: str +) -> "s3.Bucket": + logger.debug("Creating bucket '%s' in region %s.", bucket_name, region) + if region == "us-east-1": # see https://github.com/boto/boto3/issues/125 + bucket = s3_session.create_bucket(Bucket=bucket_name) + else: + bucket = s3_session.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={"LocationConstraint": region}, + ) + return bucket diff --git a/src/toil/test/jobStores/jobStoreTest.py b/src/toil/test/jobStores/jobStoreTest.py index 788539fd0c..296ff92f87 100644 --- a/src/toil/test/jobStores/jobStoreTest.py +++ b/src/toil/test/jobStores/jobStoreTest.py @@ -41,6 +41,7 @@ NoSuchJobException) from toil.jobStores.fileJobStore import FileJobStore from toil.lib.memoize import memoize +from toil.lib.aws.utils import create_s3_bucket from toil.statsAndLogging import StatsAndLogging from toil.test import (ToilTest, make_tests, @@ -1408,14 +1409,17 @@ def _hashTestFile(self, url: str) -> str: def _createExternalStore(self): """A S3.Bucket instance is returned""" from toil.jobStores.aws.utils import retry_s3 - from toil.jobStores.aws.utils import region_to_bucket_location from toil.jobStores.aws.jobStore import establish_boto3_session - resource = establish_boto3_session().resource('s3', region_name=self.awsRegion()) - bucket = resource.Bucket('import-export-test-%s' % uuid.uuid4()) + + resource = establish_boto3_session().resource( + "s3", region_name=self.awsRegion() + ) + bucket_name = f"import-export-test-{uuid.uuid4()}" + location = self.awsRegion() for attempt in retry_s3(): with attempt: - bucket.create(CreateBucketConfiguration={'LocationConstraint': region_to_bucket_location(self.awsRegion())}) + bucket = create_s3_bucket(resource, bucket_name, location) bucket.wait_until_exists() return bucket diff --git a/src/toil/test/lib/aws/__init__.py b/src/toil/test/lib/aws/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/toil/test/lib/aws/test_s3.py b/src/toil/test/lib/aws/test_s3.py new file mode 100644 index 0000000000..27b093b183 --- /dev/null +++ b/src/toil/test/lib/aws/test_s3.py @@ -0,0 +1,53 @@ +# Copyright (C) 2021 Michael R. Crusoe +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import logging +import os + +from toil.jobStores.aws.jobStore import AWSJobStore +from toil.lib.aws.utils import create_s3_bucket +from toil.lib.ec2 import establish_boto3_session +from toil.test import ToilTest, needs_aws_s3 + +logger = logging.getLogger(__name__) +logging.basicConfig(level=logging.DEBUG) + + +@needs_aws_s3 +class S3Test(ToilTest): + """Confirm the workarounds for us-east-1.""" + + @classmethod + def setUpClass(cls): + session = establish_boto3_session(region_name="us-east-1") + cls.s3_resource = session.resource("s3", region_name="us-east-1") + cls.s3_client = cls.s3_resource.meta.client + cls.bucket = None + + def test_create_bucket(self): + """Test bucket creation for us-east-1.""" + bucket_name = f"toil-S3Test-{uuid.uuid4()}" + cls.bucket = create_s3_bucket(cls.s3_resource, bucket_name, "us-east-1") + cls.bucket.wait_until_exists() + owner_tag = os.environ.get("TOIL_OWNER_TAG") + if owner_tag: + bucket_tagging = cls.s3_resource.BucketTagging(bucket_name) + bucket_tagging.put( + Tagging={"TagSet": [{"Key": "Owner", "Value": owner_tag}]} + ) + self.assertTrue(AWSJobStore.getBucketRegion, "us-east-1") + + @classmethod + def tearDownClass(cls): + AWSJobStore._delete_bucket(cls.bucket) + super(S3Test, cls).tearDownClass()