Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect runtime file locations #4820

Merged
merged 5 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cloudinit/cmd/devel/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from typing import NamedTuple

from cloudinit.cmd.devel import read_cfg_paths
from cloudinit.helpers import Paths
from cloudinit.stages import Init
from cloudinit.subp import ProcessExecutionError, subp
from cloudinit.temp_utils import tempdir
Expand All @@ -28,7 +27,8 @@
write_file,
)

CLOUDINIT_RUN_DIR = "/run/cloud-init"
PATHS = read_cfg_paths()
holmanb marked this conversation as resolved.
Show resolved Hide resolved
CLOUDINIT_RUN_DIR = PATHS.run_dir
blackboxsw marked this conversation as resolved.
Show resolved Hide resolved


class ApportFile(NamedTuple):
Expand Down Expand Up @@ -144,7 +144,7 @@ def _copytree_rundir_ignore_files(curdir, files):
]
if os.getuid() != 0:
# Ignore root-permissioned files
ignored_files.append(Paths({}).lookups["instance_data_sensitive"])
ignored_files.append(PATHS.lookups["instance_data_sensitive"])
return ignored_files


Expand Down
5 changes: 3 additions & 2 deletions cloudinit/cmd/devel/render.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
)

NAME = "render"
CLOUDINIT_RUN_DIR = read_cfg_paths().run_dir

LOG = logging.getLogger(__name__)

