Merge lp:~newell-jensen/maas/amt_wsman into lp:~maas-committers/maas/trunk

[Newell Jensen]

AMT power template changes to use `wsman` in place of `amttool` for any AMT versions > 8.

[Christian Reis (kiko)]

Comments below. Note that there is one thing I'm not sure of -- is the AMT port always 16992, even on these new AMT 9 machines?

[Christian Reis (kiko)]

See http://en.community.dell.com/techcenter/systems-management/w/wiki/4375.wsman-windows-linux-command-conversion for some interesting examples of manipulation.

[Christian Reis (kiko)]

Do we need to provide special schemas for the different machine types we are to control?

[Christian Reis (kiko)]

Do we need a packaging update for wsmancli?

[Christian Reis (kiko)]

Here's the output for the identify on AMT 9 systems: http://pastebin.ubuntu.com/10073842/

[Jeff Lane  (bladernr)]

> Comments below. Note that there is one thing I'm not sure of -- is the AMT
> port always 16992, even on these new AMT 9 machines?

For wsman connections to AMT, 16992 is the HTTP port. 16993 is available for HTTPS connections but requires valid certificates before you can make it work, AIUI.

Here's a listing of the available ports and their usage for AMT:

https://software.intel.com/sites/manageability/AMT_Implementation_and_Reference_Guide/default.htm?turl=WordDocuments%2Fmanageabilityports.htm

[Newell Jensen (newell-jensen)]

This is ready to be reviewed.

[Christian Reis (kiko)]

No, you haven't addressed any of my comments as far as I can see.

review: Needs Fixing

[Christian Reis (kiko)]

And I have more comments.

[Newell Jensen (newell-jensen)]

> Comments below. Note that there is one thing I'm not sure of -- is the AMT
> port always 16992, even on these new AMT 9 machines?

See Jeff's comment below:

https://code.launchpad.net/~newell-jensen/maas/amt_wsman/+merge/248174/comments/616865

[Newell Jensen (newell-jensen)]

> Do we need a packaging update for wsmancli?

Since it worked and there isn't an update candidate for the package I dont' believe so:

$ apt-cache policy wsmancli
wsmancli:
  Installed: 2.3.0-0ubuntu4
  Candidate: 2.3.0-0ubuntu4
  Version table:
 *** 2.3.0-0ubuntu4 0
        500 http://us.archive.ubuntu.com/ubuntu/ trusty/universe amd64 Packages
        100 /var/lib/dpkg/status

[Christian Reis (kiko)]

On Wed, Feb 11, 2015 at 05:55:04PM -0000, Newell Jensen wrote:
> > Do we need a packaging update for wsmancli?
>
> Since it worked and there isn't an update candidate for the package I dont' believe so:

I meant do we need to add it to the recommends line?

[Jeff Lane  (bladernr)]

I've made several comments to the diff comments... some of which Newell may have also addressed in the time I've been working on cleaning the script up.

[Jeff Lane  (bladernr)]

> On Wed, Feb 11, 2015 at 05:55:04PM -0000, Newell Jensen wrote:
> > > Do we need a packaging update for wsmancli?
> >
> > Since it worked and there isn't an update candidate for the package I dont'
> believe so:
>
> I meant do we need to add it to the recommends line?

Good point, I say so since it's a universe package. I'll make that change to the tree as well.

[Jeff Lane  (bladernr)]

> Good point, I say so since it's a universe package. I'll make that change to
> the tree as well.

I take that back, I can't because the control file isn't included in this tree... so Yes, I would suggest that we include wsmancli as a recommends.

[Jeff Lane  (bladernr)]

> Do we need to provide special schemas for the different machine types we are
> to control?

AFAIK, we do not. I've used the same schemas/xml on Lenovo hardware that is used on the NUCs in the Orange Boxes and Newell has also tested them on a Dell. So three different makes using the same schemas, we should be safe.

[Jeff Lane  (bladernr)]

More replies, I think I've replied to everything now. Next step is I will upload my changes to lp and do a merge request from my branch into Newell's... then Newell can merge that into his, and update this Merge Proposal.

[Christian Reis (kiko)]

I've consolidated my comments now.

review: Needs Fixing

[Newell Jensen (newell-jensen)]

Latest changes from bladernr have been merged in.

[Jeff Lane  (bladernr)]

See inline... I KNOW now why the formatting looks wonky in that one place... here's a tab in there instead of all spaces. doesn't matter to bash, python would have told me immediately on running.

[Newell Jensen (newell-jensen)]

Jeff,

Glad you found out the real reason for the comment I had. Since this hasn't caused in issue in executing the code and it is only showing up in the LP diff, I don't see the harm in it.

[Jeff Lane  (bladernr)]

Kiko, any chance you can take another look at the latest changes and comment replies?

review: Needs Resubmitting

[Andres Rodriguez (andreserl)]

I just have a few comments:

1. I still believe that in the WebUI we should display the AMT Version and allow the user to chose what AMT version to use. This is actually a mechanism to make aware the user that there's 2 different AMT versions being supported.

2. See inline.

3. Consider making this a python power driver instead of continuing using shell.

review: Abstain

[Jeff Lane  (bladernr)]

Andres,

First, I do agree but the I really need the template changes in maas packages as soon as possible, as this is blocking certification currently, and will definitely be blocking even more systems in March.

In any case, I DO agree with you, and I would like to propose this:

0: We KNOW this current implementation works on old and new AMT versions, I have tested it repeatedly with AMT9 and Mark Wenning has tested it at Dell with AMT4, 7, and 8 at the least. Because of that I would like to get this merged as soon as we can, and get the fix SRU'd into MAAS 1.7.1 so I can continue on with certification of AMT systems which currently are blocked by MAAS.

1: Regarding Comment #1 above, the nice thing about what it does now is that the user doesn't NEED to know or care what AMT version the system is using. That said, which versions should we care about? Realistically, it's a matter of "8 or older" vs "9 or newer". I have asked Mark to check the wsman commands manually on his array of older AMT hardware. If wsman-cli works on all AMT versions, I'd be happy with removing the amttool parts completely and just settling on wsman for all AMT systems. That much I think, assuming mark can test them quickly, I can get into this merge. It should also address your Commend #2 and negate the need for a UI change and making the user care about what version AMT they're running.

