Skip to content

Commit

Permalink
fence_ipmilan: Add action diag
Browse files Browse the repository at this point in the history 
Previous version of fence_ipmilan (C-based) contains also action 'diag'. This commit adds this action to new fence agent.

Resolves: rhbz#1286045
  • Loading branch information
@marxsk
marxsk committed Dec 2, 2015
1 parent e1b4c5a commit 7e65180
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 2 deletions.
15 changes: 13 additions & 2 deletions fence/agents/ipmilan/fence_ipmilan.py
View file
Expand Up @@ -27,6 +27,10 @@ def reboot_cycle(_, options):
output = run_command(options, create_command(options, "cycle"))
return bool(re.search('chassis power control: cycle', str(output).lower()))

def reboot_diag(_, options):
output = run_command(options, create_command(options, "diag"))
return bool(re.search('chassis power control: diag', str(output).lower()))

def create_command(options, action):
cmd = options["--ipmitool-path"]

Expand Down Expand Up @@ -123,7 +127,7 @@ def define_new_opts():
def main():
atexit.register(atexit_handler)

device_opt = ["ipaddr", "login", "no_login", "no_password", "passwd",
device_opt = ["ipaddr", "login", "no_login", "no_password", "passwd", "diag",

This comment has been minimized.

Sign in to view

Copy link
@btravouillon

btravouillon Dec 2, 2015

Is it a requirement to add this new device option? Is it a boolean? Reading the whole patch, it seems that action="diag" should work without this new device option? Am I wrong?

This comment has been minimized.

Sign in to view

Copy link
@marxsk

marxsk Dec 7, 2015

Author Contributor

Action 'diag' will work without this device option but we need to setup metadata/help properly.
"lanplus", "auth", "cipher", "privlvl", "sudo", "ipmitool_path", "method"]
define_new_opts()

Expand Down Expand Up @@ -156,7 +160,14 @@ def main():
if not is_executable(options["--ipmitool-path"]):
fail_usage("Ipmitool not found or not accessible")

result = fence_action(None, options, set_power_status, get_power_status, None, reboot_cycle)
reboot_fn = reboo_cycle

This comment has been minimized.

Sign in to view

Copy link
@martinetd

martinetd Dec 3, 2015

I'm pretty sure this should be reboot_cycle, not reboo_cycle.

This comment has been minimized.

Sign in to view

Copy link
@marxsk

marxsk Dec 7, 2015

Author Contributor

Yup, you are right.
if options["--action"] == "diag":
# Diag is a special action that can't be verified so we will reuse reboot functionality
# to minimize impact on generic library
options["--action"] = "reboot"
reboot_fn = reboot_diag

result = fence_action(None, options, set_power_status, get_power_status, None, reboot_fn)
sys.exit(result)

if __name__ == "__main__":
Expand Down
1 change: 1 addition & 0 deletions fence/agents/lib/fence2man.xsl
View file
Expand Up @@ -30,6 +30,7 @@
<xsl:when test="@name = 'enable'">Enable fabric access.</xsl:when>
<xsl:when test="@name = 'disable'">Disable fabric access.</xsl:when>
<xsl:when test="@name = 'reboot'">Reboot machine.</xsl:when>
<xsl:when test="@name = 'diag'">Pulse a diagnostic interrupt to the processor(s).</xsl:when>
<xsl:when test="@name = 'monitor'">Check the health of fence device</xsl:when>
<xsl:when test="@name = 'metadata'">Display the XML metadata describing this resource.</xsl:when>
<xsl:when test="@name = 'list'">List available plugs with aliases/virtual machines if there is support for more then one device. Returns N/A otherwise.</xsl:when>
Expand Down
6 changes: 6 additions & 0 deletions fence/agents/lib/fencing.py.py
View file
Expand Up @@ -140,6 +140,10 @@
"getopt" : "",
"help" : "",
"order" : ""},
"diag" : {
"getopt" : "",
"help" : "",
"order" : ""},
"passwd" : {
"getopt" : "p:",
"longopt" : "password",
Expand Down Expand Up @@ -1385,5 +1389,7 @@ def _get_available_actions(device_opt):
if not device_opt.count("separator"):
available_actions.remove("list")
available_actions.remove("list-status")
if device_opt.count("diag"):
available_actions.append("diag")

return (available_actions, default_value)
1 change: 1 addition & 0 deletions tests/data/metadata/fence_idrac.xml
View file
Expand Up @@ -168,5 +168,6 @@
<action name="monitor" />
<action name="metadata" />
<action name="validate-all" />
<action name="diag" />
</actions>
</resource-agent>
1 change: 1 addition & 0 deletions tests/data/metadata/fence_ilo3.xml
View file
Expand Up @@ -168,5 +168,6 @@
<action name="monitor" />
<action name="metadata" />
<action name="validate-all" />
<action name="diag" />
</actions>
</resource-agent>
1 change: 1 addition & 0 deletions tests/data/metadata/fence_ilo4.xml
View file
Expand Up @@ -168,5 +168,6 @@
<action name="monitor" />
<action name="metadata" />
<action name="validate-all" />
<action name="diag" />
</actions>
</resource-agent>
1 change: 1 addition & 0 deletions tests/data/metadata/fence_imm.xml
View file
Expand Up @@ -168,5 +168,6 @@
<action name="monitor" />
<action name="metadata" />
<action name="validate-all" />
<action name="diag" />
</actions>
</resource-agent>
1 change: 1 addition & 0 deletions tests/data/metadata/fence_ipmilan.xml
View file
Expand Up @@ -168,5 +168,6 @@
<action name="monitor" />
<action name="metadata" />
<action name="validate-all" />
<action name="diag" />
</actions>
</resource-agent>

3 comments on commit 7e65180

@martinetd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I'm actually testing this, fencing.py.py is also missing an update for in-line help in the action block e.g.:
"help" : "-o, --action=[action] Action: status, reboot (default), off or on",
should probably list diag too so people know they can use it?

Also, I finally took some time to test this, and... this does not behave as expected at all when I tried this?
I had to specify -m cycle to get the expected behavior, otherwise it does ipmitool chassis power off then waits till off then turns it back on (default -m offon)
e.g. fence_ipmilan -v -a IP -o diag -m cycle -P worked but fence_ipmilan -v -a IP -o diag -P did exactly the same as fence_ipmilan -v -a IP -o reboot -P

It's really easy to test so please do.

@marxsk
Copy link
Contributor Author

@marxsk marxsk commented on 7e65180 Dec 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.

With testing, it is quite difficult as I don't have direct access to most of the devices that are supported. My obsolete ipmi does not have 'diag' so it was not tested. Currently, I had obtained access to ilo4 (what is ipmi) and it works as expected.

@martinetd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm right I thought you could use a dummy IP and -v to see what command it tries to run but it will always try to do status before doing anything else.. Should be possible to test if the command it tries to run looks alright with verbose and an old ipmi though, even if the command diag itself will fail :)

Please sign in to comment.