Mailing List Archive

Call for review of undocumented parameters in resource agent meta data
On Fri, Jan 30, 2015 at 09:52:49PM +0100, Dejan Muhamedagic wrote:
> Hello,
>
> We've tagged today (Jan 30) a new stable resource-agents release
> (3.9.6) in the upstream repository.
>
> Big thanks go to all contributors! Needless to say, without you
> this release would not be possible.

Big thanks to Dejan.
Who once again finally did,
what I meant to do in late 2013 already, but simply pushed off
for over a year (and no-one else stepped up, either...)

So: Thank You.

I just today noticed that apparently some resource agents
accept and use parameters that are not documented in their meta data.

I now came up with a bash two-liner,
which likely still produces a lot of noise,
because it does not take into account that some agents
"source" additional helper files.

But here is the list:

--- used, but not described
+++ described, but apparently not used.

EvmsSCC +OCF_RESKEY_ignore_deprecation
Evmsd +OCF_RESKEY_ignore_deprecation

?? intentionally undocumented ??

IPaddr +OCF_RESKEY_iflabel
IPaddr -OCF_RESKEY_netmask

Not sure.


IPaddr2 -OCF_RESKEY_netmask

intentional, backward compat, quoting the agent:
# Note: We had a version out there for a while which used
# netmask instead of cidr_netmask. Don't remove this aliasing code!


Please help review these:

IPsrcaddr -OCF_RESKEY_ip
IPsrcaddr +OCF_RESKEY_cidr_netmask
IPv6addr.c -OCF_RESKEY_cidr_netmask
IPv6addr.c -OCF_RESKEY_ipv6addr
IPv6addr.c -OCF_RESKEY_nic
LinuxSCSI +OCF_RESKEY_ignore_deprecation
Squid -OCF_RESKEY_squid_confirm_trialcount
Squid -OCF_RESKEY_squid_opts
Squid -OCF_RESKEY_squid_suspend_trialcount
SysInfo -OCF_RESKEY_clone
WAS6 -OCF_RESKEY_profileName
apache +OCF_RESKEY_use_ipv6
conntrackd -OCF_RESKEY_conntrackd
dnsupdate -OCF_RESKEY_opts
dnsupdate +OCF_RESKEY_nsupdate_opts
docker -OCF_RESKEY_container
ethmonitor -OCF_RESKEY_check_level
ethmonitor -OCF_RESKEY_multiplicator

galera +OCF_RESKEY_additional_parameters
galera +OCF_RESKEY_binary
galera +OCF_RESKEY_client_binary
galera +OCF_RESKEY_config
galera +OCF_RESKEY_datadir
galera +OCF_RESKEY_enable_creation
galera +OCF_RESKEY_group
galera +OCF_RESKEY_log
galera +OCF_RESKEY_pid
galera +OCF_RESKEY_socket
galera +OCF_RESKEY_user

Probably all bogus, it source "mysql-common.sh".
Someone please have a more detailed look.


iSCSILogicalUnit +OCF_RESKEY_product_id
iSCSILogicalUnit +OCF_RESKEY_vendor_id

false positive

surprise: florian learned some wizardry back then ;-)
for var in scsi_id scsi_sn vendor_id product_id; do
envar="OCF_RESKEY_${var}"
if [ -n "${!envar}" ]; then
params="${params} ${var}=${!envar}"
fi
done

If such magic is used elsewhere,
that could mask "Used but not documented" cases.


iface-bridge -OCF_RESKEY_multicast_querier

!! Yep, that needs to be documented!

mysql-proxy -OCF_RESKEY_group
mysql-proxy -OCF_RESKEY_user

Oops, apparently my magic scriptlet below needs to learn to
ignore script comments...

named -OCF_RESKEY_rootdir

!! Probably a bug:
named_rootdir is documented.


nfsserver -OCF_RESKEY_nfs_notify_cmd

!! Yep, that needs to be documented!


nginx -OCF_RESKEY_client
nginx +OCF_RESKEY_testclient
!! client is used, but not documented,
!! testclient is documented, but unused...
Bug?

nginx -OCF_RESKEY_nginx

Bogus. Needs to be dropped from leading comment block.

oracle -OCF_RESKEY_tns_admin

!! Yep, that needs to be documented!

pingd +OCF_RESKEY_ignore_deprecation

?? intentionally undocumented ??

pingd -OCF_RESKEY_update

!! Yep, is undocumented.

