From fb895f2c123cb6c9f69724d56bd095a4253cd93d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 22 Jan 2022 12:45:49 -0800 Subject: [PATCH] Don't try to remove the root if it appears corrupt The `--root` is often a volume and can't be removed. Instead, remove the contents of it. Adjust tests to hit this. --- cmd/git-sync/main.go | 24 +++++++++++++++++++++- test_e2e.sh | 48 +++++++++++++++++++++++++++----------------- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index c2cd9c6..37c5613 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -566,6 +566,25 @@ func main() { } } +func removeDirContents(dir string, log *logging.Logger) error { + dirents, err := ioutil.ReadDir(dir) + if err != nil { + return err + } + + for _, fi := range dirents { + p := filepath.Join(dir, fi.Name()) + if log != nil { + log.V(2).Info("removing path recursively", "path", p, "isDir", fi.IsDir()) + } + if err := os.RemoveAll(p); err != nil { + return err + } + } + + return nil +} + func updateSyncMetrics(key string, start time.Time) { syncDuration.WithLabelValues(key).Observe(time.Since(start).Seconds()) syncCount.WithLabelValues(key).Inc() @@ -849,7 +868,10 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot if strings.Contains(err.Error(), "already exists and is not an empty directory") { // Maybe a previous run crashed? Git won't use this dir. log.V(0).Info("git root exists and is not empty (previous crash?), cleaning up", "path", gitRoot) - err := os.RemoveAll(gitRoot) + // We remove the contents rather than the dir itself, because a + // common use-case is to have a volume mounted at git.root, which + // makes removing it impossible. + err := removeDirContents(gitRoot, log) if err != nil { return err } diff --git a/test_e2e.sh b/test_e2e.sh index d62c600..931e0a4 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -98,10 +98,19 @@ function docker_kill() { docker kill "$1" >/dev/null } +# DIR is the directory in which all this test's state lives. RUNID="${RANDOM}${RANDOM}" DIR="/tmp/git-sync-e2e.$RUNID" mkdir "$DIR" +# WORK is temp space and in reset for each testcase. +WORK="$DIR/work" +function clean_work() { + rm -rf "$WORK" + mkdir -p "$WORK" +} + +# REPO is the source repo under test. REPO="$DIR/repo" MAIN_BRANCH="e2e-branch" function init_repo() { @@ -113,6 +122,7 @@ function init_repo() { git -C "$REPO" commit -aqm "init file" } +# ROOT is the volume (usually) used as --root. ROOT="$DIR/root" function clean_root() { rm -rf "$ROOT" @@ -144,7 +154,9 @@ function GIT_SYNC() { --label git-sync-e2e="$RUNID" \ --network="host" \ -u $(id -u):$(id -g) \ - -v "$DIR":"$DIR":rw \ + -v "$ROOT":"$ROOT":rw \ + -v "$REPO":"$REPO":ro \ + -v "$WORK":"$WORK":ro \ -v "$(pwd)/slow_git_clone.sh":"$SLOW_GIT_CLONE":ro \ -v "$(pwd)/slow_git_fetch.sh":"$SLOW_GIT_FETCH":ro \ -v "$(pwd)/askpass_git.sh":"$ASKPASS_GIT":ro \ @@ -200,7 +212,6 @@ function e2e::head_once() { function e2e::non_zero_exit() { echo "$FUNCNAME" > "$REPO"/file git -C "$REPO" commit -qam "$FUNCNAME" - ln -s "$ROOT" "$DIR/rootlink" # symlink to test ( set +o errexit GIT_SYNC \ @@ -208,7 +219,7 @@ function e2e::non_zero_exit() { --repo="file://$REPO" \ --branch="$MAIN_BRANCH" \ --rev=does-not-exit \ - --root="$DIR/rootlink" \ + --root="$ROOT" \ --dest="link" \ >> "$1" 2>&1 RET=$? @@ -981,7 +992,7 @@ function e2e::exechook_fail_once() { # Test webhook success ############################################## function e2e::webhook_success() { - HITLOG="$DIR/hitlog" + HITLOG="$WORK/hitlog" # First sync cat /dev/null > "$HITLOG" @@ -1028,7 +1039,7 @@ function e2e::webhook_success() { # Test webhook fail-retry ############################################## function e2e::webhook_fail_retry() { - HITLOG="$DIR/hitlog" + HITLOG="$WORK/hitlog" # First sync - return a failure to ensure that we try again cat /dev/null > "$HITLOG" @@ -1077,7 +1088,7 @@ function e2e::webhook_fail_retry() { # Test webhook success with --one-time ############################################## function e2e::webhook_success_once() { - HITLOG="$DIR/hitlog" + HITLOG="$WORK/hitlog" # First sync cat /dev/null > "$HITLOG" @@ -1114,7 +1125,7 @@ function e2e::webhook_success_once() { # Test webhook fail with --one-time ############################################## function e2e::webhook_fail_retry_once() { - HITLOG="$DIR/hitlog" + HITLOG="$WORK/hitlog" # First sync - return a failure to ensure that we try again cat /dev/null > "$HITLOG" @@ -1150,7 +1161,7 @@ function e2e::webhook_fail_retry_once() { # Test webhook fire-and-forget ############################################## function e2e::webhook_fire_and_forget() { - HITLOG="$DIR/hitlog" + HITLOG="$WORK/hitlog" # First sync cat /dev/null > "$HITLOG" @@ -1239,7 +1250,7 @@ function e2e::http() { function e2e::submodule_sync() { # Init submodule repo SUBMODULE_REPO_NAME="sub" - SUBMODULE="$DIR/$SUBMODULE_REPO_NAME" + SUBMODULE="$WORK/$SUBMODULE_REPO_NAME" mkdir "$SUBMODULE" git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH" @@ -1249,7 +1260,7 @@ function e2e::submodule_sync() { # Init nested submodule repo NESTED_SUBMODULE_REPO_NAME="nested-sub" - NESTED_SUBMODULE="$DIR/$NESTED_SUBMODULE_REPO_NAME" + NESTED_SUBMODULE="$WORK/$NESTED_SUBMODULE_REPO_NAME" mkdir "$NESTED_SUBMODULE" git -C "$NESTED_SUBMODULE" init -q -b "$MAIN_BRANCH" @@ -1339,7 +1350,7 @@ function e2e::submodule_sync() { function e2e::submodule_sync_depth() { # Init submodule repo SUBMODULE_REPO_NAME="sub" - SUBMODULE="$DIR/$SUBMODULE_REPO_NAME" + SUBMODULE="$WORK/$SUBMODULE_REPO_NAME" mkdir "$SUBMODULE" git -C "$SUBMODULE" init -b "$MAIN_BRANCH" > /dev/null @@ -1417,7 +1428,7 @@ function e2e::submodule_sync_depth() { function e2e::submodule_sync_off() { # Init submodule repo SUBMODULE_REPO_NAME="sub" - SUBMODULE="$DIR/$SUBMODULE_REPO_NAME" + SUBMODULE="$WORK/$SUBMODULE_REPO_NAME" mkdir "$SUBMODULE" git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH" @@ -1448,7 +1459,7 @@ function e2e::submodule_sync_off() { function e2e::submodule_sync_shallow() { # Init submodule repo SUBMODULE_REPO_NAME="sub" - SUBMODULE="$DIR/$SUBMODULE_REPO_NAME" + SUBMODULE="$WORK/$SUBMODULE_REPO_NAME" mkdir "$SUBMODULE" git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH" @@ -1458,7 +1469,7 @@ function e2e::submodule_sync_shallow() { # Init nested submodule repo NESTED_SUBMODULE_REPO_NAME="nested-sub" - NESTED_SUBMODULE="$DIR/$NESTED_SUBMODULE_REPO_NAME" + NESTED_SUBMODULE="$WORK/$NESTED_SUBMODULE_REPO_NAME" mkdir "$NESTED_SUBMODULE" git -C "$NESTED_SUBMODULE" init -q -b "$MAIN_BRANCH" @@ -1522,9 +1533,9 @@ function e2e::ssh() { # Test sparse-checkout files ############################################## function e2e::sparse_checkout() { - echo "!/*" > "$DIR"/sparseconfig - echo "!/*/" >> "$DIR"/sparseconfig - echo "file2" >> "$DIR"/sparseconfig + echo "!/*" > "$WORK"/sparseconfig + echo "!/*/" >> "$WORK"/sparseconfig + echo "file2" >> "$WORK"/sparseconfig echo "$FUNCNAME" > "$REPO"/file echo "$FUNCNAME" > "$REPO"/file2 mkdir "$REPO"/dir @@ -1540,7 +1551,7 @@ function e2e::sparse_checkout() { --rev=HEAD \ --root="$ROOT" \ --dest="link" \ - --sparse-checkout-file="$DIR/sparseconfig" \ + --sparse-checkout-file="$WORK/sparseconfig" \ >> "$1" 2>&1 assert_link_exists "$ROOT"/link assert_file_exists "$ROOT"/link/file2 @@ -1695,6 +1706,7 @@ echo # Iterate over the chosen tests and run them. for t; do clean_root + clean_work init_repo echo -n "testcase $t: " if "e2e::${t}" "${DIR}/log.$t"; then