[Fuego] [RFC] power-control for Fuego

[Fuego] [RFC] power-control for Fuego

[Daniel Sangorrin]

Hello Tim,

I have updated the power-control patches to the latest
code (next branch). Unfortunately, I couldn't install
the latest Fuego due to a "GPG error" in Debian. For
that reason, the I couln't test the modifications. 
Please consider the patches only as "sketch" or 
"mud on the wall" for further discussion.

[PATCH] ftc: add power-control subcommands
[PATCH] power-control: readme about pdudaemon and ttc

Q: should we use a separate python module?
Q: should we use a board-control class with on/off/reboot
operations?
Q: does the ttc function look OK to you?
Q: how do we move from here to the "board management 
abstraction" that you have in mind?

Thanks
Daniel

[Fuego] [PATCH] ftc: add power-control subcommands

[Daniel Sangorrin]

This is a temporary patch to discuss the implementation of
power-control on Fuego. The code hasn't been tested at all
because of a problem when installing Fuego.

Q: should we use a separate python module?
Q: should we use a board-control class with on/off/reboot
operations?

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 overlays/base/base-board-test-vars.fuegoclass |  12 +-
 overlays/base/base-board.fuegoclass           |  12 +-
 scripts/ftc                                   | 118 ++++++++++++++++++
 scripts/ftc_completion.sh                     |   3 +
 tests/Functional.fuego_ftc_test/ftc-tests     |  12 +-
 5 files changed, 124 insertions(+), 33 deletions(-)

diff --git a/overlays/base/base-board-test-vars.fuegoclass b/overlays/base/base-board-test-vars.fuegoclass
index f834f47..7619608 100644
--- a/overlays/base/base-board-test-vars.fuegoclass
+++ b/overlays/base/base-board-test-vars.fuegoclass
@@ -196,17 +196,7 @@ function ov_transport_cmd() {
 
 # function to reboot the board
 function ov_board_control_reboot() {
-  if [ -z "$TTC_TARGET" ] ; then
-    TTC_TARGET="$NODE_NAME"
-  fi
-  case "$BOARD_CONTROL" in
-  "ttc")
-    $TTC $TTC_TARGET reboot
-    ;;
-  *)
-    abort_job "Error reason: unsupported BOARD_CONTROL ${BOARD_CONTROL}"
-    ;;
-  esac
+  ftc power-cycle -b $NODE_NAME
 }
 
 # the following definitions are used to test variable precedence
diff --git a/overlays/base/base-board.fuegoclass b/overlays/base/base-board.fuegoclass
index ae35464..75bcb82 100644
--- a/overlays/base/base-board.fuegoclass
+++ b/overlays/base/base-board.fuegoclass
@@ -292,15 +292,5 @@ function ov_transport_cmd() {
 
 # function to reboot the board
 function ov_board_control_reboot() {
-  if [ -z "$TTC_TARGET" ] ; then
-    TTC_TARGET="$NODE_NAME"
-  fi
-  case "$BOARD_CONTROL" in
-  "ttc")
-    $TTC $TTC_TARGET reboot
-    ;;
-  *)
-    abort_job "Error reason: unsupported BOARD_CONTROL ${BOARD_CONTROL}"
-    ;;
-  esac
+  ftc power-cycle -b $NODE_NAME
 }
diff --git a/scripts/ftc b/scripts/ftc
index b81d511..7ff6d62 100755
--- a/scripts/ftc
+++ b/scripts/ftc
@@ -425,6 +425,33 @@ A message and the exit code indicate if the resource was released.
 
    ex: ftc release-resource -b beaglebone"""),
 
+"power-on": ("Power on the selected board.",
+    """Usage: ftc power-on -b <board1>[,<board2>...]
+
+This command can be used to power on a board. The board file must
+contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
+the corresponding options (see README.pdu).
+
+Example: ftc power-on -b raspberrypi3"""),
+
+"power-off": ("Power off the selected board.",
+    """Usage: ftc power-off -b <board1>[,<board2>...]
+
+This command can be used to power off a board. The board file must
+contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
+the corresponding options (see README.pdu).
+
+Example: ftc power-off -b raspberrypi3"""),
+
+"power-cycle": ("Power cycle the selected board.",
+    """Usage: ftc power-off -b <board1>[,<board2>...]
+
+This command can be used to power cycle (reboot) a board. The board file must
+contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
+the corresponding options (see README.pdu).
+
+Example: ftc power-cycle -b raspberrypi3"""),
+
 "package-test": ("Package a test.",
     """Usage: ftc package-test <test_name> [options]
 
@@ -4951,6 +4978,90 @@ def do_release_resource(conf, options):
 
     return 0
 
+def do_pdudaemon(command, bvars):
+    # pdudaemon hostname
+    if 'PDUDAEMON_HOSTNAME' in bvars:
+        pdudaemon_hostname = bvars['PDUDAEMON_HOSTNAME']
+    else:
+        pdudaemon_hostname = 'localhost'
+
+    # pdudaemon port (optional, default 16421)
+    if 'PDUDAEMON_PORT' in bvars:
+        pdudaemon_port = bvars['PDUDAEMON_PORT']
+    else:
+        pdudaemon_port = '16421'
+
+    # pdu hostname
+    if 'PDU_HOSTNAME' in bvars:
+        pdu_hostname = bvars['PDU_HOSTNAME']
+    else:
+        error_out('PDU_HOSTNAME not specified in board file')
+
+    # pdu port
+    if 'PDU_PORT' in bvars:
+        pdu_port = bvars['PDU_PORT']
+    else:
+        error_out('PDU_PORT not specified in board file')
+
+    # pdu delay
+    if 'PDU_DELAY' in bvars:
+        pdu_delay = bvars['PDU_PORT']
+    else:
+        pdu_delay = 1
+
+    cmd_map = {
+        'power-on' : 'on',
+        'power-off' : 'off',
+        'power-cycle' : 'reboot'
+    }
+    cmd = cmd_map[command]
+
+    query='&'.join(['hostname='+pdu_hostname, 'port='+pdu_port, 'delay='+pdu_delay])
+    url = 'http://%s:%s/power/control/%s?%s' % (pdudaemon_hostname, pdudaemon_port, cmd, query)
+    resp = requests.get(url)
+    if resp.status_code != 200:
+        error_out("pdudaemon did not accept the request")
+
+    return 0
+
+def do_ttc(command, bvars, board_name):
+    if 'TTC_TARGET' in bvars:
+        ttc_target = bvars['TTC_TARGET']
+    else:
+        ttc_target = board_name
+
+    cmd_map = {
+        'power-on' : 'on',
+        'power-off' : 'off',
+        'power-cycle' : 'reboot'
+    }
+    cmd = cmd_map[command]
+
+    subprocess.check_call('ttc %s %s' % (ttc_target, cmd), shell=True)
+
+    return 0
+
+def do_power_control(conf, options, command):
+    bmap = get_fuego_boards(conf)
+    board_names, options = get_board_arg(command, conf, options, "!m")
+
+    for board_name in board_names:
+        board = bmap[board_name]
+        bvars = get_board_vars(board, conf)
+        if 'BOARD_CONTROL' in bvars:
+            board_control = bvars['BOARD_CONTROL']
+        else:
+            error_out("BOARD_CONTROL is not defined for %s", board_name)
+
+        if board_control in ["pdudaemon", "PDUDAEMON", "PDUDaemon", "PDUdaemon"]:
+            do_pdudaemon(command, bvars)
+        elif board_control in ["ttc", "TTC", "Ttc"]:
+            do_ttc(command, bvars, board_name)
+        else:
+            error_out("BOARD_CONTROL value %s is not supported.")
+
+    return 0
+
 def main():
     # use global module names
     global re, time, copy2, subprocess, signal, fcntl, requests, json
@@ -5244,6 +5355,13 @@ def main():
         user_check(conf)
         do_add_view(conf, options)
 
+    if command in ["power-on", "power-off", "power-cycle"]:
+        user_check(conf)
+        try:
+            do_power_control(conf, options, command)
+        except Exception as e:
+            sys.exit(str(e) + '\n' + command_help[command][1])
+
     # all non-board commands have been handled
     #if command in board_mod_commands:
     #   check_reservation(bmap, command)
diff --git a/scripts/ftc_completion.sh b/scripts/ftc_completion.sh
index 984e003..67b1daf 100755
--- a/scripts/ftc_completion.sh
+++ b/scripts/ftc_completion.sh
@@ -55,6 +55,9 @@ _ftc()
         ["list-tests"]="-q"
         ["package-run"]="-o -f"
         ["package-test"]="-o -f"
+        ["power-on"]="-b"
+        ["power-off"]="-b"
+        ["power-cycle"]="-b"
         ["put-request"]="-R -s"
         ["put-run"]=""
         ["put-test"]=""
