Fix disk test timeout by making disk occupation asynchronous by bonzofenix · Pull Request #901 · cloudfoundry/app-autoscaler

bonzofenix · 2026-01-22T19:01:30Z

Summary

Fixes the failing acceptance test by making the disk occupation operation asynchronous.

The test AutoScaler dynamic policy > when there is a scaling policy for diskutil > should scale out and in was failing with a 502 Bad Gateway error because writing 800MB of random data to disk was taking longer than the HTTP timeout (GoRouter backend timeout).

Changes

Made disk occupation operation asynchronous (runs in goroutine)
HTTP response is returned immediately after starting occupation
Proper synchronization with isRunning flag to prevent race conditions
Error handling properly resets the isRunning flag if the operation fails
Removed unused checkAlreadyRunning helper method

Root Cause

The /disk/800/5 endpoint was performing a synchronous blocking operation that:

Generated 800MB of cryptographically random data using crypto/rand.Reader
Wrote it to disk
Only then returned the HTTP response

This operation exceeded the GoRouter's backend timeout, causing 502 errors.

Solution

By making the disk write asynchronous:

The HTTP handler returns immediately with a success response
The disk occupation continues in the background as intended
The test can proceed without waiting for the slow I/O operation to complete
The autoscaler can still detect the disk usage increase and scale accordingly

Testing

This fix should resolve the flaky test failure seen in PR #879 and allow the disk utilization scaling tests to pass consistently.

🤖 Generated with Claude Code

Update scripts/run-acceptance-tests-task.sh Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Fix reviewdog linter issues Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Update error message for missing config in tests to include $ACCEPTANCE_CONFIG_JSON option Update shellcheck fail behavior in linters workflow Enable test coverage for acceptance tests and update shellcheck source references in scripts Disable shellcheck warning and update service broker creation in register-broker.sh; enable test coverage in run-acceptance-tests-task.sh

- Include Go module files and main.go in acceptance build artifacts - Create a placeholder main.go for acceptance tests - Set GOVERSION to go1.23 in mta.tpl.yaml - Change buildpack from binary_buildpack to go_buildpack for acceptance tests

…md64

…o binary_buildpack in mta.tpl.yaml

…ding if the extension file already exists

…inkgo binary compilation in compile-acceptance-tests.sh. Updated messaging in build-extension-file.sh for existing extension file check.

… BOSH login • Replace explicit environment variable checks with default assignments • Remove usage function and associated checks for required variables • Simplify BOSH login process by using a bbl_login function

…rectory

… various scripts

Simplified the BBL_STATE_PATH setup to avoid redundant realpath calls and prevent script failure when the default path doesn't exist. The path is now set to the default first, then validated once with proper error suppression. Co-Authored-By: Claude <noreply@anthropic.com>

Moved BBL_STATE_PATH initialization from vars.source.sh into the bbl_login function in common.sh. This prevents path resolution errors when BBL is not being used and the path doesn't exist. Changes: - Removed BBL_STATE_PATH setup from vars.source.sh - Updated bbl_login() to define and validate BBL_STATE_PATH internally - Removed parameter from all bbl_login() calls across scripts - Simplified validation logic in scripts that checked BBL_STATE_PATH Benefits: - Scripts that don't use BBL won't fail if BBL_STATE_PATH doesn't exist - BBL_STATE_PATH is only resolved when actually needed - Cleaner separation of concerns Co-Authored-By: Claude <noreply@anthropic.com>

• Simplified BOSH login by removing unnecessary path canonicalization and error messages in bbl_login function. • Removed redundant comments and environment variable checks. • Updated cf_target to use non-optional variables for org and space. • Ensured consistent use of BBL_STATE_PATH across scripts for BOSH login. • Clarified error message for missing bbl-state folder in bbl_login.

Co-authored-by: Arsalan Khan <asalan316@hotmail.com>

…script

The disk test was failing with a 502 Bad Gateway error because writing 800MB of random data to disk was taking longer than the HTTP timeout. Changes: - Made disk occupation operation asynchronous (runs in goroutine) - HTTP response is returned immediately after starting occupation - The isRunning flag is set before starting the goroutine to prevent races - On error, the isRunning flag is properly reset This fixes the failing acceptance test: "AutoScaler dynamic policy > when there is a scaling policy for diskutil > should scale out and in" Co-Authored-By: Claude <noreply@anthropic.com>

geigerj0 · 2026-01-25T06:54:40Z

acceptance/assets/app/go_app/internal/app/disk.go

