install-deps: rectify docker packages for fedora #39995

rishabh-d-dave · 2021-03-10T08:32:10Z

Fixes: https://tracker.ceph.com/issues/49695

Checklist

References tracker ticket

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

aaSharma14 · 2021-03-10T10:50:07Z

install-deps.sh

@@ -339,7 +339,7 @@ else
        case "$ID" in
            fedora)
                $SUDO dnf install -y dnf-utils
-                $SUDO dnf install -y docker-ce docker-ce-cli containerd.io
+                $SUDO dnf install -y moby-engine containerd


Thanks @rishabh-d-dave . Am using Fedora 32 and am getting error while installing moby-engine package docker-ce-cli-1:19.03.12-3.fc31.x86_64 conflicts with docker provided by moby-engine-19.03.11-1.ce.git42e35e6.fc32.x86_64. This error would be there if we have docker already installed using docker-ce or docker-ce-cli. To avoid this we can instead use this-

Suggested change

$SUDO dnf install -y moby-engine containerd

$SUDO dnf install -y dnf-utils

if rpm -q docker-ce || rpm -q docker-ce-cli || rpm -q moby-engine; then

echo "docker is already installed"

else

$SUDO dnf install -y moby-engine containerd

fi

$SUDO systemctl start docker

$SUDO systemctl enable docker

@aaSharma14 Are you sure? I get same error on Fedora 32 VM -

$ sudo ./install-deps.sh Using dnf to install dependencies Fedora Modular 32 - x86_64 5.0 kB/s | 6.6 kB 00:01 Fedora Modular 32 - x86_64 - Updates 5.6 kB/s | 5.0 kB 00:00 Fedora Modular 32 - x86_64 - Updates 30 kB/s | 296 kB 00:10 Fedora 32 - x86_64 - Updates 6.6 kB/s | 4.9 kB 00:00 Fedora 32 - x86_64 - Updates 128 kB/s | 3.7 MB 00:29 Last metadata expiration check: 0:00:01 ago on Sat Mar 13 11:34:59 2021. Package dnf-utils-4.0.18-1.fc32.noarch is already installed. Dependencies resolved. Nothing to do. Complete! Last metadata expiration check: 0:00:21 ago on Sat Mar 13 11:34:59 2021. No match for argument: docker-ce No match for argument: docker-ce-cli No match for argument: containerd.io Error: Unable to find a match: docker-ce docker-ce-cli containerd.io $ cat /etc/redhat-release Fedora release 32 (Thirty Two)

Can you please recheck?

@aaSharma14 Are you sure? I get same error on Fedora 32 VM -

$ sudo ./install-deps.sh Using dnf to install dependencies Fedora Modular 32 - x86_64 5.0 kB/s | 6.6 kB 00:01 Fedora Modular 32 - x86_64 - Updates 5.6 kB/s | 5.0 kB 00:00 Fedora Modular 32 - x86_64 - Updates 30 kB/s | 296 kB 00:10 Fedora 32 - x86_64 - Updates 6.6 kB/s | 4.9 kB 00:00 Fedora 32 - x86_64 - Updates 128 kB/s | 3.7 MB 00:29 Last metadata expiration check: 0:00:01 ago on Sat Mar 13 11:34:59 2021. Package dnf-utils-4.0.18-1.fc32.noarch is already installed. Dependencies resolved. Nothing to do. Complete! Last metadata expiration check: 0:00:21 ago on Sat Mar 13 11:34:59 2021. No match for argument: docker-ce No match for argument: docker-ce-cli No match for argument: containerd.io Error: Unable to find a match: docker-ce docker-ce-cli containerd.io $ cat /etc/redhat-release Fedora release 32 (Thirty Two)

Can you please recheck?

@aaSharma14 ping.

I cant see any error with this on my Fedora32 -
$SUDO dnf install -y dnf-utils
if rpm -q docker-ce || rpm -q docker-ce-cli || rpm -q moby-engine || rpm -q podman; then
echo "docker is already installed"
else
$SUDO dnf install -y moby-engine containerd
fi
$SUDO systemctl start docker
$SUDO systemctl enable docker

