Skip to content

Commit 19b98c6

Browse files
authored
Merge pull request #1504 from bwitt/error_outofspace
Return error on out of space
2 parents a70f572 + 06fea59 commit 19b98c6

File tree

7 files changed

+1003
-35
lines changed

7 files changed

+1003
-35
lines changed

.github/workflows/ci.yml

Lines changed: 64 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,32 @@ env:
1717
DEBIAN_FRONTEND: noninteractive
1818

1919
jobs:
20+
unit-test:
21+
name: "Unit Tests"
22+
runs-on: ubuntu-22.04
23+
continue-on-error: false
24+
timeout-minutes: 30
25+
steps:
26+
- name: "Checkout Repository"
27+
uses: actions/checkout@v4
28+
with:
29+
# fetch the whole repo for `git describe` to work
30+
fetch-depth: 0
31+
- name: "Docker Image"
32+
run: |
33+
make docker-image
34+
- name: "Unit Tests"
35+
run: |
36+
make docker-unit-tests
37+
mkdir -p out/coverage
38+
mv unit.out out/coverage/
39+
- uses: actions/upload-artifact@v4
40+
with:
41+
name: unit-tests-coverage
42+
path: out/
43+
2044
test:
21-
name: "Test (Ubuntu 22.04)"
45+
name: "System Test"
2246
runs-on: ubuntu-22.04
2347
continue-on-error: false
2448
timeout-minutes: 30
@@ -63,21 +87,10 @@ jobs:
6387
with:
6488
directory: ${{ runner.temp }}
6589

66-
- name: "Run Unit Tests"
67-
env:
68-
RUN_LONG_TESTS: 'yes'
69-
AZURE_STORAGE_ENDPOINT: "http://127.0.0.1:10000/devstoreaccount1"
70-
AZURE_STORAGE_ACCOUNT: "devstoreaccount1"
71-
AZURE_STORAGE_ACCESS_KEY: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=="
72-
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
73-
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
74-
run: |
75-
sudo mkdir -p /srv ; sudo chown runner /srv
76-
COVERAGE_DIR=${{ runner.temp }} make test
77-
7890
- name: "Run Benchmark"
7991
run: |
80-
COVERAGE_DIR=${{ runner.temp }} make bench
92+
mkdir -p out/coverage
93+
COVERAGE_DIR=$PWD/out/coverage make bench
8194
8295
- name: "Run System Tests"
8396
env:
@@ -89,22 +102,53 @@ jobs:
89102
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
90103
run: |
91104
sudo mkdir -p /srv ; sudo chown runner /srv
92-
COVERAGE_DIR=${{ runner.temp }} make system-test
105+
mkdir -p out/coverage
106+
COVERAGE_DIR=$PWD/out/coverage make system-test
107+
108+
- uses: actions/upload-artifact@v4
109+
with:
110+
name: system-tests-coverage
111+
path: out/
112+
113+
coverage:
114+
name: "Upload Coverage"
115+
runs-on: ubuntu-22.04
116+
continue-on-error: false
117+
timeout-minutes: 30
118+
needs:
119+
- unit-test
120+
- test
121+
steps:
122+
- name: "Checkout Repository"
123+
uses: actions/checkout@v4
124+
125+
- name: "Download Unit Test Coverage"
126+
uses: actions/download-artifact@v4
127+
with:
128+
name: unit-tests-coverage
129+
130+
- name: "Download System Test Coverage"
131+
uses: actions/download-artifact@v4
132+
with:
133+
name: system-tests-coverage
93134

94135
- name: "Merge Code Coverage"
95136
run: |
96137
go install github.com/wadey/[email protected]
97-
~/go/bin/gocovmerge unit.out ${{ runner.temp }}/*.out > coverage.txt
138+
~/go/bin/gocovmerge coverage/*.out > coverage.txt
98139
99140
- name: "Upload Code Coverage"
100-
uses: codecov/codecov-action@v2
141+
uses: codecov/codecov-action@v5
101142
with:
102143
token: ${{ secrets.CODECOV_TOKEN }}
103144
files: coverage.txt
145+
fail_ci_if_error: true
146+
104147

105148
ci-debian-build:
106149
name: "Build"
107-
needs: test
150+
needs:
151+
- coverage
108152
runs-on: ubuntu-latest
109153
strategy:
110154
fail-fast: false
@@ -224,7 +268,8 @@ jobs:
224268

225269
ci-binary-build:
226270
name: "Build"
227-
needs: test
271+
needs:
272+
- coverage
228273
runs-on: ubuntu-latest
229274
strategy:
230275
matrix:

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,4 @@ List of contributors, in chronological order:
7878
* Juan Calderon-Perez (https://github.com/gaby)
7979
* Ato Araki (https://github.com/atotto)
8080
* Roman Lebedev (https://github.com/LebedevRI)
81+
* Brian Witt (https://github.com/bwitt)

Makefile

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ GOOS=$(shell go env GOHOSTOS)
88
GOARCH=$(shell go env GOHOSTARCH)
99

1010
export PODMAN_USERNS = keep-id
11-
DOCKER_RUN = docker run --security-opt label=disable -it --user 0:0 --rm -v ${PWD}:/work/src
11+
DOCKER_RUN = docker run --security-opt label=disable --user 0:0 --rm -v ${PWD}:/work/src
1212

1313
# Setting TZ for certificates
1414
export TZ=UTC
@@ -185,16 +185,16 @@ docker-image-no-cache: ## Build aptly-dev docker image (no cache)
185185
@docker build --no-cache -f system/Dockerfile . -t aptly-dev
186186

187187
docker-build: ## Build aptly in docker container
188-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper build
188+
@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper build
189189

190190
docker-shell: ## Run aptly and other commands in docker container
191-
@$(DOCKER_RUN) -p 3142:3142 aptly-dev /work/src/system/docker-wrapper || true
191+
@$(DOCKER_RUN) -it -p 3142:3142 aptly-dev /work/src/system/docker-wrapper || true
192192

193193
docker-deb: ## Build debian packages in docker container
194-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper dpkg DEBARCH=amd64
194+
@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper dpkg DEBARCH=amd64
195195

196-
docker-unit-test: ## Run unit tests in docker container (add TEST=regex to specify which tests to run)
197-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper \
196+
docker-unit-tests: ## Run unit tests in docker container (add TEST=regex to specify which tests to run)
197+
$(DOCKER_RUN) -t --tmpfs /smallfs:rw,size=1m aptly-dev /work/src/system/docker-wrapper \
198198
azurite-start \
199199
AZURE_STORAGE_ENDPOINT=http://127.0.0.1:10000/devstoreaccount1 \
200200
AZURE_STORAGE_ACCOUNT=devstoreaccount1 \
@@ -203,7 +203,7 @@ docker-unit-test: ## Run unit tests in docker container (add TEST=regex to spec
203203
azurite-stop
204204

205205
docker-system-test: ## Run system tests in docker container (add TEST=t04_mirror or TEST=UpdateMirror26Test to run only specific tests)
206-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper \
206+
@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper \
207207
azurite-start \
208208
AZURE_STORAGE_ENDPOINT=http://127.0.0.1:10000/devstoreaccount1 \
209209
AZURE_STORAGE_ACCOUNT=devstoreaccount1 \
@@ -214,16 +214,16 @@ docker-system-test: ## Run system tests in docker container (add TEST=t04_mirro
214214
azurite-stop
215215

216216
docker-serve: ## Run development server (auto recompiling) on http://localhost:3142
217-
@$(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
217+
@$(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
218218

219219
docker-lint: ## Run golangci-lint in docker container
220-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper lint
220+
@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper lint
221221

222222
docker-binaries: ## Build binary releases (FreeBSD, macOS, Linux generic) in docker container
223-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper binaries
223+
@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper binaries
224224

225225
docker-man: ## Create man page in docker container
226-
@$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper man
226+
@$(DOCKER_RUN) -t aptly-dev /work/src/system/docker-wrapper man
227227

228228
mem.png: mem.dat mem.gp
229229
gnuplot mem.gp
@@ -240,4 +240,4 @@ clean: ## remove local build and module cache
240240
rm -f unit.out aptly.test VERSION docs/docs.go docs/swagger.json docs/swagger.yaml docs/swagger.conf
241241
find system/ -type d -name __pycache__ -exec rm -rf {} \; 2>/dev/null || true
242242

243-
.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
243+
.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

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ import (
1313
"github.com/saracen/walker"
1414
)
1515

16+
// syncFile is a seam to allow tests to force fsync failures (e.g. ENOSPC).
17+
// In production it calls (*os.File).Sync().
18+
var syncFile = func(f *os.File) error { return f.Sync() }
19+
1620
func verifyPath(path string) bool {
1721
path = filepath.Clean(path)
1822
for _, part := range strings.Split(path, string(filepath.Separator)) {
@@ -114,34 +118,69 @@ func apiFilesUpload(c *gin.Context) {
114118
}
115119

116120
stored := []string{}
121+
openFiles := []*os.File{}
117122

123+
// Write all files first
118124
for _, files := range c.Request.MultipartForm.File {
119125
for _, file := range files {
120126
src, err := file.Open()
121127
if err != nil {
128+
// Close any files we've opened
129+
for _, f := range openFiles {
130+
_ = f.Close()
131+
}
122132
AbortWithJSONError(c, 500, err)
123133
return
124134
}
125-
defer func() { _ = src.Close() }()
126135

127136
destPath := filepath.Join(path, filepath.Base(file.Filename))
128137
dst, err := os.Create(destPath)
129138
if err != nil {
139+
_ = src.Close()
140+
// Close any files we've opened
141+
for _, f := range openFiles {
142+
_ = f.Close()
143+
}
130144
AbortWithJSONError(c, 500, err)
131145
return
132146
}
133-
defer func() { _ = dst.Close() }()
134147

135148
_, err = io.Copy(dst, src)
149+
_ = src.Close()
136150
if err != nil {
151+
_ = dst.Close()
152+
// Close any files we've opened
153+
for _, f := range openFiles {
154+
_ = f.Close()
155+
}
137156
AbortWithJSONError(c, 500, err)
138157
return
139158
}
140159

160+
// Keep file open for batch sync
161+
openFiles = append(openFiles, dst)
141162
stored = append(stored, filepath.Join(c.Params.ByName("dir"), filepath.Base(file.Filename)))
142163
}
143164
}
144165

166+
// Sync all files at once to catch ENOSPC errors
167+
for i, dst := range openFiles {
168+
err := syncFile(dst)
169+
if err != nil {
170+
// Close all files
171+
for _, f := range openFiles {
172+
_ = f.Close()
173+
}
174+
AbortWithJSONError(c, 500, fmt.Errorf("error syncing file %s: %s", stored[i], err))
175+
return
176+
}
177+
}
178+
179+
// Close all files
180+
for _, dst := range openFiles {
181+
_ = dst.Close()
182+
}
183+
145184
apiFilesUploadedCounter.WithLabelValues(c.Params.ByName("dir")).Inc()
146185
c.JSON(200, stored)
147186
}

0 commit comments

Comments
 (0)