Skip to content

hack: support $DOCKER_ROOTLESS for testing rootless#40538

Merged
cpuguy83 merged 1 commit intomoby:masterfrom
AkihiroSuda:test-rootless
Mar 9, 2020
Merged

hack: support $DOCKER_ROOTLESS for testing rootless#40538
cpuguy83 merged 1 commit intomoby:masterfrom
AkihiroSuda:test-rootless

Conversation

@AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Feb 18, 2020

- What I did
Added rootless test suite.

test-integration-cli is unsupported currently.
Also, tests that spawn custom daemon (testutil/daemon) are skipped.

Relates to #40484

- How I did it

Spawn dockerd-rootless.sh instead of dockerdwhen$DOCKER_ROOTLESS` is set.

- How to verify it

$ DOCKER_EXPERIMENTAL=1 DOCKER_ROOTLESS=1 TEST_SKIP_INTEGRATION_CLI=1 \
 make test-integration

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)
🐧

@AkihiroSuda AkihiroSuda requested a review from tianon as a code owner February 18, 2020 12:57
@AkihiroSuda AkihiroSuda mentioned this pull request Feb 18, 2020
Closed
6 tasks
@AkihiroSuda AkihiroSuda force-pushed the test-rootless branch 3 times, most recently from 1ceb386 to 91c9ba2 Compare February 18, 2020 13:36
AkihiroSuda
AkihiroSuda commented Feb 18, 2020
View reviewed changes
@AkihiroSuda AkihiroSuda changed the title [WIP] hack: support $DOCKER_ROOTLESS for testing rootless hack: support $DOCKER_ROOTLESS for testing rootless Feb 18, 2020
thaJeztah
thaJeztah reviewed Feb 24, 2020
View reviewed changes
@AkihiroSuda AkihiroSuda force-pushed the test-rootless branch 4 times, most recently from 6199179 to a40de08 Compare February 26, 2020 14:22
thaJeztah
thaJeztah requested changes Feb 26, 2020
View reviewed changes
@thaJeztah
Copy link
Member

thaJeztah commented Feb 26, 2020

Thanks! Left some comments, but take note that GitHub is hiding some of them 😞

@AkihiroSuda AkihiroSuda force-pushed the test-rootless branch 4 times, most recently from b861e14 to ceeb496 Compare March 6, 2020 13:56
thaJeztah
thaJeztah reviewed Mar 6, 2020
View reviewed changes
Dockerfile Outdated
Copy link
Member

@thaJeztah thaJeztah Mar 6, 2020

Choose a reason for hiding this comment

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

This is the only one I was a bit on the fence on; considering if it would be better if we make this a -v /home/unprivilegeduser/.local/share/docker at runtime instead

Copy link
Member Author

@AkihiroSuda AkihiroSuda Mar 6, 2020

Choose a reason for hiding this comment

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

Do we do that for VOLUME /var/lib/docker as well?

Copy link
Member

@thaJeztah thaJeztah Mar 6, 2020

Choose a reason for hiding this comment

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

No, I don't think we do; I think the main reason for that being there was that otherwise the container would be (largely) non-functional (unless vfs is used). Downside of these VOLUME declarations is that now at least two anonymous volumes are created each time.

Not a big deal.

thaJeztah
thaJeztah approved these changes Mar 6, 2020
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Dockerfile Outdated
Copy link
Member

@thaJeztah thaJeztah Mar 6, 2020

Choose a reason for hiding this comment

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

No, I don't think we do; I think the main reason for that being there was that otherwise the container would be (largely) non-functional (unless vfs is used). Downside of these VOLUME declarations is that now at least two anonymous volumes are created each time.

Not a big deal.

@thaJeztah
Copy link
Member

thaJeztah commented Mar 7, 2020

@cpuguy83 @tianon PTAL

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Mar 7, 2020

Failure in s390x seems unrelated

...
[2020-03-07T16:45:28.206Z] fatal: unable to access 'https://github.com/docker/buildx.git/': Could not resolve host: github.com

[2020-03-07T16:45:28.951Z] The command '/bin/sh -c git clone "${BUILDX_REPO}" /buildx' returned a non-zero code: 128

[2020-03-07T16:45:28.951Z] Makefile:288: recipe for target 'bundles/buildx' failed

cpuguy83
cpuguy83 approved these changes Mar 9, 2020
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda
hack: support $DOCKER_ROOTLESS for testing rootless
5bf33ad 
```
$ DOCKER_EXPERIMENTAL=1 DOCKER_ROOTLESS=1 TEST_SKIP_INTEGRATION_CLI=1 \
 make test-integration
```

test-integration-cli is unsupported currently.
Also, tests that spawn custom daemon (testutil/daemon) are skipped.

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Mar 9, 2020

Can we ignore s390 error and merge this?

@cpuguy83 cpuguy83 merged commit 51c119c into moby:master Mar 9, 2020
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

@tianon tianon Awaiting requested review from tianon tianon is a code owner

Assignees

No one assigned

Labels

area/rootless Rootless Mode area/testing status/4-merge

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@AkihiroSuda @thaJeztah @cpuguy83