Add aws snapshot image upload test

This adds test coverage of the AWS snapshot image upload method
(and fixes some issues preventing it from working).

One section of the upload method is removed from an exception handler
because it is required to succeed by code that follows.  This just
allows for better error reporting in the unlikely case that section
fails (the result is the same).

Change-Id: Ifc05a34e2c8e4bb71eed5ba39be69d2de476556e

tests/base.py

@@ -3234,6 +3234,13 @@ class ZuulTestCase(BaseTestCase):
         return (self.scheds.first.sched.local_layout_state ==
                 self.launcher.local_layout_state)
 
+    def __areAllImagesUploaded(self):
+        # TODO: this may need to check for failed image uploads
+        for upload in self.launcher.image_upload_registry.getItems():
+            if not upload.external_id:
+                return False
+        return True
+
     def waitUntilSettled(self, msg="", matcher=None) -> None:
         self.log.debug("Waiting until settled... (%s)", msg)
         start = time.time()
@@ -3244,7 +3251,7 @@ class ZuulTestCase(BaseTestCase):
                 self.log.error("Timeout waiting for Zuul to settle")
                 self.log.debug("All schedulers primed: %s",
                                self.__areAllSchedulersPrimed(matcher))
-                self.log.debug("All launchers primed: %s",
+                self.log.debug("All launchers synced: %s",
                                self.__areAllLaunchersSynced())
                 self._logQueueStatus(
                     self.log.error, matcher,
@@ -3254,6 +3261,7 @@ class ZuulTestCase(BaseTestCase):
                     self.__areAllBuildsWaiting(),
                     self.__areAllNodeRequestsComplete(),
                     self.__areAllNodesetRequestsComplete(),
+                    self.__areAllImagesUploaded(),
                     all(self.__eventQueuesEmpty(matcher))
                 )
                 raise Exception("Timeout waiting for Zuul to settle")
@@ -3277,6 +3285,7 @@ class ZuulTestCase(BaseTestCase):
                         self.__areAllBuildsWaiting() and
                         self.__areAllNodeRequestsComplete() and
                         self.__areAllNodesetRequestsComplete() and
+                        self.__areAllImagesUploaded() and
                         self.__areZooKeeperEventQueuesEmpty() and
                         all(self.__eventQueuesEmpty(matcher))):
                         # The queue empty check is placed at the end to
@@ -3322,6 +3331,7 @@ class ZuulTestCase(BaseTestCase):
                         all_merge_jobs_waiting, all_builds_reported,
                         all_builds_waiting, all_node_requests_completed,
                         all_nodeset_requests_completed,
+                        all_images_uploaded,
                         all_event_queues_empty):
         logger("Queue status:")
         for event_queue in self.__event_queues(matcher):
@@ -3334,6 +3344,7 @@ class ZuulTestCase(BaseTestCase):
         logger("All requests completed: %s", all_node_requests_completed)
         logger("All nodeset requests completed: %s",
                all_nodeset_requests_completed)
+        logger("All images uploaded: %s", all_images_uploaded)
         logger("All event queues empty: %s", all_event_queues_empty)
 
     def waitForPoll(self, poller, timeout=30):

tests/fake_aws.py

@@ -239,6 +239,8 @@ class FakeAws:
 
 
 class FakeAwsProviderEndpoint(AwsProviderEndpoint):
+    IMAGE_UPLOAD_SLEEP = 1
+
     # Patch/override adapter methods to aid unit tests
     def __init__(self, *args, **kw):
         super().__init__(*args, **kw)

tests/fixtures/layouts/aws/nodepool-image-snapshot.yaml

