On Wed, Mar 25, 2015 at 4:20 PM, Deepak Shetty <dpkshe...@gmail.com> wrote:
> > > On Wed, Mar 25, 2015 at 3:58 PM, Sean Dague <s...@dague.net> wrote: > >> On 03/25/2015 03:16 AM, Deepak Shetty wrote: >> > >> > >> > On Wed, Mar 25, 2015 at 11:29 AM, Deepak Shetty <dpkshe...@gmail.com >> > <mailto:dpkshe...@gmail.com>> wrote: >> > >> > >> > >> > On Wed, Mar 25, 2015 at 12:58 AM, Ian Wienand <iwien...@redhat.com >> > <mailto:iwien...@redhat.com>> wrote: >> > >> > On 03/24/2015 03:17 PM, Deepak Shetty wrote: >> > > For eg: Look at [1] >> > > [1] >> https://github.com/stackforge/devstack-plugin-glusterfs/blob/master/devstack/settings >> > >> > > I would like ability to change these while I use the >> enable_plugin >> > > apporach to setup devstack w/ GlusterFS per my local >> glusterfs setup >> > >> > So I think the plugin should do >> > >> > >> >> CINDER_ENABLED_BACKENDS=${CINDER_ENABLED_BACKENDS:-glusterfs:glusterfs,lvm:lvm1} >> > >> > i.e. provide a default only if the variable is unset. >> > >> > >> > Bah! That was easy, i should have figured that myself :) >> > Thanks for catching that >> > >> > >> > >> > This seems like one of those "traps for new players" and is one >> > concern I have with devstack plugins -- that authors keep >> having to >> > find out lessons learned independently. I have added a note on >> this >> > to the documentation in [1]. >> > >> > -i >> > >> > [1] https://review.openstack.org/#/c/167375/ >> > >> > >> > Great, i +1'ed it. >> > >> > Also i posted patch to fix settings file @ >> > https://review.openstack.org/167494 >> > >> > >> > Ian, >> > Looks like usign bash default in settings file of plugin is not >> > working, in my patch it didn't use glusterfs driver, it used LVM >> (default) >> > I think whats happening here is that by the time settings file is >> > sourced, CINDER_ENABLED_BACKENDS is already set to lvm by lib/cinder >> > so settings file's default value is never taken >> > >> > IIUC there are 3 scenarios (taking CINDER_ENABLED_BACKENDS as example >> var) : >> > >> > 1) localrc doesn't have CINDER_ENABLED_BACKENDS and enable_plugin >> > - Here we want the lib/cinder's default value to be taken >> > - this should work fine >> > >> > 2) localrc doesn't have CINDER_ENABLED_BACKENDS but has enable_plugin >> > glusterfs >> > - Here we want the plugin's default values to be taken, but its not >> > as lib/cinder already initialized CINDER_ENABLED_BACKENDS to use lvm >> backend >> > - Thus broken >> > >> > 3) localrc has both CINDER_ENABLED_BACKENDS and enable_plugin glusterfs >> > specified >> > - Here we want CINDER_ENABLED_BACKENDS present in my localrc to be >> > chosen >> > - This will work as by the time settings file is sourced >> > CINDER_ENABLED_BACKENDS is already initialised to my value in localrc >> > >> > So #2 scenario would need some changes in stack.sh handling of plugin >> code ? >> >> Right, so this code runs late enough that you don't get to change the >> defaults. I think that's ok. >> >> I would instead do the following: >> >> 1) CINDER_ENABLED_BACKENDS+=,glusterfs:glusterfs >> >> or >> >> 2) CINDER_ENABLED_BACKENDS=glusterfs:glusterfs >> >> in the plugin. >> >> Clearly, if you've enabled the plugin, you want glusterfs. I think that >> in most cases you probably only want glusterfs as your backend, so >> option #2 seems sensible. >> > > > #1 is needed for multi-backend testing > #2 is needed for single-backend testing > > #2 is what we currently, we blindly override the var, but that forces the > devstack user to > use the config given in the plugin, I wanted a way to either use plugin > config or override it > > I think #1 is better, since it gives the power in localrc to do: > > 1) CINDER_ENABLED_BACKENDS= > This will ensure lib/cinder doesn't populate it and plugin adds > glusterfs:glusterfs for single backend > My bad here, lib/cinder uses :- which IIUC means empty or unset, use default so with #1 or #2 there isn't a way to provide ability to use plugin config or override it , both ? back to square one. thanx, deepak > > 2) No mention of CINDER_ENABLED_BACKENDS in localrc > This will make it CINDER_ENABLED_BACKENDS=lvm:lvm-driver1, > glusterfs:glusterfs for multi-backend > > Also for vars in settings file that are backend specific (hence not > touched by lib/cinder): > > GLUSTERFS_LOOPBACK_DISK_SIZE & CINDER_GLUSTERFS_SHARES > > They can remain as : > GLUSTERFS_LOOPBACK_DISK_SIZE=${GLUSTERFS_LOOPBACK_DISK_SIZE:-8G} > > CINDER_GLUSTERFS_SHARES=${CINDER_GLUSTERFS_SHARES:-"127.0.0.1: > /vol1;127.0.0.1:/vol2"} > > (as mentioned in the patch @ > https://review.openstack.org/#/c/167494/1/devstack/settings) > > This will give the end user the ability to change loopback size and/or > gluster server IPs > based on the needs of his/her local setup > > Agree ? > > If yes, then we must mention this in the plugin.rst in a nice way for > other plugin writers to > understand properly :) ? > > thanx, > deepak > > >> >> >> -Sean >> >> -- >> Sean Dague >> http://dague.net >> >> >> __________________________________________________________________________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: >> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
