Merge lp:~andreserl/maas/lp1073462_fence_cdu_power_type into lp:~maas-committers/maas/trunk

[Andres Rodriguez]

Add fence_cdu power type for SentrySwitch CDU's.

[Jeroen T. Vermeulen (jtv)]

Looks very nice.

Two small notes:

1. In python, when we have to line-break the arguments to a function call, we start doing so right after the opening parenthesis, so that the arguments start on a line of their own.

So instead of:

    call_function(one_argument,
        another_argument, final_argument)

...we say:

    call_function(
        one_argument, another_argument,
        final_argument)

2. It would be nice if the test explained (in a comment, doesn't need to be very long) how the error condition happens. As I understand it, you substitute "echo" for the power command, and when that command gets called, you get something that isn't recognized as a power state.

I think I probably did this myself at some point. But reading it back now, I realize it's not obvious. So for the sake of whoever maintains the code next, it would be helpful to point out this subtlety.

Jeroen

review: Approve

[Andres Rodriguez (andreserl)]

Hi Jeroen,

Thanks for the review!

[1] I don't understand it :). The only place where a line-break would be required is inside the test_fence_cdu_checks_state function, in the line 134 which calls render_template.... However, this line-break is correctly place for such call... The whole diff follows the same code as to what the code currently is:

134 + script = action.render_template(
135 + action.get_template(), power_change='on',
136 + power_address='mysystem', power_id='system',
137 + power_user='me', power_pass='me', fence_cdu='echo')

[2]: Fixed.

Thanks!

[Raphaël Badin (rvb)]

I think Jeroen was referring to that piece of code:

24 + 'power_id',
25 + forms.CharField(label="Power ID",
26 + required=False)),
27 + (
28 + 'power_address',
29 + forms.CharField(label="IP Address or Hostname",
30 + required=False)),

Instead of:

forms.CharField(label="Power ID",
    required=False))

You should write:

forms.CharField(
    label="Power ID", required=False))

[MAAS Lander (maas-lander)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/maas-merger-trunk/159/console reported an error when processing this lp:~andreserl/maas/lp1073462_fence_cdu_power_type branch.
Not merging it.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-12-18 12:31:47 +0000
+++ src/maasserver/models/node.py	2012-12-19 17:24:23 +0000
@@ -768,6 +768,7 @@
 
         power_params.setdefault('system_id', self.system_id)
         power_params.setdefault('virsh', '/usr/bin/virsh')
+        power_params.setdefault('fence_cdu', '/usr/sbin/fence_cdu')
         power_params.setdefault('ipmipower', '/usr/sbin/ipmipower')
         power_params.setdefault(
             'ipmi_chassis_config', '/usr/sbin/ipmi-chassis-config')
 
=== modified file 'src/maasserver/power_parameters.py'
--- src/maasserver/power_parameters.py	2012-09-17 17:27:02 +0000
+++ src/maasserver/power_parameters.py	2012-12-19 17:24:23 +0000
@@ -64,6 +64,26 @@
             ],
             required=False,
             skip_check=True),
+    POWER_TYPE.CDU:
+        DictCharField(
+            [
+                (
+                    'power_id',
+                    forms.CharField(label="Power ID",
+                        required=False)),
+                (
+                    'power_address',
+                    forms.CharField(label="IP Address or Hostname",
+                        required=False)),
+                (
+                    'power_user',
+                    forms.CharField(label="Username", required=False)),
+                (
+                    'power_pass',
+                    forms.CharField(label="Password", required=False)),
+            ],
+            required=False,
+            skip_check=True),
     POWER_TYPE.IPMI:
         DictCharField(
             [
 
=== modified file 'src/provisioningserver/enum.py'
--- src/provisioningserver/enum.py	2012-09-17 17:27:02 +0000
+++ src/provisioningserver/enum.py	2012-12-19 17:24:23 +0000
@@ -33,6 +33,9 @@
     # Network wake-up.
     WAKE_ON_LAN = 'ether_wake'
 
+    # Sentry Switch CDU's.
+    CDU = 'fence_cdu'
+
     # IPMI (Intelligent Platform Management Interface).
     IPMI = 'ipmi'
 
@@ -40,6 +43,7 @@
 POWER_TYPE_CHOICES = (
     (POWER_TYPE.VIRSH, "virsh (virtual systems)"),
     (POWER_TYPE.WAKE_ON_LAN, "Wake-on-LAN"),
+    (POWER_TYPE.CDU, "Sentry Switch CDU"),
     (POWER_TYPE.IPMI, "IPMI"),
     )
 
 
=== added file 'src/provisioningserver/power/templates/fence_cdu.template'
--- src/provisioningserver/power/templates/fence_cdu.template	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/power/templates/fence_cdu.template	2012-12-19 17:24:23 +0000
@@ -0,0 +1,54 @@
+# -*- mode: shell-script -*-
+#
+# Control virtual system's "power" through virsh.
+#
+
+# Parameters.
+# Choose command for virsh to make the requested power change happen.
+power_change={{power_change}}
+power_address={{power_address}}
+power_user={{power_user}}
+power_pass={{power_pass}}
+power_id={{power_id}}
+fence_cdu={{fence_cdu}}
+
+
+formulate_power_command() {
+  if [ ${power_change} = 'on' ]
+  then
+      echo 'on'
+  else
+      echo 'off'
+  fi
+}
+
+
+# Express system's current state as expressed by virsh as "on" or "off".
+formulate_power_state() {
+    case $2 in
+    'on'|'ON') echo 'on' ;;
+    'off'|'OFF') echo 'off' ;;
+    *)
+        echo "Got unknown power state from fence_cdu: '$1'" >&2
+        exit 1
+    esac
+}
+
+
+# Issue command to virsh, for the given system.
+issue_fence_cdu_command() {
+    ${fence_cdu} -a ${power_address} -n ${power_id} -l ${power_user} -p ${power_pass} -o "$@"
+}
+
+
+# Get the given system's power state: 'on' or 'off'.
+get_power_state() {
+    fence_cdu_state=$(issue_fence_cdu_command status)
+    formulate_power_state ${fence_cdu_state}
+}
+
+
+if [ "$(get_power_state)" != "${power_change}" ]
+then
+    issue_fence_cdu_command $(formulate_power_command)
+fi
 
=== modified file 'src/provisioningserver/power/tests/test_poweraction.py'
--- src/provisioningserver/power/tests/test_poweraction.py	2012-10-05 16:33:37 +0000
+++ src/provisioningserver/power/tests/test_poweraction.py	2012-12-19 17:24:23 +0000
@@ -159,6 +159,20 @@
         stdout, stderr = action.run_shell(script)
         self.assertIn("Got unknown power state from virsh", stderr)
 
+    def test_fence_cdu_checks_state(self):
+        # We can't test the fence_cdu template in detail (and it may be
+        # customized), but by making it use "echo" instead of a real
+        # fence_cdu we can make it get a bogus answer from its status check.
+        # The bogus answer is actually the rest of the fence_cdu command
+        # line.  It will complain about this and fail.
+        action = PowerAction(POWER_TYPE.CDU)
+        script = action.render_template(
+            action.get_template(), power_change='on',
+            power_address='mysystem', power_id='system',
+            power_user='me', power_pass='me', fence_cdu='echo')
+        stdout, stderr = action.run_shell(script)
+        self.assertIn("Got unknown power state from fence_cdu", stderr)
+
     def test_ipmi_checks_state(self):
         action = PowerAction(POWER_TYPE.IPMI)
         script = action.render_template(