Skip to content

fix(tooler): detect both legacy docker-compose and the docker cli compose plugin#126

Closed
smaEti wants to merge 3 commits into
SigNoz:mainfrom
smaEti:main
Closed

fix(tooler): detect both legacy docker-compose and the docker cli compose plugin#126
smaEti wants to merge 3 commits into
SigNoz:mainfrom
smaEti:main

Conversation

@smaEti
Copy link
Copy Markdown

@smaEti smaEti commented May 25, 2026

Fixes

  • dockercomposetooler.Gauge now detects the Docker CLI Compose plugin in addition to the legacy docker-compose binary

Context

Gauge previously called AnyOneExecChecker(ctx, "docker-compose", "docker compose"), which resolves to exec.LookPath on each name. The "docker compose" arm could never succeed, the modern Compose ships as a Docker CLI plugin under Docker's plugin directory, not as a standalone binary on PATH. The check only passed incidentally on systems that also expose a docker-compose shim.

Changes

  • internal/tooler/dockercomposetooler/tooler.go: replace the single AnyOneExecChecker call with explicit fallback:
    • ExecChecker("docker-compose"): legacy binary (unchanged behaviour)
    • else ExecChecker("docker") + docker compose version: actually invokes the CLI so the plugin dispatch can resolve (mirrors dockerswarmtooler)
    • else errors.Newf(errors.TypeNotFound, …) with a descriptive message

Test plan

  • System with only legacy docker-compose — passes
  • System with only the docker compose plugin — passes
  • System with neither — returns TypeNotFound with a clear message

@smaEti smaEti requested a review from therealpandey as a code owner May 25, 2026 23:19
@Nageshbansal Nageshbansal added safe-to-test Run CI on external PRs. and removed safe-to-test Run CI on external PRs. labels May 26, 2026
@Nageshbansal Nageshbansal changed the title dockercomposetooler: detect both legacy docker-compose and the Docker CLI compose plugin fix(tooler): detect both legacy docker-compose and the docker cli compose plugin May 26, 2026
@Nageshbansal Nageshbansal added the safe-to-test Run CI on external PRs. label May 26, 2026
@Nageshbansal
Copy link
Copy Markdown
Member

Hey @smaEti, can you push a small commit (or an empty git commit --allow-empty -m "ci: retrigger" && git push) to retrigger CI? The postci labeler job failed due to a stale event payload, and a fresh push should clear it. Thanks!

@Nageshbansal Nageshbansal added safe-to-test Run CI on external PRs. and removed safe-to-test Run CI on external PRs. labels May 27, 2026
@Nageshbansal
Copy link
Copy Markdown
Member

Hey @smaEti, thanks for this. It's approved, and the code checks are fine. The only thing blocking the merge is our branch protection requiring signed commits, and the commits on this PR are currently unsigned. Could you re-sign them and update the pr?

@smaEti
Copy link
Copy Markdown
Author

smaEti commented May 28, 2026

Hey @smaEti, thanks for this. It's approved, and the code checks are fine. The only thing blocking the merge is our branch protection requiring signed commits, and the commits on this PR are currently unsigned. Could you re-sign them and update the pr?

Hey,
I ran into some issues while trying to re-sign the commits on this PR because there’s a merge commit in the history that makes the rebase process messy and complicated. (because i commited into the main branch of the forked repo, mybad :) )

Because of that, I’ll create a new PR with the same changes on a clean branch with properly signed commits.

Could you close this PR and review the new one instead?
Sorry for the inconvenience.

@Nageshbansal
Copy link
Copy Markdown
Member

Hey @smaEti, no issues. Please create a new PR. Thanks :)

@therealpandey
Copy link
Copy Markdown
Member

Works @smaEti :)

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

Labels

safe-to-test Run CI on external PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants