Skip to content

exec:support to set custom log path - #112

Merged
samuelkarp merged 1 commit into
containerd:mainfrom
ningmingxiao:custom_args
Jul 15, 2026
Merged

exec:support to set custom log path#112
samuelkarp merged 1 commit into
containerd:mainfrom
ningmingxiao:custom_args

Conversation

@ningmingxiao

@ningmingxiao ningmingxiao commented Jun 11, 2025

Copy link
Copy Markdown
Contributor

I create a pr containerd/containerd#11862 to address containerd/containerd#8972
I want to specify runc log to custom path when exec .

@ningmingxiao
ningmingxiao force-pushed the custom_args branch 5 times, most recently from 4646899 to 43fa230 Compare June 11, 2025 04:58
@ningmingxiao

ningmingxiao commented Jun 11, 2025

Copy link
Copy Markdown
Contributor Author

ping @fuweid @AkihiroSuda @mikebrow can you review this pr thanks?

@ningmingxiao
ningmingxiao force-pushed the custom_args branch 3 times, most recently from 3b92e61 to be762e7 Compare June 11, 2025 05:53
@ningmingxiao
ningmingxiao force-pushed the custom_args branch 9 times, most recently from 1e7ee71 to 8e276f3 Compare June 11, 2025 09:57

@mikebrow mikebrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ningmingxiao

Copy link
Copy Markdown
Contributor Author

thank you

Comment thread runc.go Outdated
Comment thread runc.go Outdated
@ningmingxiao

Copy link
Copy Markdown
Contributor Author

ping @fuweid

@ningmingxiao

ningmingxiao commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

@thaJeztah can you review this pr? docker may have same issue.

@ningmingxiao

Copy link
Copy Markdown
Contributor Author

‌PTAL‌ @dmcgowan try to fix containerd/containerd#8972

@thaJeztah

Copy link
Copy Markdown
Member

Looks like this PR will auto-close the containerd issue because the PR description has "to fix <ticket>"

Can someone with permissions edit the description to remove that part (change it to "to address")?

@ningmingxiao

Copy link
Copy Markdown
Contributor Author

Looks like this PR will auto-close the containerd issue because the PR description has "to fix <ticket>"

Can someone with permissions edit the description to remove that part (change it to "to address")?

done

@thaJeztah

Copy link
Copy Markdown
Member

Thx! Commented from my phone, and didn't review yet, but thought I'd comment before github got trigger happy and close the ticket pre-maturely ☺️

Comment thread runc.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for overriding the runc --log path specifically for exec calls, enabling a per-exec log destination without changing the global Runc.Log setting.

Changes:

  • Add ExecOpts.LogFile and wire it into Exec() so runc can be invoked with a custom --log path for exec.
  • Refactor command construction to support an optional log override (via commandWithCustomLogFile and args(logFile)).
  • Add a unit test for log-override argument behavior and pin containerd/project-checks to a specific commit in CI.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
runc.go Adds ExecOpts.LogFile, routes Exec() through a log-override capable command path, and updates global arg construction to accept an override.
command_linux.go Introduces commandWithCustomLogFile and passes the override into r.args(logFile) on Linux.
command_other.go Introduces commandWithCustomLogFile and passes the override into r.args(logFile) on non-Linux platforms.
runc_test.go Adds a test around log-file override behavior for argument construction.
.github/workflows/ci.yml Pins containerd/project-checks to a specific commit (annotated as v1.2.2).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread runc.go
Comment thread runc_test.go
@ningmingxiao

ningmingxiao commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

done thanks @samuelkarp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread runc_test.go
Comment thread runc.go
Signed-off-by: ningmingxiao <ning.mingxiao@zte.com.cn>
Signed-off-by: 宁明晓10296073 <ning.mingxiao@zte.com.cn>
@ningmingxiao

Copy link
Copy Markdown
Contributor Author

done @samuelkarp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread runc.go
@ningmingxiao

Copy link
Copy Markdown
Contributor Author

can this pr be merged @samuelkarp ?

@samuelkarp

Copy link
Copy Markdown
Member

@mikebrow can you re-review since it's been a year since your last approval?

@mikebrow mikebrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ningmingxiao

Copy link
Copy Markdown
Contributor Author

ping @samuelkarp

@samuelkarp
samuelkarp merged commit 293a041 into containerd:main Jul 15, 2026
5 checks passed
@ningmingxiao
ningmingxiao deleted the custom_args branch July 15, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants