Merge lp:~soren/vmbuilder/0.12.settings into lp:vmbuilder

[Soren Hansen]

[Andreas Heck (aheck)]

Great to see that you are creating an abstraction of the VM configuration to clearly separate it from the CLI option parsing or whatever is used to tell vmbuilder how to configure the VM :)

Following questions occurred to me:

What about discoverability of available settings? Ideally the configuration should contain enough metadata so that you can create the usage output of the CLI version and input fields in a GTK or web UI from it.

Also IMHO there should be a distinction on which settings are mandatory and which are optional because you also have to check that when validating the input of the frontend.

How do you intend to model relationships between different settings (e.g. when the user chooses 'Dapper' as the suite he can no longer use 'ext4' or 'grub2')? Such decisions could be delegated to the plugin maybe by putting a callback into the Setting object.

If you don't solve those problems in a generic way right down in the core you have to duplicate this logic for every frontend.

[Soren Hansen (soren)]

On Fri, Jan 22, 2010 at 11:22:50PM -0000, Andreas Heck wrote:
> Great to see that you are creating an abstraction of the VM
> configuration to clearly separate it from the CLI option parsing or
> whatever is used to tell vmbuilder how to configure the VM :)

Yes, it's long overdue. :)

> Following questions occurred to me:
>
> What about discoverability of available settings? Ideally the
> configuration should contain enough metadata so that you can create
> the usage output of the CLI version and input fields in a GTK or web
> UI from it.

That is a good question.

A frontend can get a list of plugins, and interrogate them for available
settings. This will not get you further than a simple list of settings
and their help text, i.e. no multiple choice answers or anything like
that. Perhaps it would be an idea to extend the Setting class with a
"valid_options" attribute.

> Also IMHO there should be a distinction on which settings are
> mandatory and which are optional because you also have to check that
> when validating the input of the frontend.

All settings should be optional and have good default values. What sort
of setting would you consider mandatory?

> How do you intend to model relationships between different settings
> (e.g. when the user chooses 'Dapper' as the suite he can no longer use
> 'ext4' or 'grub2')?

That's a good question as well :)

Once my whole refactoring change set is in, plugins will not be
instantiated until after the Distro object has been initialised, so a
plugin could set a Setting's valid_options attribute knowing which
distro is being built. However, something like the mkfs code in disk.py
should have no notion of which filesystems are supported by which of
Ubuntu's different series. In other words, it needs to have a way to ask
the distro which filesystems would be acceptable. I'm inclined to use a
get_acceptable_filesystems method on the Distro objects for now and try
to attach this more generically once we have a few more examples of this
sort of interaction.

Also, the plugin priority system also plays to our benefit here: If a
particular plugin needs to make decisions based on settings from other
plugins, we can set a low priority so that it runs late in the sequence
and can use data from earlier initialised plugins.

--
Soren Hansen |
Lead virtualisation engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/

[Andreas Heck (aheck)]

> > Following questions occurred to me:
> >
> > What about discoverability of available settings? Ideally the
> > configuration should contain enough metadata so that you can create
> > the usage output of the CLI version and input fields in a GTK or web
> > UI from it.
>
> That is a good question.
>
> A frontend can get a list of plugins, and interrogate them for available
> settings. This will not get you further than a simple list of settings
> and their help text, i.e. no multiple choice answers or anything like
> that. Perhaps it would be an idea to extend the Setting class with a
> "valid_options" attribute.

I also queried the available plugins for their information in vmbuilder-gui
but it is kind of cumbersome. It would be great if there was some kind of
easy to use internal interface like methods in the Settings class or something
similar where frontends can get all the information from they need.

> > Also IMHO there should be a distinction on which settings are
> > mandatory and which are optional because you also have to check that
> > when validating the input of the frontend.
>
> All settings should be optional and have good default values. What sort
> of setting would you consider mandatory?

I'm not sure anymore. Now that I think about it again the distinction is not
at strict as I thought previously. But if you call vmbuilder without parameters
you get this:

"vmbuilder: error: You need to specify at least the hypervisor type and the distro"

So maybe hypervisor and distro could be considered as mandatory.

But I'm not sure anymore if such a distinction makes that much sense.

> > How do you intend to model relationships between different settings
> > (e.g. when the user chooses 'Dapper' as the suite he can no longer use
> > 'ext4' or 'grub2')?
>
> That's a good question as well :)
>
> Once my whole refactoring change set is in, plugins will not be
> instantiated until after the Distro object has been initialised, so a
> plugin could set a Setting's valid_options attribute knowing which
> distro is being built. However, something like the mkfs code in disk.py
> should have no notion of which filesystems are supported by which of
> Ubuntu's different series. In other words, it needs to have a way to ask
> the distro which filesystems would be acceptable. I'm inclined to use a
> get_acceptable_filesystems method on the Distro objects for now and try
> to attach this more generically once we have a few more examples of this
> sort of interaction.
>
> Also, the plugin priority system also plays to our benefit here: If a
> particular plugin needs to make decisions based on settings from other
> plugins, we can set a low priority so that it runs late in the sequence
> and can use data from earlier initialised plugins.

It would also be great if you could interactively build a configuration object
with a program or with the python cli so that you can change the distro version
for example and query the config object which filesystems are available with
this distro version as often as you want.

Feel free to take a look at vmbuilder-gui and play around with the options on
the first tab to get a live expression of what I mean. I ...

[Soren Hansen (soren)]

On Sun, Jan 24, 2010 at 12:38:20AM -0000, Andreas Heck wrote:
>> A frontend can get a list of plugins, and interrogate them for
>> available settings. This will not get you further than a simple list
>> of settings and their help text, i.e. no multiple choice answers or
>> anything like that. Perhaps it would be an idea to extend the Setting
>> class with a "valid_options" attribute.
> I also queried the available plugins for their information in
> vmbuilder-gui but it is kind of cumbersome. It would be great if there
> was some kind of easy to use internal interface like methods in the
> Settings class or something similar where frontends can get all the
> information from they need.

Can you explain a bit more about the sort of information you would like,
but cannot get or that is difficult to get?

>>> Also IMHO there should be a distinction on which settings are
>>> mandatory and which are optional because you also have to check that
>>> when validating the input of the frontend.
>> All settings should be optional and have good default values. What
>> sort of setting would you consider mandatory?
> I'm not sure anymore. Now that I think about it again the distinction
> is not at strict as I thought previously. But if you call vmbuilder
> without parameters you get this:
>
> "vmbuilder: error: You need to specify at least the hypervisor type and the distro"
>
> So maybe hypervisor and distro could be considered as mandatory.

Yes, I never considered hypervisor and distro "settings" like this. They
are core definitions of context, without which no settings really make
sense. It's not until both hypervisor and distro have been set that the
rest of the settings are even registered. This is completely
intentional. There are options that cannot be registered until we know
which distro or hypervisor is used. This is going to be reworked
somewhat going forward, but the basic idea is the same: Registering
settings and setting their defaults should not happen until we know
something about the context.

> Feel free to take a look at vmbuilder-gui and play around with the
> options on the first tab to get a live expression of what I mean. I
> implemented it with the MVC pattern where the Controllers (GTK signal
> handlers) modify the model (configuration) which then calls into the
> view (GTK widgets) to change it to reflect the changes of dependent
> options that occurred because of the selection the user just made.

Would all of your concerns be addressed by extending the Setting class
with a .valid_options attribute holding a list of valid choices for the
particular setting?

> It would be highly desirable to have this logic in the frontend
> agnostic core of vmbuilder.

I definitely agree that this logic does not belong in the frontend.

--
Soren Hansen |
Lead virtualisation engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/

[Andreas Heck (aheck)]

> Can you explain a bit more about the sort of information you would like,
> but cannot get or that is difficult to get?

The main difficulty with getting the necessary information was to actually find out where
to get it from.

I basically queried stuff like:

