Enhancement/5235 insufficient disk handling retry shows underlying error #9122

kaanyalti · 2025-07-24T22:15:53Z

PR 1/6

Enhancement

What does this PR do?

Updates the upgrade downloaders to return insufficient disk error
Introduces custom error for insufficient disk space
Insufficient disk errors stops download retries

Why is it important?

The current error is less user friendly and includes unnecessary information
Disk errors get replaced by context deadline error when retries time out, this pr fixes this for disk space errors.

Checklist

I have read and understood the pull request guidelines of this project.
My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
~~[ ] I have made corresponding changes to the documentation~~
~~[ ] I have made corresponding change to the default configuration files~~
I have added tests that prove my fix is effective or that my feature works
I have added an entry in ./changelog/fragments using the changelog tool
I have added an integration test or an E2E test

Disruptive User Impact

None, unless user relies on specific error messages in upgrade details in the status output.

How to test this PR locally

Build for windows, mac, linux
Install agent (managed and standalone)
Fill up disk until there is approximately 200mb left
Trigger upgrade
- From fleet
- Cli with remote url and file
Validate the upgrade detail error message shows insufficient disk error message both in the status output and on fleet ui.

Related issues

mergify · 2025-07-24T22:16:34Z

This pull request does not have a backport label. Could you fix it @kaanyalti? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-./d./d is the label that automatically backports to the 8./d branch. /d is the digit
backport-active-all is the label that automatically backports to all active branches.
backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

internal/pkg/agent/application/upgrade/insufficient_disk_space_err_windows.go

kaanyalti · 2025-08-04T14:09:56Z

/testthis

kaanyalti · 2025-08-04T14:10:39Z

failing due to known flaky test

elasticmachine · 2025-08-05T08:50:19Z

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

internal/pkg/agent/application/upgrade/errors/disk_space.go

ycombinator · 2025-08-06T00:25:45Z

Hey @kaanyalti, I'm having a bit of a hard time reviewing this PR because of the amount of changes in it. I understand most of them are related to testing — making the code more testable and/or adding more test coverage — and that's a good thing. But I do think these changes have made the code more indirect and harder to review.

Would it be possible to break this PR up into multiple PRs, each one only containing a set of related changes? For example, you're introducing a ProgressReporter type in this PR and tests + code related to it. Would it be possible to extract these changes into their own separate PR, then have a follow up PR that builds upon them with another set of related changes, and so on? Ultimately, it would be ideal if there was a final PR in such a series that introduced ErrInsufficientDiskSpace and only any changes strictly related to that.

kaanyalti · 2025-08-06T10:27:58Z

Hey @kaanyalti, I'm having a bit of a hard time reviewing this PR because of the amount of changes in it. I understand most of them are related to testing — making the code more testable and/or adding more test coverage — and that's a good thing. But I do think these changes have made the code more indirect and harder to review.

Would it be possible to break this PR up into multiple PRs, each one only containing a set of related changes? For example, you're introducing a ProgressReporter type in this PR and tests + code related to it. Would it be possible to extract these changes into their own separate PR, then have a follow up PR that builds upon them with another set of related changes, and so on? Ultimately, it would be ideal if there was a final PR in such a series that introduced ErrInsufficientDiskSpace and only any changes strictly related to that.

Yes I can split the PR, will start posting the smaller PRs right away

kaanyalti · 2025-08-11T12:10:00Z

After having a discussion with @ycombinator I switched my approach to mocking stdlib functions and simplified the tests. This way I was able to remove much of the complexity from before and reduced the amount of changes. I won't be splitting the PR, however I'd still be happy to do so if it'll help reviewers.

internal/pkg/agent/application/upgrade/common/stdlib_wrappers.go

internal/pkg/agent/application/upgrade/artifact/download/errors/disk_space_errors_unix.go

internal/pkg/agent/application/upgrade/artifact/download/fs/downloader.go

internal/pkg/agent/application/upgrade/artifact/download/http/downloader.go

internal/pkg/agent/application/upgrade/artifact/download/fs/downloader.go

internal/pkg/agent/application/upgrade/step_unpack_test.go

internal/pkg/agent/application/upgrade/artifact/download/fs/downloader.go

