search

rcbops / rpc-maas

Ansible playbooks for deploying Rackspace Monitoring-as-a-Service within Openstack Environments
Apache License 2.0
32 stars 68 forks source link

Assuming the regex always finds something #82

Closed git-harry closed 10 years ago

git-harry commented 10 years ago

https://github.com/rcbops/rcbops-maas/blob/master/openmanage.py#L22

all() returns True on an empty list.

I wonder if the script should also check the version of the tools installed.

sigmavirus24 commented 10 years ago

We can check for an empty list and use status_err.

I wonder if the script should also check the version of the tools installed.

Is there a minimum version number required? If so, wouldn't that be the responsibility of the script that installs OpenManage to ensure that the minimum version requirement is satisfied?

git-harry commented 10 years ago

@sigmavirus24 we can only say this plugin works for the versions we test. It strikes me as reasonable guard against unintended consequences.

sigmavirus24 commented 10 years ago

True, but we haven't been doing this for other plugins (one of which sticks out to me as being especially necessary (the percona toolkit which we use for galera_consistency has to be a certain version to work as expected)). Should we start adding this to each plugin that relies on a minimum version of a tool? I suspect it wouldn't take a great deal of effort.

git-harry commented 10 years ago

@sigmavirus24 good point on the other plugins. Spoke with @mancdaz who agrees it's worth it. I'd say we should specify a specific version instead of a minimum.

sigmavirus24 commented 10 years ago

So... as a side note, while running omconfig about to get the version number of OpenManage that we've been testing with, I see:

sh: 1: /bin/rpm: not found

start the output (but the exit code is still 0, since it prints the about information as expected). This isn't breaking anything but I figured I'd note it somewhere.