Data loss of properties during normal server snapshot process

[Jay Pipes]

This is a critical issue.

Identified during testing with Tempest on current devstack. If you create a server and then take a snapshot of that server to create a new image, the properties (metadata in the Compute API) originally given to the image during image "registration" (to get an image ID) are marked deleted when the snapshot image file is actually uploaded.

In Tempest's test_image_metadata test cases, this becomes obvious, and the results are not good.

Basically, the test case does this:

POST /servers/{$server_id}/action

with {'createImage': {'name': 'image12345', 'metadata': {'key1': 'value1', 'key2': 'value2'}}} as the request body.

The call succeeds and a new image is created. However, the 'key1' and 'key2' metadata keys are conspicuously absent from the output of GET /images/{$image_id}/metadata, along with a number of other important metadata items added when the original image registration occurred. Here is what the set of image properties for one such image looks like:

mysql> select deleted_at, deleted, name, value from image_properties where image_id = 'f411e336-fdd2-4d9b-8fd6-78dfe6809d3b';
+------------+---------+----------------+-------------------------------------------------------------------------+
| deleted_at | deleted | name | value |
+------------+---------+----------------+-------------------------------------------------------------------------+
| NULL | 1 | backup_type | NULL |
| NULL | 0 | image_location | snapshot |
| NULL | 0 | image_state | available |
| NULL | 1 | image_type | snapshot |
| NULL | 1 | instance_ref | http://127.0.0.1:8774/v1.1/servers/60fc2546-a436-461d-a7b6-36285c8379ae |
| NULL | 1 | instance_uuid | 60fc2546-a436-461d-a7b6-36285c8379ae |
| NULL | 0 | kernel_id | 89ab6edc-1d2e-437f-a2e7-13bb1a2d3e01 |
| NULL | 1 | key1 | value1 |
| NULL | 1 | key2 | value2 |
| NULL | 0 | owner_id | admin |
| NULL | 0 | ramdisk_id | NULL |
| NULL | 1 | user_id | admin |
+------------+---------+----------------+-------------------------------------------------------------------------+

Note a number of things:

a) The deleted_at column is NULL even for the properties that have deleted=1

b) The deleted column is 1 for properties that were "added" to the image on the original call to POST /images

  * backup_type, image_type, instance_ref, instance_uuid, user_id, key1, and key2 are all passed to the original call to register a snapshot of server 60fc2546-a436-461d-a7b6-36285c8379ae.
  * image_location, image_state, kernel_id, ramdisk_id and owner_id are given in the call to PUT /images/f411e336-fdd2-4d9b-8fd6-78dfe6809d3b when the actual snapshot image file is uploaded

This is all due to the following code in the glance.api.v1.images.Controller.update() method:

        orig_image_meta = self.get_image_meta_or_404(req, id)
        orig_status = orig_image_meta['status']

        if image_data is not None and orig_status != 'queued':
            raise HTTPConflict(_("Cannot upload to an unqueued image"))

        try:
            image_meta = registry.update_image_metadata(req.context, id,
                                                        image_meta, True)
            if image_data is not None:
                image_meta = self._upload_and_activate(req, image_meta)

Note that the call to registry.update_image_metadata(), the last parameter is True. That parameter is the purge_props parameter and causes every previously-set property to be marked deleted :(

The easiest solution is one of the following:

1) Have the client set the X-Glance-Registry-Purge-Props header manually and change True to a boolean of whether that header is present

2) If an image file is being uploaded, do not purge existing props

Neither is particularly palatable, but unfortunately we need to deal with this in some non-force-purge-props way...

[Thierry Carrez (ttx)]

https://review.openstack.org/#change,2207

[Openstack Gerrit (openstack-gerrit)]

Reviewed: https://review.openstack.org/2207
Committed: http://github.com/openstack/glance/commit/5620ddff24ecf783f37922aa435d71d301b27269
Submitter: Jenkins
Branch: master

 status fixcommitted
 done

commit 5620ddff24ecf783f37922aa435d71d301b27269
Author: Jay Pipes <email address hidden>
Date: Thu Dec 8 17:20:50 2011 -0500

    Fixes LP Bug#901534 - Lost properties in upload

    Adds test cases that verify the bug behaviour.

    Adds some logic to the main API images controller
    update() method that does the following:

    * If an image file is being uploaded, do not purge
      existing properties when updating metadata. The idea
      behind this change is that a very common scenario is
      to register an image entry with Glance with some
      properties (like instance_uuid, instance_type, etc) and
      then immediately upload an image (usually snapshots).
      We don't want to mark deleted the originally-registered
      properties during the image upload, which was what was
      happening.

    * Add ability to force Glance NOT to purge properties
      when calling PUT /images/<IMAGE_ID> if the
      X-Glance-Registry-Purge-Props: false
      header is passed.

    Change-Id: Ie66af8f052ab40d5dca7a3235fdbbb7de20372de

[Openstack Gerrit (openstack-gerrit)]

Reviewed: https://review.openstack.org/2379
Committed: http://github.com/openstack/glance/commit/9958c137a865dff62ae4efd95c75900f0d5c0827
Submitter: Jenkins
Branch: milestone-proposed

 tag in-milestone-proposed
 done

commit 9958c137a865dff62ae4efd95c75900f0d5c0827
Author: Jay Pipes <email address hidden>
Date: Thu Dec 8 17:20:50 2011 -0500

    Fixes LP Bug#901534 - Lost properties in upload

    Adds test cases that verify the bug behaviour.

    Adds some logic to the main API images controller
    update() method that does the following:

    * If an image file is being uploaded, do not purge
      existing properties when updating metadata. The idea
      behind this change is that a very common scenario is
      to register an image entry with Glance with some
      properties (like instance_uuid, instance_type, etc) and
      then immediately upload an image (usually snapshots).
      We don't want to mark deleted the originally-registered
      properties during the image upload, which was what was
      happening.

    * Add ability to force Glance NOT to purge properties
      when calling PUT /images/<IMAGE_ID> if the
      X-Glance-Registry-Purge-Props: false
      header is passed.

    Change-Id: Ie66af8f052ab40d5dca7a3235fdbbb7de20372de