Skip to content

Commit

Permalink
fix error on upgrade caused by new vendordata2 attributes (#869)
Browse files Browse the repository at this point in the history
In #777, we added 'vendordata2' and 'vendordata2_raw' attributes to
the DataSource class, but didn't use the upgrade framework to deal
with an unpickle after upgrade. This commit adds the necessary
upgrade code.

Additionally, added a smaller-scope upgrade test to our integration
tests that will be run on every CI run so we catch these issues
immediately in the future.

LP: #1922739
  • Loading branch information
TheRealFalcon committed Apr 19, 2021
1 parent 45db197 commit d132356
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 4 deletions.
12 changes: 11 additions & 1 deletion cloudinit/sources/__init__.py
Expand Up @@ -24,6 +24,7 @@
from cloudinit.atomic_helper import write_json
from cloudinit.event import EventType
from cloudinit.filters import launch_index
from cloudinit.persistence import CloudInitPickleMixin
from cloudinit.reporting import events

DSMODE_DISABLED = "disabled"
Expand Down Expand Up @@ -134,7 +135,7 @@ def redact_sensitive_keys(metadata, redact_value=REDACT_SENSITIVE_VALUE):
'URLParms', ['max_wait_seconds', 'timeout_seconds', 'num_retries'])


class DataSource(metaclass=abc.ABCMeta):
class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta):

dsmode = DSMODE_NETWORK
default_locale = 'en_US.UTF-8'
Expand Down Expand Up @@ -196,6 +197,8 @@ class DataSource(metaclass=abc.ABCMeta):
# non-root users
sensitive_metadata_keys = ('merged_cfg', 'security-credentials',)

_ci_pkl_version = 1

def __init__(self, sys_cfg, distro, paths, ud_proc=None):
self.sys_cfg = sys_cfg
self.distro = distro
Expand All @@ -218,6 +221,13 @@ def __init__(self, sys_cfg, distro, paths, ud_proc=None):
else:
self.ud_proc = ud_proc

def _unpickle(self, ci_pkl_version: int) -> None:
"""Perform deserialization fixes for Paths."""
if not hasattr(self, 'vendordata2'):
self.vendordata2 = None
if not hasattr(self, 'vendordata2_raw'):
self.vendordata2_raw = None

def __str__(self):
return type_utils.obj_name(self)

Expand Down
4 changes: 4 additions & 0 deletions cloudinit/tests/test_upgrade.py
Expand Up @@ -46,3 +46,7 @@ def test_blacklist_drivers_set_on_networking(self, previous_obj_pkl):

def test_paths_has_run_dir_attribute(self, previous_obj_pkl):
assert previous_obj_pkl.paths.run_dir is not None

def test_vendordata_exists(self, previous_obj_pkl):
assert previous_obj_pkl.vendordata2 is None
assert previous_obj_pkl.vendordata2_raw is None
4 changes: 2 additions & 2 deletions tests/integration_tests/clouds.py
Expand Up @@ -111,14 +111,14 @@ def __init__(self, settings=integration_settings):
# Even if we're using the default key, it may still have a
# different name in the clouds, so we need to set it separately.
self.cloud_instance.key_pair.name = settings.KEYPAIR_NAME
self._released_image_id = self._get_initial_image()
self.released_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._released_image_id
return self.released_image_id

def emit_settings_to_log(self) -> None:
log.info(
Expand Down
25 changes: 24 additions & 1 deletion tests/integration_tests/test_upgrade.py
@@ -1,4 +1,5 @@
import logging
import os
import pytest
import time
from pathlib import Path
Expand All @@ -8,6 +9,8 @@
get_validated_source,
session_start_time,
)
from tests.integration_tests.instances import CloudInitSource


log = logging.getLogger('integration_testing')

Expand Down Expand Up @@ -63,7 +66,7 @@ def test_upgrade(session_cloud: IntegrationCloud):
return # type checking doesn't understand that skip raises

launch_kwargs = {
'image_id': session_cloud._get_initial_image(),
'image_id': session_cloud.released_image_id,
}

image = ImageSpecification.from_os_image()
Expand Down Expand Up @@ -93,6 +96,26 @@ def test_upgrade(session_cloud: IntegrationCloud):
instance.install_new_cloud_init(source, take_snapshot=False)
instance.execute('hostname something-else')
_restart(instance)
assert instance.execute('cloud-init status --wait --long').ok
_output_to_compare(instance, after_path, netcfg_path)

log.info('Wrote upgrade test logs to %s and %s', before_path, after_path)


@pytest.mark.ci
@pytest.mark.ubuntu
def test_upgrade_package(session_cloud: IntegrationCloud):
if get_validated_source(session_cloud) != CloudInitSource.DEB_PACKAGE:
not_run_message = 'Test only supports upgrading to build deb'
if os.environ.get('TRAVIS'):
# If this isn't running on CI, we should know
pytest.fail(not_run_message)
else:
pytest.skip(not_run_message)

launch_kwargs = {'image_id': session_cloud.released_image_id}

with session_cloud.launch(launch_kwargs=launch_kwargs) as instance:
instance.install_deb()
instance.restart()
assert instance.execute('cloud-init status --wait --long').ok

0 comments on commit d132356

Please sign in to comment.