Merge lp:~nijaba/charm-tools/peer-replay into lp:~charmers/charm-tools/trunk

[Nick Barcet]

Adds 3 new features:
 * the possibility to do a ch_peer_[rsync|scp] outside of a peer relation (typically in a config-changed event) once at least one copy has been done within the peer relation. The copy will be done from the leader to all hosts to which it has done copies to previously
 * the possibility to do a ch_peer_copy_replay which will redo all copies (scp or rsync) done previously to all hosts (typically in a config-changed event)
 * ch_peer_copy_cleanup <remote-unit> which should be called every time there is a peer-relation-departed event received to remove the unit from the host cache.

A big commit which adds:
 * the possibility to do a ch_peer_[rsync|scp] outside of a peer relation (typically in a config-changed event) once at least one copy has been done within the peer relation. The copy will be done from the leader to all hosts to which it has done copies to previously
 * the possibility to do a ch_peer_copy_replay which will redo all copies (scp or rsync) done previously to all hosts (typically in a config-changed event)
 * ch_peer_copy_cleanup <remote-unit> which should be called every time there is a peer-relation-departed event received to remove the unit from the host cache.

This branch assumes that lp:~nijaba/charm-tools/buildd-fixes has been merged already.

[lp:~nijaba/charm-tools/peer-replay]

112. By Nick Barcet on 2011-12-23

merging with buildd-fixes-part-deux

113. By Nick Barcet on 2011-12-23

fixes after API change

[Nick Barcet (nijaba)]

Now using the new API and changes from buildd-fixes-part-deux

review: Resubmit

[lp:~nijaba/charm-tools/peer-replay]

114. By Nick Barcet on 2012-01-04

ensure proper return results

115. By Nick Barcet on 2012-01-04

rebasing from trunk

116. By Nick Barcet on 2012-01-04