Expand All @@ -40,8 +41,8 @@ def get_parser(parser=None):
"--instance-data",
type=str,
help=(
"Optional path to instance-data.json file. Defaults to"
" /run/cloud-init/instance-data.json"
"Optional path to instance-data.json file. "
f"Defaults to {CLOUDINIT_RUN_DIR}"
),
)
parser.add_argument(
Expand Down
10 changes: 4 additions & 6 deletions cloudinit/cmd/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,12 +697,10 @@ def main_single(name, args):
return 0


def status_wrapper(name, args, data_d=None, link_d=None):
if data_d is None:
paths = read_cfg_paths()
data_d = paths.get_cpath("data")
if link_d is None:
link_d = os.path.normpath("/run/cloud-init")
holmanb marked this conversation as resolved.
Show resolved Hide resolved
def status_wrapper(name, args):
paths = read_cfg_paths()
data_d = paths.get_cpath("data")
link_d = os.path.normpath(paths.run_dir)

status_path = os.path.join(data_d, "status.json")
status_link = os.path.join(link_d, "status.json")
Expand Down
4 changes: 2 additions & 2 deletions cloudinit/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from io import StringIO
from time import time

from cloudinit import persistence, type_utils, util
from cloudinit import persistence, settings, type_utils, util
from cloudinit.settings import CFG_ENV_NAME, PER_ALWAYS, PER_INSTANCE, PER_ONCE

LOG = logging.getLogger(__name__)
Expand Down Expand Up @@ -307,7 +307,7 @@ def __init__(self, path_cfgs: dict, ds=None):
self.cfgs = path_cfgs
# Populate all the initial paths
self.cloud_dir: str = path_cfgs.get("cloud_dir", "/var/lib/cloud")
self.run_dir: str = path_cfgs.get("run_dir", "/run/cloud-init")
self.run_dir: str = path_cfgs.get("run_dir", settings.DEFAULT_RUN_DIR)
self.instance_link: str = os.path.join(self.cloud_dir, "instance")
self.boot_finished: str = os.path.join(
self.instance_link, "boot-finished"
Expand Down
2 changes: 1 addition & 1 deletion cloudinit/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

CLEAN_RUNPARTS_DIR = "/etc/cloud/clean.d"

RUN_CLOUD_CONFIG = "/run/cloud-init/cloud.cfg"
DEFAULT_RUN_DIR = "/run/cloud-init"

# What u get if no config is provided
CFG_BUILTIN = {
Expand Down
38 changes: 31 additions & 7 deletions cloudinit/stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@
from cloudinit.reporting import events
from cloudinit.settings import (
CLOUD_CONFIG,
DEFAULT_RUN_DIR,
PER_ALWAYS,
PER_INSTANCE,
PER_ONCE,
RUN_CLOUD_CONFIG,
)
from cloudinit.sources import NetworkConfigSource

Expand Down Expand Up @@ -275,15 +275,36 @@ def read_cfg(self, extra_fns=None):
self._cfg = self._read_cfg(extra_fns)

def _read_cfg(self, extra_fns):
no_cfg_paths = helpers.Paths({}, self.datasource)
"""read and merge our configuration"""
# No config is passed to Paths() here because we don't yet have a
# config to pass. We must bootstrap a config to identify
# distro-specific run_dir locations. Once we have the run_dir
# we re-read our config with a valid Paths() object. This code has to
# assume the location of /etc/cloud/cloud.cfg && /etc/cloud/cloud.cfg.d

initial_config = self._read_bootstrap_cfg(extra_fns, {})
paths = initial_config.get("system_info", {}).get("paths", {})

# run_dir hasn't changed so we can safely return the config
if paths.get("run_dir") in (DEFAULT_RUN_DIR, None):
return initial_config

# run_dir has changed so re-read the config to get a valid one
# using the new location of run_dir
return self._read_bootstrap_cfg(extra_fns, paths)

def _read_bootstrap_cfg(self, extra_fns, bootstrapped_config: dict):
no_cfg_paths = helpers.Paths(bootstrapped_config, self.datasource)
instance_data_file = no_cfg_paths.get_runpath(
"instance_data_sensitive"
)
merger = helpers.ConfigMerger(
paths=no_cfg_paths,
datasource=self.datasource,
additional_fns=extra_fns,
base_cfg=fetch_base_config(instance_data_file=instance_data_file),
base_cfg=fetch_base_config(
no_cfg_paths.run_dir, instance_data_file=instance_data_file
),
)
return merger.cfg

Expand Down Expand Up @@ -510,6 +531,9 @@ def is_new_instance(self):
return ret

def fetch(self, existing="check"):
"""optionally load datasource from cache, otherwise discover
datasource
"""
return self._get_data_source(existing=existing)

def instancify(self):
Expand Down Expand Up @@ -1088,11 +1112,11 @@ def should_run_on_boot_event():
return


def read_runtime_config():
return util.read_conf(RUN_CLOUD_CONFIG)
def read_runtime_config(run_dir: str):
return util.read_conf(os.path.join(run_dir, "cloud.cfg"))


def fetch_base_config(*, instance_data_file=None) -> dict:
def fetch_base_config(run_dir: str, *, instance_data_file=None) -> dict:
return util.mergemanydict(
[
# builtin config, hardcoded in settings.py.
Expand All @@ -1102,7 +1126,7 @@ def fetch_base_config(*, instance_data_file=None) -> dict:
CLOUD_CONFIG, instance_data_file=instance_data_file
),
# runtime config. I.e., /run/cloud-init/cloud.cfg
read_runtime_config(),
read_runtime_config(run_dir),
# Kernel/cmdline parameters override system config
util.read_conf_from_cmdline(),
],
Expand Down
2 changes: 1 addition & 1 deletion config/cloud.cfg.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ system_info:
templates_dir: /etc/cloud/templates/
{% elif is_bsd %}
paths:
run_dir: /var/run/
run_dir: /var/run/cloud-init/
{% endif %}
{% if variant == "debian" %}
package_mirrors:
Expand Down
2 changes: 1 addition & 1 deletion sysvinit/freebsd/cloudinitlocal.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
``cloudinitlocal`` purposefully does not depend on ``dsidentify``.
That makes it easy for image builders to disable ``dsidentify``.
#}
# REQUIRE: ldconfig mountcritlocal
# REQUIRE: ldconfig cleanvar
# BEFORE: NETWORKING cloudinit cloudconfig cloudfinal

. /etc/rc.subr
Expand Down
8 changes: 1 addition & 7 deletions sysvinit/freebsd/dsidentify.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@
#!/bin/sh

# PROVIDE: dsidentify
{#
once we are correctly using ``paths.run_dir`` / ``paths.get_runpath()`` in the
python code-base, we can start thinking about how to bring that into
``ds-identify`` itself, and then!, then we can depend on (``REQUIRE``)
``var_run`` instead of ``mountcritlocal`` here.
#}
# REQUIRE: mountcritlocal
# REQUIRE: cleanvar
# BEFORE: cloudinitlocal

. /etc/rc.subr
Expand Down
53 changes: 37 additions & 16 deletions tests/unittests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
mock = test_helpers.mock

M_PATH = "cloudinit.cmd.main."
Tmpdir = namedtuple("Tmpdir", ["tmpdir", "link_d", "data_d"])


@pytest.fixture(autouse=False)
@pytest.fixture()
def mock_get_user_data_file(mocker, tmpdir):
yield mocker.patch(
"cloudinit.cmd.devel.logs._get_user_data_file",
Expand All @@ -33,6 +34,19 @@ def disable_setup_logging():
yield


@pytest.fixture()
def mock_status_wrapper(mocker, tmpdir):
link_d = os.path.join(tmpdir, "link")
data_d = os.path.join(tmpdir, "data")
with mocker.patch(
"cloudinit.cmd.main.read_cfg_paths",
return_value=mock.Mock(get_cpath=lambda _: data_d),
), mocker.patch(
"cloudinit.cmd.main.os.path.normpath", return_value=link_d
):
yield Tmpdir(tmpdir, link_d, data_d)


class TestCLI:
def _call_main(self, sysv_args=None):
if not sysv_args:
Expand All @@ -59,29 +73,29 @@ def _call_main(self, sysv_args=None):
),
],
)
def test_status_wrapper_errors(self, action, name, match, caplog, tmpdir):
data_d = tmpdir.join("data")
link_d = tmpdir.join("link")
def test_status_wrapper_errors(
self, action, name, match, caplog, mock_status_wrapper
):
FakeArgs = namedtuple("FakeArgs", ["action", "local", "mode"])
my_action = mock.Mock()

myargs = FakeArgs((action, my_action), False, "bogusmode")
with pytest.raises(ValueError, match=match):
cli.status_wrapper(name, myargs, data_d, link_d)
cli.status_wrapper(name, myargs)
assert [] == my_action.call_args_list

@mock.patch("cloudinit.cmd.main.atomic_helper.write_json")
def test_status_wrapper_init_local_writes_fresh_status_info(
self,
m_json,
tmpdir,
mock_status_wrapper,
):
"""When running in init-local mode, status_wrapper writes status.json.

Old status and results artifacts are also removed.
"""
data_d = tmpdir.join("data")
link_d = tmpdir.join("link")
data_d = mock_status_wrapper.data_d
link_d = mock_status_wrapper.link_d
# Write old artifacts which will be removed or updated.
for _dir in data_d, link_d:
test_helpers.populate_dir(
Expand All @@ -95,7 +109,7 @@ def myaction(name, args):
return "SomeDatasource", ["an error"]

myargs = FakeArgs(("ignored_name", myaction), True, "bogusmode")
cli.status_wrapper("init", myargs, data_d, link_d)
cli.status_wrapper("init", myargs)
# No errors reported in status
status_v1 = m_json.call_args_list[1][0][1]["v1"]
assert status_v1.keys() == {
Expand All @@ -117,14 +131,14 @@ def myaction(name, args):

@mock.patch("cloudinit.cmd.main.atomic_helper.write_json")
def test_status_wrapper_init_local_honor_cloud_dir(
self, m_json, mocker, tmpdir
self, m_json, mocker, mock_status_wrapper
):
"""When running in init-local mode, status_wrapper honors cloud_dir."""
cloud_dir = tmpdir.join("cloud")
cloud_dir = mock_status_wrapper.tmpdir.join("cloud")
paths = helpers.Paths({"cloud_dir": str(cloud_dir)})
mocker.patch(M_PATH + "read_cfg_paths", return_value=paths)
data_d = cloud_dir.join("data")
link_d = tmpdir.join("link")
data_d = mock_status_wrapper.data_d
link_d = mock_status_wrapper.link_d

FakeArgs = namedtuple("FakeArgs", ["action", "local", "mode"])

Expand All @@ -133,7 +147,7 @@ def myaction(name, args):
return "SomeDatasource", ["an_error"]

myargs = FakeArgs(("ignored_name", myaction), True, "bogusmode")
cli.status_wrapper("init", myargs, link_d=link_d) # No explicit data_d
cli.status_wrapper("init", myargs) # No explicit data_d

# Access cloud_dir directly
status_v1 = m_json.call_args_list[1][0][1]["v1"]
Expand Down Expand Up @@ -243,7 +257,12 @@ def test_modules_subcommand_parser(self, m_status_wrapper, subcommand):
)
@mock.patch("cloudinit.stages.Init._read_cfg", return_value={})
def test_conditional_subcommands_from_entry_point_sys_argv(
self, m_read_cfg, subcommand, capsys, mock_get_user_data_file, tmpdir
self,
m_read_cfg,
subcommand,
capsys,
mock_get_user_data_file,
mock_status_wrapper,
):
"""Subcommands from entry-point are properly parsed from sys.argv."""
expected_error = f"usage: cloud-init {subcommand}"
Expand All @@ -264,7 +283,9 @@ def test_conditional_subcommands_from_entry_point_sys_argv(
"status",
],
)
def test_subcommand_parser(self, subcommand, mock_get_user_data_file):
def test_subcommand_parser(
self, subcommand, mock_get_user_data_file, mock_status_wrapper
):
"""cloud-init `subcommand` calls its subparser."""
# Provide -h param to `subcommand` to avoid having to mock behavior.
out = io.StringIO()
Expand Down
Loading
Loading