theforeman / foreman-infra

Puppet modules and scripts to manage Foreman project infrastructure
https://theforeman.github.io/foreman-infra
Apache License 2.0
23 stars 51 forks source link

Calculate release for staginyum rsync script properly #1994

Closed ehelms closed 10 months ago

ehelms commented 10 months ago

The command we send is:

SSH_ORIGINAL_COMMAND='rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial "tmp/pulpcore/nightly" "yumrepostage@web01.osuosl.theforeman.org:rsync_cache/pulpcore"'

And with the current version this results in:

"yumrepostage@web01.osuosl.theforeman.org:rsync_cache/pulpcore"
pulpcore"

With this change, it ends up trying to rsync everything:

rsync -rvx --delete-after /home/yumrepostage/rsync_cache/pulpcore/ /var/www/vhosts/stagingyum/htdocs/pulpcore/
ehelms commented 10 months ago

Actually, I am not sure this works either now that I dig more. The current implementation means we end up with the original SSH command being:

rsync --server -vlprcxe.iLsfxCIvu --delete-after --partial . rsync_cache/pulpcore

Which with the current script ends up triggering:

rsync -rvx --delete-after /home/yumrepostage/rsync_cache/pulpcore/ /var/www/vhosts/stagingyum/htdocs/pulpcore/

Where the RELEASE is not present, but it's not available in the original SSH command either.

ehelms commented 10 months ago

I am wondering then if my original assumption around the commands does not hold and instead of:

rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial "tmp/pulpcore/nightly" "yumrepostage@web01.osuosl.theforeman.org:rsync_cache/pulpcore"

It needs to be running:

rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial "tmp/pulpcore/nightly/el8" "yumrepostage@web01.osuosl.theforeman.org:rsync_cache/pulpcore/nightly"
rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial "tmp/pulpcore/nightly/el9" "yumrepostage@web01.osuosl.theforeman.org:rsync_cache/pulpcore/nightly"
ehelms commented 10 months ago

I do not think this fix is needed, instead it's to update our upload:

And yes, I desire to merge these to a single script first I must solve https://github.com/theforeman/theforeman-rel-eng/pull/291

ekohl commented 10 months ago

For completeness, the completely rendered file

#!/bin/sh

set -e

case "$SSH_ORIGINAL_COMMAND" in
*\&*)
echo "Rejected"
;;
*\(*)
echo "Rejected"
;;
*\{*)
echo "Rejected"
;;
*\;*)
echo "Rejected"
;;
*\<*)
echo "Rejected"
;;
*\`*)
echo "Rejected"
;;
*\|*)
echo "Rejected"
;;
rsync\ --server*)
# Only push to the rsync cache
if [[ `echo "${SSH_ORIGINAL_COMMAND}" | awk '{ print $NF }'` =~ ^rsync_cache/.* ]] ; then
  # Make sure target dir can be created
  YUM_PATH=`echo "${SSH_ORIGINAL_COMMAND}" | awk '{ print $NF }'`
  PROJECT=`echo $YUM_PATH | /bin/cut -f2 -d/`
  RELEASE=`echo $YUM_PATH | /bin/cut -f3 -d/`
  mkdir -p /home/yumrepostage/rsync_cache/$PROJECT/$RELEASE

  # Permit transfer
  $SSH_ORIGINAL_COMMAND

  # Publish the site - stderr/out redirect is required to stop the noninteractive shell from hanging
  rsync -rvx --delete-after /home/yumrepostage/rsync_cache/$PROJECT/$RELEASE /var/www/vhosts/stagingyum/htdocs/$PROJECT/ 2>&1 >/dev/null ;

fi
;;
*)
echo "Rejected"
;;
esac
# ERB highlighting looks terrible in this script...
# vim: set ft=sh : #

Focusing on just the relevant part:

if [[ `echo "${SSH_ORIGINAL_COMMAND}" | awk '{ print $NF }'` =~ ^rsync_cache/.* ]] ; then
  # Make sure target dir can be created
  YUM_PATH=`echo "${SSH_ORIGINAL_COMMAND}" | awk '{ print $NF }'`
  PROJECT=`echo $YUM_PATH | /bin/cut -f2 -d/`
  RELEASE=`echo $YUM_PATH | /bin/cut -f3 -d/`
  mkdir -p /home/yumrepostage/rsync_cache/$PROJECT/$RELEASE

  # Permit transfer
  $SSH_ORIGINAL_COMMAND

  # Publish the site - stderr/out redirect is required to stop the noninteractive shell from hanging
  rsync -rvx --delete-after /home/yumrepostage/rsync_cache/$PROJECT/$RELEASE /var/www/vhosts/stagingyum/htdocs/$PROJECT/ 2>&1 >/dev/null ;

fi

We know:

In this particular example, we already know rsync_cache exists, so the mkdir part is technically not needed, but it could be mkdir -p $(dirname "$YUM_PATH"). That removes the need for $PROJECT and $RELEASE in one part. In https://github.com/theforeman/jenkins-jobs/pull/381 you use --mkpath instead, which is probably better since it's less fragile.

Then we have the actual rsync command. From my testing, adding a trailing slash works:

$ PS1='\$ '
$ tree
.
└── source
    └── foreman
        └── 3.9
            ├── el8
            └── el9
                └── test.txt

6 directories, 1 file
$ rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial --mkpath source/foreman/3.9/ destination/foreman/3.9/
building file list ... done
created 3 directories for destination/foreman/3.9
el8/
el9/
el9/test.txt

sent 161 bytes  received 90 bytes  502.00 bytes/sec
total size is 0  speedup is 0.00
$ tree
.
├── destination
│   └── foreman
│       └── 3.9
│           ├── el8
│           └── el9
│               └── test.txt
└── source
    └── foreman
        └── 3.9
            ├── el8
            └── el9
                └── test.txt

11 directories, 2 files
$ touch source/foreman/3.9/el8/file.txt
$ rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial --mkpath source/foreman/3.9/ destination/foreman/3.9/
building file list ... done
el8/file.txt

sent 197 bytes  received 30 bytes  454.00 bytes/sec
total size is 0  speedup is 0.00
$ tree
.
├── destination
│   └── foreman
│       └── 3.9
│           ├── el8
│           │   └── file.txt
│           └── el9
│               └── test.txt
└── source
    └── foreman
        └── 3.9
            ├── el8
            │   └── file.txt
            └── el9
                └── test.txt

11 directories, 4 files
ehelms commented 10 months ago
  • SSH_ORIGINAL_COMMAND='rsync --checksum --perms --recursive --links -v --one-file-system --delete-after --partial "tmp/pulpcore/nightly" "yumrepostage@web01.osuosl.theforeman.org:rsync_cache/pulpcore"'

This is what I thought too but this is the logic fallacy, this is not the SSH_ORIGINAL_COMMAND, it's actually:

rsync --server -vlprcxe.iLsfxCIvu --delete-after --partial . rsync_cache/pulpcore

Notice the script checks for case "$SSH_ORIGINAL_COMMAND" in and hits the entry rsync\ --server*) which triggers the block.

This is why I feel the command has to have the version in to, aka the target has to be rsync_cahce/<collection>/<version>

ekohl commented 10 months ago

This is why I feel the command has to have the version in to, aka the target has to be rsync_cahce/<collection>/<version>

Right, but if you set the target to rsync_cache/collection/version/ (so with a trailing slash) it works as you'd like. At least in my experience. Variable extraction also does the right thing:

$ YUM_PATH=rsync_cache/pulpcore/3.9/
$ echo $YUM_PATH | /bin/cut -f2 -d/
pulpcore
$ echo $YUM_PATH | /bin/cut -f3 -d/
3.9

That way you don't need to list all OSes, but can still sync a whole version at once.