cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1784699-netplan-macaddress-on-bond into cloud-init:master.

Commit message:
netplan: Correctly render macaddress on a bond when provided.

When converting network config v1 to netplan, we were not correctly
rendering the 'macaddress' key on a bond.  Not that the difference
in spelling between v1 'mac_address' and v2 'macaddress' is intentional.

LP: #1784699

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1784699 in cloud-init: "cloud-init not setting mac address for bond or bridge in bionic"
  https://bugs.launchpad.net/cloud-init/+bug/1784699

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/352033

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/1784699-netplan-macaddress-on-bond into cloud-init:master.

diff --git a/tools/net-convert.py b/cloudinit/cmd/devel/net_convert.py
index d1a4a64..5d109af 100755
--- a/tools/net-convert.py
+++ b/cloudinit/cmd/devel/net_convert.py
@@ -1,6 +1,6 @@
-#!/usr/bin/python3
 # This file is part of cloud-init. See LICENSE file for license information.
 
+"""Debug network config format conversions."""
 import argparse
 import json
 import os
@@ -9,15 +9,23 @@ import yaml
 
 from cloudinit.sources.helpers import openstack
 
-from cloudinit.net import eni
+from cloudinit.net import eni, netplan, network_state, sysconfig
 from cloudinit import log
-from cloudinit.net import netplan
-from cloudinit.net import network_state
-from cloudinit.net import sysconfig
 
+NAME = 'net-convert'
 
-def main():
-    parser = argparse.ArgumentParser()
+
+def get_parser(parser=None):
+    """Build or extend and arg parser for collect-logs utility.
+
+    @param parser: Optional existing ArgumentParser instance representing the
+        collect-logs subcommand which will be extended to support the args of
+        this utility.
+
+    @returns: ArgumentParser with proper argument configuration.
+    """
+    if not parser:
+        parser = argparse.ArgumentParser(prog=NAME, description=__doc__)
     parser.add_argument("--network-data", "-p", type=open,
                         metavar="PATH", required=True)
     parser.add_argument("--kind", "-k",
@@ -36,8 +44,10 @@ def main():
     parser.add_argument("--output-kind", "-ok",
                         choices=['eni', 'netplan', 'sysconfig'],
                         required=True)
-    args = parser.parse_args()
+    return parser
 
+
+def handle_args(name, args):
     if not args.directory.endswith("/"):
         args.directory += "/"
 
@@ -99,6 +109,8 @@ def main():
 
 
 if __name__ == '__main__':
-    main()
+    args = get_parser().parse_args()
+    handle_args(NAME, args)
+
 
 # vi: ts=4 expandtab
diff --git a/cloudinit/cmd/devel/parser.py b/cloudinit/cmd/devel/parser.py
index acacc4e..40a4b01 100644
--- a/cloudinit/cmd/devel/parser.py
+++ b/cloudinit/cmd/devel/parser.py
@@ -5,8 +5,9 @@
 """Define 'devel' subcommand argument parsers to include in cloud-init cmd."""
 
 import argparse
-from cloudinit.config.schema import (
-    get_parser as schema_parser, handle_schema_args)
+from cloudinit.config import schema
+
+from . import net_convert
 
 
 def get_parser(parser=None):
@@ -17,10 +18,15 @@ def get_parser(parser=None):
     subparsers = parser.add_subparsers(title='Subcommands', dest='subcommand')
     subparsers.required = True
 
-    parser_schema = subparsers.add_parser(
-        'schema', help='Validate cloud-config files or document schema')
-    # Construct schema subcommand parser
-    schema_parser(parser_schema)
-    parser_schema.set_defaults(action=('schema', handle_schema_args))
+    subcmds = [
+        ('schema', 'Validate cloud-config files for document schema',
+         schema.get_parser, schema.handle_schema_args),
+        (net_convert.NAME, net_convert.__doc__,
+         net_convert.get_parser, net_convert.handle_args)
+    ]
+    for (subcmd, helpmsg, get_parser, handler) in subcmds:
+        parser = subparsers.add_parser(subcmd, help=helpmsg)
+        get_parser(parser)
+        parser.set_defaults(action=(subcmd, handler))
 
     return parser
diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 4014363..be254cd 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -291,6 +291,8 @@ class Renderer(renderer.Renderer):
 
                 if len(bond_config) > 0:
                     bond.update({'parameters': bond_config})
+                if ifcfg.get('mac_address'):
+                    bond['macaddress'] = ifcfg.get('mac_address')
                 slave_interfaces = ifcfg.get('bond-slaves')
                 if slave_interfaces == 'none':
                     _extract_bond_slaves_by_name(interfaces, bond, ifname)
diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py
index 0c0f427..199d69b 100644
--- a/tests/unittests/test_cli.py
+++ b/tests/unittests/test_cli.py
@@ -208,8 +208,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase):
         for subcommand in expected_subcommands:
             self.assertIn(subcommand, error)
 
-    @mock.patch('cloudinit.config.schema.handle_schema_args')
-    def test_wb_devel_schema_subcommand_parser(self, m_schema):
+    def test_wb_devel_schema_subcommand_parser(self):
         """The subcommand cloud-init schema calls the correct subparser."""
         exit_code = self._call_main(['cloud-init', 'devel', 'schema'])
         self.assertEqual(1, exit_code)
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 5ab61cf..a0952e0 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -708,6 +708,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                         interfaces:
                         - eth1
                         - eth2
+                        macaddress: aa:bb:cc:dd:ee:ff
                         parameters:
                             mii-monitor-interval: 100
                             mode: active-backup
@@ -1075,6 +1076,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                      interfaces:
                      - bond0s0
                      - bond0s1
+                     macaddress: aa:bb:cc:dd:e8:ff
                      mtu: 9000
                      parameters:
                          mii-monitor-interval: 100