Merge pull request #5449 from abutcher/wildcard-router-cert-redeploy

Automatic merge from submit-queue.

Bug 1490186: Router pod not running after router certificates redeployment

This carries https://github.com/openshift/openshift-ansible/pull/5417. More of the router cert redeploy logic could be moved into the `openshift_hosted` role with a flag. I may pull those over.

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

lookup_plugins/README.md

@@ -0,0 +1 @@
+openshift-ansible lookup plugins.

lookup_plugins/oo_option.py

@@ -1,74 +0,0 @@
-#!/usr/bin/env python2
-# -*- coding: utf-8 -*-
-'''
-oo_option lookup plugin for openshift-ansible
-
-Usage:
-
-    - debug:
-      msg: "{{ lookup('oo_option', '<key>') | default('<default_value>', True) }}"
-
-This returns, by order of priority:
-
-* if it exists, the `cli_<key>` ansible variable. This variable is set by `bin/cluster --option <key>=<value> …`
-* if it exists, the envirnoment variable named `<key>`
-* if none of the above conditions are met, empty string is returned
-'''
-
-
-import os
-
-# pylint: disable=no-name-in-module,import-error,unused-argument,unused-variable,super-init-not-called,too-few-public-methods,missing-docstring
-try:
-    # ansible-2.0
-    from ansible.plugins.lookup import LookupBase
-except ImportError:
-    # ansible-1.9.x
-    class LookupBase(object):
-        def __init__(self, basedir=None, runner=None, **kwargs):
-            self.runner = runner
-            self.basedir = self.runner.basedir
-
-            def get_basedir(self, variables):
-                return self.basedir
-
-
-# Reason: disable too-few-public-methods because the `run` method is the only
-#     one required by the Ansible API
-# Status: permanently disabled
-# pylint: disable=too-few-public-methods
-class LookupModule(LookupBase):
-    ''' oo_option lookup plugin main class '''
-
-    # Reason: disable unused-argument because Ansible is calling us with many
-    #     parameters we are not interested in.
-    #     The lookup plugins of Ansible have this kwargs “catch-all” parameter
-    #     which is not used
-    # Status: permanently disabled unless Ansible API evolves
-    # pylint: disable=unused-argument
-    def __init__(self, basedir=None, **kwargs):
-        ''' Constructor '''
-        self.basedir = basedir
-
-    # Reason: disable unused-argument because Ansible is calling us with many
-    #     parameters we are not interested in.
-    #     The lookup plugins of Ansible have this kwargs “catch-all” parameter
-    #     which is not used
-    # Status: permanently disabled unless Ansible API evolves
-    # pylint: disable=unused-argument
-    def run(self, terms, variables, **kwargs):
-        ''' Main execution path '''
-
-        ret = []
-
-        for term in terms:
-            option_name = term.split()[0]
-            cli_key = 'cli_' + option_name
-            if 'vars' in variables and cli_key in variables['vars']:
-                ret.append(variables['vars'][cli_key])
-            elif option_name in os.environ:
-                ret.append(os.environ[option_name])
-            else:
-                ret.append('')
-
-        return ret

openshift-ansible.spec

@@ -84,10 +84,6 @@ touch %{buildroot}%{_datadir}/ansible/%{name}/roles/contiv/.empty_dir
 pushd %{buildroot}%{_datadir}/ansible/%{name}/roles/openshift_master_facts/filter_plugins
 ln -sf ../../../../../ansible_plugins/filter_plugins/oo_filters.py oo_filters.py
 popd
-# openshift_master_facts symlinks lookup_plugins/oo_option.py from ansible_plugins/lookup_plugins
-pushd %{buildroot}%{_datadir}/ansible/%{name}/roles/openshift_master_facts/lookup_plugins
-ln -sf ../../../../../ansible_plugins/lookup_plugins/oo_option.py oo_option.py
-popd
 
 # openshift-ansible-filter-plugins install
 cp -rp filter_plugins %{buildroot}%{_datadir}/ansible_plugins/

playbooks/byo/rhel_subscribe.yml

@@ -11,6 +11,6 @@
     when:
     - deployment_type == 'openshift-enterprise'
     - ansible_distribution == "RedHat"
-    - lookup('oo_option', 'rhel_skip_subscription') | default(rhsub_skip, True) | default('no', True) | lower in ['no', 'false']
+    - lookup('env', 'rhel_skip_subscription') | default(rhsub_skip, True) | default('no', True) | lower in ['no', 'false']
   - role: openshift_repos
   - role: os_update_latest

