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

[Vladimir Popovski]

First cut of Admin-VM changes:

- added instance categories. By default, all instances have 'regular' category assigned
- each category has default image_id
- new param "category" in ec2/openstack APIs
- filter admin instances/images/volumes

[Devin Carlen (devcamcar)]

Hi Vladimir,

You have a number of merge conflicts that need to be resolved.

review: Needs Fixing

[Vladimir Popovski (vladimir.p)]

Hi Devin,

Thanks. Working on merging trunk back to our code. Will be ready soon.

[Brian Waldon (bcwaldon)]

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 :)

review: Needs Information

[Vladimir Popovski (vladimir.p)]

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.

[Brian Waldon (bcwaldon)]

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.

[Josh Kearney (jk0)]

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

review: Needs Fixing

[Vladimir Popovski (vladimir.p)]

Merged with latest nova revision (#1187)

[lp:~vladimir.p/nova/admin-vm]

1112. By Vladimir Popovski on 2011-06-16

Pep8 normalization

1113. By Vladimir Popovski on 2011-06-16

changed unit_test properties (otherwise was skipped)

[Devin Carlen (devcamcar)]

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

review: Needs Fixing

[Vladimir Popovski (vladimir.p)]

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?

[Brian Waldon (bcwaldon)]

> 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?

[Vladimir Popovski (vladimir.p)]

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 :-)

[Brian Waldon (bcwaldon)]

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.

[Vladimir Popovski (vladimir.p)]

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?

[Vish Ishaya (vishvananda)]

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.

[Vish Ishaya (vishvananda)]

> 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

[Brian Lamar (blamar)]

=== 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."""

review: Needs Fixing

[Vladimir Popovski (vladimir.p)]

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.

[lp:~vladimir.p/nova/admin-vm]

1114. By Vladimir Popovski on 2011-06-28

merged with 1221

1115. By Vladimir Popovski on 2011-06-28

merged with 1222

1116. By Vladimir Popovski on 2011-06-28

replaced my fix for nova-api with community's

[Vladimir Popovski (vladimir.p)]

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

[Brian Waldon (bcwaldon)]

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?

[Vladimir Popovski (vladimir.p)]

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.

[Brian Waldon (bcwaldon)]

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

[Brian Waldon (bcwaldon)]

What about 'InstancePlan' or 'InstanceConfig?

[Vladimir Popovski (vladimir.p)]

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 :-)

[Dan Prince (dan-prince)]

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.

review: Needs Fixing

[Vladimir Popovski (vladimir.p)]

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.

[Brian Waldon (bcwaldon)]

> 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.

Unmerged revisions

1116. By Vladimir Popovski on 2011-06-28

replaced my fix for nova-api with community's

1115. By Vladimir Popovski on 2011-06-28

merged with 1222

1114. By Vladimir Popovski on 2011-06-28

merged with 1221

1113. By Vladimir Popovski on 2011-06-16

changed unit_test properties (otherwise was skipped)

1112. By Vladimir Popovski on 2011-06-16

Pep8 normalization

1111. By Vladimir Popovski on 2011-06-16

Admin-VM: merged with 1187

1110. By Vladimir Popovski on 2011-06-16

Admin-VM: merged with 1186

1109. By Vladimir Popovski on 2011-06-10

pep8 code normalization

1108. By Vladimir Popovski on 2011-06-07

Admin-VM: removed nova.conf (accidentally added)

1107. By Vladimir Popovski on 2011-06-07

Admin-VM: merged with nova trunk-1159

=== modified file 'Authors'
--- Authors	2011-06-28 00:23:19 +0000
+++ Authors	2011-06-28 22:54:37 +0000
@@ -88,6 +88,7 @@
 Vasiliy Shlykov <[e-mail]>
 Vishvananda Ishaya <[e-mail]>
 Vivek Y S <[e-mail]>
+Vladimir Popovski <[e-mail]>
 William Wolf <[e-mail]>
 Yoshiaki Tamura <[e-mail]>
 Youcef Laribi <[e-mail]>

=== modified file 'bin/nova-manage'
--- bin/nova-manage	2011-06-25 19:38:07 +0000
+++ bin/nova-manage	2011-06-28 22:54:37 +0000
@@ -87,6 +87,7 @@
 from nova.auth import manager
 from nova.cloudpipe import pipelib
 from nova.compute import instance_types
+from nova.compute import instance_categories
 from nova.db import migration
 
 FLAGS = flags.FLAGS
@@ -140,7 +141,9 @@
             ipport = "%s:%s" % (project.vpn_ip, project.vpn_port)
             print "%-20s\t" % ipport,
             ctxt = context.get_admin_context()
-            vpn = db.instance_get_project_vpn(ctxt, project.id)
+            cat_id = instance_categories.get_instance_cat_id_by_name(
+                                            FLAGS.instance_cat_vpn)
+            vpn = db.instance_get_project_admin(ctxt, project.id, cat_id)
             if vpn:
                 address = None
                 state = 'down'
@@ -611,14 +614,14 @@
 class VmCommands(object):
     """Class for mangaging VM instances."""
 
-    def list(self, host=None):
+    def list(self, host=None, cat_id=None):
         """Show a list of all instances
 
         :param host: show all instance on specified host.
         :param instance: show specificed instance.
         """
         print "%-10s %-15s %-10s %-10s %-19s %-12s %-12s %-12s" \
-              "  %-10s %-10s %-10s %-5s" % (
+              "  %-10s %-10s %-10s %-5s %-8s" % (
             _('instance'),
             _('node'),
             _('type'),
@@ -630,20 +633,23 @@
             _('project'),
             _('user'),
             _('zone'),
-            _('index'))
+            _('index'),
+            _('category'))
 
-        if host is None:
-            instances = db.instance_get_all(context.get_admin_context())
+        ctxt = context.get_admin_context()
+        if cat_id is not None:
+            instances = db.instance_get_all_by_cat(ctxt, cat_id)
+        elif host is None:
+            instances = db.instance_get_all(ctxt)
         else:
-            instances = db.instance_get_all_by_host(
-                           context.get_admin_context(), host)
+            instances = db.instance_get_all_by_host(ctxt, host)
 
         for instance in instances:
             print "%-10s %-15s %-10s %-10s %-19s %-12s %-12s %-12s" \
-                  "  %-10s %-10s %-10s %-5d" % (
+                  "  %-10s %-10s %-10s %-5d %-8s" % (
                 instance['hostname'],
                 instance['host'],
-                instance['instance_type'],
+                instance['instance_type']['name'],
                 instance['state_description'],
                 instance['launched_at'],
                 instance['image_id'],
@@ -652,7 +658,8 @@
                 instance['project_id'],
                 instance['user_id'],
                 instance['availability_zone'],
-                instance['launch_index'])
+                instance['launch_index'],
+                instance['instance_cat']['name'])
 
     def live_migration(self, ec2_id, dest):
         """Migrates a running instance to a new machine.
@@ -937,6 +944,98 @@
             self._print_instance_types(name, inst_types)
 
 
+class InstanceCatCommands(object):
+    """Class for managing instance categories."""
+
+    def _print_instance_categories(self, name, val):
+        deleted = ('', ', inactive')[val["deleted"] == 1]
+        print ("%-16s: id: %2d, Desc: %-30s, image_id %-10s, Specs: %s%s") % \
+                (name, val["id"], val["description"], val["image_id"],
+                val["specs"], deleted)
+
+    def create(self, name, description, image_id=None, specs=None):
+        """Creates instance category
+           arguments: name description [image_id] [specs]
+        """
+        try:
+            instance_categories.create(name, description, image_id, specs)
+        except exception.InvalidInputException:
+            print "Must supply valid parameters to create instance_category"
+            print e
+            sys.exit(1)
+        except exception.ApiError, e:
+            print "\n\n"
+            print "\n%s" % e
+            print "Please ensure category name is unique."
+            print "To complete remove a category, use the --purge flag:"
+            print "\n  # nova-manage instance_category delete <name> --purge"
+            print "\nCurrently defined category names:"
+            self.list("--all")
+            sys.exit(2)
+        except:
+            print "Unknown error"
+            sys.exit(3)
+        else:
+            print "Category %s created" % name
+
+    def update(self, name, description=None, image_id=None, specs=None):
+        """Updates instance category
+           arguments: name [description] [image_id] [specs]
+        """
+        try:
+            instance_categories.update(name, description, image_id, specs)
+        except exception.ApiError:
+            print "Valid instance category name is required"
+            sys.exit(1)
+        except exception.DBError, e:
+            print "DB Error: %s" % e
+            sys.exit(2)
+        except:
+            print "Unknown error"
+            sys.exit(3)
+        else:
+            print "Category %s updated" % (name)
+
+    def delete(self, name, purge=None):
+        """Marks instance category as deleted
+        arguments: name"""
+        try:
+            if purge == "--purge":
+                instance_categories.purge(name)
+                verb = "purged"
+            else:
+                instance_categories.destroy(name)
+                verb = "deleted"
+        except exception.ApiError:
+            print "Valid instance category name is required"
+            sys.exit(1)
+        except exception.DBError, e:
+            print "DB Error: %s" % e
+            sys.exit(2)
+        except:
+            sys.exit(3)
+        else:
+            print "Category %s %s" % (name, verb)
+
+    def list(self, name=None):
+        """Lists all active or specific instance categories
+        arguments: [name]"""
+        try:
+            if name is None:
+                inst_cats = instance_categories.get_all_categories()
+            elif name == "--all":
+                inst_cats = instance_categories.get_all_categories(True)
+            else:
+                inst_cats = instance_categories.get_instance_cat_by_name(name)
+        except exception.DBError, e:
+            _db_error(e)
+        if isinstance(inst_cats.values()[0], dict):
+            for k, v in inst_cats.iteritems():
+                self._print_instance_categories(k, v)
+        else:
+            self._print_instance_categories(name, inst_cats)
+
+
 class ImageCommands(object):
     """Methods for dealing with a cloud in an odd state"""
 
