Skip to content

Update Azure _unpickle (SC-500)#1067

Merged
blackboxsw merged 2 commits into
canonical:mainfrom
TheRealFalcon:azure-pickle
Oct 19, 2021
Merged

Update Azure _unpickle (SC-500)#1067
blackboxsw merged 2 commits into
canonical:mainfrom
TheRealFalcon:azure-pickle

Conversation

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Proposed Commit Message

Update Azure _unpickle

When self.failed_desired_api_version was added to DataSourceAzure, the
attribute was never added to the _unpickle method using the upgrade
framework. This commit adds the attribute.

LP: #1946644

Additional Context

Test Steps

Unfortunately, there's no easy way to test this via unit or integration test. Using a pre-existing pickle doesn't work because specific things like instance id or network interfaces will be different instance to instance and raise other issues.

To test manually:
Launch an Azure Bionic VM
On the instance, 'rm /lib/systemd/system/cloud-init-local.service.d/50-azure-clear-persistent-obj-pkl.conf'
Checkout cloud-init source at the 19.1 tag.
Build a package with the source (DEB_BUILD_OPTIONS=nocheck packages/bddeb -d)
Push the deb to the azure instance and install it
cloud-init clean --logs --reboot
Similarly, create deb of this branch and push and install
DON'T cloud-init clean
reboot, verify no failure due to attributes not found in Azure datasource.

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

When self.failed_desired_api_version was added to DataSourceAzure, the
attribute was never added to the _unpickle method using the upgrade
framework. This commit adds the attribute.

LP: #1946644
@TheRealFalcon TheRealFalcon changed the title Update Azure _unpickle Update Azure _unpickle (SC-500) Oct 13, 2021
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize we can't add the test/data/old_pickles option here for integration testing on Azure because we only support that integration test on NoCloud.

But, shouldn't this have been caught by our existing test_upgrade integration test](https://github.com/canonical/cloud-init/blob/main/tests/integration_tests/test_upgrade.py#L93-L97)? I'm thinking that the cloud-init init call should actually have been called before the inst.restart() a couple lines above. Because, cloud-init init deserializing won't fail after an instance restart on Azure because of the /lib/systemd/system/cloud-init-local.service.d/50-azure-clear-persistent-obj-pkl.conf clearing that pickled datasource during init-local timeframe of next boot.

@TheRealFalcon
Copy link
Copy Markdown
Contributor Author

@blackboxsw , yeah, you're definitely right...especially given that comment in the code stating such 🤦‍♂️ . I updated the test accordingly.

Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 Let's bring this home.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants