Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix metric setting of nmconnection for rhel #5878

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xiachen-rh
Copy link
Contributor

@xiachen-rh xiachen-rh commented Nov 15, 2024

Most RHEL systems use network manager to bring up manage network connections.
Network Manager uses route-metric for the default route and route/plen[,gateway,metric] for the additional static routes, please see
https://networkmanager.dev/docs/api/latest/nm-settings-nmcli.html
https://networkmanager.dev/docs/api/latest/nm-settings-keyfile.html

This changes ensures that cloud-init generates nmconnection files with route metric settings that are compatible with RHEL and network manager.

Fixes: GH-5877

Test steps

For the config data

#cloud-config

network:
  version: 1
  config:
    - type: physical
      name: enp1s0
      subnets:
      - type: static
        address: 10.0.2.2/24
        gateway: 10.0.2.1
        routes:
          - destination: 0.0.0.0/0
            gateway: 10.0.2.1
            metric: 99
          - destination: 192.168.122.1/24
            gateway: 10.0.2.1
            metric: 98

After this changes the nmconnetion file looks as below

# cat /etc/NetworkManager/system-connections/cloud-init-enp1s0.nmconnection
# Generated by cloud-init. Changes will be lost.

[connection]
id=cloud-init enp1s0
uuid=a41601f3-3acc-5f60-ac5f-9d9011ab7c25
autoconnect-priority=120
type=ethernet
interface-name=enp1s0

[user]
org.freedesktop.NetworkManager.origin=cloud-init

[ethernet]

[ipv4]
method=manual
may-fail=false
address1=10.0.2.2/24
gateway=10.0.2.1
route-metric=99
route1=0.0.0.0/0,10.0.2.1
route2=192.168.122.1/24,10.0.2.1,98

and the routes are configured with metric

$ ip r show
default via 10.0.2.1 dev enp1s0 proto static metric 99 
10.0.2.0/24 dev enp1s0 proto kernel scope link src 10.0.2.2 metric 99 
192.168.122.0/24 via 10.0.2.1 dev enp1s0 proto static metric 98 

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

Most RHEL systems use network manager to bring up manage network connections.
Network Manager uses route-metric for the default route
and route/plen[,gateway,metric] for the additional static routes,
please see
https://networkmanager.dev/docs/api/latest/nm-settings-nmcli.html
https://networkmanager.dev/docs/api/latest/nm-settings-keyfile.html

This changes ensures that cloud-init generates nmconnection files with
route metric settings that are compatible with RHEL and network manager.

Fixes: canonicalGH-5877

Signed-off-by: Amy Chen <xiachen@redhat.com>
@xiachen-rh
Copy link
Contributor Author

Hi @ani-sinha, could you please help to take a review?

@@ -420,6 +421,7 @@
[ipv4]
method=auto
may-fail=false
route-metric=10000
route1=0.0.0.0/0,65.61.151.37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the metric value were added here, it would have been the same effect since it also specifies default route. I think we should do it this way instead of adding route-metric . In other words, there is no need to check for default route and add route-metric. All matric values can be added as a comma separated value.

@@ -245,6 +249,11 @@ def _add_route(self, route):
value = f'{route["network"]}/{route["prefix"]}'
if "gateway" in route:
value += f',{route["gateway"]}'
if "metric" in route:
if self._is_default_route(route):
self.config[family]["route-metric"] = f'{route["metric"]}'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we really want to add route-metric separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuring metric for default gateway is not working with NM renderer
2 participants