[Pacemaker] Resource agents: parameter type enforcement and normalization

[Lars Marowsky-Bree]

On 2009-07-03T08:55:21, Florian Haas <florian.haas at linbit.com> wrote:

> do nothing to actually enforce parameter types in the CIB. Instead, that
> metadata is intended simply as a hint for the GUI or the CRM shell, and
> parameter type enforcement is left to them. In other words, if a
> parameter is declared as 'content type="integer"' and one enters a
> string in that place in the CIB, then it will happily pass validation.
> It would be up to the GUI or CRM shell to catch that configuration
> error, if at all. Andrew, Dejan, correct me if I'm wrong.

That is correct and by design. The DTD didn't actually allow for this
easily, but of course with schemas, in theory the RA's meta-data could
reference the schema it wants to be validated against ...

> 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!).

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.


> 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.
;-)


> 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"

> 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.

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 ;-)

If the CRM shell and GUI protect against this, users would be
sufficiently protected. Much easier to localize etc too.

> All of these are possibly feature requests not for the 1.0 series but
> for 1.1, but I'd like to hear people's thoughts about it anyway.

In general I agree with the goal, but I don't think the CIB is the place
for this.



Regards,
    Lars

-- 
SuSE Labs, OPS Engineering, Novell, Inc.
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
"Experience is the name everyone gives to their mistakes." -- Oscar Wilde