@@ -1075,6 +1174,15 @@
         self._convert_images(other_images)
         self._convert_images(machine_images)
 
+    def show_admin_images(self, category=None):
+        """Show registered admin images [per category]
+        arguments: [category]"""
+
+        if category == "--all":
+            category = None
+        image_names = instance_categories.get_admin_image_names(category)
+        print image_names
+
 
 class AgentBuildCommands(object):
     """Class for managing agent builds."""
@@ -1158,6 +1266,7 @@
     ('fixed', FixedIpCommands),
     ('flavor', InstanceTypeCommands),
     ('floating', FloatingIpCommands),
+    ('instance_category', InstanceCatCommands),
     ('instance_type', InstanceTypeCommands),
     ('image', ImageCommands),
     ('network', NetworkCommands),

=== modified file 'nova/api/ec2/admin.py'
--- nova/api/ec2/admin.py	2011-06-25 19:38:07 +0000
+++ nova/api/ec2/admin.py	2011-06-28 22:54:37 +0000
@@ -33,6 +33,7 @@
 from nova import utils
 from nova.api.ec2 import ec2utils
 from nova.auth import manager
+from nova.compute import instance_categories
 
 
 FLAGS = flags.FLAGS
@@ -91,6 +92,13 @@
             'flavor_id': inst['flavorid']}
 
 
+def instance_cat_dict(inst):
+    return {'name': inst['name'],
+            'description': inst['description'],
+            'image_id': inst['image_id'],
+            'specs': inst['specs']}
+
+
 def vpn_dict(project, vpn_instance):
     rv = {'project_id': project.id,
           'public_ip': project.vpn_ip,
@@ -129,6 +137,11 @@
         return {'instanceTypeSet': [instance_dict(v) for v in
                                    db.instance_type_get_all(context).values()]}
 
+    def describe_instance_categories(self, context, **_kwargs):
+        """Returns all active instance categories data"""
+        return {'instanceCatSet': [instance_cat_dict(v) for v in
+                                   db.instance_cat_get_all(context).values()]}
+
     def describe_user(self, _context, name, **_kwargs):
         """Returns user data, including access and secret keys."""
         return user_dict(manager.AuthManager().get_user(name))
@@ -272,7 +285,9 @@
     def _vpn_for(self, context, project_id):
         """Get the VPN instance for a project ID."""
         for instance in db.instance_get_all_by_project(context, project_id):
-            if (instance['image_id'] == str(FLAGS.vpn_image_id)
+            if (instance_categories.is_admin_instance(
+                        instance,
+                        FLAGS.instance_cat_vpn) \
                 and not instance['state_description'] in
                     ['shutting_down', 'shutdown']):
                 return instance

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2011-06-25 19:38:07 +0000
+++ nova/api/ec2/cloud.py	2011-06-28 22:54:37 +0000
@@ -44,6 +44,7 @@
 from nova import volume
 from nova.api.ec2 import ec2utils
 from nova.compute import instance_types
+from nova.compute import instance_categories
 from nova.image import s3
 
 
@@ -620,6 +621,40 @@
         return self.compute_api.get_vnc_console(context,
                                                 instance_id=instance_id)
 
+    def _filter_volumes(self, context, volumes, category):
+        result = []
+        for volume in volumes:
+            add_image = False
+            if volume.get('instance', None):
+                instance_id = volume['instance']['id']
+                instance = db.instance_get(context, instance_id)
+                is_admin_inst = \
+                    instance_categories.is_admin_instance(instance, category)
+            else:
+                is_admin_inst = False
+
+            if category is None:
+                if context.is_admin \
+                   or not is_admin_inst:
+                    add_image = True
+            else:
+                if is_admin_inst:
+                    add_image = True
+
+            if add_image:
+                LOG.debug(("filter_volumes: +++ add volume %d, "
+                           "is_admin_inst %s, cat %s, is_admin=%s"),
+                           volume['id'], str(is_admin_inst), category,
+                           str(context.is_admin))
+                result.append(volume)
+            else:
+                LOG.debug(("filter_volumes: --- skip volume %d, "
+                           "is_admin_inst %s, cat %s, is_admin=%s"),
+                           volume['id'], str(is_admin_inst), category,
+                           str(context.is_admin))
+
+        return result
+
     def describe_volumes(self, context, volume_id=None, **kwargs):
         if volume_id:
             volumes = []
@@ -629,7 +664,13 @@
                 volumes.append(volume)
         else:
             volumes = self.volume_api.get_all(context)
-        volumes = [self._format_volume(context, v) for v in volumes]
+
+        category = self._get_category_name(**kwargs)
+        filtered_volumes = self._filter_volumes(context,
+                                                volumes,
+                                                category=category)
+
+        volumes = [self._format_volume(context, v) for v in filtered_volumes]
         return {'volumeSet': volumes}
 
     def _format_volume(self, context, volume):
@@ -748,6 +789,16 @@
             lst = [lst]
         return [{label: x} for x in lst]
 
+    def _get_category_name(self, **kwargs):
+        return kwargs.get('category', None)
+
+    def _get_category(self, **kwargs):
+        cat_name = self._get_category_name(**kwargs)
+        if cat_name is None:
+            return instance_categories.get_default_instance_cat()
+        else:
+            return instance_categories.get_instance_cat_by_name(cat_name)
+
     def describe_instances(self, context, **kwargs):
         return self._format_describe_instances(context, **kwargs)
 
@@ -758,8 +809,10 @@
     def _format_describe_instances(self, context, **kwargs):
         return {'reservationSet': self._format_instances(context, **kwargs)}
 
-    def _format_run_instances(self, context, reservation_id):
-        i = self._format_instances(context, reservation_id=reservation_id)
+    def _format_run_instances(self, context, reservation_id, category):
+        i = self._format_instances(context,
+                                   reservation_id=reservation_id,
+                                   category=category)
         assert len(i) == 1
         return i[0]
 
@@ -772,16 +825,18 @@
         # NOTE(vish): instance_id is an optional list of ids to filter by
         if instance_id:
             instances = []
+            instance_cat = self._get_category(**kwargs)
             for ec2_id in instance_id:
                 internal_id = ec2utils.ec2_id_to_id(ec2_id)
                 instance = self.compute_api.get(context,
-                                                instance_id=internal_id)
+                                                instance_id=internal_id,
+                                                instance_cat=instance_cat)
                 instances.append(instance)
         else:
             instances = self.compute_api.get_all(context, **kwargs)
         for instance in instances:
             if not context.is_admin:
-                if instance['image_ref'] == str(FLAGS.vpn_image_id):
+                if instance_categories.is_admin_instance(instance):
                     continue
             i = {}
             instance_id = instance['id']
@@ -820,6 +875,10 @@
                 i['instanceType'] = instance['instance_type'].get('name')
             else:
                 i['instanceType'] = None
+            if instance['instance_cat']:
+                i['category'] = instance['instance_cat'].get('name')
+            else:
+                i['category'] = None
             i['launchTime'] = instance['created_at']
             i['amiLaunchIndex'] = instance['launch_index']
             i['displayName'] = instance['display_name']
@@ -938,6 +997,12 @@
         if image_state != 'available':
             raise exception.ApiError(_('Image must be available'))
 
+        category = self._get_category_name(**kwargs)
+        instance_cat = self._get_category(**kwargs)
+        if instance_categories.is_admin_category(instance_cat) \
+           and not context.is_admin:
+            raise exception.AdminRequired()
+
         instances = self.compute_api.create(context,
             instance_type=instance_types.get_instance_type_by_name(
                 kwargs.get('instance_type', None)),
@@ -953,53 +1018,67 @@
             security_group=kwargs.get('security_group'),
             availability_zone=kwargs.get('placement', {}).get(
                                   'AvailabilityZone'),
-            block_device_mapping=kwargs.get('block_device_mapping', {}))
+            block_device_mapping=kwargs.get('block_device_mapping', {}),
+            instance_cat=instance_cat)
         return self._format_run_instances(context,
-                                          instances[0]['reservation_id'])
+                                          instances[0]['reservation_id'],
+                                          category=category)
 
-    def _do_instance(self, action, context, ec2_id):
+    def _do_instance(self, action, context, ec2_id, instance_cat=None):
         instance_id = ec2utils.ec2_id_to_id(ec2_id)
-        action(context, instance_id=instance_id)
+        action(context, instance_id=instance_id, instance_cat=instance_cat)
 
-    def _do_instances(self, action, context, instance_id):
+    def _do_instances(self, action, context, instance_id, instance_cat=None):
         for ec2_id in instance_id:
-            self._do_instance(action, context, ec2_id)
+            self._do_instance(action, context, ec2_id, instance_cat)
 
     def terminate_instances(self, context, instance_id, **kwargs):
         """Terminate each instance in instance_id, which is a list of ec2 ids.
         instance_id is a kwarg so its name cannot be modified."""
         LOG.debug(_("Going to start terminating instances"))
-        self._do_instances(self.compute_api.delete, context, instance_id)
+        instance_cat = self._get_category(**kwargs)
+        self._do_instances(self.compute_api.delete, context, instance_id,
+                            instance_cat)
         return True
 
     def reboot_instances(self, context, instance_id, **kwargs):
         """instance_id is a list of instance ids"""
         LOG.audit(_("Reboot instance %r"), instance_id, context=context)
-        self._do_instances(self.compute_api.reboot, context, instance_id)
+        instance_cat = self._get_category(**kwargs)
+        self._do_instances(self.compute_api.reboot, context, instance_id,
+                            instance_cat)
         return True
 
     def stop_instances(self, context, instance_id, **kwargs):
         """Stop each instances in instance_id.
         Here instance_id is a list of instance ids"""
         LOG.debug(_("Going to stop instances"))