sg_persist +OCF_RESKEY_binary
sg_persist -OCF_RESKEY_sg_persist_binary

!! BUG? binary vs sg_persist_binary

varnish -OCF_RESKEY_binary

!! Yep, is undocumented.


Please someone find the time to prepare pull requests
to fix these...

Thanks,

Lars

-----------------------------------------
List was generated by below scriptlet,
which can be improved. The improved version should probably be part of
a "unit test" check, when building resource-agents.

# In the git checkout of the resource agents,
# get a list of files that look like actual agent scripts.
cd heartbeat
A=$(git ls-files | xargs grep -s -l '<resource-agent ')

# and for each of these files,
# diff the list of OCF_RESKEY_* occurrences
# with the list of <parameter name="*" ones.
for a in $A; do
diff -U0 \
<( grep -h -o 'OCF_RESKEY_[[:alnum:]_]*' $a |
sort -u |
grep -v -e '_default$' -e 'OCF_RESKEY_$' -e 'OCF_RESKEY_CRM_meta' ) \
<( grep -h '<parameter ' $a |
sed -ne 's/^.*name="\([^"]*\)".*$/OCF_RESKEY_\1/p' |
sort -u) |
sed -e "/^@@\|^---\|^[+][+][+]/d;s#^#$a\t#";
done | column -t
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
Re: Call for review of undocumented parameters in resource agent meta data [ In reply to ]
Hi Lars,

On Thu, Feb 12, 2015 at 01:29:35AM +0100, Lars Ellenberg wrote:
> On Fri, Jan 30, 2015 at 09:52:49PM +0100, Dejan Muhamedagic wrote:
> > Hello,
> >
> > We've tagged today (Jan 30) a new stable resource-agents release
> > (3.9.6) in the upstream repository.
> >
> > Big thanks go to all contributors! Needless to say, without you
> > this release would not be possible.
>
> Big thanks to Dejan.
> Who once again finally did,
> what I meant to do in late 2013 already, but simply pushed off
> for over a year (and no-one else stepped up, either...)
>
> So: Thank You.

Thanks. But your contributions, which were numerous, are
certainly appreciated.

> I just today noticed that apparently some resource agents
> accept and use parameters that are not documented in their meta data.
>
> I now came up with a bash two-liner,
> which likely still produces a lot of noise,
> because it does not take into account that some agents
> "source" additional helper files.

> But here is the list:
>
> --- used, but not described

This is bad and needs to be fixed.

> +++ described, but apparently not used.

Just drop?

> EvmsSCC +OCF_RESKEY_ignore_deprecation
> Evmsd +OCF_RESKEY_ignore_deprecation
>
> ?? intentionally undocumented ??

No idea, but I doubt that anybody out there is using evms.

> IPaddr +OCF_RESKEY_iflabel

According to the history, this was never used.

> IPaddr -OCF_RESKEY_netmask

This got renamed to cidr_netmask, in an effort to make it more
consistent with IPaddr2 :) The same as what you found below.

> Not sure.
>
>
> IPaddr2 -OCF_RESKEY_netmask
>
> intentional, backward compat, quoting the agent:
> # Note: We had a version out there for a while which used
> # netmask instead of cidr_netmask. Don't remove this aliasing code!
>
>
> Please help review these:
>
> IPsrcaddr -OCF_RESKEY_ip
> IPsrcaddr +OCF_RESKEY_cidr_netmask
> IPv6addr.c -OCF_RESKEY_cidr_netmask
> IPv6addr.c -OCF_RESKEY_ipv6addr
> IPv6addr.c -OCF_RESKEY_nic
> LinuxSCSI +OCF_RESKEY_ignore_deprecation
> Squid -OCF_RESKEY_squid_confirm_trialcount
> Squid -OCF_RESKEY_squid_opts
> Squid -OCF_RESKEY_squid_suspend_trialcount
> SysInfo -OCF_RESKEY_clone
> WAS6 -OCF_RESKEY_profileName
> apache +OCF_RESKEY_use_ipv6

This is used in http-mon.sh, sourced by apache.

> conntrackd -OCF_RESKEY_conntrackd

This one got renamed to binary, so it's OK. I can still recall
the discussion--IMO not a biggie to have various RA differently
named parameters for the program (but at the time the other party
prevailed :)

> dnsupdate -OCF_RESKEY_opts
> dnsupdate +OCF_RESKEY_nsupdate_opts