-		return err
-	}
-
-	if err := d.occupy(space); err != nil {


how long does this synchronous occupying of disk space take? what is the configured timeout in the router?

geigerj0 · 2026-01-25T06:55:47Z

acceptance/assets/app/go_app/internal/app/disk.go

 }

 func (d *defaultDiskOccupier) Occupy(space int64, duration time.Duration) error {
-	if err := d.checkAlreadyRunning(); err != nil {


why does the test for diskutil fail (as it was described in the PR description) but the one for disk not? 🤷

app-autoscaler/acceptance/app/dynamic_policy_test.go

Line 328 in 4855d4a

Context("when there is a scaling policy for disk", func() {

geigerj0 · 2026-01-25T06:58:11Z

acceptance/assets/app/go_app/internal/app/disk.go

+	d.mu.Unlock()
+
+	// Start disk occupation asynchronously to avoid HTTP timeout
+	go func() {


Do we need to adjust anything here since this may be running async now? I wonder if the timeouts are still enough. Shall we potentially add a comment explaining why the timeout is longer here?

app-autoscaler/acceptance/app/dynamic_policy_test.go

Lines 320 to 321 in 4855d4a

helpers.StartDiskUsage(cfg, appToScaleName, 800, 5)

helpers.WaitForNInstancesRunning(appToScaleGUID, 2, 5*time.Minute)

geigerj0 · 2026-01-25T06:59:00Z

acceptance/assets/app/go_app/internal/app/disk.go

 }

 func (d *defaultDiskOccupier) Occupy(space int64, duration time.Duration) error {
-	if err := d.checkAlreadyRunning(); err != nil {


The test has not been flaky whenever it got introduced with feat(disk, diskutil): Add new metric type disk and diskutil by geigerj0 · Pull Request #2811 · cloudfoundry/app-autoscaler-release

How come it become "flaky" now out of a sudden?

sonarqubecloud · 2026-01-27T17:17:19Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

app-autoscaler-ci-bot and others added 30 commits January 8, 2026 22:52

Merge branch 'main' into acceptance-test-tasks

d30424a

Add buildpack.yml to acceptance build and download CF CLI for linux_a…

5a15cf0

…md64

Remove buildpack.yml and disable test coverage for acceptance tests

770edaa

Remove coverage option from GINKGO_OPTS in Makefile

159dfa1

Merge branch 'main' into acceptance-test-tasks

dda039a

Remove shell pattern and flags from shellcheck in linters workflow

37e05d4

Remove Go module files and main.go from acceptance build and switch t…

0abe9f0

…o binary_buildpack in mta.tpl.yaml

Add early exit condition in build-extension-file.sh to prevent rebuil…

60de904

…ding if the extension file already exists

Removed duplicate and unnecessary lines in scripts, and streamlined g…

12bb705

…inkgo binary compilation in compile-acceptance-tests.sh. Updated messaging in build-extension-file.sh for existing extension file check.

Update default APP_DB_USER to 'pgadmin' in provision_db.sh

8d43f36

Updated shellcheck source paths in scripts to reference 'scripts/' di…

f912896

…rectory

Merge branch 'main' into acceptance-test-tasks

9ce3651

Updated shellcheck directives to use 'source' for script inclusion in…

4227a56

… various scripts

Small fiexes

5db0d08

Replace rg command with find for listing relevant Go files in Makefile

92922d1

Init claude

5588f20

Add 'analyze-ci' skill for analyzing failed GitHub Action jobs

17fc32d

Merge branch 'main' into acceptance-test-tasks

9e1a9a5

Merge branch 'main' into acceptance-test-tasks

ad06ab3

Merge branch 'main' into acceptance-test-tasks

cab1ce9

Update scripts/build-extension-file.sh

ad38f8d

Co-authored-by: Arsalan Khan <asalan316@hotmail.com>

Add .alexrc to allow specific terms during linting

16afe82

Remove CLAUDE.md and update .gitignore to exclude CLAUDE-related files

3848cd7

Added EOF marker and message output to build-extension-file.sh script

b65becf

bonzofenix and others added 2 commits January 22, 2026 19:01

Add retry logic to task visibility check in run-mta-acceptance-tests …

3decc57

…script

bonzofenix changed the base branch from main to acceptance-test-tasks January 22, 2026 19:02

bonzofenix added the bug label Jan 22, 2026

geigerj0 reviewed Jan 25, 2026

View reviewed changes

Base automatically changed from acceptance-test-tasks to main January 26, 2026 11:45

Merge branch 'main' into fix-disk-test-timeout

e375dc5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix disk test timeout by making disk occupation asynchronous#901

Fix disk test timeout by making disk occupation asynchronous#901
bonzofenix wants to merge 33 commits intomainfrom
fix-disk-test-timeout

bonzofenix commented Jan 22, 2026

Uh oh!

geigerj0 Jan 25, 2026

Uh oh!

geigerj0 Jan 25, 2026

Uh oh!

geigerj0 Jan 25, 2026

Uh oh!

geigerj0 Jan 25, 2026

Uh oh!

sonarqubecloud bot commented Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	helpers.StartDiskUsage(cfg, appToScaleName, 800, 5)
	helpers.WaitForNInstancesRunning(appToScaleGUID, 2, 5*time.Minute)

Conversation

bonzofenix commented Jan 22, 2026

Summary

Changes

Root Cause

Solution

Testing

Uh oh!

geigerj0 Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

geigerj0 Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

geigerj0 Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

geigerj0 Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 27, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants