From de46354ed3eb8ef7489e368d0995c36e218a8d33 Mon Sep 17 00:00:00 2001
From: Sean Dague <sean@dague.net>
Date: Fri, 10 Feb 2017 16:57:34 -0500
Subject: [PATCH] new localrc set strategy

the localrc is really just executable shell, which means there are
tons of edgecases where you can't treat it the same as simple
assignment a=b.

Instead of trying to be smart about things and reduce duplication in
the localrc files, just do the naive thing and stack up all the shell
declarations in order. When evaluated in shell they will end up
stacking up as expected.

Change-Id: I231d130b24b02cdd79618f85472cee21905884e0
---
 devstack/dsconf.py                         | 57 ++++++++--------------
 devstack/tests/test_localconf_merge.py     |  4 +-
 devstack/tests/test_localconf_set_local.py |  7 +--
 3 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/devstack/dsconf.py b/devstack/dsconf.py
index 72b8d6d..8b4d6f3 100644
--- a/devstack/dsconf.py
+++ b/devstack/dsconf.py
@@ -199,36 +199,16 @@ class LocalConf(object):
 
                     if re.match(re.escape("[[local|localrc]]"), line):
                         in_meta = True
-                        writer.write(line)
-                    elif re.match("\[\[.*\|.*\]\]", line):
-                        # if we're not done yet, we
-                        if in_meta:
-                            func(writer, None)
-                            writer.write(line)
-                            done = True
-                        in_meta = False
-                        continue
-                    elif re.match("\s*%s\s*\=" % re.escape(name), line):
-                        # we found our key, pass to the writer func
-                        func(writer, line)
+                    elif in_meta and re.match(re.escape("[["), line):
+                        func(writer, None)
                         done = True
-                    else:
-                        # write out whatever we find
-                        writer.write(line)
+                        in_meta = False
+
+                    # otherwise, just write what we found
+                    writer.write(line)
             if not done:
                 func(writer, None)
 
-    def set_local(self, name, value):
-        if not os.path.exists(self.fname):
-            with open(self.fname, "w+") as writer:
-                writer.write("[[local|localrc]]\n")
-                writer.write("%s=%s\n" % (name, value))
-                return
-
-        def _do_set(writer, line):
-            writer.write("%s=%s\n" % (name, value))
-        self._at_insert_point_local(name, _do_set)
-
     def set_local_raw(self, line):
         if not os.path.exists(self.fname):
             with open(self.fname, "w+") as writer:
@@ -312,19 +292,20 @@ class LocalConf(object):
         for group, conf in groups:
             if group == "local":
                 for line in lc._section(group, conf):
-                    if line.startswith('#'):
-                        continue
-                    m = re.match(r"([^#=\s]+)\s*\=\s*(.+)", line)
+                    self.set_local_raw(line)
+                    # if line.startswith('#'):
+                    #     continue
+                    # m = re.match(r"([^#=\s]+)\s*\=\s*(.+)", line)
 
-                    if m:
-                        self.set_local(m.group(1), m.group(2))
-                    elif re.match("(enable|disable)", line):
-                        # special case appending enable* disable*
-                        # function lines
-                        self.set_local_raw(line)
-                    else:
-                        print("SKIPPING ``%s`` from '%s'" %
-                              (line.lstrip(), lcfile))
+                    # if m:
+                    #     self.set_local(m.group(1), m.group(2))
+                    # elif re.match("(enable|disable)", line):
+                    #     # special case appending enable* disable*
+                    #     # function lines
+                    #     self.set_local_raw(line)
+                    # else:
+                    #     print("SKIPPING ``%s`` from '%s'" %
+                    #           (line.lstrip(), lcfile))
             else:
                 for section, name, value in lc._conf(group, conf):
                     self.set(group, conf, section, name, value)
diff --git a/devstack/tests/test_localconf_merge.py b/devstack/tests/test_localconf_merge.py
index d1ad75f..a89e36c 100644
--- a/devstack/tests/test_localconf_merge.py
+++ b/devstack/tests/test_localconf_merge.py
@@ -57,9 +57,10 @@ TEMPEST_PLUGINS+=" /opt/stack/new/ironic"
 
 RESULT1 = """
 [[local|localrc]]
-a=5
+a=b
 c=d
 f=1
+a=5
 g=2
 [[post-config|$NEUTRON_CONF]]
 [DEFAULT]
@@ -77,6 +78,7 @@ RESULT2 = """
 a=b
 c=d
 f=1
+# some other comment
 enable_plugin ironic https://github.com/openstack/ironic
 TEMPEST_PLUGINS+=" /opt/stack/new/ironic"
 [[post-config|$NEUTRON_CONF]]
diff --git a/devstack/tests/test_localconf_set_local.py b/devstack/tests/test_localconf_set_local.py
index daf6c59..8e9f019 100644
--- a/devstack/tests/test_localconf_set_local.py
+++ b/devstack/tests/test_localconf_set_local.py
@@ -51,9 +51,10 @@ compute = auto
 
 RESULT2 = """
 [[local|localrc]]
-a=2
+a=b
 c=d
 f=1
+a=2
 [[post-config|$NEUTRON_CONF]]
 [DEFAULT]
 global_physnet_mtu=1450
@@ -89,14 +90,14 @@ class TestLcSet(testtools.TestCase):
 
     def test_set_new(self):
         conf = dsconf.LocalConf(self._path)
-        conf.set_local("g", "2")
+        conf.set_local_raw("g=2")
         with open(self._path) as f:
             content = f.read()
             self.assertEqual(content, RESULT1)
 
     def test_set_existing(self):
         conf = dsconf.LocalConf(self._path)
-        conf.set_local("a", "2")
+        conf.set_local_raw("a=2")
         with open(self._path) as f:
             content = f.read()
             self.assertEqual(content, RESULT2)