From d39cc6d7ceb31d30aa1923c04033b727427529bc Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Mon, 2 Jul 2018 14:46:39 -0400
Subject: [PATCH] Fix for referencing cloud image by ID

For pre-existing cloud images (not managed by nodepool), referencing
them by ID was failing since they could not be found with this data,
only by name.

Current code expects the shade get_image() call to accept a dict with
an 'id' key, which will return that same dict without any provider API
calls. This dict can then be used in createServer() to bypass looking
up the image to get the image ID. However, shade does not accept a dict
for this purpose, but an object with an 'id' attribute. This is
possibly a bug in shade to not accept a dict. But since nodepool knows
whether or not it has an ID (image-id) vs. an image name (image-name),
it can bypass shade altogether when image-id is used in the config.

Note: There is currently no image ID validation done before image
creation when an image-id value is supplied. Not even shade validated
the image ID with a passed in object. Server creation will fail with
an easily identifiable message about this, though.

Change-Id: I732026d1a305c71af53917285f4ebb2beaf3341d
Story: 2002013
Task: 19653
---
 nodepool/driver/openstack/config.py                    |  8 --------
 nodepool/driver/openstack/handler.py                   | 10 +++++++++-
 nodepool/driver/openstack/provider.py                  |  9 ++++++++-
 .../notes/unmanaged_image_id-cf916620abc630e4.yaml     |  6 ++++++
 4 files changed, 23 insertions(+), 10 deletions(-)
 create mode 100644 releasenotes/notes/unmanaged_image_id-cf916620abc630e4.yaml

diff --git a/nodepool/driver/openstack/config.py b/nodepool/driver/openstack/config.py
index 1fe030faa..5aaf53c81 100644
--- a/nodepool/driver/openstack/config.py
+++ b/nodepool/driver/openstack/config.py
@@ -69,14 +69,6 @@ class ProviderCloudImage(ConfigValue):
     def __repr__(self):
         return "<ProviderCloudImage %s>" % 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.'''
diff --git a/nodepool/driver/openstack/handler.py b/nodepool/driver/openstack/handler.py
index 6fe2128b4..acc8fd043 100644
--- a/nodepool/driver/openstack/handler.py
+++ b/nodepool/driver/openstack/handler.py
@@ -76,6 +76,8 @@ class OpenStackNodeLauncher(NodeLauncher):
                 )
 
             config_drive = diskimage.config_drive
+            # Using a dict with the ID bypasses an image search during
+            # server creation.
             image_external = dict(id=cloud_image.external_id)
             image_id = "{path}/{upload_id}".format(
                 path=self.handler.zk._imageUploadPath(
@@ -92,7 +94,13 @@ class OpenStackNodeLauncher(NodeLauncher):
             # launch using unmanaged cloud image
             config_drive = self.label.cloud_image.config_drive
 
-            image_external = self.label.cloud_image.external
+            if self.label.cloud_image.image_id:
+                # Using a dict with the ID bypasses an image search during
+                # server creation.
+                image_external = dict(id=self.label.cloud_image.image_id)
+            else:
+                image_external = self.label.cloud_image.external_name
+
             image_id = self.label.cloud_image.name
             image_name = self.label.cloud_image.name
             username = self.label.cloud_image.username
diff --git a/nodepool/driver/openstack/provider.py b/nodepool/driver/openstack/provider.py
index 62fe6ade4..550e2eaa7 100755
--- a/nodepool/driver/openstack/provider.py
+++ b/nodepool/driver/openstack/provider.py
@@ -388,7 +388,14 @@ class OpenStackProvider(Provider):
     def labelReady(self, label):
         if not label.cloud_image:
             return False
-        image = self.getImage(label.cloud_image.external)
+
+        # If an image ID was supplied, we'll assume it is ready since
+        # we don't currently have a way of validating that (except during
+        # server creation).
+        if label.cloud_image.image_id:
+            return True
+
+        image = self.getImage(label.cloud_image.external_name)
         if not image:
             self.log.warning(
                 "Provider %s is configured to use %s as the"
diff --git a/releasenotes/notes/unmanaged_image_id-cf916620abc630e4.yaml b/releasenotes/notes/unmanaged_image_id-cf916620abc630e4.yaml
new file mode 100644
index 000000000..5bb1a35c2
--- /dev/null
+++ b/releasenotes/notes/unmanaged_image_id-cf916620abc630e4.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    For pre-existing cloud images (not managed by nodepool), referencing
+    them by ID was failing since they could not be found with this data,
+    only by name.