-        self._do_instances(self.compute_api.stop, context, instance_id)
+        instance_cat = self._get_category(**kwargs)
+        self._do_instances(self.compute_api.stop, context, instance_id,
+                            instance_cat)
         return True
 
     def start_instances(self, context, instance_id, **kwargs):
         """Start each instances in instance_id.
         Here instance_id is a list of instance ids"""
         LOG.debug(_("Going to start instances"))
-        self._do_instances(self.compute_api.start, context, instance_id)
+        instance_cat = self._get_category(**kwargs)
+        self._do_instances(self.compute_api.start, context, instance_id,
+                            instance_cat)
         return True
 
     def rescue_instance(self, context, instance_id, **kwargs):
         """This is an extension to the normal ec2_api"""
-        self._do_instance(self.compute_api.rescue, contect, instnace_id)
+        instance_cat = self._get_category(**kwargs)
+        self._do_instance(self.compute_api.rescue, context, instance_id,
+                            instance_cat)
         return True
 
     def unrescue_instance(self, context, instance_id, **kwargs):
         """This is an extension to the normal ec2_api"""
-        self._do_instance(self.compute_api.unrescue, context, instance_id)
+        instance_cat = self._get_category(**kwargs)
+        self._do_instance(self.compute_api.unrescue, context, instance_id,
+                            instance_cat)
         return True
 
     def update_instance(self, context, instance_id, **kwargs):
@@ -1083,6 +1162,38 @@
         i['architecture'] = image['properties'].get('architecture')
         return i
 
+    def _filter_images(self, context, images, category):
+        result = []
+        admin_image_names = instance_categories.get_admin_image_names(category)
+        for i in admin_image_names:
+            LOG.debug(_("admin_image_names: %s"), i)
+
+        for image in images:
+            name = str(image.get('id'))
+            add_name = False
+            if category is None:
+                # if no category set - filter all admin images
+                #if name not in admin_image_names:
+                if context.is_admin \
+                    or name not in admin_image_names:
+                    add_name = True
+            else:
+                # if category is set - return images for this category only
+                if name in admin_image_names:
+                    add_name = True
+
+            if add_name:
+                LOG.debug(("filter_images: +++ add name %s, "
+                           "category %s, is_admin=%s"),
+                           name, category, str(context.is_admin))
+                result.append(image)
+            else:
+                LOG.debug(("filter_images: --- skip name %s, "
+                           "category %s, is_admin=%s"),
+                           name, category, str(context.is_admin))
+
+        return result
+
     def describe_images(self, context, image_id=None, **kwargs):
         # NOTE: image_id is a list!
         if image_id:
@@ -1095,7 +1206,12 @@
                 images.append(image)
         else:
             images = self.image_service.detail(context)
-        images = [self._format_image(i) for i in images]
+
+        category = self._get_category_name(**kwargs)
+        filtered_images = self._filter_images(context,
+                                              images, category=category)
+
+        images = [self._format_image(i) for i in filtered_images]
         return {'imagesSet': images}
 
     def deregister_image(self, context, image_id, **kwargs):

=== modified file 'nova/api/openstack/create_instance_helper.py'
--- nova/api/openstack/create_instance_helper.py	2011-06-17 17:17:14 +0000
+++ nova/api/openstack/create_instance_helper.py	2011-06-28 22:54:37 +0000
@@ -28,6 +28,7 @@
 from nova import utils
 
 from nova.compute import instance_types
+from nova.compute import instance_categories
 from nova.api.openstack import faults
 from nova.api.openstack import wsgi
 from nova.auth import manager as auth_manager
@@ -104,6 +105,17 @@
 
         flavor_id = self.controller._flavor_id_from_req_data(body)
 
+        if 'category' in body['server']:
+            category = self.controller._instance_cat_from_req_data(body)
+            try:
+                instance_cat = \
+                    instance_categories.get_instance_cat_by_name(category)
+            except exception.NoFound:
+                raise exc.HTTPBadRequest(_("Category %(category)s not found"))
+        else:
+            instance_cat = None
+            category = instance_categories.default_category_name()
+
         if not 'name' in body['server']:
             msg = _("Server name is not defined")
             raise exc.HTTPBadRequest(explanation=msg)
@@ -120,6 +132,7 @@
                     instance_types.get_instance_type_by_flavor_id(flavor_id)
             extra_values = {
                 'instance_type': inst_type,
+                'category': category,
                 'image_ref': image_href,
                 'password': password}
 
@@ -137,7 +150,8 @@
                                   injected_files=injected_files,
                                   admin_password=password,
                                   zone_blob=zone_blob,
-                                  reservation_id=reservation_id))
+                                  reservation_id=reservation_id,
+                                  instance_cat=instance_cat))
         except quota.QuotaError as error:
             self._handle_quota_error(error)
         except exception.ImageNotFound as error:

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-06-15 20:58:55 +0000
+++ nova/api/openstack/servers.py	2011-06-28 22:54:37 +0000
@@ -124,6 +124,7 @@
 
         builder = self._get_view_builder(req)
         server = builder.build(inst, is_detail=True)
+        server['server']['category'] = extra_values['category']
         server['server']['adminPass'] = extra_values['password']
         return server
 
@@ -410,6 +411,9 @@
     def _flavor_id_from_req_data(self, data):
         return data['server']['flavorId']
 
+    def _instance_cat_from_req_data(self, data):
+        return data['server']['category']
+
     def _get_view_builder(self, req):
         addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV10()
         return nova.api.openstack.views.servers.ViewBuilderV10(
@@ -473,6 +477,10 @@
         href = data['server']['flavorRef']
         return common.get_id_from_href(href)
 
+    def _instance_cat_from_req_data(self, data):
+        href = data['server']['category']
+        return common.get_id_from_href(href)
+
     def _get_view_builder(self, req):
         base_url = req.application_url
         flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11(
@@ -591,7 +599,7 @@
         "attributes": {
             "server": ["id", "imageId", "name", "flavorId", "hostId",
                        "status", "progress", "adminPass", "flavorRef",
-                       "imageRef"],
+                       "imageRef", "category"],
             "link": ["rel", "type", "href"],
         },
         "dict_collections": {

=== modified file 'nova/cloudpipe/pipelib.py'
--- nova/cloudpipe/pipelib.py	2011-04-19 16:32:33 +0000
+++ nova/cloudpipe/pipelib.py	2011-06-28 22:54:37 +0000
@@ -107,6 +107,7 @@
             max_count=1,
             min_count=1,
             instance_type='m1.tiny',
+            instance_cat=FLAGS.instance_cat_vpn,
             image_id=ec2_id,
             key_name=key_name,
             security_group=[group_name])

=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-06-24 12:07:33 +0000
+++ nova/compute/api.py	2011-06-28 22:54:37 +0000
@@ -33,6 +33,7 @@
 from nova import utils
 from nova import volume
 from nova.compute import instance_types
+from nova.compute import instance_categories
 from nova.compute import power_state
 from nova.compute.utils import terminate_volumes
 from nova.scheduler import api as scheduler_api
@@ -148,13 +149,17 @@
                key_name=None, key_data=None, security_group='default',
                availability_zone=None, user_data=None, metadata={},
                injected_files=None, admin_password=None, zone_blob=None,
-               reservation_id=None):
+               reservation_id=None,
+               instance_cat=None):
         """Verify all the input parameters regardless of the provisioning
         strategy being performed."""
 
         if not instance_type:
             instance_type = instance_types.get_default_instance_type()
 
+        if not instance_cat:
+            instance_cat = instance_categories.get_default_instance_cat()
+
         num_instances = quota.allowed_instances(context, max_count,
                                                 instance_type)
         if num_instances < min_count:
@@ -234,6 +239,7 @@
             'project_id': context.project_id,
             'launch_time': time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()),
             'instance_type_id': instance_type['id'],
+            'instance_cat_id': instance_cat['id'],
             'memory_mb': instance_type['memory_mb'],
             'vcpus': instance_type['vcpus'],
             'local_gb': instance_type['local_gb'],
@@ -303,7 +309,8 @@
         return instance
 
     def _ask_scheduler_to_create_instance(self, context, base_options,
-                                          instance_type, zone_blob,
+                                          instance_type, instance_cat,
+                                          zone_blob,
                                           availability_zone, injected_files,
                                           admin_password,
                                           instance_id=None, num_instances=1):
@@ -317,10 +324,14 @@
             LOG.debug(_("Casting to scheduler for %(pid)s/%(uid)s's"
                     " (all-at-once)") % locals())
 
+        if not instance_cat:
+            instance_cat = instance_categories.get_default_instance_cat()
+
         filter_class = 'nova.scheduler.host_filter.InstanceTypeFilter'
         request_spec = {
             'instance_properties': base_options,
             'instance_type': instance_type,
+            'category': instance_cat['name'],
             'filter': filter_class,
             'blob': zone_blob,
             'num_instances': num_instances,
@@ -343,7 +354,8 @@
                key_name=None, key_data=None, security_group='default',
                availability_zone=None, user_data=None, metadata={},
                injected_files=None, admin_password=None, zone_blob=None,
-               reservation_id=None):
+               reservation_id=None,
+               instance_cat=None):
         """Provision the instances by passing the whole request to
         the Scheduler for execution. Returns a Reservation ID
         related to the creation of all of these instances."""
@@ -356,10 +368,10 @@
                                key_name, key_data, security_group,
                                availability_zone, user_data, metadata,
                                injected_files, admin_password, zone_blob,
-                               reservation_id)
+                               reservation_id, instance_cat)
 
         self._ask_scheduler_to_create_instance(context, base_options,
-                                      instance_type, zone_blob,
+                                      instance_type, instance_cat, zone_blob,
                                       availability_zone, injected_files,
                                       admin_password,
                                       num_instances=num_instances)
