Remove dependency on playbook2image, rebase directly on OS. #4742

juanvallejo · 2017-07-12T19:22:48Z

Remove dependency on playbook2image by only adding the functionality that we currently use from it directly to our container image.

Draft: https://docs.google.com/document/d/1AlvlI0dw_vGHD_Fx4psWxbAuCXIc7blJjT554bGfdUo
Trello card: https://trello.com/c/BHJYd3Y5

TODO

Known issues

Running playbooks/byo/config.yml deploys OpenShift in a remote host successfully but fails when deploying logging with: Failed to get information on remote file (/tmp/openshift-logging-ansible-PERtiY/signing.conf): /bin/sh: sudo: command not found
- when openshift_hosted_logging_deploy=true

cc @sosiouxme @rhcarvalho @codificat @brenton

rhcarvalho · 2017-07-12T20:49:49Z

Out of curiosity, why do we want to part ways with playbook2image?

sosiouxme · 2017-07-12T19:33:05Z

images/installer/Dockerfile.rhel7

@@ -18,6 +18,8 @@ LABEL name="openshift3/ose-ansible" \
      version="v3.6.0" \
      release="1" \
      architecture="x86_64"
+      vcs-url="https://github.com/openshift/openshift-ansible" \
+      vcs-type="git" \

 # Playbooks, roles and their dependencies are installed from packages.
 # Unlike in Dockerfile, we don't invoke the 'assemble' script here


Can probably remove the rest of this comment

sosiouxme · 2017-07-12T19:36:05Z

images/installer/Dockerfile.rhel7

@@ -18,6 +18,8 @@ LABEL name="openshift3/ose-ansible" \
      version="v3.6.0" \
      release="1" \
      architecture="x86_64"
+      vcs-url="https://github.com/openshift/openshift-ansible" \
+      vcs-type="git" \


thought there was another label p2i had that o-a didn't... something pep posted in chat

cc @codificat

atomic.run="once" \

Also the last line shouldn't have a \

sosiouxme · 2017-07-12T19:36:32Z

images/installer/bin/run

+
+if [[ -z PLAYBOOK_FILE ]]; then
+    exec /usr/bin/usage
+    exit 0


not necessary after exec

sosiouxme · 2017-07-12T19:37:10Z

images/installer/bin/run

+  curl -o ${INVENTORY} ${DYNAMIC_SCRIPT_URL}
+  chmod 755 ${INVENTORY}
+else
+  echo "One of INVENTORY_FILE, INVENTORY_URL or DYNAMIC_SCRIPT_URL must be provided"


can this run usage after?

sosiouxme · 2017-07-13T02:26:11Z

Most of what playbook2image installs is for operating as a builder, and we don't need that capability. So we can simplify and drop the size a lot by leaving it out. But more importantly, the downstream playbook2image is based on the python SCL which operates with different paths and versions from the system python packages and from the upstream pip-installed modules. It seems prudent to keep upstream and downstream for openshift-ansible as similar as possible - the differences have already caused some problems. We can cut out the SCL and pip and keep things the same.

…

On Wed, Jul 12, 2017 at 4:49 PM, Rodolfo Carvalho ***@***.***> wrote: Out of curiosity, why do we want to part ways with playbook2image? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#4742 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABz-q8q_3uQdAQDmgqBiXnSZue4cjuBks5sNTFwgaJpZM4OWFiO> .

rhcarvalho · 2017-07-13T10:04:33Z

images/installer/Dockerfile

+RUN pip install -Iv ansible==2.2.0.0
+
+COPY ./images/installer/bin /usr/bin
+COPY ./images/installer/bin/user_setup /tmp


Since we are optimizing the image, we could reduce the number of layers by having a single COPY instruction, and placing all the files under ./images/installer/root. Then, we can group together all RUN instructions into a single one.

rhcarvalho · 2017-07-13T10:05:17Z

images/installer/Dockerfile

+      io.openshift.tags="openshift,install,upgrade,ansible" \
+      vcs-url="https://github.com/openshift/openshift-ansible" \
+      vcs-type="git" \
+      version="alpha"


What is this label used for?

https://docs.docker.com/engine/reference/builder/#label just part of image metadata; however, I seem to have kept the default used by p2i. Not sure what the value should be here; 1.6.0?

Sure, some of those labels are used for example in the web console. It made sense for p2i since it was meant to be used as a builder image (thus the io.k8s.* and io.openshift.* labels). Some other labels are used in build systems.

