Skip to content

Update taskOptions based on runtimeOptions when creating a task #11569

Merged
fuweid merged 3 commits intocontainerd:mainfrom
Iceber:fix_task_options
Mar 28, 2025
Merged

Update taskOptions based on runtimeOptions when creating a task #11569
fuweid merged 3 commits intocontainerd:mainfrom
Iceber:fix_task_options

Conversation

@Iceber
Copy link
Member

@Iceber Iceber commented Mar 19, 2025 •
edited
Loading

fix: #11568

related pr: #11442 #11557

@github-project-automation github-project-automation bot moved this to Needs Triage in Pull Request Review Mar 19, 2025
@dosubot dosubot bot added the area/runtime Runtime label Mar 19, 2025
@pacoxu
Copy link
Contributor

pacoxu commented Mar 19, 2025

cc @dims @fuweid

This is a follow up of kubernetes/test-infra#34510 (comment).

@Iceber Iceber force-pushed the fix_task_options branch from 9989386 to 9fb431d Compare March 19, 2025 09:18
@dims
Copy link
Member

dims commented Mar 19, 2025

@Iceber @pacoxu thank you! can we please add tests. specifically that show that the BinaryName is not being passed through? (which if i remember right was the problem why we have to copy runc to the existing $PATH)

Iceber
Iceber commented Mar 20, 2025
View reviewed changes
@mikebrow
Copy link
Member

mikebrow commented Mar 20, 2025

/test pull-containerd-node-e2e

mikebrow
mikebrow reviewed Mar 20, 2025
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

Thanks for the work.. see comments. I think we need to talk. I don't see all the merge discussed .. don't fully understand beyond the revert part what is going on.

fuweid
fuweid reviewed Mar 26, 2025
View reviewed changes
fuweid
fuweid reviewed Mar 26, 2025
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

Thanks @Iceber for this fix!

It looks good to me. And would you please add this test in your pull request? Thanks

fuweid@f83b227

@fuweid fuweid mentioned this pull request Mar 26, 2025
Closed
@fuweid fuweid added cherry-pick/2.0.x Change to be cherry picked to release/2.0 branch priority/P2 labels Mar 26, 2025
@Iceber Iceber force-pushed the fix_task_options branch from 6a9a006 to e464cb4 Compare March 26, 2025 06:54
Iceber and others added 3 commits March 26, 2025 14:58
@Iceber
update taskOptions based on runtimeOptions when creating a task
ec3567d 
Signed-off-by: Iceber Gu <caiwei95@hotmail.com>
@Iceber
prefer task options for PluginInfo request
7e5c503 
Signed-off-by: Iceber Gu <caiwei95@hotmail.com>
@Iceber @fuweid
integration/client: add tests for TaskOptions is not empty
450038a 
Co-authored-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Iceber Gu <caiwei95@hotmail.com>
@Iceber Iceber force-pushed the fix_task_options branch from e464cb4 to 450038a Compare March 26, 2025 07:00
fuweid
fuweid approved these changes Mar 26, 2025
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

mikebrow
mikebrow approved these changes Mar 26, 2025
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

@github-project-automation github-project-automation bot moved this from Needs Triage to Review In Progress in Pull Request Review Mar 26, 2025
@fuweid fuweid added this pull request to the merge queue Mar 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 26, 2025
@fuweid fuweid added this pull request to the merge queue Mar 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 26, 2025
@fuweid fuweid added this pull request to the merge queue Mar 28, 2025
Merged via the queue into containerd:main with commit cac8b40 Mar 28, 2025
58 checks passed
@github-project-automation github-project-automation bot moved this from Review In Progress to Done in Pull Request Review Mar 28, 2025
@fuweid
Copy link
Member

fuweid commented Mar 28, 2025

/cherry-pick release-2.0

@k8s-infra-cherrypick-robot
Copy link

k8s-infra-cherrypick-robot commented Mar 28, 2025

@fuweid: cannot checkout release-2.0: error checking out "release-2.0": exit status 1 error: pathspec 'release-2.0' did not match any file(s) known to git

Details

In response to this:

/cherry-pick release-2.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@fuweid

This comment was marked as duplicate.

Sign in to view
1 similar comment
@fuweid
Copy link
Member

fuweid commented Mar 28, 2025

/cherry-pick release/2.0

@k8s-infra-cherrypick-robot
Copy link

k8s-infra-cherrypick-robot commented Mar 28, 2025

@fuweid: new pull request created: #11618

Details

In response to this:

/cherry-pick release/2.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request Mar 28, 2025
Merged
@fuweid fuweid added cherry-picked/2.0.x PR commits are cherry picked into the release/2.0 branch and removed cherry-pick/2.0.x Change to be cherry picked to release/2.0 branch labels Mar 28, 2025
@jfernandez
Copy link
Contributor

jfernandez commented Apr 9, 2025

@Iceber does this also close #11480 ?

@Iceber
Copy link
Member Author

Iceber commented Apr 9, 2025 •
edited
Loading

@Iceber does this also close #11480 ?

Yeah, this fix also covers that issue. I'll close that. Thanks for the reminder!

@Iceber Iceber linked an issue Apr 9, 2025 that may be closed by this pull request
RuntimeOptions are not passed to the shim when user namespace is enabled #11480
Closed
@Iceber Iceber mentioned this pull request Apr 9, 2025
Closed
@dmcgowan dmcgowan changed the title update taskOptions based on runtimeOptions when creating a task Update taskOptions based on runtimeOptions when creating a task Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dims dims dims left review comments

@mikebrow mikebrow mikebrow approved these changes

@fuweid fuweid fuweid approved these changes

+2 more reviewers

@yylt yylt yylt left review comments

@pacoxu pacoxu pacoxu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/runtime Runtime cherry-picked/2.0.x PR commits are cherry picked into the release/2.0 branch priority/P2 size/L

Projects

Pull Request Review
Archived in project

Milestone

No milestone

9 participants

@Iceber @pacoxu @dims @mikebrow @fuweid @k8s-infra-cherrypick-robot @jfernandez @yylt @k8s-ci-robot