upcloud, digitalocean: recompute network_config when set to UNSET - #7072
sundeep8967 wants to merge 2 commits into
Conversation
| UC_METADATA.get("network").get("dns")[1] == dns.get("address")[1] | ||
| ) | ||
|
|
||
| # GH-7067: Verify recomputation when _network_config is UNSET |
There was a problem hiding this comment.
We probably want a distinct unittest for this. It's not as discoverable when added to the middle of an existing test.
blackboxsw
left a comment
There was a problem hiding this comment.
Please separate the unittest for this in test_upcloud.py.
As mentioned on #7067 it may be worth inclusion of the same sort of fix in DataSourceDigitalOcean.py, and explicit unittest for the same such condition.
7fd32ef to
d9392de
Compare
|
Thanks for the review @blackboxsw! Updated the implementation per your feedback:
|
blackboxsw
left a comment
There was a problem hiding this comment.
Good changeset and thank you for the unittest coverage @sundeep8967.
One minor nit on the import and we can land this.
|
|
||
| from cloudinit import settings | ||
| from cloudinit import settings, sources | ||
| from cloudinit.sources import DataSourceDigitalOcean |
There was a problem hiding this comment.
We are already importing DataSourceDigitalOcean from sources, lets just direct import UNSET here instead of the separate imprt line
There was a problem hiding this comment.
Updated, thanks
d9392de to
c03cce2
Compare
When update_metadata_if_supported() is invoked on boot, it clears cached
network configuration by setting _network_config to UNSET ("_unset").
Because DataSourceUpCloud.network_config checked truthiness directly,
it returned the UNSET string sentinel instead of recomputing from
metadata, causing an AttributeError in apply_network_config.
Fixes canonicalGH-7067
Signed-off-by: sundeep8967 <sundeep8967@gmail.com>
The network_config property in DataSourceUpCloud and DataSourceDigitalOcean used a bare truthiness check on _network_config. When update_metadata_if_supported() sets _network_config = UNSET (the truthy string "_unset"), the sentinel is returned directly instead of recomputing network configuration, causing AttributeError: 'str' object has no attribute 'get' in apply_network_config on subsequent boots. Fix DataSourceDigitalOcean.network_config to mirror the existing pattern in DataSourceUpCloud by adding an explicit sources.UNSET sentinel check. Separate the regression test for UpCloud into its own dedicated test method (test_network_config_unset_recomputes) and add an equivalent test for DataSourceDigitalOcean. Fixes canonicalGH-7067 Signed-off-by: sundeep8967 <sundeep8967@gmail.com>
c03cce2 to
14ee8e1
Compare
Problem
When
updates.network.when: [boot]is configured,cloud-initcallsupdate_metadata_if_supported()on boot, which clears cached network configuration by settingself._network_config = UNSET("_unset").In
DataSourceUpCloud.network_configandDataSourceDigitalOcean.network_config, the check was a bare truthiness check (if self._network_config:). Because"_unset"is a non-empty string and evaluates toTrue, the methods returned the sentinel string directly instead of recomputing network configuration from metadata. On subsequent boots, this causedapply_network_configto crash with:Solution
DataSourceUpCloud.network_configto verifyif self._network_config and self._network_config != sources.UNSET:, matching the pattern used inDataSourceHetzner.DataSourceDigitalOcean.network_configto checkself._network_config != sources.UNSETbefore returning cached network configuration.tests/unittests/sources/test_upcloud.pyinto its own dedicated test method (test_network_config_unset_recomputes).tests/unittests/sources/test_digitalocean.py(test_network_config_unset_recomputes) verifying thatnetwork_configrecomputes when_network_configissources.UNSET.Fixes #7067
Testing
pytest tests/unittests/sources/test_upcloud.py tests/unittests/sources/test_digitalocean.py