juju sets home to /home/ubuntu while user is root :(

117. By Nick Barcet on 2012-01-04

better test of ch_peer_copy_new

118. By Nick Barcet on 2012-01-04

need to replace all instances of our host place holder...

119. By Nick Barcet on 2012-01-05

deal with shell expansion better, home dir and multiple files

120. By Nick Barcet on 2012-01-05

unbound variable testing and wrongly place local fix

121. By Nick Barcet on 2012-01-05

did not know -z was not protecting with set -u

122. By Nick Barcet on 2012-01-05

resync with trunk

[Nick Barcet (nijaba)]

Ok, this branch is now working great.

[Clint Byrum (clint-fewbar)]

Hi Nick! Some blockers first:

[0]

+set -eu

Don't mess with this in helpers. There's never a reason to turn it off inside your helpers either. The script author needs to make this distinction knowingly, otherwise their script may not be able to handle the extra rigor. That said, all tests should set -eu.

[1]

Changing PermitRootLogin seems like overstepping our bounds in a charm helper. Perhaps raise an error if its attempted, and a separate ch_sshd_config_set PermitRootLogin yes call would be better. I'm worried that users will be upset that we so willingly gave all boxes root on all other boxes. Another way to go is to just not allow this to be done at all, and always require the use of a non-root user for transfers... which is what I would prefer actually.

[2]

97 + set +f
198 + eval "$copy_command $scp_options $p1 $path"
199 + set -f

What if it was already +f? Now you just turned globbing back on.

Also why are you doing eval?

$copy_command $scp_options $p1 $path

should work fine. Also its not clear *why* you don't want globbing, so a comment would be useful here.

[3]

546 +ch_peer_rsync -p $CH_portnum -o "-azq" "$CH_TEMPDIR/sourcedir/*" "$CH_TEMPDIR/destdir/" ||
547 +chres=$?

If ch_peer_rsync returns 0, this will move on and fail because of the set -u, because the || is still open so we are saying "run ch_peer_rsync, OR chres=$?".

I would think the safest way to achieve what I think is the desired logic would be

if ch_peer_rsync ... ; then
  chres=0
else
  chres=$?
fi

Those are the blockers. The rest are nit picks, but should be fixed:

[4]

+ juju-log "_ch_peer_copy_set_paths"

Guessing you forgot -l DEBUG

[5]

+ if [ ! x`echo "$HOME" | grep "$USER"` = x"$HOME" ] ; then

use x$VAR or "$VAR" .. not both. The x is used when you don't want to use quotes for some reason.

[6]

+ if [ -z "$list" ] || [ x"$list" = x"" ] ; then

These conditions are redundant..

[7]

+ if [ ! -n "$copies" ] ; then

! -n is better expressed as -z

[8]

ch_peer_copy_cleanup is going to return the result of the sed command. Best add a 'return 0' if you want to ignore that result.

review: Needs Fixing

[Nick Barcet (nijaba)]

> [0]
>
> +set -eu
>
> Don't mess with this in helpers. There's never a reason to turn it off inside
> your helpers either. The script author needs to make this distinction
> knowingly, otherwise their script may not be able to handle the extra rigor.
> That said, all tests should set -eu.

fixed, thanks

> [1]
>
> Changing PermitRootLogin seems like overstepping our bounds in a charm helper.
> Perhaps raise an error if its attempted, and a separate ch_sshd_config_set
> PermitRootLogin yes call would be better. I'm worried that users will be upset
> that we so willingly gave all boxes root on all other boxes. Another way to go
> is to just not allow this to be done at all, and always require the use of a
> non-root user for transfers... which is what I would prefer actually.

ok, added ch_sshd_set_root_login [0|1] function and a warning when user is root that encourages to use another user.

Small question: how would you use another user in a hook? use 'sudo ubuntu' in front of the command?

> [2]
>
> 97 + set +f
> 198 + eval "$copy_command $scp_options $p1 $path"
> 199 + set -f
>
> What if it was already +f? Now you just turned globbing back on.

You are absolutely right ! I now store the state and restore it at the end.

> Also why are you doing eval?
>
> $copy_command $scp_options $p1 $path
>
> should work fine.

Agree that it should but it is giving me a" Missing trailing-' in remote-shell command." when running the ch_peer_rsync test. This has something to do with the quote interpretation which I could only solve by using eval. suggestion welcome.

> Also its not clear *why* you don't want globbing, so a
> comment would be useful here.

I need to turn off globbing so that the $paths variable is not expanded when on the paths ends with a * for example, as my loop expect pairs of paths and this would be messed up otherwise. I added a comment to explain this.

> [3]
>
> 546 +ch_peer_rsync -p $CH_portnum -o "-azq" "$CH_TEMPDIR/sourcedir/*"
> "$CH_TEMPDIR/destdir/" ||
> 547 +chres=$?
>
> If ch_peer_rsync returns 0, this will move on and fail because of the set -u,
> because the || is still open so we are saying "run ch_peer_rsync, OR
> chres=$?".
>
> I would think the safest way to achieve what I think is the desired logic
> would be
>
> if ch_peer_rsync ... ; then
> chres=0
> else
> chres=$?
> fi

fixed, thanks

> Those are the blockers. The rest are nit picks, but should be fixed:
>
> [4]
>
> + juju-log "_ch_peer_copy_set_paths"
>
> Guessing you forgot -l DEBUG

This is a nice way to say I did not even know it existed....

> [5]
>
> + if [ ! x`echo "$HOME" | grep "$USER"` = x"$HOME" ] ; then
>
> use x$VAR or "$VAR" .. not both. The x is used when you don't want to use
> quotes for some reason.

Not according to http://stackoverflow.com/questions/174119/why-do-shell-script-comparisons-often-use-xvar-xyes and other refs I have ran across, but I agree this might be an overkill here.

> [6]
>
>
> + if [ -z "$list" ] || [ x"$list" = x"" ] ; then
>
> These conditions are redundant..

fixed, thanks. bad understanding of -z on my side, ttytt.

> [7]
>
> + if [...

[lp:~nijaba/charm-tools/peer-replay]

123. By <email address hidden> on 2012-01-06

address comments from https://code.launchpad.net/~nijaba/charm-tools/peer-replay/+merge/86721/comments/189159

124. By Nick Barcet on 2012-01-06

adapt mock_juju_log to print all params received

125. By Nick Barcet on 2012-01-06

some logic and typo fixes

[Nick Barcet (nijaba)]

and now tested with multiple units

review: Resubmit

[lp:~nijaba/charm-tools/peer-replay]

126. By Nick Barcet on 2012-01-06

I thought I add removed this set -eu earlier...

127. By Nick Barcet on 2012-01-09

uneeded debug

128. By Nick Barcet on 2012-01-09

we have to assume that it's a file, even if it does not (yet) exist for future replay

129. By <email address hidden> on 2012-01-09

should not assume relation-list is sorted

130. By Nick Barcet on 2012-01-09

better logic and tests for peer leader functions

[Clint Byrum (clint-fewbar)]

Looks good Nick, please merge with a single commit message. :) Just so I'm clear what I mean:

bzr branch lp:charm-tools
cd charm-tools
bzr merge lp:~nijaba/charm-tools/peer-replay
bzr commit -m 'Adding peer-replay capability'
bzr push lp:charm-tools

review: Approve

=== modified file 'helpers/sh/peer.sh'
--- helpers/sh/peer.sh	2012-01-04 18:35:46 +0000
+++ helpers/sh/peer.sh	2012-01-09 18:47:25 +0000
@@ -26,13 +26,24 @@
 {
     local REMOTE_UNIT_ID=`ch_unit_id $JUJU_REMOTE_UNIT`
     local LOCAL_UNIT_ID=`ch_my_unit_id`
-    local FIRST_UNIT=`relation-list | head -n 1`
-    local FIRST_UNIT_ID=`ch_unit_id $FIRST_UNIT`
-
-    if [ $LOCAL_UNIT_ID -lt $REMOTE_UNIT_ID ] && [ $LOCAL_UNIT_ID -lt $FIRST_UNIT_ID ]; then
-        return 0
+    local FIRST_UNIT_ID=`relation-list | cut -d/ -f2 | sort -n | head -n 1`
+    if [ -z $FIRST_UNIT_ID ] ; then
+      #no one else in the list, we are obviously the leader
+      return 0
+    fi
+    if [ $REMOTE_UNIT_ID -lt $FIRST_UNIT_ID ] ; then
+      #remote must be departing, lets not worry about it
+      if [ $LOCAL_UNIT_ID -lt $FIRST_UNIT_ID ]; then
+          return 0
+      else
+          return 1
+      fi
     else
-        return 1
+      if [ $LOCAL_UNIT_ID -lt $REMOTE_UNIT_ID ] && [ $LOCAL_UNIT_ID -lt $FIRST_UNIT_ID ]; then
+          return 0
+      else
+          return 1
+      fi
     fi
 } 
 
@@ -48,15 +59,17 @@
 {
     if ch_peer_i_am_leader; then
         # this is the leader, return our own unit name
-        local leader="$JUJU_UNIT_NAME"
+        local leader_id=`ch_unit_id $JUJU_UNIT_NAME`
+        local leader_name=$JUJU_UNIT_NAME
     else
         # this is  a slave the leader is the head of the list
-        local leader="`relation-list | head -n 1`"
+        local leader_id=`relation-list | cut -d/ -f2 | sort -n | head -n 1`
+        local leader_name=`relation-list | grep "/$leader_id$"`
     fi
     if [ $# -gt 0 ] && [ "$1" = "--id" ]; then
-        echo "`ch_unit_id $leader`"
+        echo $leader_id
     else
-        echo "$leader"
+        echo $leader_name
     fi
 }
 
@@ -106,6 +119,7 @@
 #  0 when the file is copied
 #  1 when there was an error
 #  100 when the file is not copied
+#  101 files were sent from the leader
 #
 # This executes in multiple passes between the leader and each peer
 #
@@ -126,97 +140,137 @@
   local USAGE="ERROR in $*
 USAGE: ch_peer_scp [-r][-p <port>][-o \"<opt>\"] sourcepath1 destpath1 [... sourcepathN destpathN]
 USAGE: ch_peer_rsync [-p <port>][-o \"<opt>\"] sourcepath1 destpath1 [... sourcepathN destpathN]"
-  if [ x"$USER" = x"root" ] ; then
-    #juju sets $HOME to /home/ubuntu while user is root :(
-    local ssh_key_p="/root/.ssh"
-  else
-    local ssh_key_p="$HOME/.ssh"
-  fi
+  # $USER may not be set
+  USER=${USER:-`whoami`}
+  # $HOME may not be set
+  if [ -z "${HOME:-}" ] ; then
+    if [ "$USER" = "root" ] ; then 
+      HOME="/$USER"
+    else
+      HOME="/home/$USER"
+    fi  
+  fi
+  if ! echo "$HOME" | grep -q "$USER" ; then 
+    if [ "$USER" = "root" ] ; then 
+      HOME="/$USER"
+    else
+      HOME="/home/$USER"
+    fi  
+  fi
+  local ssh_key_p="$HOME/.ssh"
   local result=100
   
   if [ $# -eq 0 ]; then
-    juju-log "$USAGE"
-    juju-log "ch_peer_copy: please provide at least one argument (path)"
+    juju-log -l ERROR "$USAGE"
+    juju-log -l ERROR "ch_peer_copy: please provide at least one argument (path)"
     return 1
   fi
+  
+  local scp_options="-o StrictHostKeyChecking=no -B"
+  local rsync_options=""
+  local paths=""
+  local copy_command="scp"
+  
+  while [ "$#" -gt 0 ]; 
+  do
+    case "$1" in
+    "-r") # scp recure
+      scp_options="$scp_options -r"
+      shift
+      ;;
+    "-p") # port number
+      shift
+      scp_options="$scp_options -P $1"
+      rsync_options="$rsync_options -e 'ssh -p $1 -o StrictHostKeyChecking=no'"
+      shift
+      ;;
+    "-o") # passthrough option
+      shift
+      scp_options="$scp_options $1"
+      rsync_options="$rsync_options $1"
+
+      shift
+      ;;
+    "--rsync") # rsync secure (-e ssh)
+      copy_command="rsync"
+      shift
+      ;;
+    "$0")
+      shift
+      ;;
+    *) # should be a pair of file
+      local sourcep="$1"
+      shift
+      paths="$paths $sourcep $USER@X0X0X0X0:$1"
+      juju-log -l DEBUG "ch_peer_copy: paths found: $sourcep -> $1"
+      shift
+      ;;
+    esac
+  done
+  if [ -z "$paths" ]; then
+    juju-log -l ERROR "$USAGE"
+    juju-log -l ERROR "ch_peer_copy: please provide at least one path"
+    exit 1
+  fi
+  if [ x"$copy_command" = x"rsync" ]; then
+    scp_options=$rsync_options
+  fi
+  
+  local list=""
+  list=`relation-list`
+  # if we are not in a relation, do ch_peer_copy_new
+  if [ -z "${list:-}" ] ; then
+    _ch_peer_copy_new "$copy_command $scp_options" "$paths"
+    result=$?
+    juju-log -l DEBUG "ch_peer_copy: returning $result"
+    return $result
+  fi
 
   ## LEADER ##
   
   if ch_peer_i_am_leader ; then
-    juju-log "ch_peer_copy: This is our leader"
+    juju-log -l INFO "ch_peer_copy: This is our leader"
     
     local remote=`relation-get scp-hostname`
     local ssh_key_saved=`relation-get scp-ssh-key-saved`
-    if [ -z $remote ]; then
-      juju-log "ch_peer_copy: We do not have a remote hostname yet"
+    if [ -z $remote ] || [ "$remote" = "0" ] ; then
+      juju-log -l DEBUG "ch_peer_copy: We do not have a remote hostname yet"
+      relation-set scp-copy-done=0
       remote=0
     fi  
-  
-    local scp_options="-o StrictHostKeyChecking=no -B"
-    local rsync_options=""
-    local paths=""
-    local copy_command="scp"
-    
-    while [ "$#" -gt 0 ]; 
-    do
-      case "$1" in
-      "-r") # scp recure
-        scp_options="$scp_options -r"
-        shift
-        ;;
-      "-p") # port number
-        shift
-        scp_options="$scp_options -P $1"
-        rsync_options="$rsync_options -e 'ssh -p $1 -o StrictHostKeyChecking=no'"
-        shift
-        ;;
-      "-o") # passthrough option
-        shift
-        scp_options="$scp_options $1"
-        rsync_options="$rsync_options $1"
-        shift
-        ;;
-      "--rsync") # rsync secure (-e ssh)
-        copy_command="rsync"
-        shift
-        ;;
-      "$0")
-        shift
-        ;;
-      *) # should be a pair of file
-        if [ -e `echo "$1" | sed 's/\*$//'` ]; then
-          local sourcep="$1"
-          shift
-          paths="$paths $sourcep $USER@$remote:$1"
-          juju-log "ch_peer_copy: paths found: $sourcep -> $1"
-        else
-          juju-log "ch_peer_copy: unknown option, skipping: $1"
-        fi
-        shift
-        ;;
-      esac
-    done
-    if [ ! -n "$paths" ]; then
-      juju-log "$USAGE"
-      juju-log "ch_peer_copy: please provide at least one path"
-      return 1
-    fi
     
     if [ -n $remote ]; then
       # We know where to send file to
       
       case $ssh_key_saved in
