deployment including path to nic-config files fails

I think the issue is related to the "default" definitions in the attached files/templates.

Looking for example at:

  TenantInterfaceRoutes:
    default: []
    description: >
      Routes for the tenant network traffic.
      JSON route e.g. [{'destination':'10.0.0.0/16', 'nexthop':'10.0.0.1'}]
      Unless the default is changed, the parameter is automatically resolved
      from the subnet host_routes attribute.
    type: json

this gets translated into:

'TenantInterfaceRoutes': {'default': <map object at 0x7f84f97a9f60>, 'description': "Routes for the tenant network traffic. JSON route e.g. [{'destination':'10.0.0.0/16', 'nexthop':'10.0.0.1'}] Unless the default is changed, the parameter is automatically resolved from the subnet host_routes attribute.\n", 'type': 'json'},

Things go sideways when doing a safe_dump in /usr/lib/python3.6/site-packages/tripleoclient/utils.py

def replace_links_in_template_contents(contents, link_replacement):
    """Replace get_file and type file links in Heat template contents

    If the string contents passed in is a Heat template, scan the
    template for 'get_file' and 'type' occurrences, and replace the
    file paths according to link_replacement dict. (Key/value in
    link_replacement are from/to, respectively.)

    If the string contents don't look like a Heat template, return the
    contents unmodified.
    """

    template = {}
    try:
        template = yaml.safe_load(contents)
    except yaml.YAMLError:
        return contents

    if not (isinstance(template, dict) and
            template.get('heat_template_version')):
        return contents

    template = replace_links_in_template(template, link_replacement)

    return yaml.safe_dump(template)

Comparing the contents of the content of:

'template = yaml.safe_load(contents)'

'TenantInterfaceRoutes': {'default': [], 'description': "Routes for the tenant network traffic. JSON route e.g. [{'destination':'10.0.0.0/16', 'nexthop':'10.0.0.1'}] Unless the default is changed, the parameter is automatically resolved from the subnet host_routes attribute.\n", 'type': 'json'},

to:

'template = replace_links_in_template(template, link_replacement)'

'TenantInterfaceRoutes': {'default': <map object at 0x7fa165c1b898>, 'description': "Routes for the tenant network traffic. JSON route e.g. [{'destination':'10.0.0.0/16', 'nexthop':'10.0.0.1'}] Unless the default is changed, the parameter is automatically resolved from the subnet host_routes attribute.\n", 'type': 'json'},

Issue seems to be happening during 'replace_links_in_template'.

I think the issue is in the way lists are handled:

def replace_links_in_template(template_part, link_replacement):
    """Replace get_file and type file links in a Heat template

    Scan the template for 'get_file' and 'type' occurrences, and
    replace the file paths according to link_replacement
    dict. (Key/value in link_replacement are from/to, respectively.)
    """

    def replaced_dict_value(key, value):
        if ((key == 'get_file' or key == 'type') and
                isinstance(value, six.string_types)):
            return link_replacement.get(value, value)
        else:
            return replace_links_in_template(value, link_replacement)

    def replaced_list_value(value):
        return replace_links_in_template(value, link_replacement)

    if isinstance(template_part, dict):
        return {k: replaced_dict_value(k, v)
                for k, v in six.iteritems(template_part)}
    elif isinstance(template_part, list):
        return map(replaced_list_value, template_part)

    else:
        return template_part

both empty and non-empty lists are ending up like:

template_part: []

replaced value: <function replace_links_in_template.<locals>.replaced_list_value at 0x7fea0d31a378>
...

template_part:

[{'type': 'interface', 'name': 'nic1', 'use_dhcp': False, 'dns_servers': {'get_param': 'DnsServers'}, 'addresses': [{'ip_netmask': {'list_join': ['/', [{'get_param': 'ControlPlaneIp'}, {'get_param': 'ControlPlaneSubnetCidr'}]]}}], 'routes': [{'ip_netmask': '0.0.0.0/0', 'next_hop': {'get_param': 'ControlPlaneDefaultRoute'}, 'default': True}, {'ip_netmask': '169.254.169.254/32', 'next_hop': {'get_param': 'EC2MetadataIp'}}]}, {'type': 'ovs_bridge', 'name': 'br-isolated', 'use_dhcp': False, 'members': [{'type': 'interface', 'name': 'nic2', 'primary': True}, {'type': 'vlan', 'vlan_id': {'get_param': 'InternalApiNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'InternalApiIpSubnet'}}]}, {'type': 'vlan', 'vlan_id': {'get_param': 'StorageNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'StorageIpSubnet'}}]}, {'type': 'vlan', 'vlan_id': {'get_param': 'StorageMgmtNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'StorageMgmtIpSubnet'}}]}, {'type': 'vlan', 'vlan_id': {'get_param': 'TenantNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'TenantIpSubnet'}}]}]}, {'type': 'ovs_bridge', 'name': 'br-ex', 'use_dhcp': False, 'addresses': [{'ip_netmask': {'get_param': 'ExternalIpSubnet'}}], 'routes': [{'ip_netmask': '0.0.0.0/0', 'next_hop': {'get_param': 'ExternalInterfaceDefaultRoute'}}], 'members': [{'type': 'interface', 'name': 'nic3', 'primary': True}]}]

replaced value:

<function replace_links_in_template.<locals>.replaced_list_value at 0x7fea0d31ab70>

I can reproduce that with a simple unit test change:

diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py
index 080b496f..038277f7 100644
--- a/tripleoclient/tests/test_utils.py
+++ b/tripleoclient/tests/test_utils.py
@@ -811,6 +811,10 @@ class TestReplaceLinks(TestCase):
         source = (
             'description: my template\n'
             'heat_template_version: "2014-10-16"\n'
+ 'parameters:\n'
+ ' foo:\n'
+ ' default: []\n'
+ ' type: json\n'
             'resources:\n'
             ' test_config:\n'
             ' properties:\n'

@lmiccini - Would this fix it?

diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py
index 066e74ce..0b0c13de 100644
--- a/tripleoclient/utils.py
+++ b/tripleoclient/utils.py
@@ -901,7 +901,9 @@ def replace_links_in_template(template_part, link_replacement):
         return {k: replaced_dict_value(k, v)
                 for k, v in six.iteritems(template_part)}
     elif isinstance(template_part, list):
- return map(replaced_list_value, template_part)
+ # NOTE(hjensas): If the list have no items there is nothing to replace.
+ if template_part:
+ return map(replaced_list_value, template_part)
     else:
         return template_part

If I get it right we are doing some recursive function calling, trying to find dicts with 'get_file' or 'type' and replace things based on link_replacement. If the template_part is a list, and it's empty there is nothing to replace. So we can simply return the template_part?

hey Harald, thanks but I think we also need to account for non-empty lists, like the second example in https://bugs.launchpad.net/tripleo/+bug/1819737/comments/5:

[{'type': 'interface', 'name': 'nic1', 'use_dhcp': False, 'dns_servers': {'get_param': 'DnsServers'}, 'addresses': [{'ip_netmask': {'list_join': ['/', [{'get_param': 'ControlPlaneIp'}, {'get_param': 'ControlPlaneSubnetCidr'}]]}}], 'routes': [{'ip_netmask': '0.0.0.0/0', 'next_hop': {'get_param': 'ControlPlaneDefaultRoute'}, 'default': True}, {'ip_netmask': '169.254.169.254/32', 'next_hop': {'get_param': 'EC2MetadataIp'}}]}, {'type': 'ovs_bridge', 'name': 'br-isolated', 'use_dhcp': False, 'members': [{'type': 'interface', 'name': 'nic2', 'primary': True}, {'type': 'vlan', 'vlan_id': {'get_param': 'InternalApiNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'InternalApiIpSubnet'}}]}, {'type': 'vlan', 'vlan_id': {'get_param': 'StorageNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'StorageIpSubnet'}}]}, {'type': 'vlan', 'vlan_id': {'get_param': 'StorageMgmtNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'StorageMgmtIpSubnet'}}]}, {'type': 'vlan', 'vlan_id': {'get_param': 'TenantNetworkVlanID'}, 'addresses': [{'ip_netmask': {'get_param': 'TenantIpSubnet'}}]}]}, {'type': 'ovs_bridge', 'name': 'br-ex', 'use_dhcp': False, 'addresses': [{'ip_netmask': {'get_param': 'ExternalIpSubnet'}}], 'routes': [{'ip_netmask': '0.0.0.0/0', 'next_hop': {'get_param': 'ExternalInterfaceDefaultRoute'}}], 'members': [{'type': 'interface', 'name': 'nic3', 'primary': True}]}]

that still ends up:

<function replace_links_in_template.<locals>.replaced_list_value at 0x7fea0d31ab70>

IMHO we should either check if the content needs to be replaced at all, or maybe skip the replacement completely if the list comes from a nic-config templates. wdyt?

tbh I'm failing to understand the lists processing case, what is supposed to do to the returned iterator with the function mapped?..

(that's definitely not yaml.safe_dump'ing that iterator!)

Fix proposed to branch: master
Review: https://review.openstack.org/643091

It's quite surprising to me that we only hit this now, the code has been in place for a while. Luca, did you hit the error while using Python 3 by any chance? I didn't dive very deep into the code but by the looks of it, i'd suspect some Py2/Py3 difference where Py2 would return a list, but Py3 returns an intermediate object that still needs to be forced into a list.

Jiří is correct in his comment, this is a py2 vs py3 issue:

Here is a reproducer:

import six

def test_func(part, replacement):

    def replaced_dict_value(key, value):
        if ((key == 'get_file' or key == 'type') and
                isinstance(value, six.string_types)):
            return link_replacement.get(value, value)
        else:
            return test_func(value, replacement)

    def replaced_list_value(value):
        return test_func(value, replacement)

    if isinstance(part, dict):
        return {k: replaced_dict_value(k, v)
                for k, v in six.iteritems(part)}
    if isinstance(part, list):
       return map(replaced_list_value, part)

result = test_func({'key1': {'nest': []}}, 'replace')

print(result)

$ python3 test.py
{'key1': {'nest': <map object at 0x7fcc149acbe0>}}
$ python2 test.py
{'key1': {'nest': []}}

indeed this happens on py3.

Related fix proposed to branch: master
Review: https://review.openstack.org/643250

Change abandoned by Bogdan Dobrelya (<email address hidden>) on branch: master
Review: https://review.openstack.org/643250
Reason: ok prolly not needed as join() also derefs it :)

Reviewed: https://review.openstack.org/643091
Committed: https://git.openstack.org/cgit/openstack/python-tripleoclient/commit/?id=f388fceeeff5f9c2fe56ea1c6a592a735e136f01
Submitter: Zuul
Branch: master

commit f388fceeeff5f9c2fe56ea1c6a592a735e136f01
Author: Bogdan Dobrelya <email address hidden>
Date: Wed Mar 13 15:37:07 2019 +0100

    Force list when returning map(replaced_list_value, template_part)

    As suggested by Jiri in this review adding a list() around
    'map(replaced_list_value, template_part)' fixes the py3 issue
    for me.

    Closes-bug: #1819737
    Change-Id: I1241985eac1aa7e092a11db5a443da0055ff0141

This issue was fixed in the openstack/python-tripleoclient 11.4.0 release.