Update installation instructions#3068
Update installation instructions#3068openshift-merge-robot merged 1 commit intocontainers:masterfrom
Conversation
|
Hi @h-vetinari. 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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Can one of the admins verify this patch?
|
h-vetinari
left a comment
There was a problem hiding this comment.
Some more detailed comments about the changes.
docs/tutorials/podman_tutorial.md
Outdated
There was a problem hiding this comment.
Everything from this file that was not already available in install.md has been moved there.
install.md
Outdated
There was a problem hiding this comment.
New subsection for installing dev builds on Fedora/Ubuntu
install.md
Outdated
There was a problem hiding this comment.
Followed by the section to rebuild from scratch...
install.md
Outdated
There was a problem hiding this comment.
Sorted this package alphabetically
install.md
Outdated
There was a problem hiding this comment.
Update deprecated package name
install.md
Outdated
There was a problem hiding this comment.
This build tag has been changed compared to the tutorial due to this comment.
install.md
Outdated
There was a problem hiding this comment.
Maintaining the link to the general description, while also providing a workable minimal setup.
install.md
Outdated
There was a problem hiding this comment.
This whole block has been split up into the above sections (and expanded with the actual build commands, as in the tutorial).
install.md
Outdated
There was a problem hiding this comment.
Resolved a mix of tabs/spaces here
install.md
Outdated
There was a problem hiding this comment.
Replaced with the build instructions from the tutorial, and added BUILDTAGS="selinux seccomp" due to the discussion in #3045. I also needed to add PREFIX= to make sure all the libraries get linked into the binary correctly (although I'd hope that this could be removed again somehow).
TomSweeneyRedHat
left a comment
There was a problem hiding this comment.
Really nice start @h-vetinari thanks for taking this on. Just a few nits here and there.
docs/tutorials/podman_tutorial.md
Outdated
There was a problem hiding this comment.
nit Police! podman s/b "Podman" unless you're using it as a command.
install.md
Outdated
install.md
Outdated
There was a problem hiding this comment.
suggest s/most basic/more basic/
install.md
Outdated
There was a problem hiding this comment.
can you add a link to the golang line for "above"?
giuseppe
left a comment
There was a problem hiding this comment.
can you please squash the commits together?
No problem. Do you want this for every push or just before the final merge? |
just final merge works for me, but I saw you already fixed it. Thanks! |
install.md
Outdated
There was a problem hiding this comment.
Maybe it's just me, but when I first read this when it's resolved to a web page, I missed the link and was looking on this page. Maybe s/these instructions/the instructions here/ and have the word "here" be the link?
TomSweeneyRedHat
left a comment
There was a problem hiding this comment.
One small change for consideration, otherwise LGTM.
install.md
Outdated
There was a problem hiding this comment.
nice change to make these proper headings.
There was a problem hiding this comment.
Just following what was in the tutorial already. ;-)
|
@h-vetinari Please add a line like: |
|
/ok-to-test |
2790180 to
5168618
Compare
Signed-off-by: h-vetinari <h.vetinari@gmx.com>
|
Anything left for me to do here? |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: h-vetinari, jwhonce, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Tests are green. LGTM on my end |
|
/lgtm |
|
Thanks for merging. @rhatdan, following the install instructions for building on RHEL should give you a reproducible example for #3045, cf. your comment |
Closes #2250
As talked about at the end of that issue, @TomSweeneyRedHat and @rhatdan agreed with my suggestion to merge the installation instructions from
docs/tutorials/podman_tutorial.mdwithinstall.md, which I did here.I tried to not lose any currently existing information along the way, but rather expand it to a full set of commands necessary to build (e.g. ostree). I tested all the instructions on Ubuntu 18.04 and RHEL 7.6 (on an azure VM, without having an active subscription).
For RHEL, I ran into #3045, where the build instructions (before this PR) produced a build that would only run if
runc<1.0.0-rc7. In that issue, @rhatdan told me thatruncneeds to be built withBUILDTAGS="selinux seccomp", but that then also produces a build that is broken. Following the rabbit hole further, and also building podman withBUILDTAGS="selinux seccomp"runs into a selinux issue, which is reproducible for me (cf. this comment by @rhatdan).I documented the install instructions that allowed me to proceed as far as possible (i.e. all builds suceed, and the failure only occurs at runtime, i.e.
sudo podman build), but I presume there will be a few more kinks to iron out regarding this.I'll comment a few more things in the diff directly.