-      1) # ssh keys have been save, let's copy
-        if [ x"$copy_command" = x"rsync" ]; then
-          scp_options="$rsync_options"
-        fi
-        juju-log "ch_peer_copy: $copy_command $scp_options $paths"
-        eval "$copy_command $scp_options $paths"
+      1) # ssh keys have been saved, let's copy
+        paths=`echo "$paths" | sed "s/X0X0X0X0/$remote/g"`
+        local p1=""
+        #we do not want globbing in the loop as it would mess with the number of passes and we work on pairs
+        local globstate=`set -o | grep "noglob" | grep -q off && echo "+f" || echo "-f"`
+        set -f
+        for path in $paths ; do 
+          if [ "$p1" != "" ] ; then
+            juju-log -l INFO  "ch_peer_copy: $copy_command $scp_options $p1 $path"
+            #globbing is wanted here to handle 
+            set +f
+            eval "$copy_command $scp_options $p1 $path"
+            set -f
+            p1=""
+          else
+            p1=$path
+          fi
+        done
+        set $globstate
         relation-set scp-copy-done=1
+        #save host and paths for later use
+        _ch_peer_copy_save "$copy_command" "$scp_options" "$remote" "$paths" "$JUJU_REMOTE_UNIT"
+        juju-log -l DEBUG "ch_peer_copy: save done"
+        result=101
         ;;
         
       *) # we need to first distribute our ssh key files
-        juju-log "ch_peer_copy: distributing ssh key"
+        juju-log -l DEBUG "ch_peer_copy: distributing ssh key"
         if [ ! -f "$ssh_key_p/id_rsa" ]; then
           ssh-keygen -q -N '' -t rsa -b 2048 -f $ssh_key_p/id_rsa
         fi
@@ -229,13 +283,14 @@
   ## REMOTE ##
       
   else # Not the leader
-    juju-log "ch_peer_copy: This is a slave"
+    juju-log -l INFO "ch_peer_copy: This is a slave"
  
     local scp_copy_done=`relation-get scp-copy-done`
     local scp_ssh_key="`relation-get scp-ssh-key`"
+    relation-set scp-hostname=`unit-get private-address`
  
     if [ -n "$scp_copy_done" ] && [ $scp_copy_done = 1 ]; then
-      juju-log "ch_peer_copy: copy done, thanks"
+      juju-log -l DEBUG "ch_peer_copy: copy done, thanks"
       result=0
     else
       if [ -n "$scp_ssh_key" ]; then
@@ -243,21 +298,259 @@
         mkdir -p $ssh_key_p
         chmod 700 $ssh_key_p
         if ! grep -q -F "$scp_ssh_key" $ssh_key_p/authorized_keys ; then
-          juju-log "ch_peer_copy: saving ssh key $scp_ssh_key"
+          if [ "$USER" = "root" ] ; then
+            # if we are root, need to ensure we can login
+            if ! grep -q "^PermitRootLogin yes" /etc/ssh/sshd_config ; then
+              juju-log -l ERROR "PermitRootLogin is not set to yes and you are root. Please use another user or call 'ch_sshd_set_root_login 1' first"
+              return 1
+            fi
+          fi
+          juju-log -l DEBUG "ch_peer_copy: saving ssh key $scp_ssh_key"
           echo "$scp_ssh_key" >> $ssh_key_p/authorized_keys
           relation-set scp-ssh-key-saved=1
         else
