From f1836e78d20ef34b05b6aba002fc10a97eceb454 Mon Sep 17 00:00:00 2001

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Date: Mon, 8 Aug 2022 10:08:50 +0200

Subject: [PATCH 1/2] Revert "Add native NetworkManager support (#1224)"

RH-Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>

RH-MergeRequest: 81: Revert "Use Network-Manager and Netplan as default renderers for RHEL and Fedora (#1465)"

RH-Commit: [1/2] 5b3e51502a89c2dcfbc97dc08a86b792454fedd3

RH-Bugzilla: 2107464 2110066 2117526 2104393 2098624

RH-Acked-by: Eduardo Otubo <otubo@redhat.com>

RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com>

NM is not yet stable, so we don't want to support it for now.

This reverts commit 0d93e53fd05c44b62e3456b7580c9de8135e6b5a.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

---

 cloudinit/cmd/devel/net_convert.py     |   14 +-

 cloudinit/net/activators.py            |   25 +-

 cloudinit/net/network_manager.py       |  377 -------

 cloudinit/net/renderers.py             |    3 -

 cloudinit/net/sysconfig.py             |   37 +-

 tests/unittests/test_net.py            | 1268 +++---------------------

 tests/unittests/test_net_activators.py |   93 +-

 7 files changed, 193 insertions(+), 1624 deletions(-)

 delete mode 100644 cloudinit/net/network_manager.py

diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py

index 647fe07b..18b1e7ff 100755

--- a/cloudinit/cmd/devel/net_convert.py

+++ b/cloudinit/cmd/devel/net_convert.py

@@ -7,14 +7,7 @@ import os

 import sys

 from cloudinit import distros, log, safeyaml

-from cloudinit.net import (

-    eni,

-    netplan,

-    network_manager,

-    network_state,

-    networkd,

-    sysconfig,

-)

+from cloudinit.net import eni, netplan, network_state, networkd, sysconfig

 from cloudinit.sources import DataSourceAzure as azure

 from cloudinit.sources import DataSourceOVF as ovf

 from cloudinit.sources.helpers import openstack

@@ -81,7 +74,7 @@ def get_parser(parser=None):

     parser.add_argument(

         "-O",

         "--output-kind",

-        choices=["eni", "netplan", "networkd", "sysconfig", "network-manager"],

+        choices=["eni", "netplan", "networkd", "sysconfig"],

         required=True,

         help="The network config format to emit",

     )

@@ -155,9 +148,6 @@ def handle_args(name, args):

     elif args.output_kind == "sysconfig":

         r_cls = sysconfig.Renderer

         config = distro.renderer_configs.get("sysconfig")

-    elif args.output_kind == "network-manager":

-        r_cls = network_manager.Renderer

-        config = distro.renderer_configs.get("network-manager")

     else:

         raise RuntimeError("Invalid output_kind")

diff --git a/cloudinit/net/activators.py b/cloudinit/net/activators.py

index edbc0c06..e80c26df 100644

--- a/cloudinit/net/activators.py

+++ b/cloudinit/net/activators.py

@@ -1,14 +1,15 @@

 # This file is part of cloud-init. See LICENSE file for license information.

 import logging

+import os

 from abc import ABC, abstractmethod

 from typing import Iterable, List, Type

 from cloudinit import subp, util

 from cloudinit.net.eni import available as eni_available

 from cloudinit.net.netplan import available as netplan_available

-from cloudinit.net.network_manager import available as nm_available

 from cloudinit.net.network_state import NetworkState

 from cloudinit.net.networkd import available as networkd_available

+from cloudinit.net.sysconfig import NM_CFG_FILE

 LOG = logging.getLogger(__name__)

@@ -123,24 +124,20 @@ class IfUpDownActivator(NetworkActivator):

 class NetworkManagerActivator(NetworkActivator):

     @staticmethod

     def available(target=None) -> bool:

-        """Return true if NetworkManager can be used on this system."""

-        return nm_available(target=target)

+        """Return true if network manager can be used on this system."""

+        config_present = os.path.isfile(

+            subp.target_path(target, path=NM_CFG_FILE)

+        )

