Stop json-encoding the nodepool metadata

When we first started putting nodepool metadata into the server record
in OpenStack, we json encoded the data so that we could store a dict
into a field that only takes strings. We were also going to teach the
ansible OpenStack Inventory about this so that it could read the data
out of the groups list. However, ansible was not crazy about accepting
"attempt to json decode values in the metadata" since json-encoded
values are not actually part of the interface OpenStack expects - which
means one of our goals, which is ansible inventory groups based on
nodepool information is no longer really a thing.

We could push harder on that, but we actually don't need the functionality
we're getting from the json encoding. The OpenStack Inventory has
supported comma separated lists of groups since before day one. And the
other nodepool info we're storing stores and fetches just as easily
with 4 different top level keys as it does in a json dict - and is
easier to read and deal with when just looking at server records.
Finally, nova has a 255 byte limit on size of the value that can be
stored, so we cannot grow the information in the nodepool dict
indefinitely anyway.

Migrate the data to store into nodepool_ variables and a comma separated
list for groups. Consume both forms, so that people upgrading will not
lose track of existing stock of nodes.

Finally, we don't use snapshot_id anymore - so remove it.

Change-Id: I2c06dc7c2faa19e27d1fb1d9d6df78da45ffa6dd

doc/source/operation.rst

@@ -77,21 +77,18 @@ When Nodepool creates instances, it will assign the following nova
 metadata:
 
   groups
-    A json-encoded list containing the name of the image and the name
+    A comma separated list containing the name of the image and the name
     of the provider.  This may be used by the Ansible OpenStack
     inventory plugin.
 
-  nodepool
-    A json-encoded dictionary with the following entries:
+  nodepool_image_name
+    The name of the image as a string.
 
-    image_name
-      The name of the image as a string.
+  nodepool_provider_name
+    The name of the provider as a string.
 
-    provider_name
-      The name of the provider as a string.
-
-    node_id
-      The nodepool id of the node as an integer.
+  nodepool_node_id
+    The nodepool id of the node as an integer.
 
 Command Line Tools
 ------------------

nodepool/nodepool.py

@@ -16,7 +16,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import json
 import logging
 import os
 import os.path
@@ -1057,15 +1056,15 @@ class NodeCleanupWorker(threading.Thread):
             known = set([n.external_id for n in zk_conn.nodeIterator() if n.provider == provider.name])
 
             for server in servers:
-                meta = server.get('metadata', {}).get('nodepool')
-                if not meta:
+                meta = server.get('metadata', {})
+
+                if 'nodepool_provider_name' not in meta:
                     self.log.debug(
-                        "Instance %s (%s) in %s has no nodepool metadata",
+                        "Instance %s (%s) in %s has no nodepool_provider_name",
                         server.name, server.id, provider.name)
                     continue
 
-                meta = json.loads(meta)
-                if meta['provider_name'] != provider.name:
+                if meta['nodepool_provider_name'] != provider.name:
                     # Another launcher, sharing this provider but configured
                     # with a different name, owns this.
                     continue

nodepool/provider_manager.py

@@ -16,7 +16,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import json
 import logging
 import paramiko
 from contextlib import contextmanager
@@ -186,8 +185,7 @@ class ProviderManager(object):
     def createServer(self, name, min_ram, image_id=None, image_name=None,
                      az=None, key_name=None, name_filter=None,
                      config_drive=None, nodepool_node_id=None,
-                     nodepool_image_name=None,
-                     nodepool_snapshot_image_id=None):
+                     nodepool_image_name=None):
         if image_name:
             image = self.findImage(image_name)
         else:
@@ -218,19 +216,18 @@ class ProviderManager(object):
         # Also list each of those values directly so that non-ansible
         # consumption programs don't need to play a game of knowing that
         # groups[0] is the image name or anything silly like that.
-        nodepool_meta = dict(provider_name=self.provider.name)
-        groups_meta = [self.provider.name]
-        if nodepool_node_id:
-            nodepool_meta['node_id'] = nodepool_node_id
-        if nodepool_snapshot_image_id:
-            nodepool_meta['snapshot_image_id'] = nodepool_snapshot_image_id
+        groups_list = [self.provider.name]
         if nodepool_image_name:
-            nodepool_meta['image_name'] = nodepool_image_name
-            groups_meta.append(nodepool_image_name)
-        create_args['meta'] = dict(
-            groups=json.dumps(groups_meta),
-            nodepool=json.dumps(nodepool_meta)
+            groups_list.append(nodepool_image_name)
+        meta = dict(
+            groups=",".join(groups_list),
+            nodepool_provider_name=self.provider.name,
         )
+        if nodepool_node_id:
+            meta['nodepool_node_id'] = nodepool_node_id
+        if nodepool_image_name:
+            meta['nodepool_image_name'] = nodepool_image_name
+        create_args['meta'] = meta
 
         with shade_inner_exceptions():
             return self._client.create_server(wait=False, **create_args)