playbooks/common/openshift-cluster/config.yml

@@ -18,10 +18,6 @@
       - docker_image_availability
       - docker_storage
 
-- include: initialize_oo_option_facts.yml
-  tags:
-  - always
-
 - include: ../openshift-etcd/config.yml
 
 - include: ../openshift-nfs/config.yml

playbooks/common/openshift-cluster/initialize_oo_option_facts.yml

@@ -1,18 +0,0 @@
----
-- name: Set oo_option facts
-  hosts: oo_all_hosts
-  tags:
-  - always
-  tasks:
-  - set_fact:
-      openshift_docker_options: "{{ lookup('oo_option', 'docker_options') }}"
-    when: openshift_docker_options is not defined
-  - set_fact:
-      openshift_docker_log_driver: "{{ lookup('oo_option', 'docker_log_driver') }}"
-    when: openshift_docker_log_driver is not defined
-  - set_fact:
-      openshift_docker_log_options: "{{ lookup('oo_option', 'docker_log_options') }}"
-    when: openshift_docker_log_options is not defined
-  - set_fact:
-      openshift_docker_selinux_enabled: "{{ lookup('oo_option', 'docker_selinux_enabled') }}"
-    when: openshift_docker_selinux_enabled is not defined

playbooks/common/openshift-cluster/redeploy-certificates/router.yml

@@ -7,23 +7,34 @@
   tasks:
   - name: Create temp directory for kubeconfig
     command: mktemp -d /tmp/openshift-ansible-XXXXXX
-    register: mktemp
+    register: router_cert_redeploy_tempdir
     changed_when: false
+
   - name: Copy admin client config(s)
     command: >
-      cp {{ openshift.common.config_base }}/master//admin.kubeconfig {{ mktemp.stdout }}/admin.kubeconfig
+      cp {{ openshift.common.config_base }}/master//admin.kubeconfig {{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
     changed_when: false
 
   - name: Determine if router exists
     command: >
       {{ openshift.common.client_binary }} get dc/router -o json