If we have no use for a label, perhaps it is a good time to get rid of it instead of copying from p2i?

A version label seems to be only one more thing that needs to be kept up-to-date.

(If we really need a version label, we could have our image build script set the right version on build)

rhcarvalho · 2017-07-13T10:21:13Z

images/installer/Dockerfile

 # Create a symlink to /opt/app-root/src so that files under /usr/share/ansible are accessible.
 # This is required since the system-container uses by default the playbook under
 # /usr/share/ansible/openshift-ansible.  With this change we won't need to keep two different
 # configurations for the two images.
 RUN mkdir -p /usr/share/ansible/ && ln -s /opt/app-root/src /usr/share/ansible/openshift-ansible

+# Make home folder writable in order to let playbooks make changes
+RUN chmod a+rwx -R /opt/app-root/src
+
 RUN INSTALL_PKGS="skopeo openssl java-1.8.0-openjdk-headless httpd-tools" && \
    yum install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
    rpm -V $INSTALL_PKGS && \
    yum clean all

 USER ${USER_UID}


This was defined in p2i, now at this point it is undefined.

sosiouxme · 2017-07-13T11:04:49Z

images/installer/Dockerfile

@@ -1,7 +1,7 @@
 # Using playbook2image as a base
 # See https://github.com/openshift/playbook2image for details on the image
 # including documentation for the settings/env vars referenced below
-FROM registry.centos.org/openshift/playbook2image:latest
+FROM openshift/base-centos7


uh... why this one? This brings in a bunch of dev stuff we don't need. Why not just centos7?

Was not sure which one to use but for some reason, openshift/base-centos7 seemed more inviting. Will switch to just centos7

sosiouxme · 2017-07-13T11:09:20Z

images/installer/Dockerfile

+RUN yum install -y epel-release && yum clean all -y
+
+# workaround for https://github.com/openshift/openshift-ansible/issues/3111
+# install ansible via pip to lock version


this is resolved with later ansible, should be able to use latest ansible from EPEL or origin repos

sosiouxme · 2017-07-13T11:09:56Z

images/installer/Dockerfile

+# workaround for https://github.com/openshift/openshift-ansible/issues/3111
+# install ansible via pip to lock version
+RUN yum install -y  --setopt=tsflags=nodocs python-pip python-devel && yum clean all -y
+RUN pip install -Iv ansible==2.2.0.0


so we should just be able to yum install ansible

sounds good, should we still install python / worry about having any other deps that Dockerfile.rhel7 installs as well?

nope, don't see any need for them... however we are probably going to need to add RPMs for role dependencies that were pip installed upstream and RPM dependencies downstream.

sosiouxme · 2017-07-13T11:12:00Z

images/installer/Dockerfile

 # dependencies specified in requirements.txt and install the 'oc' client
 # as per the INSTALL_OC environment setting above
-RUN /usr/libexec/s2i/assemble
+RUN /usr/bin/assemble


we don't need any of this assemble stuff

I'm wondering what we need the oc client for either but I guess we should include that in case something is actually using it.

we don't need the INSTALL_OC variable if we just hardcode that bit from assemble

sosiouxme · 2017-07-13T11:22:37Z

images/installer/bin/assemble

+  echo "---> Installing 'oc' binary..."
+  TMPDIR=$(mktemp -d)
+  cd ${TMPDIR}
+  OC_BINARY_URL=$(python -c "import requests;releases = requests.get('https://api.github.com/repos/openshift/origin/releases').json();print [s for s in [r for r in releases if not r['prerelease'] and '1.4' in r['name']][0]['assets'] if 'linux-64' in s['browser_download_url']][0]['browser_download_url']")


gets latest origin oc... origin-only of course. err, it's a little unclear to me what this actually ends up with... 1.4 releases?

downstream, RPM deps will pull in oc

sounds good, will remove

sosiouxme · 2017-07-13T11:26:50Z

images/installer/bin/run

+else
+  echo "One of INVENTORY_FILE, INVENTORY_URL or DYNAMIC_SCRIPT_URL must be provided"
+  exec /usr/bin/usage
+  exit 1


exec doesn't return

sosiouxme · 2017-07-13T12:54:12Z

images/installer/Dockerfile.rhel7

@@ -18,26 +18,42 @@ LABEL name="openshift3/ose-ansible" \
      version="v3.6.0" \
      release="1" \
      architecture="x86_64"


