Merge pull request #345 from CloudVE/aws-test-image-secret

Instrument the cloud suites to find where the AWS time goes

.github/workflows/integration-cloud.yaml

@@ -110,6 +110,7 @@ jobs:
           # cell that needs it. Limits blast radius if a single cell is
           # compromised.
           # aws — credentials supplied via the OIDC step above
+          CB_IMAGE_AWS: ${{ matrix.cloud-provider == 'aws' && secrets.CB_IMAGE_AWS || '' }}
           CB_VM_TYPE_AWS: ${{ matrix.cloud-provider == 'aws' && secrets.CB_VM_TYPE_AWS || '' }}
           # azure
           AZURE_CLIENT_ID: ${{ matrix.cloud-provider == 'azure' && secrets.AZURE_CLIENT_ID || '' }}

tests/conftest.py

@@ -0,0 +1,133 @@
+"""
+Opt-in tracing for the cloud integration suites.
+
+The AWS suite's wall time is dominated by one or two tests, and end-to-end
+timings have proved too noisy to attribute - test_create_and_list_image alone
+has measured 26.2, 60.6 and 33.8 minutes across three runs of the same 123
+tests. Answering *where* that time goes needs the individual waits timed
+rather than the test as a whole.
+
+Set CB_TEST_TRACE=1 to capture, per xdist worker:
+
+
+* every ``wait_for`` poll, which brackets each wait - the span from a wait's
+  first poll to its last is the wait itself, and the gap between consecutive
+  polls shows whether the state call is slow (the interval is 1s for real
+  providers, so a larger gap is the API, not the sleep);
+* botocore retries, which is how throttling appears client-side. Those
+  loggers stay silent unless a request is actually retried, so anything they
+  emit is signal.
+
+Records go to cb-trace-<worker>.log rather than stderr because pytest
+captures stderr at file-descriptor level, and captured output is discarded
+for passing tests - which these are. tox prints the files once the run
+finishes.
+
+CB_TEST_TRACE=2 additionally logs one line per boto invocation. Level 1
+answers whether time is going into polling or into throttled retries; when
+the answer is neither - as it was for test_create_and_list_image, where 1184
+of 1400 seconds passed in a single stretch with no polls and no retries at
+all - level 2 is what names the call that blocked, since the gap between two
+consecutive request lines is that request.
+
+Deliberately opt-in: at a 1s poll interval a 30 minute wait is ~1800 lines
+for a single waiting resource, and level 2 is roughly 20x that again.
+"""
+import logging
+import os
+
+TRACE_FILE_PREFIX = 'cb-trace-'
+
+
+def _trace_level():
+    """
+    0 off; 1 waits and retries; 2 also every provider request.
+
+    Level 1 answers "is the time going into polling, or into throttled
+    retries" - it is cheap enough to leave on. Level 2 additionally logs each
+    boto invocation, so the gap between two consecutive lines names the call
+    that blocked; that is what level 1 cannot show, at perhaps 20x the volume.
+    """
+    raw = (os.environ.get('CB_TEST_TRACE') or '').lower()
+    if raw in ('2', 'requests', 'all'):
+        return 2
+    return 1 if raw in ('1', 'true', 'yes') else 0
+
+
+def _trace_path():
+    # Each xdist worker needs its own file; they run in one directory and
+    # would otherwise interleave mid-line.
+    worker = os.environ.get('PYTEST_XDIST_WORKER', 'main')
+    return '{0}{1}.log'.format(TRACE_FILE_PREFIX, worker)
+
+
+class _TraceFilter(logging.Filter):
+    """
+    Keep the test markers, the poll lines and genuine retries; at level 2
+    keep the provider request lines too, and drop everything else.
+
+    cloudbridge at DEBUG is far too chatty to keep wholesale - roughly 20x
+    the volume - and most of it says nothing about where the time went.
+    """
+
+    def __init__(self, level):
+        super(_TraceFilter, self).__init__()
+        self.level = level
+
+    def filter(self, record):
+        message = record.getMessage()
+        if record.name.startswith('botocore'):
+            # botocore logs a line per request whether or not it retried;
+            # only an actual backoff says anything about throttling. The two
+            # retry implementations word the negative case differently.
+            return not (message.startswith('Not retrying')
+                        or message.startswith('No retry needed'))
+        if record.name.startswith('cloudbridge.providers'):
+            return self.level >= 2
+        return message.startswith('=== ') or 'Waiting another' in message
+
+
+def pytest_configure(config):
+    level = _trace_level()
+    if not level:
+        return
+
+    handler = logging.FileHandler(_trace_path(), mode='w')
+    handler.setFormatter(logging.Formatter(
+        '%(asctime)s %(name)s %(message)s'))
+    handler.addFilter(_TraceFilter(level))
+
+    # wait_for logs one line per poll at DEBUG, naming the object and the
+    # state it is waiting on. Scope the level to that module rather than the
+    # cloudbridge tree: raising the whole tree to DEBUG would have every
+    # provider request build a log record that the filter then discards, and
+    # those records still propagate to pytest's own capture handler, which
+    # holds them for the duration of the test.
+    loggers = ['cloudbridge.base.resources',
+               # Quiet unless a request is actually retried, which is how
+               # throttling shows up client-side. Enabling botocore wholesale
+               # would log every request and response.
+               'botocore.retries', 'botocore.retryhandler']
+    if level >= 2:
+        # One line per boto invocation, so a stretch with no wait_for polling
+        # can still be attributed to the call that blocked.
+        loggers.append('cloudbridge.providers')
+
+    for name in loggers:
+        target = logging.getLogger(name)
+        target.addHandler(handler)
+        target.setLevel(logging.DEBUG)
+
+
+def pytest_runtest_logstart(nodeid, location):
+    # Stamps the trace with test boundaries, so a span of polls can be
+    # attributed to the test that caused it.
+    if _trace_level():
+        logging.getLogger('cloudbridge.base.resources').debug(
+            '=== START %s', nodeid)
+
+
+def pytest_runtest_logfinish(nodeid, location):
+    if _trace_level():
+        logging.getLogger('cloudbridge.base.resources').debug(
+            '=== END %s', nodeid)

tests/helpers/__init__.py

@@ -79,36 +79,71 @@ def skipIfPython(op, major, minor):
     return wrap
 
 
+def env_or(varname, default_value):
+    """
+    Environment variable ``varname``, treating unset and empty alike.
+
+    The cloud workflow sets every CB_* variable in every matrix cell, as
+    ``${{ matrix.cloud-provider == '<x>' && secrets.<VAR> || '' }}``, so a
+    variable belonging to another cell - or one whose secret is simply not
+    configured - arrives as an empty string rather than absent. A plain
+    ``os.environ.get`` hands that empty string straight back, silently
+    blanking the test data instead of falling back to the default.
+    """
+    return cb_helpers.get_env(varname) or default_value
+
+
 TEST_DATA_CONFIG = {
     "AWSCloudProvider": {
-        # Match the ami value with entry in custom_amis.json for use with moto
-        "image": cb_helpers.get_env('CB_IMAGE_AWS', 'ami-aa2ea6d0'),
-        "vm_type": cb_helpers.get_env('CB_VM_TYPE_AWS', 't2.nano'),
-        "placement": cb_helpers.get_env('CB_PLACEMENT_AWS', 'us-east-1a'),
+        # Canonical's Ubuntu 16.04, built 2017 - a Xen-era HVM image, and an
+        # AMI id is per-region, so anything but us-east-1 must set
+        # CB_IMAGE_AWS. Kept deliberately, not by inertia:
+        #
+        # The comment that used to sit here said moto needed this value to
+        # match an entry in tests/fixtures/custom_amis.json. That has not been
+        # true since 3fbcba2 removed MOTO_AMIS_PATH from tox.ini - nothing
+        # loads that fixture, and moto does not validate instance-launch AMI
+        # ids, so the mock provider is indifferent to this value.
+        #
+        # With that constraint gone the obvious move was a current image, on
+        # the theory that the suite's wall time - dominated by launching an
+        # instance, snapshotting it into an AMI, launching a second instance
+        # from it, and stop/start cycling - is paying a Xen-era penalty.
+        # Measured against Ubuntu 24.04 (ami-052355af2a014bd2c), it got
+        # worse: test_create_and_list_image went 26.2 -> 33.8 min and
+        # test_instance_start_stop_methods 20.3 -> 29.1 min. Plausibly the
+        # larger image costs more to snapshot while CB_VM_TYPE_AWS stays on a
+        # pre-Nitro type, so there is no launch saving to offset it. Worth
+        # revisiting only alongside a Nitro instance type, and note the same
+        # test has measured 26.2, 60.6 and 33.8 min across three runs, so a
+        # single run cannot resolve a difference this size either way.
+        "image": env_or('CB_IMAGE_AWS', 'ami-aa2ea6d0'),
+        "vm_type": env_or('CB_VM_TYPE_AWS', 't2.nano'),
+        "placement": env_or('CB_PLACEMENT_AWS', 'us-east-1a'),
         "placement_cfg_key": "aws_zone_name"
     },
     'OpenStackCloudProvider': {
-        'image': cb_helpers.get_env('CB_IMAGE_OS',
-                                    'c66bdfa1-62b1-43be-8964-e9ce208ac6a5'),
-        "vm_type": cb_helpers.get_env('CB_VM_TYPE_OS', 'm1.tiny'),
-        "placement": cb_helpers.get_env('CB_PLACEMENT_OS', 'nova'),
+        'image': env_or('CB_IMAGE_OS',
+                        'c66bdfa1-62b1-43be-8964-e9ce208ac6a5'),
+        "vm_type": env_or('CB_VM_TYPE_OS', 'm1.tiny'),
+        "placement": env_or('CB_PLACEMENT_OS', 'nova'),
         "placement_cfg_key": "os_zone_name"
     },
     'GCPCloudProvider': {
-        'image': cb_helpers.get_env(
+        'image': env_or(
             'CB_IMAGE_GCP',
             'https://www.googleapis.com/compute/v1/projects/ubuntu-os-cloud/'
             'global/images/ubuntu-1804-bionic-v20200908'),
-        'vm_type': cb_helpers.get_env('CB_VM_TYPE_GCP', 'f1-micro'),
-        'placement': cb_helpers.get_env('GCP_ZONE_NAME', 'us-central1-a'),
+        'vm_type': env_or('CB_VM_TYPE_GCP', 'f1-micro'),
+        'placement': env_or('GCP_ZONE_NAME', 'us-central1-a'),
         "placement_cfg_key": "gcp_zone_name"
     },
     "AzureCloudProvider": {
         "image":
-            cb_helpers.get_env('CB_IMAGE_AZURE',
-                               'Canonical:0001-com-ubuntu-minimal-jammy:minimal-22_04-lts-gen2:latest'),
-        "vm_type": cb_helpers.get_env('CB_VM_TYPE_AZURE', 'Standard_DC1ds_v3'),
-        "placement": cb_helpers.get_env('CB_PLACEMENT_AZURE', 'eastus'),
+            env_or('CB_IMAGE_AZURE',
+                   'Canonical:0001-com-ubuntu-minimal-jammy:minimal-22_04-lts-gen2:latest'),
+        "vm_type": env_or('CB_VM_TYPE_AZURE', 'Standard_DC1ds_v3'),
+        "placement": env_or('CB_PLACEMENT_AZURE', 'eastus'),
         "placement_cfg_key": "azure_zone_name"
     }
 }

tox.ini

@@ -10,7 +10,15 @@ envlist = py3{.10,.13}-{aws,azure,gcp,openstack,mock},lint,mypy
 
 [testenv]
 commands = # see pyproject.toml for coverage options; setup.cfg for flake8
-           coverage run --source=cloudbridge -m pytest -v {posargs:-n 5 tests/}
+           # --durations reports the slowest tests with their setup/teardown.
+           # The cloud suites' wall time is set by one or two long tests, and
+           # this is the authoritative record of which - the alternative is
+           # inferring it from gaps between result lines in the CI log, which
+           # cannot separate a slow test from a worker waiting for work.
+           coverage run --source=cloudbridge -m pytest -v --durations=25 {posargs:-n 5 tests/}
+           # Emit any CB_TEST_TRACE output (see tests/conftest.py). A no-op
+           # when tracing is off, which is the default.
+           python -c "import glob,sys;[sys.stdout.write(open(f).read()) for f in sorted(glob.glob('cb-trace-*.log'))]"
            # Combine parallel-mode data files and emit Cobertura XML for upload
            # by coverallsapp/github-action in CI. Locally this produces
            # coverage.xml in the project root, which IDEs can also consume.
@@ -28,6 +36,10 @@ setenv =
     COVERAGE_FILE=.coverage.{envname}
 passenv =
     PYTHONUNBUFFERED
+    # Set to 1 to log every wait_for poll and every botocore retry, for
+    # working out where a slow cloud suite is actually spending its time.
+    # See tests/conftest.py; verbose by design, so opt-in.
+    CB_TEST_TRACE
     aws: CB_IMAGE_AWS
     aws: CB_VM_TYPE_AWS
     aws: CB_PLACEMENT_AWS