- What Hypervisors are there?
- What Distros are there?
- Which Distro has which Suites?
- Which Suite supports which Filesystems?
- Which Suite supports which Kernels?
- Which Suite supports which Architectures?
- What are the maximums and minimums for rootsize, swapsize, number of cpus, ramsize

And as I recall I was also unsure if other important pieces of information like
possible limitations imposed by the choice of hypervisor are "present" in vmbuilder at all.
Since I never tried to use them I don't know for sure but I think it's something to
keep in mind.

Now although I got all the information from that list I always kind of had to hunt them
down. Most of this stuff can be retrieved quite easily with digging through the source
code a bit but you shouldn't need to dig through the code to get this simple pieces
of information IMHO. And then you also access pieces of VMBuilder that could be considered
as internal which also isn't a good thing.

So it would be much easier if there was just one object that you could ask all
these things and which acted as an interface for frontends. This would not also make
things much easier for frontend developers but once you got it right you could refactor
VMBuilders internals much more easily.

Actually, when confronted with this problem of getting information out of VMBuilder I just
wrote my own interface that provides all the information I'm interested in (VMBuilder.vm.model).
It's a model object that represents the buildable configuration of a VM and that can be
asked for valid options (where some of them can change when another option is changed) and
that can validate the values of its options and therefore check its consistency. If you
finished configuring it you can just tell it to give you the vmbuilder command-line that
builds this machine.

> Yes, I never considered hypervisor and distro "settings" like this. They
> are core definitions of context, without which no settings really make
> sense. It's not until both hypervisor and distro have been set that the
> rest of the settings are even registered. This is completely
> intentional. There are options that cannot be registered until we know
> which distro or hypervisor is used. This is going to be reworked
> somewhat going forward, but the basic idea is the same: Registering
> something about the context.

I think this is problematic for interactive frontends. You then have to newly instantiate
those context objects every time the user changes his mind about the hypervisor
or distro or work around that somehow. This could also make it more difficult to
serialize the VM configuration. Currently, vmbuilder VMs are just passed in emails
or on IRC by there command-line. With other frontends it might become worthwhile
to write a configuration to a file or read it back from a file to build it.

> Would all of your concerns be addressed by extending the Setting class
> with a .valid_option...

[Soren Hansen (soren)]

On Mon, Jan 25, 2010 at 10:33:37PM -0000, Andreas Heck wrote:
> > Can you explain a bit more about the sort of information you would like,
> > but cannot get or that is difficult to get?
> The main difficulty with getting the necessary information was to
> actually find out where to get it from.

There's "a bit" of documentation work to be done here.

> - Which Distro has which Suites?

Suites are an Ubuntuism. It's an internal construct the Ubuntu plugin
uses. It should not be treated differently than any other setting. I
understand why you did it, but your having to do this is a symptom that
something is wrong.

Either, we need to lose the Suite notion (essentially make the different
versions of Ubuntu top-level distributions) or bless it and move it out
of the Ubuntu plugin and into perhaps the Distro class.

This is a separate discussion, though.

> - What are the maximums and minimums for rootsize, swapsize, number of
> cpus, ramsize

I don't believe we have any notion of any of these at the moment?

> Now although I got all the information from that list I always kind of
> had to hunt them down. Most of this stuff can be retrieved quite
> easily with digging through the source code a bit but you shouldn't
> need to dig through the code to get this simple pieces of information
> IMHO. And then you also access pieces of VMBuilder that could be
> considered as internal which also isn't a good thing.

We need to do a better job of making information available to frontends.
I agree completely. I'm happy that you have written a frontend and thus
have got a bit of experience we can use.

> So it would be much easier if there was just one object that you could
> ask all these things and which acted as an interface for frontends.

I'm not sure I understand this suggestion. You want the the core to call
into the frontend asking for stuff instead of the other way around?

