Skip to content

Conversation

@kaanyalti
Copy link

@kaanyalti kaanyalti commented Aug 13, 2025

PR 3/6

  • Enhancement

What does this PR do?

  • Adds error wrapping in copayActionStore and copyRunDirectory functions so that the errors are properly propagated.
  • Adds tests for copyActionStore, copyRunDirectory and error handling test for the Upgrade function

Why is it important?

This pr makes sure that the errors that originate from copyActionStore and copyRunDirectory get propagated correctly up the call stack.

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

How to test this PR locally

  • Build for windows, mac, linux

  • Install agent (managed and standalone)

  • Fill up disk until almost full

  • 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.

  • Run the upgrade tests in upgrade_test.go

Related issues

@mergify
Copy link
Contributor

mergify bot commented Aug 13, 2025

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.

@kaanyalti kaanyalti force-pushed the enhancement/5235_check_insufficient_disk_space_errors_in_run_dir_action_store_copy branch 3 times, most recently from a731330 to 1ece371 Compare August 13, 2025 19:42
@kaanyalti kaanyalti added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team skip-changelog backport-active-all Automated backport with mergify to all the active branches labels Aug 13, 2025
@kaanyalti kaanyalti force-pushed the enhancement/5235_check_insufficient_disk_space_errors_in_run_dir_action_store_copy branch 3 times, most recently from bd0cc24 to 0c73cde Compare August 13, 2025 20:45
@kaanyalti kaanyalti marked this pull request as ready for review August 13, 2025 20:53
@kaanyalti kaanyalti requested a review from a team as a code owner August 13, 2025 20:53
@elasticmachine
Copy link
Collaborator

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

@kaanyalti kaanyalti force-pushed the enhancement/5235_check_insufficient_disk_space_errors_in_run_dir_action_store_copy branch 2 times, most recently from 6a1da9c to 7e56d22 Compare August 14, 2025 16:25
@kaanyalti kaanyalti force-pushed the enhancement/5235_check_insufficient_disk_space_errors_in_run_dir_action_store_copy branch from 7e56d22 to fa69b70 Compare August 15, 2025 13:59
@kaanyalti
Copy link
Author

kaanyalti commented Aug 15, 2025

The changelog relevant to this PR is added in #9122
The changelog will be in a separate PR

@kaanyalti
Copy link
Author

@pkoutsovasilis @michel-laterman the PR is ready for review

Copy link
Contributor

@pkoutsovasilis pkoutsovasilis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my other comment @kaanyalti I can't tell which files are regarding this PR #9322 (review)

@kaanyalti
Copy link
Author

@pkoutsovasilis I added information on all the PR descriptions addressing the repeated common files. I also updated this comment in the main issue #5235 (comment). Please let me know if these updates clarify the ambiguity around the repeated files.

@elastic-sonarqube
Copy link

@elasticmachine
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

History

cc @kaanyalti

@kaanyalti kaanyalti merged commit 96e0476 into elastic:main Sep 12, 2025
23 checks passed
@github-actions
Copy link
Contributor

@Mergifyio backport 8.18 8.19 9.0 9.1

@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2025

