Add upgrade integration test by TheRealFalcon · Pull Request #693 · canonical/cloud-init

TheRealFalcon · 2020-11-25T15:50:53Z

Proposed Commit Message

Add upgrade integration test

Add an integration test that roughly mimics many of the manual cloud
SRU tests. Also refactored some of the image setup code to make it
easier to use in non-fixture code.

Additional Context

Test Steps

pytest tests/integration_tests/test_example.py

Checklist:

My code follows the process laid out in the documentation
I have updated or added any unit tests accordingly
I have updated or added any documentation accordingly

Add an integration test that roughly mimics many of the manual cloud SRU tests. Also refactored some of the image setup code to make it easier to use in non-fixture code.

OddBloke · 2020-12-02T19:42:44Z

+        print('executing: {}'.format(command))
+        print(instance.execute(command))


Printing these isn't ideal; one has to pass -s to pytest to get the output, and even then it's interleaved with log output:

executing: cloud-id INFO pycloudlib.instance:instance.py:166 executing: sh -c cloud-id lxd

Perhaps write the full output to a before and after file (and log the names used)?

OddBloke · 2020-12-02T19:43:50Z

+        'systemd-analyze blame',
+        'cloud-init analyze show',
+        'cloud-init analyze blame',
+        'cat $NETCFG_FILE',


$NETCFG_FILE isn't set here, we should substitute the appropriate value in.

OddBloke · 2020-12-02T19:58:29Z

+        'hostname',
+        'dpkg-query --show cloud-init',
+        'cat /run/cloud-init/result.json',
+        '! grep Trace /var/log/cloud-init.log',


As we aren't checking the exit codes here, we can drop the ! prefix.

OddBloke · 2020-12-02T19:59:31Z

+        'cloud-id'
+    ]
+    for command in commands:
+        print('executing: {}'.format(command))


"==== {} ====".format(command) or similar would make these easier to pick out.

TheRealFalcon · 2020-12-03T17:08:41Z

I pushed an update to the refactor, but haven't updated the actual test yet. I realize I hurriedly put together that test and pushed it to have something out before I left for the holiday, but I didn't really consider the test itself review-ready yet. Sorry about that!

OddBloke

The rerefactor looks good to me now, just some nits, thanks!

TheRealFalcon · 2020-12-03T23:16:46Z

I think this is ready for a full review. We still just log the things we were logging manually before. Eventually it'd be nice to make this test a little "smarter".

TheRealFalcon · 2020-12-03T23:17:30Z

+            f.write(instance.execute(command) + '\n')
+
+
+@pytest.mark.sru_2020_11


Is this mark appropriate here? Not sure if we're marking all tests we're including in an SRU, or only marking tests that are only relevant to that SRU.

I think this is good. We may have to at some point add -m "not sru_2020_11" for our typical ci runs when we want to exclude them. Our final SRU pass run can just run -m sru_2020_11 to assert all tests have be validated. We could kick this off as a separate jenkins job at some point to get a passing jenkins log as reference for success. I think applying that mark to all sru manual tests is appropriate.

blackboxsw · 2020-12-04T23:13:55Z

+def test_upgrade(session_cloud: IntegrationCloud):
+    source = get_validated_source()
+    if not source.installs_new_version():
+        pytest.skip("Current install method not supported for this test")


Let's parameterize the {source.value} in this message to tell us specifically what didn't match

Skip text isn't actually printed anywhere. There's probably a way to make it show somewhere, but we haven't done it 😂

The message printed for me as I run pytest -rxXs.

blackboxsw

Looks really good @TheRealFalcon a couple of requests and I think we are good. I'd like to actually see some asserts on hostname using sample cloud-config somehow to make sure plumbing is working across reboot and also check cloud-init init for Tracebacks prior to a reboot as this is something that cached datasources would break across upgrade in a few previous SRUs

blackboxsw · 2020-12-05T04:34:47Z

+            f.write(instance.execute(command) + '\n')
+
+
+@pytest.mark.sru_2020_11


I think this is good. We may have to at some point add -m "not sru_2020_11" for our typical ci runs when we want to exclude them. Our final SRU pass run can just run -m sru_2020_11 to assert all tests have be validated. We could kick this off as a separate jenkins job at some point to get a passing jenkins log as reference for success. I think applying that mark to all sru manual tests is appropriate.

blackboxsw · 2020-12-05T04:36:07Z

+def test_upgrade(session_cloud: IntegrationCloud):
+    source = get_validated_source()
+    if not source.installs_new_version():
+        pytest.skip("Current install method not supported for this test")


The message printed for me as I run pytest -rxXs.

blackboxsw · 2020-12-05T05:22:21Z

+    with session_cloud.launch(launch_kwargs=launch_kwargs) as instance:
+        _output_to_compare(instance, before_path, netcfg_path)
+        instance.install_new_cloud_init(source, take_snapshot=False)
+        instance.instance.restart()


I think we need to fail on seeing tracebacks after reset. Also I think we need to check cloud-init init and no Tracebacks as we do in ec2 manual sru test

I also think we should try to inject some userdata that exercises parts of cloud-init on the upgrade test to ensure that various pieces are working

We could use and validate sample jinja template as we did in manual test runs to assert that hostname returned actually contains the right cloud-id from instance data

So if we can use

Suggested change

instance.instance.restart()

instance.instance.execute('cloud-init init')

tracebacks = instance.instance.execute('grep Traceback /var/log/cloud-init.log')

assert tracebacks == ''

instance.restart(raise_on_cloudinit_failure=True) # Make sure we fail this test if cloud-init fails on next clean boot

I think we need to fail on seeing tracebacks after reset. Also I think we need to check cloud-init init and no Tracebacks as we do in ec2 manual sru test

Would instance.restart(raise_on_cloudinit_failure=True) accomplish this? Why would cloud-init init need to run again if it has already run on boot?

@TheRealFalcon, some individuals who use cloud-init on a running system and don't want a reboot attempt to upgrade cloud-init and re start cloud-init services via systemctl this particular use-case, while not recommended, has raised a number of upgrade issues in the past with cloud-init and pickled DS cache handling across that upgrade path. So it's helpful for us to spot check this as it can be valuable in ascertaining if some of our pickling upgrade paths have broken across re-constitution of that cached datasource. If we do this only after reboot, some platforms invalidate their datasource cache forcing a clean run instead of a dirty run of cloud-init. A fully clean run won't raise such pickle deserializing errors.

blackboxsw

+1 thanks a lot for this @TheRealFalcon

Add upgrade integration test

397718f

Add an integration test that roughly mimics many of the manual cloud SRU tests. Also refactored some of the image setup code to make it easier to use in non-fixture code.

TheRealFalcon force-pushed the test-upgrade branch from d788750 to 397718f Compare November 25, 2020 16:24

OddBloke reviewed Dec 2, 2020

View reviewed changes

Comment thread tests/integration_tests/instances.py Outdated

OddBloke reviewed Dec 2, 2020

View reviewed changes

[squash] Refactor the refactor

6af7ab6

OddBloke reviewed Dec 3, 2020

View reviewed changes

Comment thread tests/integration_tests/instances.py

Comment thread tests/integration_tests/conftest.py Outdated

Comment thread tests/integration_tests/test_upgrade.py Outdated

Comment thread tests/integration_tests/instances.py

TheRealFalcon added 4 commits December 3, 2020 12:58

Merge branch 'master' into test-upgrade

1fafa51

[squash] added extra log message during merge

6d03fa0

[squash] review comments for the refactor of the refactor

8ca3ddc

[squash] Update upgrade test based on review

297fd3f

TheRealFalcon commented Dec 3, 2020

View reviewed changes

TheRealFalcon and others added 2 commits December 3, 2020 17:19

Merge branch 'master' into test-upgrade

95a87e4

Merge branch 'master' into test-upgrade

26a73e1

blackboxsw reviewed Dec 4, 2020

View reviewed changes

Comment thread tests/integration_tests/instances.py

blackboxsw reviewed Dec 4, 2020

View reviewed changes

blackboxsw requested changes Dec 5, 2020

View reviewed changes

TheRealFalcon added 2 commits December 7, 2020 08:22

[squash] Adding hostname userdata and context to the skip message

84c2341

[squash] Added a cloud-init init call

1f0e6e3

blackboxsw approved these changes Dec 7, 2020

View reviewed changes

blackboxsw merged commit 54e202a into canonical:master Dec 7, 2020

TheRealFalcon deleted the test-upgrade branch June 29, 2021 18:59

This was referenced May 12, 2023

Release 21.1 #3846

Closed

Randomly set credentials written in cleartext to world-readable file #3854

Closed

		print('executing: {}'.format(command))
		print(instance.execute(command))

		f.write(instance.execute(command) + '\n')


		@pytest.mark.sru_2020_11

-        instance.instance.restart()
+        instance.instance.execute('cloud-init init')
+        tracebacks = instance.instance.execute('grep Traceback /var/log/cloud-init.log')
+        assert tracebacks == ''
+        instance.restart(raise_on_cloudinit_failure=True)   # Make sure we fail this test if cloud-init fails on next clean boot

Conversation

TheRealFalcon commented Nov 25, 2020

Proposed Commit Message

Additional Context

Test Steps

Checklist:

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheRealFalcon commented Dec 3, 2020

Uh oh!

OddBloke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheRealFalcon commented Dec 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blackboxsw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blackboxsw left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants