Merge pull request #2902 from dgoodwin/fix-metrics-url

Fix metricsPublicURL only being set correctly on first master.

9 files changed, 38 insertions, 38 deletions

diff --git a/playbooks/common/openshift-cluster/openshift_hosted.yml b/playbooks/common/openshift-cluster/openshift_hosted.yml
index ccbba54b4..cd2f2e6aa 100644
--- a/playbooks/common/openshift-cluster/openshift_hosted.yml
+++ b/playbooks/common/openshift-cluster/openshift_hosted.yml
@@ -20,8 +20,8 @@
       openshift_hosted_registry_registryurl: "{{ hostvars[groups.oo_first_master.0].openshift.master.registry_url }}"
     when: "'master' in hostvars[groups.oo_first_master.0].openshift and 'registry_url' in hostvars[groups.oo_first_master.0].openshift.master"
   - set_fact:
-      logging_hostname: "{{ openshift_hosted_logging_hostname | default('kibana.' ~ (openshift.master.default_subdomain | default('router.default.svc.cluster.local', true))) }}"
-      logging_ops_hostname: "{{ openshift_hosted_logging_ops_hostname | default('kibana-ops.' ~ (openshift.master.default_subdomain | default('router.default.svc.cluster.local', true))) }}"
+      logging_hostname: "{{ openshift_hosted_logging_hostname | default('kibana.' ~ (openshift_master_default_subdomain | default('router.default.svc.cluster.local', true))) }}"
+      logging_ops_hostname: "{{ openshift_hosted_logging_ops_hostname | default('kibana-ops.' ~ (openshift_master_default_subdomain | default('router.default.svc.cluster.local', true))) }}"
       logging_master_public_url: "{{ openshift_hosted_logging_master_public_url | default(openshift.master.public_api_url) }}"
       logging_elasticsearch_cluster_size: "{{ openshift_hosted_logging_elasticsearch_cluster_size | default(1) }}"
       logging_elasticsearch_ops_cluster_size: "{{ openshift_hosted_logging_elasticsearch_ops_cluster_size | default(1) }}"
diff --git a/roles/openshift_hosted/tasks/registry/secure.yml b/roles/openshift_hosted/tasks/registry/secure.yml
index d2f6ba5f6..b6d007835 100644
--- a/roles/openshift_hosted/tasks/registry/secure.yml
+++ b/roles/openshift_hosted/tasks/registry/secure.yml
@@ -29,7 +29,7 @@
   changed_when: false
 
 - set_fact:
-    docker_registry_route_hostname: "{{ 'docker-registry-default.' ~ (openshift.master.default_subdomain | default('router.default.svc.cluster.local', true)) }}"
+    docker_registry_route_hostname: "{{ 'docker-registry-default.' ~ (openshift_master_default_subdomain | default('router.default.svc.cluster.local', true)) }}"
 
 - name: Create registry certificates if they do not exist
   command: >
diff --git a/roles/openshift_master/templates/master.yaml.v1.j2 b/roles/openshift_master/templates/master.yaml.v1.j2
index dc9226a5a..81546c829 100644
--- a/roles/openshift_master/templates/master.yaml.v1.j2
+++ b/roles/openshift_master/templates/master.yaml.v1.j2
@@ -15,8 +15,8 @@ assetConfig:
 {% if 'logging_public_url' in openshift.master %}
   loggingPublicURL: {{ openshift.master.logging_public_url }}
 {% endif %}
-{% if 'metrics_public_url' in openshift.master %}
-  metricsPublicURL: {{ openshift.master.metrics_public_url }}
+{% if openshift_hosted_metrics_deploy | default(false) | bool %}
+  metricsPublicURL: {{ openshift_hosted_metrics_public_url }}
 {% endif %}
 {% if 'extension_scripts' in openshift.master %}
   extensionScripts: {{ openshift.master.extension_scripts | to_padded_yaml(1, 2) }}
@@ -202,7 +202,7 @@ projectConfig:
     mcsLabelsPerProject: {{ openshift.master.mcs_labels_per_project }}
     uidAllocatorRange: "{{ openshift.master.uid_allocator_range  }}"
 routingConfig:
-  subdomain:  "{{ openshift.master.default_subdomain | default("") }}"
+  subdomain:  "{{ openshift_master_default_subdomain | default("") }}"
 serviceAccountConfig:
   limitSecretReferences: false
   managedNames:
