Merge pull request #71 from stfc/Fix_wrong_ip_registration

Fix wrong ip registration

OpenStack-Rabbit-Consumer/rabbit_consumer/aq_api.py

@@ -191,23 +191,23 @@ def delete_host(hostname: str) -> None:
     setup_requests(url, "delete", "Host Delete")
 
 
-def delete_address(address: OpenstackAddress, machine_name: str) -> None:
+def delete_address(address: str, machine_name: str) -> None:
     """
     Deletes an address in Aquilon
     """
-    logger.debug("Attempting to delete address for %s ", address.addr)
+    logger.debug("Attempting to delete address for %s ", address)
     url = ConsumerConfig().aq_url + "/interface_address"
-    params = {"ip": address.addr, "machine": machine_name, "interface": "eth0"}
+    params = {"ip": address, "machine": machine_name, "interface": "eth0"}
     setup_requests(url, "delete", "Address Delete", params=params)
 
 
-def delete_interface(address: OpenstackAddress) -> None:
+def delete_interface(machine_name: str) -> None:
     """
     Deletes a host interface in Aquilon
     """
-    logger.debug("Attempting to delete interface for %s ", address.mac_addr)
+    logger.debug("Attempting to delete interface for %s ", machine_name)
     url = ConsumerConfig().aq_url + "/interface/command/del"
-    params = {"mac": address.mac_addr}
+    params = {"interface": "eth0", "machine": machine_name}
     setup_requests(url, "post", "Interface Delete", params=params)
 
 
@@ -247,13 +247,27 @@ def set_interface_bootable(machine_name: str, interface_name: str) -> None:
     setup_requests(url, "post", "Update Machine Interface")
 
 
-def search_machine(mac_addr: str) -> Optional[str]:
+def search_machine_by_serial(vm_data: VmData) -> Optional[str]:
     """
-    Searches for a machine in Aquilon based on a MAC address
+    Searches for a machine in Aquilon based on a serial number
     """
-    logger.debug("Searching for host with MAC %s", mac_addr)
+    logger.debug("Searching for host with serial %s", vm_data.virtual_machine_id)
     url = ConsumerConfig().aq_url + "/find/machine"
-    params = {"mac": mac_addr}
+    params = {"serial": vm_data.virtual_machine_id}
+    response = setup_requests(url, "get", "Search Host", params=params).strip()
+
+    if response:
+        return response
+    return None
+
+
+def search_host_by_machine(machine_name: str) -> Optional[str]:
+    """
+    Searches for a host in Aquilon based on a machine name
+    """
+    logger.debug("Searching for host with machine name %s", machine_name)
+    url = ConsumerConfig().aq_url + "/find/host"
+    params = {"machine": machine_name}
     response = setup_requests(url, "get", "Search Host", params=params).strip()
 
     if response:

OpenStack-Rabbit-Consumer/rabbit_consumer/image_metadata.py

@@ -1,10 +1,7 @@
-import dataclasses
 import logging
 from dataclasses import dataclass, field
-from typing import Optional, Type
 
 from mashumaro import DataClassDictMixin, field_options
-from mashumaro.mixins.json import T
 
 logger = logging.getLogger(__name__)
 
@@ -15,29 +12,9 @@ class ImageMetadata(DataClassDictMixin):
     Deserialised metadata that is set on OpenStack images
     """
 
-    aq_archetype: Optional[str] = field(metadata=field_options(alias="AQ_ARCHETYPE"))
-    aq_domain: Optional[str] = field(metadata=field_options(alias="AQ_DOMAIN"))
+    aq_archetype: str = field(metadata=field_options(alias="AQ_ARCHETYPE"))
+    aq_domain: str = field(metadata=field_options(alias="AQ_DOMAIN"))
 
-    aq_personality: Optional[str] = field(
-        metadata=field_options(alias="AQ_PERSONALITY")
-    )
-    aq_os_version: Optional[str] = field(metadata=field_options(alias="AQ_OSVERSION"))
-    aq_os: Optional[str] = field(metadata=field_options(alias="AQ_OS"))
-
-    @classmethod
-    def __post_deserialize__(
-        cls: Type[T], obj: "ImageMetadata"
-    ) -> Optional["ImageMetadata"]:
-        """
-        Post de-serialisation hook to check for missing fields
-        """
-        fields = dataclasses.fields(obj)
-        if not any(getattr(obj, f.name) for f in fields):
-            # This doesn't have any fields set, so we can't validate it
-            return None
-
-        if not all(getattr(obj, f.name) for f in fields):
-            logger.error("Missing data for on object %s", obj)
-            return None
-
-        return obj
+    aq_personality: str = field(metadata=field_options(alias="AQ_PERSONALITY"))
+    aq_os_version: str = field(metadata=field_options(alias="AQ_OSVERSION"))
+    aq_os: str = field(metadata=field_options(alias="AQ_OS"))

OpenStack-Rabbit-Consumer/rabbit_consumer/message_consumer.py

@@ -1,5 +1,6 @@
 import json
 import logging
+import socket
 from typing import Optional, List
 
 import rabbitpy
@@ -25,12 +26,12 @@ def is_aq_managed_image(rabbit_message: RabbitMessage) -> Optional[ImageMetadata
     Check to see if the metadata in the message contains entries that suggest it
     is for an Aquilon VM.
     """
-    image = openstack_api.get_image_name(VmData.from_message(rabbit_message))
-    image_meta = ImageMetadata.from_dict(image.metadata)
-
-    if not image_meta:
+    image = openstack_api.get_image(VmData.from_message(rabbit_message))
+    if "AQ_OS" not in image.metadata:
         logger.debug("Skipping non-Aquilon image: %s", image.name)
         return None
+
+    image_meta = ImageMetadata.from_dict(image.metadata)
     return image_meta
 
 
@@ -49,49 +50,86 @@ def consume(message: RabbitMessage) -> None:
         raise ValueError(f"Unsupported message type: {message.event_type}")
 
 
-def delete_machine(addresses: List[OpenstackAddress]):
-    for address in addresses:
-        if aq_api.check_host_exists(address.hostname):
-            logger.info("Host exists for %s. Deleting old", address.hostname)
-            aq_api.delete_host(address.hostname)
+def delete_machine(
+    vm_data: VmData, network_details: Optional[OpenstackAddress] = None
+) -> None:
+    """
+    Deletes a machine in Aquilon and all associated addresses based on
+    the serial, MAC and hostname provided. This is the best effort attempt
+    to clean-up, since we can have partial or incorrect information.
+    """
+    # First handle hostnames
+    if network_details and aq_api.check_host_exists(network_details.hostname):
+        logger.info("Deleting host %s", network_details.hostname)
+        aq_api.delete_host(network_details.hostname)
+
+    machine_name = aq_api.search_machine_by_serial(vm_data)
+    if not machine_name:
+        logger.info("No existing record found for %s", vm_data.virtual_machine_id)
+        return
+
+    # We have to do this manually because AQ has neither a:
+    # - Just delete the machine please
+    # - Delete this if it exists
+    # So alas we have to do everything by hand, whilst adhering to random rules
+    # of deletion orders which it enforces...
 
-        machine_name = aq_api.search_machine(address.mac_addr)
-        if not machine_name:
-            # Nothing else to do at this point
-            return
+    hostname = aq_api.search_host_by_machine(machine_name)
+    machine_details = aq_api.get_machine_details(machine_name)
+    # We have to clean-up all the interfaces and addresses first
+    if hostname:
+        if aq_api.check_host_exists(hostname):
+            # This is a different hostname to the one we have in the message
+            # so, we need to delete it
+            logger.info("Host exists for %s. Deleting old", hostname)
+            aq_api.delete_host(hostname)
 
-        logger.info("Machine exists for MAC: %s. Deleting old", address.mac_addr)
-        machine_details = aq_api.get_machine_details(machine_name)
+        # First delete the interfaces
+        ipv4_address = socket.gethostbyname(hostname)
+        if ipv4_address in machine_details:
+            aq_api.delete_address(ipv4_address, machine_name)
 
-        # We have to do this manually because AQ has neither a:
-        # - Just delete the machine please
-        # - Delete this if it exists
-        # So alas we have to do everything by hand, whilst adhering to random rules
-        # of deletion orders which it enforces...
+        if "eth0" in machine_details:
+            aq_api.delete_interface(machine_name)
 
-        if address.addr in machine_details:
-            aq_api.delete_address(address, machine_name)
-        if address.mac_addr in machine_details:
-            aq_api.delete_interface(address)
-        aq_api.delete_machine(machine_name)
+    logger.info("Machine exists for %s. Deleting old", vm_data.virtual_machine_id)
 
+    # Then delete the machine
+    aq_api.delete_machine(machine_name)
 
-def handle_create_machine(rabbit_message: RabbitMessage) -> None:
+
+def check_machine_valid(rabbit_message: RabbitMessage) -> bool:
     """
-    Handles the creation of a machine in Aquilon. This includes
-    creating the machine, adding the nics, and managing the host.
+    Checks to see if the machine is valid for creating in Aquilon.
     """
-    logger.info("=== Received Aquilon VM create message ===")
-    _print_debug_logging(rabbit_message)
-
     vm_data = VmData.from_message(rabbit_message)
     if not openstack_api.check_machine_exists(vm_data):
         # User has likely deleted the machine since we got here
         logger.warning(
             "Machine %s does not exist, skipping creation", vm_data.virtual_machine_id
         )
+        return False
+
+    if not is_aq_managed_image(rabbit_message):
+        logger.debug("Ignoring non AQ Image: %s", rabbit_message)
+        return False
+
+    return True
+
+
+def handle_create_machine(rabbit_message: RabbitMessage) -> None:
+    """
+    Handles the creation of a machine in Aquilon. This includes
+    creating the machine, adding the nics, and managing the host.
+    """
+    logger.info("=== Received Aquilon VM create message ===")
+    _print_debug_logging(rabbit_message)
+
+    if not check_machine_valid(rabbit_message):
         return
 
+    vm_data = VmData.from_message(rabbit_message)
+
     image_meta = is_aq_managed_image(rabbit_message)
     network_details = openstack_api.get_server_networks(vm_data)
 
@@ -100,7 +138,7 @@ def handle_create_machine(rabbit_message: RabbitMessage) -> None:
         logger.info("Skipping novalocal only host: %s", vm_name)
         return
 
-    delete_machine(network_details)
+    delete_machine(vm_data, network_details[0])
 
     # Configure networking
     machine_name = aq_api.create_machine(rabbit_message, vm_data)
@@ -128,7 +166,7 @@ def _print_debug_logging(rabbit_message: RabbitMessage) -> None:
     logger.debug(
         "Project Name: %s (%s)", rabbit_message.project_name, vm_data.project_id
     )
-    logger.debug(
+    logger.info(
         "VM Name: %s (%s) ", rabbit_message.payload.vm_name, vm_data.virtual_machine_id
     )
     logger.debug("Username: %s", rabbit_message.user_name)
@@ -143,14 +181,7 @@ def handle_machine_delete(rabbit_message: RabbitMessage) -> None:
     _print_debug_logging(rabbit_message)
 
     vm_data = VmData.from_message(rabbit_message)
-    network_data = openstack_api.get_server_networks(vm_data)
-
-    if not network_data or not network_data[0].hostname:
-        vm_name = rabbit_message.payload.vm_name
-        logger.debug("No hostnames found for %s, skipping delete", vm_name)
-        return
-
-    delete_machine(addresses=network_data)
+    delete_machine(vm_data=vm_data)
 
     logger.info(
         "=== Finished Aquilon deletion hook for VM %s ===", vm_data.virtual_machine_id
@@ -193,11 +224,6 @@ def on_message(message: rabbitpy.Message) -> None:
     decoded = RabbitMessage.from_json(body)
     logger.debug("Decoded message: %s", decoded)
 
-    if not is_aq_managed_image(decoded):
-        logger.debug("Ignoring non AQ Image: %s", decoded)
-        message.ack()
-        return
-
     consume(decoded)
     message.ack()
 

OpenStack-Rabbit-Consumer/rabbit_consumer/openstack_api.py

@@ -18,8 +18,7 @@ class OpenstackConnection:
     in subsequent functions.
     """
 
-    def __init__(self, project_name: str):
-        self.project_name = project_name
+    def __init__(self):
         self.conn = None
 
     def __enter__(self):
@@ -41,18 +40,20 @@ def check_machine_exists(vm_data: VmData) -> bool:
     """
     Checks to see if the machine exists in Openstack.
     """
-    with OpenstackConnection(vm_data.project_id) as conn:
+    with OpenstackConnection() as conn:
         return bool(conn.compute.find_server(vm_data.virtual_machine_id))
 
 
 def get_server_details(vm_data: VmData) -> Server:
     """
     Gets the server details from Openstack with details included
     """
-    with OpenstackConnection(vm_data.project_id) as conn:
+    with OpenstackConnection() as conn:
         # Workaround for details missing from find_server
         # on the current version of openstacksdk
-        found = list(conn.compute.servers(vm_data.virtual_machine_id))
+        found = list(
+            conn.compute.servers(uuid=vm_data.virtual_machine_id, all_projects=True)
+        )
         if not found:
             raise ValueError(f"Server not found for id: {vm_data.virtual_machine_id}")
         return found[0]
@@ -64,6 +65,9 @@ def get_server_networks(vm_data: VmData) -> List[OpenstackAddress]:
     of deserialized OpenstackAddresses.
     """
     server = get_server_details(vm_data)
+    if "Internal" not in server.addresses:
+        logger.warning("No internal network found for server %s", server.name)
+        return []
     return OpenstackAddress.get_internal_networks(server.addresses)
 
 
@@ -75,13 +79,13 @@ def get_metadata(vm_data: VmData) -> dict:
     return server.metadata
 
 
-def get_image_name(vm_data: VmData) -> Image:
+def get_image(vm_data: VmData) -> Image:
     """
     Gets the image name from Openstack for the virtual machine.
     """
     server = get_server_details(vm_data)
     uuid = server.image.id
-    with OpenstackConnection(vm_data.project_id) as conn:
+    with OpenstackConnection() as conn:
         image = conn.compute.find_image(uuid)
         return image
 
@@ -91,7 +95,7 @@ def update_metadata(vm_data: VmData, metadata) -> None:
     Updates the metadata for the virtual machine.
     """
     server = get_server_details(vm_data)
-    with OpenstackConnection(vm_data.project_id) as conn:
+    with OpenstackConnection() as conn:
         conn.compute.set_server_metadata(server, **metadata)
 
     logger.debug("Setting metadata successful")