Merge pull request #1504 from bwitt/error_outofspace

Return error on out of space

Author: neolynx

.github/workflows/ci.yml

@@ -17,8 +17,32 @@ env:
   DEBIAN_FRONTEND: noninteractive
 
 jobs:
+  unit-test:
+    name: "Unit Tests"
+    runs-on: ubuntu-22.04
+    continue-on-error: false
+    timeout-minutes: 30
+    steps:
+      - name: "Checkout Repository"
+        uses: actions/checkout@v4
+        with:
+          # fetch the whole repo for `git describe` to work
+          fetch-depth: 0
+      - name: "Docker Image"
+        run: |
+          make docker-image
+      - name: "Unit Tests"
+        run: |
+          make docker-unit-tests
+          mkdir -p out/coverage
+          mv unit.out out/coverage/
+      - uses: actions/upload-artifact@v4
+        with:
+          name: unit-tests-coverage
+          path: out/
+
   test:
-    name: "Test (Ubuntu 22.04)"
+    name: "System Test"
     runs-on: ubuntu-22.04
     continue-on-error: false
     timeout-minutes: 30
@@ -63,21 +87,10 @@ jobs:
         with:
           directory: ${{ runner.temp }}
 
-      - name: "Run Unit Tests"
-        env:
-          RUN_LONG_TESTS: 'yes'
-          AZURE_STORAGE_ENDPOINT: "http://127.0.0.1:10000/devstoreaccount1"
-          AZURE_STORAGE_ACCOUNT: "devstoreaccount1"
-          AZURE_STORAGE_ACCESS_KEY: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=="
-          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
-          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
-        run: |
-          sudo mkdir -p /srv ; sudo chown runner /srv
-          COVERAGE_DIR=${{ runner.temp }} make test
-
       - name: "Run Benchmark"
         run: |
-          COVERAGE_DIR=${{ runner.temp }} make bench
+          mkdir -p out/coverage
+          COVERAGE_DIR=$PWD/out/coverage make bench
 
       - name: "Run System Tests"
         env:
@@ -89,22 +102,53 @@ jobs:
           AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
         run: |
           sudo mkdir -p /srv ; sudo chown runner /srv
-          COVERAGE_DIR=${{ runner.temp }} make system-test
+          mkdir -p out/coverage
+          COVERAGE_DIR=$PWD/out/coverage make system-test
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: system-tests-coverage
+          path: out/
+
+  coverage:
+    name: "Upload Coverage"
+    runs-on: ubuntu-22.04
+    continue-on-error: false
+    timeout-minutes: 30
+    needs:
+      - unit-test
+      - test
+    steps:
+      - name: "Checkout Repository"
+        uses: actions/checkout@v4
+
+      - name: "Download Unit Test Coverage"
+        uses: actions/download-artifact@v4
+        with:
+          name: unit-tests-coverage
+
+      - name: "Download System Test Coverage"
+        uses: actions/download-artifact@v4
+        with:
+          name: system-tests-coverage
 
       - name: "Merge Code Coverage"
         run: |
           go install github.com/wadey/gocovmerge@v0.0.0-20160331181800-b5bfa59ec0ad
-          ~/go/bin/gocovmerge unit.out ${{ runner.temp }}/*.out > coverage.txt
+          ~/go/bin/gocovmerge coverage/*.out > coverage.txt
 
       - name: "Upload Code Coverage"
-        uses: codecov/codecov-action@v2
+        uses: codecov/codecov-action@v5
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: coverage.txt
+          fail_ci_if_error: true
+
 
   ci-debian-build:
     name: "Build"
-    needs: test
+    needs:
+      - coverage
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -224,7 +268,8 @@ jobs:
 
   ci-binary-build:
     name: "Build"
-    needs: test
+    needs:
+      - coverage
     runs-on: ubuntu-latest
     strategy:
       matrix:

AUTHORS

@@ -78,3 +78,4 @@ List of contributors, in chronological order:
 * Juan Calderon-Perez (https://github.com/gaby)
 * Ato Araki (https://github.com/atotto)
 * Roman Lebedev (https://github.com/LebedevRI)
+* Brian Witt (https://github.com/bwitt)

Makefile

@@ -8,7 +8,7 @@ GOOS=$(shell go env GOHOSTOS)
 GOARCH=$(shell go env GOHOSTARCH)
 
 export PODMAN_USERNS = keep-id
-DOCKER_RUN = docker run --security-opt label=disable -it --user 0:0 --rm -v ${PWD}:/work/src
+DOCKER_RUN = docker run --security-opt label=disable --user 0:0 --rm -v ${PWD}:/work/src
 
 # Setting TZ for certificates
 export TZ=UTC
@@ -185,16 +185,16 @@ docker-image-no-cache:  ## Build aptly-dev docker image (no cache)
 	@docker build --no-cache -f system/Dockerfile . -t aptly-dev
 
 docker-build:  ## Build aptly in docker container
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper build
+	@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper build
 
 docker-shell:  ## Run aptly and other commands in docker container
-	@$(DOCKER_RUN) -p 3142:3142 aptly-dev /work/src/system/docker-wrapper || true
+	@$(DOCKER_RUN) -it -p 3142:3142 aptly-dev /work/src/system/docker-wrapper || true
 
 docker-deb:  ## Build debian packages in docker container
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper dpkg DEBARCH=amd64
+	@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper dpkg DEBARCH=amd64
 
-docker-unit-test:  ## Run unit tests in docker container (add TEST=regex to specify which tests to run)
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper \
+docker-unit-tests:  ## Run unit tests in docker container (add TEST=regex to specify which tests to run)
+	$(DOCKER_RUN) -t --tmpfs /smallfs:rw,size=1m aptly-dev /work/src/system/docker-wrapper \
 		azurite-start \
 		AZURE_STORAGE_ENDPOINT=http://127.0.0.1:10000/devstoreaccount1 \
 		AZURE_STORAGE_ACCOUNT=devstoreaccount1 \
@@ -203,7 +203,7 @@ docker-unit-test:  ## Run unit tests in docker container (add TEST=regex to spec
 		azurite-stop
 
 docker-system-test:  ## Run system tests in docker container (add TEST=t04_mirror or TEST=UpdateMirror26Test to run only specific tests)
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper \
+	@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper \
 		azurite-start \
 		AZURE_STORAGE_ENDPOINT=http://127.0.0.1:10000/devstoreaccount1 \
 		AZURE_STORAGE_ACCOUNT=devstoreaccount1 \
@@ -214,16 +214,16 @@ docker-system-test:  ## Run system tests in docker container (add TEST=t04_mirro
 		azurite-stop
 
 docker-serve:  ## Run development server (auto recompiling) on http://localhost:3142
-	@$(DOCKER_RUN) -p 3142:3142 -v /tmp/cache-go-aptly:/var/lib/aptly/.cache/go-build aptly-dev /work/src/system/docker-wrapper serve || true
+	@$(DOCKER_RUN) -it -p 3142:3142 -v /tmp/cache-go-aptly:/var/lib/aptly/.cache/go-build aptly-dev /work/src/system/docker-wrapper serve || true
 
 docker-lint:  ## Run golangci-lint in docker container
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper lint
+	@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper lint
 
 docker-binaries:  ## Build binary releases (FreeBSD, macOS, Linux generic) in docker container
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper binaries
+	@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper binaries
 
 docker-man:  ## Create man page in docker container
-	@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper man
+	@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper man
 
 mem.png: mem.dat mem.gp
 	gnuplot mem.gp
@@ -240,4 +240,4 @@ clean:  ## remove local build and module cache
 	rm -f unit.out aptly.test VERSION docs/docs.go docs/swagger.json docs/swagger.yaml docs/swagger.conf
 	find system/ -type d -name __pycache__ -exec rm -rf {} \; 2>/dev/null || true
 
-.PHONY: help man prepare swagger version binaries build docker-release docker-system-test docker-unit-test docker-lint docker-build docker-image docker-man docker-shell docker-serve clean releasetype dpkg serve flake8
+.PHONY: help man prepare swagger version binaries build docker-release docker-system-tests docker-unit-test docker-lint docker-build docker-image docker-man docker-shell docker-serve clean releasetype dpkg serve flake8

api/files.go

@@ -13,6 +13,10 @@ import (
 	"github.com/saracen/walker"
 )
 
+// syncFile is a seam to allow tests to force fsync failures (e.g. ENOSPC).
+// In production it calls (*os.File).Sync().
+var syncFile = func(f *os.File) error { return f.Sync() }
+
 func verifyPath(path string) bool {
 	path = filepath.Clean(path)
 	for _, part := range strings.Split(path, string(filepath.Separator)) {
@@ -114,34 +118,69 @@ func apiFilesUpload(c *gin.Context) {
 	}
 
 	stored := []string{}
+	openFiles := []*os.File{}
 
+	// Write all files first
 	for _, files := range c.Request.MultipartForm.File {
 		for _, file := range files {
 			src, err := file.Open()
 			if err != nil {
+				// Close any files we've opened
+				for _, f := range openFiles {
+					_ = f.Close()
+				}
 				AbortWithJSONError(c, 500, err)
 				return
 			}
-			defer func() { _ = src.Close() }()
 
 			destPath := filepath.Join(path, filepath.Base(file.Filename))
 			dst, err := os.Create(destPath)
 			if err != nil {
+				_ = src.Close()
+				// Close any files we've opened
+				for _, f := range openFiles {
+					_ = f.Close()
+				}
 				AbortWithJSONError(c, 500, err)
 				return
 			}
-			defer func() { _ = dst.Close() }()
 
 			_, err = io.Copy(dst, src)
+			_ = src.Close()
 			if err != nil {
+				_ = dst.Close()
+				// Close any files we've opened
+				for _, f := range openFiles {
+					_ = f.Close()
+				}
 				AbortWithJSONError(c, 500, err)
 				return
 			}
 
+			// Keep file open for batch sync
+			openFiles = append(openFiles, dst)
 			stored = append(stored, filepath.Join(c.Params.ByName("dir"), filepath.Base(file.Filename)))
 		}
 	}
 
+	// Sync all files at once to catch ENOSPC errors
+	for i, dst := range openFiles {
+		err := syncFile(dst)
+		if err != nil {
+			// Close all files
+			for _, f := range openFiles {
+				_ = f.Close()
+			}
+			AbortWithJSONError(c, 500, fmt.Errorf("error syncing file %s: %s", stored[i], err))
+			return
+		}
+	}
+
+	// Close all files
+	for _, dst := range openFiles {
+		_ = dst.Close()
+	}
+
 	apiFilesUploadedCounter.WithLabelValues(c.Params.ByName("dir")).Inc()
 	c.JSON(200, stored)
 }