← Back to team overview

openstack team mailing list archive

Re: [OpenStack] [Glance] Reuse the image id when recreate an image in Glance which had been deleted


The option to "really delete", delete with malice, etc. is something that we've seen use for in many scenarios (to free up the UUID specifically). I don't have +1 capability, but high five :)

Richard Goodwin
Product Manager – Cloud Servers & Cloud Images
Ideation | Intellection | Activator | Relator | Responsibility
Phone: (512) 788 5403 – Cell: (512) 736-7897 (Austin)
Skype: rtgoodwin<file:///callto/::rtgoodwin> - Yahoo: richardtgoodwin<file:///ymsgr/sendim%3Frichardtgoodwin>
AIM: dellovision - IRC: goody / rgoodwin

From: Fei Long Wang <flwang@xxxxxxxxxx<mailto:flwang@xxxxxxxxxx>>
Date: Thursday, May 16, 2013 9:38 AM
To: "openstack@xxxxxxxxxxxxxxxxxxx<mailto:openstack@xxxxxxxxxxxxxxxxxxx>" <openstack@xxxxxxxxxxxxxxxxxxx<mailto:openstack@xxxxxxxxxxxxxxxxxxx>>
Subject: [Openstack] [OpenStack] [Glance] Reuse the image id when recreate an image in Glance which had been deleted

Hi stackers,

Now I'm running into an issue about reusing image id in Glance. Based on current design, the image id can be specified during create image by end user. But now there is a problem, which is tracked by defect https://bugs.launchpad.net/glance/+bug/1176978. In summary, if user want to port an image from VMware vCenter which has been used by many VMs. So when user port the image into Glance, it would be nice if the original image id with UUID format from vCenter can be used. But if the image is removed and recreated again, he will run into current issue. I have discussed it with flaper87 in Glance IRC channel, below is the chat log. FYI. And I also attached the patch, please help review it. Any comment/suggestion is welcome. Thanks.

Chat log between flaper87 and me.
(07:46:34 AM) flwang: btw, may I get your thought about this defect? https://bugs.launchpad.net/glance/+bug/1176978
(07:47:48 AM) flwang: about if the image id should be reused after the image is deleted
(07:49:18 AM) flaper87: flwang: it shouldn't, images are never *truly* deleted, see https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/models.py#L65
(07:49:27 AM) flwang: I see
(07:49:39 AM) flaper87: that's the BaseModel
(07:49:43 AM) flwang: I know it's just marked 1 in the deleted column
(07:49:45 AM) flaper87: and Image inherits from it
(07:49:52 AM) flaper87: correct
(07:50:21 AM) flaper87: there were some discussions about completely delelting those images (cleaning up the db)
(07:50:23 AM) flwang: now there is a scenario
(07:50:34 AM) flaper87: but nothing has been decided yet
(07:51:28 AM) flaper87: TBH, I don't like the idea of people passing the id on the create command, but that's another story for another time
(07:51:30 AM) flwang: user is porting some images from vmware vcenter, and then these images are deleted
(07:51:49 AM) flaper87: flwang: not sure I understand
(07:52:00 AM) flaper87: flwang: you mean, import image from vmware into glance
(07:52:04 AM) flwang: but next, user want to add these images again with the image id given by vcenter
(07:52:05 AM) flaper87: then delete image in glance
(07:52:09 AM) flwang: yes
(07:53:20 AM) flwang: anyway, if user want to re-register an image with its given id from other place, he will run into problem based on current code
(07:54:10 AM) flaper87: flwang: yep, I guess passing the ID makes sense when you've 3K services pointing to that specific ID and you want them to use glance
(07:54:35 AM) flwang: exactly
(07:55:19 AM) flaper87: mmh, interesting. I'm trying to find the bp / discussion about "really deleting" images. I can't find it
(07:58:04 AM) flwang: for short term, I want to reuse the id
(07:59:03 AM) flwang: when create new image, glance will try to check if there is a dead image if there is a given image id
(07:59:14 AM) flwang: if yes, reuse id
(07:59:55 AM) flwang: any concern?
(08:00:01 AM) flaper87: might make sense. It would be cool if you could write this to the mailing list so that other folks can comment. Make sure your point the use-cases and ideas there
(08:00:19 AM) ***flaper87 is still trying to find that stuff
(08:00:22 AM) flaper87: arrgh
(08:00:24 AM) flaper87: internet is sooo big
(08:02:23 AM) flwang: good suggestion, I will send it to mailing list. Actually, I have worked out the patch, and I think it works fine.
(08:03:57 AM) flaper87: If you have some code to show, I'd suggest to either push it somewhere in GH or submitting the patch (thing is that this change, most likely, requires a blueprint so, I'd rather get some feedback before going down that road)
(08:13:45 AM) flwang: I'm going to submit the patch as the fix of this bug, make sense?
(08:14:30 AM) flaper87: mmh, yeah, that makes sense. Expect some discussions, though.


diff -rupN glance-unpatched/glance/db/sqlalchemy/api.py glance/glance/db/sqlalchemy/api.py
--- glance-unpatched/glance/db/sqlalchemy/api.py 2013-04-17 19:45:48.175132054 -0500
+++ glance/glance/db/sqlalchemy/api.py 2013-04-17 19:46:03.905136572 -0500
@@ -235,7 +235,34 @@ def wrap_db_error(f):

 def image_create(context, values):
     """Create an image from the values dictionary."""
-    return _image_update(context, values, None, False)
+    image_id = values.get('id', None)
+    if image_id:
+        session = get_session()
+        with session.begin():
+            # The create image request comes with an ID, since these are
+            # supposed to be universal IDs, this may mean that a user is
+            # trying to re-add an image that he had previously deleted.
+            # Let's let him do that, by "undeleting" the image.
+            try:
+                dead_image = _image_get(context,
+                                       image_id,
+                                       session=session,
+                                       force_show_deleted=True)
+                LOG.debug("Dead Image: %s" % dead_image.deleted)
+                if dead_image.deleted:
+                    LOG.info("Image '%s' had been deleted before "
+                             "and will be reactivated." % image_id)
+                    dead_image.undelete(session=session)
+            except exception.NotFound:
+                LOG.debug("Adding new image with user provided "
+                          "UUID '%s'" % image_id)
+                # Not really a failure, this just means the user provided a
+                # UUID for the very first time, still let's null it out
+                # for _image+_update() because even if the user provided
+                # an ID, this method does not expect the ID here when creating
+                # new images
+                image_id = None
+    return _image_update(context, values, image_id, False)

 def image_update(context, image_id, values, purge_props=False):
diff -rupN glance-unpatched/glance/db/sqlalchemy/models.py glance/glance/db/sqlalchemy/models.py
--- glance-unpatched/glance/db/sqlalchemy/models.py 2013-04-27 19:45:48.175132054 -0500
+++ glance/glance/db/sqlalchemy/models.py 2013-04-17 19:46:03.918174498 -0500
@@ -66,6 +66,13 @@ class ModelBase(object):
         self.deleted_at = timeutils.utcnow()

+    def undelete(self, session=None):
+        """Undelete this object"""
+        self.deleted = False
+        self.deleted_at = None
+        self.status = 'active'
+        self.save(session=session)
     def update(self, values):
         """dict.update() behaviour."""
         for k, v in values.iteritems():

Thanks & Best regards,
Fei Long Wang (王飞龙)
Scrum Master, Cloud Solutions and OpenStack Development
Tel: 8610-82450513 | T/L: 905-0513
Email: flwang@xxxxxxxxxx<mailto:flwang@xxxxxxxxxx>
China Systems & Technology Laboratory in Beijing