From 3f562226112b99d2044dd05a2e15b9752d5c9c1d Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 4 Sep 2018 20:57:33 -0400 Subject: [PATCH] Fix nova-status "_check_resource_providers" check The way in which this check counted compute nodes was broken because of an incorrect for/else condition. If the check is run with a nova.conf like we have in devstack, where the API database is configured but the [database]/connection is pointing at cell0, where there are no compute nodes, the check passes saying there are no compute nodes even if the are compute nodes found in the cell databases (in the for loop). This is because the else executes because the for loop doesn't break, and then _count_compute_nodes returns 0 for cell0 and overwrites the num_computes variable. This fixes the issue by checking if we have cell mappings before running the loop, else we hit the else block as was originally intended. Change-Id: I1a706d028a9ca894348a19b7b3df1ea673e4ec90 Partial-Bug: #1790721 (cherry picked from commit dcd421ae9e6f0391fea06c9d20949267354c3b3c) (cherry picked from commit f588a0b6c337368596bc8732141e864f56f72212) (cherry picked from commit 6a806de71b945509c8c5d3de53ff7ba5aa75bf87) --- nova/cmd/status.py | 7 ++++--- nova/tests/unit/cmd/test_status.py | 11 +++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/nova/cmd/status.py b/nova/cmd/status.py index 34bae85d4a84..8712570188ee 100644 --- a/nova/cmd/status.py +++ b/nova/cmd/status.py @@ -290,9 +290,10 @@ class UpgradeCommands(object): cell_mappings = self._get_non_cell0_mappings() ctxt = nova_context.get_admin_context() num_computes = 0 - for cell_mapping in cell_mappings: - with nova_context.target_cell(ctxt, cell_mapping) as cctxt: - num_computes += self._count_compute_nodes(cctxt) + if cell_mappings: + for cell_mapping in cell_mappings: + with nova_context.target_cell(ctxt, cell_mapping) as cctxt: + num_computes += self._count_compute_nodes(cctxt) else: # There are no cell mappings, cells v2 was maybe not deployed in # Newton, but placement might have been, so let's check the single diff --git a/nova/tests/unit/cmd/test_status.py b/nova/tests/unit/cmd/test_status.py index bf2fad0a90ed..6c5b62d2d8bc 100644 --- a/nova/tests/unit/cmd/test_status.py +++ b/nova/tests/unit/cmd/test_status.py @@ -652,6 +652,17 @@ class TestUpgradeCheckResourceProviders(test.NoDBTestCase): # create an externally shared IP allocation pool resource provider self._create_resource_provider(FAKE_IP_POOL_INVENTORY) + # Stub out _count_compute_nodes to make sure we never call it without + # a cell-targeted context. + original_count_compute_nodes = ( + status.UpgradeCommands._count_compute_nodes) + + def stub_count_compute_nodes(_self, context=None): + self.assertIsNotNone(context.db_connection) + return original_count_compute_nodes(_self, context=context) + self.stub_out('nova.cmd.status.UpgradeCommands._count_compute_nodes', + stub_count_compute_nodes) + result = self.cmd._check_resource_providers() self.assertEqual(status.UpgradeCheckCode.SUCCESS, result.code) self.assertIsNone(result.details)