+        nmcli_present = subp.which("nmcli", target=target)

+        return config_present and bool(nmcli_present)

     @staticmethod

     def bring_up_interface(device_name: str) -> bool:

-        """Bring up connection using nmcli.

+        """Bring up interface using nmcli.

         Return True is successful, otherwise return False

         """

-        from cloudinit.net.network_manager import conn_filename

-

-        filename = conn_filename(device_name)

-        cmd = ["nmcli", "connection", "load", filename]

-        if _alter_interface(cmd, device_name):

-            cmd = ["nmcli", "connection", "up", "filename", filename]

-        else:

-            _alter_interface(["nmcli", "connection", "reload"], device_name)

-            cmd = ["nmcli", "connection", "up", "ifname", device_name]

+        cmd = ["nmcli", "connection", "up", "ifname", device_name]

         return _alter_interface(cmd, device_name)

     @staticmethod

@@ -149,7 +146,7 @@ class NetworkManagerActivator(NetworkActivator):

         Return True is successful, otherwise return False

         """

-        cmd = ["nmcli", "device", "disconnect", device_name]

+        cmd = ["nmcli", "connection", "down", device_name]

         return _alter_interface(cmd, device_name)

diff --git a/cloudinit/net/network_manager.py b/cloudinit/net/network_manager.py

deleted file mode 100644

index 79b0fe0b..00000000

--- a/cloudinit/net/network_manager.py

+++ /dev/null

@@ -1,377 +0,0 @@

-# Copyright 2022 Red Hat, Inc.

-#

-# Author: Lubomir Rintel <lkundrak@v3.sk>

-# Fixes and suggestions contributed by James Falcon, Neal Gompa,

-# Zbigniew Jędrzejewski-Szmek and Emanuele Giuseppe Esposito.

-#

-# This file is part of cloud-init. See LICENSE file for license information.

-

-import configparser

-import io

-import itertools

-import os

-import uuid

-

-from cloudinit import log as logging

-from cloudinit import subp, util

-

-from . import renderer

-from .network_state import is_ipv6_addr, subnet_is_ipv6

-

-NM_RUN_DIR = "/etc/NetworkManager"

-NM_LIB_DIR = "/usr/lib/NetworkManager"

-LOG = logging.getLogger(__name__)

-

-

-class NMConnection:

-    """Represents a NetworkManager connection profile."""

-

-    def __init__(self, con_id):

-        """

-        Initializes the connection with some very basic properties,

-        notably the UUID so that the connection can be referred to.

-        """

-

-        # Chosen by fair dice roll

-        CI_NM_UUID = uuid.UUID("a3924cb8-09e0-43e9-890b-77972a800108")

-

-        self.config = configparser.ConfigParser()

-        # Identity option name mapping, to achieve case sensitivity

-        self.config.optionxform = str

-

-        self.config["connection"] = {

-            "id": f"cloud-init {con_id}",

-            "uuid": str(uuid.uuid5(CI_NM_UUID, con_id)),

-        }

-

-        # This is not actually used anywhere, but may be useful in future

-        self.config["user"] = {

-            "org.freedesktop.NetworkManager.origin": "cloud-init"

-        }

-

-    def _set_default(self, section, option, value):

-        """

-        Sets a property unless it's already set, ensuring the section

-        exists.

-        """

-

-        if not self.config.has_section(section):

-            self.config[section] = {}

-        if not self.config.has_option(section, option):

-            self.config[section][option] = value

-

-    def _set_ip_method(self, family, subnet_type):

-        """

-        Ensures there's appropriate [ipv4]/[ipv6] for given family

-        appropriate for given configuration type

-        """

-

-        method_map = {

-            "static": "manual",

-            "dhcp6": "dhcp",

-            "ipv6_slaac": "auto",

-            "ipv6_dhcpv6-stateless": "auto",

-            "ipv6_dhcpv6-stateful": "auto",

-            "dhcp4": "auto",

-            "dhcp": "auto",

-        }

-

-        # Ensure we got an [ipvX] section

-        self._set_default(family, "method", "disabled")

-

-        try:

