Skip to content

Stop docker watcher in docker autodiscover#50578

Open
belimawr wants to merge 1 commit intoelastic:mainfrom
belimawr:fix-docker-autodiscover-shutdown
Open

Stop docker watcher in docker autodiscover#50578
belimawr wants to merge 1 commit intoelastic:mainfrom
belimawr:fix-docker-autodiscover-shutdown

Conversation

@belimawr
Copy link
Copy Markdown
Member

@belimawr belimawr commented May 8, 2026

Proposed commit message

The docker watcher in the docker autodiscover was not stopped and the Provider Stop method did not wait the provider goroutines to return.

That caused two problems:
 - improper shutdown
 - tests using the testing.T as logger output could panic.

This commit fixes it by calling stop in the watcher and waiting all started goroutines to finish.

Checklist

  • 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. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

## Disruptive User Impact

How to test this PR locally

Run the tests, use -count=2 or a larger value

cd libbeat/autodiscover/providers/docker
go test -count=1 -v -run=TestDockerStart ./... -tags=integration -count=2

Related issues

## Use cases
## Screenshots
## Logs

The docker watcher in the docker autodiscover was not stopped and the
Provider Stop method did not wait the provider goroutines to return.

That caused two problems:
 - improper shutdown
 - tests using the testing.T as logger output could panic.

This commit fixes it by calling stop in the watcher and waiting all
started goroutines to finish.
@belimawr belimawr self-assigned this May 8, 2026
@belimawr belimawr added Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team backport-active-all Automated backport with mergify to all the active branches skip-changelog labels May 8, 2026
@botelastic botelastic Bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 8, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@belimawr belimawr marked this pull request as ready for review May 8, 2026 22:06
@belimawr belimawr requested a review from a team as a code owner May 8, 2026 22:06
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: d8343994-878f-49f9-86aa-58a8009700ae

📥 Commits

Reviewing files that changed from the base of the PR and between 2ddb445 and c848fab.

📒 Files selected for processing (1)
  • libbeat/autodiscover/providers/docker/docker.go

📝 Walkthrough

Walkthrough

This PR adds graceful shutdown coordination to the Docker autodiscover provider. The change introduces a sync.WaitGroup field to track the main event-loop goroutine's lifecycle. During Start(), the goroutine is launched via stopWg.Go() instead of a raw go func(). In Stop(), after signaling the goroutine to exit via the stop channel, the code now waits for the WaitGroup to complete before returning. This ensures the event-loop goroutine finishes its cleanup and stops logging before Stop() returns, addressing the test flake where goroutines log after test completion.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The pull request addresses all requirements from issue #50545: stops the docker watcher and ensures Provider.Stop() waits for goroutines via sync.WaitGroup to prevent logging after test completion.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the identified issue: sync.WaitGroup addition to Provider struct, goroutine coordination in Start/Stop methods, and no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestDockerStart - panic: Log in goroutine after TestDockerStart has completed

1 participant