-          juju-log "ch_peer_copy: ssh keys already saved, thanks"
+          juju-log -l DEBUG "ch_peer_copy: ssh keys already saved, thanks"
           relation-set scp-ssh-key-saved=1
         fi
         chmod 600 "$ssh_key_p/authorized_keys"
       else
-        juju-log "ch_peer_copy: ssh_keys not set yet, later"
-        relation-set scp-hostname=`unit-get private-address`
+        juju-log -l DEBUG "ch_peer_copy: ssh_keys not set yet, later"
       fi 
     fi 
   fi 
+
   juju-log "ch_peer_copy: returning: $result"
   return $result
 }
 
+##
+# ch_sshd_set_root_login [0|1]
+#
+# modify /etc/ssh/sshd_config to set PermitRootLogin and reload ssh server
+#
+# param
+#  0 disable
+#  1 enable (default)
+#
+# returns nothing
+
+ch_sshd_set_root_login() {
+  local on=${1:-1}
+  if [ $on = 1 ] ; then
+    if ! grep -q "^PermitRootLogin yes" /etc/ssh/sshd_config ; then
+      sed -i 's/PermitRootLogin no/PermitRootLogin yes/' /etc/ssh/sshd_config
+      service ssh reload
+    fi
+  else  
+    if ! grep -q "^PermitRootLogin no" /etc/ssh/sshd_config ; then
+      sed -i 's/PermitRootLogin yes/PermitRootLogin no/' /etc/ssh/sshd_config
+      service ssh reload
+    fi
+  fi
+  return 0
+}
+
+_ch_peer_copy_set_paths() {
+  juju-log -l DEBUG "_ch_peer_copy_set_paths"
+  local unitname=""
+  unitname=`echo $JUJU_UNIT_NAME | sed 's/\//-/g'`
+  CH_PEER_COPY_P="/var/lib/juju/$unitname"
+  if [ ! `mkdir -p $CH_PEER_COPY_P 2>/dev/null` ] ; then
+    CH_PEER_COPY_P="$HOME/ch_test/$unitname"
+    mkdir -p $CH_PEER_COPY_P
+  fi
+  CH_PEER_COPY_HOST_F="$CH_PEER_COPY_P/ch_peer_copy_hosts"
+  CH_PEER_COPY_PATHS_F="$CH_PEER_COPY_P/ch_peer_copy_paths"
+}
+
+# Save copy commands and host for later replay
+_ch_peer_copy_save() {
+  # $1 copy_command
+  # $2 options
+  # $3 remote host
+  # $4 paths
+  # $5 remote unit name
+  
+  juju-log -l DEBUG "_ch_peer_copy_save: $*"
+  
+  _ch_peer_copy_set_paths
+  
+  #save paths
+  local suffix=`echo "$1%$2" | base64 -w 0 `
+  local paths=`echo "$4" | base64 -w 0`
+  local do_save=1
+  
+  juju-log -l DEBUG "_ch_peer_copy_save: $* in ${CH_PEER_COPY_PATHS_F}-$suffix"
+  
+  if [ -e "${CH_PEER_COPY_PATHS_F}-$suffix" ]; then
+    if grep -q -F "$paths" "${CH_PEER_COPY_PATHS_F}-$suffix"  ; then
+      do_save=0
+    fi
+  fi
+  if [ $do_save = 1 ] ; then
+    juju-log -l DEBUG "_ch_peer_copy_save: saving $paths in ${CH_PEER_COPY_PATHS_F}-$suffix"
+    echo "$paths" >> ${CH_PEER_COPY_PATHS_F}-$suffix
+    juju-log -l DEBUG "_ch_peer_copy_save: paths: saved"
+  fi
+  do_save=1
+  
+  
+  #save hosts
+  if [ -e "${CH_PEER_COPY_HOST_F}" ]; then
+    if grep -q -F "$3" "${CH_PEER_COPY_HOST_F}" ; then
+      do_save=0
+    fi
+  fi
+  [ $do_save = 1 ] && echo "$5 $3" >> $CH_PEER_COPY_HOST_F
+  juju-log -l DEBUG "_ch_peer_copy_save: host: $5 $3"
+}
+
+# do a new copy when not called within a relation
+# returns
+#  1 when there was an error
+#  100 when the file is not copied
+#  101 when the file is sent from server
+
+_ch_peer_copy_new() {
+  # $1 "$copy_command $scp_options" 
+  # $2 "$paths"
+  
+  _ch_peer_copy_set_paths
+  local result=100
+  local hosts=""
+  local paths=""
+  
+  if [ -e "$CH_PEER_COPY_HOST_F" ] ; then
+    hosts=`cat $CH_PEER_COPY_HOST_F | cut -d" " -f2`
+    
+    for h in $hosts ;
+    do
+      paths=`echo "$2" | sed "s/X0X0X0X0/$h/"`
+      local p1=""
+      #we do not want globbing in the loop as it would mess with the number of passes and we work on pairs
+      local globstate=`set -o | grep "noglob" | grep -q off && echo "+f" || echo "-f"`
+      set -f
+      for path in $paths ; do 
+        if [ "$p1" != "" ] ; then
+          juju-log -l INFO "_ch_peer_copy_new: $1 $p1 $path"
+          set +f
+          eval "$1 $p1 $path"
+          set -f
+          p1=""
+        else
+          p1=$path
+        fi
+      done
+      set $globstate
+      result=101
+    done
+  else
+    juju-log -l DEBUG "ch_peer_copy_new: no host cache yet"
+  fi
+  juju-log -l DEBUG "_ch_peer_copy_new: returning $result"
+  return $result
+}
+
+##
+# ch_peer_copy_replay
+#
+# do a replay of all previous cached copies 
+# would typically be used within a config changed to resync
+# files that might have been modified to all peers in the relation
+#
+# param none
+#
+# returns
+#  0 when the file is copied
+#  1 when there was an error
+#  100 when the file is not copied
+
+ch_peer_copy_replay() {
+  _ch_peer_copy_set_paths
+  local result=100
+  local hosts=""
+  local copies=""
+  local h=""
+  local c=""
+  local commandopt=""
+  local list_paths=""
+  local encp=""
+  local mpath=""
+  
+  if [ ! -e "$CH_PEER_COPY_HOST_F" ] ; then
+    juju-log -l DEBUG "ch_peer_copy_replay: no host cache yet"
+    result=1
+  fi
+  
+  if [ $result = 100 ];  then
+   hosts=`cat $CH_PEER_COPY_HOST_F`
+   copies=`ls ${CH_PEER_COPY_PATHS_F}-* | xargs -n1 basename`
+   
+   if [ -z "$copies" ] ; then
+     juju-log -l DEBUG "ch_peer_copy_replay: no command cache yet"
+     result=1
+   fi
+  fi
+  
+  if [ $result = 100 ];  then
+   for h in $hosts ;
+   do
+     
+     for c in $copies ; 
+     do
+       commandopt=`echo $c | cut -d- -f2 | base64 -d | sed 's/%/ /'`  
+       list_paths=`cat $CH_PEER_COPY_P/$c`
+      
+       for encp in $list_paths ;
+       do
+         mpath=`echo $encp | base64 -d`
+         local p1=""
+         #we do not want globbing in the loop as it would mess with the number of passes and we work on pairs
+         local globstate=`set -o | grep "noglob" | grep -q off && echo "+f" || echo "-f"`
+         set -f
+         for path in $mpath ; do 
+           if [ "$p1" != "" ] ; then
+             juju-log -l INFO "ch_peer_copy_replay: $commandopt $p1 $path"
+             set +f
+             eval "$commandopt $p1 $path"
+             set -f
+             p1=""
+           else
+             p1=$path
+           fi
+         done
+         set $globstate
+         result=0
+       done
+       
+     done
+     
+   done
+  fi
+  return $result
+}
+
+##
+# ch_peer_copy_cleanup <remote-unit>
+#
+# Removes a parting remote unit fron the host cache
+# Should be called for each peer-relation-departed
+#
+# param
+#  <remote-unit>    name of the remote unit
+#
+# returns nothing
+
+ch_peer_copy_cleanup() {
+   juju-log -l DEBUG "ch_peer_copy_cleanup: $1"
+   _ch_peer_copy_set_paths
+   if [ ! -e "$CH_PEER_COPY_HOST_F" ] ; then
+    juju-log -l DEBUG "ch_peer_copy_cleanup: no host cache yet"
+    return
+   fi
+   local unitname=""
+   unitname=`echo $1 | sed -e 's/\(\.\|\/\|\*\|\[\|\]\)/\\&/g'`
+   sed -i "/^$unitname.*$/d" $CH_PEER_COPY_HOST_F
+   return 0
+}
+