-            method = method_map[subnet_type]

-        except KeyError:

-            # What else can we do

-            method = "auto"

-            self.config[family]["may-fail"] = "true"

-

-        # Make sure we don't "downgrade" the method in case

-        # we got conflicting subnets (e.g. static along with dhcp)

-        if self.config[family]["method"] == "dhcp":

-            return

-        if self.config[family]["method"] == "auto" and method == "manual":

-            return

-

-        self.config[family]["method"] = method

-        self._set_default(family, "may-fail", "false")

-        if family == "ipv6":

-            self._set_default(family, "addr-gen-mode", "stable-privacy")

-

-    def _add_numbered(self, section, key_prefix, value):

-        """

-        Adds a numbered property, such as address<n> or route<n>, ensuring

-        the appropriate value gets used for <n>.

-        """

-

-        for index in itertools.count(1):

-            key = f"{key_prefix}{index}"

-            if not self.config.has_option(section, key):

-                self.config[section][key] = value

-                break

-

-    def _add_address(self, family, subnet):

-        """

-        Adds an ipv[46]address<n> property.

-        """

-

-        value = subnet["address"] + "/" + str(subnet["prefix"])

-        self._add_numbered(family, "address", value)

-

-    def _add_route(self, family, route):

-        """

-        Adds a ipv[46].route<n> property.

-        """

-

-        value = route["network"] + "/" + str(route["prefix"])

-        if "gateway" in route:

-            value = value + "," + route["gateway"]

-        self._add_numbered(family, "route", value)

-

-    def _add_nameserver(self, dns):

-        """

-        Extends the ipv[46].dns property with a name server.

-        """

-

-        # FIXME: the subnet contains IPv4 and IPv6 name server mixed

-        # together. We might be getting an IPv6 name server while

-        # we're dealing with an IPv4 subnet. Sort this out by figuring

-        # out the correct family and making sure a valid section exist.

-        family = "ipv6" if is_ipv6_addr(dns) else "ipv4"

-        self._set_default(family, "method", "disabled")

-

-        self._set_default(family, "dns", "")

-        self.config[family]["dns"] = self.config[family]["dns"] + dns + ";"

-

-    def _add_dns_search(self, family, dns_search):

-        """

-        Extends the ipv[46].dns-search property with a name server.

-        """

-

-        self._set_default(family, "dns-search", "")

-        self.config[family]["dns-search"] = (

-            self.config[family]["dns-search"] + ";".join(dns_search) + ";"

-        )

-

-    def con_uuid(self):

-        """

-        Returns the connection UUID

-        """

-        return self.config["connection"]["uuid"]

-

-    def valid(self):

-        """

-        Can this be serialized into a meaningful connection profile?

-        """

-        return self.config.has_option("connection", "type")

-

-    @staticmethod

-    def mac_addr(addr):

-        """

-        Sanitize a MAC address.

-        """

-        return addr.replace("-", ":").upper()

-

-    def render_interface(self, iface, renderer):

-        """

-        Integrate information from network state interface information

-        into the connection. Most of the work is done here.

-        """

-

-        # Initialize type & connectivity

-        _type_map = {

-            "physical": "ethernet",

-            "vlan": "vlan",

-            "bond": "bond",

-            "bridge": "bridge",

-            "infiniband": "infiniband",

-            "loopback": None,

-        }

-

-        if_type = _type_map[iface["type"]]

-        if if_type is None:

-            return

-        if "bond-master" in iface:

-            slave_type = "bond"

-        else:

-            slave_type = None

-

-        self.config["connection"]["type"] = if_type

-        if slave_type is not None:

-            self.config["connection"]["slave-type"] = slave_type

-            self.config["connection"]["master"] = renderer.con_ref(

-                iface[slave_type + "-master"]

-            )

-

-        # Add type specific-section

-        self.config[if_type] = {}

-

-        # These are the interface properties that map nicely

-        # to NetworkManager properties