2: I agree with your inline comment (#2) about checking each time. See 1 Above which should address this one.

3: I talked with Andrew and converting all the power templates to Python (Comment #3) that need to be is something we'd like to see done, and I think the Enablement Team can likely help convert those.

SO I'll see if I can help Mark out with wsman testing on the older AMT versions and if that works, I'll make another update to this MP. And then we can consider the Python rewrites later.

[Christian Reis (kiko)]

I don't think the UI needs to indicate what version of AMT for the moment; this template change should be transparent to the end user -- if they have wsman installed, and it's AMT9, it works. I had intended to keep amttool support in to minimize change, so I'm not sure why this being being rediscussed.

I don't think we should convert this template to Python in the same step as making this change.

[Christian Reis (kiko)]

Read Andres' inline comment -- I see what he is saying. But is running `amttool info` that expensive, enough we should worry about it?

[Jeff Lane  (bladernr)]

> I don't think the UI needs to indicate what version of AMT for the moment;
> this template change should be transparent to the end user -- if they have
> wsman installed, and it's AMT9, it works. I had intended to keep amttool
> support in to minimize change, so I'm not sure why this being being
> rediscussed.

I agree, UI changes should come at a later date if necessary. As for amttool support, I understand what you're saying, my only point was that if wsman works for both older and new/current AMT versions, is there a benefit to keeping two tools around that do essentially the same job if one tool works for all?

In any case, I also think that is something to revisit later, not as part of this proposal.

> I don't think we should convert this template to Python in the same step as
> making this change.

Also agreed, I meant to imply that too would be revisited at a later date, that's up to You and Andres and Andy to mull over as we now have two teams responsible for MAAS upkeep, the MAAS team and Server Enablement.

[Jeff Lane  (bladernr)]

> Read Andres' inline comment -- I see what he is saying. But is running
> `amttool info` that expensive, enough we should worry about it?

I wonder about that too... it's not THAT expensive but I think there is the possibility that it could cause problems at scale, if we ever run into a scenario where someone actually outfitted a large DC with AMT based machines... I honestly don't expect we'll see many 10000 unit AMT datacenters, BUT stranger things have happened.

Having to check the version before choosing which tool does mean that every action requires two queries and two responses, so you're effectively doubling the traffic for each function (Power up, off, recycle and status)

So I asked Mark to check out wsman on older AMT versions for due diligence. I am certainly willing to address that later on as a different project as well, that would make perfect sense.

Or I guess maybe I'm throwing Newell under the bus there and implying HE will address it ;-)

[Blake Rouse (blake-rouse)]

Overall I am happy with the code, I only have a few questions. If those can be answered then I will approve this branch.

review: Needs Information

[Jeff Lane  (bladernr)]

I've made a couple changes per Blake's comments. I'll upload them and get Newell to merge them here for a final review.

[Blake Rouse (blake-rouse)]

Cool thanks for the information. Looks good to me.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~newell-jensen/maas/amt_wsman into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [62.0 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Get:4 http://security.ubuntu.com trusty-security/main Sources [71.5 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:5 http://nova.clouds.archive.ubuntu.com trusty-updates Release [62.0 kB]
Get:6 http://security.ubuntu.com trusty-security/universe Sources [17.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [220 kB]
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [87.7 kB]
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [181 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [105 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [446 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [254 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,509 kB in 2s (504 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 postgresql pyflakes python-bson python-bzrlib python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-iscpy python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces python-nose python-oauth python-openssl python-paramiko python-pex...

=== modified file 'etc/maas/templates/power/amt.template'
--- etc/maas/templates/power/amt.template	2015-01-29 16:03:31 +0000
+++ etc/maas/templates/power/amt.template	2015-02-26 18:05:27 +0000
@@ -38,17 +38,27 @@
     exit $1
 }
 
+# XXX: bladernr 2015-02-04 bug=1331214: AMT ver > 8 needs wsman not amttool
+# Check the version of AMT on node.
+check_amt_version() {
+    amt_version=$(issue_amt_command info 2>&1 | grep version | awk '{print $NF}')
+    major_version=$(echo $amt_version | awk -F'.' '{print $1}')
+    return $major_version
+}
+
 
 # Perform a command using amttool.
 issue_amt_command() {
   LC_ALL='C' AMT_PASSWORD="$power_pass" amttool "$ip_address" "$@"
 }
 
+
 # Check that amttool is present.
 check_amt_command() {
   command -v amttool >/dev/null 2>&1 || { fail 2 "Missing amttool (amtterm package)"; }
 }
 
+
 # Check that a node host has been passed.
 check_bmc_host() {
   if [ -z "$ip_address" ]; then
@@ -59,11 +69,11 @@
   fi
 }
 
-# Ask for node's power state: 'on' or 'off'.
-query_state() {
+
+# Ask for node's power state: 'on' or 'off', via amttool.
+amt_query_state() {
     # Retry the state if it fails because it often fails the first time.
     local state=
-    local count=
     for attempts in $(seq 0 10)
     do
         state=$(issue_amt_command info | grep '^Powerstate:' | awk '{print $2}')
@@ -91,21 +101,20 @@
 }
 
 
-# Power-cycle the machine, and boot it using the requested boot mode.
-restart() {
+# Power-cycle the machine via amttool, and boot it using the requested boot mode.
+amt_restart() {
     yes | issue_amt_command powercycle "$amt_boot_mode"
 }
 
 
-# Power the machine on, and boot it using PXE.
-power_on() {
-    local attempts=
+# Power the machine on via amttool, and boot it using PXE.
+amt_power_on() {
     # Try several times.  Power commands often fail the first time.
     for attempts in $(seq 0 10)
     do
         # Issue the AMT command; amttool will prompt for confirmation.
         yes | issue_amt_command powerup "$amt_boot_mode"
-        if [ "$(query_state)" = 'on' ]
+        if [ "$(amt_query_state)" = 'on' ]
         then
             # Success.  Machine is on.
             return 0
@@ -116,13 +125,12 @@
 }
 
 
-# Power the machine off.
-power_off() {
-    local attempts=
+# Power the machine off amttool.
+amt_power_off() {
     # Try several times.  Power commands often fail the first time.
     for attempts in $(seq 0 10)
     do
-        if [ "$(query_state)" = 'off' ]
+        if [ "$(amt_query_state)" = 'off' ]
         then
             # Success.  Machine is off.
             return 0
@@ -135,28 +143,181 @@
 }
 
 
+# Check that wsman is present.
+check_wsman_command() {
+  command -v wsman >/dev/null 2>&1 || { fail 2 "Missing wsman (wsmancli package)"; }
+}
+
+# Issue the wsman command.
+issue_wsman_command() {
+    POWER_TEMPLATE_HEAD='
+<p:RequestPowerStateChange_INPUT xmlns:p="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_PowerManagementService">
+  <p:PowerState>'
+    POWER_TEMPLATE_TAIL='</p:PowerState>
+  <p:ManagedElement xmlns:wsa="http://schemas.xmlsoap.org/ws/2004/08/addressing"
+		    xmlns:wsman="http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd">
+    <wsa:Address>http://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymous</wsa:Address>
+    <wsa:ReferenceParameters>
+      <wsman:ResourceURI>http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ComputerSystem</wsman:ResourceURI>
+      <wsman:SelectorSet>
+        <wsman:Selector Name="CreationClassName">CIM_ComputerSystem</wsman:Selector>
+        <wsman:Selector Name="Name">ManagedSystem</wsman:Selector>
+      </wsman:SelectorSet>
+    </wsa:ReferenceParameters>
+  </p:ManagedElement>
+</p:RequestPowerStateChange_INPUT>
+'
+    # Usage: issue_wsman_command on|off|restart|query
+    ACTION="invoke -a RequestPowerStateChange"
+    SCHEMA_URI='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_PowerManagementService?SystemCreationClassName="CIM_ComputerSystem",SystemName="Intel(r) AMT",CreationClassName="CIM_PowerManagementService",Name="Intel(r) AMT Power Management Service"'
+    OPTS="--port 16992 --hostname $ip_address --username admin -p $power_pass -V -v"
+    case $1 in
+    on)
+        template=${POWER_TEMPLATE_HEAD}2${POWER_TEMPLATE_TAIL}
+        echo ${template} | wsman ${ACTION} "${SCHEMA_URI}" ${OPTS} -J -
+        ;;
+    off)
+        template=${POWER_TEMPLATE_HEAD}8${POWER_TEMPLATE_TAIL}
+        echo ${template} | wsman ${ACTION} "${SCHEMA_URI}" ${OPTS} -J -
+        ;;
+    restart)
+        template=${POWER_TEMPLATE_HEAD}10${POWER_TEMPLATE_TAIL}
+        echo ${template} | wsman ${ACTION} "${SCHEMA_URI}" ${OPTS} -J -
+        ;;
+    query)
+        ACTION="enumerate"
+        SCHEMA_URI='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_AssociatedPowerManagementService'
+        OPTS="${OPTS} -o -j utf-8"
+        wsman ${ACTION} "${SCHEMA_URI}" ${OPTS}
+	;;
+    *)
+        fail 2 "Unknown command issued to wsman"
+        ;;
+    esac
+}
+
+
+# Ask for node's power state: 'on' or 'off', via wsman.
+wsman_query_state() {
+    # Retry the state if it fails because it often fails the first time.
+    local state=
+    for attempts in $(seq 0 10)
+    do
+        state=$(issue_wsman_command query | grep "h:PowerState" | awk -F [\<\>] '{print $3}')
+        if [ -n "$state" ]
+        then
+            break
+        fi
+        # Wait 1 second between retries.  AMT controllers are generally very
+        # light and may not be comfortable with more frequent queries.
+        sleep 1
+    done
+    case "$state" in
+    # There are a LOT of possible power states
+    # 1: Other                      9: Power Cycle (Off - Hard)     
+    # 2: On                         10: Master Bus Reset
+    # 3: Sleep - Light              11: Diagnostic Interrupt (NMI)
+    # 4: Sleep - Deep               12: Off - Soft Graceful
+    # 5: Power Cycle (Off - Soft)   13: Off - Hard Graceful
+    # 6: Off - Hard                 14: Master Bus Reset Graceful
+    # 7: Hibernate (Off - Soft)     15: Power Cycle (Off - Soft Graceful)
+    # 8: Off - Soft                 16: Power Cycle (Off - Hard Graceful
+    #                               17: Diagnostic Interrupt (INIT)
+    
+    # These are all power states that indicate that the system is either ON or
+    # will resume function in an ON or Powered Up state (e.g. being power
+    # cycled currently)
+    [2,3,4,5,7,9,10,14,15,16])
+        echo 'on'
+        ;;
+    # These 4 states should leave system completely powered down
+    [6,8,12,13])
+        echo 'off'
+        ;;
+    *)
+        fail 1 "Got unknown power state from node: '$state'"
+        ;;
+    esac
+}
+
+
+# Power-cycle the machine via wsman, and boot it using the requested boot mode.
+wsman_restart() {
+    issue_wsman_command restart
+}
+
+
+# Power the machine on via wsman, and boot it using PXE.
+wsman_power_on() {
+    # Issue the wsman command to change power state
+    issue_wsman_command on
+    # Check power state several times.  It usually takes a second or two to get
+    # the correct state.
+    for attempts in $(seq 0 10)
+    do
+        if [ "$(wsman_query_state)" = 'on' ]
+        then
+            # Success.  Machine is on.
+            return 0
+        fi
+        sleep 1
+    done
+    fail 1 "Machine is not powering on.  Giving up."
+}
+
+# Power the machine off via wsman.
+# Currently commented out because we need a way to power query the machines
+wsman_power_off() {
+    # Issue the wsman command to change power state
+    issue_wsman_command off
+    # Check power state several times.  It usually takes a second or two to get 
+    # the correct state
+    for attempts in $(seq 0 10)
+    do
+        if [ "$(wsman_query_state)" = 'off' ]
+        then
+            # Success.  Machine is off.
+            return 0
+        fi
+        sleep 1
+    done
+    fail 1 "Machine is not powering off.  Giving up."
+}
+
 main() {
     check_bmc_host
     check_amt_command
+    # Check AMT version
+    check_amt_version
+    amt_version=$?
+    if [ ${amt_version} -gt 8 ]; then
+	# version > 8, use wsman
+	    check_wsman_command
+        tool="wsman"
+    else
+    # Use amttool
+        tool="amt"
+    fi
+
     case "${power_change}" in
     'on')
-        if [ "$(query_state)" = 'on' ]
+        if [ "${tool}_query_state" = 'on' ]
         then
             # The machine is already up, so powering it up will do nothing.
             # Cycle the power instead.
-            restart
+            ${tool}_restart
         else
-            power_on
+            ${tool}_power_on
         fi
         ;;
     'off')
-        if [ "$(query_state)" != 'off' ]
+        if [ "${tool}_query_state" != 'off' ]
         then
-            power_off
+            ${tool}_power_off
         fi
         ;;
     'query')
-        query_state
+        ${tool}_query_state
         ;;
     *)
         fail 1 "Unknown power command: '$1'"