=== modified file 'tests/helpers/test_peer.sh'
--- tests/helpers/test_peer.sh	2012-01-05 19:35:03 +0000
+++ tests/helpers/test_peer.sh	2012-01-09 18:47:25 +0000
@@ -13,6 +13,8 @@
 
 set -ue
 
+JUJU_UNIT_NAME="EMPTY"
+
 #mock relation-list
 alias relation-list=mock_relation_list
 mock_relation_list()
@@ -20,7 +22,21 @@
     [ -z $CH_MASTER ] && let CH_MASTER=1
     
     case $CH_MASTER in
-    1)
+    4)
+        echo "TEST/3
+TEST/2
+TEST/4"
+        ;;
+    3)
+        echo "TEST/4
+TEST/3
+TEST/1"
+        ;;
+    2)
+        echo "TEST/3
+TEST/4"
+        ;;
+     1)
         echo "TEST/2
 TEST/3
 TEST/4"
@@ -30,8 +46,11 @@
 TEST/3
 TEST/4"
         ;;
-    esac
-        
+    -1)
+        echo ""
+        ;;
+    esac 
+
 }
 
 #Save juju-log for debugging
@@ -41,7 +60,7 @@
 alias juju-log=mock_juju_log
 mock_juju_log()
 {
-    echo "$1" >> $CH_TEMPLOG
+    echo "$*" >> $CH_TEMPLOG
 }
 
 #mock unit-get
@@ -50,7 +69,7 @@
 {
    case $1 in
    "private-address")
-       echo "127.0.0.1"
+       echo "localhost"
        ;;
    *)
        echo "UNDEFINED"
@@ -135,6 +154,9 @@
         output "Backing up created $HOME/.ssh to $backup_dir/dot-ssh"
         mv $HOME/.ssh $backup_dir/dot-ssh
     fi
+    if [ -e $HOME/ch_test ] ; then
+        rm -rf $HOME/ch_test
+    fi
 }
 trap cleanup_peer EXIT
 if [ $debug -gt 0 ] ; then echo "user: $USER, home: $HOME"; fi
@@ -236,10 +258,53 @@
 ch_peer_i_am_leader || return 1 && :
 echo PASS
 
+start_test "ch_peer_i_am_leader (unordered list 1)..."
+JUJU_REMOTE_UNIT="TEST/3"
+JUJU_UNIT_NAME="TEST/2"
+CH_MASTER=3
+ch_peer_i_am_leader && return 1 || :
+echo PASS
+
+start_test "ch_peer_i_am_leader (unordered list 2)..."
+JUJU_UNIT_NAME="TEST/1"
+CH_MASTER=4
+ch_peer_i_am_leader || return 1 && :
+echo PASS
+
+start_test "ch_peer_i_am_leader (unordered list 3)..."
+JUJU_UNIT_NAME="TEST/2"
+CH_MASTER=3
+ch_peer_i_am_leader && return 1 || :
+echo PASS
+
+start_test "ch_peer_i_am_leader (unordered list 4)..."
+JUJU_UNIT_NAME="TEST/3"
+ch_peer_i_am_leader && return 1 && :
+echo PASS
+
+start_test "ch_peer_i_am_leader (empty list)..."
+JUJU_REMOTE_UNIT="TEST/3"
+JUJU_UNIT_NAME="TEST/1"
+CH_MASTER=-1
+ch_peer_i_am_leader || return 1 && :
+echo PASS
+
+start_test "ch_peer_i_am_leader (departed leader)..."
+JUJU_REMOTE_UNIT="TEST/1"
+JUJU_UNIT_NAME="TEST/4"
+CH_MASTER=2
+ch_peer_i_am_leader && return 1 || :
+JUJU_UNIT_NAME="TEST/2"
+ch_peer_i_am_leader || return 1 && :
+echo PASS
+
 start_test ch_peer_leader...
+JUJU_REMOTE_UNIT="TEST/3"
+JUJU_UNIT_NAME="TEST/1"
+CH_MASTER=1
 [ "`ch_peer_leader`" = "TEST/1" ] ||  return 1
 [ `ch_peer_leader --id` -eq 1 ] || return 1
-JUJU_UNIT_NAME="TEST/3"
+JUJU_UNIT_NAME="TEST/2"
 [ "`ch_peer_leader`" = "TEST/2" ] || return 1
 [ `ch_peer_leader --id` -eq 2 ] || return 1
 echo PASS
@@ -305,18 +370,102 @@
     JUJU_UNIT_NAME="TEST/2" 
     JUJU_REMOTE_UNIT="TEST/1"
     CH_MASTER=0
-    if ch_peer_scp -p $CH_portnum -o "-q" $CH_TEMPDIR/sourcedir/testfile $CH_TEMPDIR/destdir/ ; then break ; fi
+    if ch_peer_scp -p $CH_portnum -o "-q" "$CH_TEMPDIR/sourcedir/testfile" "$CH_TEMPDIR/destdir/" "$CH_TEMPDIR/sourcedir/testfile1" "$CH_TEMPDIR/destdir/" ; then break ; fi
     #master relation joined
     JUJU_UNIT_NAME="TEST/1" 
     JUJU_REMOTE_UNIT="TEST/2"
     CH_MASTER=1
-    if ch_peer_scp -p $CH_portnum -o "-q" $CH_TEMPDIR/sourcedir/testfile $CH_TEMPDIR/destdir/ ; then break ; fi
+    if ch_peer_scp -p $CH_portnum -o "-q" "$CH_TEMPDIR/sourcedir/testfile" "$CH_TEMPDIR/destdir/" "$CH_TEMPDIR/sourcedir/testfile1" "$CH_TEMPDIR/destdir/" ; then break ; fi
 done
