openstack/adjutant
Code Issues Proposed changes

Allow default networks in multiple regions

Browse Source 
This adds the optional `create_in_regions` and `create_in_all_regions`
parameters to the `NewDefaultNetworkAction` and the
`NewProjectDefaultNetworkAction` actions.

When `create_in_regions` is set, the action will ignore the default
region set when creating a project, and instead create default networks
and routers for each region set in the `create_in_regions` list.
When `create_in_regions` is unset (or set to an empty list),
the actions will default to the behaviour dictated by
`create_in_all_regions`.

When `create_in_all_regions` is set to `False` (the default),
a default network is created in the default region only, as it
always has. When it is set to `True`, it will instead create
default networks in *all* active cloud regions,
as discovered by Adjutant.

Improve error handling in `NewDefaultNetworkAction` to give more
detail in the task log when resource creation fails.

Fix bugs in the fake Neutron client in the unit tests where created
resources were hard-coded to be added to the `RegionOne` region.

Change-Id: I0e0885dda449b22788bd4c9ae72b874d8ce661b5
This commit is contained in:
Callum Dickinson 2024-06-25 14:10:13 +12:00
parent c2cb1dafda
commit 69f95cc5bb
4 changed files with 445 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

181
adjutant/actions/v1/resources.py
View File

@ -83,6 +83,28 @@ class NewDefaultNetworkAction(BaseAction, ProjectMixin):
"See 'region_defaults'.",
default={},
),
fields.ListConfig(
"create_in_regions",
help_text=(
"Set the regions in which a default network will be "
"created. When unset or empty, the default behaviour "
"depends on the value of 'create_in_all_regions'."
),
default=[],
required=False,
),
fields.BoolConfig(
"create_in_all_regions",
help_text=(
"When set to False (the default), if no regions are set "
"in 'create_in_regions', a default network will only be "
"created in the default region for new sign-ups. "
"When set to True, a default network will be created in "
"all regions, not just the default region."
),
default=False,
required=False,
),
]
)
@ -114,14 +136,34 @@ class NewDefaultNetworkAction(BaseAction, ProjectMixin):
self.action.save()
def _create_network(self):
neutron = openstack_clients.get_neutronclient(region=self.region)
if self.config.create_in_regions:
for region in self.config.create_in_regions:
self._create_network_in_region(region)
elif self.config.create_in_all_regions:
id_manager = user_store.IdentityManager()
for region in id_manager.list_regions():
region_id = region.id
self._create_network_in_region(region_id)
else:
self._create_network_in_region(self.region)
def _create_network_in_region(self, region):
neutron = openstack_clients.get_neutronclient(region=region)
try:
region_config = self.config.regions[self.region]
region_config = self.config.regions[region]
network_config = self.config.region_defaults.overlay(region_config)
except KeyError:
network_config = self.config.region_defaults
if not self.get_cache("network_id"):
cache_suffix = f"_{region}"
is_default_region = region == self.region
network_id = self.get_cache(f"network_id{cache_suffix}")
# NOTE(callumdickinson): Backwards compatibility with older tasks.
if not network_id and is_default_region:
network_id = self.get_cache("network_id")
# If the default network does not exist, create it.
if not network_id:
try:
network_body = {
"network": {
@ -133,26 +175,39 @@ class NewDefaultNetworkAction(BaseAction, ProjectMixin):
network = neutron.create_network(body=network_body)
except Exception as e:
self.add_note(
"Error: '%s' while creating network: %s"
% (e, network_config.network_name)
(
f"Error while creating network '{network_config.network_name}' "
f"for project '{self.project_id}' in region '{region}': {e}"
),
)
raise
self.set_cache("network_id", network["network"]["id"])
network_id = network["network"]["id"]
self.add_note(
"Network %s created for project %s"
% (network_config.network_name, self.project_id)
(
f"Network '{network_config.network_name}' "
f"created for project '{self.project_id}' in region '{region}'"
),
)
else:
self.add_note(
"Network %s already created for project %s"
% (network_config.network_name, self.project_id)
(
f"Network '{network_config.network_name}' "
f"already created for project '{self.project_id}' "
f"in region '{region}'"
),
)
self.set_cache(f"network_id{cache_suffix}", network_id)
if not self.get_cache("subnet_id"):
subnet_id = self.get_cache(f"subnet_id{cache_suffix}")
# NOTE(callumdickinson): Backwards compatibility with older tasks.
if not subnet_id and is_default_region:
subnet_id = self.get_cache("subnet_id")
# If the default subnet does not exist, create it.
if not subnet_id:
try:
subnet_body = {
"subnet": {
"network_id": self.get_cache("network_id"),
"network_id": network_id,
"ip_version": 4,
"tenant_id": self.project_id,
"dns_nameservers": network_config.dns_nameservers,
@ -161,16 +216,38 @@ class NewDefaultNetworkAction(BaseAction, ProjectMixin):
}
subnet = neutron.create_subnet(body=subnet_body)
except Exception as e:
self.add_note("Error: '%s' while creating subnet" % e)
self.add_note(
(
"Error while creating subnet "
f"in network '{network_config.network_name}' "
f"for project '{self.project_id}' in region '{region}': {e}"
),
)
raise
self.set_cache("subnet_id", subnet["subnet"]["id"])
self.add_note("Subnet created for network %s" % network_config.network_name)
subnet_id = subnet["subnet"]["id"]
self.add_note(
(
f"Subnet '{subnet_id}' created "
f"in network '{network_config.network_name}' "
f"created for project '{self.project_id}' in region '{region}'"
),
)
else:
self.add_note(
"Subnet already created for network %s" % network_config.network_name
(
f"Subnet '{subnet_id}' already created "
f"in network '{network_config.network_name}' "
f"created for project '{self.project_id}' in region '{region}'"
),
)
self.set_cache(f"subnet_id{cache_suffix}", subnet_id)
if not self.get_cache("router_id"):
router_id = self.get_cache(f"router_id{cache_suffix}")
# NOTE(callumdickinson): Backwards compatibility with older tasks.
if not router_id and is_default_region:
router_id = self.get_cache("router_id")
# If the default router does not exist, create it.
if not router_id:
try:
router_body = {
"router": {
@ -185,28 +262,70 @@ class NewDefaultNetworkAction(BaseAction, ProjectMixin):
router = neutron.create_router(body=router_body)
except Exception as e:
self.add_note(
"Error: '%s' while creating router: %s"
% (e, network_config.router_name)
(
f"Error while creating router '{network_config.router_name}' "
f"for project '{self.project_id}' in region '{region}': {e}"
),
)
raise
self.set_cache("router_id", router["router"]["id"])
self.add_note("Router created for project %s" % self.project_id)
router_id = router["router"]["id"]
self.add_note(
(
f"Router '{network_config.router_name}' "
f"created for project '{self.project_id}' in region '{region}'"
),
)
else:
self.add_note("Router already created for project %s" % self.project_id)
self.add_note(
(
f"Router '{network_config.router_name}' "
f"already created for project '{self.project_id}' "
f"in region '{region}'"
),
)
self.set_cache(f"router_id{cache_suffix}", router_id)
if not self.get_cache("port_id"):
port_id = self.get_cache(f"port_id{cache_suffix}")
# NOTE(callumdickinson): Backwards compatibility with older tasks.
if not port_id and is_default_region:
port_id = self.get_cache("port_id")
# If the subnet port on the default router does not exist, create it.
if not port_id:
try:
interface_body = {"subnet_id": self.get_cache("subnet_id")}
interface = neutron.add_interface_router(
self.get_cache("router_id"), body=interface_body
)
interface_body = {"subnet_id": subnet_id}
interface = neutron.add_interface_router(router_id, body=interface_body)
except Exception as e:
self.add_note("Error: '%s' while attaching interface" % e)
self.add_note(
(
"Error while adding interface "
f"for network '{network_config.network_name}' "
f"to router '{network_config.router_name}' "
f"for project '{self.project_id}' "
f"in region '{region}': {e}"
),
)
raise
self.set_cache("port_id", interface["port_id"])
self.add_note("Interface added to router for subnet")
port_id = interface["port_id"]
self.add_note(
(
f"Interface '{interface['port_id']}' "
f"for network '{network_config.network_name}' "
f"added to router '{network_config.router_name}' "
f"for project '{self.project_id}' "
f"in region '{region}'"
),
)
else:
self.add_note("Interface added to router for project %s" % self.project_id)
self.add_note(
(
f"Interface '{port_id}' "
f"for network '{network_config.network_name}' "
f"already added to router '{network_config.router_name}' "
f"for project '{self.project_id}' "
f"in region '{region}'"
),
)
self.set_cache(f"port_id{cache_suffix}", port_id)
def _prepare(self):
# Note: Do we need to get this from cache? it is a required setting

289
adjutant/actions/v1/tests/test_resource_actions.py
View File

@ -124,10 +124,10 @@ class ProjectSetupActionTests(AdjutantTestCase):
self.assertEqual(
action.action.cache,
{
"network_id": "net_id_0",
"port_id": "port_id_3",
"router_id": "router_id_2",
"subnet_id": "subnet_id_1",
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
},
)
@ -142,6 +142,93 @@ class ProjectSetupActionTests(AdjutantTestCase):
len(neutron_cache["RegionOne"]["test_project_id"]["subnets"]), 1
)
def test_network_setup_old_cache_compatibility(self):
"""
Check that a task with old-tyle cache values defined is compatible
with the new implementation.
"""
setup_neutron_cache("RegionOne", "test_project_id")
global neutron_cache
task = Task.objects.create(
keystone_user={"roles": ["admin"], "project_id": "test_project_id"}
)
project = mock.Mock()
project.id = "test_project_id"
project.name = "test_project"
project.domain = "default"
project.roles = {}
setup_identity_cache(projects=[project])
data = {
"setup_network": True,
"region": "RegionOne",
"project_id": "test_project_id",
}
action = NewDefaultNetworkAction(data, task=task, order=1)
neutron_cache[data["region"]][project.id]["networks"]["net_id_0"] = {
"name": "somenetwork",
"tenant_id": project.id,
"admin_state_up": True,
}
neutron_cache[data["region"]][project.id]["subnets"]["subnet_id_1"] = {
"network_id": "net_id_0",
"ip_version": 4,
"tenant_id": project.id,
"dns_nameservers": ["193.168.1.2", "193.168.1.3"],
"cidr": "192.168.1.0/24",
}
neutron_cache[data["region"]][project.id]["routers"]["router_id_2"] = {
"name": "somerouter",
"external_gateway_info": {
"network_id": "3cb50f61-5bce-4c03-96e6-8e262e12bb35",
},
"tenant_id": project.id,
"admin_state_up": True,
}
neutron_cache[data["region"]]["i"] = 4
action.action.cache = {
"network_id": "net_id_0",
"port_id": "port_id_3",
"router_id": "router_id_2",
"subnet_id": "subnet_id_1",
}
action.prepare()
self.assertEqual(action.valid, True)
action.approve()
self.assertEqual(action.valid, True)
self.assertEqual(
action.action.cache,
{
"network_id": "net_id_0",
"port_id": "port_id_3",
"router_id": "router_id_2",
"subnet_id": "subnet_id_1",
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
},
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["networks"]), 1
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["routers"]), 1
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["subnets"]), 1
)
def test_network_setup_no_setup(self):
"""
Told not to setup, should do nothing.
@ -186,6 +273,164 @@ class ProjectSetupActionTests(AdjutantTestCase):
len(neutron_cache["RegionOne"]["test_project_id"]["subnets"]), 0
)
@conf_utils.modify_conf(
CONF,
operations={
(
"adjutant.workflow.action_defaults.NewDefaultNetworkAction"
".create_in_regions"
): [
{"operation": "override", "value": ["RegionOne", "RegionTwo"]},
],
},
)
def test_network_setup_create_in_regions(self):
"""
Check that all regions specified in 'create_in_regions'
have a default network created.
"""
setup_neutron_cache("RegionOne", "test_project_id")
setup_neutron_cache("RegionTwo", "test_project_id")
task = Task.objects.create(
keystone_user={"roles": ["admin"], "project_id": "test_project_id"}
)
project = mock.Mock()
project.id = "test_project_id"
project.name = "test_project"
project.domain = "default"
project.roles = {}
setup_identity_cache(projects=[project])
data = {
"setup_network": True,
"region": "RegionOne",
"project_id": "test_project_id",
}
action = NewDefaultNetworkAction(data, task=task, order=1)
action.prepare()
self.assertEqual(action.valid, True)
action.approve()
self.assertEqual(action.valid, True)
self.assertEqual(
action.action.cache,
{
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
"network_id_RegionTwo": "net_id_0",
"port_id_RegionTwo": "port_id_3",
"router_id_RegionTwo": "router_id_2",
"subnet_id_RegionTwo": "subnet_id_1",
},
)
global neutron_cache
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["networks"]), 1
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["routers"]), 1
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["subnets"]), 1
)
self.assertEqual(
len(neutron_cache["RegionTwo"]["test_project_id"]["networks"]), 1
)
self.assertEqual(
len(neutron_cache["RegionTwo"]["test_project_id"]["routers"]), 1
)
self.assertEqual(
len(neutron_cache["RegionTwo"]["test_project_id"]["subnets"]), 1
)
@conf_utils.modify_conf(
CONF,
operations={
(
"adjutant.workflow.action_defaults.NewDefaultNetworkAction"
".create_in_all_regions"
): [
{"operation": "override", "value": True},
],
},
)
def test_network_setup_create_in_all_regions(self):
"""
Check that all regions have a default network created
when 'create_in_all_regions' is set to True.
"""
setup_neutron_cache("RegionOne", "test_project_id")
setup_neutron_cache("RegionTwo", "test_project_id")
task = Task.objects.create(
keystone_user={"roles": ["admin"], "project_id": "test_project_id"}
)
project = mock.Mock()
project.id = "test_project_id"
project.name = "test_project"
project.domain = "default"
project.roles = {}
setup_identity_cache(projects=[project])
data = {
"setup_network": True,
"region": "RegionOne",
"project_id": "test_project_id",
}
action = NewDefaultNetworkAction(data, task=task, order=1)
action.prepare()
self.assertEqual(action.valid, True)
action.approve()
self.assertEqual(action.valid, True)
self.assertEqual(
action.action.cache,
{
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
"network_id_RegionTwo": "net_id_0",
"port_id_RegionTwo": "port_id_3",
"router_id_RegionTwo": "router_id_2",
"subnet_id_RegionTwo": "subnet_id_1",
},
)
global neutron_cache
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["networks"]), 1
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["routers"]), 1
)
self.assertEqual(
len(neutron_cache["RegionOne"]["test_project_id"]["subnets"]), 1
)
self.assertEqual(
len(neutron_cache["RegionTwo"]["test_project_id"]["networks"]), 1
)
self.assertEqual(
len(neutron_cache["RegionTwo"]["test_project_id"]["routers"]), 1
)
self.assertEqual(
len(neutron_cache["RegionTwo"]["test_project_id"]["subnets"]), 1
)
def test_network_setup_fail(self):
"""
Should fail, but on re_approve will continue where it left off.
@ -224,7 +469,11 @@ class ProjectSetupActionTests(AdjutantTestCase):
pass
self.assertEqual(
action.action.cache, {"network_id": "net_id_0", "subnet_id": "subnet_id_1"}
action.action.cache,
{
"network_id_RegionOne": "net_id_0",
"subnet_id_RegionOne": "subnet_id_1",
},
)
self.assertEqual(
@ -244,10 +493,10 @@ class ProjectSetupActionTests(AdjutantTestCase):
self.assertEqual(
action.action.cache,
{
"network_id": "net_id_0",
"port_id": "port_id_3",
"router_id": "router_id_2",
"subnet_id": "subnet_id_1",
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
},
)
@ -297,10 +546,10 @@ class ProjectSetupActionTests(AdjutantTestCase):
self.assertEqual(
action.action.cache,
{
"network_id": "net_id_0",
"port_id": "port_id_3",
"router_id": "router_id_2",
"subnet_id": "subnet_id_1",
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
},
)
@ -435,7 +684,11 @@ class ProjectSetupActionTests(AdjutantTestCase):
pass
self.assertEqual(
action.action.cache, {"network_id": "net_id_0", "subnet_id": "subnet_id_1"}
action.action.cache,
{
"network_id_RegionOne": "net_id_0",
"subnet_id_RegionOne": "subnet_id_1",
},
)
self.assertEqual(
@ -455,10 +708,10 @@ class ProjectSetupActionTests(AdjutantTestCase):
self.assertEqual(
action.action.cache,
{
"network_id": "net_id_0",
"port_id": "port_id_3",
"router_id": "router_id_2",
"subnet_id": "subnet_id_1",
"network_id_RegionOne": "net_id_0",
"port_id_RegionOne": "port_id_3",
"router_id_RegionOne": "router_id_2",
"subnet_id_RegionOne": "subnet_id_1",
},
)

22
adjutant/common/tests/fake_clients.py
View File

@ -575,13 +575,13 @@ class FakeNeutronClient(object):
project_id = body["network"]["tenant_id"]
net = {
"network": {
"id": "net_id_%s" % neutron_cache["RegionOne"]["i"],
"id": "net_id_%s" % neutron_cache[self.region]["i"],
"body": body,
}
}
net_id = net["network"]["id"]
neutron_cache["RegionOne"][project_id]["networks"][net_id] = net
neutron_cache["RegionOne"]["i"] += 1
neutron_cache[self.region][project_id]["networks"][net_id] = net
neutron_cache[self.region]["i"] += 1
return net
def create_subnet(self, body):
@ -589,13 +589,13 @@ class FakeNeutronClient(object):
project_id = body["subnet"]["tenant_id"]
subnet = {
"subnet": {
"id": "subnet_id_%s" % neutron_cache["RegionOne"]["i"],
"id": "subnet_id_%s" % neutron_cache[self.region]["i"],
"body": body,
}
}
sub_id = subnet["subnet"]["id"]
neutron_cache["RegionOne"][project_id]["subnets"][sub_id] = subnet
neutron_cache["RegionOne"]["i"] += 1
neutron_cache[self.region][project_id]["subnets"][sub_id] = subnet
neutron_cache[self.region]["i"] += 1
return subnet
def create_router(self, body):
@ -603,19 +603,19 @@ class FakeNeutronClient(object):
project_id = body["router"]["tenant_id"]
router = {
"router": {
"id": "router_id_%s" % neutron_cache["RegionOne"]["i"],
"id": "router_id_%s" % neutron_cache[self.region]["i"],
"body": body,
}
}
router_id = router["router"]["id"]
neutron_cache["RegionOne"][project_id]["routers"][router_id] = router
neutron_cache["RegionOne"]["i"] += 1
neutron_cache[self.region][project_id]["routers"][router_id] = router
neutron_cache[self.region]["i"] += 1
return router
def add_interface_router(self, router_id, body):
global neutron_cache
port_id = "port_id_%s" % neutron_cache["RegionOne"]["i"]
neutron_cache["RegionOne"]["i"] += 1
port_id = "port_id_%s" % neutron_cache[self.region]["i"]
neutron_cache[self.region]["i"] += 1
interface = {
"port_id": port_id,
"id": router_id,

13
releasenotes/notes/multiple-default-networks-5a89766d377b06d2.yaml Normal file
View File

@ -0,0 +1,13 @@
---
features:
- |
The ``create_in_all_regions`` option has been added to
``NewDefaultNetworkAction`` and ``NewProjectDefaultNetworkAction``.
When set to ``true``, default networks and routers will be created in
all enabled regions, instead of just the default region.
- |
The ``create_in_regions`` option has been added to
``NewDefaultNetworkAction`` and ``NewProjectDefaultNetworkAction``.
This defines a list of regions to create default networks and routers in
for new sign ups, instead of the default region
(or all regions, if ``create_in_all_regions`` is ``true``).