diff --git a/tests/Functional.fuego_ftc_test/ftc-tests b/tests/Functional.fuego_ftc_test/ftc-tests
index 4d38cbc..29865a6 100644
--- a/tests/Functional.fuego_ftc_test/ftc-tests
+++ b/tests/Functional.fuego_ftc_test/ftc-tests
@@ -108,17 +108,7 @@ Test board variable manipulation
 	                    ZZFOO : "not here"
 	               board_path : /fuego-ro/boards/ftc-test.board
 	function_ov_board_control_reboot : function ov_board_control_reboot() {
-	                              if [ -z "$TTC_TARGET" ] ; then
-	                                TTC_TARGET="$NODE_NAME"
-	                              fi
-	                              case "$BOARD_CONTROL" in
-	                              "ttc")
-	                                $TTC $TTC_TARGET reboot
-	                                ;;
-	                              *)
-	                                abort_job "Error reason: unsupported BOARD_CONTROL ${BOARD_CONTROL}"
-	                                ;;
-	                              esac
+	                              ftc power-cycle -b $NODE_NAME
 	                            }
 	                            
 	  function_ov_board_setup : function ov_board_setup () {
-- 
2.17.1

[Fuego] [PATCH] power-control: readme about pdudaemon and ttc

[Daniel Sangorrin]

Temporary readme to explain how my power control patch works.
Ultimately it should be integrated with the documentation
and the website

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 fuego-ro/boards/README.pdu | 146 +++++++++++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100644 fuego-ro/boards/README.pdu

diff --git a/fuego-ro/boards/README.pdu b/fuego-ro/boards/README.pdu
new file mode 100644
index 0000000..f6050cf
--- /dev/null
+++ b/fuego-ro/boards/README.pdu
@@ -0,0 +1,146 @@
+Using power control
+===================
+
+To power on, off and reboot a board use.
+
+  $ ftc power-on -b bbb
+  $ ftc power-off -b bbb
+  $ ftc power-cycle -b bbb
+
+You can also specify multiple boards at once.
+
+  $ ftc power-on -b bbb,raspi3
+  $ ftc power-off -b bbb,raspi3
+  $ ftc power-cycle -b bbb,raspi3
+
+Adding board control to your board
+==================================
+
+First, you need to decide what board control software to use. For example,
+if you want to use PDUdaemon write:
+
+  $ vi fuego-ro/boards/bbb.board
+    BOARD_CONTROL="pdudaemon"
+
+If you prefer to use TTC then write
+
+  $ vi fuego-ro/boards/bbb.board
+    BOARD_CONTROL="ttc"
+
+Next, you need to provide the options required by the board control software
+that you chose.
+
+PDUdaemon-related board options
+-------------------------------
+
+First, you need to configure your PDU hostname and PORT.
+ - PDU_HOSTNAME: the hostname or IP address of your PDU. This needs to match
+   the one defined in the PDUDaemon configuration file (see below)
+ - PDU_PORT: the PDU port number that identifies the outlet where the board
+   is plugged.
+
+  $ vi fuego-ro/boards/bbb.board
+    BOARD_CONTROL="pdudaemon"
+    PDU_HOSTNAME=192.168.11.68
+    PDU_PORT=1
+
+You can optionally specify a delay.
+  - PDU_DELAY: seconds before command runs, or between off/on when rebooting
+    (default: 1 second)
+
+  $ vi fuego-ro/boards/bbb.board
+    BOARD_CONTROL="pdudaemon"
+    PDU_HOSTNAME=192.168.11.68
+    PDU_PORT=1
+    PDU_DELAY=5
+
+Finally, if you are running PDUDaemon outside of your host machine then you
+need to configure PDUDAEMON_HOSTNAME and PDUDAEMON_PORT.
+ - PDUDAEMON_HOSTNAME: hostname or IP address of the computer where the
+   PDUDaemon container is running (default: localhost)
+ - PDUDAEMON_PORT: port where PDUDaemon is listening (default: 16421)
+
+  $ vi fuego-ro/boards/bbb.board
+    BOARD_CONTROL="pdudaemon"
+    PDU_HOSTNAME=192.168.11.68
+    PDU_PORT=1
+    PDUDAEMON_HOSTNAME=localhost <-- change as necessary
+    PDUDAEMON_PORT=16421 <-- change only if you know what you are doing
+
+TTC-related board options
+-------------------------
+
+You only need to configure the TTC target name on your board. Otherwise,
+the board name will be used by default.
+
+  $ vi fuego-ro/boards/bbb.board
+    TTC_TARGET="bbb"
+
+PDUDaemon installation notes
+============================
+
+PDUDaemon can be installed quickly as a Docker container. The first step is to
+clone the source code.
+
+  $ git clone https://github.com/pdudaemon/pdudaemon
+  $ cd pdudaemon
+
+Next, configure the PDUDaemon's hostname and port. This is where the daemon
+will listen for requests.
+
+  $ vi share/pdudaemon.conf
+    {
+        "daemon": {
+            "hostname": "localhost", <-- PDUDaemon hostname or IP address
+            "port": 16421, <-- PDUDaemon port
+            "dbname": "pdudaemon",
+            "logging_level": "DEBUG",
+            "listener": "http"
+        },
+
+You also need to configure your PDU(s). Try to find a driver that supports
+your PDU under pdudaemon/drivers/ (e.g. ip9258.py). Open the driver file
+and check for adjustable settings (e.g., grep settings).
+
+  $ vi share/pdudaemon.conf
+    [...]
+        "pdus": {
+            "192.168.11.68": { <-- PDU hostname or IP address
+                "driver": "ip9258" <-- PDU driver
+            }
+        }
+    }
+
+If you cannot find a driver for your PDU, you will have to write one.
+Alternatively, you may be able to configure the "localcmdline" driver for
+your needs. The symbol %d represents a "port number" that identifies the
+outlet where a board is connected
+
+  $ vi share/pdudaemon.conf
+    [...]
+        "myscript_hostname": {
+            "driver": "localcmdline",
+            "cmd_on": "myscript %d on",
+            "cmd_off": "myscript %d off"
+        },
+
+Finally, build the Docker image and run it on the machine where the daemon
+is configured to run. Here we run it locally because we are using "localhost".
+
+  $ docker build -t pdudaemon --build-arg HTTP_PROXY=$http_proxy -f Dockerfile.dockerhub .
+  $ docker run --rm -it -e http_proxy=$http_proxy -e https_proxy=$https_proxy -e NO_PROXY="$no_proxy" --net="host" pdudaemon:latest
+
+Test the installation manually
+------------------------------
+
+First, make sure that the PDUDaemon is reachable.
+
+  $ ping $PDUDAEMON_HOSTNAME
+
+Next, you can use CURL to power on and off the outlet number 1.
+
+  $ curl "http://localhost:16421/power/control/on?hostname=192.168.11.68&port=1"
+    OK - accepted request
+  $ curl "http://localhost:16421/power/control/off?hostname=192.168.11.68&port=1"
+    OK - accepted request
+
-- 
2.17.1

Re: [Fuego] [RFC] power-control for Fuego

[Tim.Bird]

> -----Original Message-----
> From: Daniel Sangorrin
> 
> Hello Tim,
> 
> I have updated the power-control patches to the latest
> code (next branch). Unfortunately, I couldn't install
> the latest Fuego due to a "GPG error" in Debian. For
> that reason, the I couln't test the modifications.
> Please consider the patches only as "sketch" or
> "mud on the wall" for further discussion.
> 
> [PATCH] ftc: add power-control subcommands
> [PATCH] power-control: readme about pdudaemon and ttc
> 
OK - I think some of this stuff is worth having a face-to-face discussion about.

I'm going to brainstorm a bit in this message, so this might be a bit disorganized.

Taking a step back, here are the issues that I think we're trying to solve:
1) - Fuego doesn't have a board provisioning layer, which puts the burden on the
user to do this themselves.  That's not good.
2) - In order to build a provisioning layer in Fuego, there must be some interface
to a board control layer (ie a board control API)
3) - Fuego itself needs to use some board control APIS (e.g. reboot) in order
to perform automated recovery from test failures.
4) - we'd like to add support for an external board control layer, rather than
build our own, to avoid adding more fragmentation to the industry for this section
of the overall automated testing stack.

Here are some desired features:
 - ability to call different external board control layers, depending on the board
 - ability for a test to call board control functions
 - ability for ftc to call board control functions
 - ability for a human to perform board control functions (manually)
 - ability to isolate the board control details from callers
 - can store board control data in a single place (not replicate board control data in multiple layers)

I had started to do this by adding the BOARD_CONTROL variable, and fuegoclass
functions (shell functions), that ftc or a test could call.  However, that only allowed
the board control functions to be called in "Fuego context" - that is, Fuego had to
instantiate an entire Fuego test context in order to execute a simple control function.

Here's the old API:
 - nature: shell function
 - commands: ov_board_control_reboot() - no sub-commands or arguments
 - meta-data storage: in board file

The new API in the proposed patch is:
 - nature: command line (via 'ftc')
 - commands: power-on, power-off, power-cycle

> Q: should we use a separate python module?
> Q: should we use a board-control class with on/off/reboot
> operations?
I think these questions have to do with  the power driver API (ie, how does a new
power control layer plug into Fuego), rather than the power API (how does Fuego
invoke the control functions).

If that's true,  I would say:
 - I don't think we have enough board control systems that we want to support to warrant coming up with our own driver API - so doing something inline in ftc is OK

> Q: does the ttc function look OK to you?
Yes, I think so.

> Q: how do we move from here to the "board management
> abstraction" that you have in mind?

Well, I think our priorities are:
1 - support Fuego users' needs
2 - defragment the industry and move it forward

So, although  we are inventing a new layer (and thus a new API)
for power control, I think that It serves our first priority, which is to
get something that Fuego users need.  More specifically, a boot test
needs to do provisioning, which requires automation of the board
control layer, which requires an API.

One question, it looks like you kept: ov_board_control_reboot.
That is what ftc and one test (Benchmark.reboot) call.

What do you envision a Fuego boot test
(one that does board provisioning) would use:
 - ov_board_control_reboot()?
 - ftc power-cycle 
 - something else?

Is more needed for Fuego to do board provisioning?
ttc has things like 'get_kernel', get_config', 'set_config', 'kbuild',
and 'kinstall', to perform different operations during a kernel CI
loop.  Do you envision that we would add more commands in ftc
for these operations?  Or are you thinking we would implement
these other operations as separate Fuego tests, and string them
together with the equivalent of a batch test? Or is there some other
plan?

As far as the patch goes, I think this one looks pretty good, but I'll
comment on its details separately.
 -- Tim

Re: [Fuego] [PATCH] ftc: add power-control subcommands

[Tim.Bird]

> -----Original Message-----
> From: Daniel Sangorrin
> 
> This is a temporary patch to discuss the implementation of
> power-control on Fuego. The code hasn't been tested at all
> because of a problem when installing Fuego.
> 
> Q: should we use a separate python module?
No.

> Q: should we use a board-control class with on/off/reboot
> operations?

No.

 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  overlays/base/base-board-test-vars.fuegoclass |  12 +-
>  overlays/base/base-board.fuegoclass           |  12 +-
>  scripts/ftc                                   | 118 ++++++++++++++++++
>  scripts/ftc_completion.sh                     |   3 +
>  tests/Functional.fuego_ftc_test/ftc-tests     |  12 +-
>  5 files changed, 124 insertions(+), 33 deletions(-)
> 
> diff --git a/overlays/base/base-board-test-vars.fuegoclass
> b/overlays/base/base-board-test-vars.fuegoclass
> index f834f47..7619608 100644
> --- a/overlays/base/base-board-test-vars.fuegoclass
> +++ b/overlays/base/base-board-test-vars.fuegoclass
It is unfortunate that we now need to modify two files
for what is essentially one set of functions.  That is,
it's going to be a pain to keep base-board.fuegoclass and
base-board-test-vars.fuegoclass synchronized. But that issue
is outside the scope of this thread.  Nice catch that this also
needed to be modified!

> @@ -196,17 +196,7 @@ function ov_transport_cmd() {
> 
>  # function to reboot the board
>  function ov_board_control_reboot() {
> -  if [ -z "$TTC_TARGET" ] ; then
> -    TTC_TARGET="$NODE_NAME"
> -  fi
> -  case "$BOARD_CONTROL" in
> -  "ttc")
> -    $TTC $TTC_TARGET reboot
> -    ;;
> -  *)
> -    abort_job "Error reason: unsupported BOARD_CONTROL
> ${BOARD_CONTROL}"
> -    ;;
> -  esac
> +  ftc power-cycle -b $NODE_NAME
>  }
> 
>  # the following definitions are used to test variable precedence
> diff --git a/overlays/base/base-board.fuegoclass b/overlays/base/base-
> board.fuegoclass
> index ae35464..75bcb82 100644
> --- a/overlays/base/base-board.fuegoclass
> +++ b/overlays/base/base-board.fuegoclass
> @@ -292,15 +292,5 @@ function ov_transport_cmd() {
> 
>  # function to reboot the board
>  function ov_board_control_reboot() {
> -  if [ -z "$TTC_TARGET" ] ; then
> -    TTC_TARGET="$NODE_NAME"
> -  fi
> -  case "$BOARD_CONTROL" in
> -  "ttc")
> -    $TTC $TTC_TARGET reboot
> -    ;;
> -  *)
> -    abort_job "Error reason: unsupported BOARD_CONTROL
> ${BOARD_CONTROL}"
> -    ;;
> -  esac
> +  ftc power-cycle -b $NODE_NAME
We probably need some error handling here.  We just removed
the error handling above.  Maybe move that down here and call
abort_job if 'ftc power-cycle' fails?

>  }
> diff --git a/scripts/ftc b/scripts/ftc
> index b81d511..7ff6d62 100755
> --- a/scripts/ftc
> +++ b/scripts/ftc
> @@ -425,6 +425,33 @@ A message and the exit code indicate if the resource
> was released.
> 
>     ex: ftc release-resource -b beaglebone"""),
> 
> +"power-on": ("Power on the selected board.",
> +    """Usage: ftc power-on -b <board1>[,<board2>...]
> +
> +This command can be used to power on a board. The board file must
> +contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
> +the corresponding options (see README.pdu).
> +
> +Example: ftc power-on -b raspberrypi3"""),
> +
> +"power-off": ("Power off the selected board.",
> +    """Usage: ftc power-off -b <board1>[,<board2>...]
> +
> +This command can be used to power off a board. The board file must
> +contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
> +the corresponding options (see README.pdu).
> +
> +Example: ftc power-off -b raspberrypi3"""),
> +
> +"power-cycle": ("Power cycle the selected board.",

Just out of curiosity - are we establishing with this any conventions
for naming board control functions?  These functions have names
consisting of noun-verb.  Some other ftc functions have names
consisting of verb-noun.  I find this lack of consistency annoying,
but I don't know if anyone else cares.

I think I've discussed this before, but it's a shame that English doesn't
have a single word (a verb) for turning off something.  It's also unfortunate
that in English, 'power' is both a noun and a verb.  Alas, I can't fix English
so maybe I'll just have to get used to a little inconsistency in the ftc
command names.

> +    """Usage: ftc power-off -b <board1>[,<board2>...]
> +
> +This command can be used to power cycle (reboot) a board. The board file
> must
> +contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
> +the corresponding options (see README.pdu).
> +
> +Example: ftc power-cycle -b raspberrypi3"""),
> +
>  "package-test": ("Package a test.",
>      """Usage: ftc package-test <test_name> [options]
> 
> @@ -4951,6 +4978,90 @@ def do_release_resource(conf, options):
> 
>      return 0
> 
> +def do_pdudaemon(command, bvars):
> +    # pdudaemon hostname
> +    if 'PDUDAEMON_HOSTNAME' in bvars:
> +        pdudaemon_hostname = bvars['PDUDAEMON_HOSTNAME']
> +    else:
> +        pdudaemon_hostname = 'localhost'
> +
> +    # pdudaemon port (optional, default 16421)
> +    if 'PDUDAEMON_PORT' in bvars:
> +        pdudaemon_port = bvars['PDUDAEMON_PORT']
> +    else:
> +        pdudaemon_port = '16421'
> +
> +    # pdu hostname
> +    if 'PDU_HOSTNAME' in bvars:
> +        pdu_hostname = bvars['PDU_HOSTNAME']
> +    else:
> +        error_out('PDU_HOSTNAME not specified in board file')
> +
> +    # pdu port
> +    if 'PDU_PORT' in bvars:
> +        pdu_port = bvars['PDU_PORT']
> +    else:
> +        error_out('PDU_PORT not specified in board file')
> +
> +    # pdu delay
> +    if 'PDU_DELAY' in bvars:
> +        pdu_delay = bvars['PDU_PORT']
> +    else:
> +        pdu_delay = 1
Isn't this something pdudaemon should encapsulate?  It seems like this should
be specific to the pdu hardware, and not the board?

