Open Stack

I wonder if the best solution here is to change the uniqueness constraint
in our tables (as has been mentioned in several other OpenStack-related
database discussions) so that deleted images do not conflict with active
images with the same uuid. As I recall, the best practice suggestion was to
key the uniqueness constraint off the pair <uuid, deleted>. In this scheme,
deleted is no longer a simple boolean, but rather either 0 (not deleted) or
equal to the auto-increment row id (deleted).

Does that make sense?


On Thu, May 16, 2013 at 8:29 AM, Fei Long Wang <flwang at cn.ibm.com> wrote:

> 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*<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*<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
> *<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: flwang at cn.ibm.com
> China Systems & Technology Laboratory in Beijing
> --------------------------------------------------
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack at lists.launchpad.net
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130517/89c2b259/attachment.html>