diff --git a/roles/openshift_master_facts/tasks/main.yml b/roles/openshift_master_facts/tasks/main.yml
index e1cd94d75..f5923ecf8 100644
--- a/roles/openshift_master_facts/tasks/main.yml
+++ b/roles/openshift_master_facts/tasks/main.yml
@@ -1,4 +1,32 @@
 ---
+
+# Ensure the default sub-domain is set:
+- name: Migrate legacy osm_default_subdomain fact
+  set_fact:
+    openshift_master_default_subdomain: "{{ osm_default_subdomain | default(None) }}"
+  when: openshift_master_default_subdomain is not defined
+
+- fail:
+    msg: openshift_master_default_subdomain must be set to deploy metrics
+  when: openshift_hosted_metrics_deploy | default(false) | bool and openshift_master_default_subdomain | default("") == ""
+
+# NOTE: These metrics variables are unfortunately needed by both the master and the metrics roles
+# to properly configure the master-config.yaml file.
+#
+# NOTE: Today only changing the hostname for the metrics public URL is supported, the
+# path must stay consistent. As such if openshift_hosted_metrics_public_url is set in
+# inventory, we extract the hostname, and then reset openshift_hosted_metrics_public_url
+# to the format that we know is valid. (This may change in future)
+- set_fact:
+    g_metrics_hostname: "{{ openshift_hosted_metrics_public_url
+                        | default('hawkular-metrics.' ~ (openshift_master_default_subdomain))
+                        | oo_hostname_from_url }}"
+  when: openshift_hosted_metrics_deploy | default(false) | bool
+
+- set_fact:
+    openshift_hosted_metrics_public_url: "https://{{ g_metrics_hostname }}/hawkular/metrics"
+  when: openshift_hosted_metrics_deploy | default(false) | bool
+
 - name: Set master facts
   openshift_facts:
     role: master
@@ -49,7 +77,6 @@
       oauth_grant_method: "{{ openshift_master_oauth_grant_method | default(None) }}"
       sdn_cluster_network_cidr: "{{ osm_cluster_network_cidr | default(None) }}"
       sdn_host_subnet_length: "{{ osm_host_subnet_length | default(None) }}"
-      default_subdomain: "{{ openshift_master_default_subdomain | default(osm_default_subdomain | default(None), true) }}"
       custom_cors_origins: "{{ osm_custom_cors_origins | default(None) }}"
       default_node_selector: "{{ osm_default_node_selector | default(None) }}"
       project_request_message: "{{ osm_project_request_message | default(None) }}"
@@ -75,7 +102,6 @@
       api_env_vars: "{{ openshift_master_api_env_vars | default(None) }}"
       controllers_env_vars: "{{ openshift_master_controllers_env_vars | default(None) }}"
       audit_config: "{{ openshift_master_audit_config | default(None) }}"
-      metrics_public_url: "{% if openshift_hosted_metrics_deploy | default(false) %}https://{{ metrics_hostname }}/hawkular/metrics{% endif %}"
       scheduler_args: "{{ openshift_master_scheduler_args | default(None) }}"
 
 - name: Determine if scheduler config present
diff --git a/roles/openshift_master_facts/vars/main.yml b/roles/openshift_master_facts/vars/main.yml
index 3f328dcfe..a5ad580e7 100644
--- a/roles/openshift_master_facts/vars/main.yml
+++ b/roles/openshift_master_facts/vars/main.yml
@@ -24,7 +24,3 @@ builddefaults_yaml:
       - name: no_proxy
         value: "{{ openshift.master.builddefaults_no_proxy | default(omit, true) | join(',') }}"
 
-metrics_hostname: "{{ openshift_hosted_metrics_public_url
-                    | default('hawkular-metrics.' ~ (openshift.master.default_subdomain
-                    | default(openshift_master_default_subdomain )))
-                    | oo_hostname_from_url }}"
diff --git a/roles/openshift_metrics/README.md b/roles/openshift_metrics/README.md
index f3c0f3474..015a673a8 100644
--- a/roles/openshift_metrics/README.md
+++ b/roles/openshift_metrics/README.md
@@ -27,17 +27,11 @@ From this role:
 | openshift_hosted_metrics_resolution             | `10s`                 | Metrics resolution                                          |
 
 
