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.

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

Copy the content from the underlying image into the newly created volume. #10911

Closed
wants to merge 0 commits into from
Closed

Copy the content from the underlying image into the newly created volume. #10911

wants to merge 0 commits into from

Conversation

vikas-goel
Copy link
Contributor

@vikas-goel vikas-goel commented Jul 12, 2021

The copy happens if prepare_volume_on_create is set to true in containers.conf file.

Docker does this by default.

Fixes: #10262

Signed-off-by: Vikas Goel [email protected]

libpod/runtime_ctr.go Outdated Show resolved Hide resolved
libpod/runtime_ctr.go Outdated Show resolved Hide resolved
libpod/runtime_ctr.go Outdated Show resolved Hide resolved
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.

I think we should split the network setup out from this, you only need storage. CNI setup is expensive. I also think in case of slirp4netns it will leak the process. cleanup() will not kill the slirp process.
Does c.mountStorage() and c.cleanupStorage() works for you?

@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 15, 2021

I think we should split the network setup out from this, you only need storage. CNI setup is expensive. I also think in case of slirp4netns it will leak the process. cleanup() will not kill the slirp process.
Does c.mountStorage() and c.cleanupStorage() works for you?

That will work. Thank you, @Luap99 . I had looked at that option earlier but thought there was some complexity. Not sure why. Checking again after your comment, it seems to be fine.

Currently, I am blocked and waiting for someone to merge the containers/common#678 PR before I can make this change.

@vikas-goel vikas-goel changed the title Prepare container after creation for volume copy-up. Copy the content from the underlying image into the newly created volume. Jul 15, 2021
@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 15, 2021

Hi @mheon ,
| The updated c/common will have to be vendored into Podman to be used once this is merged.

I am not familiar with the command/steps. Could you help me on this one?
Will this require releasing a new version of containers/common repo first? Who can do that?

CC: @rhatdan , @Luap99

@mheon
Copy link
Member

mheon commented Jul 15, 2021

In the top-level directory for the Podman git repo, run:
go get github.com/containers/common@$COMMITHASH (where $COMMITHASH is the hash of the commit you want to vendor)

Then run make vendor-in-container to finalize the changes and commit the result.

Please note that you cannot have symlinks in the path of your working directory when you run these - has to be an absolute path or the commands will misbehave.

@vikas-goel
Copy link
Contributor Author

Thank you @mheon . Looks like a new version of common is not needed per your instruction.
The build is failing for something in containers/buildah

@mheon
Copy link
Member

mheon commented Jul 16, 2021

I think it is asking for Common - from the tests:

libpod/runtime_ctr.go:466:24: r.config.Containers.PrepareVolumeOnCreate undefined (type "github.com/containers/common/pkg/config".ContainersConfig has no field or method PrepareVolumeOnCreate)

@vikas-goel
Copy link
Contributor Author

vikas-goel commented Jul 16, 2021

I think it is asking for Common - from the tests:

libpod/runtime_ctr.go:466:24: r.config.Containers.PrepareVolumeOnCreate undefined (type "github.com/containers/common/pkg/config".ContainersConfig has no field or method PrepareVolumeOnCreate)

Correct. That comment I made was for vendor PR failure.

@vikas-goel
Copy link
Contributor Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 16, 2021

@vikas-goel: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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/test-infra repository.

@vikas-goel
Copy link
Contributor Author

Now that vendor c/common is done, could someone add /ok-to-test message or start a /retest? It requires a "trusted user" review.

@mheon
Copy link
Member

mheon commented Jul 16, 2021

I think you need to rebase on top of latest main and force-push - that will pick up the change and force CI to rerun

@vikas-goel vikas-goel requested a review from cdoern July 16, 2021 21:18
@vikas-goel
Copy link
Contributor Author

@mheon , @rhatdan ,
Do we have a data volume container image in the pipeline to that I can use to write a test case?

Or, do you think creating a configuration file with prepare_volume_on_create = true and creating a container will be good enough?

@mheon
Copy link
Member

mheon commented Jul 16, 2021

Copy-up triggers on any volume, including volumes from -v - our general test is to use -v to bind-mount a host directory over a directory in the container that we know contains files.

@vikas-goel vikas-goel closed this Jul 17, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 17, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vikas-goel

The full list of commands accepted by this bot can be found 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

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data not copying up to volumes for containers created but not started
4 participants