From c71192989a1d45a7c3fc39f685b6c91a9f36e3b9 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 24 Oct 2024 14:10:39 -0700 Subject: [PATCH] Fix openstack region handling * Remove the unused connection region_name attribute * Make the cloud name connection parameter required * Handle the case where a user does not specify the region and we use the sdk default region * Set the node region from the actual region, not the unused connection region * Verify that we get both variables set on the resulting node Change-Id: Ic9356b1ca0b674f99296dd5f2ca4e5790d5ce55c --- tests/fake_openstack.py | 1 + tests/fixtures/zuul-connections-nodepool.conf | 1 + tests/unit/test_openstack_driver.py | 5 +++++ zuul/driver/openstack/openstackconnection.py | 4 +++- zuul/driver/openstack/openstackendpoint.py | 10 ++++++++-- 5 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/fake_openstack.py b/tests/fake_openstack.py index cd0b5d9682..72a9c42342 100644 --- a/tests/fake_openstack.py +++ b/tests/fake_openstack.py @@ -147,6 +147,7 @@ class FakeOpenstackConnection: self.config = FakeOpenstackConfig() self.config.config = {} self.config.config['image_format'] = 'qcow2' + self.config.config['region_name'] = 'region1' def _needs_floating_ip(self, server, nat_destination): return self.cloud._fake_needs_floating_ip diff --git a/tests/fixtures/zuul-connections-nodepool.conf b/tests/fixtures/zuul-connections-nodepool.conf index f4c2835b33..a2058ed487 100644 --- a/tests/fixtures/zuul-connections-nodepool.conf +++ b/tests/fixtures/zuul-connections-nodepool.conf @@ -43,3 +43,4 @@ secret_access_key=fake [connection openstack] driver=openstack +cloud=fakecloud diff --git a/tests/unit/test_openstack_driver.py b/tests/unit/test_openstack_driver.py index cd7408f9bc..0484743094 100644 --- a/tests/unit/test_openstack_driver.py +++ b/tests/unit/test_openstack_driver.py @@ -77,6 +77,11 @@ class BaseOpenstackDriverTest(ZuulTestCase): class TestOpenstackDriver(BaseOpenstackDriverTest, BaseCloudDriverTest): + def _assertProviderNodeAttributes(self, pnode): + super()._assertProviderNodeAttributes(pnode) + self.assertEqual('fakecloud', pnode.cloud) + self.assertEqual('region1', pnode.region) + @simple_layout('layouts/openstack/nodepool.yaml', enable_nodepool=True) def test_openstack_node_lifecycle(self): self._test_node_lifecycle('debian-normal') diff --git a/zuul/driver/openstack/openstackconnection.py b/zuul/driver/openstack/openstackconnection.py index 6624a807e1..e4f50198cc 100644 --- a/zuul/driver/openstack/openstackconnection.py +++ b/zuul/driver/openstack/openstackconnection.py @@ -32,8 +32,10 @@ class OpenstackConnection(BaseConnection): 'client_config_file', os.getenv('OS_CLIENT_CONFIG_FILE', None)) + if 'cloud' not in self.connection_config: + raise Exception('The "cloud" parameter is required for ' + f'OpenStack connections in {self.connection_name}') self.cloud_name = self.connection_config.get('cloud') - self.region_name = self.connection_config.get('region') # Rate limit: requests/second self.rate = self.connection_config.get('rate', 2) diff --git a/zuul/driver/openstack/openstackendpoint.py b/zuul/driver/openstack/openstackendpoint.py index 50ad4964c4..bc68ee285d 100644 --- a/zuul/driver/openstack/openstackendpoint.py +++ b/zuul/driver/openstack/openstackendpoint.py @@ -564,7 +564,7 @@ class OpenstackProviderEndpoint(BaseProviderEndpoint): def _getInstance(self, server, quota): return OpenstackInstance( self.connection.cloud_name, - self.connection.region_name, + self.getRegionName(), server, quota) def _getClient(self): @@ -574,7 +574,7 @@ class OpenstackProviderEndpoint(BaseProviderEndpoint): app_name='zuul', ) region = config.get_one(cloud=self.connection.cloud_name, - region_name=self.connection.region_name) + region_name=self.region) return openstack.connection.Connection( config=region, use_direct_get=False, @@ -584,6 +584,12 @@ class OpenstackProviderEndpoint(BaseProviderEndpoint): def getImageFormat(self): return self._client.config.config['image_format'] + def getRegionName(self): + # With OpenStackSDK, users can omit the region and the SDK may + # supply a default region. This helper method will return the + # actual region in use regardless of what the user supplied. + return self._client.config.config['region_name'] + def _submitApi(self, api, *args, **kw): return self.api_executor.submit( api, *args, **kw)