From 340df68a7b9d3d2fcaec9e48931bd4bb6cfe8cd7 Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Tue, 17 Mar 2020 18:52:44 +1100 Subject: [PATCH] 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 | 10 ++- nodepool/config.py | 9 +-- .../duplicate_diskimage_name.yaml | 70 +++++++++++++++++++ .../config_validate/no_diskimage_name.yaml | 68 ++++++++++++++++++ .../tests/unit/test_config_comparisons.py | 6 +- nodepool/tests/unit/test_config_validator.py | 18 +++++ 6 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 nodepool/tests/fixtures/config_validate/duplicate_diskimage_name.yaml create mode 100644 nodepool/tests/fixtures/config_validate/no_diskimage_name.yaml diff --git a/nodepool/cmd/config_validator.py b/nodepool/cmd/config_validator.py index 4fd12228b..f6cefb42c 100644 --- a/nodepool/cmd/config_validator.py +++ b/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: diff --git a/nodepool/config.py b/nodepool/config.py index 771182c51..9fe35dd48 100644 --- a/nodepool/config.py +++ b/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 diff --git a/nodepool/tests/fixtures/config_validate/duplicate_diskimage_name.yaml b/nodepool/tests/fixtures/config_validate/duplicate_diskimage_name.yaml new file mode 100644 index 000000000..fb2e21328 --- /dev/null +++ b/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 diff --git a/nodepool/tests/fixtures/config_validate/no_diskimage_name.yaml b/nodepool/tests/fixtures/config_validate/no_diskimage_name.yaml new file mode 100644 index 000000000..e4e4f071b --- /dev/null +++ b/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 diff --git a/nodepool/tests/unit/test_config_comparisons.py b/nodepool/tests/unit/test_config_comparisons.py index 674df51b6..26b6a3617 100644 --- a/nodepool/tests/unit/test_config_comparisons.py +++ b/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): diff --git a/nodepool/tests/unit/test_config_validator.py b/nodepool/tests/unit/test_config_validator.py index 289369ae4..8eeb54cbc 100644 --- a/nodepool/tests/unit/test_config_validator.py +++ b/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',