@@ -373,7 +385,8 @@
                key_name=None, key_data=None, security_group='default',
                availability_zone=None, user_data=None, metadata={},
                injected_files=None, admin_password=None, zone_blob=None,
-               reservation_id=None, block_device_mapping=None):
+               reservation_id=None, block_device_mapping=None,
+               instance_cat=None):
         """
         Provision the instances by sending off a series of single
         instance requests to the Schedulers. This is fine for trival
@@ -392,7 +405,7 @@
                                key_name, key_data, security_group,
                                availability_zone, user_data, metadata,
                                injected_files, admin_password, zone_blob,
-                               reservation_id)
+                               reservation_id, instance_cat)
 
         block_device_mapping = block_device_mapping or []
         instances = []
@@ -405,7 +418,8 @@
             instance_id = instance['id']
 
             self._ask_scheduler_to_create_instance(context, base_options,
-                                          instance_type, zone_blob,
+                                          instance_type, instance_cat,
+                                          zone_blob,
                                           availability_zone, injected_files,
                                           admin_password,
                                           instance_id=instance_id)
@@ -520,9 +534,9 @@
         rv = self.db.instance_update(context, instance_id, kwargs)
         return dict(rv.iteritems())
 
-    def _get_instance(self, context, instance_id, action_str):
+    def _get_instance(self, context, instance_id, instance_cat, action_str):
         try:
-            return self.get(context, instance_id)
+            return self.get(context, instance_id, instance_cat)
         except exception.NotFound:
             LOG.warning(_("Instance %(instance_id)s was not found during "
                           "%(action_str)s") %
@@ -530,10 +544,11 @@
             raise
 
     @scheduler_api.reroute_compute("delete")
-    def delete(self, context, instance_id):
+    def delete(self, context, instance_id, instance_cat=None):
         """Terminate an instance."""
         LOG.debug(_("Going to try to terminate %s"), instance_id)
-        instance = self._get_instance(context, instance_id, 'terminating')
+        instance = self._get_instance(context, instance_id, instance_cat,
+                                      'terminating')
 
         if not _is_able_to_shutdown(instance, instance_id):
             return
@@ -553,11 +568,12 @@
             self.db.instance_destroy(context, instance_id)
 
     @scheduler_api.reroute_compute("stop")
-    def stop(self, context, instance_id):
+    def stop(self, context, instance_id, instance_cat=None):
         """Stop an instance."""
         LOG.debug(_("Going to try to stop %s"), instance_id)
 
-        instance = self._get_instance(context, instance_id, 'stopping')
+        instance = self._get_instance(context, instance_id, instance_cat,
+                                      'stopping')
         if not _is_able_to_shutdown(instance, instance_id):
             return
 
@@ -572,10 +588,11 @@
             self._cast_compute_message('stop_instance', context,
                     instance_id, host)
 
-    def start(self, context, instance_id):
+    def start(self, context, instance_id, instance_cat=None):
         """Start an instance."""
         LOG.debug(_("Going to try to start %s"), instance_id)
-        instance = self._get_instance(context, instance_id, 'starting')
+        instance = self._get_instance(context, instance_id, instance_cat,
+                                      'starting')
         if instance['state_description'] != 'stopped':
             _state_description = instance['state_description']
             LOG.warning(_("Instance %(instance_id)s is not "
@@ -591,7 +608,7 @@
                   "args": {"topic": FLAGS.compute_topic,
                            "instance_id": instance_id}})
 
-    def get(self, context, instance_id):
+    def get(self, context, instance_id, instance_cat=None):
         """Get a single instance with the given instance_id."""
         # NOTE(sirp): id used to be exclusively integer IDs; now we're
         # accepting both UUIDs and integer IDs. The handling of this
@@ -601,17 +618,22 @@
             instance = self.db.instance_get_by_uuid(context, uuid)
         else:
             instance = self.db.instance_get(context, instance_id)
-        return dict(instance.iteritems())
+        instance = dict(instance.iteritems())
+        if instance_cat is not None:
+            if not instance_categories.is_access_allowed(context, instance,
+                                                         instance_cat):
+                raise exception.AdminRequired()
+        return instance
 
     @scheduler_api.reroute_compute("get")
-    def routing_get(self, context, instance_id):
+    def routing_get(self, context, instance_id, instance_cat=None):
         """A version of get with special routing characteristics.
 
         Use this method instead of get() if this is the only operation you
         intend to to. It will route to novaclient.get if the instance is not
         found.
         """
-        return self.get(context, instance_id)
+        return self.get(context, instance_id, instance_cat)
 
     def get_all_across_zones(self, context, reservation_id):
         """Get all instances with this reservation_id, across
@@ -633,33 +655,47 @@
         return instances
 
     def get_all(self, context, project_id=None, reservation_id=None,
-                fixed_ip=None):
+                fixed_ip=None, category=None):
         """Get all instances filtered by one of the given parameters.
 
         If there is no filter and the context is an admin, it will retreive
         all instances in the system.
         """
+        instance_cat = instance_categories.get_instance_cat_by_name(category)
+
         if reservation_id is not None:
-            return self.get_all_across_zones(context, reservation_id)
-
-        if fixed_ip is not None:
-            return self.db.fixed_ip_get_instance(context, fixed_ip)
-
-        if project_id or not context.is_admin:
+            instances = self.get_all_across_zones(context, reservation_id)
+
+        elif fixed_ip is not None:
+            instances = \
+                self.db.fixed_ip_get_instance(context, fixed_ip)
+
+        elif project_id or not context.is_admin:
             if not context.project:
-                return self.db.instance_get_all_by_user(
-                    context, context.user_id)
-
-            if project_id is None:
-                project_id = context.project_id
-
-            return self.db.instance_get_all_by_project(
-                context, project_id)
-
-        return self.db.instance_get_all(context)
+                instances = self.db.instance_get_all_by_user(
+                                        context, context.user_id)
+            else:
+                if project_id is None:
+                    project_id = context.project_id
+
+                instances = self.db.instance_get_all_by_project(
+                                        context, project_id)
+
+        elif category is not None:
+            instances = self.db.instance_get_all_by_cat(context,
+                                                        instance_cat['id'])
+        else:
+            instances = self.db.instance_get_all(context)
+
+        return self._filter_instances(context, instances, instance_cat)
+
+    def _filter_instances(self, context, instances, instance_cat):
+        return [instance for instance in instances \
+                if instance_categories.is_access_allowed(context, instance,
+                                                         instance_cat)]
 
     def _cast_compute_message(self, method, context, instance_id, host=None,
-                              params=None):
+                              params=None, instance_cat=None):
         """Generic handler for RPC casts to compute.
 
         :param params: Optional dictionary of arguments to be passed to the
@@ -667,18 +703,23 @@
 
         :returns: None
         """
+        instance = self.get(context, instance_id)
+        if not instance_categories.is_access_allowed(context, instance,
+                                                     instance_cat):
+            raise exception.AdminRequired()
+
         if not params:
             params = {}
         if not host:
-            instance = self.get(context, instance_id)
             host = instance['host']
+
         queue = self.db.queue_get_for(context, FLAGS.compute_topic, host)
         params['instance_id'] = instance_id
         kwargs = {'method': method, 'args': params}
         rpc.cast(context, queue, kwargs)
 
     def _call_compute_message(self, method, context, instance_id, host=None,
-                              params=None):
+                              params=None, instance_cat=None):
         """Generic handler for RPC calls to compute.
 
         :param params: Optional dictionary of arguments to be passed to the
@@ -686,10 +727,14 @@
 
         :returns: Result returned by compute worker
         """
+        instance = self.get(context, instance_id)
+        if not instance_categories.is_access_allowed(context, instance,
+                                                     instance_cat):
+            raise exception.AdminRequired()
+
         if not params:
             params = {}
         if not host:
-            instance = self.get(context, instance_id)
             host = instance["host"]
         queue = self.db.queue_get_for(context, FLAGS.compute_topic, host)
         params['instance_id'] = instance_id
@@ -728,9 +773,10 @@
                                    params=params)
         return recv_meta
 
-    def reboot(self, context, instance_id):
+    def reboot(self, context, instance_id, instance_cat=None):
         """Reboot the given instance."""
-        self._cast_compute_message('reboot_instance', context, instance_id)
+        self._cast_compute_message('reboot_instance', context, instance_id,
+                                   instance_cat=instance_cat)
 
     def rebuild(self, context, instance_id, image_href, name=None,
             metadata=None, files_to_inject=None):
@@ -857,14 +903,16 @@
         self._cast_compute_message('resume_instance', context, instance_id)
 
     @scheduler_api.reroute_compute("rescue")
-    def rescue(self, context, instance_id):
+    def rescue(self, context, instance_id, instance_cat=None):
         """Rescue the given instance."""
-        self._cast_compute_message('rescue_instance', context, instance_id)
+        self._cast_compute_message('rescue_instance', context, instance_id,
+                                    instance_cat=instance_cat)
 
     @scheduler_api.reroute_compute("unrescue")
-    def unrescue(self, context, instance_id):
+    def unrescue(self, context, instance_id, instance_cat=None):
         """Unrescue the given instance."""
-        self._cast_compute_message('unrescue_instance', context, instance_id)
+        self._cast_compute_message('unrescue_instance', context, instance_id,
+                                    instance_cat=instance_cat)
 
     def set_admin_password(self, context, instance_id, password=None):
         """Set the root/admin password for the given instance."""

=== added file 'nova/compute/instance_categories.py'
--- nova/compute/instance_categories.py	1970-01-01 00:00:00 +0000
+++ nova/compute/instance_categories.py	2011-06-28 22:54:37 +0000
@@ -0,0 +1,196 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# All Rights Reserved.
+# Copyright (c) 2010 Citrix Systems, Inc.
+# Copyright 2011 Ken Pepple
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Built-in instance categories properties."""
+
+from nova import context
+from nova import db
+from nova import exception
+from nova import flags
+from nova import log as logging
+
+FLAGS = flags.FLAGS
+LOG = logging.getLogger('nova.instance_categories')
+
+
+def create(name, description, image_id=None, specs=None):
+    """Creates instance category."""
+    try:
+        db.instance_cat_create(
+                context.get_admin_context(),
+                dict(name=name,
+                    description=description,
+                    image_id=image_id,
+                    specs=specs))
+    except exception.DBError, e:
+        LOG.exception(_('DB error: %s') % e)
+        raise exception.ApiError(_("Cannot create instance category with name"
+                                   " %(name)s, description %(description)s and"
+                                   " specs %(specs)s") % locals())
+
+
+def update(name, description=None, image_id=None, specs=None):
+    """Updates instance category the instance in the datastore. """
+    rv = db.instance_cat_update(
+                    context.get_admin_context(),
+                    name,
+                    dict(description=description,
+                        image_id=image_id,
+                        specs=specs))
+    return dict(rv.iteritems())
+
+
+def destroy(name):
+    """Marks instance categories as deleted."""
+    if name is None:
+        raise exception.InvalidInstanceCategory(instance_cat=name)
+    else:
+        try:
+            db.instance_cat_destroy(context.get_admin_context(), name)
+        except exception.NotFound:
+            LOG.exception(_('Category %s not found for deletion') % name)
+            raise exception.ApiError(_("Unknown instance category: %s") % name)
+
+
+def purge(name):
+    """Removes instance categories from database."""
+    if name is None:
+        raise exception.InvalidInstanceCategory(instance_cat=name)
+    else:
+        try:
+            db.instance_cat_purge(context.get_admin_context(), name)
+        except exception.NotFound:
+            LOG.exception(_('Instance category %s not found for purge') % name)
+            raise exception.ApiError(_("Unknown instance category: %s") % name)
+
+
+def get_all_categories(inactive=0):
+    """Get all non-deleted instance categories.
+
+    Pass true as argument if you want deleted categories returned also.
+
+    """
+    return db.instance_cat_get_all(context.get_admin_context(), inactive)
+
+
+def default_category_name():
+    return FLAGS.instance_cat_regular
+
+
+def get_default_instance_cat():
+    """Get the default instance category."""
+    return get_instance_cat_by_name(default_category_name())
+
+
+def get_instance_cat(id):
+    """Retrieves single instance category by id."""
+    if id is None:
+        return get_default_instance_cat()
+    try:
+        ctxt = context.get_admin_context()
+        return db.instance_cat_get_by_id(ctxt, id)
+    except exception.NotFound:
+        LOG.exception(_('Instance category with id %d not found') % id)
+        raise
+
+
+def get_instance_cat_by_name(name):
+    """Retrieves single instance category by name."""
+    if name is None:
+        return get_default_instance_cat()
+    try:
+        ctxt = context.get_admin_context()
+        return db.instance_cat_get_by_name(ctxt, name)
+    except exception.NotFound:
+        LOG.exception(_('Instance category with name %s not found') % name)
+        raise
+
+
+def get_instance_cat_id_by_name(name):
+    """Retrieves single instance category ID by name."""
+    return get_instance_cat_by_name(name)['id']
+
+
+def get_instance_cat_name(instance):
+    """Returns instance category name by instance_ref"""
+    if 'instance_cat' in instance \
+        and 'name' in instance['instance_cat']:
+        return instance['instance_cat']['name']
+    else:
+        return get_instance_cat(instance['instance_cat_id'])['name']
+
+
+def is_admin_cat_name(name):
+    return False if (name is None) or \
+            (name == FLAGS.instance_cat_regular) else True
+
+
+def is_access_allowed(context, instance, instance_cat=None):
+    """Determines if access to instance is allowed
+
+    If admin context - allow everything, otherwise:
+    if regular instance - allow
+    otherwise (one of categories)
+        if instance's cat matches instance_cat - allow
+        if not matches: - restrict
+    """
+
+    if context.is_admin:
+        return True
+    if not is_admin_instance(instance):
+        return True
+    if instance_cat is not None \
+        and get_instance_cat_name(instance) == instance_cat['name']:
+        return True
+    else:
+        return False
+
+
+def is_admin_category(instance_cat):
+    return is_admin_cat_name(instance_cat['name'])
+
+
+def is_admin_instance(instance, category=None):
+    """ Verifies if instance is one of admin instances.
+
+    If category specified - returns True only if category matches
+    """
+    if instance['instance_cat_id'] is None:
+        return False
+
+    try:
+        inst_cat_name = get_instance_cat_name(instance)
+    except exception.NotFound:
+        raise
+
+    if inst_cat_name == FLAGS.instance_cat_regular:
+        return False
+
+    if category is not None \
+        and category != inst_cat_name:
+            return False
+
+    return True
+
+
+def get_admin_image_names(category=None):
+    """Return list of image_ids for all or specified categories"""
+    return db.instance_cat_get_all_images(context.get_admin_context(),
+                                          category)

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2011-06-28 20:37:05 +0000
+++ nova/compute/manager.py	2011-06-28 22:54:37 +0000
@@ -54,6 +54,7 @@
 from nova import utils
 from nova import volume
 from nova.compute import power_state
+from nova.compute import instance_categories
 from nova.notifier import api as notifier_api
 from nova.compute.utils import terminate_volumes
 from nova.virt import driver
@@ -297,7 +298,8 @@
                                    power_state.NOSTATE,
                                    'networking')
 
-        is_vpn = instance_ref['image_ref'] == str(FLAGS.vpn_image_id)
+        is_vpn = instance_categories.is_admin_instance(instance_ref,
+                                                       FLAGS.instance_cat_vpn)
         try:
             # NOTE(vish): This could be a cast because we don't do anything
             #             with the address currently, but I'm leaving it as

=== modified file 'nova/db/api.py'
--- nova/db/api.py	2011-06-28 20:53:45 +0000
+++ nova/db/api.py	2011-06-28 22:54:37 +0000
@@ -467,6 +467,11 @@
     return IMPL.instance_get_all_by_reservation(context, reservation_id)
 
 
+def instance_get_all_by_cat(context, instance_cat_id):
+    """Get all instance belonging to a category."""
+    return IMPL.instance_get_all_by_cat(context, instance_cat_id)
+
+
 def instance_get_fixed_address(context, instance_id):
     """Get the fixed ip address of an instance."""
     return IMPL.instance_get_fixed_address(context, instance_id)
@@ -481,9 +486,9 @@
     return IMPL.instance_get_floating_address(context, instance_id)
 
 
-def instance_get_project_vpn(context, project_id):
+def instance_get_project_admin(context, project_id, cat_id):
     """Get a vpn instance by project or return None."""
-    return IMPL.instance_get_project_vpn(context, project_id)
+    return IMPL.instance_get_project_admin(context, project_id, cat_id)
 
 
 def instance_set_state(context, instance_id, state, description=None):
@@ -1279,6 +1284,57 @@
     return IMPL.instance_type_purge(context, name)
 
 
+##################
+
+
+def instance_cat_create(context, values):
+    """Create a new instance category."""
+    return IMPL.instance_cat_create(context, values)
+
+
+def instance_cat_update(context, name, values):
+    """Set the given properties on an instance category and update it.
+
+    Raises NotFound if category does not exist.
+
+    """
+    return IMPL.instance_cat_update(context, name, values)
+
+
+def instance_cat_get_all(context, inactive=False):
+    """Get all instance categories."""
+    return IMPL.instance_cat_get_all(context, inactive)
+
+
+def instance_cat_get_by_id(context, id):
+    """Get instance category by id."""
+    return IMPL.instance_cat_get_by_id(context, id)
+
+
+def instance_cat_get_by_name(context, name):
+    """Get instance category by name."""
+    return IMPL.instance_cat_get_by_name(context, name)
+
+
+def instance_cat_get_all_images(context, category=None):
+    """Get registered images for all or selected categories."""
+    return IMPL.instance_cat_get_all_images(context, category)
+
+
+def instance_cat_destroy(context, name):
+    """Delete an instance category."""
+    return IMPL.instance_cat_destroy(context, name)
+
+
+def instance_cat_purge(context, name):
+    """Purges (removes) an instance category from DB.
+
+    Use instance_type_destroy for most cases
+
+    """
+    return IMPL.instance_cat_purge(context, name)
+
+
 ####################
 
 

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-06-28 20:53:45 +0000
+++ nova/db/sqlalchemy/api.py	2011-06-28 22:54:37 +0000
@@ -932,6 +932,7 @@
                      options(joinedload('volumes')).\
                      options(joinedload_all('fixed_ip.network')).\
                      options(joinedload('metadata')).\
+                     options(joinedload('instance_cat')).\
                      options(joinedload('instance_type'))
 
     if is_admin_context(context):
@@ -951,6 +952,7 @@
                    options(joinedload_all('fixed_ip.network')).\
                    options(joinedload('metadata')).\
                    options(joinedload('instance_type')).\
+                   options(joinedload('instance_cat')).\
                    filter_by(deleted=can_read_deleted(context)).\
                    all()
 
@@ -982,6 +984,7 @@
                    options(joinedload_all('fixed_ip.network')).\
                    options(joinedload('metadata')).\
                    options(joinedload('instance_type')).\
+                   options(joinedload('instance_cat')).\
                    filter_by(deleted=can_read_deleted(context)).\
                    filter_by(user_id=user_id).\
                    all()
@@ -996,6 +999,7 @@
                    options(joinedload_all('fixed_ip.network')).\
                    options(joinedload('metadata')).\
                    options(joinedload('instance_type')).\
+                   options(joinedload('instance_cat')).\
                    filter_by(host=host).\
                    filter_by(deleted=can_read_deleted(context)).\
                    all()
@@ -1012,6 +1016,7 @@
                    options(joinedload_all('fixed_ip.network')).\
                    options(joinedload('metadata')).\
                    options(joinedload('instance_type')).\
+                   options(joinedload('instance_cat')).\
                    filter_by(project_id=project_id).\
                    filter_by(deleted=can_read_deleted(context)).\
                    all()
@@ -1028,6 +1033,7 @@
                        options(joinedload_all('fixed_ip.network')).\
                        options(joinedload('metadata')).\
                        options(joinedload('instance_type')).\
+                       options(joinedload('instance_cat')).\
                        filter_by(reservation_id=reservation_id).\
                        filter_by(deleted=can_read_deleted(context)).\
                        all()
@@ -1038,6 +1044,7 @@
                        options(joinedload_all('fixed_ip.network')).\
                        options(joinedload('metadata')).\
                        options(joinedload('instance_type')).\
+                       options(joinedload('instance_cat')).\
                        filter_by(project_id=context.project_id).\
                        filter_by(reservation_id=reservation_id).\
                        filter_by(deleted=False).\
@@ -1045,16 +1052,32 @@
 
 
 @require_admin_context
-def instance_get_project_vpn(context, project_id):
-    session = get_session()
-    return session.query(models.Instance).\
-                   options(joinedload_all('fixed_ip.floating_ips')).\
-                   options(joinedload('security_groups')).\
-                   options(joinedload_all('fixed_ip.network')).\
-                   options(joinedload('metadata')).\
-                   options(joinedload('instance_type')).\
+def instance_get_all_by_cat(context, instance_cat_id):
+    session = get_session()
+    return session.query(models.Instance).\
+                   options(joinedload_all('fixed_ip.floating_ips')).\
+                   options(joinedload('security_groups')).\
+                   options(joinedload_all('fixed_ip.network')).\
+                   options(joinedload('metadata')).\
+                   options(joinedload('instance_type')).\
+                   options(joinedload('instance_cat')).\
+                   filter_by(instance_cat_id=instance_cat_id).\
+                   filter_by(deleted=can_read_deleted(context)).\
+                   all()
+
+
+@require_admin_context
+def instance_get_project_admin(context, project_id, cat_id):
+    session = get_session()
+    return session.query(models.Instance).\
+                   options(joinedload_all('fixed_ip.floating_ips')).\
+                   options(joinedload('security_groups')).\
+                   options(joinedload_all('fixed_ip.network')).\
+                   options(joinedload('metadata')).\
+                   options(joinedload('instance_type')).\
+                   options(joinedload('instance_cat')).\
                    filter_by(project_id=project_id).\
-                   filter_by(image_ref=str(FLAGS.vpn_image_id)).\
+                   filter_by(instance_cat_id=cat_id).\
                    filter_by(deleted=can_read_deleted(context)).\
                    first()
 
@@ -2733,7 +2756,7 @@
                     first()
 
     if not inst_type:
-        raise exception.InstanceTypeNotFound(instance_type=id)
+        raise exception.InstanceTypeNotFound(instance_type_id=id)
     else:
         return _dict_with_extra_specs(inst_type)
 
@@ -2794,6 +2817,133 @@
         return instance_type_ref
 
 
+    ##################
+
+
+@require_admin_context
+def instance_cat_create(_context, values):
+    try:
+        instance_cat_ref = models.InstanceCategories()
+        instance_cat_ref.update(values)
+        instance_cat_ref.save()
+    except Exception, e:
+        raise exception.DBError(e)
+    return instance_cat_ref
+
+
+@require_admin_context
+def instance_cat_update(context, name, values):
+    session = get_session()
+    with session.begin():
+        instance_cat_ref = session.query(models.InstanceCategories).\
+                    filter_by(name=name).\
+                    first()
+        instance_cat_ref.update(values)
+        instance_cat_ref.save(session=session)
+        return instance_cat_ref
+
+
+@require_context
+def instance_cat_get_all(context, inactive=False):
+    """
+    Returns a dict describing all instance_categories with name as key.
+    """
+    session = get_session()
+    if inactive:
+        inst_cats = session.query(models.InstanceCategories).\
+                        order_by("name").\
+                        all()
+    else:
+        inst_cats = session.query(models.InstanceCategories).\
+                        filter_by(deleted=False).\
+                        order_by("name").\
+                        all()
+    if inst_cats:
+        inst_cat_dict = {}
+        for i in inst_cats:
+            inst_cat_dict[i['name']] = dict(i)
+        return inst_cat_dict
+    else:
+        raise exception.NoInstanceCategoriesFound()
+
+
+@require_context
+def instance_cat_get_by_id(context, id):
+    """Returns a dict describing specific instance category"""
+    session = get_session()
+    inst_cat = session.query(models.InstanceCategories).\
+                    filter_by(id=id).\
+                    first()
+    if not inst_cat:
+        raise exception.InstanceCatNotFound(instance_cat_id=id)
+    else:
+        return dict(inst_cat)
+
+
+@require_context
+def instance_cat_get_by_name(context, name):
+    """Returns a dict describing specific instance category"""
+    session = get_session()
+    inst_cat = session.query(models.InstanceCategories).\
+                    filter_by(name=name).\
+                    first()
+    if not inst_cat:
+        raise exception.InstanceCatNotFoundByName(instance_cat_name=name)
+    else:
+        return dict(inst_cat)
+
+
+@require_admin_context
+def instance_cat_destroy(context, name):
+    """ Marks specific instance category as deleted"""
+    session = get_session()
+    instance_cat_ref = session.query(models.InstanceCategories).\
+                                      filter_by(name=name)
+    records = instance_cat_ref.update(dict(deleted=True))
+    if records == 0:
+        raise exception.InstanceCatNotFoundByName(instance_cat_name=name)
+    else:
+        return instance_cat_ref
+
+
+@require_admin_context
+def instance_cat_purge(context, name):
+    """ Removes specific instance category from DB
+        Usually instance_cat_destroy should be used
+    """
+    session = get_session()
+    instance_cat_ref = session.query(models.InstanceCategories).\
+                                      filter_by(name=name)
+    records = instance_cat_ref.delete()
+    if records == 0:
+        raise exception.InstanceCatNotFoundByName(instance_cat_name=name)
+    else:
+        return instance_cat_ref
+
+
+@require_context
+def instance_cat_get_all_images(context, category=None):
+    """
+    Returns a dict describing all instance_categories with name as key.
+    """
+    session = get_session()
+    if category is None:
+        images = session.query(models.InstanceCategories).\
+                        filter(models.InstanceCategories.image_id != None).\
+                        filter(models.InstanceCategories.name != \
+                                        FLAGS.instance_cat_regular).\
+                        filter_by(deleted=False).\
+                        all()
+    else:
+        images = session.query(models.InstanceCategories).\
+                        filter(models.InstanceCategories.image_id != None).\
+                        filter(models.InstanceCategories.name != \
+                                        FLAGS.instance_cat_regular).\
+                        filter_by(deleted=False).\
+                        filter_by(name=category).\
+                        all()
+    return [image['image_id'] for image in images]
+
 ####################
 
 

=== added file 'nova/db/sqlalchemy/migrate_repo/versions/029_add_instance_categories.py'
--- nova/db/sqlalchemy/migrate_repo/versions/029_add_instance_categories.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/029_add_instance_categories.py	2011-06-28 22:54:37 +0000
@@ -0,0 +1,152 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 OpenStack LLC.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import Column, DateTime, Integer, MetaData, String, Table
+from sqlalchemy import Text, Boolean
+# ForeignKey
+
+from nova import log as logging
+
+meta = MetaData()
+
+instances = Table('instances', meta,
+        Column('id', Integer(),  primary_key=True, nullable=False),
+        Column('image_ref', String()),
+        )
+
+instance_cat_id = Column('instance_cat_id', Integer(), nullable=True)
+
+#
+# New Tables
+#
+instance_categories = Table('instance_categories', meta,
+        Column('created_at', DateTime(timezone=False)),
+        Column('updated_at', DateTime(timezone=False)),
+        Column('deleted_at', DateTime(timezone=False)),
+        Column('deleted', Boolean(create_constraint=True, name=None)),
+        Column('id', Integer(),  primary_key=True, nullable=False),
+        Column('name',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False),
+                      unique=True),
+        Column('description',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False),
+                      unique=False, nullable=True),
+        Column('image_id',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False),
+                      unique=False, nullable=True),
+        Column('specs',
+               Text(length=None, convert_unicode=False, assert_unicode=None,
+                    unicode_error=None, _warn_on_bytestring=False),
+                    nullable=True))
+
+
+def upgrade(migrate_engine):
+
+    from nova import context
+    from nova import db
+    from nova import flags
+
+    FLAGS = flags.FLAGS
+
+    # Upgrade operations go here. Don't create your own engine;
+    # bind migrate_engine to your metadata
+    meta.bind = migrate_engine
+
+    # Create instance_categories table
+    try:
+        instance_categories.create()
+    except Exception:
+        logging.info(repr(instance_categories))
+        logging.exception('Exception while creating instance_categories table')
+        raise
+
+    # Fill it up with default Categories
+    INSTANCE_CATEGORIES = {
+        FLAGS.instance_cat_regular: \
+            dict(description='Regular Compute Instance',
+                 image_id=None,
+                 specs=None),
+        FLAGS.instance_cat_vc: \
+            dict(description='Virtual Storage Controller',
+                 image_id=None,
+                 specs=None),
+        FLAGS.instance_cat_vpn: \
+            dict(description='VPN Instance',
+                 image_id=str(FLAGS.vpn_image_id),
+                 specs=None),
+        FLAGS.instance_cat_admin: \
+            dict(description='Generic Admin Instance',
+                 image_id=None,
+                 specs=None)}
+
+    try:
+        for name, values in INSTANCE_CATEGORIES.iteritems():
+            db.instance_cat_create(context.get_admin_context(),
+                                    {'name': name,
+                                     'description': values["description"],
+                                     'image_id': values["image_id"],
+                                     'specs': values["specs"]})
+    except Exception:
+        logging.exception('Exception while adding default values to ' \
+                          'instance_categories table')
+        instance_categories.drop()
+        raise
+
+    # Add appropriate column to instances and fill it up with default values
+    instances.create_column(instance_cat_id)
+
+    try:
+        reg_recs = migrate_engine.execute(
+                        instance_categories.select().\
+                        where(instance_categories.c.name == \
+                                FLAGS.instance_cat_regular)).\
+                        fetchone()
+        vpn_recs = migrate_engine.execute(
+                        instance_categories.select().\
+                        where(instance_categories.c.name == \
+                                FLAGS.instance_cat_vpn)).\
+                        fetchone()
+
+        reg_cat_id = reg_recs['id']
+        vpn_cat_id = vpn_recs['id']
+
+        migrate_engine.execute(instances.update().\
+                               where(instances.c.instance_cat_id == None).\
+                               where(instances.c.image_ref != \
+                                        str(FLAGS.vpn_image_id)).\
+                               values(instance_cat_id=reg_cat_id))
+
+        migrate_engine.execute(instances.update().\
+                               where(instances.c.instance_cat_id == None).\
+                               where(instances.c.image_ref == \
+                                        str(FLAGS.vpn_image_id)).\
+                               values(instance_cat_id=vpn_cat_id))
+    except Exception:
+        logging.info(repr(instances))
+        logging.exception('Exception while setting default categories ' \
+                          'in instances table')
+        raise
+
+
+def downgrade(migrate_engine):
+    meta.bind = migrate_engine
+
+    instances.drop_column(instance_cat_id)
+
+    instance_categories.drop()

=== modified file 'nova/db/sqlalchemy/migration.py'
--- nova/db/sqlalchemy/migration.py	2011-06-24 12:01:51 +0000
+++ nova/db/sqlalchemy/migration.py	2011-06-28 22:54:37 +0000
@@ -64,7 +64,7 @@
                           'users', 'user_project_association',
                           'user_project_role_association',
                           'user_role_association',
-                          'volumes'):
+                          'volumes', 'instance_categories'):
                 assert table in meta.tables
             return db_version_control(1)
         except AssertionError:

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-06-27 00:02:18 +0000
+++ nova/db/sqlalchemy/models.py	2011-06-28 22:54:37 +0000
@@ -210,6 +210,7 @@
     host = Column(String(255))  # , ForeignKey('hosts.id'))
 
     instance_type_id = Column(Integer)
+    instance_cat_id = Column(Integer)
 
     user_data = Column(Text)
 
@@ -278,6 +279,22 @@
                                        'InstanceTypes.id)')
 
 
+class InstanceCategories(BASE, NovaBase):
+    """Represent possible instance_categories of VM offered"""
+    __tablename__ = "instance_categories"
+    id = Column(Integer, primary_key=True)
+    name = Column(String(255), unique=True)
+    description = Column(String(255), nullable=True)
+    image_id = Column(String(255), nullable=True)
+    specs = Column(Text, nullable=True)
+
+    instances = relationship(Instance,
+                           backref=backref('instance_cat', uselist=False),
+                           foreign_keys=id,
+                           primaryjoin='and_(Instance.instance_cat_id == '
+                                       'InstanceCategories.id)')
+
+
 class Volume(BASE, NovaBase):
     """Represents a block storage device that can be attached to a vm."""
     __tablename__ = 'volumes'
@@ -765,7 +782,8 @@
               Network, SecurityGroup, SecurityGroupIngressRule,
               SecurityGroupInstanceAssociation, AuthToken, User,
               Project, Certificate, ConsolePool, Console, Zone,
-              AgentBuild, InstanceMetadata, InstanceTypeExtraSpecs, Migration)
+              AgentBuild, InstanceMetadata, InstanceTypeExtraSpecs, Migration,
+              InstanceCategories)
     engine = create_engine(FLAGS.sql_connection, echo=False)
     for model in models:
         model.metadata.create_all(engine)

=== modified file 'nova/exception.py'
--- nova/exception.py	2011-06-28 15:13:52 +0000
+++ nova/exception.py	2011-06-28 22:54:37 +0000
@@ -145,6 +145,10 @@
     message = _("Invalid instance type %(instance_type)s.")
 
 
+class InvalidInstanceCategory(Invalid):
+    message = _("Invalid instance category %(instance_cat)s.")
+
+
 class InvalidPortRange(Invalid):
     message = _("Invalid port range %(from_port)s:%(to_port)s.")
 
@@ -482,6 +486,19 @@
                 "could not be found.")
 
 
+class NoInstanceCategoriesFound(NotFound):
+    message = _("Zero instance categories found.")
+
+
+class InstanceCatNotFound(NotFound):
+    message = _("Instance category %(instance_cat_id)d could not be found.")
+
+
+class InstanceCatNotFoundByName(InstanceCatNotFound):
+    message = _("Instance category with name %(instance_cat_name)s "
+                "could not be found.")
+
+
 class FlavorNotFound(NotFound):
     message = _("Flavor %(flavor_id)s could not be found.")
 

=== modified file 'nova/flags.py'
--- nova/flags.py	2011-06-24 12:01:51 +0000
+++ nova/flags.py	2011-06-28 22:54:37 +0000
@@ -324,6 +324,16 @@
               'default image to use, testing only')
 DEFINE_string('default_instance_type', 'm1.small',
               'default instance type to use, testing only')
+
+DEFINE_string('instance_cat_regular', 'regular',
+              'default instance category for regular compute instances')
+DEFINE_string('instance_cat_vc', 'vc',
+              'instance category for VC instances')
+DEFINE_string('instance_cat_vpn', 'vpn',
+              'instance category for VPN instances')
+DEFINE_string('instance_cat_admin', 'admin',
+              'instance category for generic Admin instances')
+
 DEFINE_string('null_kernel', 'nokernel',
               'kernel image that indicates not to use a kernel,'
               ' but to use a raw disk image instead')

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-06-24 12:01:51 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-06-28 22:54:37 +0000
@@ -34,6 +34,7 @@
 from nova.api.openstack import create_instance_helper
 import nova.compute.api
 from nova.compute import instance_types
+from nova.compute import instance_categories
 from nova.compute import power_state
 import nova.db.api
 import nova.scheduler.api
@@ -129,6 +130,7 @@
     metadata.append(InstanceMetadata(key='seq', value=id))
 
     inst_type = instance_types.get_instance_type_by_flavor_id(1)
+    inst_cat = instance_categories.get_default_instance_cat()
 
     if public_addresses is None:
         public_addresses = list()
@@ -160,6 +162,8 @@
         "hostname": "",
         "host": host,
         "instance_type": dict(inst_type),
+        "instance_cat": dict(inst_cat),
+        "instance_cat_id": dict(inst_cat)['id'],
         "user_data": "",
         "reservation_id": reservation_id,
         "mac_address": "",

=== added file 'nova/tests/test_instance_categories.py'
--- nova/tests/test_instance_categories.py	1970-01-01 00:00:00 +0000
+++ nova/tests/test_instance_categories.py	2011-06-28 22:54:37 +0000
@@ -0,0 +1,295 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 Ken Pepple
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+"""
+Unit Tests for instance categories code
+"""
+import time
+
+from nova import context
+from nova import db
+from nova import exception
+from nova import flags
+from nova import log as logging
+from nova import test
+from nova import utils
+from nova.compute import instance_categories
+from nova.db.sqlalchemy.session import get_session
+from nova.db.sqlalchemy import models
+
+FLAGS = flags.FLAGS
+LOG = logging.getLogger('nova.tests.compute')
+
+
+class InstanceCatTestCase(test.TestCase):
+    """Test cases for instance categories code"""
+    def setUp(self):
+        super(InstanceCatTestCase, self).setUp()
+        self.cntx = context.RequestContext(None, None)
+        self.cntx_admin = context.get_admin_context()
+        session = get_session()
+        max_id = session.query(models.InstanceCategories).\
+                                     order_by("id desc").\
+                                     first()
+        self.name = str(int(time.time()))
+        self.description = "Description_for_" + self.name
+        self.image_id = "Image_str_for_" + self.name
+
+    def test_instance_cat_create_then_delete(self):
+        """Ensure instance types can be created"""
+        starting_list_active = instance_categories.get_all_categories()
+        starting_list_inactive = instance_categories.get_all_categories(1)
+        instance_categories.create(self.name, self.description)
+        new = instance_categories.get_all_categories()
+        self.assertNotEqual(len(starting_list_active),
+                            len(new),
+                            'instance category was not created')
+        self.assertEqual(self.name,
+                         instance_categories.\
+                             get_instance_cat_by_name(self.name)['name'],
+                         'instance category with expected name not found '\
+                         'or doesnt match')
+
+        cat_id = instance_categories.get_instance_cat_id_by_name(self.name)
+
+        self.assertEqual(self.name,
+                         instance_categories.get_instance_cat(cat_id)["name"],
+                         'instance category with expected ID not found '\
+                         'or doesnt match')
+        self.assertEqual(self.description,
+                         instance_categories.\
+                             get_instance_cat(cat_id)["description"],
+                         'instance category description with expected ID '\
+                         'not found or doesnt match')
+
+        instance_categories.destroy(self.name)
+        self.assertNotEqual(instance_categories.get_all_categories(),
+                            instance_categories.get_all_categories(1),
+                            'active and inactive should be different')
+        self.assertEqual(1,
+                         instance_categories.\
+                             get_instance_cat(cat_id)["deleted"],
+                         'instance category is not marked as deleted')
+        self.assertEqual(starting_list_active,
+                         instance_categories.get_all_categories(),
+                         'instance category was not deleted')
+
+        instance_categories.purge(self.name)
+        self.assertEqual(starting_list_inactive,
+                         instance_categories.get_all_categories(1),
+                         'instance category not purged')
+
+    def test_instance_cat_update(self):
+        """Ensure instance types can be created"""
+        instance_categories.create(self.name, self.description, self.image_id)
+        cat_id = instance_categories.get_instance_cat_id_by_name(self.name)
+
+        self.assertEqual(self.name,
+                         instance_categories.get_instance_cat(cat_id)["name"],
+                         'instance category with expected ID not found '\
+                         'or doesnt match')
+        self.assertEqual(self.description,
+                         instance_categories.\
+                             get_instance_cat(cat_id)["description"],
+                         'description not found or doesnt match')
+        self.assertEqual(self.image_id,
+                         instance_categories.\
+                             get_instance_cat(cat_id)["image_id"],
+                         'image_id not found or doesnt match')
+
+        new_desc = 'New desc'
+        new_image_id = 'New image id'
+        instance_categories.update(self.name, new_desc, new_image_id)
+
+        self.assertEqual(new_desc,
+                         instance_categories.\
+                             get_instance_cat(cat_id)["description"],
+                         'description not found or doesnt match')
+        self.assertEqual(new_image_id,
+                         instance_categories.\
+                             get_instance_cat(cat_id)["image_id"],
+                         'image_id not found or doesnt match')
+
+        instance_categories.destroy(self.name)
+        instance_categories.purge(self.name)
+
+    def test_instance_cat_non_existant_shouldnt_delete_purge(self):
+        """
+        Ensures that instance category deletion/purge fails with invalid args
+        """
+        self.assertRaises(exception.InvalidInstanceCategory,
+                          instance_categories.destroy, None)
+        self.assertRaises(exception.ApiError,
+                          instance_categories.destroy, "sfsfsdfdfs")
+
+        self.assertRaises(exception.InvalidInstanceCategory,
+                          instance_categories.purge, None)
+        self.assertRaises(exception.ApiError,
+                          instance_categories.purge, "qweqwe")
+
+    def test_instance_cat_get_all_cats(self):
+        """Ensures that all instance categories can be retrieved"""
+        session = get_session()
+        total_instance_cats = session.query(models.InstanceCategories).\
+                                            count()
+        instance_cats = len(instance_categories.get_all_categories(1))
+        self.assertEqual(total_instance_cats, instance_cats)
+
+    def test_instance_cat_dup_name_should_raise_api_error(self):
+        """Ensures that duplicate instance category raises ApiError"""
+        new_name = self.name + "dup"
+        instance_categories.create(new_name, "desc - duplicate test")
+        instance_categories.destroy(new_name)
+        self.assertRaises(exception.ApiError,
+                          instance_categories.create, new_name,
+                          "desc - duplicate category")
+        instance_categories.purge(new_name)
+
+    def test_instance_cat_get_by_incorrect_id_name(self):
+        """Ensures that get cat fails with incorrect args
+        or returns default values"""
+        self.assertEqual(instance_categories.get_instance_cat(None),
+                         instance_categories.get_default_instance_cat(),
+                         'get instance category without id should return '\
+                         'default category')
+
+        self.assertEqual(instance_categories.get_instance_cat_by_name(None),
+                         instance_categories.get_default_instance_cat(),
+                         'get instance category without name should return '\
+                         'default category')
+
+        session = get_session()
+        max_id = session.query(models.InstanceCategories).\
+                                     order_by("id desc").\
+                                     first()
+        wrong_id = max_id["id"] + 100
+        self.assertRaises(exception.NotFound,
+                          instance_categories.get_instance_cat, wrong_id)
+
+        self.assertRaises(exception.NotFound,
+                          instance_categories.get_instance_cat_by_name,
+                          "invalid_cat_name_qweqwe")
+
+    def test_instance_cat_is_admin_name(self):
+        """Ensures that is_admin_cat_name returns True for exerything except
+        empty or regular cat names"""
+        self.assertFalse(instance_categories.is_admin_cat_name(None))
+        self.assertFalse(instance_categories.\
+                            is_admin_cat_name(FLAGS.instance_cat_regular))
+        self.assertTrue(instance_categories.\
+                            is_admin_cat_name(FLAGS.instance_cat_vpn))
+        self.assertTrue(instance_categories.\
+                            is_admin_cat_name(FLAGS.instance_cat_vc))
+        self.assertTrue(instance_categories.\
+                            is_admin_cat_name(FLAGS.instance_cat_admin))
+        self.assertTrue(instance_categories.\
+                            is_admin_cat_name("any_cat_name_asdqwe"))
+
+    def test_instance_cat_is_admin_category(self):
+        """Ensures that is_admin_category returns True for exerything
+        except empty or regular cat names"""
+        cat = {}
+        cat['name'] = FLAGS.instance_cat_regular
+        self.assertFalse(instance_categories.is_admin_category(cat))
+        cat['name'] = FLAGS.instance_cat_admin
+        self.assertTrue(instance_categories.is_admin_cat_name(cat))
+
+    def test_instance_cat_check_default_values(self):
+        """Ensures correctness of default values"""
+        self.assertEqual(instance_categories.default_category_name(),
+                         FLAGS.instance_cat_regular)
+        self.assertEqual(instance_categories.\
+                            get_default_instance_cat()['name'],
+                         FLAGS.instance_cat_regular)
+
+    def test_instance_cat_create_check_params_and_admin_func(self):
+        """Ensures that different get and is_admin funcs operate correctly"""
+        instance_categories.create(self.name, self.description)
+
+        self.assertEqual(self.name,
+                         instance_categories.\
+                            get_instance_cat_by_name(self.name)['name'])
+
+        cat_id = instance_categories.get_instance_cat_id_by_name(self.name)
+        self.assertEqual(self.name,
+                         instance_categories.get_instance_cat(cat_id)['name'])
+
+        instance = {}
+        instance['name'] = 'test compute instance'
+        instance['instance_cat_id'] = None
+        # no 'instance_cat_id' field
+        self.assertFalse(instance_categories.is_admin_instance(instance))
+
+        instance['instance_cat_id'] = cat_id + 1
+        # invalid category id
+        self.assertRaises(exception.NotFound,
+                          instance_categories.is_admin_instance, instance)
+
+        instance['instance_cat_id'] = cat_id
+        self.assertTrue(instance_categories.is_admin_instance(instance))
+        self.assertEqual(self.name,
+                         instance_categories.get_instance_cat_name(instance))
+
+        instance['instance_cat'] = {}
+        self.assertEqual(self.name,
+                         instance_categories.get_instance_cat_name(instance))
+        instance['instance_cat']['name'] = self.name
+        self.assertEqual(self.name,
+                         instance_categories.get_instance_cat_name(instance))
+
+        self.assertTrue(instance_categories.is_admin_instance(instance))
+        self.assertTrue(instance_categories.\
+                            is_admin_instance(instance,
+                                              self.name))
+        self.assertFalse(instance_categories.\
+                            is_admin_instance(instance,
+                                              FLAGS.instance_cat_vpn))
+
+        instance['instance_cat']['name'] = FLAGS.instance_cat_regular
+        self.assertFalse(instance_categories.is_admin_instance(instance))
+        self.assertFalse(instance_categories.\
+                            is_admin_instance(instance,
+                                              FLAGS.instance_cat_regular))
+        self.assertFalse(instance_categories.\
+                            is_admin_instance(instance,
+                                              FLAGS.instance_cat_vpn))
+
+        instance_categories.destroy(self.name)
+        instance_categories.purge(self.name)
+
+        self.assertTrue(instance_categories.\
+                            is_access_allowed(self.cntx, instance))
+
+        instance['instance_cat']['name'] = FLAGS.instance_cat_vpn
+        self.assertFalse(instance_categories.\
+                            is_access_allowed(self.cntx, instance))
+
+        instance_cat = instance_categories.\
+                            get_instance_cat_by_name(FLAGS.instance_cat_vpn)
+        self.assertTrue(instance_categories.\
+                            is_access_allowed(self.cntx,
+                                              instance, instance_cat))
+        self.assertTrue(instance_categories.\
+                            is_access_allowed(self.cntx_admin,
+                                              instance))
+
+        instance_cat = instance_categories.\
+                            get_instance_cat_by_name(FLAGS.instance_cat_vc)
+        self.assertTrue(instance_categories.\
+                            is_access_allowed(self.cntx_admin,
+                                              instance, instance_cat))
+
+        instance['instance_cat']['name'] = FLAGS.instance_cat_regular
+        self.assertTrue(instance_categories.\
+                            is_access_allowed(self.cntx_admin, instance))

=== modified file 'nova/virt/libvirt/firewall.py'
--- nova/virt/libvirt/firewall.py	2011-06-24 12:01:51 +0000
+++ nova/virt/libvirt/firewall.py	2011-06-28 22:54:37 +0000
@@ -26,6 +26,7 @@
 from nova import log as logging
 from nova import utils
 from nova.virt.libvirt import netutils
+from nova.compute import instance_categories
 
 
 LOG = logging.getLogger("nova.virt.libvirt.firewall")
@@ -204,7 +205,8 @@
         logging.info('ensuring static filters')
         self._ensure_static_filters()
 
-        if instance['image_ref'] == str(FLAGS.vpn_image_id):
+        if instance_categories.is_admin_instance(instance,
+                                                 FLAGS.instance_cat_vpn):
             base_filter = 'nova-vpn'
         else:
             base_filter = 'nova-base'
@@ -375,7 +377,8 @@
 
     def _create_network_filters(self, instance, network_info,
                                instance_secgroup_filter_name):
-        if instance['image_ref'] == str(FLAGS.vpn_image_id):
+        if instance_categories.is_admin_instance(instance,
+                                                 FLAGS.instance_cat_vpn):
             base_filter = 'nova-vpn'
         else:
             base_filter = 'nova-base'