Skip to content

Commit

Permalink
Make "esi mdc baremetal node list" work
Browse files Browse the repository at this point in the history 
OpenStackConfig.get_all() will fail with a keystone exception if there are
any OS_* variables set in the environment [1]. This is easy to trigger if,
for example, you have set OS_BAREMETAL_API_VERSION in your environment to
work around [2].

We can avoid the problem by removing the call to get_all(), and
instead initializing the list of clouds using get_cloud_names().

This commit makes a number of additional changes to
esiclient/v1/mdc/mdc_node_baremetal.py:

- Several attribute names appear to have changed since this code was
  written (node.uuid is now node.id; node.instance_uuid is now
  node.instance_id).

- Replace single-letter variable names with more meaningful names.

- Add the '--ignore-invalid' command, which allows the command to continue
  in the event that it cannot successfully connect to one or more clouds.

[1]: https://bugs.launchpad.net/openstacksdk/+bug/2096621
[2]: CCI-MOC/esi#669
  • Loading branch information
@larsks
larsks committed Jan 24, 2025
1 parent 75d6644 commit 4f6aa4a
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 130 deletions.
155 changes: 49 additions & 106 deletions esiclient/tests/unit/v1/mdc/test_mdc_node_baremetal.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,89 +11,55 @@
# under the License.
#

import mock
import munch
from unittest import mock
import openstack.baremetal.v1.node
import openstack.connection

from esiclient.tests.unit import base
from esiclient.v1.mdc import mdc_node_baremetal


@mock.patch("esiclient.v1.mdc.mdc_node_baremetal.openstack.connect")
@mock.patch("openstack.config.loader.OpenStackConfig.get_cloud_names")
class TestMDCBaremetalNodeList(base.TestCommand):
def setUp(self):
super(TestMDCBaremetalNodeList, self).setUp()
super().setUp()
self.cmd = mdc_node_baremetal.MDCBaremetalNodeList(self.app, None)

class FakeConfig(object):
def __init__(self, name, region):
self.name = name
self.config = {"region_name": region}

self.cloud1 = FakeConfig("esi", "regionOne")
self.cloud2 = FakeConfig("esi", "regionTwo")
self.cloud3 = FakeConfig("esi2", "regionOne")

self.node = munch.Munch(
uuid="node_uuid_1",
name="node1",
instance_uuid="instance_uuid_1",
power_state="off",
provision_state="active",
maintenance=False,
)

self.node2 = munch.Munch(
uuid="node_uuid_2",
name="node2",
instance_uuid="instance_uuid_2",
power_state="off",
provision_state="active",
maintenance=False,
)

self.node3 = munch.Munch(
uuid="node_uuid_3",
name="node3",
instance_uuid="instance_uuid_3",
power_state="off",
provision_state="active",
maintenance=False,
)

self.node4 = munch.Munch(
uuid="node_uuid_4",
name="node4",
instance_uuid="instance_uuid_4",
power_state="off",
provision_state="active",
maintenance=False,
)

