Refactor integration tests to use pycloudlib LXD #1241

lucasmoura · 2020-11-11T20:08:28Z

Refactor integration tests to use pycloudlib LXD implementation. This also allows us to test enabling FIPS for xenial.

PS: This PR depends on the following pycloudlib PR

blackboxsw · 2020-11-12T03:01:40Z

features/cloud.py

+            Location of the private key path to use. If None, the location
+            will be a default location.
+        """
+        pub_key_path = "lxd-pubkey"


As you mentioned in passing, the integration tests currently take 4 times as long running via ssh to the lxd instance instead of lxc exec. Should this method be a noop (pass) or not be called in typical lxd testing used in our per-PR integration tests? We could rely on the ssh-based lxd communication in either our daily CRON or daily jenkins tests.

blackboxsw

just a quick pass on this. will get detailed review tomorrow.

features/cloud.py

lucasmoura · 2020-11-12T15:00:30Z

@blackboxsw One idea that I had is that we can use the ssh option only for vm tests, since it is only those tests that require the use of ssh.

To make that possible, we need to first merge this PR

lucasmoura · 2020-11-24T14:46:52Z

@blackboxsw I have update the code with the refactoring and FIPS tests.

Right now, the FIPS Updates upgrade test is just assuring that we end up in a scenario were we have the FIPS metapackage uninstalled because of the do-release-upgrade issue. Once it has been fixed, I can update the test without an issue.

blackboxsw

As you mentioned in chat. something is occuring with the shlex usage when trying to run cmds that emit json since we are splitting the cmdstring blindly on spaces. I've suggested a few changes inline but I think we also may have to sort to remaining checks in features/environment which check for a possible path where cloud_api is not set since I don't think we would have that path anymore right?

features/environment: (before_all) if context.config.cloud_api:
features/environment:583 (_install_uat_in_container) if not config.cloud_api

blackboxsw · 2020-11-24T22:35:34Z

features/cloud.py

@@ -116,10 +123,12 @@ def launch(
        :returns:
            An cloud provider instance
        """
