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 kube play support for volumes of type image #23819

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Add kube play support for volumes of type image #23819

merged 1 commit into from
Sep 11, 2024
+302 −52

Conversation

l0rd
Copy link
Member

@l0rd l0rd commented Aug 30, 2024 •
edited by ygalblum
Loading

Add the support for Kubernetes volumes of type image.

Does this PR introduce a user-facing change?

kube play supports Kubernetes volumes of type image, introduced as an alpha feature in v1.31

Resolves: #23775

Luap99
Luap99 reviewed Sep 2, 2024
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

Does this address #23775?

cc @ygalblum

pkg/domain/infra/abi/play.go Outdated Show resolved Hide resolved
test/system/700-play.bats Outdated
@@ -1090,6 +1090,87 @@ EOF
run_podman rmi $imgname1 $imgname2
}

@test "podman play with image volumes" {
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be copy pasta from the previous test, please just merge them.
Building the same images several times make the tests slow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Test updated

@l0rd l0rd force-pushed the kube-play-image-type-volumes branch 2 times, most recently from ce54233 to 4121dac Compare September 2, 2024 10:57
ygalblum
ygalblum reviewed Sep 3, 2024
View reviewed changes
Copy link
Contributor

@ygalblum ygalblum 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 PR. Some comments and continued discussion

pkg/domain/infra/abi/play.go Outdated Show resolved Hide resolved
pkg/k8s.io/api/core/v1/types.go Outdated Show resolved Hide resolved
pkg/k8s.io/api/core/v1/types.go Outdated Show resolved Hide resolved
test/system/700-play.bats Outdated Show resolved Hide resolved
@l0rd l0rd force-pushed the kube-play-image-type-volumes branch from 4121dac to a1a0d0c Compare September 9, 2024 14:47
@packit-as-a-service Packit-as-a-Service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@l0rd l0rd force-pushed the kube-play-image-type-volumes branch 2 times, most recently from 3507f88 to 5f090c9 Compare September 9, 2024 16:31
@l0rd
Add kube play support for image volume source
db12343 
Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
@l0rd l0rd force-pushed the kube-play-image-type-volumes branch from 5f090c9 to db12343 Compare September 10, 2024 12:37
@l0rd
Copy link
Member Author

l0rd commented Sep 10, 2024

@ygalblum please have a look. I have updated the PR. The PullPolicy is honored, there are more tests and the Kubernetes API comments are adapted to Podman.

@rhatdan
Copy link
Member

rhatdan commented Sep 10, 2024

LGTM
/approve

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: l0rd, rhatdan

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2024
@ygalblum
Copy link
Contributor

Thanks, I've also updated the initial comment so that the issue will be closed

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 7764bea into containers:main Sep 11, 2024
80 checks passed
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Dec 11, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Luap99 Luap99 Luap99 left review comments

@ygalblum ygalblum ygalblum left review comments

Assignees

@ygalblum ygalblum

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Kube Play - Add support for image based volumes
4 participants
@l0rd @rhatdan @ygalblum @Luap99