needs a \ at the end

sosiouxme · 2017-07-13T12:54:47Z

images/installer/Dockerfile.rhel7

 USER root
 RUN INSTALL_PKGS="atomic-openshift-utils atomic-openshift-clients python-boto skopeo openssl java-1.8.0-openjdk-headless httpd-tools" && \
    yum repolist > /dev/null && \
    yum-config-manager --enable rhel-7-server-ose-3.6-rpms && \
    yum-config-manager --enable rhel-7-server-rh-common-rpms && \
+    yum install -y --setopt=tsflags=nodocs ansible


needs && \

Looking at INSTALL_PKS, do we still need some of them? (looking at java in particular)

I think Java was added to be able to run the logging installer? Not sure. It was added relatively recently, we can check the history.

codificat

Adding some comments/questions/issues found so far, haven't had time to actually build and test the images yet

codificat · 2017-07-14T17:40:55Z

images/installer/Dockerfile

-      io.k8s.display-name="openshift-ansible" \
-      io.k8s.description="A containerized openshift-ansible image to let you run playbooks to install, upgrade, maintain and check an OpenShift cluster" \
-      io.openshift.expose-services="" \
-      io.openshift.tags="openshift,install,upgrade,ansible"


I'm not sure we should be removing these: there are use cases that include running the image inside openshift, for which this metadata would be useful...

codificat · 2017-07-14T17:56:23Z

images/installer/Dockerfile

+ && yum install -y --setopt=tsflags=nodocs --enablerepo=epel ansible \
+ && INSTALL_PKGS="python-lxml pyOpenSSL python2-cryptography skopeo openssl java-1.8.0-openjdk-headless httpd-tools" \
+ && yum install -y --setopt=tsflags=nodocs $INSTALL_PKGS \
+ && rpm -V $INSTALL_PKGS \


is this rpm verification needed/useful? (just wondering)

I have seen too many instances of doing yum install X Y Z and having Z not available for some reason but yum still installs everything else and reports success. This fails if you don't install everything you wanted.

although rpm -q should work just as well...

Case in point! skopeo comes from extras and wasn't being installed. And we don't actually need it.

What @sosiouxme said. We use rpm -V in many of the images we ship with OpenShift, e.g.:

https://github.com/sclorg/postgresql-container/blob/master/9.5/Dockerfile#L44

And yes, it saves time debugging why an image is missing some package we think was installed.

codificat · 2017-07-14T18:01:46Z

images/installer/Dockerfile

+    PLAYBOOK_FILE=playbooks/byo/openshift_facts.yml \
+    OPTS="-v"
+
+COPY ./images/installer/bin /usr/bin


I'm being picky, but this copies a README file into /usr/bin... I'd suggest moving the README.md inside images/installer/bin one level up and expand it a bit (see below)

Also leveraging the practice of writing Dockerfiles... what @codificat says + make a single top-level dir root and place files mirroring what we want to see in the image. Things that need not / should not be in the image should be outside of root. E.g.:

https://github.com/sclorg/postgresql-container/blob/master/9.5/Dockerfile#L56

codificat · 2017-07-14T18:03:44Z

images/installer/bin/README.md

+==================================================
+
+Contains image entrypoint as well as any other scripts useful
+in aiding an image run.


I believe this file can be moved up one level and briefly document the contents, pointing to other readmes for details... it should make it easier to find and also avoid it ending up in /usr/bin

codificat · 2017-07-14T18:06:59Z

images/installer/Dockerfile

+ && chmod -R ug+x /opt/app-root/{bin,etc} /usr/bin/user_setup \
+#
+# Make home folder writable in order to let playbooks make changes
+ && chmod a+rwx -R /opt/app-root/src \


I'm failing to see why we need this...

At some point I was getting an error because .ansible could not be written to /opt/app-root/src. Will test again though

codificat · 2017-07-14T18:19:48Z

images/installer/Dockerfile.rhel7

-      io.k8s.display-name="openshift-ansible" \
-      io.k8s.description="$DESCRIPTION" \
-      io.openshift.expose-services="" \
-      io.openshift.tags="openshift,install,upgrade,ansible" \


Ditto about these labels.. they are certainly not required, but I believe it doesn't hurt to have them in this image

codificat · 2017-07-14T18:20:56Z

images/installer/Dockerfile.rhel7

      com.redhat.component="aos3-installation-docker" \
