Merge pull request #224 from thockin/meaningful-healthcheck

Make health check meaningful

cmd/git-sync/main.go

@@ -35,6 +35,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/go-logr/glogr"
@@ -307,7 +308,12 @@ func main() {
 
 			// This is a dumb liveliness check endpoint. Currently this checks
 			// nothing and will always return 200 if the process is live.
-			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {})
+			mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+				if !getRepoReady() {
+					http.Error(w, "repo is not ready", http.StatusServiceUnavailable)
+				}
+				// Otherwise success
+			})
 			http.Serve(ln, mux)
 		}()
 	}
@@ -454,6 +460,22 @@ func updateSymlink(ctx context.Context, gitRoot, link, newDir string) (string, e
 	return oldWorktreePath, nil
 }
 
+// repoReady indicates that the repo has been cloned and synced.
+var readyLock sync.Mutex
+var repoReady = false
+
+func getRepoReady() bool {
+	readyLock.Lock()
+	defer readyLock.Unlock()
+	return repoReady
+}
+
+func setRepoReady() {
+	readyLock.Lock()
+	defer readyLock.Unlock()
+	repoReady = true
+}
+
 // addWorktreeAndSwap creates a new worktree and calls updateSymlink to swap the symlink to point to the new worktree
 func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string, depth int, hash string) error {
 	log.V(0).Info("syncing git", "rev", rev, "hash", hash)
@@ -525,9 +547,12 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string,
 	}
 
 	// Flip the symlink.
-	if oldWorktree, err := updateSymlink(ctx, gitRoot, dest, worktreePath); err != nil {
+	oldWorktree, err := updateSymlink(ctx, gitRoot, dest, worktreePath)
+	if err != nil {
 		return err
-	} else if oldWorktree != "" {
+	}
+	setRepoReady()
+	if oldWorktree != "" {
 		// Clean up previous worktree
 		log.V(1).Info("removing old worktree", "path", oldWorktree)
 		if err := os.RemoveAll(oldWorktree); err != nil {

slow_git.sh

@@ -1,3 +1,9 @@
 #!/bin/sh
+
+if [ "$1" != "clone" ]; then
+  git "$@"
+  exit $?
+fi
+
 sleep 1.1
 git "$@"

test_e2e.sh

@@ -133,6 +133,7 @@ function GIT_SYNC() {
 }
 
 function remove_containers() {
+    sleep 2 # Let docker finish saving container metadata
     docker ps --filter label="git-sync-e2e=$RUNID" --format="{{.ID}}" \
         | while read CTR; do
             docker kill "$CTR" >/dev/null
@@ -772,6 +773,7 @@ BINDPORT=8888
 echo "$TESTCASE 1" > "$REPO"/file
 git -C "$REPO" commit -qam "$TESTCASE 1"
 GIT_SYNC \
+    --git="$SLOW_GIT" \
     --logtostderr \
     --v=5 \
     --repo="file://$REPO" \
@@ -781,6 +783,14 @@ GIT_SYNC \
     --http-pprof \
     --dest="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
+while ! curl --silent --output /dev/null http://localhost:$BINDPORT; do
+    # do nothing, just wait for the HTTP to come up
+    true
+done
+# check that health endpoint fails
+if [[ $(curl --write-out %{http_code} --silent --output /dev/null http://localhost:$BINDPORT) -ne 503 ]] ; then
+    fail "health endpoint should have failed: $(curl --write-out %{http_code} --silent --output /dev/null http://localhost:$BINDPORT)"
+fi
 sleep 2
 # check that health endpoint is alive
 if [[ $(curl --write-out %{http_code} --silent --output /dev/null http://localhost:$BINDPORT) -ne 200 ]] ; then