rpm: build a repo and use it #4815

ssbarnea · 2020-01-08T14:12:21Z

Using a repository improves testing of podman installation instead of
using the bare rpm install method by using the likely installation
method used on production.

Depends-On: https://review.rdoproject.org/r/#/c/27527/

openshift-ci-robot · 2020-01-08T14:12:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ssbarnea
To complete the pull request process, please assign giuseppe
You can assign the PR to them by writing /assign @giuseppe in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ssbarnea · 2020-01-08T14:38:29Z

@edsantiago @NevilleC @cevich -- interestingly this uncovers a bug related to centos-8 where rhel8 where yum decides to install the distro version instead of our code. The same bug does not happen with centos-7 and fedora-30.

This makes me believe we have another spec issue. Advise would be welcomed.
Here is what I found:

repolist reports 0 rpms in podman repo, which for sure I know there are two
whatprovides also seems to miss to see rpms from the podman repo, even if manually enabled.
there content of repodata seems valid based on a visual inspection, for example I see stuff like:

<package type="rpm">
  <name>podman</name>
  <arch>x86_64</arch>
  <version epoch="0" ver="1.7.1" rel="1578494472.git4617e49b.el8"/>
  <checksum type="sha256" pkgid="YES">2e839a9998ae488862eb9d7c5c38a3c0a77f7a39d6c7f15faa9d6487f97507ee</checksum>
  <summary>Manage Pods, Containers and Container Images</summary>
  <description>Manage Pods, Containers and Container Images
libpod provides a library for applications looking to use
the Container Pod concept popularized by Kubernetes.</description>
  <packager></packager>
  <url>%{git_podman}</url>
  <time file="1578494562" build="1578494557"/>
  <size package="12329228" installed="51530596" archive="51534520"/>
  <location href="podman-1.7.1-1578494472.git4617e49b.el8.x86_64.rpm"/>
  <format>
    <rpm:license>ASL 2.0</rpm:license>
    <rpm:vendor></rpm:vendor>
    <rpm:group>Unspecified</rpm:group>
    <rpm:buildhost>n0.sbarnea.com</rpm:buildhost>
    <rpm:sourcerpm>podman-1.7.1-1578494472.git4617e49b.el8.src.rpm</rpm:sourcerpm>
    <rpm:header-range start="4504" end="18864"/>

sudo yum whatprovides  --enablerepo podman podman
Last metadata expiration check: 0:00:53 ago on Wed 08 Jan 2020 02:49:47 PM GMT.
podman-1.0.5-1.gitf604175.module_el8.0.0+194+ac560166.x86_64 : Manage Pods, Containers and Container Images
Repo        : AppStream
Matched from:
Provide    : podman = 1.0.5-1.gitf604175.module_el8.0.0+194+ac560166

podman-1.4.2-5.module_el8.1.0+237+63e26edc.x86_64 : Manage Pods, Containers and Container Images
Repo        : Stream-AppStream
Matched from:
Provide    : podman = 1.4.2-5.module_el8.1.0+237+63e26edc

 sudo yum repolist --enablerepo podman
Podman Override                                                                                                                                                                           2.9 MB/s | 3.0 kB     00:00
repo id                                                                                                 repo name                                                                                                   status
AppStream                                                                                               CentOS-8 - AppStream                                                                                        5,089
BaseOS                                                                                                  CentOS-8 - Base                                                                                             2,843
PowerTools                                                                                              CentOS-8 - PowerTools                                                                                       1,507
Stream-AppStream                                                                                        CentOS-Stream - AppStream                                                                                   4,629
Stream-BaseOS                                                                                           CentOS-Stream - Base                                                                                        2,326
Stream-extras                                                                                           CentOS-Stream - Extras                                                                                          3
copr:copr.fedorainfracloud.org:carlwgeorge:ripgrep                                                      Copr repo for ripgrep owned by carlwgeorge                                                                      3
datadog                                                                                                 Datadog, Inc.                                                                                                  84
docker-ce-stable                                                                                        Docker CE Stable - x86_64                                                                                      57
*epel                                                                                                   Extra Packages for Enterprise Linux 8 - x86_64                                                              4,401
*epel-modular                                                                                           Extra Packages for Enterprise Linux Modular 8 - x86_64                                                          0
extras                                                                                                  CentOS-8 - Extras                                                                                               3
podman                                                                                                  Podman Override                                                                                                 0
puppet                                                                                                  Puppet Repository el 8 - x86_64                                                                               109

edsantiago

Nice work. Some suggestions for improvement; and good luck with tracking down the CentOS issues, sorry I can't help.

edsantiago · 2020-01-08T15:02:10Z

Makefile

-package-install: package  ## Install rpm packages
-	sudo ${PKG_MANAGER} -y install ${HOME}/rpmbuild/RPMS/*/*.rpm
+package-install: package build/podman.repo  ## Install rpm packages
+	#sudo ${PKG_MANAGER} -y install ${HOME}/rpmbuild/RPMS/*/*.rpm


