Open Stack

Thanks Alex for your detailed inspection of my work. Comments inline..

On 3 February 2015 at 21:32, Alexandre Levine <alevine at cloudscaling.com>
wrote:

>  I'm writing this in regard to several reviews concering tagging
> functionality for EC2 API in nova.
> The list of the reviews concerned is here:
>
> https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/ec2-volume-and-snapshot-tags,n,z
>
> I don't think it's a good idea to merge these reviews. The analysis is
> below:
>
> *Tagging in AWS*
>
> Main goal for the tagging functionality in AWS is to be able to
> efficiently distinguish various resources based on user-defined criteria:
>
> "Tags enable you to categorize your AWS resources in different ways, for
> example, by purpose, owner, or environment.
> ...
> You can search and filter the resources based on the tags you add."
>
> (quoted from here:
> http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html)
>
> It means that one of the two main use-cases is to be able to use Tags as
> filter when you describe something. Another one is to be able to get
> information about particular tag with all of the resources tagged by it.
> Also there is a constraint:
>
> "You can tag public or shared resources, but the tags you assign are
> available only to your AWS account and not to the other accounts sharing
> the resource."
> The important part here is "shared resources" which are visible to
> different users but tags are not shared - each user sees his own.
>
>
>
> *Existing implementation in nova *Existing implementation of tags in
> nova's EC2 API covers only instances. But it does so in both areas:
> 1. Tags management (create, delete, describe,...)
> 2. Instances filtering (describe_instances with filtering by tags).
> The implementation is based on storing tags in each instance's metadata.
> And nova DB sqlalchemy level uses "tag:" in queries to allow instances
> describing with tag filters.
>
> I see the following design flaws in existing implementation:
>
> 1. It uses instance's own metadata for storing information about assigned
> tags.
> Problems:
> - it doesn't scale when you want to start using tags for other resources.
> Following this design decision you'll have to store tags in other resources
> metadata, which mean different services APIs and other databases. So
> performance for searching for tags or tagged resources in main use cases
> should suffer. You'll have to search through several remote APIs, querying
> different metadatas to collect all info and then to compile the result.
> - instances are not shared resources, but images are. It means that, when
> developed, metadata for images will have to store different tags for
> different users somehow.
>
> 2. EC2-specific code ("tag:" searching in novaDB sqlalchemy) leaked into
> lower layers of nova.
> - layering is violated. There should be no EC2-specifics below EC2 API
> library in nova, ideally.
>
All of the Nova-EC2 mapping happens in Nova's DB currently. See
InstanceIdMapping model in nova/db/sqlalchemy/models.py. EC2 API which
resides in Nova will keep using the Nova database as long as it is
functional.

> - each other service will have to implement the same solution in its own
> DB level to support tagging for EC2 API.
>
> *Proposed review changes*
>
> The review in question introduces tagging for volumes and snapshots. It
> follows design decisions of existing instance tagging implementation, but
> realizes only one of the two use cases. It provides "create", "delete",
> "describe" for tags. But it doesn't provide describe_volumes or
> describe_snapshots for filtering.
>
I honestly forgot about those two methods. I can implement them.

>
> It suffers from the design flaws I listed above. It has to query remote
> API (cinder) for metadata. It didn't implement filtering by "tag:" in
> cinder DB level so we don't see implementation of describe_volumes with
> tags filtering.
>
Cinder do support filtering based on tags, and I marked the work as TODO in
https://review.openstack.org/#/c/112325/23/nova/volume/cinder.py . This was
not the reason why I didn't implement describe_volumes and
describe_snapshots. Those two methods just missed my attention :)

Nova's EC2 API's tag filtering is also done in-memory presently if I'm
correct, as Nova's API doesn't support filtering only on the basis of tag
names or tag values alone..

>
> *Current stackforge/ec2-api tagging implementation*
>
> In comparison, the implementation of tagging in stackforge/ec2-api, stores
> all of the tags and their links to resources and users in a separate place.
> So we can efficiently list tags and its resources or filter by tags during
> describing of some of the resources. Also user-specific tagging is
> supported.
>
>
>
> *Conclusion *Keeping in mind all of the above, and seeing your discussion
> about deprecation of EC2 API in nova, I don't feel it's a good time to add
> such a half-baked code with some potential problems into nova.
> I think it's better to concentrate on cleaning up, fixing, reviving and
> making bullet-proof whatever functionality is currently present in nova for
> EC2 and used by clients.
>
EC2 API already shares database with Nova's, so the tight coupling between
EC2 API and Nova's database is not going to go away till the time EC2 API
server/controller is present in Nova. Nova instance metadata is being used
as EC2 instance tags, and what the above-referenced spec is doing is is
very similar: Cinder volume metadata is being used as EC2 volume tags, and
similarly for volume snapshots. I don't see a difference between instances
and volumes and volume snapshots in the sense that they all are
non-share-able (yet).

I completely understand that these patches look like feature additions. I
started working on them first in January 2014 (
https://review.openstack.org/#/c/64690/ ), and at that time it was just a
sincere effort to improve EC2 API using the first possible way I could find
out. Since we have not deprecated the in-Nova EC2 support yet, and we are
yet to reach a concrete plan to move forward, I am tempted to ask for
allowing this patch to be considered for review..

I am fine if people think these patches shouldn't be allowed to go in. I
can only wish that the patches got more attention when it was possible to
get them merged :)

Regards,
Rushi Agrawal


> Best regards,
>   Alex Levine
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> 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/20150204/6ce54b8d/attachment.html>