@@ -0,0 +1,80 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- pipeline:
+    name: image
+    manager: independent
+    trigger:
+      zuul:
+        - event: image-build
+    success:
+      zuul:
+        image-built: true
+        image-validated: true
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+    nodeset:
+      nodes:
+        - label: ubuntu-xenial
+          name: controller
+
+- job:
+    name: build-debian-local-image
+    image-build-name: debian-local
+
+- project:
+    name: org/common-config
+    image:
+      jobs:
+        - build-debian-local-image
+
+- image:
+    name: debian-local
+    type: zuul
+
+- flavor:
+    name: normal
+
+- label:
+    name: debian-local-normal
+    image: debian-local
+    flavor: normal
+
+- section:
+    name: aws-base
+    abstract: true
+    connection: aws
+    boot-timeout: 120
+    launch-timeout: 600
+    object-storage:
+      bucket-name: zuul
+    flavors:
+      - name: normal
+        instance-type: t3.medium
+    images:
+      - name: debian-local
+
+- section:
+    name: aws-us-west-2
+    parent: aws-base
+    region: us-west-2
+
+- provider:
+    name: aws-us-west-2-main
+    section: aws-us-west-2
+    labels:
+      - name: debian-local-normal
+        key-name: zuul

tests/unit/test_aws_driver.py

@@ -29,6 +29,7 @@ from tests.base import (
     ZuulTestCase,
     iterate_timeout,
     simple_layout,
+    return_data,
 )
 
 
@@ -195,3 +196,48 @@ class TestAwsDriver(ZuulTestCase):
                 'zuul.driver.aws.awsendpoint.AwsProviderEndpoint.'
                 '_completeCreateInstance', return_value=None)):
             yield
+
+    debian_return_data = {
+        'zuul': {
+            'artifacts': [
+                {
+                    'name': 'raw image',
+                    'url': 'http://example.com/image.raw',
+                    'metadata': {
+                        'type': 'zuul_image',
+                        'image_name': 'debian-local',
+                        'format': 'raw',
+                        'sha256': ('59984dd82f51edb3777b969739a92780'
+                                   'a520bb314b8d64b294d5de976bd8efb9'),
+                        'md5sum': '262278e1632567a907e4604e9edd2e83',
+                    }
+                },
+            ]
+        }
+    }
+
+    @simple_layout('layouts/aws/nodepool-image-snapshot.yaml',
+                   enable_nodepool=True)
+    @return_data(
+        'build-debian-local-image',
+        'refs/heads/master',
+        debian_return_data,
+    )
+    def test_aws_diskimage_snapshot(self):
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name='build-debian-local-image', result='SUCCESS'),
+        ], ordered=False)
+
+        name = 'review.example.com%2Forg%2Fcommon-config/debian-local'
+        artifacts = self.launcher.image_build_registry.\
+            getArtifactsForImage(name)
+        self.assertEqual(1, len(artifacts))
+        self.assertEqual('raw', artifacts[0].format)
+        self.assertTrue(artifacts[0].validated)
+        uploads = self.launcher.image_upload_registry.getUploadsForImage(
+            name)
+        self.assertEqual(1, len(uploads))
+        self.assertEqual(artifacts[0].uuid, uploads[0].artifact_uuid)
+        self.assertIn('ami-', uploads[0].external_id)
+        self.assertTrue(uploads[0].validated)

zuul/driver/aws/awsendpoint.py

@@ -751,11 +751,11 @@ class AwsProviderEndpoint(BaseProviderEndpoint):
             raise Exception(f"Error uploading image: {task}")
 
         # Tag the snapshot
+        with self.non_mutating_rate_limiter:
+            resp = self.ec2_client.describe_snapshots(
+                SnapshotIds=[task['SnapshotTaskDetail']['SnapshotId']])
+            snap = resp['Snapshots'][0]
         try:
-            with self.non_mutating_rate_limiter:
-                resp = self.ec2_client.describe_snapshots(
-                    SnapshotIds=[task['SnapshotTaskDetail']['SnapshotId']])
-                snap = resp['Snapshots'][0]
             with self.rate_limiter:
                 self.ec2_client.create_tags(
                     Resources=[task['SnapshotTaskDetail']['SnapshotId']],

zuul/driver/aws/awsprovider.py

@@ -78,6 +78,11 @@ class AwsProviderImage(BaseProviderImage):
         self.import_method = 'snapshot'
         self.imds_support = None
         self.architecture = 'x86_64'
+        self.volume_size = None
+        self.volume_type = 'gp3'
+        self.iops = None
+        self.throughput = None
+        self.ena_support = True
         super().__init__(config)