Merge pull request #84 from stfc/Handle_sandboxes_v2

Handle sandboxes v2

Author: meoflynn

OpenStack-Rabbit-Consumer/rabbit_consumer/aq_api.py

@@ -13,8 +13,6 @@
 from rabbit_consumer.rabbit_message import RabbitMessage
 from rabbit_consumer.vm_data import VmData
 
-MAKE_SUFFIX = "/host/{0}/command/make"
-
 HOST_CHECK_SUFFIX = "/host/{0}"
 
 UPDATE_INTERFACE_SUFFIX = "/machine/{0}/interface/{1}?boot&default_route"
@@ -84,21 +82,11 @@ def setup_requests(
     return response.text
 
 
-def aq_make(addresses: List[OpenstackAddress], image_meta: AqMetadata) -> None:
+def aq_make(addresses: List[OpenstackAddress]) -> None:
     """
-    Runs AQ make against a list of addresses passed to build the default personality
+    Runs AQ make against a list of addresses passed to refresh
+    the given host
     """
-    params = {
-        "personality": image_meta.aq_personality,
-        "osversion": image_meta.aq_os_version,
-        "osname": image_meta.aq_os,
-        "archetype": image_meta.aq_archetype,
-    }
-
-    assert all(
-        i for i in params.values()
-    ), "Some fields were not set in the OS description"
-
     # Manage and make these back to default domain and personality
     address = addresses[0]
     hostname = address.hostname
@@ -107,8 +95,8 @@ def aq_make(addresses: List[OpenstackAddress], image_meta: AqMetadata) -> None:
     if not hostname or not hostname.strip():
         raise ValueError("Hostname cannot be empty")
 
-    url = ConsumerConfig().aq_url + MAKE_SUFFIX.format(hostname)
-    setup_requests(url, "post", "Make Template: ", params)
+    url = ConsumerConfig().aq_url + f"/host/{hostname}/command/make"
+    setup_requests(url, "post", "Make Template")
 
 
 def aq_manage(addresses: List[OpenstackAddress], image_meta: AqMetadata) -> None:
@@ -121,9 +109,13 @@ def aq_manage(addresses: List[OpenstackAddress], image_meta: AqMetadata) -> None
 
     params = {
         "hostname": hostname,
-        "domain": image_meta.aq_domain,
         "force": True,
     }
+    if image_meta.aq_sandbox:
+        params["sandbox"] = image_meta.aq_sandbox
+    else:
+        params["domain"] = image_meta.aq_domain
+
     url = ConsumerConfig().aq_url + f"/host/{hostname}/command/manage"
     setup_requests(url, "post", "Manage Host", params=params)
 
@@ -158,14 +150,6 @@ def delete_machine(machine_name: str) -> None:
     setup_requests(url, "delete", "Delete Machine")
 
 
-def _is_sandbox(image_meta: AqMetadata) -> bool:
-    """
-    Returns True if the image is a sandbox image
-    we do a simple test for a '/' in the domain name
-    """
-    return "/" in image_meta.aq_domain
-
-
 def create_host(
     image_meta: AqMetadata, addresses: List[OpenstackAddress], machine_name: str
 ) -> None:
@@ -184,8 +168,8 @@ def create_host(
         "osversion": image_meta.aq_os_version,
     }
 
-    if _is_sandbox(image_meta):
-        params["sandbox"] = image_meta.aq_domain
+    if image_meta.aq_sandbox:
+        params["sandbox"] = image_meta.aq_sandbox
     else:
         params["domain"] = image_meta.aq_domain
 

OpenStack-Rabbit-Consumer/rabbit_consumer/aq_metadata.py

@@ -1,6 +1,6 @@
 import logging
 from dataclasses import dataclass
-from typing import Dict
+from typing import Dict, Optional
 
 from mashumaro import DataClassDictMixin
 from mashumaro.config import BaseConfig
@@ -16,13 +16,14 @@ class AqMetadata(DataClassDictMixin):
     """
 
     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
 
+    aq_sandbox: Optional[str] = None
+
     # pylint: disable=too-few-public-methods
     class Config(BaseConfig):
         """
@@ -32,6 +33,7 @@ class Config(BaseConfig):
         aliases = {
             "aq_archetype": "AQ_ARCHETYPE",
             "aq_domain": "AQ_DOMAIN",
+            "aq_sandbox": "AQ_SANDBOX",
             "aq_personality": "AQ_PERSONALITY",
             "aq_os_version": "AQ_OSVERSION",
             "aq_os": "AQ_OS",
@@ -45,6 +47,3 @@ def override_from_vm_meta(self, vm_meta: Dict[str, str]):
         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

@@ -160,9 +160,7 @@ def handle_create_machine(rabbit_message: RabbitMessage) -> None:
 
     # Manage host in Aquilon
     aq_api.create_host(image_meta, network_details, machine_name)
-
-    aq_api.aq_manage(network_details, image_meta)
-    aq_api.aq_make(network_details, image_meta)
+    aq_api.aq_make(network_details)
 
     add_aq_details_to_metadata(vm_data, network_details)
 

OpenStack-Rabbit-Consumer/test/test_aq_api.py

@@ -130,45 +130,17 @@ def test_setup_requests_rest_methods(_, kerb_auth, requests, rest_verb):
 
 @patch("rabbit_consumer.aq_api.setup_requests")
 @patch("rabbit_consumer.aq_api.ConsumerConfig")
-def test_aq_make_calls(config, setup, openstack_address_list, image_metadata):
+def test_aq_make_calls(config, setup, openstack_address_list):
     """
     Test that aq_make calls the correct URLs with the correct parameters
     """
     domain = "domain"
     config.return_value.aq_url = domain
 
-    aq_make(openstack_address_list, image_metadata)
-
-    expected_params = {
-        "personality": image_metadata.aq_personality,
-        "osversion": image_metadata.aq_os_version,
-        "osname": image_metadata.aq_os,
-        "archetype": image_metadata.aq_archetype,
-    }
+    aq_make(openstack_address_list)
 
     expected_url = f"{domain}/host/{openstack_address_list[0].hostname}/command/make"
-    setup.assert_called_once_with(expected_url, "post", mock.ANY, expected_params)
-
-
-@pytest.mark.parametrize(
-    "field_to_blank",
-    [
-        "aq_personality",
-        "aq_os_version",
-        "aq_os",
-    ],
-)
-def test_aq_make_missing_fields(field_to_blank, openstack_address_list, image_metadata):
-    """
-    Test that aq_make throws an exception when a required field is missing
-    """
-    with pytest.raises(AssertionError):
-        setattr(image_metadata, field_to_blank, None)
-        aq_make(openstack_address_list, image_metadata)
-
-    with pytest.raises(AssertionError):
-        setattr(image_metadata, field_to_blank, "")
-        aq_make(openstack_address_list, image_metadata)
+    setup.assert_called_once_with(expected_url, "post", mock.ANY)
 
 
 @pytest.mark.parametrize("hostname", ["  ", "", None])
@@ -185,7 +157,7 @@ def test_aq_make_none_hostname(config, setup, openstack_address, hostname):
     address.hostname = hostname
 
     with pytest.raises(ValueError):
-        aq_make([address], NonCallableMock())
+        aq_make([address])
 
     setup.assert_not_called()
 
@@ -211,6 +183,30 @@ def test_aq_manage(config, setup, openstack_address_list, image_metadata):
     setup.assert_called_once_with(expected_url, "post", mock.ANY, params=expected_param)
 
 
+@patch("rabbit_consumer.aq_api.setup_requests")
+@patch("rabbit_consumer.aq_api.ConsumerConfig")
+def test_aq_manage_with_sandbox(config, setup, openstack_address_list, image_metadata):
+    """
+    Test that aq_manage calls the correct URLs with the sandbox
+    instead of the domain
+    """
+    config.return_value.aq_url = "https://example.com"
+
+    image_metadata.aq_sandbox = "some_sandbox"
+
+    aq_manage(openstack_address_list, image_metadata)
+    address = openstack_address_list[0]
+
+    expected_param = {
+        "hostname": address.hostname,
+        "sandbox": image_metadata.aq_sandbox,
+        "force": True,
+    }
+
+    expected_url = f"https://example.com/host/{address.hostname}/command/manage"
+    setup.assert_called_once_with(expected_url, "post", mock.ANY, params=expected_param)
+
+
 @patch("rabbit_consumer.aq_api.ConsumerConfig")
 @patch("rabbit_consumer.aq_api.setup_requests")
 def test_aq_create_machine(setup, config, rabbit_message, vm_data):
@@ -292,7 +288,8 @@ def test_aq_create_host_with_sandbox(
     env_config = config.return_value
     env_config.aq_url = "https://example.com"
 
-    image_metadata.aq_domain = "example/sandbox"
+    image_metadata.aq_domain = "example_domain"
+    image_metadata.aq_sandbox = "example/sandbox"
 
     create_host(image_metadata, openstack_address_list, machine_name)
     address = openstack_address_list[0]
@@ -304,7 +301,7 @@ def test_aq_create_host_with_sandbox(
         "personality": image_metadata.aq_personality,
         "osname": image_metadata.aq_os,
         "osversion": image_metadata.aq_os_version,
-        "sandbox": image_metadata.aq_domain,
+        "sandbox": image_metadata.aq_sandbox,
     }
 
     expected_url = f"https://example.com/host/{address.hostname}"

OpenStack-Rabbit-Consumer/test/test_aq_metadata.py

@@ -55,10 +55,10 @@ def test_aq_metadata_override_all(image_metadata):
     assert returned.aq_os_version == "osversion_mock"
 
 
-def test_aq_metadata_override_sandbox(image_metadata):
+def test_aq_metadata_sandbox(image_metadata):
     """
-    Tests overriding the sandbox value in an AQ metadata object
-    maps correctly onto the domain value
+    Tests the sandbox value in an AQ metadata object
+    maps correctly onto the sandbox value
     """
     returned = AqMetadata.from_dict(image_metadata)
     returned.override_from_vm_meta(
@@ -67,7 +67,7 @@ def test_aq_metadata_override_sandbox(image_metadata):
         }
     )
     # This should be the only value that has changed
-    assert returned.aq_domain == "sandbox_mock"
+    assert returned.aq_sandbox == "sandbox_mock"
 
     assert returned.aq_archetype == "archetype_mock"
     assert returned.aq_personality == "personality_mock"

OpenStack-Rabbit-Consumer/test/test_message_consumer.py

@@ -248,8 +248,7 @@ def test_consume_create_machine_hostnames_good_path(
     aq_api.create_host.assert_called_once_with(
         image_metadata, network_details, machine_name
     )
-    aq_api.aq_manage.assert_called_once_with(network_details, image_metadata)
-    aq_api.aq_make.assert_called_once_with(network_details, image_metadata)
+    aq_api.aq_make.assert_called_once_with(network_details)
 
     # Metadata
     metadata.assert_called_once_with(vm_data, network_details)

OpenStack-Rabbit-Consumer/version.txt

@@ -1 +1 @@
-2.3.3
+2.3.4

charts/rabbit-consumer/Chart.yaml

@@ -6,11 +6,11 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 1.4.3
+version: 1.4.4
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
 # It is recommended to use it with quotes.
-appVersion: "v2.3.3"
+appVersion: "v2.3.4"