openstack
/
nova
Code Issues Proposed changes

Provider Config File: Coding style and test cases improvement 

This patch is mainly for handling the remaining code review comments
as a follow-up work to improve coding style and test cases and there
is no code logic change to feature itself.

Change Summary:
1) remove unnecessary logic judgement code in _validate_rc function
2) regroup import order for standard libary and 3rd-party library
   in test_resource_tracker.py
3) unify test cases in ValidateProviderConfigTestCases class
   for both positive and negative test
4) rename test cases and test data files with more meaningful names

Change-Id: If940dfeb5b62ff9f11ca98e9125357c0a472dbfe
Blueprint: provider-config-file
This commit is contained in:
Tony Su 2020-08-31 01:23:59 +00:00
parent 75b5535e34
commit 38757964ed
5 changed files with 87 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/compute/provider_config.py
View File

@ -269,8 +269,7 @@ def _validate_provider_config(config, provider_config_path):
for inventory in additional_inventories:
inventory_conflicts = [rc for rc in inventory
if not os_resource_classes.is_custom(rc)]
if inventory_conflicts:
all_inventory_conflicts += inventory_conflicts
all_inventory_conflicts += inventory_conflicts
if all_inventory_conflicts:
# sort for more predictable message for testing

4
nova/tests/functional/compute/test_resource_tracker.py
View File

@ -11,10 +11,10 @@
# under the License.
import copy
import fixtures
import mock
import os
import fixtures
import mock
import os_resource_classes as orc
import os_traits
from oslo_utils.fixture import uuidsentinel as uuids

72
nova/tests/unit/compute/provider_config_data/validate_provider_bad_config.yaml Normal file
View File