Please remove

edsantiago · 2020-01-08T15:03:16Z

contrib/build_rpm.sh

@@ -58,6 +58,8 @@ fi
 export extra_arg="--without doc --without debug"

 echo ${PKGS[*]}
+# we disable podman repo because that can become invalid during builds and is
+# not needed for dependencies.


Please remove comment: it's misleading (a holdover from previous PR, perahps).

edsantiago · 2020-01-08T15:04:26Z

contrib/build_rpm.sh

+
+# builds build/podman.repo which should be ready to install locally
+mkdir -p build/buildset
+rm -rf build/buildset/*


A better option is to reorder:

rm -rf build/buildset mkdir -p build/buildset

edsantiago · 2020-01-08T15:09:59Z

Makefile

+package-install: package build/podman.repo  ## Install rpm packages
+	#sudo ${PKG_MANAGER} -y install ${HOME}/rpmbuild/RPMS/*/*.rpm
+	sudo cp -f build/podman.repo /etc/yum.repos.d/podman.repo
+	sudo ${PKG_MANAGER} -y --enablerepo podman install podman podman-remote


This approach (repo disabled by default) is much better -- but the logical extension is to remove the cp entirely and use --repofrompath instead; this way the system isn't left in an unmaintainable state. Could you look into using that, esp. seeing if it works with yum?

edsantiago · 2020-01-08T15:15:10Z

contrib/build_rpm.sh

+   cp -l ~/rpmbuild/RPMS/*/*.rpm .
+   createrepo .
+popd
+cat <<EOF >build/podman.repo


This is probably not absolutely critical, but the correct way to create files is to make a tempfile and rename it on success:

cat <<EOF >build/podman.repo.tmp.$$ && mv build/podman.repo.tmp.$$ build/podman.repo

With this form, if build/podman.repo exists it is guaranteed to be correct; without a tmp-rename, it is possible for an error (typically out-of-space but also possibly i/o error) to leave a corrupt file in place. This is a good habit to get into.

cevich · 2020-01-08T16:13:54Z

Makefile

+package-install: package build/podman.repo  ## Install rpm packages
+	#sudo ${PKG_MANAGER} -y install ${HOME}/rpmbuild/RPMS/*/*.rpm
+	sudo cp -f build/podman.repo /etc/yum.repos.d/podman.repo
+	sudo ${PKG_MANAGER} -y --enablerepo podman install podman podman-remote


Using sudo in a makefile leaves me slightly uncomfortable as it assumes a lot. For example, if this target is running inside a container as root, it will fail if sudo isn't installed (perhaps for security reasons, perhaps just to keep the image small). Would it make sense to leave out the sudo and just rely on the user running sudo make package-install when required?

Yes I agree, I don't think sudo should be in the Makefile.

I will address all comments receives so far, but before implementing these changes I need help addresing the issue i mentioned at #4815 (comment) -- so far in 2/4 platforms yum/dnf is unable to find the rpms from the builder repo.

@rhatdan Calling make with sudo would break a huge number of things, think about temp files created as root.

While is technically easy to detect root and fail if you did not call it as root, it would create a never ending list of problems as all make targets run as root will produce artifacts with wrong user. I really do not with to create any file owned by root in a non-root user home directory.

The jobs we run on rdo now are expecting to find a repository on project_root/build/builset and I would prefer not to have to alter them to look in different location.

Look inside the makefile and you will see that package-install target depends-on package for good reasons. This means that if user calls it with sudo, both targets will run as root. build_rpm.sh is also calling copr makefile which produces the source rpms innside build/ folder, so it would introduce root owned files to current project_root folder.

I am ok to remove sudo from makefile if we find a way to avoid messing build when someone runs sudo make <random-target> by mistake. I personally find more annoying the messed file ownership that the feature of not calling sudo exactly where is needed.

Maybe we should add a protection for each target and prevent it from running other than how is inteded (like some kind of ensure-root and ensure-non-root checks)?

edsantiago · 2020-01-08T18:21:25Z

If I may ask everyone to take a step back for a moment: what problem are we trying to solve?

Podman is an open-source project, not a Fedora-RHEL one. The little I understand of this PR is that it's intended for a narrow niche testing environment. Does this code really belong in the podman repo? Might there be a better place for it, more tightly integrated with the niche environment it's intended for?

ssbarnea · 2020-01-08T19:46:12Z

@edsantiago @rhatdan @cevich I guess none of you tried to run even the most based sudo make help, which produces somethink like:

 sudo make help
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Usage: make <target>

Based on this I would say that other things need to be addressed before we can remove sudo from inside the makefile.

Using a repository improves testing of podman installation instead of using the bare rpm install method by using the likely installation method used on production.

