diskimage: make name primary key

Ensure 'name' is a primary key for diskimages.

Change the constructor to take the name as an argument.  Update the
config validator to ensure there is a name, and that it is unique.

Add tests for both these cases.

Change-Id: I3931dc1457c023154cde0df2bb7b0a41cc6f20d3

nodepool/cmd/config_validator.py

@@ -41,7 +41,7 @@ class ConfigValidator:
         }
 
         diskimage = {
-            'name': str,
+            v.Required('name'): str,
             'dib-cmd': str,
             'pause': bool,
             'elements': [str],
@@ -113,6 +113,14 @@ class ConfigValidator:
                                   "not in top-level labels" %
                                   (label['name'], provider['name']))
 
+        diskimages = {}
+        for diskimage in config.get('diskimages', []):
+            name = diskimage['name']
+            if name in diskimages:
+                log.error("diskimage %s already defined" % name)
+                errors = True
+            diskimages[name] = diskimage
+
         if errors is True:
             return 1
         else:

nodepool/config.py

@@ -99,8 +99,9 @@ class Config(ConfigValue):
             return
 
         for diskimage in diskimages_cfg:
-            d = DiskImage()
-            d.name = diskimage['name']
+            name = diskimage['name']
+            d = DiskImage(name)
+
             if 'elements' in diskimage:
                 d.elements = u' '.join(diskimage['elements'])
             else:
@@ -171,8 +172,8 @@ class Label(ConfigValue):
 
 
 class DiskImage(ConfigValue):
-    def __init__(self):
-        self.name = None
+    def __init__(self, name):
+        self.name = name
         self.build_timeout = None
         self.dib_cmd = None
         self.elements = None

nodepool/tests/fixtures/config_validate/duplicate_diskimage_name.yaml

@@ -0,0 +1,70 @@
+elements-dir: /etc/nodepool/elements
+images-dir: /opt/nodepool_dib
+
+webapp:
+  port: 8005
+  listen_address: '0.0.0.0'
+
+zookeeper-servers:
+  - host: zk1.openstack.org
+    port: 2181
+    chroot: /test
+
+labels:
+  - name: trusty
+    max-ready-age: 3600
+    min-ready: 1
+
+providers:
+  - name: cloud1
+    driver: openstack
+    cloud: vanilla-cloud
+    region-name: 'vanilla'
+    boot-timeout: 120
+    max-concurrency: 10
+    launch-retries: 3
+    port-cleanup-interval: 600
+    rate: 1
+    diskimages:
+      - name: trusty
+    pools:
+      - name: main
+        max-servers: 184
+        auto-floating-ip: True
+        host-key-checking: True
+        node-attributes:
+          key1: value1
+          key2: value2
+        networks:
+          - public
+          - private
+        labels:
+          - name: trusty
+            diskimage: trusty
+            min-ram: 8192
+            console-log: True
+            networks:
+              - public
+
+diskimages:
+  - name: trusty
+    formats:
+      - tar
+    pause: False
+    elements:
+      - ubuntu
+      - vm
+      - openstack-repos
+      - puppet
+      - nodepool-base
+      - cache-devstack
+    release: trusty
+    rebuild-age: 3600
+    build-timeout: 3600
+    python-path: /bin/python3.6
+    env-vars:
+      TMPDIR: /opt/dib_tmp
+      DIB_IMAGE_CACHE: /opt/dib_cache
+      QEMU_IMG_OPTIONS: compat=0.10
+  # Duplicate
+  - name: trusty

nodepool/tests/fixtures/config_validate/no_diskimage_name.yaml

@@ -0,0 +1,68 @@
+elements-dir: /etc/nodepool/elements
+images-dir: /opt/nodepool_dib
+
+webapp:
+  port: 8005
+  listen_address: '0.0.0.0'
+
+zookeeper-servers:
+  - host: zk1.openstack.org
+    port: 2181
+    chroot: /test
+
+labels:
+  - name: trusty
+    max-ready-age: 3600
+    min-ready: 1
+
+providers:
+  - name: cloud1
+    driver: openstack
+    cloud: vanilla-cloud
+    region-name: 'vanilla'
+    boot-timeout: 120
+    max-concurrency: 10
+    launch-retries: 3
+    port-cleanup-interval: 600
+    rate: 1
+    diskimages:
+      - name: trusty
+    pools:
+      - name: main
+        max-servers: 184
+        auto-floating-ip: True
+        host-key-checking: True
+        node-attributes:
+          key1: value1
+          key2: value2
+        networks:
+          - public
+          - private
+        labels:
+          - name: trusty
+            diskimage: trusty
+            min-ram: 8192
+            console-log: True
+            networks:
+              - public
+
+diskimages:
+  # No name
+  - formats:
+      - tar
+    pause: False
+    elements:
+      - ubuntu
+      - vm
+      - openstack-repos
+      - puppet
+      - nodepool-base
+      - cache-devstack
+    release: trusty
+    rebuild-age: 3600
+    build-timeout: 3600
+    python-path: /bin/python3.6
+    env-vars:
+      TMPDIR: /opt/dib_tmp
+      DIB_IMAGE_CACHE: /opt/dib_cache
+      QEMU_IMG_OPTIONS: compat=0.10

nodepool/tests/unit/test_config_comparisons.py

@@ -65,10 +65,10 @@ class TestConfigComparisons(tests.BaseTestCase):
         self.assertNotEqual(a, b)
 
     def test_DiskImage(self):
-        a = DiskImage()
-        b = DiskImage()
+        a = DiskImage('foo')
+        b = DiskImage('foo')
         self.assertEqual(a, b)
-        a.name = "foo"
+        a.name = 'bar'
         self.assertNotEqual(a, b)
 
     def test_ProviderDiskImage(self):

nodepool/tests/unit/test_config_validator.py

@@ -48,6 +48,24 @@ class TestConfigValidation(tests.BaseTestCase):
         ret = validator.validate()
         self.assertEqual(ret, 1)
 
+    def test_no_diskimage_name(self):
+        config = os.path.join(os.path.dirname(tests.__file__),
+                              'fixtures', 'config_validate',
+                              'no_diskimage_name.yaml')
+
+        validator = ConfigValidator(config)
+        ret = validator.validate()
+        self.assertEqual(ret, 1)
+
+    def test_duplicate_diskimage_name(self):
+        config = os.path.join(os.path.dirname(tests.__file__),
+                              'fixtures', 'config_validate',
+                              'duplicate_diskimage_name.yaml')
+
+        validator = ConfigValidator(config)
+        ret = validator.validate()
+        self.assertEqual(ret, 1)
+
     def test_schema(self):
         config = os.path.join(os.path.dirname(tests.__file__),
                               'fixtures', 'config_validate',