-From openshift_common:
-
-| Name                                  | Default Value  |                                        |
-|---------------------------------------|----------------|----------------------------------------|
-| openshift_master_default_subdomain    | null           | Subdomain FQDN (Mandatory)             |
-
-
 Dependencies
 ------------
 openshift_facts
 openshift_examples
+openshift_master_facts
 
 Example Playbook
 ----------------
diff --git a/roles/openshift_metrics/meta/main.yaml b/roles/openshift_metrics/meta/main.yaml
index a89467de5..debca3ca6 100644
--- a/roles/openshift_metrics/meta/main.yaml
+++ b/roles/openshift_metrics/meta/main.yaml
@@ -15,3 +15,4 @@ galaxy_info:
 dependencies:
 - { role: openshift_examples }
 - { role: openshift_facts }
+- { role: openshift_master_facts }
diff --git a/roles/openshift_metrics/tasks/install.yml b/roles/openshift_metrics/tasks/install.yml
index 98e21375a..0f520e685 100644
--- a/roles/openshift_metrics/tasks/install.yml
+++ b/roles/openshift_metrics/tasks/install.yml
@@ -89,7 +89,7 @@
   set_fact:
     deployer_cmd: "{{ openshift.common.client_binary }} process -f \
       {{ hosted_base }}/metrics-deployer.yaml -v \
-      HAWKULAR_METRICS_HOSTNAME={{ metrics_hostname }} \
+      HAWKULAR_METRICS_HOSTNAME={{ g_metrics_hostname }} \
       -v USE_PERSISTENT_STORAGE={{metrics_persistence | string | lower }} \
       -v DYNAMICALLY_PROVISION_STORAGE={{metrics_dynamic_vol | string | lower }} \
       -v METRIC_DURATION={{ openshift.hosted.metrics.duration }} \
@@ -128,12 +128,5 @@
   modify_yaml:
     dest: "{{ openshift.common.config_base }}/master/master-config.yaml"
     yaml_key: assetConfig.metricsPublicURL
-    yaml_value: "https://{{ metrics_hostname }}/hawkular/metrics"
+    yaml_value: "{{ openshift_hosted_metrics_public_url }}"
   notify: restart master
-
-- name: Store metrics public_url
-  openshift_facts:
-    role: master
-    local_facts:
-      metrics_public_url: "https://{{ metrics_hostname }}/hawkular/metrics"
-  when: deploy_result | changed
diff --git a/roles/openshift_metrics/tasks/main.yaml b/roles/openshift_metrics/tasks/main.yaml
index 26af279b1..be3256f02 100644
--- a/roles/openshift_metrics/tasks/main.yaml
+++ b/roles/openshift_metrics/tasks/main.yaml
@@ -1,8 +1,4 @@
 ---
-- fail:
-    msg: This role required openshift_master_default_subdomain or openshift_hosted_metrics_public_url be set
-  when: openshift.master.metrics_public_url | default(openshift_hosted_metrics_public_url | default(openshift.master.default_subdomain | default(openshift_master_default_subdomain | default(none)))) is none
-
 - name: Create temp directory for kubeconfig
   command: mktemp -d /tmp/openshift-ansible-XXXXXX
   register: mktemp
@@ -27,12 +23,6 @@
     - 'openshift.hosted.metrics.*'
 
 - set_fact:
-    # Prefer the master facts over bare variables if present, prefer
-    # metrics_public_url over creating a default using default_subdomain
-    metrics_hostname: "{{ openshift.hosted.metrics.public_url
-                          | default('hawkular-metrics.' ~ (openshift.master.default_subdomain
-                          | default(openshift_master_default_subdomain )))
-                          | oo_hostname_from_url }}"
     metrics_persistence: "{{ openshift.hosted.metrics.storage_kind | default(none) is not none }}"
     metrics_dynamic_vol: "{{ openshift.hosted.metrics.storage_kind | default(none) == 'dynamic' }}"
     metrics_template_dir: "{{ openshift.common.config_base if openshift.common.is_containerized | bool else '/usr/share/openshift' }}/examples/infrastructure-templates/{{ 'origin' if deployment_type == 'origin' else 'enterprise' }}"