From a3a2b835947f96f963ef3659592e9d9068e3f9f7 Mon Sep 17 00:00:00 2001 From: satoshi-sh Date: Fri, 14 Feb 2025 18:45:42 +0000 Subject: [PATCH] Add ContainerHardwareManager Implement container-based cleaning process Partial-Bug: #2100556 Change-Id: I39b92462d1454df888fc413e0aac439b9df199f7 --- ironic_python_agent/config.py | 29 ++++- .../hardware_managers/container.py | 109 ++++++++++++++++++ ironic_python_agent/tests/unit/test_agent.py | 44 ++++++- setup.cfg | 1 + 4 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 ironic_python_agent/hardware_managers/container.py diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 1ce795bb7..09db511fb 100644 --- a/ironic_python_agent/config.py +++ b/ironic_python_agent/config.py @@ -442,13 +442,39 @@ disk_part_opts = [ ' having failed.') ] +container_opts = [ + cfg.StrOpt('container_steps_file', + default='/etc/ironic-python-agent.d/mysteps.yaml', + help='Path to the YAML file containing container-based' + 'cleaning steps.'), + cfg.StrOpt('runner', + default='podman', + choices=['podman', 'docker'], + help='Container runtime to use for cleaning steps.'), + cfg.ListOpt('pull_options', + default=['--tls-verify=false'], + help='Options to use when pulling container images.'), + cfg.ListOpt('run_options', + default=['--rm', '--network=host', '--tls-verify=false'], + help='Options to use when running containers.'), + cfg.BoolOpt('allow_arbitrary_containers', + default=False, + help='Allow arbitrary containers to be run' + 'without restriction.'), + cfg.ListOpt('allowed_containers', + default=[], + help='List of allowed containers that can be run.'), +] + def list_opts(): return [('DEFAULT', cli_opts), ('disk_utils', disk_utils_opts), ('disk_partitioner', disk_part_opts), ('metrics', metrics_opts), - ('metrics_statsd', statsd_opts)] + ('metrics_statsd', statsd_opts), + ('container', container_opts) + ] def populate_config(): @@ -458,6 +484,7 @@ def populate_config(): CONF.register_opts(disk_part_opts, group='disk_partitioner') CONF.register_opts(metrics_opts, group='metrics') CONF.register_opts(statsd_opts, group='metrics_statsd') + CONF.register_opts(container_opts, group='container') def override(params): diff --git a/ironic_python_agent/hardware_managers/container.py b/ironic_python_agent/hardware_managers/container.py new file mode 100644 index 000000000..f474cea1e --- /dev/null +++ b/ironic_python_agent/hardware_managers/container.py @@ -0,0 +1,109 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +from ironic_python_agent import hardware +from ironic_python_agent import utils +from oslo_config import cfg +from oslo_log import log + +import yaml + +CONF = cfg.CONF +LOG = log.getLogger() + + +class ContainerHardwareManager(hardware.HardwareManager): + """Hardware manager for container-based cleanup.""" + + HARDWARE_MANAGER_NAME = "ContainerHardwareManager" + HARDWARE_MANAGER_VERSION = "1" + ALLOW_ARBITARY_CONTAINERS = CONF.container.allow_arbitrary_containers + ALLOWED_CONTAINERS = CONF.container.allowed_containers + + def __init__(self): + """Dynamically create cleanup methods.""" + self.STEPS = self._load_steps_from_yaml( + CONF.container.container_steps_file) + LOG.debug("Loaded steps: %s", self.STEPS) + for step in self.STEPS: + if not self.ALLOW_ARBITARY_CONTAINERS: + if step['image'] not in self.ALLOWED_CONTAINERS: + LOG.debug( + "%s is not registered as ALLOWED_CONTAINERS " + "in ironic-python-agent.conf", step['image'] + ) + + continue + setattr(self, step["name"], self._create_cleanup_method(step)) + + def _load_steps_from_yaml(self, file_path): + """Load steps from YAML file.""" + try: + with open(file_path, 'r') as file: + data = yaml.safe_load(file) + return data.get('steps', []) + except Exception as e: + LOG.debug("Error loading steps from YAML file: %s", e) + return [] + + def evaluate_hardware_support(self): + """Determine if container runner exists and return support level.""" + try: + stdout, _ = utils.execute("which", CONF.container.runner) + if stdout.strip(): + LOG.debug("Found %s, returning MAINLINE", + CONF.container.runner) + return hardware.HardwareSupport.MAINLINE + except Exception as e: + LOG.debug("Error checking container runner: %s", str(e)) + LOG.debug("No container runner found, returning NONE") + return hardware.HardwareSupport.NONE + + def get_clean_steps(self, node, ports): + """Dynamically generate cleaning steps.""" + steps = [] + for step in self.STEPS: + steps.append( + { + "step": step["name"], + "priority": step["priority"], + "interface": step['interface'], + "reboot_requested": step['reboot_requested'], + "abortable": step["abortable"], + } + ) + return steps + + def _create_cleanup_method(self, step): + """Return a function that runs the container with the given image.""" + + def _cleanup(node, ports): + try: + utils.execute( + CONF.container.runner, + "pull", + *step.get("pull_options", CONF.container.pull_options), + step["image"], + ) + utils.execute( + CONF.container.runner, + "run", + *step.get("run_options", CONF.container.run_options), + step["image"], + ) + except Exception as e: + LOG.exception("Error during cleanup: %s", e) + raise + return True + + return _cleanup diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 113208a84..231c74ef1 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -580,10 +580,16 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): @mock.patch('oslo_service.wsgi.Server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info', autospec=True) - def test_run_with_inspection(self, mock_list_hardware, mock_wsgi, - mock_dispatch, mock_inspector, mock_wait, - mock_mpath): + @mock.patch( + 'ironic_python_agent.hardware_managers.container.' + 'ContainerHardwareManager.evaluate_hardware_support', + autospec=True + ) + def test_run_with_inspection(self, mock_hardware, mock_list_hardware, + mock_wsgi, mock_dispatch, mock_inspector, + mock_wait, mock_mpath): CONF.set_override('inspection_callback_url', 'http://foo/bar') + mock_hardware.return_value = 0 def set_serve_api(): self.agent.serve_api = False @@ -635,7 +641,13 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): @mock.patch('oslo_service.wsgi.Server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info', autospec=True) + @mock.patch( + 'ironic_python_agent.hardware_managers.container.' + 'ContainerHardwareManager.evaluate_hardware_support', + autospec=True + ) def test_run_with_inspection_without_apiurl(self, + mock_hardware, mock_list_hardware, mock_wsgi, mock_dispatch, @@ -649,6 +661,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): # server will work as usual. Also, make sure api_client and heartbeater # will not be initialized in this case. CONF.set_override('inspection_callback_url', 'http://foo/bar') + mock_hardware.return_value = 0 self.agent = agent.IronicPythonAgent(None, agent.Host('203.0.113.1', 9990), @@ -694,7 +707,13 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): @mock.patch('oslo_service.wsgi.Server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info', autospec=True) + @mock.patch( + 'ironic_python_agent.hardware_managers.container.' + 'ContainerHardwareManager.evaluate_hardware_support', + autospec=True + ) def test_run_without_inspection_and_apiurl(self, + mock_hardware, mock_list_hardware, mock_wsgi, mock_dispatch, @@ -703,6 +722,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): mock_mdns, mock_mpath): mock_mdns.side_effect = errors.ServiceLookupFailure() + mock_hardware.return_value = 0 # If both api_url and inspection_callback_url are not configured when # the agent starts, ensure that the inspection will be skipped and wsgi # server will work as usual. Also, make sure api_client and heartbeater @@ -1090,11 +1110,18 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest): autospec=True) @mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card', autospec=True) - def test_with_network_interface(self, mock_cna, mock_get_ipv4, mock_mpath, + @mock.patch( + 'ironic_python_agent.hardware_managers.container.' + 'ContainerHardwareManager.evaluate_hardware_support', + autospec=True + ) + def test_with_network_interface(self, mock_hardware, mock_cna, + mock_get_ipv4, mock_mpath, mock_exec, mock_getaddrinfo): self.agent.network_interface = 'em1' mock_get_ipv4.return_value = '1.2.3.4' mock_cna.return_value = False + mock_hardware.return_value = 0 self.agent.set_agent_advertise_addr() @@ -1109,12 +1136,19 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest): autospec=True) @mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card', autospec=True) - def test_with_network_interface_failed(self, mock_cna, mock_get_ipv4, + @mock.patch( + 'ironic_python_agent.hardware_managers.container.' + 'ContainerHardwareManager.evaluate_hardware_support', + autospec=True + ) + def test_with_network_interface_failed(self, mock_hardware, + mock_cna, mock_get_ipv4, mock_mpath, mock_exec, mock_getaddrinfo): self.agent.network_interface = 'em1' mock_get_ipv4.return_value = None mock_cna.return_value = False + mock_hardware.return_value = 0 self.assertRaises(errors.LookupAgentIPError, self.agent.set_agent_advertise_addr) diff --git a/setup.cfg b/setup.cfg index e08926212..0cab5234a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -51,6 +51,7 @@ ironic_python_agent.hardware_managers = generic = ironic_python_agent.hardware:GenericHardwareManager mlnx = ironic_python_agent.hardware_managers.mlnx:MellanoxDeviceHardwareManager cna = ironic_python_agent.hardware_managers.cna:IntelCnaHardwareManager + container = ironic_python_agent.hardware_managers.container:ContainerHardwareManager ironic_python_agent.inspector.collectors = default = ironic_python_agent.inspector:collect_default