From 1de0b0781bcf71d5ea2edacc59c0a1f329f3770b Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 20:44:59 +0200 Subject: [PATCH 1/6] Removed default for hostgroups and fixed bug for hostgroup attributes which do not exist --- modules/device.py | 3 +++ modules/hostgroups.py | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/device.py b/modules/device.py index 28e78b4..c3baa17 100644 --- a/modules/device.py +++ b/modules/device.py @@ -564,6 +564,9 @@ class PhysicalDevice: final_data = [] # Check if the hostgroup is in a nested format and check each parent for hostgroup in self.hostgroups: + # Check if hostgroup is string. If Nonetype skip hostgroup + if not isinstance(hostgroup, str): + continue for pos in range(len(hostgroup.split("/"))): zabbix_hg = hostgroup.rsplit("/", pos)[0] if self.lookupZabbixHostgroup(hostgroups, zabbix_hg): diff --git a/modules/hostgroups.py b/modules/hostgroups.py index f98c09b..d5306a3 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -108,11 +108,6 @@ class Hostgroup: def generate(self, hg_format=None): """Generate hostgroup based on a provided format""" - # Set format to default in case its not specified - if not hg_format: - hg_format = ( - "site/manufacturer/role" if self.type == "dev" else "cluster/role" - ) # Split all given names hg_output = [] hg_items = hg_format.split("/") From a522c989298429a3dd165943f15b1e6f706c9f15 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 20:50:04 +0200 Subject: [PATCH 2/6] Removed default None for hg_format making a hostgroup format input required. --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index d5306a3..38c44c0 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -106,7 +106,7 @@ class Hostgroup: "region": {"flag": nested_region_flag, "data": nb_regions}, } - def generate(self, hg_format=None): + def generate(self, hg_format): """Generate hostgroup based on a provided format""" # Split all given names hg_output = [] From 6d4f1ac0a54ebed6883f8f882bb27ffc5c0cef20 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:28:13 +0200 Subject: [PATCH 3/6] Added hostgroup tests --- tests/test_hostgroups.py | 80 +++++++++++----- tests/test_list_hostgroup_formats.py | 137 +++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 24 deletions(-) create mode 100644 tests/test_list_hostgroup_formats.py diff --git a/tests/test_hostgroups.py b/tests/test_hostgroups.py index 1e652ec..995d26c 100644 --- a/tests/test_hostgroups.py +++ b/tests/test_hostgroups.py @@ -163,10 +163,6 @@ class TestHostgroups(unittest.TestCase): """Test different hostgroup formats for devices.""" hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) - # Default format: site/manufacturer/role - default_result = hostgroup.generate() - self.assertEqual(default_result, "TestSite/TestManufacturer/TestRole") - # Custom format: site/region custom_result = hostgroup.generate("site/region") self.assertEqual(custom_result, "TestSite/TestRegion") @@ -180,7 +176,7 @@ class TestHostgroups(unittest.TestCase): hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) # Default format: cluster/role - default_result = hostgroup.generate() + default_result = hostgroup.generate("cluster/role") self.assertEqual(default_result, "TestCluster/TestRole") # Custom format: site/tenant @@ -251,7 +247,7 @@ class TestHostgroups(unittest.TestCase): hostgroup = Hostgroup("dev", minimal_device, "4.0", self.mock_logger) # Generate with default format - result = hostgroup.generate() + result = hostgroup.generate("site/manufacturer/role") # Site is missing, so only manufacturer and role should be included self.assertEqual(result, "MinimalManufacturer/MinimalRole") @@ -259,24 +255,6 @@ class TestHostgroups(unittest.TestCase): with self.assertRaises(HostgroupError): hostgroup.generate("site/nonexistent/role") - def test_hostgroup_missing_required_attributes(self): - """Test handling when no valid hostgroup can be generated.""" - # Create a VM with minimal attributes that won't satisfy any format - minimal_vm = MagicMock() - minimal_vm.name = "minimal-vm" - minimal_vm.site = None - minimal_vm.tenant = None - minimal_vm.platform = None - minimal_vm.role = None - minimal_vm.cluster = None - minimal_vm.custom_fields = {} - - hostgroup = Hostgroup("vm", minimal_vm, "4.0", self.mock_logger) - - # With default format of cluster/role, both are None, so should raise an error - with self.assertRaises(HostgroupError): - hostgroup.generate() - def test_nested_region_hostgroups(self): """Test hostgroup generation with nested regions.""" # Mock the build_path function to return a predictable result @@ -334,6 +312,60 @@ class TestHostgroups(unittest.TestCase): calls = [call.write(f"The following options are available for host test-device"), call.write('\n')] mock_stdout.assert_has_calls(calls, any_order=True) + + def test_vm_list_based_hostgroup_format(self): + """Test VM hostgroup generation with a list-based format.""" + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Test with a list of format strings + format_list = ["platform", "role", "cluster_type/cluster"] + + # Generate hostgroups for each format in the list + hostgroups = [] + for fmt in format_list: + result = hostgroup.generate(fmt) + if result: # Only add non-None results + hostgroups.append(result) + + # Verify each expected hostgroup is generated + self.assertEqual(len(hostgroups), 3) # Should have 3 hostgroups + self.assertIn("TestPlatform", hostgroups) + self.assertIn("TestRole", hostgroups) + self.assertIn("TestClusterType/TestCluster", hostgroups) + + def test_nested_format_splitting(self): + """Test that formats with slashes correctly split and resolve each component.""" + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Test a format with slashes that should be split + complex_format = "cluster_type/cluster" + result = hostgroup.generate(complex_format) + + # Verify the format is correctly split and each component resolved + self.assertEqual(result, "TestClusterType/TestCluster") + + def test_multiple_hostgroup_formats_device(self): + """Test device hostgroup generation with multiple formats.""" + hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) + + # Test with various formats that would be in a list + formats = [ + "site", + "manufacturer/role", + "platform/location", + "tenant_group/tenant" + ] + + # Generate and check each format + results = {} + for fmt in formats: + results[fmt] = hostgroup.generate(fmt) + + # Verify results + self.assertEqual(results["site"], "TestSite") + self.assertEqual(results["manufacturer/role"], "TestManufacturer/TestRole") + self.assertEqual(results["platform/location"], "TestPlatform/TestLocation") + self.assertEqual(results["tenant_group/tenant"], "TestTenantGroup/TestTenant") if __name__ == "__main__": diff --git a/tests/test_list_hostgroup_formats.py b/tests/test_list_hostgroup_formats.py new file mode 100644 index 0000000..9b8cc21 --- /dev/null +++ b/tests/test_list_hostgroup_formats.py @@ -0,0 +1,137 @@ +"""Tests for list-based hostgroup formats in configuration.""" +import unittest +from unittest.mock import MagicMock, patch +from modules.hostgroups import Hostgroup +from modules.exceptions import HostgroupError +from modules.tools import verify_hg_format + + +class TestListHostgroupFormats(unittest.TestCase): + """Test class for list-based hostgroup format functionality.""" + + def setUp(self): + """Set up test fixtures.""" + # Create mock logger + self.mock_logger = MagicMock() + + # Create mock device + self.mock_device = MagicMock() + self.mock_device.name = "test-device" + + # Set up site information + site = MagicMock() + site.name = "TestSite" + + # Set up region information + region = MagicMock() + region.name = "TestRegion" + region.__str__.return_value = "TestRegion" + site.region = region + + # Set device site + self.mock_device.site = site + + # Set up role information + self.mock_device_role = MagicMock() + self.mock_device_role.name = "TestRole" + self.mock_device_role.__str__.return_value = "TestRole" + self.mock_device.role = self.mock_device_role + + # Set up rack information + rack = MagicMock() + rack.name = "TestRack" + self.mock_device.rack = rack + + # Set up platform information + platform = MagicMock() + platform.name = "TestPlatform" + self.mock_device.platform = platform + + # Device-specific properties + device_type = MagicMock() + manufacturer = MagicMock() + manufacturer.name = "TestManufacturer" + device_type.manufacturer = manufacturer + self.mock_device.device_type = device_type + + # Create mock VM + self.mock_vm = MagicMock() + self.mock_vm.name = "test-vm" + + # Reuse site from device + self.mock_vm.site = site + + # Set up role for VM + self.mock_vm.role = self.mock_device_role + + # Set up platform for VM + self.mock_vm.platform = platform + + # VM-specific properties + cluster = MagicMock() + cluster.name = "TestCluster" + cluster_type = MagicMock() + cluster_type.name = "TestClusterType" + cluster.type = cluster_type + self.mock_vm.cluster = cluster + + def test_verify_list_based_hostgroup_format(self): + """Test verification of list-based hostgroup formats.""" + # List format with valid items + valid_format = ["region", "site", "rack"] + + # List format with nested path + valid_nested_format = ["region", "site/rack"] + + # List format with invalid item + invalid_format = ["region", "invalid_item", "rack"] + + # Should not raise exception for valid formats + verify_hg_format(valid_format, hg_type="dev", logger=self.mock_logger) + verify_hg_format(valid_nested_format, hg_type="dev", logger=self.mock_logger) + + # Should raise exception for invalid format + with self.assertRaises(HostgroupError): + verify_hg_format(invalid_format, hg_type="dev", logger=self.mock_logger) + + def test_simulate_hostgroup_generation_from_config(self): + """Simulate how the main script would generate hostgroups from list-based config.""" + # Mock configuration with list-based hostgroup format + config_format = ["region", "site", "rack"] + hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) + + # Simulate the main script's hostgroup generation process + hostgroups = [] + for fmt in config_format: + result = hostgroup.generate(fmt) + if result: + hostgroups.append(result) + + # Check results + self.assertEqual(len(hostgroups), 3) + self.assertIn("TestRegion", hostgroups) + self.assertIn("TestSite", hostgroups) + self.assertIn("TestRack", hostgroups) + + def test_vm_hostgroup_format_from_config(self): + """Test VM hostgroup generation with list-based format.""" + # Mock VM configuration with mixed format + config_format = ["platform", "role", "cluster_type/cluster"] + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Simulate the main script's hostgroup generation process + hostgroups = [] + for fmt in config_format: + result = hostgroup.generate(fmt) + if result: + hostgroups.append(result) + + # Check results + self.assertEqual(len(hostgroups), 3) + self.assertIn("TestPlatform", hostgroups) + self.assertIn("TestRole", hostgroups) + self.assertIn("TestClusterType/TestCluster", hostgroups) + + +if __name__ == "__main__": + unittest.main() From 4a53b53789d9af793e18fdae9b911d87e61d7375 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:28:32 +0200 Subject: [PATCH 4/6] Removed previous patch for Nonetype hostgroups and made a proper fix by refactoring the set_hostgroup() function and removing it from virtual_machines.py --- modules/device.py | 6 ++---- modules/hostgroups.py | 1 - modules/virtual_machine.py | 20 +------------------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/modules/device.py b/modules/device.py index c3baa17..03ca191 100644 --- a/modules/device.py +++ b/modules/device.py @@ -48,6 +48,7 @@ class PhysicalDevice: self.zbx_template_names = [] self.zbx_templates = [] self.hostgroups = [] + self.hostgroup_type = "dev" self.tenant = nb.tenant self.config_context = nb.config_context self.zbxproxy = None @@ -121,7 +122,7 @@ class PhysicalDevice: """Set the hostgroup for this device""" # Create new Hostgroup instance hg = Hostgroup( - "dev", + self.hostgroup_type, self.nb, self.nb_api_version, logger=self.logger, @@ -564,9 +565,6 @@ class PhysicalDevice: final_data = [] # Check if the hostgroup is in a nested format and check each parent for hostgroup in self.hostgroups: - # Check if hostgroup is string. If Nonetype skip hostgroup - if not isinstance(hostgroup, str): - continue for pos in range(len(hostgroup.split("/"))): zabbix_hg = hostgroup.rsplit("/", pos)[0] if self.lookupZabbixHostgroup(hostgroups, zabbix_hg): diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 38c44c0..213b4cf 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -144,7 +144,6 @@ class Hostgroup: ) self.logger.warning(msg) return None - #raise HostgroupError(msg) def list_formatoptions(self): """ diff --git a/modules/virtual_machine.py b/modules/virtual_machine.py index e0f7abb..847375d 100644 --- a/modules/virtual_machine.py +++ b/modules/virtual_machine.py @@ -16,6 +16,7 @@ class VirtualMachine(PhysicalDevice): super().__init__(*args, **kwargs) self.hostgroup = None self.zbx_template_names = None + self.hostgroup_type = "vm" def _inventory_map(self): """use VM inventory maps""" @@ -29,25 +30,6 @@ class VirtualMachine(PhysicalDevice): """use VM tag maps""" return config["vm_tag_map"] - def set_hostgroup(self, hg_format, nb_site_groups, nb_regions): - """Set the hostgroup for this device""" - # Create new Hostgroup instance - hg = Hostgroup( - "vm", - self.nb, - self.nb_api_version, - logger=self.logger, - nested_sitegroup_flag=config["traverse_site_groups"], - nested_region_flag=config["traverse_regions"], - nb_groups=nb_site_groups, - nb_regions=nb_regions, - ) - # Generate hostgroup based on hostgroup format - if isinstance(hg_format, list): - self.hostgroups = [hg.generate(f) for f in hg_format] - else: - self.hostgroups.append(hg.generate(hg_format)) - def set_vm_template(self): """Set Template for VMs. Overwrites default class to skip a lookup of custom fields.""" From 29a54e5a86e6fca5b1858cac5f6c5b71fd512628 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:29:36 +0200 Subject: [PATCH 5/6] Removed unused hostgroup import since the hostgroup generate function function has been moved to devices.py --- modules/virtual_machine.py | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/virtual_machine.py b/modules/virtual_machine.py index 847375d..8c52033 100644 --- a/modules/virtual_machine.py +++ b/modules/virtual_machine.py @@ -2,7 +2,6 @@ """Module that hosts all functions for virtual machine processing""" from modules.device import PhysicalDevice from modules.exceptions import InterfaceConfigError, SyncInventoryError, TemplateError -from modules.hostgroups import Hostgroup from modules.interface import ZabbixInterface from modules.config import load_config # Load config From 5923682d48b8836d339ae2c05f826adf80af65cd Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:42:46 +0200 Subject: [PATCH 6/6] Fixes workflows to be executed 2 times. --- .github/workflows/quality.yml | 1 - .github/workflows/run_tests.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 8dfbe3f..81cad97 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -2,7 +2,6 @@ name: Pylint Quality control on: - push: pull_request: workflow_call: diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 6a16a64..589fc47 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -2,7 +2,6 @@ name: Pytest code testing on: - push: pull_request: workflow_call: