Kernel doesn't have 'static' it has 'none'

[donnydavis]

Credit for this patch kevindtimm@gmail.com
https://launchpadlibrarian.net/385938425/0001-change-none-to-static-for-ip-address-in-GRUB_CMDLINE.patch

Closes bug : https://bugs.launchpad.net/cloud-init/+bug/1785275

Abandon PR #138 in favor of this one

[igalic]

👍

[donnydavis]

Is there something holding this patch from being merged?

[igalic]

@donnydavis, yes: holidays.

[blackboxsw]

Thanks for this commit @donnydavis, as with your PR #138, please add unit tests

And per your comment linked here #138 (comment) it sounds like you've signed the CLA. In order to track the accountability of that signature, we need to link your launchpad account name to your github name

please run ./tools/migrate-lp-user-to-github <your_launchpad_user> <your_github_user> to allow us to link your LP account to github account.

Ok I signed the CLA and updated the unit tests. Apologies for the messy commits - used to git review

[blackboxsw]

Thanks for the re-submit @donnydavis Needs unit test coverage and CLA signing

[donnydavis]

So do I need to write a separate unit test? Isn't the one that is already run sufficient? All this does is correct the behavior of the linux kernel passing 'none' when a static ip address is used. And if I run this migration from LP to github will it mess anything up with my launchpad acct or does it just link things?

[blackboxsw]

So do I need to write a separate unit test? Isn't the one that is already run sufficient? All this does is correct the behavior of the linux kernel passing 'none' when a static ip address is used. And if I run this migration from LP to github will it mess anything up with my launchpad acct or does it just link things?

@donnydavis sorry I missed this response. The migrate-lp-to-github script only creates an identical pull request on luanchpad and github so we can validate you control both accounts (and then we can validated that the Launchpad user had indeed signed the CLA). It only adds a single documentation line into the file tools/.lp-to-git-user which contains your username mapping. It does no other work. We manually merge your pull request into the github project and move on with your PR reviews.

[blackboxsw]

Can we instead make this an elif clause so we don't double check a proto that we just setup because it was absent?

Suggested change

	elif proto == 'none':
	proto = 'static'

[blackboxsw]

Here's the unit test I'd like to see which shows that cloud-init parses None. It may be nice to have a comment in either the unit test or at the place in net/cmdline where we are overriding 'none' to 'static'

# Some platforms vsphere can provide a authoconf=none on the commandline
# LP: 1785275

diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 01119e0ad..abd008325 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -79,9 +79,9 @@ DHCP6_EXPECTED_1 = {
                  'type': 'dhcp6'}]}
 
 
-STATIC_CONTENT_1 = """
+STATIC_CONTENT_1_TMPL = """
 DEVICE='eth1'
-PROTO='static'
+PROTO='{proto}'
 IPV4ADDR='10.0.0.2'
 IPV4BROADCAST='10.0.0.255'
 IPV4NETMASK='255.255.255.0'
@@ -3909,13 +3909,16 @@ class TestCmdlineConfigParsing(CiTestCase):
         self.assertEqual(found, ('eno1', DHCP6_EXPECTED_1))
 
     def test_cmdline_convert_static(self):
-        found = cmdline._klibc_to_config_entry(STATIC_CONTENT_1)
-        self.assertEqual(found, ('eth1', STATIC_EXPECTED_1))
+        for proto in ('static', 'none'):
+            found = cmdline._klibc_to_config_entry(
+                STATIC_CONTENT_1_TMPL.format(proto=proto))
+            self.assertEqual(found, ('eth1', STATIC_EXPECTED_1))
 
     def test_config_from_cmdline_net_cfg(self):
         files = []
         pairs = (('net-eth0.cfg', DHCP_CONTENT_1),
-                 ('net-eth1.cfg', STATIC_CONTENT_1))
+                 ('net-eth1.cfg', STATIC_CONTENT_1_TMPL.format(
+                     proto='static')))
 
         macs = {'eth1': 'b8:ae:ed:75:ff:2b',
                 'eth0': 'b8:ae:ed:75:ff:2a'}

[OddBloke]

I believe that this patch has been made obsolete by #201 landing, so I'm closing this out. Thanks for your contribution!

[donnydavis]

happy to see this bug resolved