-      --config={{ mktemp.stdout }}/admin.kubeconfig
+      --config={{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
       -n default
     register: l_router_dc
     failed_when: false
     changed_when: false
 
-  - set_fact:
+  - name: Determine if router service exists
+    command: >
+      {{ openshift.common.client_binary }} get svc/router -o json
+      --config={{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
+      -n default
+    register: l_router_svc
+    failed_when: false
+    changed_when: false
+
+  - name: Collect router environment variables and secrets
+    set_fact:
       router_env_vars: "{{ ((l_router_dc.stdout | from_json)['spec']['template']['spec']['containers'][0]['env']
                              | oo_collect('name'))
                              | default([]) }}"
@@ -34,20 +45,32 @@
     changed_when: false
     when: l_router_dc.rc == 0
 
+  - name: Collect router service annotations
+    set_fact:
+      router_service_annotations: "{{ (l_router_svc.stdout | from_json)['metadata']['annotations'] if 'annotations' in (l_router_svc.stdout | from_json)['metadata'] else [] }}"
+    when: l_router_svc.rc == 0
+
   - name: Update router environment variables
     shell: >
       {{ openshift.common.client_binary }} env dc/router
       OPENSHIFT_CA_DATA="$(cat /etc/origin/master/ca.crt)"
       OPENSHIFT_CERT_DATA="$(cat /etc/origin/master/openshift-router.crt)"
       OPENSHIFT_KEY_DATA="$(cat /etc/origin/master/openshift-router.key)"
-      --config={{ mktemp.stdout }}/admin.kubeconfig
+      --config={{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
       -n default
-    when: l_router_dc.rc == 0 and 'OPENSHIFT_CA_DATA' in router_env_vars and 'OPENSHIFT_CERT_DATA' in router_env_vars and 'OPENSHIFT_KEY_DATA' in router_env_vars
+    when:
+    - l_router_dc.rc == 0
+    - ('OPENSHIFT_CA_DATA' in router_env_vars)
+    - ('OPENSHIFT_CERT_DATA' in router_env_vars)
+    - ('OPENSHIFT_KEY_DATA' in router_env_vars)
 
+  # When the router service contains service signer annotations we
+  # will delete the existing certificate secret and allow OpenShift to
+  # replace the secret.
   - block:
     - name: Delete existing router certificate secret
       oc_secret:
-        kubeconfig: "{{ mktemp.stdout }}/admin.kubeconfig"
+        kubeconfig: "{{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig"
         name: router-certs
         namespace: default
         state: absent
@@ -58,86 +81,61 @@
         {{ openshift.common.client_binary }} annotate service/router
         service.alpha.openshift.io/serving-cert-secret-name-
         service.alpha.openshift.io/serving-cert-signed-by-
-        --config={{ mktemp.stdout }}/admin.kubeconfig
+        --config={{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
         -n default
 
     - name: Add serving-cert-secret annotation to router service
       command: >
         {{ openshift.common.client_binary }} annotate service/router
         service.alpha.openshift.io/serving-cert-secret-name=router-certs
-        --config={{ mktemp.stdout }}/admin.kubeconfig
+        --config={{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
         -n default
-    when: l_router_dc.rc == 0 and 'router-certs' in router_secrets and openshift_hosted_router_certificate is undefined
+    when:
+    - l_router_dc.rc == 0
+    - l_router_svc.rc == 0
+    - ('router-certs' in router_secrets)
+    - openshift_hosted_router_certificate is undefined
+    - ('service.alpha.openshift.io/serving-cert-secret-name') in router_service_annotations
+    - ('service.alpha.openshift.io/serving-cert-signed-by') in router_service_annotations
 
-  - block:
-    - assert:
-        that:
-        - "'certfile' in openshift_hosted_router_certificate"
-        - "'keyfile' in openshift_hosted_router_certificate"
-        - "'cafile' in openshift_hosted_router_certificate"
-        msg: |-
-          openshift_hosted_router_certificate has been set in the inventory but is
-          missing one or more required keys. Ensure that 'certfile', 'keyfile',
-          and 'cafile' keys have been specified for the openshift_hosted_router_certificate
-          inventory variable.
-
-    - name: Read router certificate and key
-      become: no
-      local_action:
-        module: slurp
-        src: "{{ item }}"
-      register: openshift_router_certificate_output
-      # Defaulting dictionary keys to none to avoid deprecation warnings
-      # (future fatal errors) during template evaluation. Dictionary keys
-      # won't be accessed unless openshift_hosted_router_certificate is
-      # defined and has all keys (certfile, keyfile, cafile) which we
-      # check above.
-      with_items:
-      - "{{ (openshift_hosted_router_certificate | default({'certfile':none})).certfile }}"
-      - "{{ (openshift_hosted_router_certificate | default({'keyfile':none})).keyfile }}"
-      - "{{ (openshift_hosted_router_certificate | default({'cafile':none})).cafile }}"
-
-    - name: Write temporary router certificate file
-      copy:
-        content: "{% for certificate in openshift_router_certificate_output.results -%}{{ certificate.content | b64decode }}{% endfor -%}"
-        dest: "{{ mktemp.stdout }}/openshift-hosted-router-certificate.pem"
-        mode: 0600
-
-    - name: Write temporary router key file
-      copy:
-        content: "{{ (openshift_router_certificate_output.results
-                         | oo_collect('content', {'source':(openshift_hosted_router_certificate | default({'keyfile':none})).keyfile}))[0] | b64decode }}"
-        dest: "{{ mktemp.stdout }}/openshift-hosted-router-certificate.key"
-        mode: 0600
-
-    - name: Replace router-certs secret
-      shell: >
-        {{ openshift.common.client_binary }} secrets new router-certs
-        tls.crt="{{ mktemp.stdout }}/openshift-hosted-router-certificate.pem"
-        tls.key="{{ mktemp.stdout }}/openshift-hosted-router-certificate.key"
-        --type=kubernetes.io/tls
-        --config={{ mktemp.stdout }}/admin.kubeconfig
-        --confirm
-        -o json | {{ openshift.common.client_binary }} --config={{ mktemp.stdout }}/admin.kubeconfig replace -f -
+  # When there are no annotations on the router service we will allow
+  # the openshift_hosted role to either create a new wildcard
+  # certificate (since we deleted the original) or reapply a custom
+  # openshift_hosted_router_certificate.
+  - file:
+      path: "{{ item }}"
+      state: absent
+    with_items:
+    - /etc/origin/master/openshift-router.crt
+    - /etc/origin/master/openshift-router.key
+    when:
+    - l_router_dc.rc == 0
+    - l_router_svc.rc == 0
+    - ('router-certs' in router_secrets)
+    - ('service.alpha.openshift.io/serving-cert-secret-name') not in router_service_annotations
+    - ('service.alpha.openshift.io/serving-cert-signed-by') not in router_service_annotations
 
-    - name: Remove temporary router certificate and key files
-      file:
-        path: "{{ item }}"
-        state: absent
-      with_items:
-      - "{{ mktemp.stdout }}/openshift-hosted-router-certificate.pem"
-      - "{{ mktemp.stdout }}/openshift-hosted-router-certificate.key"
-    when: l_router_dc.rc == 0 and 'router-certs' in router_secrets and openshift_hosted_router_certificate is defined
+  - include_role:
+      name: openshift_hosted
+      tasks_from: main
+    vars:
+      openshift_hosted_manage_registry: false
+    when:
+    - l_router_dc.rc == 0
+    - l_router_svc.rc == 0
+    - ('router-certs' in router_secrets)
+    - ('service.alpha.openshift.io/serving-cert-secret-name') not in router_service_annotations
+    - ('service.alpha.openshift.io/serving-cert-signed-by') not in router_service_annotations
 
   - name: Redeploy router
     command: >
       {{ openshift.common.client_binary }} deploy dc/router
       --latest
-      --config={{ mktemp.stdout }}/admin.kubeconfig
+      --config={{ router_cert_redeploy_tempdir.stdout }}/admin.kubeconfig
       -n default
 
   - name: Delete temp directory
     file:
-      name: "{{ mktemp.stdout }}"
+      name: "{{ router_cert_redeploy_tempdir.stdout }}"
       state: absent
     changed_when: False

playbooks/common/openshift-cluster/upgrades/init.yml

@@ -5,8 +5,6 @@
     g_new_master_hosts: []
     g_new_node_hosts: []
 
-- include: ../initialize_oo_option_facts.yml
-
 - include: ../initialize_facts.yml
 
 - name: Ensure firewall is not switched during upgrade

playbooks/common/openshift-master/config.yml

@@ -20,9 +20,6 @@
 
 - name: Gather and set facts for master hosts
   hosts: oo_masters_to_config
-  vars:
-    t_oo_option_master_debug_level: "{{ lookup('oo_option', 'openshift_master_debug_level') }}"
-
   pre_tasks:
   # Per https://bugzilla.redhat.com/show_bug.cgi?id=1469336
   #
@@ -55,33 +52,12 @@
     - .config_managed
 
   - set_fact:
-      openshift_master_pod_eviction_timeout: "{{ lookup('oo_option', 'openshift_master_pod_eviction_timeout') | default(none, true) }}"
-    when: openshift_master_pod_eviction_timeout is not defined
-
-  - set_fact:
       openshift_master_etcd_port: "{{ (etcd_client_port | default('2379')) if (groups.oo_etcd_to_config is defined and groups.oo_etcd_to_config) else none }}"
       openshift_master_etcd_hosts: "{{ hostvars
                                        | oo_select_keys(groups['oo_etcd_to_config']
                                                         | default([]))
                                        | oo_collect('openshift.common.hostname')
                                        | default(none, true) }}"
-
-  - set_fact:
-      openshift_master_debug_level: "{{ t_oo_option_master_debug_level }}"
-    when: openshift_master_debug_level is not defined and t_oo_option_master_debug_level != ""
-
-  - set_fact:
-      openshift_master_default_subdomain: "{{ lookup('oo_option', 'openshift_master_default_subdomain') | default(None, true) }}"
-    when: openshift_master_default_subdomain is not defined
-  - set_fact:
-      openshift_hosted_metrics_deploy: "{{ lookup('oo_option', 'openshift_hosted_metrics_deploy') | default(false, true) }}"
-    when: openshift_hosted_metrics_deploy is not defined
-  - set_fact:
-      openshift_hosted_metrics_duration: "{{ lookup('oo_option', 'openshift_hosted_metrics_duration') | default(7) }}"
-    when: openshift_hosted_metrics_duration is not defined
-  - set_fact:
-      openshift_hosted_metrics_resolution: "{{ lookup('oo_option', 'openshift_hosted_metrics_resolution') | default('10s', true) }}"
-    when: openshift_hosted_metrics_resolution is not defined
   roles:
   - openshift_facts
   post_tasks:

roles/openshift_hosted/tasks/router.yml

@@ -52,9 +52,9 @@
         certfile: "{{ openshift_master_config_dir ~ '/openshift-router.crt' }}"
         keyfile: "{{ openshift_master_config_dir ~ '/openshift-router.key' }}"
         cafile: "{{ openshift_master_config_dir ~ '/ca.crt' }}"
-
-  # End Block
-  when: ( openshift_hosted_router_create_certificate | bool ) and openshift_hosted_router_certificate == {}
+  when:
+  - openshift_hosted_router_create_certificate | bool
+  - openshift_hosted_router_certificate == {}
 
 - name: Create the router service account(s)
   oc_serviceaccount:

roles/openshift_master_facts/defaults/main.yml

@@ -1,5 +1,5 @@
 ---
-openshift_master_default_subdomain: "{{ lookup('oo_option', 'openshift_master_default_subdomain') | default(None, true) }}"
+openshift_master_default_subdomain: "router.default.svc.cluster.local"
 openshift_master_admission_plugin_config:
   openshift.io/ImagePolicy:
     configuration:

roles/openshift_master_facts/lookup_plugins/oo_option.py

@@ -1 +0,0 @@
-../../../lookup_plugins/oo_option.py

roles/openshift_master_facts/tasks/main.yml

@@ -1,5 +1,4 @@
 ---
-
 # Ensure the default sub-domain is set:
 - name: Migrate legacy osm_default_subdomain fact
   set_fact:

roles/openshift_node_facts/tasks/main.yml

@@ -1,10 +1,4 @@
 ---
-- set_fact:
-    openshift_node_debug_level: "{{ lookup('oo_option', 'openshift_node_debug_level') }}"
-  when:
-  - openshift_node_debug_level is not defined
-  - lookup('oo_option', 'openshift_node_debug_level') != ""
-
 - name: Set node facts
   openshift_facts:
     role: "{{ item.role }}"
@@ -20,7 +14,7 @@
       debug_level: "{{ openshift_node_debug_level | default(openshift.common.debug_level) }}"
       iptables_sync_period: "{{ openshift_node_iptables_sync_period | default(None) }}"
       kubelet_args: "{{ openshift_node_kubelet_args | default(None) }}"
-      labels: "{{ lookup('oo_option', 'openshift_node_labels') | default( openshift_node_labels | default(none), true) }}"
+      labels: "{{ openshift_node_labels | default(None) }}"
       registry_url: "{{ oreg_url_node | default(oreg_url) | default(None) }}"
       schedulable: "{{ openshift_schedulable | default(openshift_scheduleable) | default(None) }}"
       sdn_mtu: "{{ openshift_node_sdn_mtu | default(None) }}"

roles/rhel_subscribe/tasks/enterprise.yml

@@ -7,7 +7,7 @@
   when: deployment_type == 'openshift-enterprise'
 
 - set_fact:
-    ose_version: "{{ lookup('oo_option', 'ose_version') | default(default_ose_version, True) }}"
+    ose_version: "{{ lookup('env', 'ose_version') | default(default_ose_version, True) }}"
 
 - fail:
     msg: "{{ ose_version }} is not a valid version for {{ deployment_type }} deployment type"

roles/rhel_subscribe/tasks/main.yml

@@ -4,10 +4,10 @@
 #       to make it able to enable repositories
 
 - set_fact:
-    rhel_subscription_pool: "{{ lookup('oo_option', 'rhel_subscription_pool') | default(rhsub_pool, True) | default('Red Hat OpenShift Container Platform, Premium*', True) }}"
-    rhel_subscription_user: "{{ lookup('oo_option', 'rhel_subscription_user') | default(rhsub_user, True) | default(omit, True) }}"
-    rhel_subscription_pass: "{{ lookup('oo_option', 'rhel_subscription_pass') | default(rhsub_pass, True) | default(omit, True) }}"
-    rhel_subscription_server: "{{ lookup('oo_option', 'rhel_subscription_server') | default(rhsub_server) }}"
+    rhel_subscription_pool: "{{ lookup('env', 'rhel_subscription_pool') | default(rhsub_pool | default('Red Hat OpenShift Container Platform, Premium*')) }}"
+    rhel_subscription_user: "{{ lookup('env', 'rhel_subscription_user') | default(rhsub_user | default(omit, True)) }}"
+    rhel_subscription_pass: "{{ lookup('env', 'rhel_subscription_pass') | default(rhsub_pass | default(omit, True)) }}"
+    rhel_subscription_server: "{{ lookup('env', 'rhel_subscription_server') | default(rhsub_server | default(omit, True)) }}"
 
 - fail:
     msg: "This role is only supported for Red Hat hosts"