> Actually, when confronted with this problem of getting information out
> of VMBuilder I just wrote my own interface that provides all the
> information I'm interested in (VMBuilder.vm.model). It's a model
> object that represents the buildable configuration of a VM and that
> can be asked for valid options (where some of them can change when
> another option is changed) and that can validate the values of its
> options and therefore check its consistency. If you finished
> configuring it you can just tell it to give you the vmbuilder
> command-line that builds this machine.

Do you think perhaps each call to set_setting should result in a call to
a "revalidate" sort of method for every setting?

>> Yes, I never considered hypervisor and distro "settings" like this.
>> They are core definitions of context, without which no settings
>> really make sense. It's not until both hypervisor and distro have
>> been set that the rest of the settings are even registered. This is
>> completely intentional. There are options that cannot be registered
>> until we know which distro or hypervisor is used. This is going to be
>> reworked somewhat going forward, but the basic idea is the same:
>> Registering something about the context.
> I think this is problematic for interactive frontends....

[lp:~soren/vmbuilder/0.12.settings]

360. By Soren Hansen on 2010-01-27

Add a notion of valid options to Settings. These can be passed as a
valid_options kwarg to the Setting's __init__.

An exception will be raised if the user attempts to set a Setting to an invalid
value.

A valid_options attribute value of None will allow any value to be set (if
it passes the type check).

[Andreas Heck (aheck)]

> > - What are the maximums and minimums for rootsize, swapsize, number of
> > cpus, ramsize
>
> I don't believe we have any notion of any of these at the moment?

They aren't that important for a non-interactive CLI program (although there
certainly are some limits even if not very relevant in practice i suppose)
but for a GUI frontend they are very important because there are no infinite sliders ;)

> > So it would be much easier if there was just one object that you could
> > ask all these things and which acted as an interface for frontends.
>
> I'm not sure I understand this suggestion. You want the the core to call
> into the frontend asking for stuff instead of the other way around?

Of course not! Just the other way around. I think there should be a single interface
that can be used by frontends and that has all the information a frontend could ever
need so that the frontend doesn't have to know anything about vmbuilders internals.

So this interface calls into vmbuilder on behalf of the framework.

> > Actually, when confronted with this problem of getting information out
> > of VMBuilder I just wrote my own interface that provides all the
> > information I'm interested in (VMBuilder.vm.model). It's a model
> > object that represents the buildable configuration of a VM and that
> > can be asked for valid options (where some of them can change when
> > another option is changed) and that can validate the values of its
> > options and therefore check its consistency. If you finished
> > configuring it you can just tell it to give you the vmbuilder
> > command-line that builds this machine.
>
> Do you think perhaps each call to set_setting should result in a call to
> a "revalidate" sort of method for every setting?

Yeah, something like that would be useful. That way you always know that
your configuration is consistent and all frontends could use the same
validation logic.

> I'm not sure how to fix this. Actually, I'm not sure it needs fixing at
> all. Plugin Foo may only apply if the distro is Ubuntu or whatnot. This
> seems perfectly reasonable to me. How this is presented in a GUI, I
> don't know. I've always imagined a wizard of some sort that would start
> by asking for distro and hypervisor to set the stage, and then go on to
> collect answers for all the other questions.

I also thought about using a wizard first but then I don't really like wizards
much because they impose the order the programmer thinks is best on the user
and stop him from exploring and playing around with options.

With a very technical program for technical users like vmbuilder I think this
is even more of a problem. I then decided to group all options on different tabs
and let the user press a "Build" button when he thinks he is done.

I known that there are some constraints due to the current initialization
process and I don't know enough about it to discuss this in detail but
please check first if there isn't there another which doesn't lead to
a situation where the API of vmbuilder directly puts constraints on the
way a frontend UI has to be modeled.

> >> Would all of your concerns be addressed by extending the Setting
> >> class with a .valid_op...

[Soren Hansen (soren)]

On Wed, Jan 27, 2010 at 09:25:08PM -0000, Andreas Heck wrote:
>>> - What are the maximums and minimums for rootsize, swapsize, number
>>> of cpus, ramsize
>> I don't believe we have any notion of any of these at the moment?
> They aren't that important for a non-interactive CLI program (although
> there certainly are some limits even if not very relevant in practice
> i suppose) but for a GUI frontend they are very important because
> there are no infinite sliders ;)

