Skip to content

Commit

Permalink
t/t-batch-transfer.sh: test multiple SSH uploads
Browse files Browse the repository at this point in the history 
In commit 691de51 of PR git-lfs#4446 we added
the "batch transfers with ssh endpoint (git-lfs-transfer)" test to our
t/t-batch-transfer.sh test script in order to validate that the new
SSH object transfer protocol for Git LFS was operating as expected.

This test only creates a single test object, and so does not cause
the Git LFS client to establish multiple SSH connections during the
batch object transfer phase.  For this reason, no SSH connections are
started with the ControlMaster option set to "no".

As described in a prior commit in this PR, our lfs-ssh-echo test
utility program had a bug which prevented it from accepting SSH
connections with the ControlMaster option set to "no", so the
"batch transfers with ssh endpoint (git-lfs-transfer)" test would
have failed if it tried to push more than a single object.

Such a test would also fail if it tried to fetch more than a single
object, due to the bug described in git-lfs#5880.  Specifically, we returned
a nil from the Connection() method of the SSHTransfer structure in our
"ssh" package when we had closed all the SSH connections, but then
sometimes tried to reference that nil pointer in the batchInternal()
method of the SSHBatchClient structure in the "tq" package, causing
a Go panic condition.

In prior commits in this PR we resolved these problems, both the bug
in the lfs-ssh-echo test utility and the bad return value from the
Connection() method of the SSHTransfer structure in the Git LFS client
code.  In the case of the latter issue, we adjusted the Connection()
method to return a non-nil error when the requested connection number
exceeds the maximum number of permitted SSH connections, which also
covers the case where all the connections have already been closed.

Therefore we can now add additional tests of the SSH object transfer
protocol which push multiple objects and then fetch them.  In these
tests we use the new assert_remote_object() assertion function that we
added in a prior commit in this PR to confirm that the objects we push
are all written to the "remote" repository.  As well, we check that
the expected number of SSH connections are reported in the trace logs
from both the push and clone operations, and that some connections are
made with the ControlMaster option set to "no" as well as "yes".

The first of our new tests leaves the maximum number of concurrent object
transfers set to the default value of 8, so that all three of the objects
we create in the test are pushed or fetched in the same batch.  This
requires that the Git LFS client establish three separate SSH connections,
two of which should have the ControlMaster option set to "no", and the
first of which should have the option set to "yes".

The second of our new tests sets the maximum number of concurrent
transfers to two, so we expect to see only two SSH connections
established, the first of which should have the ControlMaster option
set to "yes".  Since there are only two concurrent connections,
we can assume the third object was transferred in a second batch.

(Note that the Git LFS client, at present, always creates an extra
SSH connection with the ControlMaster option set to "yes" when
pushing objects over the SSH object transfer protocol.  It uses this
extra connection exclusively to perform locking commands.  Our tests
therefore expect this extra connection to be reported in the push
trace logs, and we include comments to explain why the expected
connection totals are one higher than might otherwise be the case.)

Next, we expand the existing "batch transfers with ssh endpoint
(git-lfs-transfer)" test to perform the same set of checks as our new
tests, bringing the tests into close alignment with each other.  We
again use the assert_remote_object() function to confirm that the
single object pushed in this test is written to the "remote" repository.
We also check that only a single SSH connection is reported in the
trace logs (plus the extra one used for locking commands during the
push operation).

Finally, we make a few small adjustments to other tests in the same
test script, removing some unnecessary redirections of stderr, and
adding one extra check to our "batch transfers with ssh endpoint
(git-lfs-authenticate)" test which just confirms that an object was
successfully pushed over HTTP to our lfstest-gitserver test helper
program, following an initial authorization handshake over SSH.
  • Loading branch information
@chrisd8088
chrisd8088 committed Oct 29, 2024
1 parent d5a3209 commit ca567d8
Showing 1 changed file with 117 additions and 4 deletions.
121 changes: 117 additions & 4 deletions t/t-batch-transfer.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ begin_test "batch transfers succeed with an empty hash algorithm"
git add .gitattributes good.dat special.dat
git commit -m "hi"

git push origin main 2>&1 | tee push.log
git push origin main
assert_server_object "$reponame" "$(calc_oid "$contents")"
)
end_test
Expand Down Expand Up @@ -159,7 +159,8 @@ begin_test "batch transfers with ssh endpoint (git-lfs-authenticate)"
git add .gitattributes test.dat
git commit -m "initial commit"

git push origin main 2>&1
git push origin main
assert_server_object "$reponame" "$(calc_oid "$contents")"
)
end_test

Expand All @@ -182,10 +183,122 @@ begin_test "batch transfers with ssh endpoint (git-lfs-transfer)"
git add .gitattributes test.dat
git commit -m "initial commit"

git push origin main 2>&1
GIT_TRACE=1 git push origin main >push.log 2>&1
# We currently spawn one extra ControlMaster=yes connection to run
# locking commands, and never shut it down cleanly.
[ "2" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=yes.*git-lfs-transfer .*$reponame.git upload" push.log)" ]
[ "0" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=no.*git-lfs-transfer .*$reponame.git upload" push.log)" ]
[ "2" -eq "$(grep -c 'spawning pure SSH connection' push.log)" ]
[ "2" -eq "$(grep -c 'pure SSH connection successful' push.log)" ]
[ "1" -eq "$(grep -c 'terminating pure SSH connection' push.log)" ]
assert_remote_object "$reponame" "$(calc_oid "$contents")" "${#contents}"

