Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add OCI Volume Source support to crictl [create|run] --with-pull #1464

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Add OCI Volume Source support to crictl [create|run] --with-pull #1464

merged 1 commit into from
Jul 17, 2024

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jun 24, 2024 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adding support for the new OCI Volume Source feature.

Which issue(s) this PR fixes:

Refers to kubernetes/enhancements#4639

Special notes for your reviewer:

Requires kubernetes/kubernetes#125659

Does this PR introduce a user-facing change?

Added support to pull OCI object mounts on container creation if the `crictl [create|run] --with-pull` flag is specified.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 24, 2024
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 24, 2024
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from 7084ef2 to 17764a1 Compare June 24, 2024 09:45
@saschagrunert saschagrunert mentioned this pull request Jun 24, 2024
Open
9 tasks
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from 17764a1 to d8fdb7a Compare June 24, 2024 09:50
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 24, 2024
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch 3 times, most recently from 2019afc to 50a59f0 Compare June 24, 2024 09:56
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from 50a59f0 to 8516ef8 Compare June 24, 2024 10:04
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 24, 2024
@saschagrunert saschagrunert reopened this Jun 24, 2024
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 24, 2024
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch 2 times, most recently from 53ff5e2 to d55497b Compare June 26, 2024 07:04
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2024
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch 2 times, most recently from 48abdd5 to eca281c Compare June 28, 2024 08:53
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from 869f7f9 to fc6c9b1 Compare July 4, 2024 06:51
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 4, 2024
@saschagrunert saschagrunert changed the title WIP: Add OCI Volume Source support Add OCI Volume Source support Jul 4, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2024
@saschagrunert saschagrunert changed the title Add OCI Volume Source support Add OCI Volume Source support to crictl [create|run] --with-pull Jul 4, 2024
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from fc6c9b1 to adec55d Compare July 4, 2024 08:14
@kwilczynski
Copy link
Member

/approve
/lgtm

@k8s-ci-robot
Copy link
Contributor

@kwilczynski: changing LGTM is restricted to collaborators

In response to this:

/approve
/lgtm

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.

haircommander
haircommander reviewed Jul 8, 2024
View reviewed changes
cmd/crictl/container.go Outdated
}

for _, image := range images {
logrus.Infof("Pulling image: %s", image)
Copy link
Contributor

Choose a reason for hiding this comment

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

can this message be a bit clearer that it's being pulled for a mount? also: maybe we should only do this if the pull-on-create flag is set?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed the log messages to reflect which image / OCI object we'd like to pull on which purpose.

I'm not sure to which flag you're referring. Right now it's only enabled when --with-pull is specified on create or run.

@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from adec55d to ab2421d Compare July 9, 2024 07:03
@kwilczynski
Copy link
Member

New changes look good!

/approve
/lgtm

@k8s-ci-robot
Copy link
Contributor

@kwilczynski: changing LGTM is restricted to collaborators

In response to this:

New changes look good!

/approve
/lgtm

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.

@saschagrunert
Copy link
Member Author

@kubernetes-sigs/cri-tools-maintainers PTAL

@saschagrunert saschagrunert changed the title Add OCI Volume Source support to crictl [create|run] --with-pull WIPAdd OCI Volume Source support to crictl [create|run] --with-pull Jul 16, 2024
@saschagrunert saschagrunert changed the title WIPAdd OCI Volume Source support to crictl [create|run] --with-pull WIP: Add OCI Volume Source support to crictl [create|run] --with-pull Jul 16, 2024
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 16, 2024
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from ab2421d to c1daf41 Compare July 17, 2024 07:58
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 17, 2024
@saschagrunert
Add OCI Volume Source support
bd276bf 
Signed-off-by: Sascha Grunert <[email protected]>
@saschagrunert saschagrunert force-pushed the oci-volumesource-cri-tools branch from c1daf41 to bd276bf Compare July 17, 2024 08:00
@saschagrunert saschagrunert changed the title WIP: Add OCI Volume Source support to crictl [create|run] --with-pull Add OCI Volume Source support to crictl [create|run] --with-pull Jul 17, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2024
@saschagrunert
Copy link
Member Author

@kubernetes-sigs/cri-tools-maintainers PTAL

@kwilczynski
Copy link
Member

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 17, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kwilczynski, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit b956b5c into kubernetes-sigs:master Jul 17, 2024
23 checks passed
@saschagrunert saschagrunert deleted the oci-volumesource-cri-tools branch July 17, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haircommander haircommander haircommander left review comments

@Random-Liu Random-Liu Awaiting requested review from Random-Liu

Assignees

@kwilczynski kwilczynski

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@saschagrunert @kwilczynski @k8s-ci-robot @haircommander