summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Gugino <mgugino@redhat.com>2017-09-27 12:03:43 -0400
committerMichael Gugino <mgugino@redhat.com>2017-09-27 12:06:32 -0400
commit9d6e86c0217c97d33aecdcb47c35521a6ee91a29 (patch)
tree66ff813418e083ed711a187810e1622d6dcf8ddf
parentcc161d1b79f53b7daa4837f9fc262455cf04a4f4 (diff)
downloadopenshift-9d6e86c0217c97d33aecdcb47c35521a6ee91a29.tar.gz
openshift-9d6e86c0217c97d33aecdcb47c35521a6ee91a29.tar.bz2
openshift-9d6e86c0217c97d33aecdcb47c35521a6ee91a29.tar.xz
openshift-9d6e86c0217c97d33aecdcb47c35521a6ee91a29.zip
Migrate enterprise registry logic to docker role
Currently, the enterprise registry to forcefully added in openshift_facts. Recently, the docker role has been modified to consume registry variables directly, bypassing openshift_facts. This commit cleans up unused code in openshift_facts, and migrates enterprise registry logic to the docker role. Fixes: https://github.com/openshift/openshift-ansible/issues/5557
-rw-r--r--playbooks/common/openshift-cluster/initialize_oo_option_facts.yml9
-rw-r--r--roles/docker/defaults/main.yml2
-rw-r--r--roles/docker/tasks/package_docker.yml8
-rw-r--r--roles/docker/tasks/systemcontainer_crio.yml12
-rw-r--r--roles/docker/tasks/systemcontainer_docker.yml6
-rw-r--r--roles/openshift_docker_facts/tasks/main.yml9
-rwxr-xr-xroles/openshift_facts/library/openshift_facts.py24
-rw-r--r--roles/openshift_health_checker/openshift_checks/docker_image_availability.py2
-rw-r--r--roles/openshift_health_checker/test/docker_image_availability_test.py10
9 files changed, 25 insertions, 57 deletions
diff --git a/playbooks/common/openshift-cluster/initialize_oo_option_facts.yml b/playbooks/common/openshift-cluster/initialize_oo_option_facts.yml
index ac3c702a0..dab17aaa9 100644
--- a/playbooks/common/openshift-cluster/initialize_oo_option_facts.yml
+++ b/playbooks/common/openshift-cluster/initialize_oo_option_facts.yml
@@ -5,15 +5,6 @@
- always
tasks:
- set_fact:
- openshift_docker_additional_registries: "{{ lookup('oo_option', 'docker_additional_registries') }}"
- when: openshift_docker_additional_registries is not defined
- - set_fact:
- openshift_docker_insecure_registries: "{{ lookup('oo_option', 'docker_insecure_registries') }}"
- when: openshift_docker_insecure_registries is not defined
- - set_fact:
- openshift_docker_blocked_registries: "{{ lookup('oo_option', 'docker_blocked_registries') }}"
- when: openshift_docker_blocked_registries is not defined
- - set_fact:
openshift_docker_options: "{{ lookup('oo_option', 'docker_options') }}"
when: openshift_docker_options is not defined
- set_fact:
diff --git a/roles/docker/defaults/main.yml b/roles/docker/defaults/main.yml
index 274fd8603..e36dfa7b9 100644
--- a/roles/docker/defaults/main.yml
+++ b/roles/docker/defaults/main.yml
@@ -9,6 +9,8 @@ openshift_docker_additional_registries: []
openshift_docker_blocked_registries: []
openshift_docker_insecure_registries: []
+openshift_docker_ent_reg: 'registry.access.redhat.com'
+
# The l2_docker_* variables convert csv strings to lists, if
# necessary. These variables should be used in place of their respective
# openshift_docker_* counterparts to ensure the properly formatted lists are
diff --git a/roles/docker/tasks/package_docker.yml b/roles/docker/tasks/package_docker.yml
index 0c5621259..4215dc5bd 100644
--- a/roles/docker/tasks/package_docker.yml
+++ b/roles/docker/tasks/package_docker.yml
@@ -50,6 +50,14 @@
src: custom.conf.j2
when: not os_firewall_use_firewalld | default(False) | bool
+- name: Add enterprise registry, if necessary
+ set_fact:
+ l2_docker_additional_registries: "{{ l2_docker_additional_registries + [openshift_docker_ent_reg] }}"
+ when:
+ - openshift.common.deployment_type == 'openshift-enterprise'
+ - openshift_docker_ent_reg != ''
+ - openshift_docker_ent_reg not in l2_docker_additional_registries
+
- stat: path=/etc/sysconfig/docker
register: docker_check
diff --git a/roles/docker/tasks/systemcontainer_crio.yml b/roles/docker/tasks/systemcontainer_crio.yml
index 5b02b72be..66ce475e1 100644
--- a/roles/docker/tasks/systemcontainer_crio.yml
+++ b/roles/docker/tasks/systemcontainer_crio.yml
@@ -1,17 +1,17 @@
---
# TODO: Much of this file is shared with container engine tasks
- set_fact:
- l_insecure_crio_registries: "{{ '\"{}\"'.format('\", \"'.join(openshift.docker.insecure_registries)) }}"
- when: openshift.docker.insecure_registries
+ l_insecure_crio_registries: "{{ '\"{}\"'.format('\", \"'.join(l2_docker_insecure_registries)) }}"
+ when: l2_docker_insecure_registries
- set_fact:
- l_crio_registries: "{{ openshift.docker.additional_registries + ['docker.io'] }}"
- when: openshift.docker.additional_registries
+ l_crio_registries: "{{ l2_docker_additional_registries + ['docker.io'] }}"
+ when: l2_docker_additional_registries
- set_fact:
l_crio_registries: "{{ ['docker.io'] }}"
- when: not openshift.docker.additional_registries
+ when: not l2_docker_additional_registries
- set_fact:
l_additional_crio_registries: "{{ '\"{}\"'.format('\", \"'.join(l_crio_registries)) }}"
- when: openshift.docker.additional_registries
+ when: l2_docker_additional_registries
- name: Ensure container-selinux is installed
package:
diff --git a/roles/docker/tasks/systemcontainer_docker.yml b/roles/docker/tasks/systemcontainer_docker.yml
index 146e5f430..8b43393cb 100644
--- a/roles/docker/tasks/systemcontainer_docker.yml
+++ b/roles/docker/tasks/systemcontainer_docker.yml
@@ -148,10 +148,10 @@
# Set local versions of facts that must be in json format for container-daemon.json
# NOTE: When jinja2.9+ is used the container-daemon.json file can move to using tojson
- set_fact:
- l_docker_insecure_registries: "{{ docker_insecure_registries | default([]) | to_json }}"
+ l_docker_insecure_registries: "{{ l2_docker_insecure_registries | default([]) | to_json }}"
l_docker_log_options: "{{ docker_log_options | default({}) | to_json }}"
- l_docker_additional_registries: "{{ docker_additional_registries | default([]) | to_json }}"
- l_docker_blocked_registries: "{{ docker_blocked_registries | default([]) | to_json }}"
+ l_docker_additional_registries: "{{ l2_docker_additional_registries | default([]) | to_json }}"
+ l_docker_blocked_registries: "{{ l2_docker_blocked_registries | default([]) | to_json }}"
l_docker_selinux_enabled: "{{ docker_selinux_enabled | default(true) | to_json }}"
# Configure container-engine using the container-daemon.json file
diff --git a/roles/openshift_docker_facts/tasks/main.yml b/roles/openshift_docker_facts/tasks/main.yml
index 334150f63..5a3e50678 100644
--- a/roles/openshift_docker_facts/tasks/main.yml
+++ b/roles/openshift_docker_facts/tasks/main.yml
@@ -6,9 +6,6 @@
with_items:
- role: docker
local_facts:
- additional_registries: "{{ openshift_docker_additional_registries | default(None) }}"
- blocked_registries: "{{ openshift_docker_blocked_registries | default(None) }}"
- insecure_registries: "{{ openshift_docker_insecure_registries | default(None) }}"
selinux_enabled: "{{ openshift_docker_selinux_enabled | default(None) }}"
log_driver: "{{ openshift_docker_log_driver | default(None) }}"
log_options: "{{ openshift_docker_log_options | default(None) }}"
@@ -23,12 +20,6 @@
sdn_mtu: "{{ openshift_node_sdn_mtu | default(None) }}"
- set_fact:
- docker_additional_registries: "{{ openshift.docker.additional_registries
- | default(omit) }}"
- docker_blocked_registries: "{{ openshift.docker.blocked_registries
- | default(omit) }}"
- docker_insecure_registries: "{{ openshift.docker.insecure_registries
- | default(omit) }}"
docker_selinux_enabled: "{{ openshift.docker.selinux_enabled | default(omit) }}"
docker_log_driver: "{{ openshift.docker.log_driver | default(omit) }}"
docker_log_options: "{{ openshift.docker.log_options | default(omit) }}"
diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py
index 1c2c91a5a..11ef9fa97 100755
--- a/roles/openshift_facts/library/openshift_facts.py
+++ b/roles/openshift_facts/library/openshift_facts.py
@@ -55,9 +55,6 @@ def migrate_docker_facts(facts):
""" Apply migrations for docker facts """
params = {
'common': (
- 'additional_registries',
- 'insecure_registries',
- 'blocked_registries',
'options'
),
'node': (
@@ -768,14 +765,6 @@ def set_deployment_facts_if_unset(facts):
service_type = 'origin'
facts['common']['service_type'] = service_type
- if 'docker' in facts:
- deployment_type = facts['common']['deployment_type']
- if deployment_type == 'openshift-enterprise':
- addtl_regs = facts['docker'].get('additional_registries', [])
- ent_reg = 'registry.access.redhat.com'
- if ent_reg not in addtl_regs:
- facts['docker']['additional_registries'] = addtl_regs + [ent_reg]
-
for role in ('master', 'node'):
if role in facts:
deployment_type = facts['common']['deployment_type']
@@ -2248,19 +2237,6 @@ class OpenShiftFacts(object):
protected_facts_to_overwrite)
if 'docker' in new_local_facts:
- # remove duplicate and empty strings from registry lists, preserving order
- for cat in ['additional', 'blocked', 'insecure']:
- key = '{0}_registries'.format(cat)
- if key in new_local_facts['docker']:
- val = new_local_facts['docker'][key]
- if isinstance(val, string_types):
- val = [x.strip() for x in val.split(',')]
- seen = set()
- new_local_facts['docker'][key] = list()
- for registry in val:
- if registry not in seen and registry != '':
- seen.add(registry)
- new_local_facts['docker'][key].append(registry)
# Convert legacy log_options comma sep string to a list if present:
if 'log_options' in new_local_facts['docker'] and \
isinstance(new_local_facts['docker']['log_options'], string_types):
diff --git a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
index 98372d979..93a5973d4 100644
--- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
@@ -153,7 +153,7 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
def known_docker_registries(self):
"""Build a list of docker registries available according to inventory vars."""
- regs = list(self.get_var("openshift.docker.additional_registries", default=[]))
+ regs = list(self.get_var("openshift_docker_additional_registries", default=[]))
deployment_type = self.get_var("openshift_deployment_type")
if deployment_type == "origin" and "docker.io" not in regs:
diff --git a/roles/openshift_health_checker/test/docker_image_availability_test.py b/roles/openshift_health_checker/test/docker_image_availability_test.py
index 952fa9aa6..c523ffd5c 100644
--- a/roles/openshift_health_checker/test/docker_image_availability_test.py
+++ b/roles/openshift_health_checker/test/docker_image_availability_test.py
@@ -72,7 +72,7 @@ def test_all_images_available_remotely(task_vars, available_locally):
return {'images': [], 'failed': available_locally}
return {}
- task_vars['openshift']['docker']['additional_registries'] = ["docker.io", "registry.access.redhat.com"]
+ task_vars['openshift_docker_additional_registries'] = ["docker.io", "registry.access.redhat.com"]
task_vars['openshift_image_tag'] = 'v3.4'
check = DockerImageAvailability(execute_module, task_vars)
check._module_retry_interval = 0
@@ -90,7 +90,7 @@ def test_all_images_unavailable(task_vars):
return {} # docker_image_facts failure
- task_vars['openshift']['docker']['additional_registries'] = ["docker.io"]
+ task_vars['openshift_docker_additional_registries'] = ["docker.io"]
task_vars['openshift_deployment_type'] = "openshift-enterprise"
task_vars['openshift_image_tag'] = 'latest'
check = DockerImageAvailability(execute_module, task_vars)
@@ -121,9 +121,9 @@ def test_no_known_registries():
service_type='origin',
is_containerized=False,
is_atomic=False,
- ),
- docker=dict(additional_registries=["docker.io"]),
+ )
),
+ openshift_docker_additional_registries=["docker.io"],
openshift_deployment_type="openshift-enterprise",
openshift_image_tag='latest',
group_names=['nodes', 'masters'],
@@ -154,7 +154,7 @@ def test_skopeo_update_failure(task_vars, message, extra_words):
return {}
- task_vars['openshift']['docker']['additional_registries'] = ["unknown.io"]
+ task_vars['openshift_docker_additional_registries'] = ["unknown.io"]
task_vars['openshift_deployment_type'] = "openshift-enterprise"
check = DockerImageAvailability(execute_module, task_vars)
check._module_retry_interval = 0