-        inst = self._create_instance(series, image_name, user_data)
+        inst = self._create_instance(
+            series, instance_name, image_name, user_data


let's use names kwargs here so if we change call signature or param ordering in the future, we don't break.
I get a bad feeling every time I see positional arguments re-ordered because we inserted a new one earlier sequentiall than previous params.

Suggested change

series, instance_name, image_name, user_data

series=series, instance_name=instance_name, image_name=image_name, user_data=user_data

blackboxsw · 2020-11-24T22:54:05Z

uaclient/entitlements/fips.py

@@ -182,7 +182,7 @@ class FIPSUpdatesEntitlement(FIPSCommonEntitlement):

    name = "fips-updates"
    title = "FIPS Updates"
-    origin = "UbuntuFIPSUpdates"
+    origin = "UbuntuFIPS"


Let's please put a comment in here # RELEASE_BLOCKER and reference your RT or a new ua-client issue so we are aware of the blocker

Suggested change

origin = "UbuntuFIPS"

origin = "UbuntuFIPS" # RELEASE_BLOCKER RT: #128770

blackboxsw · 2020-11-24T22:56:49Z

features/cloud.py

+
+
+class LXDContainer(_LXD):
+    name = "lxd-virtual-machine"


Suggested change

name = "lxd-virtual-machine"

name = "lxd-container"

blackboxsw · 2020-11-24T23:00:09Z

features/cloud.py

+        pub_key, priv_key = self.api.create_key_pair()
+
+        with open(pub_key_path, "w") as f:
+            f.write(pub_key)


We probably want to os.chmod these as 600 so that developers could use them on the cmdline to access instances using ssh -i lxd-pubkey ubuntu@<ip>

blackboxsw · 2020-11-24T23:14:43Z

features/cloud.py

+    @property
+    def api(self) -> pycloudlib.cloud.BaseCloud:
+        """Return the api used to interact with the cloud provider."""
+        if self._api is None:
+            self._api = pycloudlib.LXDVirtualMachine(tag=self.tag)
+
+        return self._api


We might be able to define this simply in either in _LXD base class or even Cloud if we use a class attr to define the pycloublib.*Cloud callable to create

index 38114a6..c2b8600 100644 --- a/features/cloud.py +++ b/features/cloud.py @@ -29,6 +29,7 @@ class Cloud: name = "" pro_ids_path = "" env_vars: "Tuple[str, ...]" = () + pycloudlib_cls = None # type: pycloudlib.cloud.BaseCloud def __init__( self, @@ -64,7 +65,13 @@ class Cloud: @property def api(self) -> pycloudlib.cloud.BaseCloud: """Return the api used to interact with the cloud provider.""" - raise NotImplementedError + if self.pycloudlib_cls is None: + raise NotImplementedError + if self._api is None: + self._api = self.pycloudlib_cls( + tag=self.tag, timestamp_suffix=self.timestamp_suffix + ) + return self.api def manage_ssh_key(self, private_key_path: "Optional[str]" = None) -> None: """Create and manage ssh key pairs to be used in the cloud provider. @@ -496,8 +503,6 @@ class Azure(Cloud): class _LXD(Cloud): - name = "_lxd" - def _create_instance( self, series: str, @@ -573,6 +578,7 @@ class _LXD(Cloud): class LXDVirtualMachine(_LXD): name = "lxd-virtual-machine" + pycloudlib_cls = pycloudlib.LXDVirtualMachine def manage_ssh_key(self, private_key_path: "Optional[str]" = None) -> None: """Create and manage ssh key pairs to be used in the cloud provider. @@ -606,6 +612,7 @@ class LXDVirtualMachine(_LXD): class LXDContainer(_LXD): name = "lxd-virtual-machine" + pycloudlib_cls = pycloudlib.LXDContainer def manage_ssh_key(self, private_key_path: "Optional[str]" = None) -> None: """Create and manage ssh key pairs to be used in the cloud provider.

blackboxsw · 2020-11-24T23:18:33Z

features/cloud.py

+    def api(self) -> pycloudlib.cloud.BaseCloud:
+        """Return the api used to interact with the cloud provider."""
+        if self._api is None:
+            self._api = pycloudlib.LXDContainer(tag=self.tag)


let's also not forget timestamp_suffix=self.timestamp_suffix

blackboxsw · 2020-11-24T23:18:51Z

features/cloud.py

+    def api(self) -> pycloudlib.cloud.BaseCloud:
+        """Return the api used to interact with the cloud provider."""
+        if self._api is None:
+            self._api = pycloudlib.LXDVirtualMachine(tag=self.tag)


don't forget timestamp_suffix=self.timestamp_suffix

blackboxsw · 2020-11-24T23:34:42Z

features/steps/steps.py


 from uaclient.defaults import DEFAULT_CONFIG_FILE


-CONTAINER_PREFIX = "behave-test-"
+CONTAINER_PREFIX = "ubuntu-behave-test-"


Why change this?

This is just to guarantee that the instance name will be properly parsed by this regex:
https://github.com/canonical/pycloudlib/blob/master/pycloudlib/lxd/cloud.py#L580

lucasmoura · 2020-11-25T22:00:25Z

@blackboxsw Thanks for the review and great catch on the leftover code for checking cloud_api.

I have updated the code and I think instead of worrying about shellify at the moment, we should wait for this pycloudlib PR to land. When that happens, we will no longer need the execute_command method anymore

Currently, our integration tests are using our own abstraction to handle LXD containers and vms. Since all other cloud we interact with are using pycloudlib, we have made the decision to apply that for LXD support as well.

The pin priority is not working for FIPS Updates because the preferences file we are creating has the wrong origin data. The product requires origin as "UbuntuFIPS" but we are using "UbuntuFIPSUpdates"

Add FIPS tests for xenial and upgrade test from xenial to bionic when FIPS is enabled in the system. However, FIPS Updates is not currently working when upgrading from xenial to bionic due to a problem on do-release-upgrade where we fail to access the right repo for FIPS Updates during the upgrade and end up disabling the ppa, causing some package, e.g. ubuntu-fips, to be removed.

blackboxsw

Much simpler to use the pycloudlib to rule all the platforms. Thanks for this work!

blackboxsw reviewed Nov 12, 2020

View reviewed changes

features/cloud.py Outdated Show resolved Hide resolved

lucasmoura force-pushed the use-lxd-support-from-pycloudlib branch 2 times, most recently from 4446fd6 to e24d7f2 Compare November 24, 2020 14:36

lucasmoura force-pushed the use-lxd-support-from-pycloudlib branch from e24d7f2 to b58b64a Compare November 24, 2020 18:44

lucasmoura requested a review from blackboxsw November 24, 2020 18:57

blackboxsw requested changes Nov 24, 2020

View reviewed changes

lucasmoura mentioned this pull request Nov 25, 2020

Update tests to run commands on LXD as non root canonical/cloud-init#694

Merged

lucasmoura force-pushed the use-lxd-support-from-pycloudlib branch from b58b64a to b091699 Compare November 25, 2020 21:58

lucasmoura force-pushed the use-lxd-support-from-pycloudlib branch 2 times, most recently from 4bfc023 to a0d7522 Compare November 27, 2020 18:00

lucasmoura added 3 commits November 27, 2020 15:49

Update tests to use new pycloudlib LXD abstraction

385dfc7

Currently, our integration tests are using our own abstraction to handle LXD containers and vms. Since all other cloud we interact with are using pycloudlib, we have made the decision to apply that for LXD support as well.

Update FIPSUpdates pin origin

1e1ba75

The pin priority is not working for FIPS Updates because the preferences file we are creating has the wrong origin data. The product requires origin as "UbuntuFIPS" but we are using "UbuntuFIPSUpdates"

lucasmoura force-pushed the use-lxd-support-from-pycloudlib branch from a0d7522 to 6440f29 Compare November 27, 2020 18:49

lucasmoura requested a review from blackboxsw November 27, 2020 19:35

blackboxsw approved these changes Dec 1, 2020

View reviewed changes

blackboxsw merged commit 4ba6698 into master Dec 1, 2020

lucasmoura deleted the use-lxd-support-from-pycloudlib branch December 1, 2020 13:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor integration tests to use pycloudlib LXD #1241

Refactor integration tests to use pycloudlib LXD #1241

lucasmoura commented Nov 11, 2020

blackboxsw Nov 12, 2020 •

edited

blackboxsw left a comment

lucasmoura commented Nov 12, 2020

lucasmoura commented Nov 24, 2020

blackboxsw left a comment

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

blackboxsw Nov 24, 2020

lucasmoura Nov 25, 2020

lucasmoura commented Nov 25, 2020

blackboxsw left a comment

	series, instance_name, image_name, user_data
	series=series, instance_name=instance_name, image_name=image_name, user_data=user_data

	origin = "UbuntuFIPS"
	origin = "UbuntuFIPS" # RELEASE_BLOCKER RT: #128770

Refactor integration tests to use pycloudlib LXD #1241

Refactor integration tests to use pycloudlib LXD #1241

Conversation

lucasmoura commented Nov 11, 2020

blackboxsw Nov 12, 2020 • edited

Choose a reason for hiding this comment

blackboxsw left a comment

Choose a reason for hiding this comment

lucasmoura commented Nov 12, 2020

lucasmoura commented Nov 24, 2020

blackboxsw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmoura commented Nov 25, 2020

blackboxsw left a comment

Choose a reason for hiding this comment

blackboxsw Nov 12, 2020 •

edited