In other words, I think we should drop PDU_DELAY.  Thoughts?

> +
> +    cmd_map = {
> +        'power-on' : 'on',
> +        'power-off' : 'off',
> +        'power-cycle' : 'reboot'
> +    }
> +    cmd = cmd_map[command]
> +
> +    query='&'.join(['hostname='+pdu_hostname, 'port='+pdu_port,
> 'delay='+pdu_delay])
> +    url = 'http://%s:%s/power/control/%s?%s' % (pdudaemon_hostname,
> pdudaemon_port, cmd, query)
> +    resp = requests.get(url)
> +    if resp.status_code != 200:
> +        error_out("pdudaemon did not accept the request")
> +
> +    return 0
> +
> +def do_ttc(command, bvars, board_name):
> +    if 'TTC_TARGET' in bvars:
> +        ttc_target = bvars['TTC_TARGET']
> +    else:
> +        ttc_target = board_name
> +
> +    cmd_map = {
> +        'power-on' : 'on',
> +        'power-off' : 'off',
> +        'power-cycle' : 'reboot'
> +    }
> +    cmd = cmd_map[command]
> +
> +    subprocess.check_call('ttc %s %s' % (ttc_target, cmd), shell=True)
> +
> +    return 0
> +
> +def do_power_control(conf, options, command):
> +    bmap = get_fuego_boards(conf)
> +    board_names, options = get_board_arg(command, conf, options, "!m")
> +
> +    for board_name in board_names:
> +        board = bmap[board_name]
> +        bvars = get_board_vars(board, conf)
> +        if 'BOARD_CONTROL' in bvars:
> +            board_control = bvars['BOARD_CONTROL']
> +        else:
> +            error_out("BOARD_CONTROL is not defined for %s", board_name)
> +
> +        if board_control in ["pdudaemon", "PDUDAEMON", "PDUDaemon",
> "PDUdaemon"]:
> +            do_pdudaemon(command, bvars)
> +        elif board_control in ["ttc", "TTC", "Ttc"]:
> +            do_ttc(command, bvars, board_name)
> +        else:
> +            error_out("BOARD_CONTROL value %s is not supported.")
> +
> +    return 0
> +
>  def main():
>      # use global module names
>      global re, time, copy2, subprocess, signal, fcntl, requests, json
> @@ -5244,6 +5355,13 @@ def main():
>          user_check(conf)
>          do_add_view(conf, options)
> 
> +    if command in ["power-on", "power-off", "power-cycle"]:
> +        user_check(conf)
> +        try:
> +            do_power_control(conf, options, command)
> +        except Exception as e:
> +            sys.exit(str(e) + '\n' + command_help[command][1])
> +
>      # all non-board commands have been handled
>      #if command in board_mod_commands:
>      #   check_reservation(bmap, command)
> diff --git a/scripts/ftc_completion.sh b/scripts/ftc_completion.sh
> index 984e003..67b1daf 100755
> --- a/scripts/ftc_completion.sh
> +++ b/scripts/ftc_completion.sh
> @@ -55,6 +55,9 @@ _ftc()
>          ["list-tests"]="-q"
>          ["package-run"]="-o -f"
>          ["package-test"]="-o -f"
> +        ["power-on"]="-b"
> +        ["power-off"]="-b"
> +        ["power-cycle"]="-b"
>          ["put-request"]="-R -s"
>          ["put-run"]=""
>          ["put-test"]=""
> diff --git a/tests/Functional.fuego_ftc_test/ftc-tests
> b/tests/Functional.fuego_ftc_test/ftc-tests
> index 4d38cbc..29865a6 100644
> --- a/tests/Functional.fuego_ftc_test/ftc-tests
> +++ b/tests/Functional.fuego_ftc_test/ftc-tests
> @@ -108,17 +108,7 @@ Test board variable manipulation
>  	                    ZZFOO : "not here"
>  	               board_path : /fuego-ro/boards/ftc-test.board
>  	function_ov_board_control_reboot : function
> ov_board_control_reboot() {
> -	                              if [ -z "$TTC_TARGET" ] ; then
> -	                                TTC_TARGET="$NODE_NAME"
> -	                              fi
> -	                              case "$BOARD_CONTROL" in
> -	                              "ttc")
> -	                                $TTC $TTC_TARGET reboot
> -	                                ;;
> -	                              *)
> -	                                abort_job "Error reason: unsupported
> BOARD_CONTROL ${BOARD_CONTROL}"
> -	                                ;;
> -	                              esac
> +	                              ftc power-cycle -b $NODE_NAME
>  	                            }
> 
>  	  function_ov_board_setup : function ov_board_setup () {
> --
> 2.17.1

OK - I think this is basically fine.  I'm not sure I want to inject it into the
1.5 release at this late stage, but overall this approach is OK.
 -- Tim

Re: [Fuego] [RFC] power-control for Fuego

[daniel.sangorrin]

> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > From: Daniel Sangorrin
[...]
> > Q: how do we move from here to the "board management
> > abstraction" that you have in mind?
> 
> Well, I think our priorities are:
> 1 - support Fuego users' needs
> 2 - defragment the industry and move it forward
> 
> So, although  we are inventing a new layer (and thus a new API)
> for power control, I think that It serves our first priority, which is to
> get something that Fuego users need.  More specifically, a boot test
> needs to do provisioning, which requires automation of the board
> control layer, which requires an API.
> 
> One question, it looks like you kept: ov_board_control_reboot.

Yes, for compatibility reasons and because it can be overwritten on the board which gives some extra flexibility.

> That is what ftc and one test (Benchmark.reboot) call.
> 
> What do you envision a Fuego boot test
> (one that does board provisioning) would use:
>  - ov_board_control_reboot()?
>  - ftc power-cycle
>  - something else?

I think that nested ftc is the most powerful approach. However, I think that tests should be using a single shared shell library. The main advantage is modularity. For example, you could re-implement the functions in the shared shell library to reuse tests in a different framework. The second advantage is that error handling code can be shared. For example, if tests call ov_board_control_reboot, ov_board_control_reboot will take care of ftc power-cycle errors (although my initial patch didn't have that error handling code).

> Is more needed for Fuego to do board provisioning?
> ttc has things like 'get_kernel', get_config', 'set_config', 'kbuild',
> and 'kinstall', to perform different operations during a kernel CI
> loop.  Do you envision that we would add more commands in ftc
> for these operations?  Or are you thinking we would implement
> these other operations as separate Fuego tests, and string them
> together with the equivalent of a batch test? Or is there some other
> plan?

I like the idea of stringing operations into a single shell or python script.

This script would be calling various commands:
  - linux commands: curl, wget, make, ...
  - ftc commands: power-cycle, expect-do on serial port, gen-report, post to squad/fserver/kernelci...

# In LAVA this is done in YAML in a declarative way. But Fuego uses the imperative approach, which I think is less elegant but easier to understand and more flexible.

I think that we want to have ftc commands for some operations and tests for others:
  - ftc commands:
      - power control commands
      - expect-do on serial port: this would be use to wait on the serial port for u-boot and then download the kernel or device tree by writing tftpboot or setenv commands to the serial port.
      - cp a file to or from the board
      - execute a command on the board
      - login to provide an interactive session (e.g. after the test fails)
  - tests:
      - kernel build and deploy: we already support deploying to a local dir, a server or to the target i think..

I would say that build tests are not really essential. They can be done on the single shell script (make -j8) or using a separate build tool (buildbot or jenkins itself). The important thing to me is that the shell script needs to know where the build artifacts are.

> As far as the patch goes, I think this one looks pretty good, but I'll
> comment on its details separately.

OK, thanks.
Daniel

>  -- Tim
> 
> 
> 
> 
> 

Re: [Fuego] [PATCH] ftc: add power-control subcommands

[daniel.sangorrin]

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> Sent: Tuesday, July 9, 2019 5:19 PM
> To: sangorrin daniel(サンゴリン ダニエル ○ＳＷＣ□ＯＳＴ) <daniel.sangorrin@toshiba.co.jp>;
> fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH] ftc: add power-control subcommands
> 
> > -----Original Message-----
> > From: Daniel Sangorrin
> >
> > This is a temporary patch to discuss the implementation of
> > power-control on Fuego. The code hasn't been tested at all
> > because of a problem when installing Fuego.
> >
> > Q: should we use a separate python module?
> No.
> 
> > Q: should we use a board-control class with on/off/reboot
> > operations?
> 
> No.
> 
> 
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > ---
> >  overlays/base/base-board-test-vars.fuegoclass |  12 +-
> >  overlays/base/base-board.fuegoclass           |  12 +-
> >  scripts/ftc                                   | 118 ++++++++++++++++++
> >  scripts/ftc_completion.sh                     |   3 +
> >  tests/Functional.fuego_ftc_test/ftc-tests     |  12 +-
> >  5 files changed, 124 insertions(+), 33 deletions(-)
> >
> > diff --git a/overlays/base/base-board-test-vars.fuegoclass
> > b/overlays/base/base-board-test-vars.fuegoclass
> > index f834f47..7619608 100644
> > --- a/overlays/base/base-board-test-vars.fuegoclass
> > +++ b/overlays/base/base-board-test-vars.fuegoclass
> It is unfortunate that we now need to modify two files
> for what is essentially one set of functions.  That is,
> it's going to be a pain to keep base-board.fuegoclass and
> base-board-test-vars.fuegoclass synchronized. But that issue
> is outside the scope of this thread.  Nice catch that this also
> needed to be modified!
> 
> > @@ -196,17 +196,7 @@ function ov_transport_cmd() {
> >
> >  # function to reboot the board
> >  function ov_board_control_reboot() {
> > -  if [ -z "$TTC_TARGET" ] ; then
> > -    TTC_TARGET="$NODE_NAME"
> > -  fi
> > -  case "$BOARD_CONTROL" in
> > -  "ttc")
> > -    $TTC $TTC_TARGET reboot
> > -    ;;
> > -  *)
> > -    abort_job "Error reason: unsupported BOARD_CONTROL
> > ${BOARD_CONTROL}"
> > -    ;;
> > -  esac
> > +  ftc power-cycle -b $NODE_NAME
> >  }
> >
> >  # the following definitions are used to test variable precedence
> > diff --git a/overlays/base/base-board.fuegoclass b/overlays/base/base-
> > board.fuegoclass
> > index ae35464..75bcb82 100644
> > --- a/overlays/base/base-board.fuegoclass
> > +++ b/overlays/base/base-board.fuegoclass
> > @@ -292,15 +292,5 @@ function ov_transport_cmd() {
> >
> >  # function to reboot the board
> >  function ov_board_control_reboot() {
> > -  if [ -z "$TTC_TARGET" ] ; then
> > -    TTC_TARGET="$NODE_NAME"
> > -  fi
> > -  case "$BOARD_CONTROL" in
> > -  "ttc")
> > -    $TTC $TTC_TARGET reboot
> > -    ;;
> > -  *)
> > -    abort_job "Error reason: unsupported BOARD_CONTROL
> > ${BOARD_CONTROL}"
> > -    ;;
> > -  esac
> > +  ftc power-cycle -b $NODE_NAME
> We probably need some error handling here.  We just removed
> the error handling above.  Maybe move that down here and call
> abort_job if 'ftc power-cycle' fails?

Yes, I also noticed about this after sending the patch. I will fix that.
 
> >  }
> > diff --git a/scripts/ftc b/scripts/ftc
> > index b81d511..7ff6d62 100755
> > --- a/scripts/ftc
> > +++ b/scripts/ftc
> > @@ -425,6 +425,33 @@ A message and the exit code indicate if the resource
> > was released.
> >
> >     ex: ftc release-resource -b beaglebone"""),
> >
> > +"power-on": ("Power on the selected board.",
> > +    """Usage: ftc power-on -b <board1>[,<board2>...]
> > +
> > +This command can be used to power on a board. The board file must
> > +contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
> > +the corresponding options (see README.pdu).
> > +
> > +Example: ftc power-on -b raspberrypi3"""),
> > +
> > +"power-off": ("Power off the selected board.",
> > +    """Usage: ftc power-off -b <board1>[,<board2>...]
> > +
> > +This command can be used to power off a board. The board file must
> > +contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
> > +the corresponding options (see README.pdu).
> > +
> > +Example: ftc power-off -b raspberrypi3"""),
> > +
> > +"power-cycle": ("Power cycle the selected board.",
> 
> Just out of curiosity - are we establishing with this any conventions
> for naming board control functions?  These functions have names
> consisting of noun-verb.  Some other ftc functions have names
> consisting of verb-noun.  I find this lack of consistency annoying,
> but I don't know if anyone else cares.

I am using "power on/off/cycle" as verbs (like a phrasal verb).
If you prefer verb-noun then it would be something like "poweron-board".
I wanted them to start with "power-" because that is the "subsystem" we are trying to use, and TAB will give you the available subcommands.

> I think I've discussed this before, but it's a shame that English doesn't
> have a single word (a verb) for turning off something.  It's also unfortunate
> that in English, 'power' is both a noun and a verb.  Alas, I can't fix English
> so maybe I'll just have to get used to a little inconsistency in the ftc
> command names.

"I suppose life is irregular" (Game of Thrones pun)

I guess we could use ftc on/off/reboot.
ftc on -b board <-- power on
ftc off -b board <-- power off
ftc reboot -b board

Or alternatively, we can use Spanish for power-on/off and reboot instead of power-cycle:

ftc encender -b board <-- power on
ftc apagar -b board <-- power off
ftc reboot -b board

> > +    """Usage: ftc power-off -b <board1>[,<board2>...]
> > +
> > +This command can be used to power cycle (reboot) a board. The board file
> > must
> > +contain a definition for BOARD_CONTROL (e.g., pdudaemon or ttc) and
> > +the corresponding options (see README.pdu).
> > +
> > +Example: ftc power-cycle -b raspberrypi3"""),
> > +
> >  "package-test": ("Package a test.",
> >      """Usage: ftc package-test <test_name> [options]
> >
> > @@ -4951,6 +4978,90 @@ def do_release_resource(conf, options):
> >
> >      return 0
> >
> > +def do_pdudaemon(command, bvars):
> > +    # pdudaemon hostname
> > +    if 'PDUDAEMON_HOSTNAME' in bvars:
> > +        pdudaemon_hostname = bvars['PDUDAEMON_HOSTNAME']
> > +    else:
> > +        pdudaemon_hostname = 'localhost'
> > +
> > +    # pdudaemon port (optional, default 16421)
> > +    if 'PDUDAEMON_PORT' in bvars:
> > +        pdudaemon_port = bvars['PDUDAEMON_PORT']
> > +    else:
> > +        pdudaemon_port = '16421'
> > +
> > +    # pdu hostname
> > +    if 'PDU_HOSTNAME' in bvars:
> > +        pdu_hostname = bvars['PDU_HOSTNAME']
> > +    else:
> > +        error_out('PDU_HOSTNAME not specified in board file')
> > +
> > +    # pdu port
> > +    if 'PDU_PORT' in bvars:
> > +        pdu_port = bvars['PDU_PORT']
> > +    else:
> > +        error_out('PDU_PORT not specified in board file')
> > +
> > +    # pdu delay
> > +    if 'PDU_DELAY' in bvars:
> > +        pdu_delay = bvars['PDU_PORT']
> > +    else:
> > +        pdu_delay = 1
> Isn't this something pdudaemon should encapsulate?  It seems like this should
> be specific to the pdu hardware, and not the board?
> In other words, I think we should drop PDU_DELAY.  Thoughts?

