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

Documentation fix: we need port forwarding to access a rootless containers TCP port. #2817

Merged
merged 1 commit into from Apr 8, 2019

Conversation

tkrypton
Copy link
Contributor

The tutorial fools new users; the output listed does not fit the output the user will get. This commit uses port forwarding in the example command, so that the curl command will work.

@openshift-ci-robot
Copy link
Collaborator

Hi @tkrypton. Thanks for your PR.

I'm waiting for a containers or openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 31, 2019
@rh-atomic-bot
Copy link
Collaborator

Can one of the admins verify this patch?
I understand the following commands:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@giuseppe
Copy link
Member

bot, test pull request

```console
$ sudo podman inspect -l | grep IPAddress\":
"IPAddress": "10.88.6.140",
"SecondaryIPAddresses": null,
"IPAddress": "",
Copy link
Member

Choose a reason for hiding this comment

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

this command runs as root. Containers created by root get an IP

Copy link
Member

Choose a reason for hiding this comment

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

Lets update the docs to state that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Look again. The container in the tutorial gets started rootless and gets no IP. I have not changed that, just took it as it was.

Copy link
Member

Choose a reason for hiding this comment

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

Need to kill the sudo then. We don't share containers between Root and Rootless Podman, so sudo podman inspect won't find a container created without using sudo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, will do.

@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 31, 2019
@giuseppe
Copy link
Member

/approve

LGTM

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, tkrypton

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

@@ -140,10 +141,11 @@ podman ps
Note: If you add *-a* to the *ps* command, Podman will show all containers.
### Inspecting a running container
You can "inspect" a running container for metadata and details about itself. We can even use
the inspect subcommand to see what IP address was assigned to the container.
the inspect subcommand to see what IP address was assigned to the container (none):
Copy link
Member

Choose a reason for hiding this comment

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

can we say "(none, as rootless containers do not receive an IP address)"?

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer making it a new sentence, but we certainly should add a bit more than '(none)' here. I'd suggest something like:
"As the container is running in rootless mode, an IP address is not assigned and the value will be listed as "none" in the output from inspect."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder how that tutorial started? Probably all commands were run as root? Should this be split into a root and a rootless tutorial? I'd be happy to add that sentence, though.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, when the tutorial was first created, rootless was not an option at that time.

@@ -123,13 +123,14 @@ sudo make install PREFIX=/usr
This sample container will run a very basic httpd server that serves only its index
page.
```console
podman run -dt -e HTTPD_VAR_RUN=/var/run/httpd -e HTTPD_MAIN_CONF_D_PATH=/etc/httpd/conf.d \
podman run -dt -p 8080:8080/tcp -e HTTPD_VAR_RUN=/var/run/httpd -e HTTPD_MAIN_CONF_D_PATH=/etc/httpd/conf.d \
-e HTTPD_MAIN_CONF_PATH=/etc/httpd/conf \
-e HTTPD_CONTAINER_SCRIPTS_PATH=/usr/share/container-scripts/httpd/ \
registry.fedoraproject.org/f27/httpd /usr/bin/run-httpd
```
Copy link
Member

Choose a reason for hiding this comment

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

This command worked for me as root, but failed as rootless:

$ podman run -dt -p 8080:8080/tcp -e HTTPD_VAR_RUN=/var/run/httpd -e HTTPD_MAIN_CONF_D_PATH=/etc/httpd/conf.d \
>                   -e HTTPD_MAIN_CONF_PATH=/etc/httpd/conf \
>                   -e HTTPD_CONTAINER_SCRIPTS_PATH=/usr/share/container-scripts/httpd/ \
>                   registry.fedoraproject.org/f27/httpd /usr/bin/run-httpd
Trying to pull registry.fedoraproject.org/f27/httpd...Getting image source signatures
Copying blob ff3dab903f92 [==================================] 80.7MiB / 80.7MiB
Copying blob 9347d6e9d864 [====================================] 7.3MiB / 7.3MiB
Copying blob 2fc5c44251d4 [==================================] 44.8MiB / 44.8MiB
Copying config 18f01f6f77 [====================================] 6.6KiB / 6.6KiB
Writing manifest to image destination
Storing signatures
Error: error from slirp4netns while setting up port redirection: map[desc:bad request: add_hostfwd: slirp_add_hostfwd failed]

@giuseppe do we have a slirp4netns dependency that we need to document in here? I'm running podman 1.1.2

Copy link
Member

@giuseppe giuseppe Apr 1, 2019

Choose a reason for hiding this comment

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

we need slirp4netns v0.3.0

Copy link
Member

Choose a reason for hiding this comment

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

That didn't come directly in with the 1.1.2 kit, will it with 1.2? Do we need to add anything here in the the tutorial?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... As I started out I just wanted to fix one issue as I ran into a brick wall by just trying out this tutorial. But I can add a hint that this rootless command needs at least slirp4netns v0.3.0.

Copy link
Member

Choose a reason for hiding this comment

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

@tkrypton thanks, that would be good. I was hoping to not have to do that, but I don't think there's a better way to do so.

@mheon
Copy link
Member

mheon commented Apr 1, 2019

Docs LGTM, but Gitvalidation is complaining about your commit - it's missing a signoff (git commit --amend -s) and the commit message is over 90 characters

@mheon
Copy link
Member

mheon commented Apr 1, 2019

/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 1, 2019
@tkrypton
Copy link
Contributor Author

tkrypton commented Apr 2, 2019

Yeah, added the missing signoff and splitted the commit message.

@mheon
Copy link
Member

mheon commented Apr 2, 2019

@tkrypton Sorry, one more thing - - FAIL - commit subject exceeds 90 characters

The Papr failures aren't your fault, so ignore those - infrastructure issues with the CI

@mheon
Copy link
Member

mheon commented Apr 3, 2019

@tkrypton Can you squash your commits together with git rebase -i? Might be enough to make CI happy

Signed-off-by: Ulrich Teichert <516052+tkrypton@users.noreply.github.com>
@tkrypton
Copy link
Contributor Author

tkrypton commented Apr 4, 2019

I'll give it a try...

@tkrypton
Copy link
Contributor Author

tkrypton commented Apr 6, 2019

I think after rebasing this has lost all labels...

@mheon
Copy link
Member

mheon commented Apr 8, 2019

We're green. LGTM

@mheon
Copy link
Member

mheon commented Apr 8, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2019
@openshift-merge-robot openshift-merge-robot merged commit 995c5d8 into containers:master Apr 8, 2019
@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 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 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. ok-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants