Fix: authenticated registry support for containerized hosts #5359

michaelgugino · 2017-09-11T17:14:12Z

Currently, openshift-anisble supports authentication to
container registries to pull down openshift container images.
The openshift_verison role uses the docker cli to gather
image information from container registries before authentication
credentials are provided by openshift-ansible.

This commit creates the necessary token to authenticate to
private registries during openshift_version.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1316341

sdodson · 2017-09-11T17:17:45Z

aos-ci-test

sdodson · 2017-09-11T17:17:48Z

[test]

michaelgugino · 2017-09-11T17:24:06Z

I have not tested this locally yet, and I don't think it will be tested by current CI. I am going to try to test this today.

kwoodson · 2017-09-11T18:17:03Z

roles/openshift_version/tasks/set_version_containerized.yml

@@ -2,6 +2,21 @@
 - set_fact:
    l_use_crio: "{{ openshift_use_crio | default(false) }}"


Shouldn't this move to the defaults/main.yml.

Probably should happen in another PR.

michaelgugino · 2017-09-11T19:03:45Z

This is not quite working according to my testing, please don't merge yet.

openshift-bot · 2017-09-11T21:34:39Z

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

openshift-bot · 2017-09-11T21:37:15Z

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

Currently, openshift-anisble supports authentication to container registries to pull down openshift container images. The openshift_verison role uses the docker cli to gather image information from container registries before authentication credentials are provided by openshift-ansible. This commit creates the necessary token to authenticate to private registries during openshift_version. The token is generated by the role 'docker' on all hosts where docker is installed/configured when oreg_auth_users is defined. This commit also adds a read-only mount into the openshift master and node container services. This mount is '/var/lib/origin/.docker:/root/.docker:ro'. This is because the container images do not currently read the values in '/var/lib/origin/.docker' as this may be a bug upstream. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1316341

openshift-bot · 2017-09-12T02:47:54Z

Evaluated for openshift ansible test up to db30a2e

michaelgugino · 2017-09-12T02:48:04Z

This patch is now tested as working with RHEL Atomic Host 7.x with the following variables:

oreg_auth_user: <my user>
oreg_auth_password: <my password>
oreg_url: private.<my domain>.com:443/openshift3/ose-${component}:latest
openshift_docker_additional_registries: 'private.<my domain>.com:443'
openshift_examples_modify_imagestreams: True
openshift_deployment_type: openshift-enterprise
openshift_release: v3.7
openshift_image_tag: v3.7.0-0.125.0.0
deployment_type: openshift-enterprise

Tested using v3.7 to force usage of staging server to ensure we're only pulling from there and not normal registry.

michaelgugino · 2017-09-12T02:49:36Z

roles/docker/tasks/package_docker.yml

+  when: oreg_auth_user is defined
+  register: docker_cli_auth_credentials_stat
+
+- name: Create credentials for docker cli registry auth


We need to create these credentials in /root/.docker/config.json to enable openshift_version to pull down the base container image. Additionally, having this populated on each container host is necessary for those hosts to pull down containers to themselves.

michaelgugino · 2017-09-12T02:51:44Z

roles/openshift_master/tasks/registry_auth.yml

+  register: master_oreg_auth_credentials_stat
+
+# Container images may need the registry credentials
+- name: Setup ro mount of /root/.docker for containerized hosts


We need to bind this directory into the container so the container can pull images. This may not be necessary on the master container services, I am not sure. Previous details indicated we need these credentials on both masters and nodes.

What we're doing here: Binding /var/lib/origin/.docker to /root/.docker.

michaelgugino · 2017-09-12T02:52:35Z

roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2

@@ -11,7 +11,17 @@ PartOf={{ openshift.docker.service_name }}.service
 EnvironmentFile=/etc/sysconfig/{{ openshift.common.service_type }}-master-controllers
 Environment=GOTRACEBACK=crash
 ExecStartPre=-/usr/bin/docker rm -f {{ openshift.common.service_type}}-master-controllers
-ExecStart=/usr/bin/docker run --rm --privileged --net=host --name {{ openshift.common.service_type }}-master-controllers --env-file=/etc/sysconfig/{{ openshift.common.service_type }}-master-controllers -v {{ r_openshift_master_data_dir }}:{{ r_openshift_master_data_dir }} -v /var/run/docker.sock:/var/run/docker.sock -v {{ openshift.common.config_base }}:{{ openshift.common.config_base }} {% if openshift_cloudprovider_kind | default('') != '' -%} -v {{ openshift.common.config_base }}/cloudprovider:{{ openshift.common.config_base}}/cloudprovider {% endif -%} -v /etc/pki:/etc/pki:ro {{ openshift.master.master_image }}:${IMAGE_VERSION} start master controllers --config=${CONFIG_FILE} $OPTIONS
+ExecStart=/usr/bin/docker run --rm --privileged --net=host \


systemd service files support line returns just like bash. This is much nicer and in the future will make for easier code reviews.

michaelgugino · 2017-09-12T02:53:05Z

roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2

+  -v {{ openshift.common.config_base }}:{{ openshift.common.config_base }} \
+  {% if openshift_cloudprovider_kind | default('') != '' -%} -v {{ openshift.common.config_base }}/cloudprovider:{{ openshift.common.config_base}}/cloudprovider {% endif -%} \
+  -v /etc/pki:/etc/pki:ro \
+  {% if l_bind_docker_reg_auth %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\


Here is where we potentially bind our credentials to /root/.docker

openshift-bot · 2017-09-12T04:39:55Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/645/) (Base Commit: 33d254a) (PR Branch Commit: db30a2e)

michaelgugino · 2017-09-12T13:22:19Z

aos-ci-test

openshift-bot · 2017-09-12T15:51:29Z

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

openshift-bot · 2017-09-12T15:54:20Z

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

kwoodson · 2017-09-13T14:46:20Z

roles/docker/defaults/main.yml

+docker_cli_auth_config_path: '/root/.docker'
+
+oreg_url: ''
+oreg_host: "{{ oreg_url.split('/')[0] if '.' in oreg_url.split('/')[0] else '' }}"


In the future we should update this to use urlsplit

sdodson · 2017-09-13T19:10:22Z

[merge]

sdodson · 2017-09-13T19:11:02Z

We need backports for the other releases that you backported the original PR to, right?

michaelgugino · 2017-09-13T19:14:45Z

@sdodson
Potentially. Or we could say this is only supported in containerized hosts past version X. Since it's sort of a RFE. I'm fine either way, what do you think?

openshift-bot · 2017-09-13T19:16:15Z

Evaluated for openshift ansible merge up to db30a2e

openshift-bot · 2017-09-13T19:33:13Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/1027/) (Base Commit: 665c5c2) (PR Branch Commit: db30a2e)

sdodson · 2017-09-13T19:37:49Z

Potentially. Or we could say this is only supported in containerized hosts past version X. Since it's sort of a RFE. I'm fine either way, what do you think?

We should backport this PR to all the branches that got your original work since QE is considering this change as necessary to close out bug 1316341

which would be all the way back to release-1.4

michaelgugino · 2017-09-13T19:42:19Z

@sdodson as soon as this merges, I'll cherry pick to back port.

mtnbikenc

LGTM

openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 11, 2017

michaelgugino requested review from sdodson, abutcher, kwoodson and mtnbikenc September 11, 2017 17:14

sdodson previously approved these changes Sep 11, 2017

View reviewed changes

kwoodson previously approved these changes Sep 11, 2017

View reviewed changes

michaelgugino dismissed stale reviews from kwoodson and sdodson via db30a2e September 12, 2017 02:41

michaelgugino force-pushed the version-docker-auth-config branch from 3e9533d to db30a2e Compare September 12, 2017 02:41

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 12, 2017

michaelgugino commented Sep 12, 2017

View reviewed changes

kwoodson approved these changes Sep 13, 2017

View reviewed changes

sdodson added affects_3.5 affects_3.6 labels Sep 13, 2017

mtnbikenc approved these changes Sep 13, 2017

View reviewed changes

openshift-bot merged commit a0086cb into openshift:master Sep 13, 2017

sdodson mentioned this pull request Sep 13, 2017

[3.6] Fix: authenticated registry support for containerized hosts #5406

Merged

This was referenced Sep 14, 2017

[1.5] Fix: authenticated registry support for containerized hosts #5418

Merged

[1.4] Fix: authenticated registry support for containerized hosts #5419

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: authenticated registry support for containerized hosts #5359

Fix: authenticated registry support for containerized hosts #5359

michaelgugino commented Sep 11, 2017

sdodson commented Sep 11, 2017

sdodson commented Sep 11, 2017

michaelgugino commented Sep 11, 2017

kwoodson Sep 11, 2017

michaelgugino commented Sep 11, 2017

openshift-bot commented Sep 11, 2017

openshift-bot commented Sep 11, 2017

openshift-bot commented Sep 12, 2017

michaelgugino commented Sep 12, 2017

michaelgugino Sep 12, 2017

michaelgugino Sep 12, 2017

michaelgugino Sep 12, 2017

michaelgugino Sep 12, 2017

openshift-bot commented Sep 12, 2017

michaelgugino commented Sep 12, 2017

openshift-bot commented Sep 12, 2017

openshift-bot commented Sep 12, 2017

kwoodson Sep 13, 2017

sdodson commented Sep 13, 2017

sdodson commented Sep 13, 2017

michaelgugino commented Sep 13, 2017

openshift-bot commented Sep 13, 2017

openshift-bot commented Sep 13, 2017 •

edited

sdodson commented Sep 13, 2017 •

edited

michaelgugino commented Sep 13, 2017

mtnbikenc left a comment

		@@ -2,6 +2,21 @@
		- set_fact:
		l_use_crio: "{{ openshift_use_crio \| default(false) }}"

Fix: authenticated registry support for containerized hosts #5359

Fix: authenticated registry support for containerized hosts #5359

Conversation

michaelgugino commented Sep 11, 2017

sdodson commented Sep 11, 2017

sdodson commented Sep 11, 2017

michaelgugino commented Sep 11, 2017

kwoodson Sep 11, 2017

Choose a reason for hiding this comment

michaelgugino commented Sep 11, 2017

openshift-bot commented Sep 11, 2017

openshift-bot commented Sep 11, 2017

openshift-bot commented Sep 12, 2017

michaelgugino commented Sep 12, 2017

michaelgugino Sep 12, 2017

Choose a reason for hiding this comment

michaelgugino Sep 12, 2017

Choose a reason for hiding this comment

michaelgugino Sep 12, 2017

Choose a reason for hiding this comment

michaelgugino Sep 12, 2017

Choose a reason for hiding this comment

openshift-bot commented Sep 12, 2017

michaelgugino commented Sep 12, 2017

openshift-bot commented Sep 12, 2017

openshift-bot commented Sep 12, 2017

kwoodson Sep 13, 2017

Choose a reason for hiding this comment

sdodson commented Sep 13, 2017

sdodson commented Sep 13, 2017

michaelgugino commented Sep 13, 2017

openshift-bot commented Sep 13, 2017

openshift-bot commented Sep 13, 2017 • edited

sdodson commented Sep 13, 2017 • edited

michaelgugino commented Sep 13, 2017

mtnbikenc left a comment

Choose a reason for hiding this comment

openshift-bot commented Sep 13, 2017 •

edited

sdodson commented Sep 13, 2017 •

edited