refactor integration testing infrastructure (#610)

* Separated IntegrationClient into separate cloud and instance abstractions.
  This makes it easier to control the lifetime of the pycloudlib's cloud
  and instance abstractions separately.
* Created new cloud-specific subclasses accordingly
* Moved platform parsing and initialization code into its own file
* Created new session-wide autorun fixture to automatically initialize
  and destroy the dynamic cloud

3 files changed, 202 insertions, 136 deletions

diff --git a/tests/integration_tests/clouds.py b/tests/integration_tests/clouds.py
new file mode 100644
index 00000000..06f1c623
--- /dev/null
+++ b/tests/integration_tests/clouds.py
@@ -0,0 +1,126 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+from abc import ABC, abstractmethod
+import logging
+
+from pycloudlib import EC2, GCE, Azure, OCI, LXD
+
+from tests.integration_tests import integration_settings
+from tests.integration_tests.instances import (
+    IntegrationEc2Instance,
+    IntegrationGceInstance,
+    IntegrationAzureInstance, IntegrationInstance,
+    IntegrationOciInstance,
+    IntegrationLxdContainerInstance,
+)
+
+try:
+    from typing import Optional
+except ImportError:
+    pass
+
+
+log = logging.getLogger('integration_testing')
+
+
+class IntegrationCloud(ABC):
+    datasource = None  # type: Optional[str]
+    integration_instance_cls = IntegrationInstance
+
+    def __init__(self, settings=integration_settings):
+        self.settings = settings
+        self.cloud_instance = self._get_cloud_instance()
+        self.image_id = self._get_initial_image()
+
+    @abstractmethod
+    def _get_cloud_instance(self):
+        raise NotImplementedError
+
+    def _get_initial_image(self):
+        image_id = self.settings.OS_IMAGE
+        try:
+            image_id = self.cloud_instance.released_image(
+                self.settings.OS_IMAGE)
+        except (ValueError, IndexError):
+            pass
+        return image_id
+
+    def launch(self, user_data=None, launch_kwargs=None,
+               settings=integration_settings):
+        if self.settings.EXISTING_INSTANCE_ID:
+            log.info(
+                'Not launching instance due to EXISTING_INSTANCE_ID. '
+                'Instance id: %s', self.settings.EXISTING_INSTANCE_ID)
+            self.instance = self.cloud_instance.get_instance(
+                self.settings.EXISTING_INSTANCE_ID
+            )
+            return
+        launch_kwargs = {
+            'image_id': self.image_id,
+            'user_data': user_data,
+            'wait': False,
+        }
+        launch_kwargs.update(launch_kwargs)
+        pycloudlib_instance = self.cloud_instance.launch(**launch_kwargs)
+        pycloudlib_instance.wait(raise_on_cloudinit_failure=False)
+        log.info('Launched instance: %s', pycloudlib_instance)
+        return self.get_instance(pycloudlib_instance, settings)
+
+    def get_instance(self, cloud_instance, settings=integration_settings):
+        return self.integration_instance_cls(self, cloud_instance, settings)
+
+    def destroy(self):
+        pass
+
+    def snapshot(self, instance):
+        return self.cloud_instance.snapshot(instance, clean=True)
+
+
+class Ec2Cloud(IntegrationCloud):
+    datasource = 'ec2'
+    integration_instance_cls = IntegrationEc2Instance
+
+    def _get_cloud_instance(self):
+        return EC2(tag='ec2-integration-test')
+
+
+class GceCloud(IntegrationCloud):
+    datasource = 'gce'
+    integration_instance_cls = IntegrationGceInstance
+
+    def _get_cloud_instance(self):
+        return GCE(
+            tag='gce-integration-test',
+            project=self.settings.GCE_PROJECT,
+            region=self.settings.GCE_REGION,
+            zone=self.settings.GCE_ZONE,
+        )
+
+
+class AzureCloud(IntegrationCloud):
+    datasource = 'azure'
+    integration_instance_cls = IntegrationAzureInstance
+
+    def _get_cloud_instance(self):
+        return Azure(tag='azure-integration-test')
+
+    def destroy(self):
+        self.cloud_instance.delete_resource_group()
+
+
+class OciCloud(IntegrationCloud):
+    datasource = 'oci'
+    integration_instance_cls = IntegrationOciInstance
+
+    def _get_cloud_instance(self):
+        return OCI(
+            tag='oci-integration-test',
+            compartment_id=self.settings.OCI_COMPARTMENT_ID
+        )
+
+
+class LxdContainerCloud(IntegrationCloud):
+    datasource = 'lxd_container'
+    integration_instance_cls = IntegrationLxdContainerInstance
+
+    def _get_cloud_instance(self):
+        return LXD(tag='lxd-integration-test')
diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py
index 9c4eb03a..9163ac66 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -1,21 +1,32 @@
 # This file is part of cloud-init. See LICENSE file for license information.
-import os
 import logging
+import os
 import pytest
 import sys
 from contextlib import contextmanager
 
 from tests.integration_tests import integration_settings
-from tests.integration_tests.platforms import (
-    dynamic_client,
-    LxdContainerClient,
-    client_name_to_class
+from tests.integration_tests.clouds import (
+    Ec2Cloud,
+    GceCloud,
+    AzureCloud,
+    OciCloud,
+    LxdContainerCloud,
 )
 
+
 log = logging.getLogger('integration_testing')
 log.addHandler(logging.StreamHandler(sys.stdout))
 log.setLevel(logging.INFO)
 
+platforms = {
+    'ec2': Ec2Cloud,
+    'gce': GceCloud,
+    'azure': AzureCloud,
+    'oci': OciCloud,
+    'lxd_container': LxdContainerCloud,
+}
+
 
 def pytest_runtest_setup(item):
     """Skip tests on unsupported clouds.
@@ -25,7 +36,7 @@ def pytest_runtest_setup(item):
     platform, then skip the test. If platform specific marks are not
     specified, then we assume the test can be run anywhere.
     """
-    all_platforms = client_name_to_class.keys()
+    all_platforms = platforms.keys()
     supported_platforms = set(all_platforms).intersection(
         mark.name for mark in item.iter_markers())
     current_platform = integration_settings.PLATFORM
@@ -40,33 +51,51 @@ def disable_subp_usage(request):
     pass
 
 
+@pytest.yield_fixture(scope='session')
+def session_cloud():
+    if integration_settings.PLATFORM not in platforms.keys():
+        raise ValueError(
+            "{} is an invalid PLATFORM specified in settings. "
+            "Must be one of {}".format(
+                integration_settings.PLATFORM, list(platforms.keys())
+            )
+        )
+
+    cloud = platforms[integration_settings.PLATFORM]()
+    yield cloud
+    cloud.destroy()
+
+
 @pytest.fixture(scope='session', autouse=True)
-def setup_image():
+def setup_image(session_cloud):
     """Setup the target environment with the correct version of cloud-init.
 
     So we can launch instances / run tests with the correct image
     """
-    client = dynamic_client()
-    log.info('Setting up environment for %s', client.datasource)
-    client.emit_settings_to_log()
+    client = None
+    log.info('Setting up environment for %s', session_cloud.datasource)
     if integration_settings.CLOUD_INIT_SOURCE == 'NONE':
         pass  # that was easy
     elif integration_settings.CLOUD_INIT_SOURCE == 'IN_PLACE':
-        if not isinstance(client, LxdContainerClient):
+        if session_cloud.datasource != 'lxd_container':
             raise ValueError(
                 'IN_PLACE as CLOUD_INIT_SOURCE only works for LXD')
         # The mount needs to happen after the instance is launched, so
         # no further action needed here
     elif integration_settings.CLOUD_INIT_SOURCE == 'PROPOSED':
-        client.launch()
+        client = session_cloud.launch()
         client.install_proposed_image()
     elif integration_settings.CLOUD_INIT_SOURCE.startswith('ppa:'):
-        client.launch()
+        client = session_cloud.launch()
         client.install_ppa(integration_settings.CLOUD_INIT_SOURCE)
     elif os.path.isfile(str(integration_settings.CLOUD_INIT_SOURCE)):
-        client.launch()
+        client = session_cloud.launch()
         client.install_deb()
-    if client.instance:
+    else:
+        raise ValueError(
+            'Invalid value for CLOUD_INIT_SOURCE setting: {}'.format(
+                integration_settings.CLOUD_INIT_SOURCE))
+    if client:
         # Even if we're keeping instances, we don't want to keep this
         # one around as it was just for image creation
         client.destroy()
@@ -74,7 +103,7 @@ def setup_image():
 
 
 @contextmanager
-def _client(request, fixture_utils):
+def _client(request, fixture_utils, session_cloud):
     """Fixture implementation for the client fixtures.
 
     Launch the dynamic IntegrationClient instance using any provided
@@ -82,28 +111,28 @@ def _client(request, fixture_utils):
     """
     user_data = fixture_utils.closest_marker_first_arg_or(
         request, 'user_data', None)
-    with dynamic_client(user_data=user_data) as instance:
+    with session_cloud.launch(user_data=user_data) as instance:
         yield instance
 
 
 @pytest.yield_fixture
-def client(request, fixture_utils):
+def client(request, fixture_utils, session_cloud):
     """Provide a client that runs for every test."""
-    with _client(request, fixture_utils) as client:
+    with _client(request, fixture_utils, session_cloud) as client:
         yield client
 
 
 @pytest.yield_fixture(scope='module')
-def module_client(request, fixture_utils):
+def module_client(request, fixture_utils, session_cloud):
     """Provide a client that runs once per module."""
-    with _client(request, fixture_utils) as client:
+    with _client(request, fixture_utils, session_cloud) as client:
         yield client
 
 
 @pytest.yield_fixture(scope='class')
-def class_client(request, fixture_utils):
+def class_client(request, fixture_utils, session_cloud):
     """Provide a client that runs once per class."""
-    with _client(request, fixture_utils) as client:
+    with _client(request, fixture_utils, session_cloud) as client:
         yield client
 
 
diff --git a/tests/integration_tests/platforms.py b/tests/integration_tests/instances.py
index bade5fe0..d64c1ab2 100644
--- a/tests/integration_tests/platforms.py
+++ b/tests/integration_tests/instances.py
@@ -1,11 +1,8 @@
 # This file is part of cloud-init. See LICENSE file for license information.
-from abc import ABC, abstractmethod
 import logging
 import os
 from tempfile import NamedTemporaryFile
 
-from pycloudlib import EC2, GCE, Azure, OCI, LXD
-from pycloudlib.cloud import BaseCloud
 from pycloudlib.instance import BaseInstance
 
 import cloudinit
@@ -13,7 +10,9 @@ from cloudinit.subp import subp
 from tests.integration_tests import integration_settings
 
 try:
-    from typing import Callable, Optional
+    from typing import TYPE_CHECKING
+    if TYPE_CHECKING:
+        from tests.integration_tests.clouds import IntegrationCloud
 except ImportError:
     pass
 
@@ -21,21 +20,14 @@ except ImportError:
 log = logging.getLogger('integration_testing')
 
 
-class IntegrationClient(ABC):
-    client = None  # type: Optional[BaseCloud]
-    instance = None  # type: Optional[BaseInstance]
-    datasource = None  # type: Optional[str]
+class IntegrationInstance:
     use_sudo = True
-    current_image = None
 
-    def __init__(self, user_data=None, instance_type=None,
-                 settings=integration_settings, launch_kwargs=None):
-        self.user_data = user_data
-        self.instance_type = settings.INSTANCE_TYPE if \
-            instance_type is None else instance_type
+    def __init__(self, cloud: 'IntegrationCloud', instance: BaseInstance,
+                 settings=integration_settings):
+        self.cloud = cloud
+        self.instance = instance
         self.settings = settings
-        self.launch_kwargs = launch_kwargs if launch_kwargs else {}
-        self.client = self._get_client()
 
     def emit_settings_to_log(self) -> None:
         log.info(
@@ -48,42 +40,6 @@ class IntegrationClient(ABC):
             )
         )
 
-    @abstractmethod
-    def _get_client(self):
-        raise NotImplementedError
-
-    def _get_image(self):
-        if self.current_image:
-            return self.current_image
-        image_id = self.settings.OS_IMAGE
-        try:
-            image_id = self.client.released_image(self.settings.OS_IMAGE)
-        except (ValueError, IndexError):
-            pass
-        return image_id
-
-    def launch(self):
-        if self.settings.EXISTING_INSTANCE_ID:
-            log.info(
-                'Not launching instance due to EXISTING_INSTANCE_ID. '
-                'Instance id: %s', self.settings.EXISTING_INSTANCE_ID)
-            self.instance = self.client.get_instance(
-                self.settings.EXISTING_INSTANCE_ID
-            )
-            return
-        image_id = self._get_image()
-        launch_args = {
-            'image_id': image_id,
-            'user_data': self.user_data,
-            'wait': False,
-        }
-        if self.instance_type:
-            launch_args['instance_type'] = self.instance_type
-        launch_args.update(self.launch_kwargs)
-        self.instance = self.client.launch(**launch_args)
-        self.instance.wait(raise_on_cloudinit_failure=False)
-        log.info('Launched instance: %s', self.instance)
-
     def destroy(self):
         self.instance.delete()
 
@@ -115,7 +71,7 @@ class IntegrationClient(ABC):
             os.unlink(tmp_file.name)
 
     def snapshot(self):
-        return self.client.snapshot(self.instance, clean=True)
+        return self.cloud.snapshot(self.instance)
 
     def _install_new_cloud_init(self, remote_script):
         self.execute(remote_script)
@@ -124,7 +80,7 @@ class IntegrationClient(ABC):
         self.instance.clean()
         image_id = self.snapshot()
         log.info('Created new image: %s', image_id)
-        IntegrationClient.current_image = image_id
+        self.cloud.image_id = image_id
 
     def install_proposed_image(self):
         log.info('Installing proposed image')
@@ -159,7 +115,6 @@ class IntegrationClient(ABC):
         self._install_new_cloud_init(remote_script)
 
     def __enter__(self):
-        self.launch()
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
@@ -167,48 +122,30 @@ class IntegrationClient(ABC):
             self.destroy()
 
 
-class Ec2Client(IntegrationClient):
-    datasource = 'ec2'
-
-    def _get_client(self):
-        return EC2(tag='ec2-integration-test')
-
-
-class GceClient(IntegrationClient):
-    datasource = 'gce'
-
-    def _get_client(self):
-        return GCE(
-            tag='gce-integration-test',
-            project=self.settings.GCE_PROJECT,
-            region=self.settings.GCE_REGION,
-            zone=self.settings.GCE_ZONE,
-        )
+class IntegrationEc2Instance(IntegrationInstance):
+    pass
 
 
-class AzureClient(IntegrationClient):
-    datasource = 'azure'
+class IntegrationGceInstance(IntegrationInstance):
+    pass
 
-    def _get_client(self):
-        return Azure(tag='azure-integration-test')
 
+class IntegrationAzureInstance(IntegrationInstance):
+    pass
 
-class OciClient(IntegrationClient):
-    datasource = 'oci'
 
-    def _get_client(self):
-        return OCI(
-            tag='oci-integration-test',
-            compartment_id=self.settings.OCI_COMPARTMENT_ID
-        )
+class IntegrationOciInstance(IntegrationInstance):
+    pass
 
 
-class LxdContainerClient(IntegrationClient):
-    datasource = 'lxd_container'
+class IntegrationLxdContainerInstance(IntegrationInstance):
     use_sudo = False
 
-    def _get_client(self):
-        return LXD(tag='lxd-integration-test')
+    def __init__(self, cloud: 'IntegrationCloud', instance: BaseInstance,
+                 settings=integration_settings):
+        super().__init__(cloud, instance, settings)
+        if self.settings.CLOUD_INIT_SOURCE == 'IN_PLACE':
+            self._mount_source()
 
     def _mount_source(self):
         command = (
@@ -218,29 +155,3 @@ class LxdContainerClient(IntegrationClient):
         ).format(
             name=self.instance.name, cloudinit_path=cloudinit.__path__[0])
         subp(command.split())
-
-    def launch(self):
-        super().launch()
-        if self.settings.CLOUD_INIT_SOURCE == 'IN_PLACE':
-            self._mount_source()
-
-
-client_name_to_class = {
-    'ec2': Ec2Client,
-    'gce': GceClient,
-    'azure': AzureClient,
-    'oci': OciClient,
-    'lxd_container': LxdContainerClient
-}
-
-try:
-    dynamic_client = client_name_to_class[
-        integration_settings.PLATFORM
-    ]  # type: Callable[..., IntegrationClient]
-except KeyError:
-    raise ValueError(
-        "{} is an invalid PLATFORM specified in settings. "
-        "Must be one of {}".format(
-            integration_settings.PLATFORM, list(client_name_to_class.keys())
-        )
-    )