Good point. 
Theoretically yes. But PDUDaemon has this interface. I suppose that there is a reason for that. 
Also, I guess it's better to have this _optional_ option, rather than having nothing. Maybe the default should be 0 instead of 1s.

> > +
> > +    cmd_map = {
> > +        'power-on' : 'on',
> > +        'power-off' : 'off',
> > +        'power-cycle' : 'reboot'
> > +    }
> > +    cmd = cmd_map[command]
> > +
> > +    query='&'.join(['hostname='+pdu_hostname, 'port='+pdu_port,
> > 'delay='+pdu_delay])
> > +    url = 'http://%s:%s/power/control/%s?%s' % (pdudaemon_hostname,
> > pdudaemon_port, cmd, query)
> > +    resp = requests.get(url)
> > +    if resp.status_code != 200:
> > +        error_out("pdudaemon did not accept the request")
> > +
> > +    return 0
> > +
> > +def do_ttc(command, bvars, board_name):
> > +    if 'TTC_TARGET' in bvars:
> > +        ttc_target = bvars['TTC_TARGET']
> > +    else:
> > +        ttc_target = board_name
> > +
> > +    cmd_map = {
> > +        'power-on' : 'on',
> > +        'power-off' : 'off',
> > +        'power-cycle' : 'reboot'
> > +    }
> > +    cmd = cmd_map[command]
> > +
> > +    subprocess.check_call('ttc %s %s' % (ttc_target, cmd), shell=True)
> > +
> > +    return 0
> > +
> > +def do_power_control(conf, options, command):
> > +    bmap = get_fuego_boards(conf)
> > +    board_names, options = get_board_arg(command, conf, options, "!m")
> > +
> > +    for board_name in board_names:
> > +        board = bmap[board_name]
> > +        bvars = get_board_vars(board, conf)
> > +        if 'BOARD_CONTROL' in bvars:
> > +            board_control = bvars['BOARD_CONTROL']
> > +        else:
> > +            error_out("BOARD_CONTROL is not defined for %s", board_name)
> > +
> > +        if board_control in ["pdudaemon", "PDUDAEMON", "PDUDaemon",
> > "PDUdaemon"]:
> > +            do_pdudaemon(command, bvars)
> > +        elif board_control in ["ttc", "TTC", "Ttc"]:
> > +            do_ttc(command, bvars, board_name)
> > +        else:
> > +            error_out("BOARD_CONTROL value %s is not supported.")
> > +
> > +    return 0
> > +
> >  def main():
> >      # use global module names
> >      global re, time, copy2, subprocess, signal, fcntl, requests, json
> > @@ -5244,6 +5355,13 @@ def main():
> >          user_check(conf)
> >          do_add_view(conf, options)
> >
> > +    if command in ["power-on", "power-off", "power-cycle"]:
> > +        user_check(conf)
> > +        try:
> > +            do_power_control(conf, options, command)
> > +        except Exception as e:
> > +            sys.exit(str(e) + '\n' + command_help[command][1])
> > +
> >      # all non-board commands have been handled
> >      #if command in board_mod_commands:
> >      #   check_reservation(bmap, command)
> > diff --git a/scripts/ftc_completion.sh b/scripts/ftc_completion.sh
> > index 984e003..67b1daf 100755
> > --- a/scripts/ftc_completion.sh
> > +++ b/scripts/ftc_completion.sh
> > @@ -55,6 +55,9 @@ _ftc()
> >          ["list-tests"]="-q"
> >          ["package-run"]="-o -f"
> >          ["package-test"]="-o -f"
> > +        ["power-on"]="-b"
> > +        ["power-off"]="-b"
> > +        ["power-cycle"]="-b"
> >          ["put-request"]="-R -s"
> >          ["put-run"]=""
> >          ["put-test"]=""
> > diff --git a/tests/Functional.fuego_ftc_test/ftc-tests
> > b/tests/Functional.fuego_ftc_test/ftc-tests
> > index 4d38cbc..29865a6 100644
> > --- a/tests/Functional.fuego_ftc_test/ftc-tests
> > +++ b/tests/Functional.fuego_ftc_test/ftc-tests
> > @@ -108,17 +108,7 @@ Test board variable manipulation
> >  	                    ZZFOO : "not here"
> >  	               board_path : /fuego-ro/boards/ftc-test.board
> >  	function_ov_board_control_reboot : function
> > ov_board_control_reboot() {
> > -	                              if [ -z "$TTC_TARGET" ] ; then
> > -	                                TTC_TARGET="$NODE_NAME"
> > -	                              fi
> > -	                              case "$BOARD_CONTROL" in
> > -	                              "ttc")
> > -	                                $TTC $TTC_TARGET reboot
> > -	                                ;;
> > -	                              *)
> > -	                                abort_job "Error reason: unsupported
> > BOARD_CONTROL ${BOARD_CONTROL}"
> > -	                                ;;
> > -	                              esac
> > +	                              ftc power-cycle -b $NODE_NAME
> >  	                            }
> >
> >  	  function_ov_board_setup : function ov_board_setup () {
> > --
> > 2.17.1
> 
> OK - I think this is basically fine.  I'm not sure I want to inject it into the
> 1.5 release at this late stage, but overall this approach is OK.
>  -- Tim

OK, thank you. I will give it some testing and resend.
I think it doesn't affect other code so including it on the next branch would be harmless
Thanks,
Daniel

Re: [Fuego] [PATCH] ftc: add power-control subcommands

[daniel.sangorrin]

> From: daniel.sangorrin@toshiba.co.jp
> > OK - I think this is basically fine.  I'm not sure I want to inject it into the
> > 1.5 release at this late stage, but overall this approach is OK.
> >  -- Tim
> 
> OK, thank you. I will give it some testing and resend.
> I think it doesn't affect other code so including it on the next branch would be harmless

The initial non-tested patch had a couple of bugs but other than that it works. I have sent you the patches for inclusion. They are on my repository (next branch) as well.

If you want to change the naming, please do. I have no strong opinions on that.

Thanks,
Daniel

Re: [Fuego] [PATCH] ftc: add power-control subcommands

[Tim.Bird]

> -----Original Message-----
> From: daniel.sangorrin@toshiba.co.jp
> 
> > From: daniel.sangorrin@toshiba.co.jp
> > > OK - I think this is basically fine.  I'm not sure I want to inject it into the
> > > 1.5 release at this late stage, but overall this approach is OK.
> > >  -- Tim
> >
> > OK, thank you. I will give it some testing and resend.
> > I think it doesn't affect other code so including it on the next branch would
> be harmless
> 
> The initial non-tested patch had a couple of bugs but other than that it works.
> I have sent you the patches for inclusion. They are on my repository (next
> branch) as well.
> 
> If you want to change the naming, please do. I have no strong opinions on
> that.

I went ahead and applied them.  I decided to leave the naming as is.
The names are easily understood, which I think overrides my desire
for strict consistency.

"A foolish consistency is the hobgoblin of little minds" - said Ralph Waldo Emmerson.
And who am I to argue with him. :-)
 -- Tim