Merge lp://qastaging/~vladimir.p/nova/admin-vm into lp://qastaging/~hudson-openstack/nova/trunk

Since this is a relatively major feature, I would like to see the blueprint expanded to describe the implementation. In particular, I want to hear why adding a 'category' to an instance is the right path to take. Why not use the metadata container?

This could also be considered an admin-only feature that doesn't belong in the end-user apis. I'm not crazy about seeing undocumented changes to the OpenStack API.

I'm pretty excited about this feature, I just want to make sure we are doing it right :)

Hi Brian,

Please take a look at short design description we put into Blueprint.

Thanks,
-Vladimir

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
Brian Waldon
Sent: Wednesday, June 08, 2011 12:58 PM
To: <email address hidden>
Subject: Re: [Merge] lp:~vladimir.p/nova/admin-vm into lp:nova

Review: Needs Information
Since this is a relatively major feature, I would like to see the blueprint
expanded to describe the implementation. In particular, I want to hear why
adding a 'category' to an instance is the right path to take. Why not use
the metadata container?

This could also be considered an admin-only feature that doesn't belong in
the end-user apis. I'm not crazy about seeing undocumented changes to the
OpenStack API.

I'm pretty excited about this feature, I just want to make sure we are doing
it right :)
--
https://code.launchpad.net/~vladimir.p/nova/admin-vm/+merge/63771
You are the owner of lp:~vladimir.p/nova/admin-vm.

Sorry it took me so long to get back to you Vladimir! Hopefully we can get the ball rolling again...

First off, there are some merge conflicts that needs to be resolved.

I'm still not quite sold on the design here. I'd like some other nova-core members to take a look at the blueprint/diff and share their thoughts.

Like I said before, I don't think we should make changes to the core OSAPI. It's looking like you will have to implement the api code as an extension unless you can convince somebody to add it to the admin api. I'd be more than happy to talk to you about this off the MP if you need some guidance here.

Looks like you still have a few merge conflicts to handle.

Merged with latest nova revision (#1187)

Hi Vladimir, I agree with Brian that you need to implement the apis as extensions.

ok, I'm fine with API extensions, but how about the general direction/idea to have instance categories (wanted to use name type, but it is already taken for describing config types/flavors)?

If having instance categories will not be approved by community there is no point in continuing in this direction.

Any feedback on categories?

> ok, I'm fine with API extensions, but how about the general direction/idea to
> have instance categories (wanted to use name type, but it is already taken for
> describing config types/flavors)?
>
> If having instance categories will not be approved by community there is no
> point in continuing in this direction.
>
> Any feedback on categories?

Personally, I feel this could be solved through the use of system-level metadata on an instance. We don't have that capability right now, but I think it would be a good fit for this. It would allow a category to be set on an instance when it needed to be, and totally ignore it when it doesn't matter. What do you think?

I suppose there are 3 general aspects for categories:
- APIs for setting categories per instance (and everything related to APIs)
- having a separate table where different category-wise settings might be stored
- distinguishing if instance belongs to category or not (where to store this field)

Having a separate table for categories helps to organize things, support different types (categories) of instances and establish better relations between tables. Potentially, we could link categories table to instance_types and set up special configurations/flavors that are applied to particular type of admin instances. In this approach such table has a metadata field, but it may have sense to put it outside in separate table a-la instance_metadata.

It is definitely possible to store this category field in the instance_metadata table as a key/value pair and we were thinking about it at the beginning. But it complicates SQL queries and you can't build normal SQL relations between key/value & categories table.
It will require first to retrieve a value and only after that to perform an additional query to retrieve properties of this category ...
Another use-case - filtering of non-regular instances. Now for each DescribeInstances call we will need to look at meta-data key/value pairs and find out if it should be hidden or not.

But I guess the main question about categories - if we think that it might be one of basic instance parameters that will benefit every installation/customer. Because in general, the instance_metadata table could hold almost all fields from instances table and it will be enough to have only instance_id field there :-)

Here's how understand the problem you are trying to solve with this merge prop: We need the ability for a deployer to create an instance that a tenant cannot access. This instance needs to be able to access the tenant's network. Please help me understand if this isn't 100% correct.

My first thoughts when solving the problem (as I defined it) are to allow an admin to change the network associated with an instance. Therefore, any instance he creates can be plugged into any project network. Do you think this is a viable solution?

I just want to make sure there isn't a simpler solution to this problem. This merge prop adds a lot of code to solve what seems to be a rather simple problem.

Actually, we are trying to do something bigger than that - we would like to
be able to spawn special instances with their special properties.
Different types of instances may perform different tasks. For example,
"virtual storage controllers / arrays" providing HA block storage to
project's tenants. Another example might be a dedicated billing or IO
statistics server per project.