@aaSharma14 Oh, OK. So you already have docker-ce repo installed on your system and you want your install-deps.sh to bypass installation in that case.

Yes actually, because in this case docker provided by docker-ce conflicts with docker provided by moby-engine. So if we can just check if docker exists already then we should bypass the installation else proceed with moby-engine( which IMO works with all the fedora versions).

I got that already. It would've been faster if you had told me why/how did you have docker-ce available on your Fedora 32 machine before. :)

Anyways, thanks!

@aaSharma14
Please check if the current patch works for you.

Interestingly, install-deps.sh right now installs docker packages only for Fedora and does nothing for RHEL, CentOS, etc. Is there a reason behind? It's more correct if we install docker for entire OS family if we are installing it for one of the family members.

Thanks @rishabh-d-dave . It works fine now. I think I just missed that part adding docker to other distros. Adding the same patch just below -

centos|rhel|ol|virtuozzo)
MAJOR_VERSION="$(echo $VERSION_ID | cut -d. -f1)"
$SUDO dnf install -y dnf-utils
dnf list installed | egrep "docker-ce|docker-ce-cli|containerd.io"
if test $? -ne 0; then
$SUDO dnf install -y moby-engine containerd
fi
$SUDO systemctl start docker
$SUDO systemctl enable docker

should do this right?

sunnyku · 2021-03-11T12:54:14Z

                 $SUDO dnf install -y dnf-utils
+                $SUDO dnf config-manager --add-repo https://download.docker.com/linux/fedora/docker-ce.repo
                 $SUDO dnf install -y docker-ce docker-ce-cli containe

This should work on both Fedora 32 & 33
Ref: https://docs.docker.com/engine/install/fedora/

rishabh-d-dave · 2021-03-12T04:31:18Z

@sunnyku

                 $SUDO dnf install -y dnf-utils
+                $SUDO dnf config-manager --add-repo https://download.docker.com/linux/fedora/docker-ce.repo
                 $SUDO dnf install -y docker-ce docker-ce-cli containe

This should work on both Fedora 32 & 33
Ref: https://docs.docker.com/engine/install/fedora/

Nice workaround but I am not sure if every developer would be fully OK with addition of a repo by install-deps.sh...

badone · 2021-03-16T21:27:43Z

Why must the people who already have podman installed install docker? Does your test not work in a container running under podman and, if so, why? I think that most developers running install-deps are going to have either podman or docker installed and those with podman installed are not going to want to install docker. There's also the issue of potential install conflicts when installing docker on a system that already has podman, podman-docker, etc. installed.

badone · 2021-03-17T00:25:16Z

I disagree with this and #39246 and I believe it should probably be reverted and this test moved to teuthology (unit test or not). How does this work when we run 'make check' inside a container? Why is there no provision for the other distros we support? How is this code handled on the jenkins build targets running centos? I just think this needs a lot more consideration.

badone · 2021-03-17T02:55:23Z

If I clone ceph master and run install-deps.sh and do_cmake.sh on a fresh install of centos and 'ninja tests' I get the following result.

 # ctest -R promtool --output-on-failure
Test project /ceph/build
    Start 15: run-promtool-unittests.sh
1/1 Test #15: run-promtool-unittests.sh ........***Failed    0.06 sec
+++ dirname /ceph/src/test/run-promtool-unittests.sh
++ cd /ceph/src/test
++ pwd -P
+ SCRIPTPATH=/ceph/src/test
+ : /ceph
+ sudo docker run --rm -v /ceph:/ceph --name=promtool --network=host dnanexus/promtool:2.9.2 test rules /ceph/monitoring/prometheus/alerts/test_alerts.yml
sudo: docker: command not found


0% tests passed, 1 tests failed out of 1

Total Test time (real) =   0.10 sec

The following tests FAILED:
         15 - run-promtool-unittests.sh (Failed)
Errors while running CTest

This was in a container so I doubt trying to install and run docker is the answer. I just don't see how this can fly as a unit test, I'm sorry.

Fixes: https://tracker.ceph.com/issues/49695 Signed-off-by: Rishabh Dave <ridave@redhat.com>

tchaikov · 2021-03-17T04:55:12Z

install-deps.sh

