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

container: resolve workdir during initialization after all the mounts are completed. #11353

Merged

Conversation

flouthoc
Copy link
Collaborator

There are use-cases where users would want to use overlay-mounts as
workdir for containers. For such use-cases workdir should be resolved after all the mounts
are completed during the container init process.

Closes: #11352

@flouthoc flouthoc changed the title container: resolve workdir during initialization after all the mounts. container: resolve workdir during initialization after all the mounts are completed. Aug 30, 2021
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@flouthoc, please update your "Signed-off-by" lines. The contributing guidelines require us to use our full name, no nickname.

@flouthoc flouthoc force-pushed the resolve-workdir-after-mounts branch from 582db60 to 20f3ced Compare August 30, 2021 08:36
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2021
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/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 Aug 30, 2021
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 30, 2021
@Luap99
Copy link
Member

Luap99 commented Aug 30, 2021

Can you add a test for this?

@flouthoc flouthoc force-pushed the resolve-workdir-after-mounts branch from 20f3ced to a09d0a8 Compare August 30, 2021 09:28
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 30, 2021
@flouthoc
Copy link
Collaborator Author

@Luap99 added a test: container run should fail if workdir is not mounted . PTAL

@flouthoc flouthoc force-pushed the resolve-workdir-after-mounts branch from a09d0a8 to 52e7eec Compare August 30, 2021 10:25
Copy link
Member

@giuseppe giuseppe 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 can drop the [NO TESTS NEEDED] from the commit message.

I'd suggest to shrink the first line in the commit message (e.g. by removing during initialization) so it can fully fit.

test/e2e/run_volume_test.go Show resolved Hide resolved
@flouthoc flouthoc force-pushed the resolve-workdir-after-mounts branch from 52e7eec to a5bb17b Compare August 30, 2021 11:30
@flouthoc flouthoc requested review from giuseppe and vrothberg August 30, 2021 11:31
@flouthoc flouthoc force-pushed the resolve-workdir-after-mounts branch from a5bb17b to 3b9eaf9 Compare August 30, 2021 14:08
There are use-cases where users would want to use overlay-mounts as
workdir. For such cases workdir should be resolved after all the mounts
are completed during the container init process.

Signed-off-by: Aditya Rajan <[email protected]>
@flouthoc flouthoc force-pushed the resolve-workdir-after-mounts branch from 3b9eaf9 to ec1f350 Compare August 30, 2021 14:19
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

test/e2e/run_volume_test.go Show resolved Hide resolved
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 30, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, 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

@jwhonce
Copy link
Member

jwhonce commented Aug 30, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 30, 2021
@TomSweeneyRedHat
Copy link
Member

LGTM
/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 Aug 30, 2021
@openshift-merge-robot openshift-merge-robot merged commit 95ac8f1 into containers:main Aug 30, 2021
@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
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workdir rejected when it's inside volume
7 participants