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 <flw...@cn.ibm.com<mailto:flw...@cn.ibm.com>>
Date: Thursday, May 16, 2013 9:38 AM
To: "openstack@lists.launchpad.net<mailto:openstack@lists.launchpad.net>" 
<openstack@lists.launchpad.net<mailto:openstack@lists.launchpad.net>>
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()
         self.save(session=session)

+    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: flw...@cn.ibm.com<mailto:flw...@cn.ibm.com>
China Systems & Technology Laboratory in Beijing
--------------------------------------------------
_______________________________________________
Mailing list: https://launchpad.net/~openstack
Post to     : openstack@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openstack
More help   : https://help.launchpad.net/ListHelp

Reply via email to