Bug? lmb? OK, just fixed it. It should be only the latter.

> docker -OCF_RESKEY_container
> ethmonitor -OCF_RESKEY_check_level
> ethmonitor -OCF_RESKEY_multiplicator
>
> galera +OCF_RESKEY_additional_parameters
> galera +OCF_RESKEY_binary
> galera +OCF_RESKEY_client_binary
> galera +OCF_RESKEY_config
> galera +OCF_RESKEY_datadir
> galera +OCF_RESKEY_enable_creation
> galera +OCF_RESKEY_group
> galera +OCF_RESKEY_log
> galera +OCF_RESKEY_pid
> galera +OCF_RESKEY_socket
> galera +OCF_RESKEY_user
>
> Probably all bogus, it source "mysql-common.sh".
> Someone please have a more detailed look.
>
>
> iSCSILogicalUnit +OCF_RESKEY_product_id
> iSCSILogicalUnit +OCF_RESKEY_vendor_id
>
> false positive
>
> surprise: florian learned some wizardry back then ;-)
> for var in scsi_id scsi_sn vendor_id product_id; do
> envar="OCF_RESKEY_${var}"
> if [ -n "${!envar}" ]; then
> params="${params} ${var}=${!envar}"
> fi
> done
>
> If such magic is used elsewhere,
> that could mask "Used but not documented" cases.
>
>
> iface-bridge -OCF_RESKEY_multicast_querier
>
> !! Yep, that needs to be documented!
>
> mysql-proxy -OCF_RESKEY_group
> mysql-proxy -OCF_RESKEY_user
>
> Oops, apparently my magic scriptlet below needs to learn to
> ignore script comments...
>
> named -OCF_RESKEY_rootdir
>
> !! Probably a bug:
> named_rootdir is documented.
>
>
> nfsserver -OCF_RESKEY_nfs_notify_cmd
>
> !! Yep, that needs to be documented!
>
>
> nginx -OCF_RESKEY_client
> nginx +OCF_RESKEY_testclient
> !! client is used, but not documented,
> !! testclient is documented, but unused...
> Bug?

Yeah. Yet another one of the kind.

> nginx -OCF_RESKEY_nginx
>
> Bogus. Needs to be dropped from leading comment block.
>
> oracle -OCF_RESKEY_tns_admin
>
> !! Yep, that needs to be documented!

Nope. tns_admin is not used in oracle but in oralsnr, but the
two share some initialization stuff. Copy&paste issue. Will fix
that too.

> pingd +OCF_RESKEY_ignore_deprecation
>
> ?? intentionally undocumented ??

This deprecation thing seems to be some kind of standard thingie
and it is perused in ocf_deprecated(), looks like Florian's thing
too :)

> pingd -OCF_RESKEY_update
>
> !! Yep, is undocumented.
>
> sg_persist +OCF_RESKEY_binary
> sg_persist -OCF_RESKEY_sg_persist_binary
>
> !! BUG? binary vs sg_persist_binary

Fixed.

> varnish -OCF_RESKEY_binary
>
> !! Yep, is undocumented.
>
>
> Please someone find the time to prepare pull requests
> to fix these...

This would probably get more attention if posted as an issue at
github with a title "Please help improve RA meta data".
Otherwise, I doubt that we're going to get volunteers here to
write documentation ;-)

Cheers,

Dejan

> Thanks,
>
> Lars
>
> -----------------------------------------
> List was generated by below scriptlet,
> which can be improved. The improved version should probably be part of
> a "unit test" check, when building resource-agents.
>
> # In the git checkout of the resource agents,
> # get a list of files that look like actual agent scripts.
> cd heartbeat
> A=$(git ls-files | xargs grep -s -l '<resource-agent ')
>
> # and for each of these files,
> # diff the list of OCF_RESKEY_* occurrences
> # with the list of <parameter name="*" ones.
> for a in $A; do
> diff -U0 \
> <( grep -h -o 'OCF_RESKEY_[[:alnum:]_]*' $a |
> sort -u |
> grep -v -e '_default$' -e 'OCF_RESKEY_$' -e 'OCF_RESKEY_CRM_meta' ) \
> <( grep -h '<parameter ' $a |
> sed -ne 's/^.*name="\([^"]*\)".*$/OCF_RESKEY_\1/p' |
> sort -u) |
> sed -e "/^@@\|^---\|^[+][+][+]/d;s#^#$a\t#";
> done | column -t
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/