javierpena · 2020-01-09T15:53:20Z

contrib/build_rpm.sh

@@ -70,3 +69,16 @@ if [ -d ~/rpmbuild/BUILD ]; then
 fi

 rpmbuild --rebuild ${extra_arg:-} podman-*.src.rpm
+createrepo -v ~/rpmbuild/RPMS/x86_64


For consistency with line 75, could we make this /${HOME}/rpmbuild/RPMS/x86_64 ?

IIRC, rpmbuild checks an env. var. for the preferred location. The script should probably reference that instead of hard-coding. Otherwise this build could break in unexpected ways.

soostdijck · 2020-01-23T19:14:42Z

Any chance of building and incuding a new slirp4netns rpm? It seems that rootless containers do not want to run on centos 7.7 with the default supplied version.

cevich · 2020-01-27T16:15:39Z

My understanding is that rootless containers aren't fully supported on 7, but I cannot be sure of my memory of the reason. I want to say it has/had something to do with a user-namespace something or other feature in the kernel.

rhatdan · 2020-01-28T19:18:07Z

Correct rootless containers on RHEL7 will not be fully supported until RHEL7.8 release.

rh-atomic-bot · 2020-01-30T14:42:28Z

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

openshift-ci-robot · 2020-01-30T14:42:36Z

@ssbarnea: PR needs rebase.

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.

github-actions · 2020-03-01T00:05:02Z

A friendly reminder that this PR had no activity for 30 days.

rhatdan · 2020-03-02T15:45:48Z

@ssbarnea Still working on this?

vrothberg · 2020-05-04T14:24:29Z

Friendly ping.

vrothberg · 2020-06-04T11:59:35Z

Going to close. Please reopen if the PR is still desired.

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 Jan 8, 2020

openshift-ci-robot requested review from giuseppe and umohnani8 January 8, 2020 14:12

openshift-ci-robot added the size/M label Jan 8, 2020

ssbarnea requested a review from NevilleC January 8, 2020 14:33

ssbarnea mentioned this pull request Jan 8, 2020

rpm: assure we build and use a rpm repository #4796

Closed

ssbarnea requested review from cevich and edsantiago January 8, 2020 14:34

ssbarnea self-assigned this Jan 8, 2020

ssbarnea added the help wanted label Jan 8, 2020

edsantiago suggested changes Jan 8, 2020

View reviewed changes

cevich reviewed Jan 8, 2020

View reviewed changes

openshift-ci-robot added size/S and removed size/M labels Jan 8, 2020

rpm: build a repo and use it

9e4243e

Using a repository improves testing of podman installation instead of using the bare rpm install method by using the likely installation method used on production.

javierpena reviewed Jan 9, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2020

github-actions bot added the stale-pr label Mar 1, 2020

vrothberg closed this Jun 4, 2020

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 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rpm: build a repo and use it #4815

rpm: build a repo and use it #4815

ssbarnea commented Jan 8, 2020 •

edited

Loading

openshift-ci-robot commented Jan 8, 2020

ssbarnea commented Jan 8, 2020 •

edited

Loading

edsantiago left a comment

edsantiago Jan 8, 2020

edsantiago Jan 8, 2020

edsantiago Jan 8, 2020

edsantiago Jan 8, 2020

edsantiago Jan 8, 2020

cevich Jan 8, 2020

rhatdan Jan 8, 2020

ssbarnea Jan 8, 2020

ssbarnea Jan 8, 2020

edsantiago commented Jan 8, 2020

ssbarnea commented Jan 8, 2020

javierpena Jan 9, 2020

cevich Jan 10, 2020

soostdijck commented Jan 23, 2020

cevich commented Jan 27, 2020

rhatdan commented Jan 28, 2020

rh-atomic-bot commented Jan 30, 2020

openshift-ci-robot commented Jan 30, 2020

github-actions bot commented Mar 1, 2020

rhatdan commented Mar 2, 2020

vrothberg commented May 4, 2020

vrothberg commented Jun 4, 2020

rpm: build a repo and use it #4815

rpm: build a repo and use it #4815

Conversation

ssbarnea commented Jan 8, 2020 • edited Loading

openshift-ci-robot commented Jan 8, 2020

ssbarnea commented Jan 8, 2020 • edited Loading

edsantiago left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edsantiago commented Jan 8, 2020

ssbarnea commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soostdijck commented Jan 23, 2020

cevich commented Jan 27, 2020

rhatdan commented Jan 28, 2020

rh-atomic-bot commented Jan 30, 2020

openshift-ci-robot commented Jan 30, 2020

github-actions bot commented Mar 1, 2020

rhatdan commented Mar 2, 2020

vrothberg commented May 4, 2020

vrothberg commented Jun 4, 2020

ssbarnea commented Jan 8, 2020 •

edited

Loading

ssbarnea commented Jan 8, 2020 •

edited

Loading