-      version="v3.6.0" \


This one is actually required by the build system

codificat · 2017-07-14T18:22:34Z

images/installer/Dockerfile.rhel7

 USER root
 RUN INSTALL_PKGS="atomic-openshift-utils atomic-openshift-clients python-boto skopeo openssl java-1.8.0-openjdk-headless httpd-tools" && \
    yum repolist > /dev/null && \
    yum-config-manager --enable rhel-7-server-ose-3.6-rpms && \
    yum-config-manager --enable rhel-7-server-rh-common-rpms && \
+    yum install -y --setopt=tsflags=nodocs ansible && \


I believe ansible gets pulled as a dependency - but if we want to install explicitly maybe just add it to INSTALL_PKGS above...

+1 it does.

codificat · 2017-07-14T18:27:42Z

images/installer/Dockerfile.rhel7


+RUN mkdir -p ${APP_HOME} ${APP_ROOT}/etc ${APP_ROOT}/bin
+RUN chmod -R ug+x ${APP_ROOT}/bin ${APP_ROOT}/etc /tmp/user_setup && \
+    /tmp/user_setup


These can be consolidated in a single RUN.
More importantly, they reference stuff that is defined in ENV and COPY statements that are below - order should be fixed

codificat · 2017-07-14T18:35:37Z

images/installer/Dockerfile.rhel7


-CMD [ "/usr/libexec/s2i/run" ]
+RUN sed "s@${USER_NAME}:x:${USER_UID}:0@${USER_NAME}:x:\${USER_ID}:\${GROUP_ID}@g" /etc/passwd > ${APP_ROOT}/etc/passwd.template


This RUN is now replaced by the entrypoint, which isn't defined... so this line should be replaced with ENTRYPOINT ["/usr/bin/entrypoint"]

codificat · 2017-07-17T10:45:17Z

images/installer/Dockerfile


-USER root
+ENV USER_UID=1000 \


In Dockerfile.rhel7 it says 1001 instead - let's pick one and make it the same in both

Will change to 1001 to match .rhel7. Changed it here to 1000 to test while I finished adding the entrypoint script that patched /etc/passwd with a 1000 user entry

codificat · 2017-07-17T10:47:35Z

images/installer/Dockerfile


 MAINTAINER OpenShift Team <dev@lists.openshift.redhat.com>

+USER root
+


Is there any reason to move the USER and RUN commands before the LABELs? If not, wouldn't it be better to leave the LABELs first to keep the metadata type of content at the top? The caching related benefits of that are probably not relevant in our case, but still wondering if it would make sense from a file organization POV?

It's really just that when iterating on changes, it helps to put the slowest thing (the install) first so it can be most likely to be cached. Not a huge deal to me but I don't like having to re-run the install just because I tweaked the labels. Labels are still pretty close to the top... but we can move it if you like.

Heh, in the "normal" use case it's usually the other way around: when you build the same image over time, labels change less than the result of 'yum install' - so having the labels before the run should help caching.

However, this doesn't really apply to our build system (I think), so it was more of a comment around the logical structure of the file. To me it looks better to have the metadata first, but up to you.

Our build system doesn't use cache so as far as I'm concerned the only optimizations to consider are for iterative local development. So I vote to leave it as is unless I hear loud complaining :)

codificat · 2017-07-17T10:50:47Z

images/installer/Dockerfile

-CMD [ "/usr/libexec/s2i/run" ]
+WORKDIR ${WORK_DIR}
+ENTRYPOINT [ "/usr/local/bin/entrypoint" ]
+CMD [ "/usr/local/bin/run" ]


Now that this has been re-arranged I think it would make sense to complete the work by moving the contents under system-container/root into root so there's only one root.

The README.md inside system-container could be moved higher up as README_SYSTEM_CONTAINER.md, or its contents integrated into README_CONTAINER_IMAGE.md...

Will go ahead and add a commit for this

Changed in e02bc5d

codificat · 2017-07-17T10:52:27Z

images/installer/Dockerfile.rhel7

+ && yum-config-manager --enable rhel-7-server-rh-common-rpms \
+ && yum install -y --setopt=tsflags=nodocs $INSTALL_PKGS \
+ && rpm -q $INSTALL_PKGS \
+ && yum clean all

 LABEL name="openshift3/ose-ansible" \


Same question as with Dockerfile: should LABEL come before USER/RUN?

sosiouxme · 2017-07-18T02:59:05Z

As best I can tell, everything is working. I have never seen the /bin/sh: sudo: command not found issue when deploying logging (and metrics). I also don't have or need the localhost line in my inventory. But it is disquieting that Juan does and makes me feel like users will manage to run into it. But is that something new?

/cc @ashcrow @giuseppe As best I can tell, the atomic install --system container works fine with both origin-ansible and ose-ansible after these changes. Would appreciate your input.

(We don't have the labels for atomic install to operate on in a non-system-container context - something to introduce in the near future hopefully as behavior is very surprising without the --system.)

codificat

One pending fix and a minor question

codificat · 2017-07-18T09:42:38Z

images/installer/Dockerfile

+# Add entrypoint and other setup scripts
+COPY images/installer/root /
+# Add files for running as a system container
+COPY images/installer/system-container/root /


With recent changes, system-container/root does not exist anymore and the previous COPY already takes care of the system container files, so this should be removed

Ah meant to remove this but never pushed the change :/

codificat · 2017-07-18T10:16:06Z

images/installer/Dockerfile

+# Add files for running as a system container
+COPY images/installer/system-container/root /
+# Include playbooks, roles, plugins, etc. from this repo
+COPY . ${WORK_DIR}


It might be a good opportunity to review .dockerignore... e.g. should we add images and `hack? anything else?

Why don't we copy only COPY root /? Or is this supposed to copy openshift-ansible top-level contents?

It's suppose to copy top-level contents - playbooks, roles, plugins, etc. These things are all installed by RPM downstream. We could certainly .ignore a lot of things.

@codificat we actually need images/ in origin-ansible :) and ose-ansible is already sparse. We can omit hack/, inventory/ and most of the top-level conf files. Do the README.md files belong in the image? Not sure how useful they are but they do little harm...

Oh actually... .md files already omitted.

sosiouxme · 2017-07-18T13:52:39Z

I think it's ready once we squash commits.

ashcrow · 2017-07-18T14:18:30Z

images/installer/Dockerfile

 LABEL name="openshift/origin-ansible" \
      summary="OpenShift's installation and configuration tool" \
      description="A containerized openshift-ansible image to let you run playbooks to install, upgrade, maintain and check an OpenShift cluster" \
      url="https://github.com/openshift/openshift-ansible" \
      io.k8s.display-name="openshift-ansible" \
      io.k8s.description="A containerized openshift-ansible image to let you run playbooks to install, upgrade, maintain and check an OpenShift cluster" \
      io.openshift.expose-services="" \
-      io.openshift.tags="openshift,install,upgrade,ansible"
+      io.openshift.tags="openshift,install,upgrade,ansible" \
+      atomic.run="once"


ashcrow · 2017-07-18T14:30:32Z

images/installer/Dockerfile.rhel7

-# As the playbooks are installed via packages instead of being copied to
-# $APP_HOME by the 'assemble' script, we set the WORK_DIR env var to the
-# location of openshift-ansible.
-ENV PLAYBOOK_FILE=playbooks/byo/openshift_facts.yml \


Just curious, why was PLAYBOOK_FILE removed while other env items were kept?

I want the image to do something helpful if you just run it without reading the docs (or if you miss salient items). To me the most helpful thing seems to print a usage message immediately and exit. If we specify PLAYBOOK_FILE then we're going to run a playbook, and running a "usage" playbook seemed like overkill when we can just... run usage.

ashcrow · 2017-07-18T14:30:55Z

images/installer/Dockerfile

-# The playbook to be run is specified via the PLAYBOOK_FILE env var.
-# This sets a default of openshift_facts.yml as it's an informative playbook
-# that can help test that everything is set properly (inventory, sshkeys)
-ENV PLAYBOOK_FILE=playbooks/byo/openshift_facts.yml \


Just curious, why was PLAYBOOK_FILE removed while other env items were kept?

ashcrow · 2017-07-18T14:34:24Z

It seems like reasonable changes to me. Nothing jumps out at me as being problematic but I suggest letting QE know they should re-test the container and system containers just to be sure.

@giuseppe WDYT?

sosiouxme · 2017-07-18T14:44:18Z

We absolutely want QE to scrutinize the major use cases once we merge and rebuild.

https://trello.com/c/BHJYd3Y5/135-drop-playbook2image-dependency

juanvallejo · 2017-07-18T16:01:55Z

README_CONTAINER_IMAGE.md

@@ -1,6 +1,6 @@
 # Containerized openshift-ansible to run playbooks

-The [Dockerfile](images/installer/Dockerfile) in this repository uses the [playbook2image](https://github.com/openshift/playbook2image) source-to-image base image to containerize `openshift-ansible`. The resulting image can run any of the provided playbooks. See [BUILD.md](BUILD.md) for image build instructions.
+The [Dockerfile](images/installer/Dockerfile) in this repository uses the [Centos:7](https://hub.docker.com/_/centos/) base image to containerize `openshift-ansible`. The resulting image can run any of the provided playbooks. See [BUILD.md](BUILD.md) for image build instructions.


@sosiouxme @codificat maybe we should just take the line about the base image out altogether?

Agreed that it can be removed

juanvallejo · 2017-07-18T16:05:25Z

BUILD.md

@@ -37,9 +37,9 @@ To build a container image of `openshift-ansible` using standalone **Docker**:

 To build an openshift-ansible image using an **OpenShift** [build and image stream](https://docs.openshift.org/latest/architecture/core_concepts/builds_and_image_streams.html) the straightforward command would be:

-        oc new-build registry.centos.org/openshift/playbook2image~https://github.com/openshift/openshift-ansible
+        oc new-build openshift/origin-ansible~https://github.com/openshift/openshift-ansible


@sosiouxme @codificat wasn't sure if this was the right thing to put here... Would this command actually work?

Also, per the docs in the lines bellow, technically we are no longer "required" to keep the images/installer structure

We could move Dockerfile back to root ... but we would need to keep images/installer for the downstream build anyway. Not inclined to change this just now.

codificat · 2017-07-18T16:11:58Z

BUILD.md

@@ -37,9 +37,9 @@ To build a container image of `openshift-ansible` using standalone **Docker**:

 To build an openshift-ansible image using an **OpenShift** [build and image stream](https://docs.openshift.org/latest/architecture/core_concepts/builds_and_image_streams.html) the straightforward command would be:

-        oc new-build registry.centos.org/openshift/playbook2image~https://github.com/openshift/openshift-ansible
+        oc new-build openshift/origin-ansible~https://github.com/openshift/openshift-ansible


that doesn't look right... openshift/origin-ansible is not a builder image...

I think it's not that easy to build the image in OpenShift anymore... the Dockerfile is under images/ but the context is the tld...

Maybe we can just leave out the whole Building on OpenShift section for now?

Maybe we can just leave out the whole Building on OpenShift section for now?

Okay, I think that might be the best approach while we figure this out

cc @brenton

works for me.

I'm sure there's a way to invoke the docker builder but I don't particularly want to try and test it right now, so save that for later.

We do not need the builder functionality from playbook2image and the resulting image was overly complicated, so this simply builds on Centos/RHEL.

sosiouxme · 2017-07-18T19:13:44Z

aos-ci-test

openshift-bot · 2017-07-18T23:06:48Z

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 5497673 (logs)

openshift-bot · 2017-07-19T03:26:17Z

error: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for 5497673 (logs)

sosiouxme · 2017-07-19T16:53:34Z

Errr... test flake? Doesn't look related.

sosiouxme · 2017-07-19T16:53:40Z

aos-ci-test

openshift-bot · 2017-07-19T18:18:43Z

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 5497673 (logs)

openshift-bot · 2017-07-19T18:21:16Z

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for 5497673 (logs)

sosiouxme · 2017-07-19T18:24:56Z

[merge] 🎉

sosiouxme · 2017-07-19T21:09:10Z

flake although it looks rather too persistent to be called that. @sdodson should I try the merge again?

sosiouxme · 2017-07-20T02:05:49Z

[merge] again just for protocol

rhcarvalho · 2017-07-20T10:16:34Z

Flake openshift/origin#15332 and openshift/origin#13977

[merge]

openshift-bot · 2017-07-20T10:23:19Z

Evaluated for openshift ansible merge up to 5497673

openshift-bot · 2017-07-20T14:23:37Z

continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/720/) (Base Commit: e432f8a) (PR Branch Commit: 5497673)

sosiouxme · 2017-07-20T14:29:10Z

Just openshift/origin#15356 again... @sdodson can you merge please?

rhcarvalho · 2017-07-20T14:44:51Z

For future reference, do we have the size of the image BEFORE and AFTER this change? @juanvallejo @sosiouxme

rhcarvalho · 2017-07-20T14:47:33Z

Also for our future selves, could you please update the PR description to reflect the current state? Are there still known issues, are the checkboxes in the TODO reasonable?

sosiouxme · 2017-07-20T14:52:56Z

In my testing, ose-ansible's size before was about 1.1G and after, 760M. origin-ansible was 880M and is now 454M. Now that I look at it I'm not sure why the difference from origin to ose is so much... should have nearly the same dependencies, and base image size is about the same.

sosiouxme · 2017-07-20T14:58:52Z

Hm, I should update the origin-ansible job to tag with v3.7 when building from master.
Update: done.

sosiouxme · 2017-07-20T19:46:13Z

There is no 3.7 branch in dist-git aos3-installation-docker yet. I am curious to see if the build tomorrow morning pulls in changes from master or has been re-pointed at release-3.6.

juanvallejo requested review from sosiouxme and codificat July 12, 2017 19:23

juanvallejo force-pushed the jvallejo/remove-dependency-on-p2i branch 2 times, most recently from e2e62ee to 2bc1a2d Compare July 12, 2017 20:15

sosiouxme reviewed Jul 12, 2017

View reviewed changes

juanvallejo force-pushed the jvallejo/remove-dependency-on-p2i branch 2 times, most recently from fe203fb to 8a82808 Compare July 12, 2017 23:02

rhcarvalho reviewed Jul 13, 2017

View reviewed changes

sosiouxme reviewed Jul 13, 2017

View reviewed changes

juanvallejo force-pushed the jvallejo/remove-dependency-on-p2i branch 3 times, most recently from bf59e7c to e7407e5 Compare July 13, 2017 21:17

codificat requested changes Jul 14, 2017

View reviewed changes

codificat requested changes Jul 17, 2017

View reviewed changes

juanvallejo force-pushed the jvallejo/remove-dependency-on-p2i branch 2 times, most recently from 68e7fb9 to 08ece2a Compare July 17, 2017 17:01

codificat approved these changes Jul 18, 2017

View reviewed changes

codificat requested changes Jul 18, 2017

View reviewed changes

sosiouxme changed the title ~~[WIP] Remove dependency on playbook2image~~ Remove dependency on playbook2image, rebase directly on OS. Jul 18, 2017

ashcrow reviewed Jul 18, 2017

View reviewed changes

codificat approved these changes Jul 18, 2017

View reviewed changes

juanvallejo commented Jul 18, 2017

View reviewed changes

codificat reviewed Jul 18, 2017

View reviewed changes

image builds: remove dependency on playbook2image

5497673

We do not need the builder functionality from playbook2image and the resulting image was overly complicated, so this simply builds on Centos/RHEL.

sosiouxme force-pushed the jvallejo/remove-dependency-on-p2i branch from b93ad95 to 5497673 Compare July 18, 2017 19:12

sosiouxme approved these changes Jul 18, 2017

View reviewed changes

rhcarvalho merged commit d13c11a into openshift:master Jul 20, 2017

sosiouxme mentioned this pull request Jul 20, 2017

master cherry picks for 3.6.1 #4814

Merged

juanvallejo deleted the jvallejo/remove-dependency-on-p2i branch July 25, 2017 13:56

		com.redhat.component="aos3-installation-docker" \
		version="v3.6.0" \


		CMD [ "/usr/libexec/s2i/run" ]
		RUN sed "s@${USER_NAME}:x:${USER_UID}:0@${USER_NAME}:x:\${USER_ID}:\${GROUP_ID}@g" /etc/passwd > ${APP_ROOT}/etc/passwd.template


		MAINTAINER OpenShift Team <dev@lists.openshift.redhat.com>

		USER root

Remove dependency on playbook2image, rebase directly on OS. #4742

Remove dependency on playbook2image, rebase directly on OS. #4742

Conversation

juanvallejo commented Jul 12, 2017 • edited by rhcarvalho Loading

rhcarvalho commented Jul 12, 2017

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

sosiouxme commented Jul 13, 2017 via email

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

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

codificat 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

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

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

sosiouxme commented Jul 18, 2017

codificat 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

sosiouxme commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosiouxme Jul 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashcrow commented Jul 18, 2017

juanvallejo commented Jul 12, 2017 •

edited by rhcarvalho

Loading

sosiouxme Jul 18, 2017 •

edited

Loading