@mock.patch("esiclient.v1.mdc.mdc_node_baremetal.openstack.connect")
@mock.patch(
"esiclient.v1.mdc.mdc_node_baremetal.openstack.config.loader."
"OpenStackConfig.get_all_clouds"
)
def test_take_action_list(self, mock_config, mock_connect):
mock_cloud = mock.Mock()
mock_cloud.list_machines.side_effect = [
[self.node],
[self.node2],
[self.node3, self.node4],
self.cloud_names = [f"esi{i}" for i in range(1, 4)]
self.connection = mock.Mock(openstack.connection.Connection, name="test-cloud")

self.node = {}
for i in range(1, 5):
node = self.node[i] = mock.Mock(
openstack.baremetal.v1.node.Node, name=f"node{i}"
)
node.configure_mock(
id=f"node_uuid_{i}",
name=f"node{i}",
instance_id=f"instance_uuid_{i}",
power_state="off",
provision_state="active",
is_maintenance=False,
)

def test_take_action_list_all_clouds(self, mock_get_cloud_names, mock_connect):
self.connection.list_machines.side_effect = [
[self.node[1]],
[self.node[2]],
[self.node[3], self.node[4]],
]
mock_connect.return_value = mock_cloud

mock_config.return_value = [self.cloud1, self.cloud2, self.cloud3]
mock_connect.return_value = self.connection
mock_get_cloud_names.return_value = self.cloud_names

arglist = []
verifylist = []

parsed_args = self.check_parser(self.cmd, arglist, verifylist)

results = self.cmd.take_action(parsed_args)

expected = (
[
"Cloud",
"Region",
"UUID",
"Name",
"Instance UUID",
Expand All @@ -103,8 +69,7 @@ def test_take_action_list(self, mock_config, mock_connect):
],
[
[
self.cloud1.name,
self.cloud1.config["region_name"],
"esi1",
"node_uuid_1",
"node1",
"instance_uuid_1",
Expand All @@ -113,8 +78,7 @@ def test_take_action_list(self, mock_config, mock_connect):
False,
],
[
self.cloud2.name,
self.cloud2.config["region_name"],
"esi2",
"node_uuid_2",
"node2",
"instance_uuid_2",
Expand All @@ -123,8 +87,7 @@ def test_take_action_list(self, mock_config, mock_connect):
False,
],
[
self.cloud3.name,
self.cloud3.config["region_name"],
"esi3",
"node_uuid_3",
"node3",
"instance_uuid_3",
Expand All @@ -133,8 +96,7 @@ def test_take_action_list(self, mock_config, mock_connect):
False,
],
[
self.cloud3.name,
self.cloud3.config["region_name"],
"esi3",
"node_uuid_4",
"node4",
"instance_uuid_4",
Expand All @@ -145,26 +107,19 @@ def test_take_action_list(self, mock_config, mock_connect):
],
)
self.assertEqual(expected, results)
mock_config.assert_called_once()
assert mock_cloud.list_machines.call_count == 3

@mock.patch("esiclient.v1.mdc.mdc_node_baremetal.openstack.connect")
@mock.patch(
"esiclient.v1.mdc.mdc_node_baremetal.openstack.config.loader."
"OpenStackConfig.get_all_clouds"
)
def test_take_action_list_cloud(self, mock_config, mock_connect):
mock_cloud = mock.Mock()
mock_cloud.list_machines.side_effect = [
[self.node],
[self.node2],
[self.node3, self.node4],
mock_get_cloud_names.assert_called_once()
assert self.connection.list_machines.call_count == 3

def test_take_action_list_specific_cloud(self, mock_get_cloud_names, mock_connect):
self.connection.list_machines.side_effect = [
[self.node[1]],
[self.node[2]],
[self.node[3], self.node[4]],
]
mock_connect.return_value = mock_cloud
mock_connect.return_value = self.connection
mock_get_cloud_names.return_value = self.cloud_names

mock_config.return_value = [self.cloud1, self.cloud2, self.cloud3]

arglist = ["--clouds", "esi"]
arglist = ["esi1"]
verifylist = []

parsed_args = self.check_parser(self.cmd, arglist, verifylist)
Expand All @@ -173,7 +128,6 @@ def test_take_action_list_cloud(self, mock_config, mock_connect):
expected = (
[
"Cloud",
"Region",
"UUID",
"Name",
"Instance UUID",
Expand All @@ -183,27 +137,16 @@ def test_take_action_list_cloud(self, mock_config, mock_connect):
],
[
[
self.cloud1.name,
self.cloud1.config["region_name"],
"esi1",
"node_uuid_1",
"node1",
"instance_uuid_1",
"off",
"active",
False,
],
[
self.cloud2.name,
self.cloud2.config["region_name"],
"node_uuid_2",
"node2",
"instance_uuid_2",
"off",
"active",
False,
],
],
)
self.assertEqual(expected, results)
mock_config.assert_called_once()
assert mock_cloud.list_machines.call_count == 2
mock_get_cloud_names.assert_called_once()
assert self.connection.list_machines.call_count == 1
47 changes: 23 additions & 24 deletions esiclient/v1/mdc/mdc_node_baremetal.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@ class MDCBaremetalNodeList(command.Lister):
auth_required = False

def get_parser(self, prog_name):
parser = super(MDCBaremetalNodeList, self).get_parser(prog_name)
parser = super().get_parser(prog_name)
parser.add_argument("--ignore-invalid", "-i", action="store_true")
parser.add_argument(
"--clouds",
dest="clouds",
"clouds",
metavar="<clouds>",
nargs="+",
nargs="*",
help=_("Specify the cloud to use from clouds.yaml."),
default=openstack.config.OpenStackConfig().get_cloud_names(),
)

return parser

def take_action(self, parsed_args):
columns = [
"Cloud",
"Region",
"UUID",
"Name",
"Instance UUID",
Expand All @@ -48,27 +48,26 @@ def take_action(self, parsed_args):
]
data = []

cloud_regions = openstack.config.loader.OpenStackConfig().get_all_clouds()
if parsed_args.clouds:
cloud_regions = filter(
lambda c: c.name in parsed_args.clouds, cloud_regions
)
for c in cloud_regions:
nodes = openstack.connect(
cloud=c.name, region=c.config["region_name"]
).list_machines()
for n in nodes:
data.append(
for cloud in parsed_args.clouds:
try:
data.extend(
[
c.name,
c.config["region_name"],
n.uuid,
n.name,
n.instance_uuid,
n.power_state,
n.provision_state,
n.maintenance,
cloud,
node.id,
node.name,
node.instance_id,
node.power_state,
node.provision_state,
node.is_maintenance,
]
for node in openstack.connect(cloud=cloud).list_machines()
)
except Exception as err:
if parsed_args.ignore_invalid:
self.log.error(
"failed to retrieve information for cloud %s: %s", cloud, err
)
continue
raise

return columns, data

0 comments on commit 4f6aa4a

Please sign in to comment.