-[ ! -e $CH_TEMPDIR/destdir/testfile ] && output"file not copied" && exit 1
-CH_t1=`md5sum $CH_TEMPDIR/sourcedir/testfile | cut -d" " -f1`
-CH_t2=`md5sum $CH_TEMPDIR/destdir/testfile | cut -d" " -f1`
-[ ! "$CH_t1" = "$CH_t2" ] && output "md5sum differ" && exit 1
-rm -rf $CH_TEMPDIR/destdir/*
+[ ! -e $CH_TEMPDIR/destdir/testfile ] && output "file1 not copied" && exit 1
+[ ! -e $CH_TEMPDIR/destdir/testfile1 ] && output "file2 not copied" && exit 1
+CH_t1=`md5sum $CH_TEMPDIR/sourcedir/testfile | cut -d" " -f1`
+CH_t2=`md5sum $CH_TEMPDIR/destdir/testfile | cut -d" " -f1`
+[ ! "$CH_t1" = "$CH_t2" ] && output "md5sum differ" && exit 1
+rm -rf $CH_TEMPDIR/destdir/*
+echo PASS
+
+start_test "ch_peer_copy_replay..."
+CH_scp_hostname=""
+CH_scp_ssh_key_saved=""
+CH_scp_ssh_key=""
+CH_scp_copy_done=""
+#We are not in a relation, we are on master
+JUJU_UNIT_NAME="TEST/1" 
+JUJU_REMOTE_UNIT=""
+CH_MASTER=-1
+if ! ch_peer_copy_replay ; then
+    output "should not have returned not copied (1)"
+    exit 1
+fi
+#We are not in a relation, we are on slave
+JUJU_UNIT_NAME="TEST/2" 
+JUJU_REMOTE_UNIT=""
+CH_MASTER=-1
+if ch_peer_copy_replay ; then
+    output "should not have returned copied (0)"
+    exit 1
+fi
+[ ! -e $CH_TEMPDIR/destdir/testfile ] && output "file not copied" && exit 1
+CH_t1=`md5sum $CH_TEMPDIR/sourcedir/testfile | cut -d" " -f1`
+CH_t2=`md5sum $CH_TEMPDIR/destdir/testfile | cut -d" " -f1`
+[ ! "$CH_t1" = "$CH_t2" ] && output "md5sum differ" && exit 1
+[ ! -e $CH_TEMPDIR/destdir/ ] && output "dir not copied" && exit 1
+[ ! -e $CH_TEMPDIR/destdir/testfile0 ] && output "file1 not copied" && exit 1
+[ ! -e $CH_TEMPDIR/destdir/testfile1 ] && output "file2 not copied" && exit 1
+CH_t1=`md5sum $CH_TEMPDIR/sourcedir/testfile0 | cut -d" " -f1`
+CH_t2=`md5sum $CH_TEMPDIR/destdir/testfile0 | cut -d" " -f1`
+[ ! "$CH_t1" = "$CH_t2" ] && output "md5sum differ" && exit 1
+rm -rf $CH_TEMPDIR/destdir/*
+echo PASS
+
+start_test "ch_peer_rsync (out of relation)..."
+CH_scp_hostname=""
+CH_scp_ssh_key_saved=""
+CH_scp_ssh_key=""
+CH_scp_copy_done=""
+#We are not in a relation, we are on master
+JUJU_UNIT_NAME="TEST/1" 
+JUJU_REMOTE_UNIT=""
+CH_MASTER=-1
+ch_peer_rsync -p $CH_portnum -o "-azq" "$CH_TEMPDIR/sourcedir/*" "$CH_TEMPDIR/destdir/" && chres=0 || chres=$?
+if [ $chres -ne 101 ] ; then
+    output "should not have returned not copied (received $chres, 101 expected)"
+    exit 1
+fi
+#We are not in a relation, we are on slave
+JUJU_UNIT_NAME="TEST/2" 
+JUJU_REMOTE_UNIT=""
+CH_MASTER=-1
+ch_peer_rsync -p $CH_portnum -o "-azq" "$CH_TEMPDIR/sourcedir/*" "$CH_TEMPDIR/destdir/" && chres=0 || chres=$?
+if [ $chres -ne 100 ] ; then
+    output "should not have returned copied (received $chres, 100 expected)"
+    exit 1
+fi
+[ ! -e $CH_TEMPDIR/destdir/ ] && output "dir not copied" && exit 1
+[ ! -e $CH_TEMPDIR/destdir/testfile0 ] && output "file1 not copied" && exit 1
+[ ! -e $CH_TEMPDIR/destdir/testfile1 ] && output "file2 not copied" && exit 1
+CH_t1=`md5sum $CH_TEMPDIR/sourcedir/testfile0 | cut -d" " -f1`
+CH_t2=`md5sum $CH_TEMPDIR/destdir/testfile0 | cut -d" " -f1`
+[ ! "$CH_t1" = "$CH_t2" ] && output "md5sum differ" && exit 1
+rm -rf $CH_TEMPDIR/destdir/*
+#restore authorized_keys & known_hosts
+echo PASS
+
+start_test "ch_peer_copy_cleanup..."
+# as a leader
+JUJU_UNIT_NAME="TEST/1" 
+JUJU_REMOTE_UNIT="TEST/2"
+CH_MASTER=-1
+ch_peer_copy_cleanup "$JUJU_REMOTE_UNIT"
+unitname=`echo $JUJU_UNIT_NAME | sed 's/\//-/g'`
+[ `grep -F "$JUJU_REMOTE_UNIT" $HOME/ch_test/$unitname` ] && output "not cleaned up" && exit 1
+# as a slave
+JUJU_UNIT_NAME="TEST/2" 
+JUJU_REMOTE_UNIT="TEST/1"
+CH_MASTER=-1
+ch_peer_copy_cleanup "$JUJU_REMOTE_UNIT"
+#nothing to check here other than if we did not choke on cleaning up something that does not exist
 echo PASS
 
 trap - EXIT