Ah, good point.

>>> So it would be much easier if there was just one object that you
>>> could ask all these things and which acted as an interface for
>>> frontends.
>> I'm not sure I understand this suggestion. You want the the core to
>> call into the frontend asking for stuff instead of the other way
>> around?
> Of course not! Just the other way around. I think there should be a
> single interface that can be used by frontends and that has all the
> information a frontend could ever need so that the frontend doesn't
> have to know anything about vmbuilders internals.
>
> So this interface calls into vmbuilder on behalf of the framework.

I agree, and I think this patch is a reasonable step in the right
direction?

>>> Actually, when confronted with this problem of getting information
>>> out of VMBuilder I just wrote my own interface that provides all the
>>> information I'm interested in (VMBuilder.vm.model). It's a model
>>> object that represents the buildable configuration of a VM and that
>>> can be asked for valid options (where some of them can change when
>>> another option is changed) and that can validate the values of its
>>> options and therefore check its consistency. If you finished
>>> configuring it you can just tell it to give you the vmbuilder
>>> command-line that builds this machine.
>> Do you think perhaps each call to set_setting should result in a call
>> to a "revalidate" sort of method for every setting?
> Yeah, something like that would be useful. That way you always know
> that your configuration is consistent and all frontends could use the
> same validation logic.

Ok.

>> I'm not sure how to fix this. Actually, I'm not sure it needs fixing
>> at all. Plugin Foo may only apply if the distro is Ubuntu or whatnot.
>> This seems perfectly reasonable to me. How this is presented in a
>> GUI, I don't know. I've always imagined a wizard of some sort that
>> would start by asking for distro and hypervisor to set the stage, and
>> then go on to collect answers for all the other questions.
> I also thought about using a wizard first but then I don't really like
> wizards much because they impose the order the programmer thinks is
> best on the user and stop him from exploring and playing around with
> options.

Yes.. I think that's why I liked it :)

> With a very technical program for technical users like vmbuilder I
> think this is even more of a problem. I then decided to group all
> options on different tabs and let the user press a "Build" button when
> he thinks he is done.
>
> I known that there are some constraints due to the current initialization
> process and I don't know enough about it to discuss this in detail but
> please check first i...

[Soren Hansen (soren)]

On Wed, Jan 27, 2010 at 09:25:08PM -0000, Andreas Heck wrote:
>> Do you think perhaps each call to set_setting should result in a call
>> to a "revalidate" sort of method for every setting?
> Yeah, something like that would be useful. That way you always know
> that your configuration is consistent and all frontends could use the
> same validation logic.

Can you give me an example of a situation where this would be useful, so
that I have something to model this around?

I don't see any reason to /not/ apply this patch in its current form
because we don't yet have this "revalidate" functionality. Would you
agree? I have a stack of other stuff that's blocking in this patch, and
I'd like to get moving on it.

[Andreas Heck (aheck)]

> On Wed, Jan 27, 2010 at 09:25:08PM -0000, Andreas Heck wrote:
> >> Do you think perhaps each call to set_setting should result in a call
> >> to a "revalidate" sort of method for every setting?
> > Yeah, something like that would be useful. That way you always know
> > that your configuration is consistent and all frontends could use the
> > same validation logic.
>
> Can you give me an example of a situation where this would be useful, so
> that I have something to model this around?

I'm not sure if I can give you a very good example at the moment since I
somewhat lost track of this discussion.

But it would apply for every situation where you must look at more than one
Setting to find out if your configuration is sane . A possible example would
be the network address example I already mentioned. I'm not sure if there are
that much more other examples of this at the moment, though. Nevertheless a
GUI needs to know about such problems and ideally it should know it as early
as possible and not just when the users says "Build this configuration".

I don't know if this collides with the preflight hooks you want to use for that
but maybe they could be wrapped by this validation code so that it becomes
transparent for the client code that puts a configuration together. Just an
unchecked idea.

> I don't see any reason to /not/ apply this patch in its current form
> because we don't yet have this "revalidate" functionality. Would you
> agree? I have a stack of other stuff that's blocking in this patch, and
> I'd like to get moving on it.

I don't want to stop you from proceeding with new patches. Actually I'm
hoping for new stuff since more than a week already :)

review: Approve

=== modified file 'VMBuilder/plugins/__init__.py'
--- VMBuilder/plugins/__init__.py	2010-01-22 13:27:16 +0000
+++ VMBuilder/plugins/__init__.py	2010-01-27 13:42:12 +0000
@@ -17,8 +17,11 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 import os
+import re
+
 import VMBuilder
 from VMBuilder.util import run_cmd
+from VMBuilder.exception import VMBuilderException
 
 def load_plugins():
     for plugin in find_plugins():
@@ -38,6 +41,7 @@
 
     def __init__(self, vm):
         self.vm = vm
+        self._setting_groups = []
         self.register_options()
 
     def register_options(self):
@@ -77,3 +81,186 @@
         if not self.vm.fsmounted:
             raise VMBuilderException('install_from_template called while file system is not mounted')
         return run_cmd('chroot', self.vm.installdir, *args, **kwargs)
+
+    # Settings
+    class SettingGroup(object):
+        def __init__(self, plugin, vm, name):
+            # The plugin that owns this setting
+            self.plugin = plugin
+            # The VM object
+            self.vm = vm
+            # Name of the Setting Group
+            self.name = name
+            # A list of Setting objects
+            self._settings = []
+
+        def add_setting(self, *args, **kwargs):
+            # kwarg['type'] is used to determine which type of Setting object to create
+            # but we don't want to pass it on to its __init__.
+            if 'type' in kwargs:
+                type = kwargs['type']
+                del kwargs['type']
+            else:
+                type = 'str'
+
+            if type == 'str':
+                setting = self.plugin.StringSetting(self, *args, **kwargs)
+            elif type == 'bool':
+                setting = self.plugin.BooleanSetting(self, *args, **kwargs)
+            elif type == 'list':
+                setting = self.plugin.ListSetting(self, *args, **kwargs)
+            elif type == 'int':
+                setting = self.plugin.IntSetting(self, *args, **kwargs)
+            else:
+                raise VMBuilderException("Unknown setting type: '%s' (Plugin: '%s', Setting group: '%s', Setting: '%s')" % 
+                                            (type,
+                                             self.plugin.__module__,
+                                             self.name,
+                                             args[0]))
+            self._settings.append(setting)
+
+    class Setting(object):
+        default = None
+
+        def __init__(self, setting_group, name, metavar=None, help=None, extra_args=None, valid_options=None, action=None, **kwargs):
+            # The Setting Group object that owns this Setting
+            self.setting_group = setting_group
+            # The name if the setting
+            name_regex = '[a-z0-9-]+$'
+            if not re.match(name_regex, name):
+                raise VMBuilderException('Invalid name for Setting: %s. Must match regex: %s' % (name, name_regex))
+            else:
+                self.name = name
+
+            self.default = kwargs.get('default', self.default)
+            self.help = help
+            # Alternate names (for the CLI)
+            self.extra_args = extra_args or []
+            self.value = None
+            self.value_set = False
+            self.valid_options = valid_options
+
+            if self.name in self.setting_group.vm._config:
+                raise VMBuilderException("Setting named %s already exists. Previous definition in %s/%s/%s." % 
+                                            (self.name,
+                                             self.setting_group.plugin.__name__,
+                                             self.setting_group.plugin._config[self.name].setting_group.name,
+                                             self.setting_group.plugin._config[self.name].name))
+
+            self.setting_group.vm._config[self.name] = self
+
+        def get_value(self):
+            """
+            If a value has previously been set, return it.
+            If not, return the default value.
+            """
+
+            if self.value_set:
+                return self.value
+            else:
+                return self.default
+
+        def do_check_value(self, value):
+            """
+            Checks the value's validity.
+            """
+            if self.valid_options is not None:
+                if value not in self.valid_options:
+                    raise VMBuilderException('%r is not a valid option for %s. Valid options are: %s' % (value, self.name, ' '.join(self.valid_options)))
+            else:
+                self.check_value(value)
+
+        def get_valid_options(self):
+            return self.valid_options
+
+        def set_valid_options(self, valid_options):
+            """
+            Set the list of valid options for this setting.
+            """
+            if not type(valid_options) == list and valid_options is not None:
+                raise VMBuilderException('set_valid_options only accepts lists or None')
+            if valid_options:
+                for option in valid_options:
+                    self.check_value(option)
+            self.valid_options = valid_options
+
+        def get_default(self):
+            """
+            Return the default value.
+            """
+            return self.default
+
+        def set_default(self, value):
+            """
+            Set a new default value.
+            """
+            self.do_check_value(value)
+            self.default = value
+
+        def set_value(self, value):
+            """
+            Set a new value.
+            """
+            self.do_check_value(value)
+            self.value = value
+            self.value_set = True
+
+    class ListSetting(Setting):
+        def __init__(self, *args, **kwargs):
+            self.default = []
+            super(Plugin.ListSetting, self).__init__(*args, **kwargs)
+
+        def check_value(self, value):
+            if not type(value) == list:
+                raise VMBuilderException('%r is type %s, expected list.' % (value, type(value)))
+
+    class IntSetting(Setting):
+        def check_value(self, value):
+            if not type(value) == int:
+                raise VMBuilderException('%r is type %s, expected int.' % (value, type(value)))
+
+    class BooleanSetting(Setting):
+        def check_value(self, value):
+            if not type(value) == bool:
+                raise VMBuilderException('%r is type %s, expected bool.' % (value, type(value)))
+
+    class StringSetting(Setting):
+        def check_value(self, value):
+            if not type(value) == str:
+                raise VMBuilderException('%r is type %s, expected str.' % (value, type(value)))
+
+    def setting_group(self, name):
+        setting_group = self.SettingGroup(self, self.vm, name)
+        self._setting_groups.append(setting_group)
+        return setting_group
+
+    def get_setting(self, name):
+        if not name in self._config:
+            raise VMBuilderException('Unknown config key: %s' % name)
+        return self._config[name].get_value()
+
+    def set_setting(self, name, value):
+        if not name in self.vm._config:
+            raise VMBuilderException('Unknown config key: %s' % name)
+        self._config[name].set_value(value)
+
+    def set_setting_default(self, name, value):
+        if not name in self.vm._config:
+            raise VMBuilderException('Unknown config key: %s' % name)
+        self._config[name].set_default(value)
+
+    def get_setting_default(self, name):
+        if not name in self.vm._config:
+            raise VMBuilderException('Unknown config key: %s' % name)
+        return self._config[name].get_default()
+
+    def get_setting_valid_options(self, name):
+        if not name in self._config:
+            raise VMBuilderException('Unknown config key: %s' % name)
+        return self._config[name].get_valid_options()
+
+    def set_setting_valid_options(self, name, valid_options):
+        if not name in self._config:
+            raise VMBuilderException('Unknown config key: %s' % name)
+        self._config[name].set_valid_options(valid_options)
+

