Skip to content

[8.19] (backport #13710) Remove Go 1.24 workaround in Windows uninstall#13903

Merged
swiatekm merged 2 commits into
8.19from
mergify/bp/8.19/pr-13710
Apr 30, 2026
Merged

[8.19] (backport #13710) Remove Go 1.24 workaround in Windows uninstall#13903
swiatekm merged 2 commits into
8.19from
mergify/bp/8.19/pr-13710

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Apr 29, 2026

What does this PR do?

Removes a workaround for uninstall on Windows that we added with the upgrade to Go 1.25 in #10156. We go back to using the Go standard library's os.RemoveAll and fix the underlying issue.

The underlying issue is roughly that our logic to delete the running executable never worked correctly, and since we ignore errors from it, we never realized. It so happened that the part that did work was sufficient for the more permissive os.RemoveAll implementation from Go 1.24, but not the one from Go 1.25.

The fix is simply to not reopen the file handle after renaming the Alternate Data Stream. Then deleting the handle succeeds and os.RemoveAll skips over the entry when enumerating directory content.

I added a fairly verbose test that checks the status of the handle after each step using windows syscalls. The intent of it is to explain how and why it works. See this comment for a high level summary of the underlying semantics.

For completeness' sake, the removeBlockingExe implementation has various other problems that don't impact the effectiveness, but make the whole thing more confusing. I'm planning to fix those in a follow-up and add more tests. This fix is intentionally minimal to make it easier to understand.

Why is it important?

Our uninstall process should be understandable rather than mysterious.

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

Related issues


This is an automatic backport of pull request #13710 done by [Mergify](https://mergify.com).

* Remove Go 1.24 workaround in Windows uninstall

* Drop some unnecessary comments

(cherry picked from commit 2b8c0aa)
@mergify mergify Bot added the backport label Apr 29, 2026
@mergify mergify Bot requested a review from a team as a code owner April 29, 2026 14:59
@mergify mergify Bot requested review from samuelvl and ycombinator and removed request for a team April 29, 2026 14:59
@github-actions github-actions Bot added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team skip-changelog labels Apr 29, 2026
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

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

@swiatekm swiatekm enabled auto-merge (squash) April 29, 2026 15:04
@swiatekm swiatekm merged commit b92cdd6 into 8.19 Apr 30, 2026
18 checks passed
@swiatekm swiatekm deleted the mergify/bp/8.19/pr-13710 branch April 30, 2026 16:30
@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

History

cc @swiatekm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants