Ansible ubi upgrade #1990

camilamacedo86 · 2019-09-28T09:34:01Z

Description of the change:

Upgrade ansible images to use ubi8 instead of 7 (for now it has the conditionals to pass in both)

NOTE:
To use ubi8 is required:
-to upgrade epel from 7 to 8 as well.
-python3-pip instead of python36-pip

Motivation
#1656

camilamacedo86 · 2019-09-28T15:44:43Z

/test e2e-aws-go
/test e2e-aws-helm
/test e2e-aws-ansible
/test e2e-aws-subcommand
/test images

ci/tests/e2e-ansible.sh

camilamacedo86 · 2019-10-01T09:32:42Z

/test e2e-aws-go

estroz

/lgtm

after rebase

joelanford

One comment about combining docker layers. I also want to experiment quickly with this because I was thinking there might be more simplifications we can make.

joelanford · 2019-10-01T19:20:05Z

ci/dockerfiles/ansible-e2e-hybrid.Dockerfile

+RUN yum clean all && rm -rf /var/cache/yum/*
+
+# todo; remove ubi7 after CI be updated with images
+# ubi7
+RUN if $(cat /etc/redhat-release | grep --quiet 'release 7'); then (yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm || true); fi
+# ubi8
+RUN if $(cat /etc/redhat-release | grep --quiet 'release 8'); then (yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-8.noarch.rpm || true); fi
+
+RUN yum -y update \
+ && yum install -y python36-devel gcc
+
+# ubi7
+RUN if $(cat /etc/redhat-release | grep --quiet 'release 7'); then (yum install -y python36-pip || true); fi
+# ubi8
+RUN if $(cat /etc/redhat-release | grep --quiet 'release 8'); then (yum install -y python3-pip || true); fi
+
+# Install inotify-tools. Note: rpm -i will install the rpm in the registry for allow yum install it.
+RUN curl -O https://rpmfind.net/linux/fedora/linux/releases/30/Everything/x86_64/os/Packages/i/inotify-tools-3.14-16.fc30.x86_64.rpm \


I think we should still be able to use the \ <newline> && syntax to have this entire set of stuff happen in a single docker layer.

We will remove all ifs in the next PR. It is just for don't break the CI. Then, all will come back to be just 1 layer. Is it make sense? WDYT? Could we move forward here?

I don't think there's a huge rush on this, and I think we should be more diligent than we normally would be because the ansible base image build is a pretty regular source of issues in CI.

I don't have a major problem with multi-layer vs. single layer since that shouldn't make a difference in the resulting image.

However, I'm a bit confused with some of the additions and changes for ubi8 and want to make sure that we get things right from the start.

For ubi7, my impression is that we are installing the EPEL repo because it contains inotify-tools, so if we're using EPEL, we should be installing inotify-tools with yum install -y inotify-tools, not by downloading and manually installing.

For ubi8, it appears that EPEL does not contain inotify-tools, so I don't think we should be installing EPEL in ubi8. In this case, I'm curious why it disappeared, and if it is now present in another repo, or if it has been removed entirely from RHEL/UBI8. I'd rather that we not download from rpmfind.net if there's an official place that inotify-tools has moved. And if it is totally removed from official RHEL/UBI8 repos, we should have a comment (and ideally a link to discussion about its removal) in the Dockerfile.

Hi @joelanford,

Thank you for your review and inputs.

Just to clarify my idea was we do it now just to pass in the prow CI and then when we have the ubi8 we could improve it.

Regards the epel I think is a good spot and we can remove it. 👍

Regards the inotify-tools it was removed from rhel8 and we have for now inotify3-tools. So, let's try to use it now and avoid do the download. Note that if we run locally the docker image with then all is installed, so let's cross the fingers for inotify3-tools work well for us as inotify-tools and pass in all ci tests as well.

Please, let me know if has anything else that you would like to change and/or are you ok with move forward with now?

joelanford · 2019-10-01T19:28:54Z

Also @camilamacedo86 can you remove the Jira link in the PR description and replace it with either direct text or a link to another public github issue. That Jira is private and not readable by the community.

fabianvf

/lgtm as long as we come back and fix the image layers right after everything merges.

camilamacedo86 · 2019-10-01T20:32:10Z

/test e2e-aws-subcommand
/test e2e-aws-ansible
/test e2e-aws-helm
/test e2e-aws-go

joelanford · 2019-10-02T13:52:13Z

/lgtm
/approve

openshift-ci-robot requested review from fabianvf and theishshah September 28, 2019 09:34

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 28, 2019

camilamacedo86 mentioned this pull request Sep 28, 2019

upgrade oper images to use ubi8 openshift/release#5217

Merged

camilamacedo86 force-pushed the ansible-ubi-upgrade branch from e98c21b to ac16889 Compare September 28, 2019 10:06

camilamacedo86 force-pushed the ansible-ubi-upgrade branch 3 times, most recently from 88e8dd8 to 13f2d22 Compare September 28, 2019 12:48

camilamacedo86 requested review from joelanford, estroz, AlexNPavel and jmrodri September 28, 2019 14:19

operator-framework deleted a comment from openshift-ci-robot Sep 28, 2019

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 30, 2019

camilamacedo86 force-pushed the ansible-ubi-upgrade branch 2 times, most recently from 6a6ec80 to 686516b Compare September 30, 2019 16:02

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 30, 2019

joelanford reviewed Sep 30, 2019

View reviewed changes

ci/tests/e2e-ansible.sh Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the ansible-ubi-upgrade branch 2 times, most recently from 585bcf4 to eb15e97 Compare September 30, 2019 17:01

openshift-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 30, 2019

camilamacedo86 requested a review from fabianvf September 30, 2019 21:11

estroz approved these changes Oct 1, 2019

View reviewed changes

openshift-ci-robot assigned estroz Oct 1, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2019

joelanford requested changes Oct 1, 2019

View reviewed changes

camilamacedo86 requested a review from joelanford October 1, 2019 19:30

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2019

camilamacedo86 requested a review from estroz October 1, 2019 19:31

fabianvf approved these changes Oct 1, 2019

View reviewed changes

camilamacedo86 force-pushed the ansible-ubi-upgrade branch from deb2943 to 3740f7d Compare October 2, 2019 09:19

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

camilamacedo86 added 6 commits October 2, 2019 10:23

upgrade ansible ubi8 images

c78f54e

upgrade rhel to 8 and remove python36-pip which do not exist

5f50996

use python3-pip instead of python36-pip

b530d36

changelog

294de12

add conitionals to pass in ubi7 and ubi8

b74569f

improve images ubi/ubi8 by usng its libs

976ee81

camilamacedo86 force-pushed the ansible-ubi-upgrade branch from 3740f7d to 976ee81 Compare October 2, 2019 09:23

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 2, 2019

camilamacedo86 requested a review from fabianvf October 2, 2019 10:37

openshift-ci-robot assigned joelanford Oct 2, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 2, 2019

joelanford approved these changes Oct 2, 2019

View reviewed changes

camilamacedo86 merged commit 1a5772e into operator-framework:master Oct 2, 2019

camilamacedo86 deleted the ansible-ubi-upgrade branch October 2, 2019 14:11

camilamacedo86 mentioned this pull request Oct 9, 2019

Ansible Operator on a Diet #1553

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ansible ubi upgrade #1990

Ansible ubi upgrade #1990

camilamacedo86 commented Sep 28, 2019 •

edited

Loading

camilamacedo86 commented Sep 28, 2019

camilamacedo86 commented Oct 1, 2019

estroz left a comment

joelanford left a comment

joelanford Oct 1, 2019

camilamacedo86 Oct 1, 2019 •

edited

Loading

joelanford Oct 1, 2019

camilamacedo86 Oct 2, 2019 •

edited

Loading

joelanford commented Oct 1, 2019

fabianvf left a comment

camilamacedo86 commented Oct 1, 2019

joelanford commented Oct 2, 2019

Ansible ubi upgrade #1990

Ansible ubi upgrade #1990

Conversation

camilamacedo86 commented Sep 28, 2019 • edited Loading

camilamacedo86 commented Sep 28, 2019

camilamacedo86 commented Oct 1, 2019

estroz left a comment

Choose a reason for hiding this comment

joelanford left a comment

Choose a reason for hiding this comment

joelanford Oct 1, 2019

Choose a reason for hiding this comment

camilamacedo86 Oct 1, 2019 • edited Loading

Choose a reason for hiding this comment

joelanford Oct 1, 2019

Choose a reason for hiding this comment

camilamacedo86 Oct 2, 2019 • edited Loading

Choose a reason for hiding this comment

joelanford commented Oct 1, 2019

fabianvf left a comment

Choose a reason for hiding this comment

camilamacedo86 commented Oct 1, 2019

joelanford commented Oct 2, 2019

camilamacedo86 commented Sep 28, 2019 •

edited

Loading

camilamacedo86 Oct 1, 2019 •

edited

Loading

camilamacedo86 Oct 2, 2019 •

edited

Loading