=== added file 'test/plugin_tests.py'
--- test/plugin_tests.py	1970-01-01 00:00:00 +0000
+++ test/plugin_tests.py	2010-01-27 13:42:12 +0000
@@ -0,0 +1,99 @@
+import unittest
+
+import VMBuilder.plugins
+from   VMBuilder.exception import VMBuilderException
+
+class TestPluginsSettings(unittest.TestCase):
+    class VM(VMBuilder.plugins.Plugin):
+        def __init__(self, *args, **kwargs):
+            self._config = {}
+            self.vm = self
+
+    class TestPlugin(VMBuilder.plugins.Plugin):
+        pass
+
+    def setUp(self):
+        self.vm = self.VM()
+        self.plugin = self.TestPlugin(self.vm)
+
+    def test_add_setting_group_and_setting(self):
+        setting_group = self.plugin.setting_group('Test Setting Group')
+        self.assertTrue(setting_group in self.plugin._setting_groups, "Setting not added correctly to plugin's registry of setting groups.")
+
+        setting_group.add_setting('testsetting')
+        self.assertEqual(self.vm.get_setting('testsetting'), None, "Setting's default value is not None.")
+
+        self.vm.set_setting_default('testsetting', 'newdefault')
+        self.assertEqual(self.vm.get_setting('testsetting'), 'newdefault', "Setting does not return custom default value when no value is set.")
+        self.assertEqual(self.vm.get_setting_default('testsetting'), 'newdefault', "Setting does not return custom default value through get_setting_default().")
+
+        self.vm.set_setting('testsetting', 'foo')
+        self.assertEqual(self.vm.get_setting('testsetting'), 'foo', "Setting does not return set value.")
+
+        self.vm.set_setting_default('testsetting', 'newerdefault')
+        self.assertEqual(self.vm.get_setting('testsetting'), 'foo', "Setting does not return set value after setting new default value.")
+
+    def test_invalid_type_raises_exception(self):
+        setting_group = self.plugin.setting_group('Test Setting Group')
+        self.assertRaises(VMBuilderException, setting_group.add_setting, 'oddsetting', type='odd')
+
+    def test_valid_options(self):
+        setting_group = self.plugin.setting_group('Test Setting Group')
+
+        setting_group.add_setting('strsetting')
+        self.assertRaises(VMBuilderException, self.vm.set_setting_valid_options, 'strsetting', '')
+        self.vm.set_setting_valid_options('strsetting', ['foo', 'bar'])
+        self.assertEqual(self.vm.get_setting_valid_options('strsetting'), ['foo', 'bar'])
+        self.vm.set_setting('strsetting', 'foo')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'strsetting', 'baz')
+        self.vm.set_setting_valid_options('strsetting', None)
+        self.vm.set_setting('strsetting', 'baz')
+        
+    def test_invalid_type_setting_raises_exception(self):
+        setting_group = self.plugin.setting_group('Test Setting Group')
+
+        setting_group.add_setting('strsetting')
+        self.vm.set_setting('strsetting', '')
+        self.vm.set_setting_default('strsetting', '')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'strsetting', 0)
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'strsetting', 0)
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'strsetting', True)
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'strsetting', True)
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'strsetting', [])
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'strsetting', [])
+
+        setting_group.add_setting('intsetting', type='int')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'intsetting', '')
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'intsetting', '')
+        self.vm.set_setting('intsetting', 0)
+        self.vm.set_setting_default('intsetting', 0)
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'intsetting', True)
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'intsetting', True)
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'intsetting', [])
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'intsetting', [])
+
+        setting_group.add_setting('boolsetting', type='bool')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'boolsetting', '')
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'boolsetting', '')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'boolsetting', 0)
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'boolsetting', 0)
+        self.vm.set_setting('boolsetting', True)
+        self.vm.set_setting_default('boolsetting', True)
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'boolsetting', [])
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'boolsetting', [])
+
+        setting_group.add_setting('listsetting', type='list')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'listsetting', '')
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'listsetting', '')
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'listsetting', 0)
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'listsetting', 0)
+        self.assertRaises(VMBuilderException, self.vm.set_setting, 'listsetting', True)
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'listsetting', True)
+        self.vm.set_setting('listsetting', [])
+        self.vm.set_setting_default('listsetting', [])
+
+    def test_set_setting_raises_exception_on_invalid_setting(self):
+        self.assertRaises(VMBuilderException, self.vm.set_setting_default, 'testsetting', 'newdefault')
+
+    def test_add_setting(self):
+        setting_group = self.plugin.setting_group('Test Setting Group')