Merge ~jasonzio/cloud-init:handleUnsetNetCfg into cloud-init:master

Proposed by Jason Zions
Status: Merged
Approved by: Ryan Harper
Approved revision: e0463108ae336485771ea5527a0f6aa3d691b7ba
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~jasonzio/cloud-init:handleUnsetNetCfg
Merge into: cloud-init:master
Diff against target: 13 lines (+1/-1)
1 file modified
cloudinit/sources/DataSourceAzure.py (+1/-1)
Reviewer Review Type Date Requested Status
Ryan Harper Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+365377@code.launchpad.net

Commit message

Azure: Treat _unset network configuration as if it were absent

When the Azure datasource persists all of its metadata to the
instance directory, it deliberately sets the self.network_config
value to be the sources.UNSET value. The goal is to ensure that
each time the system boots, fresh network configuration data is
fetched from the cloud platform so that any control plane changes
will take effect. When a VM is first created, there's no pickled
instance to restore, so self._network_config is None, resulting
in self.network_config() properly building a new config. Azure
suffered from LP: #1801364 which prevented ds from being stored
in obj.pkl in the instance directory, so subsequent reboots always
regenerated their network configuration.

Commit 0dc3a77f41f4544e4cb5a41637af7693410d4cdf introduced a
new bug in which self.network_config() assumed the
self._network_config value was either None or trustable; when
the config was unpickled, that value was _unset, thus breaking
the assumption.

LP: #1823084

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:e0463108ae336485771ea5527a0f6aa3d691b7ba
https://jenkins.ubuntu.com/server/job/cloud-init-ci/659/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/659/rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Hi Jason,

Which bug are you referring to in your commit message? And did you file a new bug for this issue yet?

I'd like to capture a cloud-init collect-logs with details so we can ensure we verify the scenario going forward.

review: Needs Information
Revision history for this message
Jason Zions (jasonzio) wrote :

> Which bug are you referring to in your commit message? And did you file a new
> bug for this issue yet?

Clearly I've been slacking on the filing of bugs before pushing commits, and I'll address that immediately.

The "previous bug" to which I referred was fixed by commit 0dc3a77f41 (https://code.launchpad.net/~jasonzio/cloud-init/+git/cloud-init/+merge/365065), a specific fix to the more-general bug 1801364.

Revision history for this message
Ryan Harper (raharper) wrote :

I didn't realize the serialization prevented cloudinit from restoring a ds from the obj.pkl.
Looking at the bug you reference, I can see why; it would have been good to have a bug for the Azure one as well so we could track that.

I see smoser has made the connection in the OpenStack version.

I've added a trello card for the general case:

https://trello.com/c/fgHtutXt

Revision history for this message
Jason Zions (jasonzio) wrote :

> I didn't realize the serialization prevented cloudinit from restoring a ds
> from the obj.pkl.

IIRC the serialization problem prevented cloudinit from *saving* the ds. The "write the ds" code tried to serialize and deserialize the ds before adding it to the larger object to be pickled; since the serialize raised an exception, the function just dropped the ds on the floor and it never wound up in the obj.pkl.

> Looking at the bug you reference, I can see why; it would have been good to
> have a bug for the Azure one as well so we could track that.

I'm writing up the bug now (for the _unset network_config problem), and I'll attach collect-logs output. Your request for "bug first, then fix" is totally legit, and I was just being lazy, and you called me on it.

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for filing the bug and the logs. In the logs I can see how we return the string _unset and then attempt to apply _that_ as the network config which of course it is not.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
2index b4e3f06..1873bc1 100644
3--- a/cloudinit/sources/DataSourceAzure.py
4+++ b/cloudinit/sources/DataSourceAzure.py
5@@ -668,7 +668,7 @@ class DataSourceAzure(sources.DataSource):
6 2. Generate a fallback network config that does not include any of
7 the blacklisted devices.
8 """
9- if not self._network_config:
10+ if not self._network_config or self._network_config == sources.UNSET:
11 if self.ds_cfg.get('apply_network_config'):
12 nc_src = self._metadata_imds
13 else:

Subscribers

People subscribed via source and target branches