Integration testing docs and refactor (#1231)

* Include CI and Fixtures sections in integration test docs
* Incorporate additional variable annotations
* Remove unnecessary IntegrationInstance subclasses
* Move setup_image teardown into its fixture

14 files changed, 135 insertions, 96 deletions

diff --git a/doc/rtd/topics/integration_tests.rst b/doc/rtd/topics/integration_tests.rst
index 2677a213..1e910af6 100644
--- a/doc/rtd/topics/integration_tests.rst
+++ b/doc/rtd/topics/integration_tests.rst
@@ -22,30 +22,22 @@ marks can be used to limit tests to particular platforms. The
 client fixture can be used to interact with the launched
 test instance.
 
-A basic example:
-
-.. code-block:: python
-
-    USER_DATA = """#cloud-config
-    bootcmd:
-    - echo 'hello config!' > /tmp/user_data.txt"""
-
-
-    class TestSimple:
-        @pytest.mark.user_data(USER_DATA)
-        @pytest.mark.ec2
-        def test_simple(self, client):
-            print(client.exec('cloud-init -v'))
+See :ref:`Examples` section for examples.
 
 Test Execution
 ==============
-Test execution happens via pytest. To run all integration tests,
-you would run:
+Test execution happens via pytest. A tox definition exists to run integration
+tests. To run all integration tests, you would run:
 
 .. code-block:: bash
 
-    pytest tests/integration_tests/
+    $ tox -e integration-tests
 
+Pytest arguments may also be passed. For example:
+
+.. code-block:: bash
+
+    $ tox -e integration-tests tests/integration_tests/modules/test_combined.py
 
 Configuration
 =============
@@ -126,3 +118,91 @@ Test setup occurs between image setup and test execution. Test setup
 is implemented via one of the ``client`` fixtures. When a client fixture
 is used, a test instance from which to run tests is launched prior to
 test execution and torn down after.
+
+Continuous Integration
+======================
+A subset of the integration tests are run when a pull request
+is submitted on Github. The tests run on these continuous
+integration (CI) runs are given a pytest mark:
+
+.. code-block:: python
+
+    @pytest.mark.ci
+
+Most new tests should *not* use this mark, so be aware that having a
+successful CI run does not necessarily mean that your test passed
+successfully.
+
+Fixtures
+========
+Integration tests rely heavily on fixtures to do initial test setup.
+One or more of these fixtures will be used in almost every integration test.
+
+Details such as the cloud platform or initial image to use are determined
+via what is specified in the :ref:`Configuration`.
+
+client
+------
+The ``client`` fixture should be used for most test cases. It ensures:
+
+- All setup performed by :ref:`session_cloud` and :ref:`setup_image`
+- `Pytest marks <https://github.com/canonical/cloud-init/blob/af7eb1deab12c7208853c5d18b55228e0ba29c4d/tests/integration_tests/conftest.py#L220-L224>`_
+  used during instance creation are obtained and applied
+- The test instance is launched
+- Test failure status is determined after test execution
+- Logs are collected (if configured) after test execution
+- The test instance is torn down after test execution
+
+``module_client`` and ``class_client`` fixtures also exist for the
+purpose of running multiple tests against a single launched instance.
+They provide the exact same functionality as ``client``, but are
+scoped to the module or class respectively.
+
+session_cloud
+-------------
+The ``session_cloud`` session-scoped fixture will provide an
+`IntegrationCloud <https://github.com/canonical/cloud-init/blob/af7eb1deab12c7208853c5d18b55228e0ba29c4d/tests/integration_tests/clouds.py#L102>`_
+instance for the currently configured cloud. The fixture also
+ensures that any custom cloud session cleanup is performed.
+
+setup_image
+-----------
+The ``setup_image`` session-scope fixture will
+create a new image to launch all further cloud instances
+during this test run. It ensures:
+
+- A cloud instance is launched on the configured platform
+- The version of cloud-init under test is installed on the instance
+- ``cloud-init clean --logs`` is run on the instance
+- A snapshot of the instance is taken to be used as the basis for
+  future instance launches
+- The originally launched instance is torn down
+- The custom created image is torn down after all tests finish
+
+Examples
+--------
+A simple test case using the ``client`` fixture:
+
+.. code-block:: python
+
+    USER_DATA = """\
+    #cloud-config
+    bootcmd:
+    - echo 'hello!' > /var/tmp/hello.txt
+    """
+
+
+    @pytest.mark.user_data(USER_DATA)
+    def test_bootcmd(client):
+        log = client.read_from_file("/var/log/cloud-init.log")
+        assert "Shellified 1 commands." in log
+        assert client.execute('cat /var/tmp/hello.txt').strip() == "hello!"
+
+Customizing the launch arguments before launching an instance manually:
+
+.. code-block:: python
+
+    def test_launch(session_cloud: IntegrationCloud, setup_image):
+        with session_cloud.launch(launch_kwargs={"wait": False}) as client:
+            client.instance.wait()
+            assert client.execute("echo hello world").strip() == "hello world"
diff --git a/tests/integration_tests/bugs/test_gh570.py b/tests/integration_tests/bugs/test_gh570.py
index ddc74503..e98ab5d0 100644
--- a/tests/integration_tests/bugs/test_gh570.py
+++ b/tests/integration_tests/bugs/test_gh570.py
@@ -16,7 +16,6 @@ runcmd:
 
 
 # Only running on LXD because we need NoCloud for this test
-@pytest.mark.sru_2020_11
 @pytest.mark.lxd_container
 @pytest.mark.lxd_vm
 def test_nocloud_seedfrom_vendordata(client: IntegrationInstance):
diff --git a/tests/integration_tests/bugs/test_gh671.py b/tests/integration_tests/bugs/test_gh671.py
index 15f204ee..2d7c8118 100644
--- a/tests/integration_tests/bugs/test_gh671.py
+++ b/tests/integration_tests/bugs/test_gh671.py
@@ -23,7 +23,6 @@ def _check_password(instance, unhashed_password):
 
 
 @pytest.mark.azure
-@pytest.mark.sru_2020_11
 def test_update_default_password(setup_image, session_cloud: IntegrationCloud):
     os_profile = {
         "os_profile": {
diff --git a/tests/integration_tests/bugs/test_lp1813396.py b/tests/integration_tests/bugs/test_lp1813396.py
index 451a9972..ddae02f5 100644
--- a/tests/integration_tests/bugs/test_lp1813396.py
+++ b/tests/integration_tests/bugs/test_lp1813396.py
@@ -19,7 +19,6 @@ apt:
 """  # noqa: E501
 
 
-@pytest.mark.sru_2020_11
 @pytest.mark.user_data(USER_DATA)
 def test_gpg_no_tty(client: IntegrationInstance):
     log = client.read_from_file("/var/log/cloud-init.log")
diff --git a/tests/integration_tests/bugs/test_lp1835584.py b/tests/integration_tests/bugs/test_lp1835584.py
index a800eab4..765d73ef 100644
--- a/tests/integration_tests/bugs/test_lp1835584.py
+++ b/tests/integration_tests/bugs/test_lp1835584.py
@@ -1,6 +1,6 @@
 """ Integration test for LP #1835584
 
-Upstream linux kernels prior to 4.15 providate DMI product_uuid in uppercase.
+Upstream linux kernels prior to 4.15 provide DMI product_uuid in uppercase.
 More recent kernels switched to lowercase for DMI product_uuid. Azure
 datasource uses this product_uuid as the instance-id for cloud-init.
 
@@ -33,7 +33,7 @@ import pytest
 
 from tests.integration_tests.clouds import ImageSpecification, IntegrationCloud
 from tests.integration_tests.conftest import get_validated_source
-from tests.integration_tests.instances import IntegrationAzureInstance
+from tests.integration_tests.instances import IntegrationInstance
 
 IMG_AZURE_UBUNTU_PRO_FIPS_BIONIC = (
     "Canonical:0001-com-ubuntu-pro-bionic-fips:pro-fips-18_04:18.04.202010201"
@@ -41,7 +41,7 @@ IMG_AZURE_UBUNTU_PRO_FIPS_BIONIC = (
 
 
 def _check_iid_insensitive_across_kernel_upgrade(
-    instance: IntegrationAzureInstance,
+    instance: IntegrationInstance,
 ):
     uuid = instance.read_from_file("/sys/class/dmi/id/product_uuid")
     assert (
@@ -73,7 +73,6 @@ def _check_iid_insensitive_across_kernel_upgrade(
 
 
 @pytest.mark.azure
-@pytest.mark.sru_next
 def test_azure_kernel_upgrade_case_insensitive_uuid(
     session_cloud: IntegrationCloud,
 ):
diff --git a/tests/integration_tests/bugs/test_lp1897099.py b/tests/integration_tests/bugs/test_lp1897099.py
index 876a2887..1f5030ce 100644
--- a/tests/integration_tests/bugs/test_lp1897099.py
+++ b/tests/integration_tests/bugs/test_lp1897099.py
@@ -18,7 +18,6 @@ swap:
 """
 
 
-@pytest.mark.sru_2020_11
 @pytest.mark.user_data(USER_DATA)
 @pytest.mark.no_container("Containers cannot configure swap")
 def test_fallocate_fallback(client):
diff --git a/tests/integration_tests/bugs/test_lp1900837.py b/tests/integration_tests/bugs/test_lp1900837.py
index 3df10883..d9ef18aa 100644
--- a/tests/integration_tests/bugs/test_lp1900837.py
+++ b/tests/integration_tests/bugs/test_lp1900837.py
@@ -4,14 +4,12 @@ This test mirrors the reproducing steps from the reported bug: it changes the
 permissions on cloud-init.log to 600 and confirms that they remain 600 after a
 reboot.
 """
-import pytest
 
 
 def _get_log_perms(client):
     return client.execute("stat -c %a /var/log/cloud-init.log")
 
 
-@pytest.mark.sru_2020_11
 class TestLogPermissionsNotResetOnReboot:
     def test_permissions_unchanged(self, client):
         # Confirm that the current permissions aren't 600
diff --git a/tests/integration_tests/bugs/test_lp1910835.py b/tests/integration_tests/bugs/test_lp1910835.py
index ddd996f9..1844594c 100644
--- a/tests/integration_tests/bugs/test_lp1910835.py
+++ b/tests/integration_tests/bugs/test_lp1910835.py
@@ -25,7 +25,6 @@ ssh_authorized_keys:
     - {}"""
 
 
-@pytest.mark.sru_2021_01
 @pytest.mark.azure
 def test_crlf_in_azure_metadata_ssh_keys(session_cloud, setup_image):
     authorized_keys_path = "/home/{}/.ssh/authorized_keys".format(
diff --git a/tests/integration_tests/clouds.py b/tests/integration_tests/clouds.py
index 3b80caf3..81d5b9a0 100644
--- a/tests/integration_tests/clouds.py
+++ b/tests/integration_tests/clouds.py
@@ -5,6 +5,7 @@ import os.path
 import random
 import string
 from abc import ABC, abstractmethod
+from typing import Optional, Type
 from uuid import UUID
 
 from pycloudlib import (
@@ -16,27 +17,16 @@ from pycloudlib import (
     LXDVirtualMachine,
     Openstack,
 )
+from pycloudlib.cloud import BaseCloud
+from pycloudlib.lxd.cloud import _BaseLXD
 from pycloudlib.lxd.instance import LXDInstance
 
 import cloudinit
 from cloudinit.subp import ProcessExecutionError, subp
 from tests.integration_tests import integration_settings
-from tests.integration_tests.instances import (
-    IntegrationAzureInstance,
-    IntegrationEc2Instance,
-    IntegrationGceInstance,
-    IntegrationInstance,
-    IntegrationLxdInstance,
-    IntegrationOciInstance,
-)
+from tests.integration_tests.instances import IntegrationInstance
 from tests.integration_tests.util import emit_dots_on_travis
 
-try:
-    from typing import Optional  # noqa: F401
-except ImportError:
-    pass
-
-
 log = logging.getLogger("integration_testing")
 
 
@@ -73,8 +63,8 @@ class ImageSpecification:
     def __init__(
         self,
         image_id: str,
-        os: "Optional[str]" = None,
-        release: "Optional[str]" = None,
+        os: Optional[str] = None,
+        release: Optional[str] = None,
     ):
         if image_id in _get_ubuntu_series():
             if os is None:
@@ -100,20 +90,18 @@ class ImageSpecification:
 
 
 class IntegrationCloud(ABC):
-    datasource = None  # type: Optional[str]
-    integration_instance_cls = IntegrationInstance
+    datasource: str
+    cloud_instance: BaseCloud
 
     def __init__(self, settings=integration_settings):
         self.settings = settings
-        self.cloud_instance = self._get_cloud_instance()
+        self.cloud_instance: BaseCloud = self._get_cloud_instance()
         self.initial_image_id = self._get_initial_image()
         self.snapshot_id = None
 
     @property
     def image_id(self):
-        if self.snapshot_id:
-            return self.snapshot_id
-        return self.initial_image_id
+        return self.snapshot_id or self.initial_image_id
 
     def emit_settings_to_log(self) -> None:
         log.info(
@@ -147,7 +135,7 @@ class IntegrationCloud(ABC):
         launch_kwargs=None,
         settings=integration_settings,
         **kwargs
-    ):
+    ) -> IntegrationInstance:
         if launch_kwargs is None:
             launch_kwargs = {}
         if self.settings.EXISTING_INSTANCE_ID:
@@ -186,7 +174,7 @@ class IntegrationCloud(ABC):
         return instance
 
     def get_instance(self, cloud_instance, settings=integration_settings):
-        return self.integration_instance_cls(self, cloud_instance, settings)
+        return IntegrationInstance(self, cloud_instance, settings)
 
     def destroy(self):
         pass
@@ -212,7 +200,6 @@ class IntegrationCloud(ABC):
 
 class Ec2Cloud(IntegrationCloud):
     datasource = "ec2"
-    integration_instance_cls = IntegrationEc2Instance
 
     def _get_cloud_instance(self):
         return EC2(tag="ec2-integration-test")
@@ -220,7 +207,6 @@ class Ec2Cloud(IntegrationCloud):
 
 class GceCloud(IntegrationCloud):
     datasource = "gce"
-    integration_instance_cls = IntegrationGceInstance
 
     def _get_cloud_instance(self):
         return GCE(
@@ -230,7 +216,7 @@ class GceCloud(IntegrationCloud):
 
 class AzureCloud(IntegrationCloud):
     datasource = "azure"
-    integration_instance_cls = IntegrationAzureInstance
+    cloud_instance: Azure
 
     def _get_cloud_instance(self):
         return Azure(tag="azure-integration-test")
@@ -248,7 +234,6 @@ class AzureCloud(IntegrationCloud):
 
 class OciCloud(IntegrationCloud):
     datasource = "oci"
-    integration_instance_cls = IntegrationOciInstance
 
     def _get_cloud_instance(self):
         return OCI(
@@ -257,10 +242,11 @@ class OciCloud(IntegrationCloud):
 
 
 class _LxdIntegrationCloud(IntegrationCloud):
-    integration_instance_cls = IntegrationLxdInstance
+    pycloudlib_instance_cls: Type[_BaseLXD]
+    instance_tag: str
+    cloud_instance: _BaseLXD
 
     def _get_cloud_instance(self):
-        # pylint: disable=no-member
         return self.pycloudlib_instance_cls(tag=self.instance_tag)
 
     @staticmethod
@@ -331,12 +317,14 @@ class _LxdIntegrationCloud(IntegrationCloud):
 
 class LxdContainerCloud(_LxdIntegrationCloud):
     datasource = "lxd_container"
+    cloud_instance: LXDContainer
     pycloudlib_instance_cls = LXDContainer
     instance_tag = "lxd-container-integration-test"
 
 
 class LxdVmCloud(_LxdIntegrationCloud):
     datasource = "lxd_vm"
+    cloud_instance: LXDVirtualMachine
     pycloudlib_instance_cls = LXDVirtualMachine
     instance_tag = "lxd-vm-integration-test"
     _profile_list = None
@@ -352,7 +340,6 @@ class LxdVmCloud(_LxdIntegrationCloud):
 
 class OpenstackCloud(IntegrationCloud):
     datasource = "openstack"
-    integration_instance_cls = IntegrationInstance
 
     def _get_cloud_instance(self):
         return Openstack(
diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py
index 2e44ef29..b2bc9eb7 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -7,8 +7,10 @@ import sys
 from contextlib import contextmanager
 from pathlib import Path
 from tarfile import TarFile
+from typing import Dict, Type
 
 import pytest
+from pycloudlib.lxd.instance import LXDInstance
 
 from tests.integration_tests import integration_settings
 from tests.integration_tests.clouds import (
@@ -32,7 +34,7 @@ log = logging.getLogger("integration_testing")
 log.addHandler(logging.StreamHandler(sys.stdout))
 log.setLevel(logging.INFO)
 
-platforms = {
+platforms: Dict[str, Type[IntegrationCloud]] = {
     "ec2": Ec2Cloud,
     "gce": GceCloud,
     "azure": AzureCloud,
@@ -102,11 +104,10 @@ def session_cloud():
 
     cloud = platforms[integration_settings.PLATFORM]()
     cloud.emit_settings_to_log()
+
     yield cloud
-    try:
-        cloud.delete_snapshot()
-    finally:
-        cloud.destroy()
+
+    cloud.destroy()
 
 
 def get_validated_source(
@@ -135,7 +136,7 @@ def get_validated_source(
 
 
 @pytest.fixture(scope="session")
-def setup_image(session_cloud: IntegrationCloud):
+def setup_image(session_cloud: IntegrationCloud, request):
     """Setup the target environment with the correct version of cloud-init.
 
     So we can launch instances / run tests with the correct image
@@ -152,6 +153,11 @@ def setup_image(session_cloud: IntegrationCloud):
     client.destroy()
     log.info("Done with environment setup")
 
+    # For some reason a yield here raises a
+    # ValueError: setup_image did not yield a value
+    # during setup so use a finalizer instead.
+    request.addfinalizer(session_cloud.delete_snapshot)
+
 
 def _collect_logs(
     instance: IntegrationInstance, node_id: str, test_failed: bool
@@ -245,10 +251,12 @@ def _client(request, fixture_utils, session_cloud: IntegrationCloud):
     with session_cloud.launch(
         user_data=user_data, launch_kwargs=launch_kwargs, **local_launch_kwargs
     ) as instance:
-        if lxd_use_exec is not None:
+        if lxd_use_exec is not None and isinstance(
+            instance.instance, LXDInstance
+        ):
             # Existing instances are not affected by the launch kwargs, so
             # ensure it here; we still need the launch kwarg so waiting works
-            instance.execute_via_ssh = False
+            instance.instance.execute_via_ssh = False
         previous_failures = request.session.testsfailed
         yield instance
         test_failed = request.session.testsfailed - previous_failures > 0
diff --git a/tests/integration_tests/instances.py b/tests/integration_tests/instances.py
index 793f729e..e3d597af 100644
--- a/tests/integration_tests/instances.py
+++ b/tests/integration_tests/instances.py
@@ -47,9 +47,7 @@ class CloudInitSource(Enum):
     UPGRADE = 6
 
     def installs_new_version(self):
-        if self.name in [self.NONE.name, self.IN_PLACE.name]:
-            return False
-        return True
+        return self.name not in [self.NONE.name, self.IN_PLACE.name]
 
 
 class IntegrationInstance:
@@ -201,23 +199,3 @@ class IntegrationInstance:
     def __exit__(self, exc_type, exc_val, exc_tb):
         if not self.settings.KEEP_INSTANCE:
             self.destroy()
-
-
-class IntegrationEc2Instance(IntegrationInstance):
-    pass
-
-
-class IntegrationGceInstance(IntegrationInstance):
-    pass
-
-
-class IntegrationAzureInstance(IntegrationInstance):
-    pass
-
-
-class IntegrationOciInstance(IntegrationInstance):
-    pass
-
-
-class IntegrationLxdInstance(IntegrationInstance):
-    pass
diff --git a/tests/integration_tests/modules/test_cli.py b/tests/integration_tests/modules/test_cli.py
index 81a5e7a8..baaa7567 100644
--- a/tests/integration_tests/modules/test_cli.py
+++ b/tests/integration_tests/modules/test_cli.py
@@ -26,7 +26,6 @@ apt_pipelining: bogus
 """
 
 
-@pytest.mark.sru_2020_11
 @pytest.mark.user_data(VALID_USER_DATA)
 def test_valid_userdata(client: IntegrationInstance):
     """Test `cloud-init devel schema` with valid userdata.
@@ -43,7 +42,6 @@ def test_valid_userdata(client: IntegrationInstance):
         )
 
 
-@pytest.mark.sru_2020_11
 @pytest.mark.user_data(INVALID_USER_DATA_HEADER)
 def test_invalid_userdata(client: IntegrationInstance):
     """Test `cloud-init devel schema` with invalid userdata.
diff --git a/tests/integration_tests/modules/test_power_state_change.py b/tests/integration_tests/modules/test_power_state_change.py
index a629029d..5cd19764 100644
--- a/tests/integration_tests/modules/test_power_state_change.py
+++ b/tests/integration_tests/modules/test_power_state_change.py
@@ -51,7 +51,6 @@ def _can_connect(instance):
 # run anywhere, I can only get it to run in an lxd container, and even then
 # occasionally some timing issues will crop up.
 @pytest.mark.unstable
-@pytest.mark.sru_2020_11
 @pytest.mark.ubuntu
 @pytest.mark.lxd_container
 class TestPowerChange:
diff --git a/tox.ini b/tox.ini
index 5037fa69..aab6d4d4 100644
--- a/tox.ini
+++ b/tox.ini
@@ -184,9 +184,6 @@ markers =
     no_container: test cannot run in a container
     user_data: the user data to be passed to the test instance
     instance_name: the name to be used for the test instance
-    sru_2020_11: test is part of the 2020/11 SRU verification
-    sru_2021_01: test is part of the 2021/01 SRU verification
-    sru_next: test is part of the next SRU verification
     ubuntu: this test should run on Ubuntu
     unstable: skip this test because it is flakey
     adhoc: only run on adhoc basis, not in any CI environment (travis or jenkins)