cd ..
GIT_TRACE=1 git clone "$sshurl" "$reponame-2" 2>&1 | tee trace.log
[ "1" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=yes.*git-lfs-transfer .*$reponame.git download" trace.log)" ]
[ "0" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=no.*git-lfs-transfer .*$reponame.git download" trace.log)" ]
[ "1" -eq "$(grep -c 'spawning pure SSH connection' trace.log)" ]
[ "1" -eq "$(grep -c 'pure SSH connection successful' trace.log)" ]
[ "1" -eq "$(grep -c 'terminating pure SSH connection' trace.log)" ]

cd "$reponame-2"
git lfs fsck
)
end_test

begin_test "batch transfers with ssh endpoint and multiple objects (git-lfs-transfer)"
(
set -e

setup_pure_ssh

reponame="batch-ssh-transfer-multiple"
setup_remote_repo "$reponame"
clone_repo "$reponame" "$reponame"

contents1="test1"
contents2="test2"
contents3="test3"
git lfs track "*.dat"
printf "%s" "$contents1" >test1.dat
printf "%s" "$contents2" >test2.dat
printf "%s" "$contents3" >test3.dat
git add .gitattributes test*.dat
git commit -m "initial commit"

sshurl=$(ssh_remote "$reponame")
git config lfs.url "$sshurl"

GIT_TRACE=1 git push origin main >push.log 2>&1
# We currently spawn one extra ControlMaster=yes connection to run
# locking commands, and never shut it down cleanly.
[ "2" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=yes.*git-lfs-transfer .*$reponame.git upload" push.log)" ]
[ "2" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=no.*git-lfs-transfer .*$reponame.git upload" push.log)" ]
[ "4" -eq "$(grep -c 'spawning pure SSH connection' push.log)" ]
[ "4" -eq "$(grep -c 'pure SSH connection successful' push.log)" ]
[ "3" -eq "$(grep -c 'terminating pure SSH connection' push.log)" ]
assert_remote_object "$reponame" "$(calc_oid "$contents1")" "${#contents1}"
assert_remote_object "$reponame" "$(calc_oid "$contents2")" "${#contents2}"
assert_remote_object "$reponame" "$(calc_oid "$contents3")" "${#contents3}"

cd ..
GIT_TRACE=1 git clone "$sshurl" "$reponame-2" 2>&1 | tee trace.log
grep "lfs-ssh-echo.*git-lfs-transfer .*$reponame.git download" trace.log
[ "1" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=yes.*git-lfs-transfer .*$reponame.git download" trace.log)" ]
[ "2" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=no.*git-lfs-transfer .*$reponame.git download" trace.log)" ]
[ "3" -eq "$(grep -c 'spawning pure SSH connection' trace.log)" ]
[ "3" -eq "$(grep -c 'pure SSH connection successful' trace.log)" ]
[ "3" -eq "$(grep -c 'terminating pure SSH connection' trace.log)" ]

cd "$reponame-2"
git lfs fsck
)
end_test

begin_test "batch transfers with ssh endpoint and multiple objects and batches (git-lfs-transfer)"
(
set -e

setup_pure_ssh

reponame="batch-ssh-transfer-multiple-batch"
setup_remote_repo "$reponame"
clone_repo "$reponame" "$reponame"

contents1="test1"
contents2="test2"
contents3="test3"
git lfs track "*.dat"
printf "%s" "$contents1" >test1.dat
printf "%s" "$contents2" >test2.dat
printf "%s" "$contents3" >test3.dat
git add .gitattributes test*.dat
git commit -m "initial commit"

sshurl=$(ssh_remote "$reponame")
git config lfs.url "$sshurl"

git config --global lfs.concurrentTransfers 2

GIT_TRACE=1 git push origin main >push.log 2>&1
# We currently spawn one extra ControlMaster=yes connection to run
# locking commands, and never shut it down cleanly.
[ "2" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=yes.*git-lfs-transfer .*$reponame.git upload" push.log)" ]
[ "1" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=no.*git-lfs-transfer .*$reponame.git upload" push.log)" ]
[ "3" -eq "$(grep -c 'spawning pure SSH connection' push.log)" ]
[ "3" -eq "$(grep -c 'pure SSH connection successful' push.log)" ]
[ "2" -eq "$(grep -c 'terminating pure SSH connection' push.log)" ]
assert_remote_object "$reponame" "$(calc_oid "$contents1")" "${#contents1}"
assert_remote_object "$reponame" "$(calc_oid "$contents2")" "${#contents2}"
assert_remote_object "$reponame" "$(calc_oid "$contents3")" "${#contents3}"

cd ..
GIT_TRACE=1 git clone "$sshurl" "$reponame-2" 2>&1 | tee trace.log
[ "1" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=yes.*git-lfs-transfer .*$reponame.git download" trace.log)" ]
[ "1" -eq "$(grep -c "exec: lfs-ssh-echo.*-oControlMaster=no.*git-lfs-transfer .*$reponame.git download" trace.log)" ]
[ "2" -eq "$(grep -c 'spawning pure SSH connection' trace.log)" ]
[ "2" -eq "$(grep -c 'pure SSH connection successful' trace.log)" ]
[ "2" -eq "$(grep -c 'terminating pure SSH connection' trace.log)" ]

cd "$reponame-2"
git lfs fsck
)
Expand Down

0 comments on commit ca567d8

Please sign in to comment.