internal/pkg/agent/application/upgrade/artifact/download/fs/downloader_test.go

kaanyalti · 2025-09-04T12:01:43Z

Quick update, caught a logic error. IsDiskSpaceError should not be returning true for ErrInsufficientDiskSpace. With the current implementation we can end up chaining multiple ErrInsufficientDiskSpace which is not what we want to do. I will address this tomorrow in the morning in a new commit, but the rest of the PR is still valid and ready for review.

Fixed this logic error in my last commit

@ycombinator @pkoutsovasilis @blakerouse @michalpristas

elasticmachine · 2025-09-04T12:52:26Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 51cbb2e

Failed CI Steps

History

💛 Build #26068 was flaky 1b7cee8
💔 Build #26027 failed 2df4465
💔 Build #26022 failed 6982b9d
💛 Build #25485 was flaky 621e9a6
💛 Build #25288 was flaky 4197209
💚 Build #25214 succeeded 18a8b8c

cc @kaanyalti

kaanyalti · 2025-09-04T13:00:00Z

Adressing ci failure

kaanyalti · 2025-09-04T13:02:11Z

Addressed the ci failure with my last commit

elastic-sonarqube · 2025-09-04T14:44:47Z

Quality Gate passed

Issues
2 New issues
1 Fixed issue
0 Accepted issues

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

See analysis details on SonarQube

internal/pkg/agent/application/upgrade/artifact/download/fs/downloader.go

internal/pkg/agent/application/upgrade/artifact/download/http/downloader.go

internal/pkg/agent/application/upgrade/upgrade_test.go

ycombinator

Left a few minor comments / nits but overall LGTM!

github-actions · 2025-09-08T13:53:48Z

@Mergifyio backport 8.18 8.19 9.0 9.1

mergify · 2025-09-08T13:54:03Z

backport 8.18 8.19 9.0 9.1

✅ Backports have been created

#9797 [8.18] (backport #9122) Enhancement/5235 insufficient disk handling retry shows underlying error has been created for branch 8.18 but encountered conflicts
#9798 [8.19] (backport #9122) Enhancement/5235 insufficient disk handling retry shows underlying error has been created for branch 8.19
#9799 [9.0] (backport #9122) Enhancement/5235 insufficient disk handling retry shows underlying error has been created for branch 9.0 but encountered conflicts
#9800 [9.1] (backport #9122) Enhancement/5235 insufficient disk handling retry shows underlying error has been created for branch 9.1

…ror (#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de) # Conflicts: # internal/pkg/agent/application/upgrade/artifact/download/http/downloader_test.go

…ror (#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de)

…ror (#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de) # Conflicts: # internal/pkg/agent/application/upgrade/artifact/download/http/downloader_test.go # internal/pkg/agent/application/upgrade/upgrade.go

…ror (#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de)

…ror (#9122) (#9798) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de) Co-authored-by: Kaan Yalti <[email protected]>

…ror (#9122) (#9800) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de) Co-authored-by: Kaan Yalti <[email protected]>

…try shows underlying error (#9799) * Enhancement/5235 insufficient disk handling retry shows underlying error (#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de) # Conflicts: # internal/pkg/agent/application/upgrade/artifact/download/http/downloader_test.go # internal/pkg/agent/application/upgrade/upgrade.go * resolved conflicts --------- Co-authored-by: Kaan Yalti <[email protected]>

…etry shows underlying error (#9797) * Enhancement/5235 insufficient disk handling retry shows underlying error (#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case (cherry picked from commit 134b3de) # Conflicts: # internal/pkg/agent/application/upgrade/artifact/download/http/downloader_test.go * resolved conflicts --------- Co-authored-by: Kaan Yalti <[email protected]>

…ror (elastic#9122) * enhancement(5235): added stdlib wrapper functions for testability enhancement(5235): added comment in stdlib wrappers enhancement(5235): added license header in stdlib wrappers * enhancement(5235): using stdlib wrappers in http downloader enhancement(5235): added comments explaining why stdlib wrappers are used in http downloader * enhancement(5235): using stdlib wrappers in fs downloader enhancement(5235): added comments in fs downloader explaining why stdlib wrappers are used * enhancement(5235): added stlib mocker utils enhancement(5235): ran mage update enhancement(5235): updated the stdlib wrapper mocker to use map instead of switch * enhancement(5235): wrapping errors in http downloader * enhancement(5235): wrapping errors in fs downloader * enhancement(5235): added disk space error and relevant tests * enhancement(5235): added diskspace error check in http downloader before calling reporter enhancement(5235): added comment in http downloader explaining why diskpace error chec is called before the download progress reporter call enhancement(5235): instead of overwriting the error we are using a copy to report. the returned error is now wrapped in the htttp downloadfile function * enhancement(5235): added http downloader test for disk space error enhancement(5235): using stdlib mocker in http donwloader test enhancement(5235): removed unnecessary fmt enhancement(5235): added state message assertion in http downloader test enhancement(5235): using common stdlib mock util in http downloader test enhancement(5235): updated http downloader test * enhancement(5235): added fs downloader disk space error tests enhancement(5235): using common stdlib mock util in fs downloader * enhancement(5235): replaced errors.New with fmt.Errorf in step_download * enhancement(5235): wrapping error in backoff.Permanent in download with retries if the error is a diskspace error. enhancement(5235): added downloaderrors import in step download * enhancement(5235): added insufficient disk space test case in step download tests * enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): added archive helper functions in upgrade tests enhancement(5235): added comments for the archive helper functions enhancement(5235): buildArchiveFiles archive helper function * enhancement(5235): added deferred error handler in upgrade function to add insufficient disk space error into the error chain enhancement(5235): added comments for the deferred error handler in upgrade func * enhancement(5235): added upgrader test for download error handling enhancement(5235): updated the download error handling test, added additional assertions in mocked functions to make sure that they are called enhancement(5235): added comments explaining steps taken in upgrader test enhancement(5235): using common stdlib mock util in upgrade test enhancement(5235): removed error string comparison * enhancement(5235): remove stdlib wrappers and mock utils * enhancement(5235): add stdlib funcs in http downloader struct, update http downloader tests * enhancement(5235): add stdlib funcs in fs downloader struct and update tests * enhancement(5235): separated artifact downloader from upgrader, updated relevant tests * enhancement(5235): added artifact downloader interface, updated SetClient function to set downloader fleet server uri. Updated upgrader tests * enhancement(5235): fix fs downloader tests * enhancement(5235): added comment in upgrader struct * enhancement(5235): refactored upgrade test for readabiltiy * enhancement(5235): updated disk space error check function to return false for ErrInsufficientDiskSpace * enhancement(5235): fixed download with retries error handling test case

mergify bot assigned kaanyalti Jul 24, 2025

kaanyalti added the backport-active-all Automated backport with mergify to all the active branches label Jul 25, 2025

michalpristas reviewed Jul 28, 2025

View reviewed changes

internal/pkg/agent/application/upgrade/insufficient_disk_space_err_windows.go Outdated Show resolved Hide resolved

kaanyalti force-pushed the enhancement/5235_insufficient_disk_handling_retry_shows_underlying_error branch from fc9adb5 to 7e18028 Compare July 31, 2025 10:39

kaanyalti marked this pull request as ready for review July 31, 2025 13:41

kaanyalti requested a review from a team as a code owner July 31, 2025 13:41

kaanyalti requested review from michalpristas and ycombinator July 31, 2025 13:41

kaanyalti mentioned this pull request Aug 4, 2025

Enhancement/5235 upgrade cleanup after error #9250

Closed

8 tasks

pierrehilbert added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label Aug 5, 2025

ycombinator reviewed Aug 5, 2025

View reviewed changes

internal/pkg/agent/application/upgrade/errors/disk_space.go Outdated Show resolved Hide resolved

kaanyalti force-pushed the enhancement/5235_insufficient_disk_handling_retry_shows_underlying_error branch from 0aedf85 to 7789dd3 Compare August 11, 2025 12:04

kaanyalti requested a review from ycombinator August 11, 2025 13:13

kaanyalti mentioned this pull request Aug 12, 2025

Enhancement/5235 handle insufficient disk space errors in artifact unpack #9322

Merged

5 tasks