From 2f6a62331c9be990d368661c088cfdaf2a77e2bd Mon Sep 17 00:00:00 2001 From: David Fairbrother Date: Thu, 8 Jun 2023 11:52:38 +0100 Subject: [PATCH 1/2] Bump rabbit consumer version Bumps the rabbit consumer version, following the changes to take VM metadata in preference to image metadata --- OpenStack-Rabbit-Consumer/version.txt | 2 +- charts/rabbit-consumer/Chart.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/OpenStack-Rabbit-Consumer/version.txt b/OpenStack-Rabbit-Consumer/version.txt index 276cbf9e..2bf1c1cc 100644 --- a/OpenStack-Rabbit-Consumer/version.txt +++ b/OpenStack-Rabbit-Consumer/version.txt @@ -1 +1 @@ -2.3.0 +2.3.1 diff --git a/charts/rabbit-consumer/Chart.yaml b/charts/rabbit-consumer/Chart.yaml index ea3f791a..dd02f1f9 100644 --- a/charts/rabbit-consumer/Chart.yaml +++ b/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.0 +version: 1.4.1 # 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.0" +appVersion: "v2.3.1" From 0af069051b942bdf99ce1b0df2f8e733bb393174 Mon Sep 17 00:00:00 2001 From: David Fairbrother Date: Thu, 8 Jun 2023 12:37:33 +0100 Subject: [PATCH 2/2] Delete hostname or delete network adapter in Aq The hostname implicitly will delete the network adapter, so we should shift this to an if-else rather than both always condition --- .../rabbit_consumer/message_consumer.py | 23 +++--- .../test/test_message_consumer.py | 79 ++++++++++++++++++- 2 files changed, 87 insertions(+), 15 deletions(-) diff --git a/OpenStack-Rabbit-Consumer/rabbit_consumer/message_consumer.py b/OpenStack-Rabbit-Consumer/rabbit_consumer/message_consumer.py index a4134d91..b844ed30 100644 --- a/OpenStack-Rabbit-Consumer/rabbit_consumer/message_consumer.py +++ b/OpenStack-Rabbit-Consumer/rabbit_consumer/message_consumer.py @@ -86,23 +86,24 @@ def delete_machine( # of deletion orders which it enforces... 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 + # we could have a machine which points to a different hostname 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) - - # We have to clean-up all the interfaces and addresses first - 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) - - if "eth0" in machine_details: - aq_api.delete_interface(machine_name) + else: + # Delete the interfaces + ipv4_address = socket.gethostbyname(hostname) + if ipv4_address in machine_details: + aq_api.delete_address(ipv4_address, machine_name) + + if "eth0" in machine_details: + aq_api.delete_interface(machine_name) logger.info("Machine exists for %s. Deleting old", vm_data.virtual_machine_id) diff --git a/OpenStack-Rabbit-Consumer/test/test_message_consumer.py b/OpenStack-Rabbit-Consumer/test/test_message_consumer.py index 2bcf9bd9..85689e80 100644 --- a/OpenStack-Rabbit-Consumer/test/test_message_consumer.py +++ b/OpenStack-Rabbit-Consumer/test/test_message_consumer.py @@ -22,6 +22,7 @@ check_machine_valid, is_aq_managed_image, get_aq_build_metadata, + delete_machine, ) from rabbit_consumer.vm_data import VmData @@ -221,7 +222,7 @@ def test_consume_create_machine_hostnames_good_path( patch( "rabbit_consumer.message_consumer.get_aq_build_metadata" ) as get_image_meta, - patch("rabbit_consumer.message_consumer.delete_machine") as delete_machine, + patch("rabbit_consumer.message_consumer.delete_machine") as delete_machine_mock, ): check_machine.return_value = True get_image_meta.return_value = image_metadata @@ -235,7 +236,7 @@ def test_consume_create_machine_hostnames_good_path( openstack.get_server_networks.assert_called_with(vm_data) # Check main Aq Flow - delete_machine.assert_called_once_with(vm_data, network_details[0]) + delete_machine_mock.assert_called_once_with(vm_data, network_details[0]) aq_api.create_machine.assert_called_once_with(rabbit_message, vm_data) machine_name = aq_api.create_machine.return_value @@ -255,7 +256,7 @@ def test_consume_create_machine_hostnames_good_path( @patch("rabbit_consumer.message_consumer.delete_machine") -def test_consume_delete_machine_good_path(delete_machine, rabbit_message): +def test_consume_delete_machine_good_path(delete_machine_mock, rabbit_message): """ Test that the function calls the correct functions in the correct order to delete a machine """ @@ -264,7 +265,9 @@ def test_consume_delete_machine_good_path(delete_machine, rabbit_message): with patch("rabbit_consumer.message_consumer.VmData") as data_patch: handle_machine_delete(rabbit_message) - delete_machine.assert_called_once_with(vm_data=data_patch.from_message.return_value) + delete_machine_mock.assert_called_once_with( + vm_data=data_patch.from_message.return_value + ) @patch("rabbit_consumer.message_consumer.is_aq_managed_image") @@ -361,3 +364,71 @@ def test_get_aq_build_metadata(openstack_api, aq_metadata_class, vm_data): aq_metadata_obj.override_from_vm_meta.assert_called_once_with( openstack_api.get_server_metadata.return_value ) + + +@patch("rabbit_consumer.message_consumer.aq_api") +def test_delete_machine_hostname_only(aq_api, vm_data, openstack_address): + """ + Tests that the function deletes a host then exits if no machine is found + """ + aq_api.check_host_exists.return_value = True + aq_api.search_machine_by_serial.return_value = None + + delete_machine(vm_data, openstack_address) + aq_api.delete_host.assert_called_once_with(openstack_address.hostname) + aq_api.delete_machine.assert_not_called() + + +@patch("rabbit_consumer.message_consumer.aq_api") +def test_delete_machine_by_serial(aq_api, vm_data, openstack_address): + """ + Tests that the function deletes a host then a machine + assuming both were found + """ + # Assume our host address doesn't match the machine record + # but the machine does have a hostname which is valid... + aq_api.check_host_exists.side_effect = [False, True] + + aq_api.search_host_by_machine.return_value = "host.example.com" + aq_api.get_machine_details.return_value = "" + + delete_machine(vm_data, openstack_address) + + aq_api.check_host_exists.assert_has_calls( + [call(openstack_address.hostname), call("host.example.com")] + ) + aq_api.delete_host.assert_called_once_with("host.example.com") + + +@patch("rabbit_consumer.message_consumer.aq_api") +@patch("rabbit_consumer.message_consumer.socket") +def test_delete_machine_no_hostname(socket_api, aq_api, vm_data): + aq_api.check_host_exists.return_value = False + + ip_address = "127.0.0.1" + socket_api.gethostbyname.return_value = ip_address + + machine_name = aq_api.search_machine_by_serial.return_value + aq_api.get_machine_details.return_value = f"eth0: {ip_address}" + + delete_machine(vm_data, NonCallableMock()) + aq_api.delete_address.assert_called_once_with(ip_address, machine_name) + aq_api.delete_interface.assert_called_once_with(machine_name) + + +@patch("rabbit_consumer.message_consumer.aq_api") +@patch("rabbit_consumer.message_consumer.socket") +def test_delete_machine_always_called(socket_api, aq_api, vm_data): + """ + Tests that the function always calls the delete machine function + """ + aq_api.check_host_exists.return_value = False + socket_api.gethostbyname.return_value = "123123" + + aq_api.get_machine_details.return_value = "Machine Details" + + machine_name = "machine_name" + aq_api.search_machine_by_serial.return_value = machine_name + + delete_machine(vm_data, NonCallableMock()) + aq_api.delete_machine.assert_called_once_with(machine_name)