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

volume: add two new options copy and nocopy #14734

Merged
merged 4 commits into from
Jun 28, 2022

Conversation

giuseppe
Copy link
Member

more details in each patch commit message

Does this PR introduce a user-facing change?

Now volume create supports copy and nocopy to control whether files from the container image must be copied up to the newly created volume on the first run.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 25, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

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 Jun 25, 2022
@giuseppe giuseppe marked this pull request as draft June 25, 2022 17:17
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2022
@giuseppe
Copy link
Member Author

the change needed for c/common: containers/common#1075

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

PR on c/common containers/common#1075 is merged so following PR can now vendor c/common.

@giuseppe
Copy link
Member Author

I think it is better to find get #14654 merged

@giuseppe giuseppe force-pushed the copyup-switch-order branch from 4054288 to 9cf5e91 Compare June 27, 2022 15:35
@giuseppe giuseppe marked this pull request as ready for review June 27, 2022 15:39
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 27, 2022
@giuseppe giuseppe force-pushed the copyup-switch-order branch from 9cf5e91 to dc32b4d Compare June 27, 2022 17:11
@baude
Copy link
Member

baude commented Jun 27, 2022

code LGTM

giuseppe added 4 commits June 27, 2022 20:22
Signed-off-by: Giuseppe Scrivano <[email protected]>
avoid any I/O operation on the volume if the source directory is empty.

This is useful on network file systems (since CAP_DAC_OVERRIDE is not
honored) where the root user might not have enough privileges to
perform an I/O operation on the NFS mount but the user running inside
the container has.

[NO NEW TESTS NEEDED] it needs a setup with a network file system

Signed-off-by: Giuseppe Scrivano <[email protected]>
the two operations are equivalent since securejoin.SecureJoin() has
solved the symlinks.  Prefer the Lstat version though to make sure
symlinks are never resolved and we do not end up using a path on the
host.

Signed-off-by: Giuseppe Scrivano <[email protected]>
add two new options to the volume create command: copy and nocopy.

When nocopy is specified, the files from the container image are not
copied up to the volume.

Closes: containers#14722

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the copyup-switch-order branch from dc32b4d to aada13f Compare June 27, 2022 18:22
@rhatdan
Copy link
Member

rhatdan commented Jun 27, 2022

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 27, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2022
@rhatdan
Copy link
Member

rhatdan commented Jun 28, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2022
@openshift-ci openshift-ci bot merged commit 8267cd3 into containers:main Jun 28, 2022
edsantiago added a commit to edsantiago/libpod that referenced this pull request Sep 9, 2022
This one is a nightmare, because --volume has been edited
in four different files throughout the years (five if you
count podman-build, which I am not including in this PR).
Those edits have not always been done in sync.

The list of options was reordered 2022-06-28 by Giuseppe in containers#14734,
but only in podman-create and -run (not in podman-pod-*). No
explanation of why, but I'll assume he knew what he was doing,
and have accepted that for the reference copy.

There was also a big edit in containers#8519.

The "Propagation property...bind mounted" sentence first appeared
in pod-clone, in containers#14299 by cdoern, with no obvious source of where
it came from. I choose to include it in the reference copy.

The "**copy**" option seems to work in pod-create, so I'm including
it in the reference copy. Someone please yell loudly if this is
not the case.

The "disables SELinux separation for containers used in the build",
no idea, changed that to just "for the container/pod"

The "advanced users / overlay / upperdir / workdir" paragraph
makes zero sense to me, but hey, I assume it applies to all
the commands, so I put it in the reference copy.

Finally, there's still a mishmash of backticks, asterisks, underscores,
and even quotation marks. Someone is gonna have to perform major
cleanup on this one day, but at least it'll be in only one place.

Signed-off-by: Ed Santiago <[email protected]>
@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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

4 participants