@ -0,0 +1,72 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
# This file contains ddt test data consumed by test_provider_config.py to test
# bad provider configrations through _validate_provider_config().
# Sample is required for each test and passed to _validate_provider_config().
# If excpetion is raised with expected message, the test passes.
one_invalid_additional_inventory:
sample:
providers:
- identification:
name: NAME
inventories:
additional:
- CUSTOM_RESOURCE_CLASS:
total: 1
- INVALID_RESOURCE_CLASS:
total: 1
expected_messages: ["An error occurred while processing a provider config
file: Invalid resource class, only custom resource
classes are allowed: INVALID_RESOURCE_CLASS"]
two_invalid_additional_inventory:
sample:
providers:
- identification:
name: NAME
inventories:
additional:
- CUSTOM_RESOURCE_CLASS:
total: 1
- INVALID_RESOURCE_CLASS:
total: 1
- INVALID_RESOURCE_CLASS2:
total: 1
expected_messages: ["An error occurred while processing a provider
config file: Invalid resource class, only custom
resource classes are allowed: INVALID_RESOURCE_CLASS,
INVALID_RESOURCE_CLASS2"]
one_invalid_additional_trait:
sample:
providers:
- identification:
name: NAME
traits:
additional:
- CUSTOM_TRAIT
- INVALID_TRAIT
expected_messages: ["An error occurred while processing a provider
config file: Invalid traits, only custom traits
are allowed: ['INVALID_TRAIT']"]
two_invalid_additional_trait:
sample:
providers:
- identification:
name: NAME
traits:
additional:
- CUSTOM_TRAIT
- INVALID_TRAIT
- INVALID_TRAIT2
expected_messages: ["An error occurred while processing a provider
config file: Invalid traits, only custom traits
are allowed: ['INVALID_TRAIT', 'INVALID_TRAIT2']"]

4
nova/tests/unit/compute/provider_config_data/validate_provider_test_data.yaml → nova/tests/unit/compute/provider_config_data/validate_provider_good_config.yaml
View File

@ -12,8 +12,8 @@
# This file contains ddt test data consumed by test_provider_config.py to test
# good paths through _validate_provider_config(). Sample is required for each
# test. This sample and passed to _validate_provider_config(). If no exception
# is raised, the test passes.
# test and passed to _validate_provider_config(). If no exception is raised,
# the test passes.
no_trait_or_inventory_keys:
sample:

111
nova/tests/unit/compute/test_provider_config.py
View File

@ -132,110 +132,19 @@ class SchemaValidationTestCasesV1(SchemaValidationMixin):
@ddt.ddt
class ValidateProviderConfigTestCases(base.BaseTestCase):
@ddt.unpack
@ddt.file_data('provider_config_data/validate_provider_test_data.yaml')
def test__validate_provider_config(self, sample):
@ddt.file_data('provider_config_data/validate_provider_good_config.yaml')
def test__validate_provider_good_config(self, sample):
provider_config._validate_provider_config(sample, "fake_path")
# TODO(tony): gibi's comment,
# negative tests should be in the same format as the other test
# for the provider config. continue work on this a follow up.
def test__validate_provider_config_one_invalid_additional_inventory(self):
config = {
"providers": [
{
"identification": {"name": "NAME"},
"inventories": {
"additional": [
{"CUSTOM_RESOURCE_CLASS": {}},
{"INVALID_RESOURCE_CLASS": {}}
]
}
}
]
}
@ddt.unpack
@ddt.file_data('provider_config_data/validate_provider_bad_config.yaml')
def test__validate_provider_bad_config(self, sample, expected_messages):
actual_msg = self.assertRaises(
nova_exc.ProviderConfigException,
provider_config._validate_provider_config,
sample, 'fake_path').message
actual = self.assertRaises(nova_exc.ProviderConfigException,
provider_config._validate_provider_config,
config, "fake_path").message
self.assertEqual(
"An error occurred while processing a provider config file: "
"Invalid resource class, only custom resource classes are allowed:"
" INVALID_RESOURCE_CLASS", actual)
def test__validate_provider_config_two_invalid_additional_inventory(self):
config = {
"providers": [
{
"identification": {"name": "NAME"},
"inventories": {
"additional": [
{"CUSTOM_RESOURCE_CLASS": {}},
{"INVALID_RESOURCE_CLASS": {}},
{"INVALID_RESOURCE_CLASS2": {}}
]
}
}
]
}
actual = self.assertRaises(nova_exc.ProviderConfigException,
provider_config._validate_provider_config,
config, "fake_path").message
self.assertEqual(
"An error occurred while processing a provider config file: "
"Invalid resource class, only custom resource classes are allowed:"
" INVALID_RESOURCE_CLASS, INVALID_RESOURCE_CLASS2", actual)
def test__validate_provider_config_one_invalid_additional_trait(self):
config = {
"providers": [
{
"identification": {"name": "NAME"},
"traits": {
"additional": [
"CUSTOM_TRAIT",
"INVALID_TRAIT"
]
}
}
]
}
actual = self.assertRaises(nova_exc.ProviderConfigException,
provider_config._validate_provider_config,
config, "fake_path").message
self.assertEqual(
"An error occurred while processing a provider config "
"file: Invalid traits, only custom traits are allowed: "
"['INVALID_TRAIT']", actual)
def test__validate_provider_config_two_invalid_additional_trait(self):
config = {
"providers": [
{
"identification": {"name": "NAME"},
"traits": {
"additional": [
"CUSTOM_TRAIT",
"INVALID_TRAIT",
"INVALID_TRAIT2"
]
}
}
]
}
actual = self.assertRaises(nova_exc.ProviderConfigException,
provider_config._validate_provider_config,
config, "fake_path").message
self.assertEqual(
"An error occurred while processing a provider config "
"file: Invalid traits, only custom traits are allowed: "
"['INVALID_TRAIT', 'INVALID_TRAIT2']", actual)
self.assertIn(actual_msg, expected_messages)
@mock.patch.object(provider_config, 'LOG')
def test__validate_provider_config_one_noop_provider(self, mock_log):