mergify bot pushed a commit that referenced this pull request Sep 12, 2025
…Directory (#9349)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)

# Conflicts:
#	internal/pkg/agent/application/upgrade/upgrade.go
mergify bot pushed a commit that referenced this pull request Sep 12, 2025
…Directory (#9349)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)
mergify bot pushed a commit that referenced this pull request Sep 12, 2025
…Directory (#9349)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)
mergify bot pushed a commit that referenced this pull request Sep 12, 2025
…Directory (#9349)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)
kaanyalti pushed a commit that referenced this pull request Sep 15, 2025
…Directory (#9349) (#9936)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)

Co-authored-by: Kaan Yalti <kaan.yalti@elastic.co>
kaanyalti pushed a commit that referenced this pull request Sep 15, 2025
…Directory (#9349) (#9935)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)

Co-authored-by: Kaan Yalti <kaan.yalti@elastic.co>
kaanyalti pushed a commit that referenced this pull request Sep 15, 2025
…Directory (#9349) (#9934)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)

Co-authored-by: Kaan Yalti <kaan.yalti@elastic.co>
kaanyalti pushed a commit that referenced this pull request Sep 15, 2025
…opyActionDir and copyRunDirectory (#9933)

* Enhancement/5235 correctly wrap errors from copyActionDir and copyRunDirectory (#9349)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name

(cherry picked from commit 96e0476)

# Conflicts:
#	internal/pkg/agent/application/upgrade/upgrade.go

* resolved merge conflicts

---------

Co-authored-by: Kaan Yalti <kaan.yalti@elastic.co>
v1v added a commit that referenced this pull request Sep 16, 2025
* upstream: (26 commits)
  fix: ensure EDOT subprocess shuts down gracefully on agent termination (#9886)
  [main][Automation] Update versions (#9976)
  Add Collector reference docs and automation (#9953)
  [beatreceivers] Integrate beatsauthextension (#9257)
  [main][Automation] Update versions (#9941)
  Update OTel components to v0.132.0/v1.38.0 (#9954)
  Enhancement/5235 wrap errors when marking upgrade (#9366)
  Mount Go build cache into crossbuild container (#9094)
  Liveness agent state (#9673)
  [main][Automation] Bump VM Image version to 1757725254 (#9942)
  Enhancement/5235 correctly wrap errors from copyActionDir and copyRunDirectory (#9349)
  [main][Automation] Update elastic/beats to afc53c0479ac (#9874)
  Add -coverpkg option when running unit test to calculate coverage across packages (#9913)
  Cache binaries downloaded for packaging locally (#9133)
  [main][Automation] Update versions (#9897)
  Disable flaky test TestBeatsReceiverLogs (#9891)
  Allow overriding AGENT_PACKAGE_VERSION and MANIFEST_URL when USE_PACKAGE_VERSION=true (#9864)
  add ingest-docs team as CODEOWNERS for release notes and docset.yml (#9865)
  fix: correct spelling of 'output' in various templates and monitoring code (#9827)
  k8s: Add comment around hostUsers for Universal Profiling deployments (#9847)
  ...
intxgo pushed a commit to intxgo/elastic-agent that referenced this pull request Sep 24, 2025
…Directory (elastic#9349)

* enhancement(5235): added  copyActionStore package var

* enhancement(5235): added copyRunDirectory package var

* enhancement(5235): wrapping errors in copyRunDirectory

enhancement(5235): wrapping mkdriall error in copyrundir function

* enhancement(5235): added package var to abstract third party package function copy.Copy

* enhancement(5235): using copyActionStoreFunc and copyRunDirectoryFunc

* enhancement(5235): wrapping errors in upgrade function

* enhancement(5235): added mkdirall wrapper in copyRunDir

enhancement(5235): added common package import

* enhancement(5235): using writefile wrapper in copyActionStore

* enhancement(5235): using readFile wrapper in copyActionStore

* enhancement(5235): added copyActionStore and copyRunDirectory tests in upgrade_test

enhancement(5235): updated the copyRunDirectory test to mock stdlib functions

* enhancement(5235): added TestUpgradeDirectoryCopyErrors test in upgrade_test

enhancement(5235): added test imports in upgrade_test

* enhancement(5235): added comment in step_unpack

* enhancement(5235): added types in upgrade.go to abstract away functions for testability. abstracted copyActionStore and copyRunDirectory. Updated tests

* enhancement(5235): updated upgrade tests, added tests cases for copyActionStore and copyRunDir error handling, removed unnecessary test, refactored copyActionStore and copyRunDirectory tests

* enhancement(5235): moved where mkdriAllFunc type is declared

* enhancement(5235): remove unnecessary change

* enhancement(5235): added action store path to error message

* enhancement(5235): remove unused commented code

* enhancement(5235): fix typo in test log name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-active-all Automated backport with mergify to all the active branches skip-changelog Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants