Fix adding qcow2 format without need

Currently nodepool is defaulting to the qcow2 if no format was given
in the diskimage config. However nodepool also can auto-detect the
image format based on clouds.yaml. This is not taken into account
leading to qcow2 added when using raw-only clouds and auto-detection.

This can be fixed by delaying the fallback to the point where we
actually do the build.

Change-Id: Ic1d9b5abd837a3225cc13da30a80ebf1dda61b64

nodepool/config.py

@@ -116,9 +116,6 @@ class Config(ConfigValue):
             if not isinstance(d.env_vars, dict):
                 d.env_vars = {}
             d.image_types = set(diskimage.get('formats', []))
-            # Ensure at least qcow2 is set to be passed to qemu-img
-            if not d.image_types:
-                d.image_types.add('qcow2')
             d.pause = bool(diskimage.get('pause', False))
             d.username = diskimage.get('username', 'zuul')
             self.diskimages[d.name] = d
@@ -250,6 +247,13 @@ def loadConfig(config_path):
     newconfig.setLabels(config.get('labels'))
     newconfig.setProviders(config.get('providers'))
 
+    # Ensure at least qcow2 is set to be passed to qemu-img.
+    # Note that this needs to be after setting the providers as they
+    # add their supported image types to the diskimages.
+    for diskimage in newconfig.diskimages.values():
+        if not diskimage.image_types:
+            diskimage.image_types.add('qcow2')
+
     return newconfig
 
 

nodepool/tests/fixtures/node_diskimage_formats.yaml

@@ -0,0 +1,66 @@
+elements-dir: .
+images-dir: '{images_dir}'
+build-log-dir: '{build_log_dir}'
+
+zookeeper-servers:
+  - host: {zookeeper_host}
+    port: {zookeeper_port}
+    chroot: {zookeeper_chroot}
+
+labels:
+  - name: fake-label-default-format
+    min-ready: 1
+  - name: fake-label-vhd
+    min-ready: 1
+
+providers:
+  - name: fake-provider-default-format
+    cloud: fake
+    driver: fake
+    region-name: fake-region
+    rate: 0.0001
+    diskimages:
+      - name: fake-image-default-format
+    pools:
+      - name: main
+        max-servers: 96
+        labels:
+          - name: fake-label-default-format
+            diskimage: fake-image-default-format
+            min-ram: 8192
+  - name: fake-provider-vhd
+    cloud: fake-vhd
+    driver: fake
+    region-name: fake-region
+    rate: 0.0001
+    diskimages:
+      - name: fake-image-vhd
+    pools:
+      - name: main
+        max-servers: 96
+        labels:
+          - name: fake-label-vhd
+            diskimage: fake-image-vhd
+            min-ram: 8192
+
+diskimages:
+  - name: fake-image-default-format
+    elements:
+      - fedora
+      - vm
+    release: 21
+    env-vars:
+      TMPDIR: /opt/dib_tmp
+      DIB_IMAGE_CACHE: /opt/dib_cache
+      DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
+      BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2
+  - name: fake-image-vhd
+    elements:
+      - fedora
+      - vm
+    release: 21
+    env-vars:
+      TMPDIR: /opt/dib_tmp
+      DIB_IMAGE_CACHE: /opt/dib_cache
+      DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
+      BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2

nodepool/tests/fixtures/node_diskimage_only.yaml

@@ -24,3 +24,14 @@ diskimages:
       DIB_IMAGE_CACHE: /opt/dib_cache
       DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
       BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2
+
+  - name: fake-image-default-format
+    elements:
+      - fedora
+      - vm
+    release: 21
+    env-vars:
+      TMPDIR: /opt/dib_tmp
+      DIB_IMAGE_CACHE: /opt/dib_cache
+      DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
+      BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2

nodepool/tests/test_builder.py

@@ -312,4 +312,19 @@ class TestNodePoolBuilder(tests.DBTestCase):
     def test_diskimage_build_only(self):
         configfile = self.setup_config('node_diskimage_only.yaml')
         self.useBuilder(configfile)
-        self.waitForBuild('fake-image', '0000000001')
+        build_tar = self.waitForBuild('fake-image', '0000000001')
+        build_default = self.waitForBuild('fake-image-default-format',
+                                          '0000000001')
+
+        self.assertEqual(build_tar._formats, ['tar'])
+        self.assertEqual(build_default._formats, ['qcow2'])
+
+    def test_diskimage_build_formats(self):
+        configfile = self.setup_config('node_diskimage_formats.yaml')
+        self.useBuilder(configfile)
+        build_default = self.waitForBuild('fake-image-default-format',
+                                          '0000000001')
+        build_vhd = self.waitForBuild('fake-image-vhd', '0000000001')
+
+        self.assertEqual(build_default._formats, ['qcow2'])
+        self.assertEqual(build_vhd._formats, ['vhd'])

releasenotes/notes/default-format-fb859338909defb9.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Nodepool now defaults to building qcow2 diskimages instead of failing if
+    the diskimage doesn't specify an image format and the diskimage isn't used
+    by any provider. This makes it more convenient to build images without
+    uploading them to a cloud provider.