-        _prop_map = {

-            "bond": {

-                "mode": "bond-mode",

-                "miimon": "bond_miimon",

-                "xmit_hash_policy": "bond-xmit-hash-policy",

-                "num_grat_arp": "bond-num-grat-arp",

-                "downdelay": "bond-downdelay",

-                "updelay": "bond-updelay",

-                "fail_over_mac": "bond-fail-over-mac",

-                "primary_reselect": "bond-primary-reselect",

-                "primary": "bond-primary",

-            },

-            "bridge": {

-                "stp": "bridge_stp",

-                "priority": "bridge_bridgeprio",

-            },

-            "vlan": {

-                "id": "vlan_id",

-            },

-            "ethernet": {},

-            "infiniband": {},

-        }

-

-        device_mtu = iface["mtu"]

-        ipv4_mtu = None

-

-        # Deal with Layer 3 configuration

-        for subnet in iface["subnets"]:

-            family = "ipv6" if subnet_is_ipv6(subnet) else "ipv4"

-

-            self._set_ip_method(family, subnet["type"])

-            if "address" in subnet:

-                self._add_address(family, subnet)

-            if "gateway" in subnet:

-                self.config[family]["gateway"] = subnet["gateway"]

-            for route in subnet["routes"]:

-                self._add_route(family, route)

-            if "dns_nameservers" in subnet:

-                for nameserver in subnet["dns_nameservers"]:

-                    self._add_nameserver(nameserver)

-            if "dns_search" in subnet:

-                self._add_dns_search(family, subnet["dns_search"])

-            if family == "ipv4" and "mtu" in subnet:

-                ipv4_mtu = subnet["mtu"]

-

-        if ipv4_mtu is None:

-            ipv4_mtu = device_mtu

-        if not ipv4_mtu == device_mtu:

-            LOG.warning(

-                "Network config: ignoring %s device-level mtu:%s"

-                " because ipv4 subnet-level mtu:%s provided.",

-                iface["name"],

-                device_mtu,

-                ipv4_mtu,

-            )

-

-        # Parse type-specific properties

-        for nm_prop, key in _prop_map[if_type].items():

-            if key not in iface:

-                continue

-            if iface[key] is None:

-                continue

-            if isinstance(iface[key], bool):

-                self.config[if_type][nm_prop] = (

-                    "true" if iface[key] else "false"

-                )

-            else:

-                self.config[if_type][nm_prop] = str(iface[key])

-

-        # These ones need special treatment

-        if if_type == "ethernet":

-            if iface["wakeonlan"] is True:

-                # NM_SETTING_WIRED_WAKE_ON_LAN_MAGIC

-                self.config["ethernet"]["wake-on-lan"] = str(0x40)

-            if ipv4_mtu is not None:

-                self.config["ethernet"]["mtu"] = str(ipv4_mtu)

-            if iface["mac_address"] is not None:

-                self.config["ethernet"]["mac-address"] = self.mac_addr(

-                    iface["mac_address"]

-                )

-        if if_type == "vlan" and "vlan-raw-device" in iface:

-            self.config["vlan"]["parent"] = renderer.con_ref(

-                iface["vlan-raw-device"]

-            )

-        if if_type == "bridge":

-            # Bridge is ass-backwards compared to bond

-            for port in iface["bridge_ports"]:

-                port = renderer.get_conn(port)

-                port._set_default("connection", "slave-type", "bridge")

-                port._set_default("connection", "master", self.con_uuid())

-            if iface["mac_address"] is not None:

-                self.config["bridge"]["mac-address"] = self.mac_addr(

-                    iface["mac_address"]

-                )

-        if if_type == "infiniband" and ipv4_mtu is not None:

-            self.config["infiniband"]["transport-mode"] = "datagram"

-            self.config["infiniband"]["mtu"] = str(ipv4_mtu)

-            if iface["mac_address"] is not None:

-                self.config["infiniband"]["mac-address"] = self.mac_addr(

-                    iface["mac_address"]

-                )

-

-        # Finish up

-        if if_type == "bridge" or not self.config.has_option(

-            if_type, "mac-address"

-        ):

-            self.config["connection"]["interface-name"] = iface["name"]

-

-    def dump(self):

-        """

-        Stringify.

-        """

-

-        buf = io.StringIO()

-        self.config.write(buf, space_around_delimiters=False)

-        header = "# Generated by cloud-init. Changes will be lost.\n\n"

-        return header + buf.getvalue()

-

-

-class Renderer(renderer.Renderer):

-    """Renders network information in a NetworkManager keyfile format."""

-

-    def __init__(self, config=None):

-        self.connections = {}

-

-    def get_conn(self, con_id):

-        return self.connections[con_id]

-

-    def con_ref(self, con_id):

-        if con_id in self.connections:

-            return self.connections[con_id].con_uuid()

-        else:

-            # Well, what can we do...

-            return con_id

-

-    def render_network_state(self, network_state, templates=None, target=None):

-        # First pass makes sure there's NMConnections for all known

-        # interfaces that have UUIDs that can be linked to from related

-        # interfaces

-        for iface in network_state.iter_interfaces():

-            self.connections[iface["name"]] = NMConnection(iface["name"])

-

-        # Now render the actual interface configuration

-        for iface in network_state.iter_interfaces():

-            conn = self.connections[iface["name"]]

-            conn.render_interface(iface, self)

-

-        # And finally write the files

-        for con_id, conn in self.connections.items():

-            if not conn.valid():

-                continue

-            name = conn_filename(con_id, target)

-            util.write_file(name, conn.dump(), 0o600)

-

-

-def conn_filename(con_id, target=None):

-    target_con_dir = subp.target_path(target, NM_RUN_DIR)

-    con_file = f"cloud-init-{con_id}.nmconnection"

-    return f"{target_con_dir}/system-connections/{con_file}"

-

-

-def available(target=None):

-    target_nm_dir = subp.target_path(target, NM_LIB_DIR)

-    return os.path.exists(target_nm_dir)

-

-

-# vi: ts=4 expandtab

diff --git a/cloudinit/net/renderers.py b/cloudinit/net/renderers.py

index 7edc34b5..c755f04c 100644

--- a/cloudinit/net/renderers.py

+++ b/cloudinit/net/renderers.py

@@ -8,7 +8,6 @@ from . import (

     freebsd,

     netbsd,

     netplan,

-    network_manager,

     networkd,

     openbsd,

     renderer,

@@ -20,7 +19,6 @@ NAME_TO_RENDERER = {

     "freebsd": freebsd,

     "netbsd": netbsd,

     "netplan": netplan,

-    "network-manager": network_manager,

     "networkd": networkd,

     "openbsd": openbsd,

     "sysconfig": sysconfig,

@@ -30,7 +28,6 @@ DEFAULT_PRIORITY = [

     "eni",

     "sysconfig",

     "netplan",

-    "network-manager",

     "freebsd",

     "netbsd",

     "openbsd",

diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py

index c3b0c795..362e8d19 100644

--- a/cloudinit/net/sysconfig.py

+++ b/cloudinit/net/sysconfig.py

@@ -5,6 +5,8 @@ import io

 import os

 import re

+from configobj import ConfigObj

+

 from cloudinit import log as logging

 from cloudinit import subp, util

 from cloudinit.distros.parsers import networkmanager_conf, resolv_conf

@@ -64,6 +66,24 @@ def _quote_value(value):

         return value

+def enable_ifcfg_rh(path):

+    """Add ifcfg-rh to NetworkManager.cfg plugins if main section is present"""

+    config = ConfigObj(path)

+    if "main" in config:

+        if "plugins" in config["main"]:

+            if "ifcfg-rh" in config["main"]["plugins"]:

+                return

+        else:

+            config["main"]["plugins"] = []

+

+        if isinstance(config["main"]["plugins"], list):

+            config["main"]["plugins"].append("ifcfg-rh")

+        else:

+            config["main"]["plugins"] = [config["main"]["plugins"], "ifcfg-rh"]

+        config.write()

+        LOG.debug("Enabled ifcfg-rh NetworkManager plugins")

+

+

 class ConfigMap(object):

     """Sysconfig like dictionary object."""

@@ -1011,6 +1031,8 @@ class Renderer(renderer.Renderer):

             netrules_content = self._render_persistent_net(network_state)

             netrules_path = subp.target_path(target, self.netrules_path)

             util.write_file(netrules_path, netrules_content, file_mode)

+        if available_nm(target=target):

+            enable_ifcfg_rh(subp.target_path(target, path=NM_CFG_FILE))

         sysconfig_path = subp.target_path(target, templates.get("control"))

         # Distros configuring /etc/sysconfig/network as a file e.g. Centos

@@ -1049,9 +1071,14 @@ def _supported_vlan_names(rdev, vid):

 def available(target=None):

-    if not util.system_info()["variant"] in KNOWN_DISTROS:

-        return False

+    sysconfig = available_sysconfig(target=target)

+    nm = available_nm(target=target)

+    return util.system_info()["variant"] in KNOWN_DISTROS and any(

+        [nm, sysconfig]

+    )

+

+def available_sysconfig(target=None):

     expected = ["ifup", "ifdown"]

     search = ["/sbin", "/usr/sbin"]

     for p in expected:

@@ -1068,4 +1095,10 @@ def available(target=None):

     return False

+def available_nm(target=None):

+    if not os.path.isfile(subp.target_path(target, path=NM_CFG_FILE)):

+        return False

+    return True

+

+

 # vi: ts=4 expandtab

diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py

index ef21ad76..591241b3 100644

--- a/tests/unittests/test_net.py

+++ b/tests/unittests/test_net.py

@@ -21,7 +21,6 @@ from cloudinit.net import (

     interface_has_own_mac,

     natural_sort_key,

     netplan,

-    network_manager,

     network_state,

     networkd,

     renderers,

@@ -612,37 +611,6 @@ dns = none

                 ),

             ),

         ],

-        "expected_network_manager": [

-            (

-                "".join(

-                    [

-                        "etc/NetworkManager/system-connections",

-                        "/cloud-init-eth0.nmconnection",

-                    ]

-                ),

-                """

-# Generated by cloud-init. Changes will be lost.

-

-[connection]

-id=cloud-init eth0

-uuid=1dd9a779-d327-56e1-8454-c65e2556c12c

-type=ethernet

-

-[user]

-org.freedesktop.NetworkManager.origin=cloud-init

-

-[ethernet]

-mac-address=FA:16:3E:ED:9A:59

-

-[ipv4]

-method=manual

-may-fail=false

-address1=172.19.1.34/22

-route1=0.0.0.0/0,172.19.3.254

-

-""".lstrip(),

-            ),

-        ],

     },

     {

         "in_data": {

@@ -1105,50 +1073,6 @@ NETWORK_CONFIGS = {

                 USERCTL=no"""

             ),

         },

-        "expected_network_manager": {

-            "cloud-init-eth1.nmconnection": textwrap.dedent(

-                """\

-                # Generated by cloud-init. Changes will be lost.

-

-                [connection]

-                id=cloud-init eth1

-                uuid=3c50eb47-7260-5a6d-801d-bd4f587d6b58

-                type=ethernet

-

-                [user]

-                org.freedesktop.NetworkManager.origin=cloud-init

-

-                [ethernet]

-                mac-address=CF:D6:AF:48:E8:80

-

-                """

-            ),

-            "cloud-init-eth99.nmconnection": textwrap.dedent(

-                """\

-                # Generated by cloud-init. Changes will be lost.

-

-                [connection]

-                id=cloud-init eth99

-                uuid=b1b88000-1f03-5360-8377-1a2205efffb4

-                type=ethernet

-

-                [user]

-                org.freedesktop.NetworkManager.origin=cloud-init

-

-                [ethernet]

-                mac-address=C0:D6:9F:2C:E8:80

-

-                [ipv4]

-                method=auto

-                may-fail=false

-                address1=192.168.21.3/24

-                route1=0.0.0.0/0,65.61.151.37

-                dns=8.8.8.8;8.8.4.4;

-                dns-search=barley.maas;sach.maas;

-

-                """

-            ),

-        },

         "yaml": textwrap.dedent(

             """

             version: 1

@@ -1221,34 +1145,6 @@ NETWORK_CONFIGS = {