I'll try and sum up while rebutting as I get confused when mail threads become to
long.
Your points as I understand them:
1. Moving the configuration to the instance level instead of system level is harder to
implement.
[A] You are right when you talk about existing codebase, the question is will future
configuration be written directly as instance configuration or will we continue abusing
the system configuration until it's integrated in.
2. Using the config for system level defaults.
[A] Defaults are values that we as developers recommend for options. Letting the user
change the defaults is actually letting the user set the value. If you are talking about
configuration having a configuration hierarchy. Engine is not a parent objects to pin the
hierarchy to as it's not an actual actor in the BL.
3. No need to set until there is an instance level tweak is required.
[A] If you set up the configuration hierarchy properly you could have the attribute
getters recognize this hierarchy and only have a setter (and the proper db scheme) written
when instance level tweaking is required. This way you remove the complexity but keep
object relation correctness.
My opinion is usually that doing things properly might look like a lot of work now but in
the long run you save more time. Further more, you can hide the fact that you are doing
things wrong when you lack the time to actually do things properly. VDSM for instance
hides global configuration accesses in instance getters and constructors. So even though
the configuration is in the wrong place it is modeled in correctly and as we slowly move
things the rest of the code isn't (for the most part) doing things like accessing
objects configuration directly and other such atrocities.
In more practical terms, you could have settings per class\service instead of per instance
and later you can actually use this as the basis for a configuration hierarchy. IMO This
will be easier to model into the current authentication model, and integrate in. Always
remember that one of the pillars of OOP is encapsulation, so even if you decide to keep
all the values in one table, it will be smart to still define who owns what values in code
instead of having a ConfigurationManager singleton and have everyone access it directly.
But again, I'm not as versed in the Engine code as you all are and this might be more
complicated than it looks to an outsider.
In broader strokes, my personal preference is to have an idea of how you want the
configuration to work *ideally* and then start to take concessions due to resource
limitations. Instead of just deciding to hack something in and worry about things later.
----- Original Message -----
From: "Itamar Heim" <iheim(a)redhat.com>
To: "Saggi Mizrahi" <smizrahi(a)redhat.com>
Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, engine-devel(a)ovirt.org
Sent: Sunday, April 1, 2012 7:12:51 PM
Subject: Re: [Engine-devel] Reloadable Configuration - Wiki Page
On 03/31/2012 12:50 AM, Saggi Mizrahi wrote:
> I'm sorry if I'm moving the conversation sideways a bit but I was
> ill for a few days and missed out on all the fun :).
> The whole debate about permissions is a causing my spidy sense to
> tingle.
>
> My main question is, What are the options you actually want to make
> as engine configuration?
> At least from my experience with VDSM 99% of the configuration
> options we have shouldn't be VDSM configuration but instance
> attributes.
I don't think we should start by making every config variable at
instance level.
the config table allows system level defaults, which are easy to
handle/manage.
where there is sufficient need to create an instance level tweak, it
is
usually done with due consideration to making it a feature, which is
usually a lengthier process.
>
> To illustrate, the interval where you check liveness of hosts is
> not actually an Engine setting but a cluster or a host attribute.
> It should be set using the UI and the permissions to set it should
> be related to general permission on the object.
> The interval in which to check for storage liveness is a data
> center setting, not an Engine setting as well.
true, but apparently so far there wasn't a need to tweak it at DC or
cluster level, and engine level was good enough.
best is sometimes enemy of good.
and adding a lot of configurable options at entity level doesn't
necessarily make things simpler for admins to understand.
so i agree some options should be at instance level, but only when
there
is a good enough reason to do so.
>
> In general most options in a system are tied to object instances.
> The only things that I can think of that are actual Engine
> configurations are DB location, memory settings of the JVM and
> other technical details that should be accessible and set by the
> local root user only. This is also why I thought it shouldn't be
> in the DB, because if it's in the DB it's actually shared between
> installation and it's like sharing apache's "max thread num"
> setting between hosts with different capabilities.
>
> Technical limitations like not being able to update quartz tasks
> once they've been initiated should be circumvented or abstracted
> but not designed around.
>
> ----- Original Message -----
>> From: "Doron Fediuck"<dfediuck(a)redhat.com>
>> To: "Itamar Heim"<iheim(a)redhat.com>
>> Cc: engine-devel(a)ovirt.org
>> Sent: Thursday, March 29, 2012 8:42:05 AM
>> Subject: Re: [Engine-devel] Reloadable Configuration - Wiki Page
>>
>> On 29/03/12 12:13, Itamar Heim wrote:
>>> On 03/29/2012 11:59 AM, Doron Fediuck wrote:
>>>> On 29/03/12 10:54, Itamar Heim wrote:
>>>>> On 03/29/2012 10:05 AM, Muli Salem wrote:
>>>>>> Thanks for the comments, I updated the wiki page accordingly:
>>>>>>
http://www.ovirt.org/wiki/Features/ReloadableConfiguration
>>>>>>
>>>>>> 1. Instead of the new DB column is_reloadable -->
>>>>>> Annotation
>>>>>> to ConfigValues.
>>>>>> 2. Found a way to update the Quartz jobs, at least basic
>>>>>> issues
>>>>>> such as interval size.
>>>>>> 3. The values will be reloaded upon admin's decision to do
so
>>>>>> -
>>>>>> with a new command in the engine-config CLI, since that is
>>>>>> where admins make the changes.
>>>>>
>>>>> just wondering - how will the CLI do this at the technical
>>>>> level
>>>>> (via REST API? signal to service, etc.)?
>>>>
>>>> Basically we need a script using the REST sdk to trigger
>>>> re-configuration
>>>> This script will need the engine's IP so it'll know where to
>>>> find
>>>> it.
>>>> The thing is, REST also needs the admin's user and password to
>>>> run...
>>>>
>>>> We can get it in 2 options:
>>>>
>>>> 1. Store Admin's user+pass in the engine's conf file.
>>>> 2. Use engine-config to fetch the credentials.
>>>>
>>>> Once we have credentials, we can use it with a new script to
>>>> trigger configuration reload.
>>>> We can also incorporate this script into engine-config so admin
>>>> won't need to know another
>>>> script, and simply use a 'reload' verb.
>>>>
>>>> I'm not keen on storing the credentials in a conf' file, but
>>>> (unfortunately) it wouldn't
>>>> be a first time. Any better alternative is welcome (just as
>>>> patches ;-).
>>>
>>> true.
>>>
>>>>
>>>> A simple alternative to the whole credentials and IP need,
>>>> is a simple periodic reload, as suggested initially.
>>>
>>>
>> Any thoughts on this?
>>
>>>
>>>>
>>>> /d
>>>
>>> isn't there some way to send a process signal or something like
>>> that (not allowing remote access, but i think it uses the db
>>> crednetials from a local file anyway, and i don't think running
>>> config remotely is a must)
>>>
>> Simply signaling a process such as "kill TRAP PID" is problematic
>> since:
>> - The engine core is a web-app on top of JBoss, how do we know
>> which
>> pid to trigger?
>> - What happens if JBoss isn't running / using nohup / ... ?
>>
>>> other options:
>>> 1. require user to provide user/password (kind of funny for
>>> running
>>> manage-domains utility, but possible
>> This will make service xxx reload pointless, unless we decide to
>> drop
>> it and reduce to a simple
>> script (embedded or not in the config utility).
>>
>>> 2. use a way on the host to send a signal (change a file, process
>>> signal, etc.)
>> Great. This means we need to poll a folder or similar (DB)...
>> sounds
>> familiar?
>> See above the periodic reload. We can do it better if we look for
>> a
>> key in the DB
>> (enableReconfigure=true), to avoid reloading in the middle of an
>> update process
>> of several keys. We still need to be careful from concurrency, but
>> I
>> have solutions
>> for this as well (do not allow updates to vdc_options until the
>> key
>> becomes false again,
>> also set it to false on initial boot).
>>
>>>
>> 3. Write a simple public reload servlet (maybe limit to local host
>> only?), which
>> we can call using simple curl
http://xxx/reload?
>>
>> As I see it now,
>> I'd go for one of the following (in this order)-
>> 2: periodically poll DB, reload only when allowed.
>> 3: local servlet.
>>
>> Thoughts?
>> Better solutions?
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>>