@@ -339,7 +339,10 @@ else
        case "$ID" in
            fedora)
                $SUDO dnf install -y dnf-utils
-                $SUDO dnf install -y docker-ce docker-ce-cli containerd.io
+                dnf list installed | egrep "docker-ce|docker-ce-cli|containerd\.io|podman"


please use the promtools shipped by fedora instead of using a container for it, since fedora comes with a more recent prometheus.

rishabh-d-dave · 2021-03-17T04:58:53Z

Thanks for the review @badone.

Why must the people who already have podman installed install docker? Does your test not work in a container running under podman and, if so, why? I think that most developers running install-deps are going to have either podman or docker installed and those with podman installed are not going to want to install docker. There's also the issue of potential install conflicts when installing docker on a system that already has podman, podman-docker, etc. installed.

Right. installs-dep.sh should skip installing docker if podman is already installed. I've added this change to the patch now.

I disagree with this and #39246 and I believe it should probably be reverted and this test moved to teuthology (unit test or not). How does this work when we run 'make check' inside a container? Why is there no provision for the other distros we support? How is this code handled on the jenkins build targets running centos? I just think this needs a lot more consideration.

Okay. I just wanted install-deps.sh to work smoothly on Fedora 33 and I have no opinion on #39246 since I am not well-acquainted with Ceph Dashboard. If @aaSharma14 @epuertat agree with reverting changes, I'll close this PR.

badone

This whole concept needs further discussion.

badone · 2021-03-17T05:43:05Z

Okay. I just wanted install-deps.sh to work smoothly on Fedora 33 and I have no opinion on #39246 since I am not well-acquainted with Ceph Dashboard. If @aaSharma14 @epuertat agree with reverting changes, I'll close this PR.

I see that now, no worries.

rishabh-d-dave · 2021-03-17T05:47:29Z

This whole concept needs further discussion.

Agreed.

epuertat · 2021-03-17T09:20:20Z

I disagree with this and #39246 and I believe it should probably be reverted and this test moved to teuthology (unit test or not). How does this work when we run 'make check' inside a container? Why is there no provision for the other distros we support? How is this code handled on the jenkins build targets running centos? I just think this needs a lot more consideration.

@badone which distros do you mean? As said, I'm not excited about this solution, but it was chosen because it was the only one that worked both on Fedora/CentOS, OpenSUSE and Ubuntu distros. In fact, the initial approach was installing from packages, but the promtool package in Ubuntu <20 didn't provide this functionality. If we find that we can't ensure this works in every possible distro-release, we may simply enclose this into an conditional variable and only run this when defined.

Good news is that the Ubuntu build nodes are being migrated to 20, so that would allow us to run this from packages soon (@tchaikov please correct me if I'm wrong here).

On the other hand I don't seen anything wrong with running docker inside docker (you just need to map the daemon socket) and it's a pretty common scenario in Jenkins pipelines.

Regarding moving promtool to teuthology, I find it opposed to the spirit of this change: to provide quick feedback (per PR) on the health of the alerting rules. IMHO it's an overkill to build packages and run a real cluster just for a task that takes less than 3 mins to complete.

badone · 2021-03-17T21:35:11Z

@badone which distros do you mean?

Have a look at the output from centos above. The current solution only installs docker under fedora and deb/ubuntu. It does not install it under centos or opensuse so will fail to run docker on those distros. It also adds a systemctl dependency on Bionic (at least) which will fail if systemd is not installed.

As said, I'm not excited about this solution, but it was chosen because it was the only one that worked both on Fedora/CentOS, OpenSUSE and Ubuntu distros.

But at the moment it doesn't work on nearly all of these (including fedora, hence this PR) if docker is not installed and, my biggest gripe, tries to install docker even if there is a working podman installation present which is probably doomed to failure due to conflicts between podman-docker and docker.

In fact, the initial approach was installing from packages, but the promtool package in Ubuntu <20 didn't provide this functionality. If we find that we can't ensure this works in every possible distro-release, we may simply enclose this into an conditional variable and only run this when defined.

Sure, that's one approach, and as you said should be temporary.

Good news is that the Ubuntu build nodes are being migrated to 20, so that would allow us to run this from packages soon (@tchaikov please correct me if I'm wrong here).

On the other hand I don't seen anything wrong with running docker inside docker (you just need to map the daemon socket) and it's a pretty common scenario in Jenkins pipelines.

I/we have a strong preference for podman if we must go down this path so the original solution should be amended to check for podman and use it if available and even prefer to install it if available IMHO. We already have code that prefers podman over docker so there is precedent for this.

Regarding moving promtool to teuthology, I find it opposed to the spirit of this change: to provide quick feedback (per PR) on the health of the alerting rules. IMHO it's an overkill to build packages and run a real cluster just for a task that takes less than 3 mins to complete.

Sure, I understand, but it's another approach that does not involve forcing docker on people, and as you said should be temporary.

At the moment if I want to run install-deps.sh my work flow looks like this.

$ git fetch upstream master && git reset --hard upstream/master
$ git revert 53a5816
$ ./install-deps.sh

Also a temporary solution but not ideal.

badone · 2021-03-18T05:09:19Z

@epuertat would it be possible to do the following?

$ curl -O https://github.com/prometheus/prometheus/releases/download/v2.25.2/prometheus-2.25.2.linux-amd64.tar.gz
$ tar x --strip-components=1 -f prometheus-2.25.2.linux-amd64.tar.gz prometheus-2.25.2.linux-amd64/promtool
$ ./promtool test rules ../monitoring/prometheus/alerts/test_alerts.yml

I tested this on centos:latest, centos:7, ubuntu:bionic, ubuntu:focal, opensuse:tumbleweed, opensuse:leap, and fedora 33 and it seems to function the same on all of them. Admittedly this only works on amd64 but you are using an exclusively amd64 container anyway right? At least one test fails with 2.25.2 BTW.

[edit] seems you could cover the other arches by adjusting the download. See https://github.com/prometheus/prometheus/releases/tag/v2.9.2 [/edit]

epuertat · 2021-03-18T09:32:26Z

@badone thanks a lot for the suggestion!

@aaSharma14 could you please have a look at that one. Let's try to install from RPMs where available:

CentOS/Fedora: golang-github-prometheus on Fedora/CentOS
OpenSUSE: golang-github-prometheus-prometheus on OpenSUSE
Ubuntu: prometheus_2.20.0 when available.
Otherwise, fall back to the releases pointed out by @badone.

We'd still need to conditionally skip this testing on platforms where promtool wasn't installed/not found.

BTW, I have DNM'ed the backports to Octopus and Nautilus, but this change is already in Pacific.

badone · 2021-03-18T09:55:37Z

BTW, I have DNM'ed the backports to Octopus and Nautilus, but this change is already in Pacific.

We should be able to backport the new solution to Pacific to get rid of that. Let me know if I can help in any way.

epuertat · 2021-03-18T16:58:51Z

@tchaikov already submitted #40205 which addresses these issues (thanks a lot, Kefu!).

github-actions bot added the build/ops label Mar 10, 2021

rishabh-d-dave requested review from tchaikov and aaSharma14 March 10, 2021 08:32

rishabh-d-dave added the needs-review label Mar 10, 2021

aaSharma14 reviewed Mar 10, 2021

View reviewed changes

aaSharma14 requested review from avanthakkar and epuertat March 11, 2021 05:14

aaSharma14 added this to In progress in Dashboard via automation Mar 11, 2021

aaSharma14 mentioned this pull request Mar 12, 2021

mgr/dashboard: prometheus alert test dependency docker.io breaks install.sh #40059

Closed

3 tasks

rishabh-d-dave requested a review from aaSharma14 March 16, 2021 03:40

rishabh-d-dave force-pushed the install-deps-docker branch from e4e6194 to 190d09f Compare March 16, 2021 07:39

aaSharma14 mentioned this pull request Mar 16, 2021

mgr/dashboard: test prometheus rules through promtool #39246

Merged

aaSharma14 requested a review from badone March 16, 2021 08:23

rishabh-d-dave force-pushed the install-deps-docker branch from 190d09f to 47f87ef Compare March 16, 2021 08:44

install-deps: rectify docker packages for fedora

562182b

Fixes: https://tracker.ceph.com/issues/49695 Signed-off-by: Rishabh Dave <ridave@redhat.com>

rishabh-d-dave force-pushed the install-deps-docker branch from 47f87ef to 562182b Compare March 17, 2021 04:51

Dashboard automation moved this from In progress to Review in progress Mar 17, 2021

tchaikov requested changes Mar 17, 2021

View reviewed changes

badone requested changes Mar 17, 2021

View reviewed changes

rishabh-d-dave closed this Mar 19, 2021

Dashboard automation moved this from Review in progress to Done Mar 19, 2021

rishabh-d-dave deleted the install-deps-docker branch March 23, 2021 04:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

install-deps: rectify docker packages for fedora #39995

install-deps: rectify docker packages for fedora #39995

rishabh-d-dave commented Mar 10, 2021

aaSharma14 Mar 10, 2021 •

edited

rishabh-d-dave Mar 13, 2021 •

edited

rishabh-d-dave Mar 16, 2021

aaSharma14 Mar 16, 2021

rishabh-d-dave Mar 16, 2021 •

edited

aaSharma14 Mar 16, 2021

rishabh-d-dave Mar 16, 2021

rishabh-d-dave Mar 16, 2021

aaSharma14 Mar 16, 2021

sunnyku commented Mar 11, 2021

rishabh-d-dave commented Mar 12, 2021

badone commented Mar 16, 2021

badone commented Mar 17, 2021 •

edited

badone commented Mar 17, 2021

tchaikov Mar 17, 2021

rishabh-d-dave commented Mar 17, 2021

badone left a comment

badone commented Mar 17, 2021

rishabh-d-dave commented Mar 17, 2021

epuertat commented Mar 17, 2021

badone commented Mar 17, 2021

badone commented Mar 18, 2021 •

edited

epuertat commented Mar 18, 2021

badone commented Mar 18, 2021

epuertat commented Mar 18, 2021

-                $SUDO dnf install -y moby-engine containerd
+            $SUDO dnf install -y dnf-utils
+            if rpm -q docker-ce || rpm -q docker-ce-cli || rpm -q moby-engine; then
+              echo "docker is already installed"
+            else
+              $SUDO dnf install -y moby-engine containerd
+            fi
+            $SUDO systemctl start docker
+            $SUDO systemctl enable docker

install-deps: rectify docker packages for fedora #39995

install-deps: rectify docker packages for fedora #39995

Conversation

rishabh-d-dave commented Mar 10, 2021

Checklist

aaSharma14 Mar 10, 2021 • edited

Choose a reason for hiding this comment

rishabh-d-dave Mar 13, 2021 • edited

Choose a reason for hiding this comment

rishabh-d-dave Mar 16, 2021

Choose a reason for hiding this comment

aaSharma14 Mar 16, 2021

Choose a reason for hiding this comment

rishabh-d-dave Mar 16, 2021 • edited

Choose a reason for hiding this comment

aaSharma14 Mar 16, 2021

Choose a reason for hiding this comment

rishabh-d-dave Mar 16, 2021

Choose a reason for hiding this comment

rishabh-d-dave Mar 16, 2021

Choose a reason for hiding this comment

aaSharma14 Mar 16, 2021

Choose a reason for hiding this comment

sunnyku commented Mar 11, 2021

rishabh-d-dave commented Mar 12, 2021

badone commented Mar 16, 2021

badone commented Mar 17, 2021 • edited

badone commented Mar 17, 2021

tchaikov Mar 17, 2021

Choose a reason for hiding this comment

rishabh-d-dave commented Mar 17, 2021

badone left a comment

Choose a reason for hiding this comment

badone commented Mar 17, 2021

rishabh-d-dave commented Mar 17, 2021

epuertat commented Mar 17, 2021

badone commented Mar 17, 2021

badone commented Mar 18, 2021 • edited

epuertat commented Mar 18, 2021

badone commented Mar 18, 2021

epuertat commented Mar 18, 2021

aaSharma14 Mar 10, 2021 •

edited

rishabh-d-dave Mar 13, 2021 •

edited

rishabh-d-dave Mar 16, 2021 •

edited

badone commented Mar 17, 2021 •

edited

badone commented Mar 18, 2021 •

edited