Open Stack

Rushi,

Thank you for the response. I totally understand the effort and your 
problems with getting it through at the time. Your design is completely 
inline with what's currently present in Nova for EC2, no doubt about 
that. I did whatever I could to review your patches and consider if it's 
worth to go forward with them in current circumstances. I believe it'll 
add more complications than value if we go on with them.

The main design problems, introduced before you, won't go away:
1. User isolation for shared objects is close to impossible to implement 
in this model.
2. Marked resources listing when describing tags, will require going to 
all of the possible different APIs and their databases eventually and 
then compiling the result.

The stackforge/ec2-api implementation fortunately had no constraints or 
previously implemented code with some conceptual problems. So it could 
and did store the tags and their associations with resources separately. 
It allowed efficient searching for both describing tags and resources.

Strategically if, as I understand, eventual aim is to switch to separate 
ec2-api solution, it makes little sense (to me) to add more 
functionality, especially partial functionality (no describe_volumes, 
describe_snapshots and even if added, no tagging for other resources), 
to current nova code. If the decision was made to enhance nova with new 
features like this, I'd still be for a separate table in DB for all of 
the tags and their associations - it would've made universal, complete 
and efficient solution with one effort.

And again, I more than agree with this:
"
...I can only wish that the patches got more attention when it was 
possible to get them merged :)
"

But that's a different story.

Best regards,
   Alex Levine


On 2/4/15 4:32 PM, Rushi Agrawal wrote:
> Thanks Alex for your detailed inspection of my work. Comments inline..
>
> On 3 February 2015 at 21:32, Alexandre Levine 
> <alevine at cloudscaling.com <mailto: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://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe>
>     http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
>
>
> __________________________________________________________________________
> 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/baf3630c/attachment.html>