systemd_networkd network template doesn't allow multiple static routes

Bug #2045819 reported by Jimmy McCrory
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack-Ansible
Fix Released
High
Dmitriy Rabotyagov

Bug Description

The systemd-network.j2 template loops static routes into individual "[Route]" sections.
https://github.com/openstack/ansible-role-systemd_networkd/blob/master/templates/systemd-network.j2#L13-L19

Duplicate section names doesn't seem to be supported by the config_template module, so all keys are written within a single Route section.

In yoga, the lxc_container_create role contained this template and used the 'template' module which writes the file as expected.

https://github.com/openstack/openstack-ansible-lxc_container_create/blob/stable/yoga/tasks/lxc_container_network_new.yml#L21-L28

This was migrated in zed to use the systemd_network role, which using config_template.

https://github.com/openstack/openstack-ansible-lxc_container_create/commit/517b75ac615d7a1bf9968853188f308193da7117

https://github.com/openstack/ansible-role-systemd_networkd/blob/stable/zed/tasks/main.yml#L155-L164

---
Given:

"static_routes": [
    {
        "cidr": "10.91.0.0/26",
        "gateway": "10.91.0.1"
    },
    {
        "cidr": "10.89.202.64/28",
        "gateway": "10.91.0.1"
    }
]

Expected:

[Route]
Destination=10.91.0.0/26
Gateway=10.91.0.1
Metric=21

[Route]
Destination=10.89.202.64/28
Gateway=10.91.0.1
Metric=22

Actual:

[Route]
Destination = 10.89.202.64/28
Destination = 10.91.0.0/26
Gateway = 10.91.0.1
Metric = 21
Metric = 22

description: updated
description: updated
description: updated
Changed in openstack-ansible:
status: New → Confirmed
importance: Undecided → High
Revision history for this message
Dmitriy Rabotyagov (noonedeadpunk) wrote :

Thanks for reporting that.

That is indeed the case with config_template. I've looked back at systemd_networkd role and it was using config_template almost from the very beginning.

How config_template works, is that it uses Python ConfigParser for .ini files, which is generally do not contain duplicated sections, and structure is being parsed as a simple dictionary.
Though we already modify default type that is being passed to ConfigParser from a dictionary to custom type parented from OrderedDict.

What was changed there, is that sections are actually merged together, so keys from the section names that are same appear in that section all together, since value is quite trivial to transform.

But the problem is that section names act as a dictionary key, that must be unique. Maybe we can somehow trick around and represent data as sets while still have dict interface, though it could be slightly tricky.

Another way around is that we can indeed just use template for systemd_networkd role, though overrides won't be possible there which is also nasty downside.

I will have a look at what can be done shortly.

Changed in openstack-ansible:
assignee: nobody → Dmitriy Rabotyagov (noonedeadpunk)
Revision history for this message
Dmitriy Rabotyagov (noonedeadpunk) wrote :

Ok, eventually patching config_template in this way doesn't make much practical sense, as it will be completely confused if you'd ask to apply override for some section, while multiple occurrences are there. So doing a set out of these pretty much kills override functionality.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ansible-role-systemd_networkd (master)
Changed in openstack-ansible:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ansible-role-systemd_networkd (master)

Reviewed: https://review.opendev.org/c/openstack/ansible-role-systemd_networkd/+/903232
Committed: https://opendev.org/openstack/ansible-role-systemd_networkd/commit/70442c5efb34222d0e333422fc469608795c70e4
Submitter: "Zuul (22348)"
Branch: master

commit 70442c5efb34222d0e333422fc469608795c70e4
Author: Dmitriy Rabotyagov <email address hidden>
Date: Fri Dec 8 18:47:35 2023 +0100

    Fix defenition of multiple static routes for network

    Current logic was relying on iteration inside the template. However,
    since config_template module was used to deliver network configuration
    it was merging sections having same name together.

    While this behaviour is correct one for config_template as all sections
    must be unique from ConfigParser perspective and in order to apply
    overrides properly, it was not suiting the way how routes should be
    defined in networkd configuration.

    To workaround the issue we place routes separately under <network>.d
    directory, which should be supported by systemd [1]

    [1] https://www.freedesktop.org/software/systemd/man/latest/systemd.network.html

    Closes-Bug: #2045819
    Change-Id: I01aa44dcdc85e32d18dd52bcd4878a9017fb6ead

Changed in openstack-ansible:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ansible-role-systemd_networkd (stable/2023.2)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ansible-role-systemd_networkd (stable/2023.1)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ansible-role-systemd_networkd (stable/2023.1)

Reviewed: https://review.opendev.org/c/openstack/ansible-role-systemd_networkd/+/903779
Committed: https://opendev.org/openstack/ansible-role-systemd_networkd/commit/597adb7b2d08db2b1aa5f9f33f2009dc6b78135c
Submitter: "Zuul (22348)"
Branch: stable/2023.1

commit 597adb7b2d08db2b1aa5f9f33f2009dc6b78135c
Author: Dmitriy Rabotyagov <email address hidden>
Date: Fri Dec 8 18:47:35 2023 +0100

    Fix defenition of multiple static routes for network

    Current logic was relying on iteration inside the template. However,
    since config_template module was used to deliver network configuration
    it was merging sections having same name together.

    While this behaviour is correct one for config_template as all sections
    must be unique from ConfigParser perspective and in order to apply
    overrides properly, it was not suiting the way how routes should be
    defined in networkd configuration.

    To workaround the issue we place routes separately under <network>.d
    directory, which should be supported by systemd [1]

    [1] https://www.freedesktop.org/software/systemd/man/latest/systemd.network.html

    Closes-Bug: #2045819
    Change-Id: I01aa44dcdc85e32d18dd52bcd4878a9017fb6ead
    (cherry picked from commit 70442c5efb34222d0e333422fc469608795c70e4)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ansible-role-systemd_networkd (stable/2023.2)

Reviewed: https://review.opendev.org/c/openstack/ansible-role-systemd_networkd/+/903754
Committed: https://opendev.org/openstack/ansible-role-systemd_networkd/commit/a05e610cc53239f0c62dc0aae64d00f1d8e75be0
Submitter: "Zuul (22348)"
Branch: stable/2023.2

commit a05e610cc53239f0c62dc0aae64d00f1d8e75be0
Author: Dmitriy Rabotyagov <email address hidden>
Date: Fri Dec 8 18:47:35 2023 +0100

    Fix defenition of multiple static routes for network

    Current logic was relying on iteration inside the template. However,
    since config_template module was used to deliver network configuration
    it was merging sections having same name together.

    While this behaviour is correct one for config_template as all sections
    must be unique from ConfigParser perspective and in order to apply
    overrides properly, it was not suiting the way how routes should be
    defined in networkd configuration.

    To workaround the issue we place routes separately under <network>.d
    directory, which should be supported by systemd [1]

    [1] https://www.freedesktop.org/software/systemd/man/latest/systemd.network.html

    Closes-Bug: #2045819
    Change-Id: I01aa44dcdc85e32d18dd52bcd4878a9017fb6ead
    (cherry picked from commit 70442c5efb34222d0e333422fc469608795c70e4)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.