[Pacemaker] Resource agents: parameter type enforcement and normalization

[Florian Haas]

Lars,

On 07/14/2009 05:15 PM, Lars Marowsky-Bree wrote:
>> Now considering the CIB already does, for example, IDREF validation, I
>> really wonder why it shouldn't be doing content validation as well.
> 
> It could, of course.
> 
> But the problem would remain - the CLI/GUI would need to do their own
> validation and parsing, to be able to guide the user's input and provide
> them with useful error messages (and not just "Validation faild", and it
> is _hard_ to get useful errors out of the schema/DTD validators!).

Point taken.

> The RA would need to validate its input values anyway - it is good
> practice, they might be called from older code bases, manually, via
> ocf-tester, and simply because they shouldn't crash or cause data loss
> when fed "bogus" values.

That is true, and I am not contesting that they need to validate. I was
actually asking for, what shall we call it, instance attribute value
normalization.

>> Thereby one could make sure that, for example, something that is
>> expected to be passed in as integer, is in fact an integer, and we could
>> stop doing stupid tests in the RAs such as this one:
>>
>> # Do we have a valid target ID?
>> [ $OCF_RESKEY_tid -ge 1 ]
>> case $? in
>>     0)
>>         # OK
>>         ;;
>>     1)
>>         ocf_log err "Invalid target ID $OCF_RESKEY_tid (must be greater
>> than 0)."
>>         return $OCF_ERR_CONFIGURED
>>         ;;
>>     *)
>>         # this is the utterly braindead part
>>         ocf_log err "Invalid target ID $OCF_RESKEY_tid (must be an integer)\
>> ."
>>         return $OCF_ERR_CONFIGURED
>> esac
> 
> What would make sense would be to have ocf_test_* wrapper functions
> though to abstract these. Surprisingly, "ocf_is_decimal" already exists.
> ;-)

I love learning new things. Thanks for educating me! Although at first
sight it seems that ocf_is_decimal is actually a misnomer, as the
expression it checks for is actually for integers (it doesn't include
any decimal separator).

>> Likewise, we could finally have something like normalized booleans.
>> Because I find this sort of tests even more braindead than the above
>> mentioned one:
>>
>> case $OCF_RESKEY_force_stop in
>>     [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]|[Oo][Nn]|1)
>>         op="destroy"
>>         ;;
>>     *)
>>         op="shutdown"
>> 	;;
>> esac
> 
> "ocf_is_true"

Funny. If you had used that one in your own drbd OCF RA, I would have
caught that sooner. :)

>> And finally, I really want enum support for resource agents. And, this
>> I've not made myself clear about yesterday, my question is _not_ how to
>> implement this in the shell or GUI, my question is how to express the
>> following in the RA metadata XML: this parameter is named "foobar", it's
>> expected to be passed in as a string, and the allowed values are
>> "alpha", "bravo", and "charlie".
>>
>> At this time appears to be supported for cluster properties like
>> no-quorum-policy (freeze|stop|ignore|suicide), but not for resource
>> agent parameters. And if it's not even expressible in the metadata, I
>> don't see how the GUI or the shell should know about it.
> 
> Yes, the meta-data needs more extensive functionality; at the risk of
> being shot, I've contemplated borrowing the HTML "FORM" syntax.

Why borrow that? IIUC "enum" is the supported syntax for cluster
properties already, why introduce something new?

> And the CLI/GUI need to get better at handling them and supporting users
> entering data. But I personally don't think much of enforcing this
> validation in the CIB - it focuses on syntactic validation, not
> semantics. Otherwise, someone will next complain that it doesn't offer
> the full abilities of database constraints and triggers, and I really
> really don't want to go there ;-)

Again, point taken.

Cheers,
Florian

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <https://lists.clusterlabs.org/pipermail/pacemaker/attachments/20090714/788581ee/attachment-0003.sig>