Merge ~kurt-easygo/cloud-init:fix-network-state-dhcp into cloud-init:master

[Kurt Stieger]

net: append type:dhcp[46] only if dhcp[46] is True in v2 netconfig

When providing netplan configuration to cloud-init, the internal
network state would enable DHCP if the 'dhcp' key was present in
the source config. In netplan, dhcp[46] is a boolean and the
value of the boolean should control whether DHCP is enabled rather
than the presence of the key. This issue leaded to inconsistant
sysconfig/network-scripts on fedora. 'BOOTPROTO' was always 'dhcp',
even if the address config was static.

After this change a dhcp subnet is added only if the 'dhcp' setting
in source cfg dict is True.

LP: #1818032

[Ryan Harper (raharper)]

Hi,
Thank you for contributing to cloud-init.

To contribute, you must sign the Canonical Contributor License Agreement
(CLA) [1].

If you have already signed it as an individual, your Launchpad user will
be listed in the contributor-agreement-canonical launchpad group [2].
Unfortunately there is no easy way to check if an organization or company
you are doing work for has signed. If you are unsure or have questions,
email <email address hidden> or ping rharper in #cloud-init channel
via freenode.

For information on how to sign, please see the HACKING document [3].

Thanks again, and please feel free to reach out with any questions.

–
[1] http://www.canonical.com/contributors
[2] https://launchpad.net/~contributor-agreement-canonical/+members
[3] http://cloudinit.readthedocs.io/en/latest/topics/hacking.html

[Kurt Stieger (kurt-easygo)]

Hi Ryan,
I signed the Agreement a few minutes ago. Don't know how long it takes
until I am listed on the contributor list.
Thank you for testing and verifying my change.

Greetings,
Kurt

On Thu, 28 Feb 2019 at 18:54 Ryan Harper <email address hidden> wrote:

> Hi,
> Thank you for contributing to cloud-init.
>
> To contribute, you must sign the Canonical Contributor License Agreement
> (CLA) [1].
>
> If you have already signed it as an individual, your Launchpad user will
> be listed in the contributor-agreement-canonical launchpad group [2].
> Unfortunately there is no easy way to check if an organization or company
> you are doing work for has signed. If you are unsure or have questions,
> email <email address hidden> or ping rharper in #cloud-init channel
> via freenode.
>
> For information on how to sign, please see the HACKING document [3].
>
> Thanks again, and please feel free to reach out with any questions.
>
> –
> [1] http://www.canonical.com/contributors
> [2] https://launchpad.net/~contributor-agreement-canonical/+members
> [3] http://cloudinit.readthedocs.io/en/latest/topics/hacking.html
> --
>
> https://code.launchpad.net/~kurt-easygo/cloud-init/+git/cloud-init/+merge/363732
> You are the owner of ~kurt-easygo/cloud-init:fix-network-state-dhcp.
>
--
Beste Grüße,

Kurt Stieger
Hinterwaldnerstraße 26/8
A-6020 Innsbruck

[Ryan Harper (raharper)]

Thanks.

I've added a unittest for this scenario, if you'd like to apply that to your branch and update.

http://paste.ubuntu.com/p/xGH5CgWFGM/

After you do, I'll have our CI run against this and if it checks out OK, and LP has updated your CLA status, we can accept.

Thanks again for the contribution.

bde6968... by Kurt Stieger on 2019-02-28

ADD: unitest for fix-network-state-dhcp
     thanks to rharper

[Kurt Stieger (kurt-easygo)]

Thanks.
Patch already applied and committed.

/Kurt

On Thu, 28 Feb 2019 at 21:04 Ryan Harper <email address hidden> wrote:

> Thanks.
>
> I've added a unittest for this scenario, if you'd like to apply that to
> your branch and update.
>
> http://paste.ubuntu.com/p/xGH5CgWFGM/
>
> After you do, I'll have our CI run against this and if it checks out OK,
> and LP has updated your CLA status, we can accept.
>
> Thanks again for the contribution.
> --
>
> https://code.launchpad.net/~kurt-easygo/cloud-init/+git/cloud-init/+merge/363732
> You are the owner of ~kurt-easygo/cloud-init:fix-network-state-dhcp.
>
--
Beste Grüße,

Kurt Stieger
Hinterwaldnerstraße 26/8
A-6020 Innsbruck

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:bde69680d50f0d2fe5c04014ea1527ed7d5b501f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/606/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/606/rebuild

review: Approve (continuous-integration)

[Kurt Stieger (kurt-easygo)]

> PASSED: Continuous integration, rev:bde69680d50f0d2fe5c04014ea1527ed7d5b501f
> https://jenkins.ubuntu.com/server/job/cloud-init-ci/606/
> Executed test runs:
> SUCCESS: Checkout
> SUCCESS: Unit & Style Tests
> SUCCESS: Ubuntu LTS: Build
> SUCCESS: Ubuntu LTS: Integration
> IN_PROGRESS: Declarative: Post Actions
>
> Click here to trigger a rebuild:
> https://jenkins.ubuntu.com/server/job/cloud-init-ci/606/rebuild

Is there additional work from my side necessary? CI looks fine.

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index f76e508..539b76d 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -706,9 +706,9 @@ class NetworkStateInterpreter(object):
         """Common ipconfig extraction from v2 to v1 subnets array."""
 
         subnets = []
-        if 'dhcp4' in cfg:
+        if cfg.get('dhcp4'):
             subnets.append({'type': 'dhcp4'})
-        if 'dhcp6' in cfg:
+        if cfg.get('dhcp6'):
             self.use_ipv6 = True
             subnets.append({'type': 'dhcp6'})
 
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index f001ae5..e3b9e02 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -406,6 +406,23 @@ network:
                 - maas
 """
 
+NETPLAN_DHCP_FALSE = """
+version: 2
+ethernets:
+  ens3:
+    match:
+      macaddress: 52:54:00:ab:cd:ef
+    dhcp4: false
+    dhcp6: false
+    addresses:
+      - 192.168.42.100/24
+      - 2001:db8::100/32
+    gateway4: 192.168.42.1
+    gateway6: 2001:db8::1
+    nameservers:
+      search: [example.com]
+      addresses: [192.168.42.53, 1.1.1.1]
+"""
 
 # Examples (and expected outputs for various renderers).
 OS_SAMPLES = [
@@ -2590,6 +2607,50 @@ USERCTL=no
         config = sysconfig.ConfigObj(nm_cfg)
         self.assertIn('ifcfg-rh', config['main']['plugins'])
 
+    def test_netplan_dhcp_false_disable_dhcp_in_state(self):
+        """netplan config with dhcp[46]: False should not add dhcp in state"""
+        net_config = yaml.load(NETPLAN_DHCP_FALSE)
+        ns = network_state.parse_net_config_data(net_config,
+                                                 skip_broken=False)
+
+        dhcp_found = [snet for iface in ns.iter_interfaces()
+                      for snet in iface['subnets'] if 'dhcp' in snet['type']]
+
+        self.assertEqual([], dhcp_found)
+
+    def test_netplan_dhcp_false_no_dhcp_in_sysconfig(self):
+        """netplan cfg with dhcp[46]: False should not have bootproto=dhcp"""
+
+        entry = {
+            'yaml': NETPLAN_DHCP_FALSE,
+            'expected_sysconfig': {
+                'ifcfg-ens3': textwrap.dedent("""\
+                   BOOTPROTO=none
+                   DEFROUTE=yes
+                   DEVICE=ens3
+                   DNS1=192.168.42.53
+                   DNS2=1.1.1.1
+                   DOMAIN=example.com
+                   GATEWAY=192.168.42.1
+                   HWADDR=52:54:00:ab:cd:ef
+                   IPADDR=192.168.42.100
+                   IPV6ADDR=2001:db8::100/32
+                   IPV6INIT=yes
+                   IPV6_DEFAULTGW=2001:db8::1
+                   NETMASK=255.255.255.0
+                   NM_CONTROLLED=no
+                   ONBOOT=yes
+                   STARTMODE=auto
+                   TYPE=Ethernet
+                   USERCTL=no
+                   """),
+            }
+        }
+
+        found = self._render_and_read(network_config=yaml.load(entry['yaml']))
+        self._compare_files_to_expected(entry['expected_sysconfig'], found)
+        self._assert_headers(found)
+
 
 class TestOpenSuseSysConfigRendering(CiTestCase):