diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index c06f05924..73bdc2c51 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -542,8 +542,10 @@ Example configuration:: Refers to provider's diskimages, see :ref:`provider_diskimages`. ``cloud-image`` - Refers to an externally managed image name or id already existing on the - provider, see :ref:`provider_cloud_images`. + Refers to the name of an externally managed image in the cloud that already + exists on the provider. The value of ``cloud-image`` should match the + ``name`` of a previously configured entry from the ``cloud-images`` section + of the provider. See :ref:`provider_cloud_images`. **at least one of** diff --git a/nodepool/config.py b/nodepool/config.py index ac44420e3..2ccef9f4f 100755 --- a/nodepool/config.py +++ b/nodepool/config.py @@ -87,6 +87,19 @@ class ProviderCloudImage(ConfigValue): def __repr__(self): return "" % self.name + @property + def external(self): + '''External identifier to pass to the cloud.''' + if self.image_id: + return dict(id=self.image_id) + else: + return self.image_name or self.name + + @property + def external_name(self): + '''Human readable version of external.''' + return self.image_id or self.image_name or self.name + class Label(ConfigValue): def __repr__(self): @@ -290,7 +303,17 @@ def loadConfig(config_path): pl.diskimage = newconfig.diskimages[diskimage] else: pl.diskimage = None - pl.cloud_image = label.get('cloud-image', None) + cloud_image_name = label.get('cloud-image', None) + if cloud_image_name: + cloud_image = p.cloud_images.get(cloud_image_name, None) + if not cloud_image: + raise ValueError( + "cloud-image %s does not exist in provider %s" + " but is referenced in label %s" % + (cloud_image_name, p.name, pl.name)) + else: + cloud_image = None + pl.cloud_image = cloud_image pl.min_ram = label.get('min-ram', 0) pl.flavor_name = label.get('flavor-name', None) pl.key_name = label.get('key-name') diff --git a/nodepool/driver/openstack/handler.py b/nodepool/driver/openstack/handler.py index 2cf6024ce..3baec6292 100644 --- a/nodepool/driver/openstack/handler.py +++ b/nodepool/driver/openstack/handler.py @@ -62,7 +62,6 @@ class NodeLauncher(threading.Thread, stats.StatsReporter): self._diskimage = self._provider.diskimages[self._label.diskimage.name] else: self._diskimage = None - self._cloud_image = self._provider.cloud_images.get(self._label.cloud_image, None) def logConsole(self, server_id, hostname): if not self._label.console_log: @@ -97,22 +96,11 @@ class NodeLauncher(threading.Thread, stats.StatsReporter): else: # launch using unmanaged cloud image - config_drive = self._cloud_image.config_drive + config_drive = self._label.cloud_image.config_drive - # These are different values for zk, but it's all the same - # for cloud-images. - # image_external is what we use for OpenStack. - # image_id is what we record in the node for zk. - # image_name is what we log, so matches the config. - image_external = self._cloud_image.name - if self._cloud_image.image_id: - image_external = dict(id=self._cloud_image.image_id) - elif self._cloud_image.image_name: - image_external = self._cloud_image.image_name - else: - image_external = self._cloud_image.name - image_id = self._cloud_image.name - image_name = self._cloud_image.name + image_external = self._label.cloud_image.external + image_id = self._label.cloud_image.name + image_name = self._label.cloud_image.name # TODO(tobiash): support username also for unmanaged cloud images username = None @@ -308,13 +296,12 @@ class OpenStackNodeRequestHandler(NodeRequestHandler): for label in self.request.node_types: if self.pool.labels[label].cloud_image: - img = self.pool.labels[label].cloud_image - if not self.manager.labelReady(img): + if not self.manager.labelReady(self.pool.labels[label]): return False else: - img = self.pool.labels[label].diskimage.name - - if not self.zk.getMostRecentImageUpload(img, self.provider.name): + if not self.zk.getMostRecentImageUpload( + self.pool.labels[label].diskimage.name, + self.provider.name): return False return True diff --git a/nodepool/driver/openstack/provider.py b/nodepool/driver/openstack/provider.py index 1939e1671..8f9f9fde8 100755 --- a/nodepool/driver/openstack/provider.py +++ b/nodepool/driver/openstack/provider.py @@ -273,8 +273,20 @@ class OpenStackProvider(Provider): with shade_inner_exceptions(): return self._client.get_image(image_id) - def labelReady(self, image_id): - return self.getImage(image_id) + def labelReady(self, label): + if not label.cloud_image: + return False + image = self.getImage(label.cloud_image.external) + if not image: + self.log.warning( + "Provider %s is configured to use %s as the" + " cloud-image for label %s and that" + " cloud-image could not be found in the" + " cloud." % (self.provider.name, + label.cloud_image.external_name, + label.name)) + return False + return True def uploadImage(self, image_name, filename, image_type=None, meta=None, md5=None, sha256=None): diff --git a/nodepool/launcher.py b/nodepool/launcher.py index 4234c8315..da45fc841 100755 --- a/nodepool/launcher.py +++ b/nodepool/launcher.py @@ -796,13 +796,14 @@ class NodePool(threading.Thread): # Provider doesn't manage images, assuming label is ready return True for pool_label in pool.labels.values(): - if pool_label.cloud_image: - manager = self.getProviderManager(pool.provider.name) - if manager.labelReady(pool_label.cloud_image): + if pool_label.diskimage: + if self.zk.getMostRecentImageUpload( + pool_label.diskimage.name, pool.provider.name): + return True + else: + manager = self.getProviderManager(pool.provider.name) + if manager.labelReady(pool_label): return True - elif self.zk.getMostRecentImageUpload(pool_label.diskimage.name, - pool.provider.name): - return True return False def createMinReady(self): diff --git a/nodepool/tests/fixtures/unmanaged_image_provider_name.yaml b/nodepool/tests/fixtures/unmanaged_image_provider_name.yaml new file mode 100644 index 000000000..c95561432 --- /dev/null +++ b/nodepool/tests/fixtures/unmanaged_image_provider_name.yaml @@ -0,0 +1,28 @@ +elements-dir: . +images-dir: '{images_dir}' + +zookeeper-servers: + - host: {zookeeper_host} + port: {zookeeper_port} + chroot: {zookeeper_chroot} + +labels: + - name: fake-label + min-ready: 1 + +providers: + - name: fake-provider + cloud: fake + driver: fake + region-name: fake-region + rate: 0.0001 + cloud-images: + - name: fake-image + image-name: provider-named-image + pools: + - name: main + max-servers: 96 + labels: + - name: fake-label + cloud-image: fake-image + min-ram: 8192 diff --git a/nodepool/tests/test_launcher.py b/nodepool/tests/test_launcher.py index 5ccce5f25..6d8c9f8e3 100644 --- a/nodepool/tests/test_launcher.py +++ b/nodepool/tests/test_launcher.py @@ -647,6 +647,22 @@ class TestLauncher(tests.DBTestCase): nodes = self.waitForNodes('fake-label') self.assertEqual(len(nodes), 1) + def test_unmanaged_image_provider_name(self): + """ + Test node launching using an unmanaged image referencing the + image name as known by the provider. + """ + configfile = self.setup_config('unmanaged_image_provider_name.yaml') + pool = self.useNodepool(configfile, watermark_sleep=1) + + pool.start() + self.wait_for_config(pool) + manager = pool.getProviderManager('fake-provider') + manager._client.create_image(name="provider-named-image") + + nodes = self.waitForNodes('fake-label') + self.assertEqual(len(nodes), 1) + def test_paused_gets_declined(self): """Test that a paused request, that later gets declined, unpauses."""