fix(backup): find race and disable search in subdir, always test latest backup image in e2e, bump backup to 0.3.x (#1011)

Co-authored-by: brokenpip3 <brokenpip3@gmail.com>

Makefile

@@ -89,11 +89,18 @@ test: ## Runs the go tests
 	@RUNNING_TESTS=1 go test -tags "$(BUILDTAGS) cgo" $(PACKAGES_FOR_UNIT_TESTS)
 
 .PHONY: e2e
-e2e: deepcopy-gen manifests ## Runs e2e tests, you can use EXTRA_ARGS
+e2e: deepcopy-gen manifests backup-kind-load ## Runs e2e tests, you can use EXTRA_ARGS
 	@echo "+ $@"
 	RUNNING_TESTS=1 go test -parallel=1 "./test/e2e/" -ginkgo.v -tags "$(BUILDTAGS) cgo" -v -timeout 60m -run "$(E2E_TEST_SELECTOR)" \
 		-jenkins-api-hostname=$(JENKINS_API_HOSTNAME) -jenkins-api-port=$(JENKINS_API_PORT) -jenkins-api-use-nodeport=$(JENKINS_API_USE_NODEPORT) $(E2E_TEST_ARGS)
 
+## Backup Section
+
+.PHONY: backup-kind-load
+backup-kind-load: ## Load latest backup image in the cluster
+	@echo "+ $@"
+	make -C backup/pvc backup-kind-load
+
 ## HELM Section
 
 .PHONY: helm
@@ -370,6 +377,12 @@ kind-clean: ## Delete kind cluster
 	@echo "+ $@"
 	kind delete cluster --name $(KIND_CLUSTER_NAME)
 
+.PHONY: kind-load-backup
+kind-load-backup:
+	@echo "+ $@"
+	make -C
+	kind delete cluster --name $(KIND_CLUSTER_NAME)
+
 .PHONY: bats-tests
 IMAGE_NAME := quay.io/$(QUAY_ORGANIZATION)/$(QUAY_REGISTRY):$(GITCOMMIT)-amd64
 BUILD_PRESENT := $(shell docker images |grep -q ${IMAGE_NAME})

backup/pvc/Makefile

@@ -118,6 +118,12 @@ docker-release-latest: docker-build ## Release image with latest tags (in additi
 docker-release: docker-release-version docker-release-latest ## Release image with version and latest tags (in addition to build tag)
 	@echo "+ $@"
 
+.PHONY: backup-kind-load
+backup-kind-load: docker-build ## Build and load backup img in kind with e2e-test tag
+	@echo "+ $@"
+	docker tag quay.io/$(QUAY_ORGANIZATION)/$(QUAY_REGISTRY)-$(NAME):$(GITCOMMIT) quay.io/$(QUAY_ORGANIZATION)/$(QUAY_REGISTRY)-$(NAME):e2e-test
+	kind load docker-image quay.io/$(QUAY_ORGANIZATION)/$(QUAY_REGISTRY)-$(NAME):e2e-test --name $(KIND_CLUSTER_NAME)
+
 # if this session isn't interactive, then we don't want to allocate a
 # TTY, which would fail, but if it is interactive, we do want to attach
 # so that the user can send e.g. ^C through.
@@ -127,7 +133,7 @@ ifeq ($(INTERACTIVE), 1)
 endif
 
 .PHONY: docker-run
-docker-run: docker-build ## Run the container in docker, you can use EXTRA_ARGS
+docker-run: docker-build
 	@echo "+ $@"
 	docker run --rm -i $(DOCKER_FLAGS) \
 		quay.io/$(QUAY_ORGANIZATION)/$(QUAY_REGISTRY)-$(NAME):$(GITCOMMIT) $(ARGS)

backup/pvc/VERSION.txt

@@ -1 +1 @@
-v0.2.5
+v0.3.0

backup/pvc/bin/get-latest.sh

@@ -2,9 +2,38 @@
 
 set -eo pipefail
 
-[[ -z "${BACKUP_DIR}" ]] && echo "Required 'BACKUP_DIR' env not set" && exit 1
+is_backup_not_exist() {
+    local backup_dir="$1"
+    # Save the current value of 'set -e'
+    local previous_e
+    previous_e=$(set +e; :; echo $?)
+
+    # Temporarily turn off 'set -e'
+    set +e
+
+    # Run ls command to check if any files matching the pattern exist
+    ls "${backup_dir}"/*.tar.* 1> /dev/null 2>&1
+
+    # Store the exit status of the ls command
+    local ls_exit_status=$?
+
+    # Restore the previous value of 'set -e'
+    [ "$previous_e" = "0" ] && set -e
+
+    # Return true if ls command succeeded (no files found), otherwise return false
+    [ $ls_exit_status -ne 0 ]
+}
+
+[[ -z "${BACKUP_DIR}" ]] && { echo "Required 'BACKUP_DIR' env not set"; exit 1; }
+
+# Check if we have any backup
+if is_backup_not_exist "${BACKUP_DIR}"; then
+  echo "-1"
+  exit 0
+fi
+
 # Search for all the tar.* inside the backup dir to support the migration between gzip vs zstd
-latest=$(find ${BACKUP_DIR} -name '*.tar.*' -exec basename {} \; | sort -g | tail -n 1)
+latest=$(find "${BACKUP_DIR}"/*.tar.* -maxdepth 0 -exec basename {} \; | sort -g | tail -n 1)
 
 if [[ "${latest}" == "" ]]; then
   echo "-1"

backup/pvc/bin/run.sh

@@ -2,15 +2,70 @@
 
 set -eo pipefail
 
-[[ -z "${BACKUP_DIR}" ]] && echo "Required 'BACKUP_DIR' env not set" && exit 1;
+# Use 60 as default in case BACKUP_CLEANUP_INTERVAL did not set
-[[ -z "${JENKINS_HOME}" ]] && echo "Required 'JENKINS_HOME' env not set" && exit 1;
+BACKUP_CLEANUP_INTERVAL=${BACKUP_CLEANUP_INTERVAL:=60}
+
+# Ensure required environment variables are set
+check_env_var() {
+    if [[ -z "${!1}" ]]; then
+        echo "Required '$1' environment variable is not set"
+        exit 1
+    fi
+}
+
+is_backup_not_exist() {
+    local backup_dir="$1"
+    # Save the current value of 'set -e'
+    local previous_e
+    previous_e=$(set +e; :; echo $?)
+
+    # Temporarily turn off 'set -e'
+    set +e
+
+    # Run ls command to check if any files matching the pattern exist
+    ls "${backup_dir}"/*.tar.* 1> /dev/null 2>&1
+
+    # Store the exit status of the ls command
+    local ls_exit_status=$?
+
+    # Restore the previous value of 'set -e'
+    [ "$previous_e" = "0" ] && set -e
+
+    # Return true if ls command succeeded (no files found), otherwise return false
+    [ $ls_exit_status -ne 0 ]
+}
+
+# Function to find exceeding backups
+find_exceeding_backups() {
+    local backup_dir="$1"
+    local backup_count="$2"
+    # Check if we have any backup
+    if is_backup_not_exist "${backup_dir}"; then
+        echo "backups not found in ${backup_dir}" >&2
+        return
+    fi
+    find "${backup_dir}"/*.tar.zstd -maxdepth 0 -exec basename {} \; | sort -gr | tail -n +$((backup_count +1))
+}
+
+check_env_var "BACKUP_DIR"
+check_env_var "JENKINS_HOME"
+
+if [[ -z "${BACKUP_COUNT}" ]]; then
+    echo "ATTENTION! No BACKUP_COUNT set, it means you MUST delete old backups manually or by custom script"
+else
+    echo "Retaining only the ${BACKUP_COUNT} most recent backups, cleanup occurs every ${BACKUP_CLEANUP_INTERVAL} seconds"
+fi
 
 while true;
 do
-    sleep 10
+    sleep "$BACKUP_CLEANUP_INTERVAL"
-    if [[ ! -z "${BACKUP_COUNT}" ]]; then
+    if [[ -n "${BACKUP_COUNT}" ]]; then
-        echo "Trimming to only ${BACKUP_COUNT} recent backups in preparation for new backup"
+        exceeding_backups=$(find_exceeding_backups "${BACKUP_DIR}" "${BACKUP_COUNT}")
-        #TODO: add the list of exceeding backup before delete
+        if [[ -n "$exceeding_backups" ]]; then
-        find ${BACKUP_DIR} -maxdepth 1 -name '*.tar.zstd' -exec basename {} \; | sort -gr | tail -n +$((BACKUP_COUNT +1)) | xargs -I '{}' rm ${BACKUP_DIR}/'{}'
+            echo "Removing backups: $(echo "$exceeding_backups" | tr '\n' ', ' | sed 's/,$//')"
+            echo "$exceeding_backups" | while read -r file; do
+                rm "${BACKUP_DIR}/${file}"
+            done
+        fi
     fi
 done

backup/pvc/config.env

@@ -4,3 +4,4 @@ QUAY_ORGANIZATION=jenkins-kubernetes-operator
 QUAY_REGISTRY=backup
 UID=1000
 GID=1000
+KIND_CLUSTER_NAME=jenkins

backup/pvc/e2e/get-latest/test.sh

@@ -30,6 +30,9 @@ touch ${BACKUP_DIR}/8.tar.zstd
 touch ${BACKUP_DIR}/9.tar.zstd
 touch ${BACKUP_DIR}/10.tar.zstd
 touch ${BACKUP_DIR}/11.tar.zstd
+# Emulate backup creation
+BACKUP_TMP_DIR=$(mktemp -d --tmpdir="${BACKUP_DIR}")
+touch ${BACKUP_TMP_DIR}/12.tar.zstd
 
 # Create an instance of the container under testing
 cid="$(docker run -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
@@ -38,10 +41,20 @@ echo "Docker container ID '${cid}'"
 # Remove test directory and container afterwards
 trap "docker rm -vf $cid > /dev/null;rm -rf ${BACKUP_DIR};rm -rf ${JENKINS_HOME}" EXIT
 
+echo "Try to get latest against 11 backups and one in progress"
 latest=$(docker exec ${cid} /bin/bash -c "JENKINS_HOME=${RESTORE_FOLDER};/home/user/bin/get-latest.sh")
+
 rm ${BACKUP_DIR}/*.tar.zstd
+echo "Try to get latest against one in progress"
 empty_latest=$(docker exec ${cid} /bin/bash -c "JENKINS_HOME=${RESTORE_FOLDER};/home/user/bin/get-latest.sh")
 
+rmdir ${BACKUP_DIR}/lost+found
+rm ${BACKUP_TMP_DIR}/*.tar.zstd
+rmdir ${BACKUP_TMP_DIR}
+echo "Try to get latest against empty dir"
+empty_dir_latest=$(docker exec ${cid} /bin/bash -c "JENKINS_HOME=${RESTORE_FOLDER};/home/user/bin/get-latest.sh")
+
+
 if [[ "${DEBUG}" ]]; then
     docker logs ${cid}
     ls -la ${BACKUP_DIR}
@@ -55,5 +68,9 @@ if [[ ! "${empty_latest}" == "-1" ]]; then
     echo "Latest backup number should be '-1' but is '${empty_latest}'"
     exit 1
 fi
+if [[ ! "${empty_dir_latest}" == "-1" ]]; then
+    echo "Latest backup number should be '-1' but is '${empty_dir_latest}'"
+    exit 1
+fi
 
 echo PASS

backup/pvc/e2e/limit_backup_count/test.sh

@@ -1,6 +1,8 @@
 #!/bin/bash
 set -eo pipefail
 
+echo "Running limit_backup_count e2e test..."
+
 [[ "${DEBUG}" ]] && set -x
 
 # set current working directory to the directory of the script
@@ -30,17 +32,20 @@ touch ${BACKUP_DIR}/8.tar.zstd
 touch ${BACKUP_DIR}/9.tar.zstd
 touch ${BACKUP_DIR}/10.tar.zstd
 touch ${BACKUP_DIR}/11.tar.zstd
+# Emulate backup creation
+BACKUP_TMP_DIR=$(mktemp -d --tmpdir="${BACKUP_DIR}")
+touch ${BACKUP_TMP_DIR}/12.tar.zstd
 
 # Create an instance of the container under testing
-cid="$(docker run -e BACKUP_COUNT=2 -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
+cid="$(docker run -e BACKUP_CLEANUP_INTERVAL=1 -e BACKUP_COUNT=2 -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
 echo "Docker container ID '${cid}'"
 
 # Remove test directory and container afterwards
 trap "docker rm -vf $cid > /dev/null;rm -rf ${BACKUP_DIR};rm -rf ${JENKINS_HOME}" EXIT
 
-sleep 11
+sleep 2
-touch ${BACKUP_DIR}/12.tar.zstd
+mv ${BACKUP_TMP_DIR}/12.tar.zstd ${BACKUP_DIR}/
-sleep 11
+sleep 2
 
 if [[ "${DEBUG}" ]]; then
     docker logs ${cid}

backup/pvc/e2e/limit_backup_count_no_backups/test.sh

@@ -1,6 +1,8 @@
 #!/bin/bash
 set -eo pipefail
 
+echo "Running limit_backup_count_no_backups e2e test..."
+
 [[ "${DEBUG}" ]] && set -x
 
 # set current working directory to the directory of the script
@@ -19,7 +21,7 @@ mkdir -p ${BACKUP_DIR}
 mkdir -p ${JENKINS_HOME}
 
 # Create an instance of the container under testing
-cid="$(docker run -e BACKUP_COUNT=2 -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
+cid="$(docker run -e BACKUP_CLEANUP_INTERVAL=1 -e BACKUP_COUNT=2 -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
 echo "Docker container ID '${cid}'"
 
 # Remove test directory and container afterwards
@@ -27,8 +29,16 @@ trap "docker rm -vf $cid > /dev/null;rm -rf ${BACKUP_DIR};rm -rf ${JENKINS_HOME}
 
 # container should be running
 echo 'Checking if container is running'
-sleep 11
+sleep 3
+set +e
 docker exec ${cid} echo
+exit_code=$?
+set -e
+if [ $exit_code -ne 0 ]; then
+    echo "container terminated with following logs:"
+    docker logs "${cid}"
+    exit 1
+fi
 echo 'Container is running'
 
 echo PASS

backup/pvc/e2e/tmp_dir_clean_after_backup_creation/test.sh

@@ -1,6 +1,8 @@
 #!/bin/bash
 set -eo pipefail
 
+echo "Running tmp_dir_clean_after_backup_creation e2e test..."
+
 [[ "${DEBUG}" ]] && set -x
 
 # set current working directory to the directory of the script
@@ -18,7 +20,7 @@ BACKUP_DIR="$(pwd)/backup"
 mkdir -p ${BACKUP_DIR}
 
 # Create an instance of the container under testing
-cid="$(docker run -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
+cid="$(docker run -e BACKUP_CLEANUP_INTERVAL=1 -e JENKINS_HOME=${JENKINS_HOME} -v ${JENKINS_HOME}:${JENKINS_HOME}:ro -e BACKUP_DIR=${BACKUP_DIR} -v ${BACKUP_DIR}:${BACKUP_DIR}:rw -d ${docker_image})"
 echo "Docker container ID '${cid}'"
 
 # Remove test directory and container afterwards
@@ -28,6 +30,8 @@ backup_number=1
 docker exec ${cid} /home/user/bin/backup.sh ${backup_number}
 
 [ "$(docker exec ${cid} ls /tmp | grep 'tmp')" ] && echo "tmp directory not empty" && exit 1;
+# We should also check backup directory, since after #1000 we create temp directory at backup filesystem
+[ "$(docker exec ${cid} ls ${BACKUP_DIR} | grep 'tmp')" ] && echo "backup dir consists temp directory" && exit 1;
 
 backup_file="${BACKUP_DIR}/${backup_number}.tar.zstd"
 [[ ! -f ${backup_file} ]] && echo "Backup file ${backup_file} not found" && exit 1;

test/e2e/restorebackup_test.go

@@ -157,7 +157,7 @@ func createJenkinsWithBackupAndRestoreConfigured(name, namespace string) *v1alph
 					},
 					{
 						Name:            containerName,
-						Image:           "virtuslab/jenkins-operator-backup-pvc:v0.1.1",
+						Image:           "quay.io/jenkins-kubernetes-operator/backup-pvc:e2e-test",
 						ImagePullPolicy: corev1.PullIfNotPresent,
 						Env: []corev1.EnvVar{
 							{