Check for physical_resource_id before getting deployments
Heat does non-strict validation of nested stack resources.
Most constraints are not enforced for nested stack resources
It's possible to set {Role}DeploymentActions as something like
[''] and it would not fail in validation. This would result in
a deployment resource with no physical_resource_id. The existing
check[1] for config-download would not be sufficient for some
cases as it can be another nested stack (we alias
both OS::Tripleo::SoftwareDeployment and
OS::Heat::StructuredDeployment).
[1]
deployment.attributes['value'].get('deployment') == \
'TripleOSoftwareDeployment'
This adds a new check not to try and download heat deployments with
no physical_resource_id.
Change-Id: I234e629cc5377cda8d5d01bfe4416ad967cdc067
Closes-Bug: #1812604
(cherry picked from commit 54c448f683
)
This commit is contained in:
parent
985cb0ffd4
commit
9b3e9b2c45
|
@ -231,7 +231,12 @@ class TestConfig(base.TestCase):
|
|||
|
||||
return deployment_data, configs
|
||||
|
||||
def _get_config_dict(self, deployment):
|
||||
def _get_deployment_id(self, deployment):
|
||||
return deployment.attributes['value']['deployment']
|
||||
|
||||
def _get_config_dict(self, deployment_id):
|
||||
deployment = list(filter(
|
||||
lambda d: d.id == deployment_id, self.deployments))[0]
|
||||
config = self.configs[deployment.attributes['value']['config']].copy()
|
||||
config['inputs'] = []
|
||||
config['inputs'].append(dict(
|
||||
|
@ -247,15 +252,16 @@ class TestConfig(base.TestCase):
|
|||
return yaml.safe_load(open(file_path).read())
|
||||
|
||||
@patch.object(ooo_config.Config, 'initialize_git_repo')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
|
||||
@patch('tripleo_common.utils.config.Config.get_config_dict')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_data')
|
||||
def test_config_download(self, mock_deployment_data, mock_config_dict,
|
||||
mock_deployment_resource_id,
|
||||
mock_git_init):
|
||||
heat = mock.MagicMock()
|
||||
self.config = ooo_config.Config(heat)
|
||||
stack = mock.MagicMock()
|
||||
heat.stacks.get.return_value = stack
|
||||
|
||||
stack.outputs = [
|
||||
{'output_key': 'RoleNetHostnameMap',
|
||||
'output_value': {
|
||||
|
@ -279,8 +285,9 @@ class TestConfig(base.TestCase):
|
|||
deployment_data, configs = \
|
||||
self._get_config_data('config_data.yaml')
|
||||
self.configs = configs
|
||||
|
||||
self.deployments = deployment_data
|
||||
mock_deployment_data.return_value = deployment_data
|
||||
mock_deployment_resource_id.side_effect = self._get_deployment_id
|
||||
mock_config_dict.side_effect = self._get_config_dict
|
||||
|
||||
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
|
||||
|
@ -346,15 +353,17 @@ class TestConfig(base.TestCase):
|
|||
d)))
|
||||
|
||||
@patch.object(ooo_config.Config, 'initialize_git_repo')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
|
||||
@patch('tripleo_common.utils.config.Config.get_config_dict')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_data')
|
||||
def test_config_download_os_apply_config(
|
||||
self, mock_deployment_data, mock_config_dict, mock_git_init):
|
||||
self, mock_deployment_data, mock_config_dict,
|
||||
mock_deployment_resource_id, mock_git_init):
|
||||
heat = mock.MagicMock()
|
||||
self.config = ooo_config.Config(heat)
|
||||
stack = mock.MagicMock()
|
||||
heat.stacks.get.return_value = stack
|
||||
|
||||
heat.resources.get.return_value = mock.MagicMock()
|
||||
stack.outputs = [
|
||||
{'output_key': 'RoleNetHostnameMap',
|
||||
'output_value': {
|
||||
|
@ -397,9 +406,10 @@ class TestConfig(base.TestCase):
|
|||
deployment_data.append(deployment_mock)
|
||||
|
||||
self.configs = configs
|
||||
|
||||
self.deployments = deployment_data
|
||||
mock_deployment_data.return_value = deployment_data
|
||||
mock_config_dict.side_effect = self._get_config_dict
|
||||
mock_deployment_resource_id.side_effect = self._get_deployment_id
|
||||
|
||||
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
|
||||
with warnings.catch_warnings(record=True) as w:
|
||||
|
@ -410,21 +420,26 @@ class TestConfig(base.TestCase):
|
|||
assert "group:os-apply-config is deprecated" in str(w[-1].message)
|
||||
|
||||
@patch.object(ooo_config.Config, 'initialize_git_repo')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_data')
|
||||
def test_config_download_no_deployment_name(self, mock_deployment_data,
|
||||
mock_git_init):
|
||||
def test_config_download_no_deployment_name(
|
||||
self, mock_deployment_data, mock_deployment_resource_id,
|
||||
mock_git_init):
|
||||
heat = mock.MagicMock()
|
||||
self.config = ooo_config.Config(heat)
|
||||
stack = mock.MagicMock()
|
||||
heat.stacks.get.return_value = stack
|
||||
heat.resources.get.return_value = mock.MagicMock()
|
||||
|
||||
deployment_data, _ = self._get_config_data('config_data.yaml')
|
||||
|
||||
# Delete the name of the first deployment and his parent.
|
||||
del deployment_data[0].attributes['value']['name']
|
||||
deployment_data[0].parent_resource = None
|
||||
self.deployments = deployment_data
|
||||
|
||||
mock_deployment_data.return_value = deployment_data
|
||||
mock_deployment_resource_id.side_effect = self._get_deployment_id
|
||||
|
||||
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
|
||||
self.assertRaises(ValueError,
|
||||
|
@ -432,15 +447,19 @@ class TestConfig(base.TestCase):
|
|||
mock_git_init.assert_called_once_with(self.tmp_dir)
|
||||
|
||||
@patch.object(ooo_config.Config, 'initialize_git_repo')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
|
||||
@patch('tripleo_common.utils.config.Config.get_config_dict')
|
||||
@patch('tripleo_common.utils.config.Config.get_deployment_data')
|
||||
def test_config_download_no_deployment_uuid(self, mock_deployment_data,
|
||||
mock_config_dict,
|
||||
mock_deployment_resource_id,
|
||||
mock_git_init):
|
||||
heat = mock.MagicMock()
|
||||
self.config = ooo_config.Config(heat)
|
||||
stack = mock.MagicMock()
|
||||
heat.stacks.get.return_value = stack
|
||||
heat.resources.get.return_value = mock.MagicMock()
|
||||
|
||||
stack.outputs = [
|
||||
{'output_key': 'RoleNetHostnameMap',
|
||||
'output_value': {
|
||||
|
@ -477,14 +496,20 @@ class TestConfig(base.TestCase):
|
|||
deployment_data[0].attributes['value']['deployment'] = \
|
||||
'TripleOSoftwareDeployment'
|
||||
|
||||
# Set the physical_resource_id as '' for the second deployment
|
||||
deployment_data[1].attributes['value']['deployment'] = ''
|
||||
|
||||
self.configs = configs
|
||||
self.deployments = deployment_data
|
||||
mock_deployment_data.return_value = deployment_data
|
||||
mock_config_dict.side_effect = self._get_config_dict
|
||||
mock_deployment_resource_id.side_effect = self._get_deployment_id
|
||||
|
||||
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
|
||||
with warnings.catch_warnings(record=True) as w:
|
||||
self.config.download_config(stack, self.tmp_dir)
|
||||
assert "Skipping deployment" in str(w[-1].message)
|
||||
assert "Skipping deployment" in str(w[-2].message)
|
||||
|
||||
@patch.object(ooo_config.Config, 'initialize_git_repo')
|
||||
@patch.object(ooo_config.git, 'Repo')
|
||||
|
@ -587,11 +612,11 @@ class TestConfig(base.TestCase):
|
|||
deployment_data, configs = \
|
||||
self._get_config_data('config_data.yaml')
|
||||
self.configs = configs
|
||||
self.deployments = deployment_data
|
||||
|
||||
stack_data = self.config.fetch_config('overcloud')
|
||||
mock_deployment_data.return_value = deployment_data
|
||||
mock_config_dict.side_effect = self._get_config_dict
|
||||
|
||||
config_dir = self.useFixture(fixtures.TempDir()).path
|
||||
|
||||
self.config.write_config(stack_data, 'overcloud', config_dir)
|
||||
|
|
|
@ -72,7 +72,7 @@ class Config(object):
|
|||
if server_id in v:
|
||||
return k
|
||||
|
||||
def get_config_dict(self, deployment):
|
||||
def get_deployment_resource_id(self, deployment):
|
||||
if '/' in deployment.attributes['value']['deployment']:
|
||||
deployment_stack_id = \
|
||||
deployment.attributes['value']['deployment'].split('/')[-1]
|
||||
|
@ -82,6 +82,9 @@ class Config(object):
|
|||
else:
|
||||
deployment_resource_id = \
|
||||
deployment.attributes['value']['deployment']
|
||||
return deployment_resource_id
|
||||
|
||||
def get_config_dict(self, deployment_resource_id):
|
||||
deployment_rsrc = self.client.software_deployments.get(
|
||||
deployment_resource_id)
|
||||
config = self.client.software_configs.get(
|
||||
|
@ -261,8 +264,15 @@ class Config(object):
|
|||
# create. If that's the case, just skip this deployment, otherwise
|
||||
# it will result in a Not found error if we try and query the
|
||||
# deployment API for this deployment.
|
||||
if deployment.attributes['value'].get('deployment') == \
|
||||
'TripleOSoftwareDeployment':
|
||||
dep_value_resource_name = deployment.attributes[
|
||||
'value'].get('deployment') == 'TripleOSoftwareDeployment'
|
||||
|
||||
# if not check the physical_resource_id
|
||||
if not dep_value_resource_name:
|
||||
deployment_resource_id = self.get_deployment_resource_id(
|
||||
deployment)
|
||||
|
||||
if dep_value_resource_name or not deployment_resource_id:
|
||||
warnings.warn('Skipping deployment %s because it has no '
|
||||
'valid uuid (physical_resource_id) '
|
||||
'associated.' %
|
||||
|
@ -270,7 +280,7 @@ class Config(object):
|
|||
continue
|
||||
|
||||
server_id = deployment.attributes['value']['server']
|
||||
config_dict = self.get_config_dict(deployment)
|
||||
config_dict = self.get_config_dict(deployment_resource_id)
|
||||
|
||||
# deployment_name should be set via the name property on the
|
||||
# Deployment resources in the templates, however, if it's None
|
||||
|
|
Loading…
Reference in New Issue