If instance will spawn multiple projects ... probably, but t will require
changes on networking level and we decided to wait for completion of
multi-nic implementation for that.
In general, tenants should not be able to access those instances in the same
manner as they access regular instances, but this is a slightly different
issue. They might be able to access such instances using REST APIs for
retrieving some data.

Does it make sense?

Brian: I think if we have plugin networking + admin managed metadata we have the components that we need. I'm a fan of putting the metadata into a generalized extra_data table. Other than that a standard way of defining an admin vm would be very valuable. It seems like a "category" definition could be defined by a tenant, networks to plug into (and whether to plug into one or more users network), base image, flavor, injected user data, and some small amount of metadata. Then you could have a command that would launch an image based on a category definition.

The special scheduler would have enough information based on the metadata to make any necessary decisions.

> In general, tenants should not be able to access those instances in the same
> manner as they access regular instances, but this is a slightly different
> issue.

This can be responsible by running the instances as a different tenant

> They might be able to access such instances using REST APIs for
> retrieving some data.

A simple command for retrieving data could get data from the flavor or metadata to return to the user.

A complicated command would require a guest agent. As long as you can specify a specific (administration) network to plug into then your REST API should be able to communicate to the guest agent to run commands.

So a complicated category def would say: run the instance as this tenant, using this image id (which has a guest agent), this special metadata, with flavor x, plugged into the admin network and the network for tenant x. Alternatively, in flatdhcp you could specify to allow access from certain tenant security groups in place of arbitrary networks.

Vish

=== modified file 'nova/virt/libvirt/connection.py' (properties changed: -x to +x)
=== modified file 'nova/virt/libvirt/firewall.py' (properties changed: -x to +x)

Should these files have been made executable?

Also it seems you may have added an import to connection.py which is unused.

Lines 107-109, 132-134, 150-151, 171-172, 197-198, and potentially others needs docstring/sphinx argument standardization:

"""One line summary.

Optional extra information which might be multiple lines.

:param param_one: This is a description of param one.
:param param_two: This is a description of param two.
:returns: This is a description of the return value.

"""

OR

"""This is a description of this method."""

Thanks.

Re: properties changed from -x to +x - we missed it. Seems like samba on one of dev servers was incorrectly configured. will fix it.

Re: probably a leftover from the merge with one of latest nova versions (when connection was removed). Will fix it

Re: comments standardization - fine. will fix it.

- fixed all conflicts related to recent merges
- comments in nova-manage are complied with other comments in this exec module

Would you mind calling this something other than 'category'? To me, a category is a grouping of similar things. I think what we are going for here is more of a 'class': a predefined configuration of an instance. Does that seem reasonable?

I suppose the best term for it is "type", but it is taken by
flavor/instance_type which actually refers to instance configuration type.

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of
Brian Waldon
Sent: Wednesday, June 29, 2011 7:17 AM
To: <email address hidden>
Subject: Re: [Merge] lp:~vladimir.p/nova/admin-vm into lp:nova

Would you mind calling this something other than 'category'? To me, a
category is a grouping of similar things. I think what we are going for here
is more of a 'class': a predefined configuration of an instance. Does that
seem reasonable?
--
https://code.launchpad.net/~vladimir.p/nova/admin-vm/+merge/63771
You are the owner of lp:~vladimir.p/nova/admin-vm.

The term 'class' may not be the best option, either, as it is a well-defined keyword in python.

What about 'InstancePlan' or 'InstanceConfig?

Definitely not InstanceConfig :-)

I suppose the main purpose is to distinguish between regular compute
instances and administrative instances (instances created for other needs):
statistics/billing VMs, some special DBs, instances for hosting Virtual
Storage arrays/controllers, even VPN instances, etc. Most likely such
instances will have different configuration description / templates, but it
is probably a different question.

I'm open to any suggestions, but for me InstanceType or InstanceCategory are
winners... meanwhile :-)

Hi Vladimir,

A couple more merge conflicts for this one:

Text conflict in bin/nova-manage
Text conflict in nova/api/openstack/create_instance_helper.py
Text conflict in nova/compute/api.py
3 conflicts encountered.

I'm missing it. The Launchpad doesn't show any conflict.

Anyway, I suppose there might be conflicts related to added new parameters
or changed prints, but how about general feedback about direction?

Thanks.

> I'm missing it. The Launchpad doesn't show any conflict.

Launchpad isn't great at showing conflicts that develop after a merge prop is submitted. On your local machine, try to merge trunk into your branch. That will uncover what Dan pointed out.