Open Stack

No problem Dan. I appreciate you guys spending time advising on this, as there are certainly more high priority tasks.  It's a learning experience and I'm starting to understand better how the startup works, by digging into this.


One proposal would be to remove default values for some configuration items (need to determine which ones) and add validation logic at the plugin __init__(), where some plug-ins already have some checking of config, exiting if the value is not set. The obvious implication here is that we have to ensure that all plugins have the needed validation logic, especially for "common" configuration options.

Would that be adequate, or should we take this a level further, and also mark those configuration options as "required"?

Marking an option as "required" would have the advantage that the existing config parsing will automatically detect which ones are not set. The plugin could (optionally) do additional validation on whether the value was "sane" or not. My concern with this is ensuring that the selected config option(s) are indeed "required" by all the plugins that are importing the config definition. Is there possibly a case where a config option definition, which is not required by the plugin, could be imported along with other options in the same file?

I was thinking initially, that it would be nice to add a "validation" function to the plugins. However, I see a lot of variation in the plugin __init__() functions, where the ordering of config option checking varies (some delegated to functions called, some at start, some spread throughout function), and the action varies (sys.exit() vs raise exceptions). Do we want to tackle reworking the validation logic in __init__() for all the plugins, or leave them as is, and just augment with additional tests for configuration items that we modify by removing their default value (and optionally marking as required)?

Which additional configuration options we want to include in this proposal? Plugins already have checking logic in their __init__() function, and they could certainly mark plugin specific config options as required.

Would it be enough to just check 'sql_connection'? Any other "shared" config options? For 'sql_connection' a check can be added to db.api.db_configure(). We could either just sys.exit() with a log message, or we could change the signature and return T/F and then modify the callers to log and take action (exit or exception). The latter is more work and adds duplicate code for logging, but gives some flexibility on how the failure is handled, as plugins do handle other config issues differently.  Preferences?

Sorry for the large number of questions - I'm trying to confirm my understanding,  and "fit" the change into the architecture consistent with what we're doing.

Thanks!

PCM (Paul Michali)


On Feb 4, 2013, at 9:46 PM, Dan Wendlandt wrote:

> 
> 
> On Mon, Feb 4, 2013 at 8:18 AM, Paul Michali <pcm at cisco.com> wrote:
> Anyone have thoughts on my questions, below?
> 
> Thanks
> 
> PCM (Paul Michali)
> 
> Hi Paul,
> 
> Well, I first want to apologize for leading you astray on this.  I tagged this bug as a low-hanging-fruit appropriate for someone's first commit, but it certainly ended up having a lot of hidden complexity.  You've done a great job working through all the comments.  Some more inline.
> 
> Dan
> 
>  
> 
> 
> On Feb 1, 2013, at 1:51 PM, Paul Michali wrote:
> 
>> Seeking some suggestions/comments/guidance on this…
>> 
>> Issue was that user started quantum server with the core config file specified, but not any plugin config file. As a result, it used sqlite, though the setup was using mysql.  The goal is to try to prevent this issue.  From some discussion already, it appears that the approach should be to "validate" the plugin config options, rather than checking for any file, as a user could aggregate config settings into one file, with both core and plugin config settings.
>> 
>> I've got a few questions for plugin agent developers (this is my first bug, so I'm trying to get up to speed).
>> 
>> Would we be able to identify a set of plugin configuration options that all plugins must contain, or will they vary per plugin?
> 
> 
> If we go this direction, I would suspect this would vary by plugin.  For example, a plugin that uses an external controller would require the IP of that controller, for which no reasonable default value is possible.  
>  
>> If there is a single set of config options that can be considered "mandatory" would we be able to change their definition and mark them as "required" so that exclusion of them is automatically detected by cfg, or will that cause issues (it seems like the cfg is shared by the server and  plugin agents)?
>> If the "required" configuration options vary per plugin, is it possible to push the validation of config down to the plugin (agent)?
> 
> This was my original thinking, which is that a plugin could pass a list of config values that must be non-empty.  However, it also seems likely that a plugin may well want to do more validation than just that the value is non-empty.  So then the question is whether there's a point to doing the validation at the config layer at all given that a plugin SHOULD do its own more specific validation.  The two possible reasons I can see are: 
> 
> 1) doing the check at the config layer may provide an error message that more usefully points to the actual cause (e.g., config value YYY not specified by any of the provided config files).  Rather than a tricker message like unable to connect to controller ''  (where '' is the empty string). 
> 2) this may be lighter weight for plugin developers. 
>  
> This discussion has increased scope beyond the original bug, but its actually interesting to note that the original bug would not have been prevented by the mechanism I suggest, as plugins currently have their sql_connection field default to "sqlite://".  Having this default built into the code was a mistake in my opinion.  It makes sense for developers, but not for users.   If nothing else, I think we should consider defaulting that field to empty string, at which point, at least the user would have gotten an error about not being able to connect to the database.  
> 
>> If we do push down the validation, though, is there a way that this can be checked at the quantum server? IOW does the server invoke the agent(s) validator function?
> 
> Presumably the agents should fail to start if not provided the proper config.  I think this would manifest itself in yong's new agent API as that agent being down.  
>  
>> Is this bug even worth pursuing, or should the proper configuration inclusion be something handled by provisioning tools? IOW rely on tooling to make sure the right files are included and "expect" that the user knows what they are doing, if they do this manually? Guess I need to ask that "is it worth fixing this" question :)
> 
> 
> Given that quantum-server can always be executed directly on the CLI with no config files at all, I think this argues that we should at least make sure it fails if any value absolutely required by the plugin is not set.
> 
> Dan
> 
> 
>  
>> 
>> Thanks in advance for any guidance!
>> 
>> 
>> PCM (Paul Michali)
>> 
> 
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
> 
> -- 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Dan Wendlandt 
> Nicira, Inc: www.nicira.com
> twitter: danwendlandt
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130205/6892339e/attachment.html>