Skip to content

Cirrus: Add dedicated rootless mode testing - #2531

Merged
openshift-merge-robot merged 1 commit into
podman-container-tools:masterfrom
cevich:rootless_stub
Mar 9, 2019
Merged

Cirrus: Add dedicated rootless mode testing#2531
openshift-merge-robot merged 1 commit into
podman-container-tools:masterfrom
cevich:rootless_stub

Conversation

@cevich

@cevich cevich commented Mar 4, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: Chris Evich cevich@redhat.com

@cevich
cevich force-pushed the rootless_stub branch 2 times, most recently from 398c3d8 to f20eb4b Compare March 4, 2019 22:32
@cevich cevich changed the title Cirrus: Add dedicated rootless mode testing WIP: Cirrus: Add dedicated rootless mode testing Mar 4, 2019
@openshift-ci-robot openshift-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 Mar 4, 2019
@cevich
cevich force-pushed the rootless_stub branch 2 times, most recently from 0f461c8 to 3afd267 Compare March 4, 2019 22:52
@cevich
cevich force-pushed the rootless_stub branch 4 times, most recently from a07663c to c464fdd Compare March 4, 2019 23:44
@cevich

cevich commented Mar 4, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@giuseppe PTAL - very early experiment. My thinking here is to get a dedicated VM and entry-point first, running as an actual user w/o any sudo/su/runuser/exec trickery. Then later (as a group) we can figure out the details of "what to test and where".

/cc @baude @mheon

@cevich

cevich commented Mar 4, 2019

Copy link
Copy Markdown
Contributor Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2019
@giuseppe

giuseppe commented Mar 5, 2019

Copy link
Copy Markdown
Contributor

this is great. Once it lands we can probably nuke the ad-hoc rootless tests, or pick just a subset of them

giuseppe
giuseppe reviewed Mar 5, 2019
View reviewed changes
Comment thread contrib/cirrus/rootless_test.sh Outdated
@cevich

cevich commented Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

Hint: See the new "rootless_testing" task among the others 😄

@rh-atomic-bot

rh-atomic-bot commented Mar 5, 2019

Copy link
Copy Markdown

☔ The latest upstream changes (presumably #2435) made this pull request unmergeable. Please resolve the merge conflicts.

@cevich cevich changed the title WIP: Cirrus: Add dedicated rootless mode testing Cirrus: Add dedicated rootless mode testing Mar 5, 2019
@openshift-ci-robot openshift-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 Mar 5, 2019
@cevich
Cirrus: Add dedicated rootless mode testing
142442e 
Certain integration tests require execution as a regular user.
This is acomplished by `PodmanTest.PodmanAsUserBase()` wrapping a
specialized execution environment, in `test/utils/utils.go`.  However,
doing this requires passing through python, which vastly increases the
complexity of debugging low-level problems.

This commit introduces a new parallel task, run as a regular user on the
VM as set by three environment variables.  All commands executed in the
``rootless_test.sh`` script, will occur as a real user with a name and
home directory, just as `$DIETY` intended.  All env. vars established
during `environment_setup.sh` (for root) are available.  The PR source
in `$GOSRC` and `$GOPATH` are owned by this user, and ready for use.

Signed-off-by: Chris Evich <cevich@redhat.com>
@cevich

cevich commented Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

@giuseppe @baude @mheon PTAL (assuming other tests pass) I think this is ready. I also added documentation about the task for reference. At the moment there's no real "test", it just compiles podman as the rootless user.

@mheon

mheon commented Mar 5, 2019

Copy link
Copy Markdown
Contributor

@giuseppe Anything else we need to do here to ensure we have a full subuid/subgid range set up for the user we test as? I'd love to drop the single user rootless container test hack

@cevich

cevich commented Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

@mheon good point. At the top-level we're still root, so config. changes can be made as needed to support testing. General updates to that setup_rootless() function should be easy/straight-forward.

Cirrus-CI can also do things in parallel within a task,as long as that doesn't hurt our brains too much 😄 One idea could be to use that to have multiple users running commands at the same time, or have root also doing operations on the system. Whichever way we want to make life challenging, it's possible 😃

@cevich

cevich commented Mar 6, 2019

Copy link
Copy Markdown
Contributor Author

@mheon I think no need to wait on this. It's designed as a stub, to be added onto later.

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 6, 2019
@mheon

mheon commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

@cevich Fair enough.
LGTM, we can iterate as we go.

@mheon

mheon commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

/approve

@openshift-ci-robot

openshift-ci-robot commented Mar 6, 2019

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, mheon

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

The pull request process is described here

Details 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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2019
@rhatdan

rhatdan commented Mar 9, 2019

Copy link
Copy Markdown
Contributor

/lgtm

One thing to think about is moving away from fedora29 to fedora 30. Also would be nice to test on RHEL8 at some point.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 9, 2019
@openshift-merge-robot
openshift-merge-robot merged commit b15273a into podman-container-tools:master Mar 9, 2019
@rh-atomic-bot rh-atomic-bot mentioned this pull request Mar 9, 2019
Merged
@cevich

cevich commented Mar 11, 2019

Copy link
Copy Markdown
Contributor Author

@rhatdan I'll file jira cards for both.

@cevich
cevich deleted the rootless_stub branch June 30, 2021 18:10
@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.

Reviewers

@giuseppe giuseppe giuseppe left review comments
@umohnani8 umohnani8 Awaiting requested review from umohnani8

Assignees

@rhatdan rhatdan

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@cevich @giuseppe @rh-atomic-bot @mheon @openshift-ci-robot @rhatdan @openshift-merge-robot