From 71ff8766b44e92dc0912a1ef76b69621869ef936 Mon Sep 17 00:00:00 2001 From: "Oleksandr K." Date: Fri, 10 Jan 2025 09:15:26 -0800 Subject: [PATCH] MCAPI-56 Fix network filter for fixed_network support (#473) --- .gitignore | 1 + magnum_cluster_api/resources.py | 42 ++++++------ magnum_cluster_api/tests/unit/test_utils.py | 75 +++++++++++++++++++++ magnum_cluster_api/utils.py | 9 +++ 4 files changed, 106 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 2bd4a625..de1d386d 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ dist site *.orig *.rej +.tox diff --git a/magnum_cluster_api/resources.py b/magnum_cluster_api/resources.py index 7c917f96..2aa20f68 100644 --- a/magnum_cluster_api/resources.py +++ b/magnum_cluster_api/resources.py @@ -1175,7 +1175,7 @@ def get_object(self) -> objects.ClusterClass: }, }, { - "name": "fixedNetworkName", + "name": "fixedNetworkId", "required": True, "schema": { "openAPIV3Schema": { @@ -1888,17 +1888,6 @@ def get_object(self) -> objects.ClusterClass: "variable": "disableAPIServerFloatingIP" }, }, - { - "op": "add", - "path": "/spec/template/spec/managedSubnets", - "valueFrom": { - "template": textwrap.dedent( - """\ - - dnsNameservers: {{ .dnsNameservers }} - """ - ), - }, - }, { "op": "add", "path": "/spec/template/spec/externalNetwork", @@ -1940,7 +1929,7 @@ def get_object(self) -> objects.ClusterClass: }, { "name": "newNetworkConfig", - "enabledIf": '{{ if eq .fixedNetworkName "" }}true{{end}}', + "enabledIf": '{{ if eq .fixedNetworkId "" }}true{{end}}', "definitions": [ { "selector": { @@ -1953,16 +1942,23 @@ def get_object(self) -> objects.ClusterClass: "jsonPatches": [ { "op": "add", - "path": "/spec/template/spec/managedSubnets/0/cidr", - "valueFrom": {"variable": "nodeCidr"}, + "path": "/spec/template/spec/managedSubnets", + "valueFrom": { + "template": textwrap.dedent( + """\ + - cidr: {{ .nodeCidr }} + dnsNameservers: {{ .dnsNameservers }} + """ + ), + }, }, ], }, ], }, { - "name": "existingFixedNetworkNameConfig", - "enabledIf": '{{ if ne .fixedNetworkName "" }}true{{end}}', + "name": "existingFixedNetworkIdConfig", + "enabledIf": '{{ if ne .fixedNetworkId "" }}true{{end}}', "definitions": [ { "selector": { @@ -1975,9 +1971,13 @@ def get_object(self) -> objects.ClusterClass: "jsonPatches": [ { "op": "add", - "path": "/spec/template/spec/network/name", + "path": "/spec/template/spec/network", "valueFrom": { - "variable": "fixedNetworkName" + "template": textwrap.dedent( + """\ + id: {{ .fixedNetworkId }} + """ + ), }, }, ], @@ -2766,8 +2766,8 @@ def get_object(self) -> objects.Cluster: ), }, { - "name": "fixedNetworkName", - "value": neutron.get_fixed_network_name( + "name": "fixedNetworkId", + "value": utils.get_fixed_network_id( self.context, self.cluster.fixed_network ) or "", diff --git a/magnum_cluster_api/tests/unit/test_utils.py b/magnum_cluster_api/tests/unit/test_utils.py index 0c8b9ca1..5e4d41cc 100644 --- a/magnum_cluster_api/tests/unit/test_utils.py +++ b/magnum_cluster_api/tests/unit/test_utils.py @@ -13,12 +13,16 @@ # under the License. import textwrap +from unittest import mock import pykube import pytest import responses +from magnum.common import exception from magnum.tests.unit.objects import utils as magnum_test_utils # type: ignore from oslo_serialization import base64, jsonutils +from oslo_utils import uuidutils +from oslotest import base from magnum_cluster_api import exceptions, utils @@ -240,3 +244,74 @@ def test_generate_cloud_controller_manager_config_for_ovn_with_invalid_algorithm utils.generate_cloud_controller_manager_config( self.context, self.pykube_api, self.cluster ) + + +class TestUtils(base.BaseTestCase): + """Test case for utils.""" + + @mock.patch("magnum.common.neutron.get_network") + def test_get_fixed_network_id_with_uuid(self, mock_get_network): + context = mock.Mock() + fixed_network = uuidutils.generate_uuid() + + network = utils.get_fixed_network_id(context, fixed_network) + + mock_get_network.assert_not_called() + self.assertEqual(fixed_network, network) + + @mock.patch("magnum.common.neutron.get_network") + def test_get_fixed_network_id_with_name(self, mock_get_network): + context = mock.Mock() + fixed_network = "fake-network" + + network_id = uuidutils.generate_uuid() + mock_get_network.return_value = network_id + + network = utils.get_fixed_network_id(context, fixed_network) + + mock_get_network.assert_called_once_with( + context, fixed_network, source="name", target="id", external=False + ) + self.assertEqual(network_id, network) + + @mock.patch("magnum.common.neutron.get_network") + def test_get_fixed_network_id_with_no_fixed_network(self, mock_get_network): + context = mock.Mock() + + network = utils.get_fixed_network_id(context, None) + + mock_get_network.assert_not_called() + self.assertEqual(None, network) + + @mock.patch("magnum.common.neutron.get_network") + def test_get_fixed_network_id_with_missing_network(self, mock_get_network): + context = mock.Mock() + fixed_network = "fake-network" + + mock_get_network.side_effect = exception.FixedNetworkNotFound( + network=fixed_network + ) + + self.assertRaises( + exception.FixedNetworkNotFound, + utils.get_fixed_network_id, + context, + fixed_network, + ) + + @mock.patch("magnum.common.neutron.get_network") + def test_get_fixed_network_id_with_multiple_networks(self, mock_get_network): + context = mock.Mock() + fixed_network = "fake-network" + + mock_get_network.side_effect = exception.Conflict( + "Multiple networks exist with same name '%s'. Please use the " + "network ID instead." % fixed_network + ) + + self.assertRaises( + exception.Conflict, + utils.get_fixed_network_id, + context, + fixed_network, + ) diff --git a/magnum_cluster_api/utils.py b/magnum_cluster_api/utils.py index 1f27a712..108b0e8b 100644 --- a/magnum_cluster_api/utils.py +++ b/magnum_cluster_api/utils.py @@ -673,3 +673,12 @@ def _delete_server_group( osc.nova().server_groups.delete(server_group_id) except nova_exception.NotFound: return + + +def get_fixed_network_id(context, network): + if network and not uuidutils.is_uuid_like(network): + return neutron.get_network( + context, network, source="name", target="id", external=False + ) + else: + return network