Merge pull request #74 from stfc/Use_vm_personality_first

Use vm personality first

OpenStack-Rabbit-Consumer/rabbit_consumer/aq_api.py

@@ -8,7 +8,7 @@
 from urllib3.util.retry import Retry
 
 from rabbit_consumer.consumer_config import ConsumerConfig
-from rabbit_consumer.image_metadata import ImageMetadata
+from rabbit_consumer.aq_metadata import AqMetadata
 from rabbit_consumer.openstack_address import OpenstackAddress
 from rabbit_consumer.rabbit_message import RabbitMessage
 from rabbit_consumer.vm_data import VmData
@@ -84,7 +84,7 @@ def setup_requests(
     return response.text
 
 
-def aq_make(addresses: List[OpenstackAddress], image_meta: ImageMetadata) -> None:
+def aq_make(addresses: List[OpenstackAddress], image_meta: AqMetadata) -> None:
     """
     Runs AQ make against a list of addresses passed to build the default personality
     """
@@ -111,7 +111,7 @@ def aq_make(addresses: List[OpenstackAddress], image_meta: ImageMetadata) -> Non
     setup_requests(url, "post", "Make Template: ", params)
 
 
-def aq_manage(addresses: List[OpenstackAddress], image_meta: ImageMetadata) -> None:
+def aq_manage(addresses: List[OpenstackAddress], image_meta: AqMetadata) -> None:
     """
     Manages the list of Aquilon addresses passed to it back to the production domain
     """
@@ -159,7 +159,7 @@ def delete_machine(machine_name: str) -> None:
 
 
 def create_host(
-    image_meta: ImageMetadata, addresses: List[OpenstackAddress], machine_name: str
+    image_meta: AqMetadata, addresses: List[OpenstackAddress], machine_name: str
 ) -> None:
     """
     Creates a host in Aquilon

OpenStack-Rabbit-Consumer/rabbit_consumer/aq_metadata.py

@@ -0,0 +1,50 @@
+import logging
+from dataclasses import dataclass
+from typing import Dict
+
+from mashumaro import DataClassDictMixin
+from mashumaro.config import BaseConfig
+
+logger = logging.getLogger(__name__)
+
+
+@dataclass
+class AqMetadata(DataClassDictMixin):
+    """
+    Deserialised metadata that is set either on an Openstack image
+    or a VM's metadata
+    """
+
+    aq_archetype: str
+    # Aq domain can hold either a domain or sandbox reference
+    aq_domain: str
+
+    aq_personality: str
+    aq_os_version: str
+    aq_os: str
+
+    # pylint: disable=too-few-public-methods
+    class Config(BaseConfig):
+        """
+        Sets the aliases for the metadata keys
+        """
+
+        aliases = {
+            "aq_archetype": "AQ_ARCHETYPE",
+            "aq_domain": "AQ_DOMAIN",
+            "aq_personality": "AQ_PERSONALITY",
+            "aq_os_version": "AQ_OSVERSION",
+            "aq_os": "AQ_OS",
+        }
+
+    def override_from_vm_meta(self, vm_meta: Dict[str, str]):
+        """
+        Overrides the values in the metadata with the values from the VM's
+        metadata
+        """
+        for attr, alias in self.Config.aliases.items():
+            if alias in vm_meta:
+                setattr(self, attr, vm_meta[alias])
+
+        if "AQ_SANDBOX" in vm_meta:
+            self.aq_domain = vm_meta["AQ_SANDBOX"]

OpenStack-Rabbit-Consumer/rabbit_consumer/message_consumer.py

@@ -9,7 +9,7 @@
 from rabbit_consumer import openstack_api
 from rabbit_consumer.aq_api import verify_kerberos_ticket
 from rabbit_consumer.consumer_config import ConsumerConfig
-from rabbit_consumer.image_metadata import ImageMetadata
+from rabbit_consumer.aq_metadata import AqMetadata
 from rabbit_consumer.openstack_address import OpenstackAddress
 from rabbit_consumer.rabbit_message import RabbitMessage, MessageEventType
 from rabbit_consumer.vm_data import VmData
@@ -21,17 +21,28 @@
 }
 
 
-def is_aq_managed_image(rabbit_message: RabbitMessage) -> Optional[ImageMetadata]:
+def is_aq_managed_image(vm_data: VmData) -> bool:
     """
     Check to see if the metadata in the message contains entries that suggest it
     is for an Aquilon VM.
     """
-    image = openstack_api.get_image(VmData.from_message(rabbit_message))
+    image = openstack_api.get_image(vm_data)
     if "AQ_OS" not in image.metadata:
         logger.debug("Skipping non-Aquilon image: %s", image.name)
-        return None
+        return False
+    return True
+
+
+def get_aq_build_metadata(vm_data: VmData) -> AqMetadata:
+    """
+    Gets the Aq Metadata from either the image or VM (where
+    VM metadata takes precedence) to determine the AQ params
+    """
+    image = openstack_api.get_image(vm_data)
+    image_meta = AqMetadata.from_dict(image.metadata)
 
-    image_meta = ImageMetadata.from_dict(image.metadata)
+    vm_metadata = openstack_api.get_server_metadata(vm_data)
+    image_meta.override_from_vm_meta(vm_metadata)
     return image_meta
 
 
@@ -111,7 +122,7 @@ def check_machine_valid(rabbit_message: RabbitMessage) -> bool:
         )
         return False
 
-    if not is_aq_managed_image(rabbit_message):
+    if not is_aq_managed_image(vm_data):
         logger.debug("Ignoring non AQ Image: %s", rabbit_message)
         return False
 
@@ -131,7 +142,7 @@ def handle_create_machine(rabbit_message: RabbitMessage) -> None:
 
     vm_data = VmData.from_message(rabbit_message)
 
-    image_meta = is_aq_managed_image(rabbit_message)
+    image_meta = get_aq_build_metadata(vm_data)
     network_details = openstack_api.get_server_networks(vm_data)
 
     if not network_details or not network_details[0].hostname:

OpenStack-Rabbit-Consumer/rabbit_consumer/openstack_api.py

@@ -71,7 +71,7 @@ def get_server_networks(vm_data: VmData) -> List[OpenstackAddress]:
     return OpenstackAddress.get_internal_networks(server.addresses)
 
 
-def get_metadata(vm_data: VmData) -> dict:
+def get_server_metadata(vm_data: VmData) -> dict:
     """
     Gets the metadata from Openstack for the virtual machine.
     """

OpenStack-Rabbit-Consumer/test/fixtures.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from rabbit_consumer.image_metadata import ImageMetadata
+from rabbit_consumer.aq_metadata import AqMetadata
 from rabbit_consumer.openstack_address import OpenstackAddress
 from rabbit_consumer.rabbit_message import RabbitMessage, RabbitMeta, RabbitPayload
 from rabbit_consumer.vm_data import VmData
@@ -14,7 +14,7 @@ def fixture_image_metadata():
     Creates an ImageMetadata object with mock data
     which represent an example OpenStack image
     """
-    return ImageMetadata(
+    return AqMetadata(
         aq_archetype="archetype_mock",
         aq_domain="domain_mock",
         aq_personality="personality_mock",

OpenStack-Rabbit-Consumer/test/test_aq_metadata.py

@@ -0,0 +1,75 @@
+from typing import Dict
+
+import pytest
+
+from rabbit_consumer.aq_metadata import AqMetadata
+
+
+@pytest.fixture(name="image_metadata")
+def fixture_image_metadata() -> Dict[str, str]:
+    """
+    Creates a dictionary with mock data
+    which represents an example OpenStack image's metadata
+    """
+    return {
+        "AQ_ARCHETYPE": "archetype_mock",
+        "AQ_DOMAIN": "domain_mock",
+        "AQ_PERSONALITY": "personality_mock",
+        "AQ_OS": "os_mock",
+        "AQ_OSVERSION": "osversion_mock",
+    }
+
+
+def test_aq_metadata_from_initial_dict(image_metadata):
+    """
+    Tests creating an AQ metadata object from an initial dictionary
+    """
+    returned = AqMetadata.from_dict(image_metadata)
+
+    assert returned.aq_archetype == "archetype_mock"
+    assert returned.aq_domain == "domain_mock"
+    assert returned.aq_personality == "personality_mock"
+    assert returned.aq_os == "os_mock"
+    assert returned.aq_os_version == "osversion_mock"
+
+
+def test_aq_metadata_override_all(image_metadata):
+    """
+    Tests overriding all values in an AQ metadata object
+    """
+    returned = AqMetadata.from_dict(image_metadata)
+    returned.override_from_vm_meta(
+        {
+            "AQ_ARCHETYPE": "archetype_mock_override",
+            "AQ_DOMAIN": "domain_mock_override",
+            "AQ_PERSONALITY": "personality_mock_override",
+        }
+    )
+
+    assert returned.aq_archetype == "archetype_mock_override"
+    assert returned.aq_domain == "domain_mock_override"
+    assert returned.aq_personality == "personality_mock_override"
+
+    # Check the original values are still there
+    assert returned.aq_os == "os_mock"
+    assert returned.aq_os_version == "osversion_mock"
+
+
+def test_aq_metadata_override_sandbox(image_metadata):
+    """
+    Tests overriding the sandbox value in an AQ metadata object
+    maps correctly onto the domain value
+    """
+    returned = AqMetadata.from_dict(image_metadata)
+    returned.override_from_vm_meta(
+        {
+            "AQ_SANDBOX": "sandbox_mock",
+        }
+    )
+    # This should be the only value that has changed
+    assert returned.aq_domain == "sandbox_mock"
+
+    assert returned.aq_archetype == "archetype_mock"
+    assert returned.aq_personality == "personality_mock"
+    assert returned.aq_os == "os_mock"
+    assert returned.aq_os_version == "osversion_mock"

OpenStack-Rabbit-Consumer/test/test_message_consumer.py

@@ -1,4 +1,4 @@
-from unittest.mock import Mock, NonCallableMock, patch, call
+from unittest.mock import Mock, NonCallableMock, patch, call, MagicMock
 
 import pytest
 
@@ -21,6 +21,7 @@
     SUPPORTED_MESSAGE_TYPES,
     check_machine_valid,
     is_aq_managed_image,
+    get_aq_build_metadata,
 )
 from rabbit_consumer.vm_data import VmData
 
@@ -217,11 +218,13 @@ def test_consume_create_machine_hostnames_good_path(
     with (
         patch("rabbit_consumer.message_consumer.VmData") as data_patch,
         patch("rabbit_consumer.message_consumer.check_machine_valid") as check_machine,
-        patch("rabbit_consumer.message_consumer.is_aq_managed_image") as is_managed,
+        patch(
+            "rabbit_consumer.message_consumer.get_aq_build_metadata"
+        ) as get_image_meta,
         patch("rabbit_consumer.message_consumer.delete_machine") as delete_machine,
     ):
         check_machine.return_value = True
-        is_managed.return_value = image_metadata
+        get_image_meta.return_value = image_metadata
 
         handle_create_machine(rabbit_message)
 
@@ -272,13 +275,14 @@ def test_check_machine_valid(openstack_api, is_aq_managed):
     """
     mock_message = NonCallableMock()
     is_aq_managed.return_value = True
+
+    vm_data = VmData.from_message(mock_message)
+
     openstack_api.check_machine_exists.return_value = True
 
     assert check_machine_valid(mock_message)
-    is_aq_managed.assert_called_once_with(mock_message)
-    openstack_api.check_machine_exists.assert_called_once_with(
-        VmData.from_message(mock_message)
-    )
+    is_aq_managed.assert_called_once_with(vm_data)
+    openstack_api.check_machine_exists.assert_called_once_with(vm_data)
 
 
 @patch("rabbit_consumer.message_consumer.is_aq_managed_image")
@@ -290,13 +294,12 @@ def test_check_machine_invalid_image(openstack_api, is_aq_managed):
     mock_message = NonCallableMock()
     is_aq_managed.return_value = False
     openstack_api.check_machine_exists.return_value = True
+    vm_data = VmData.from_message(mock_message)
 
     assert not check_machine_valid(mock_message)
 
-    openstack_api.check_machine_exists.assert_called_once_with(
-        VmData.from_message(mock_message)
-    )
-    is_aq_managed.assert_called_once_with(mock_message)
+    openstack_api.check_machine_exists.assert_called_once_with(vm_data)
+    is_aq_managed.assert_called_once_with(vm_data)
 
 
 @patch("rabbit_consumer.message_consumer.is_aq_managed_image")
@@ -316,30 +319,45 @@ def test_check_machine_invalid_machine(openstack_api, is_aq_managed):
     )
 
 
-@patch("rabbit_consumer.message_consumer.VmData")
-@patch("rabbit_consumer.message_consumer.ImageMetadata")
 @patch("rabbit_consumer.message_consumer.openstack_api")
-def test_is_aq_managed_image(openstack_api, image_meta, vm_data):
+def test_is_aq_managed_image(openstack_api, vm_data):
     """
     Test that the function returns True when the image is AQ managed
     """
-    mock_message = NonCallableMock()
     openstack_api.get_image.return_value.metadata = {"AQ_OS": "True"}
 
-    assert is_aq_managed_image(mock_message) == image_meta.from_dict.return_value
-    openstack_api.get_image.assert_called_once_with(vm_data.from_message.return_value)
+    assert is_aq_managed_image(vm_data)
+    openstack_api.get_image.assert_called_once_with(vm_data)
 
 
 @patch("rabbit_consumer.message_consumer.VmData")
-@patch("rabbit_consumer.message_consumer.ImageMetadata")
 @patch("rabbit_consumer.message_consumer.openstack_api")
-def test_is_aq_managed_image_missing_key(openstack_api, image_meta, vm_data):
+def test_is_aq_managed_image_missing_key(openstack_api, vm_data):
     """
     Test that the function returns False when the image is not AQ managed
     """
-    mock_message = NonCallableMock()
     openstack_api.get_image.return_value.metadata = {}
 
-    assert not is_aq_managed_image(mock_message)
-    openstack_api.get_image.assert_called_once_with(vm_data.from_message.return_value)
-    image_meta.from_dict.assert_not_called()
+    assert not is_aq_managed_image(vm_data)
+    openstack_api.get_image.assert_called_once_with(vm_data)
+
+
+@patch("rabbit_consumer.message_consumer.AqMetadata")
+@patch("rabbit_consumer.message_consumer.openstack_api")
+def test_get_aq_build_metadata(openstack_api, aq_metadata_class, vm_data):
+    """
+    Test that the function returns the correct metadata
+    """
+    aq_metadata_obj: MagicMock = get_aq_build_metadata(vm_data)
+
+    # We should first construct from an image
+    assert aq_metadata_obj == aq_metadata_class.from_dict.return_value
+    aq_metadata_class.from_dict.assert_called_once_with(
+        openstack_api.get_image.return_value.metadata
+    )
+
+    # Then override with an object
+    openstack_api.get_server_metadata.assert_called_once_with(vm_data)
+    aq_metadata_obj.override_from_vm_meta.assert_called_once_with(
+        openstack_api.get_server_metadata.return_value
+    )

OpenStack-Rabbit-Consumer/version.txt

@@ -1 +1 @@
-2.2.1
+2.3.0