Skip to content

Commit

Permalink
bug(schema): only write network-config if instance dir present (canon…
Browse files Browse the repository at this point in the history
…ical#4635)

Only write /var/lib/cloud/instance/network-config.json once
datasource is detected. The /var/lib/clound/instance symlink is
created by Init.instancify after datasource detection is complete.

Migrate creation of /var/lib/cloud/instance/network-config.json out
of apply_network_config and into instancify, after the datasource
is detected.

This fixes a bug by ensuring /var/lib/cloud/instance exists before
persisting /v/l/cloud/instance/network-config.json.

Fixes canonicalGH-4630
  • Loading branch information
blackboxsw committed Nov 29, 2023
1 parent 05f039c commit 15a38b8
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 42 deletions.
56 changes: 30 additions & 26 deletions cloudinit/stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def __init__(self, ds_deps: Optional[List[str]] = None, reporter=None):
self._cfg: Optional[dict] = None
self._paths: Optional[helpers.Paths] = None
self._distro: Optional[distros.Distro] = None
self._netcfg: Optional[dict] = None
# Changed only when a fetch occurs
self.datasource: Optional[sources.DataSource] = None
self.ds_restored = False
Expand Down Expand Up @@ -403,6 +404,24 @@ def _reflect_cur_instance(self):

# Write out information on what is being used for the current instance
# and what may have been used for a previous instance...
if getattr(self, "_netcfg"):
ncfg_instance_path = self.paths.get_ipath_cur("network_config")
network_link = self.paths.get_runpath("network_config")
# Persist known network-config.json
if os.path.exists(ncfg_instance_path):
# Compare write on delta of current network-config
if self._netcfg != util.load_json(
util.load_file(ncfg_instance_path)
):
atomic_helper.write_json(
ncfg_instance_path, self._netcfg, mode=0o600
)
else:
atomic_helper.write_json(
ncfg_instance_path, self._netcfg, mode=0o600
)
if not os.path.islink(network_link):
util.sym_link(ncfg_instance_path, network_link)
dp = self.paths.get_cpath("data")

# Write what the datasource was and is..
Expand Down Expand Up @@ -934,22 +953,7 @@ def _find_networking_config(self):
)

def _apply_netcfg_names(self, netcfg):
ncfg_instance_path = self.paths.get_ipath_cur("network_config")
network_link = self.paths.get_runpath("network_config")
if not self._network_already_configured():
if os.path.exists(ncfg_instance_path):
if netcfg != util.load_json(
util.load_file(ncfg_instance_path)
):
atomic_helper.write_json(
ncfg_instance_path, netcfg, mode=0o600
)
else:
atomic_helper.write_json(
ncfg_instance_path, netcfg, mode=0o600
)
if not os.path.islink(network_link):
util.sym_link(ncfg_instance_path, network_link)
self._netcfg = netcfg
try:
LOG.debug("applying net config names for %s", netcfg)
self.distro.networking.apply_network_config_names(netcfg)
Expand All @@ -974,8 +978,8 @@ def apply_network_config(self, bring_up):
"""
from cloudinit.config.schema import validate_cloudconfig_schema

netcfg, src = self._find_networking_config()
if netcfg is None:
self._netcfg, src = self._find_networking_config()
if self._netcfg is None:
LOG.info("network config is disabled by %s", src)
return

Expand Down Expand Up @@ -1004,40 +1008,40 @@ def should_run_on_boot_event():
" nor datasource network update allowed"
)
# nothing new, but ensure proper names
self._apply_netcfg_names(netcfg)
self._apply_netcfg_names(self._netcfg)
return

# refresh netcfg after update
netcfg, src = self._find_networking_config()
self._netcfg, src = self._find_networking_config()

if netcfg and netcfg.get("version") == 1:
if self._netcfg and self._netcfg.get("version") == 1:
validate_cloudconfig_schema(
config=netcfg,
config=self._netcfg,
schema_type="network-config",
strict=False,
log_details=True,
log_deprecations=True,
)

# ensure all physical devices in config are present
self.distro.networking.wait_for_physdevs(netcfg)
self.distro.networking.wait_for_physdevs(self._netcfg)

# apply renames from config
self._apply_netcfg_names(netcfg)
self._apply_netcfg_names(self._netcfg)

# rendering config
LOG.info(
"Applying network configuration from %s bringup=%s: %s",
src,
bring_up,
netcfg,
self._netcfg,
)

sem = self._get_per_boot_network_semaphore()
try:
with sem.semaphore.lock(*sem.args):
return self.distro.apply_network_config(
netcfg, bring_up=bring_up
self._netcfg, bring_up=bring_up
)
except net.RendererNotFoundError as e:
LOG.error(
Expand Down
64 changes: 64 additions & 0 deletions tests/integration_tests/datasources/test_none.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""NoCloud datasource integration tests."""
import json

import pytest
from pycloudlib.lxd.instance import LXDInstance

from tests.integration_tests.decorators import retry
from tests.integration_tests.instances import IntegrationInstance
from tests.integration_tests.integration_settings import PLATFORM
from tests.integration_tests.util import verify_clean_log

DS_NONE_BASE_CFG = """\
datasource_list: [None]
datasource:
None:
metadata:
instance-id: my-iid-uuid
userdata_raw: |
#cloud-config
runcmd:
- touch /var/tmp/success-with-datasource-none
"""

LXD_METADATA_DS_NONE_BASE_CFG = """\
/etc/cloud/cloud.cfg.d/99-force-dsnone.cfg:
when:
- create
- copy
create_only: false
template: dsnone.tpl
properties: {}
"""


@retry(tries=30, delay=1)
def wait_for_cloud_init_status_file(instance: LXDInstance):
status_file = instance.read_from_file("/run/cloud-init/status.json")
assert len(status_file)


@pytest.mark.lxd_use_exec
@pytest.mark.skipif(
PLATFORM != "lxd_container",
reason="Requires LXD container with custom metadata setup",
)
def test_datasource_none_discovery(client: IntegrationInstance):
"""Integration test for #4635.
Test that DataSourceNone detection (used by live installers) doesn't
generate errors or warnings.
"""
log = client.read_from_file("/var/log/cloud-init.log")
verify_clean_log(log)
# Limit datasource detection to DataSourceNone.
client.write_to_file(
"/etc/cloud/cloud.cfg.d/99-force-dsnone.cfg", DS_NONE_BASE_CFG
)
client.execute("cloud-init clean --logs --reboot")
wait_for_cloud_init_status_file(client)
status = json.loads(client.execute("cloud-init status --format=json"))
assert [] == status["errors"]
assert {} == status["recoverable_errors"]
client.execute("cloud-init status --wait")
assert client.execute("test -f /var/tmp/success-with-datasource-none").ok
5 changes: 4 additions & 1 deletion tests/unittests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import responses

import cloudinit
from cloudinit import cloud, distros
from cloudinit import atomic_helper, cloud, distros
from cloudinit import helpers as ch
from cloudinit import subp, util
from cloudinit.config.schema import (
Expand Down Expand Up @@ -293,6 +293,9 @@ def patchUtils(self, new_root):
("sym_link", -1),
("copy", -1),
],
atomic_helper: [
("write_json", 1),
],
}
for (mod, funcs) in patch_funcs.items():
for (f, am) in funcs:
Expand Down
24 changes: 23 additions & 1 deletion tests/unittests/runs/test_simple_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
import copy
import os

from cloudinit import safeyaml, stages, util
from cloudinit import atomic_helper, safeyaml, stages, util
from cloudinit.config.modules import Modules
from cloudinit.settings import PER_INSTANCE
from cloudinit.sources import NetworkConfigSource
from tests.unittests import helpers


Expand Down Expand Up @@ -47,6 +48,15 @@ def setUp(self):
def test_none_ds_populates_var_lib_cloud(self):
"""Init and run_section default behavior creates appropriate dirs."""
# Now start verifying whats created
netcfg = {
"version": 1,
"config": [{"type": "physical", "name": "eth9"}],
}

def fake_network_config():
return netcfg, NetworkConfigSource.FALLBACK

self.assertFalse(os.path.exists("/var/lib/cloud"))
initer = stages.Init()
initer.read_cfg()
initer.initialize()
Expand All @@ -55,10 +65,22 @@ def test_none_ds_populates_var_lib_cloud(self):
self.assertTrue(os.path.isdir(os.path.join("/var/lib/cloud", d)))

initer.fetch()
self.assertFalse(os.path.islink("var/lib/cloud/instance"))
iid = initer.instancify()
self.assertEqual(iid, "iid-datasource-none")
initer.update()
self.assertTrue(os.path.islink("var/lib/cloud/instance"))
self.assertIsNone(initer._netcfg)
initer._find_networking_config = fake_network_config
initer.apply_network_config(False)
self.assertFalse(
os.path.exists("/var/lib/cloud/instance/network-config.json")
)
initer.instancify()
self.assertEqual(
f"{atomic_helper.json_dumps(netcfg)}\n",
util.load_file("/var/lib/cloud/instance/network-config.json"),
)

def test_none_ds_runs_modules_which_do_not_define_distros(self):
"""Any modules which do not define a distros attribute are run."""
Expand Down
22 changes: 8 additions & 14 deletions tests/unittests/test_stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

import pytest

from cloudinit import sources, stages
from cloudinit import atomic_helper, sources, stages
from cloudinit.event import EventScope, EventType
from cloudinit.sources import NetworkConfigSource
from cloudinit.util import write_file
from cloudinit.util import sym_link, write_file
from tests.unittests.helpers import mock
from tests.unittests.util import TEST_INSTANCE_ID, FakeDataSource

Expand All @@ -28,7 +28,8 @@ def setup(self, tmpdir):
"paths": {"cloud_dir": self.tmpdir, "run_dir": self.tmpdir},
}
}
tmpdir.mkdir("instance")
tmpdir.mkdir("instance-uuid")
sym_link(tmpdir.join("instance-uuid"), tmpdir.join("instance"))
self.init.datasource = FakeDataSource(paths=self.init.paths)
self._real_is_new_instance = self.init.is_new_instance
self.init.is_new_instance = mock.Mock(return_value=True)
Expand Down Expand Up @@ -392,6 +393,7 @@ def fake_network_config():
self.init.apply_network_config(True)
assert caplog.records[0].levelname == "INFO"
assert f"network config is disabled by {disable_file}" in caplog.text
assert None is self.init._netcfg

@mock.patch("cloudinit.net.get_interfaces_by_mac")
@mock.patch("cloudinit.distros.ubuntu.Distro")
Expand All @@ -415,20 +417,12 @@ def fake_network_config():
m_macs.return_value = {"42:42:42:42:42:42": "eth9"}

self.init._find_networking_config = fake_network_config

self.init.apply_network_config(True)
networking = self.init.distro.networking
networking.apply_network_config_names.assert_called_with(net_cfg)
self.init.distro.apply_network_config.assert_called_with(
net_cfg, bring_up=True
)
assert net_cfg == json.loads(
self.tmpdir.join("instance/network-config.json").read()
)
assert net_cfg == json.loads(
self.tmpdir.join("network-config.json").read()
)
assert os.path.islink(self.tmpdir.join("network-config.json"))

@mock.patch("cloudinit.distros.ubuntu.Distro")
def test_apply_network_on_same_instance_id(self, m_ubuntu, caplog):
Expand Down Expand Up @@ -529,9 +523,9 @@ def test_apply_network_disabled_when_no_default_boot(
net_cfg = self._apply_network_setup(m_macs)
self.init.apply_network_config(True)
self.init.distro.apply_network_config.assert_not_called()
assert net_cfg == json.loads(
self.tmpdir.join("network-config.json").read()
)
# Preserve the _netcfg presented on Init instance
# Used by Init.instancify to create network-config.json
assert self.init._netcfg == net_cfg
assert (
"No network config applied. Neither a new instance nor datasource "
"network update allowed" in caplog.text
Expand Down

0 comments on commit 15a38b8

Please sign in to comment.