Clean up lint and other little things (polish++)

* Fix code lint
* Prefix role variables with the role name per best practices
* Add words to README

4 files changed, 149 insertions, 111 deletions

diff --git a/roles/openshift_certificate_expiry/README.md b/roles/openshift_certificate_expiry/README.md
index 9b543a335..f296c905e 100644
--- a/roles/openshift_certificate_expiry/README.md
+++ b/roles/openshift_certificate_expiry/README.md
@@ -11,12 +11,14 @@ include:
 * Master/Node/Router/Registry/Admin `kubeconfig`s
 * Etcd certificates
 
+This role pairs well with the redeploy certificates playbook:
 
+* [Redeploying Certificates Documentation](https://docs.openshift.com/container-platform/latest/install_config/redeploying_certificates.html)
 
-Requirements
-------------
+Just like the redeploying certificates playbook, this role is intended
+to be used with an inventory that is representative of the
+cluster. For best results run `ansible-playbook` with the `-v` option.
 
-* None
 
 
 Role Variables
@@ -24,26 +26,21 @@ Role Variables
 
 Core variables in this role:
 
-| Name                     | Default value                  | Description                                                           |
-|--------------------------|--------------------------------|-----------------------------------------------------------------------|
-| `config_base`            | `/etc/origin`                  | Base openshift config directory                                       |
-| `warning_days`           | `30`                           | Flag certificates which will expire in this many days from now        |
-| `show_all`               | `no`                           | Include healthy (non-expired and non-warning) certificates in results |
+| Name                                                  | Default value                  | Description                                                           |
+|-------------------------------------------------------|--------------------------------|-----------------------------------------------------------------------|
+| `openshift_certificate_expiry_config_base`            | `/etc/origin`                  | Base openshift config directory                                       |
+| `openshift_certificate_expiry_warning_days`           | `30`                           | Flag certificates which will expire in this many days from now        |
+| `openshift_certificate_expiry_show_all`               | `no`                           | Include healthy (non-expired and non-warning) certificates in results |
 
 Optional report/result saving variables in this role:
 
-| Name                     | Default value                  | Description                                                           |
-|--------------------------|--------------------------------|-----------------------------------------------------------------------|
-| `generate_html_report`   | `no`                           | Generate an HTML report of the expiry check results                   |
-| `html_report_path`       | `/tmp/cert-expiry-report.html` | The full path to save the HTML report as                              |
-| `save_json_results`      | `no`                           | Save expiry check results as a json file                              |
-| `json_results_path`      | `/tmp/cert-expiry-report.json` | The full path to save the json report as                              |
-
-
-Dependencies
-------------
+| Name                                                  | Default value                  | Description                                                           |
+|-------------------------------------------------------|--------------------------------|-----------------------------------------------------------------------|
+| `openshift_certificate_expiry_generate_html_report`   | `no`                           | Generate an HTML report of the expiry check results                   |
+| `openshift_certificate_expiry_html_report_path`       | `/tmp/cert-expiry-report.html` | The full path to save the HTML report as                              |
+| `openshift_certificate_expiry_save_json_results`      | `no`                           | Save expiry check results as a json file                              |
+| `openshift_certificate_expiry_json_results_path`      | `/tmp/cert-expiry-report.json` | The full path to save the json report as                              |
 
-* None
 
 Example Playbook
 ----------------
@@ -69,14 +66,14 @@ Generate HTML and JSON artifacts in their default paths:
   become: yes
   gather_facts: no
   vars:
-    generate_html_report: yes
-    save_json_results: yes
+    openshift_certificate_expiry_generate_html_report: yes
+    openshift_certificate_expiry_save_json_results: yes
   roles:
     - role: openshift_certificate_expiry
 ```
 
 Change the expiration warning window to 1500 days (good for testing
-the module out)
+the module out):
 
 ```yaml
 ---
@@ -85,90 +82,134 @@ the module out)
   become: yes
   gather_facts: no
   vars:
-    warning_days: 1500
+    openshift_certificate_expiry_warning_days: 1500
   roles:
     - role: openshift_certificate_expiry
 ```
 
+Change the expiration warning window to 1500 days (good for testing
+the module out) and save the results as a JSON file:
 
-Example JSON Output
--------------------
+```yaml
+---
+- name: Check cert expirys
+  hosts: all
+  become: yes
+  gather_facts: no
+  vars:
+    openshift_certificate_expiry_warning_days: 1500
+    openshift_certificate_expiry_save_json_results: yes
+  roles:
+    - role: openshift_certificate_expiry
+```
 
-Example is abbreviated to save space:
+
+JSON Output
+-----------
+
+There are two top-level keys in the saved JSON results, `data` and
+`summary`.
+
+The `data` key is a hash where the keys are the names of each host
+examined and the values are the check results for each respective
+host.
+
+The `summary` key is a hash that summarizes the number of certificates
+expiring within the configured warning window and the number of
+already expired certificates.
+
+The example below is abbreviated to save space:
 
 ```json
 {
-    "192.168.124.148": {
-        "etcd": [
-            {
-                "cert_cn": "CN:etcd-signer@1474563722",
-                "days_remaining": 350,
-                "expiry": "2017-09-22 17:02:25",
-                "health": "warning",
-                "path": "/etc/etcd/ca.crt"
+    "data": {
+        "192.168.124.148": {
+            "etcd": [
+                {
+                    "cert_cn": "CN:etcd-signer@1474563722",
+                    "days_remaining": 350,
+                    "expiry": "2017-09-22 17:02:25",
+                    "health": "warning",
+                    "path": "/etc/etcd/ca.crt"
+                },
+            ],
+            "kubeconfigs": [
+                {
+                    "cert_cn": "O:system:nodes, CN:system:node:m01.example.com",
+                    "days_remaining": 715,
+                    "expiry": "2018-09-22 17:08:57",
+                    "health": "warning",
+                    "path": "/etc/origin/node/system:node:m01.example.com.kubeconfig"
+                },
+                {
+                    "cert_cn": "O:system:cluster-admins, CN:system:admin",
+                    "days_remaining": 715,
+                    "expiry": "2018-09-22 17:04:40",
+                    "health": "warning",
+                    "path": "/etc/origin/master/admin.kubeconfig"
+                }
+            ],
+            "meta": {
+                "checked_at_time": "2016-10-07 15:26:47.608192",
+                "show_all": "True",
+                "warn_before_date": "2020-11-15 15:26:47.608192",
+                "warning_days": 1500
             },
-        ],
-        "kubeconfigs": [
-            {
-                "cert_cn": "O:system:nodes, CN:system:node:m01.example.com",
-                "days_remaining": 715,
-                "expiry": "2018-09-22 17:08:57",
-                "health": "warning",
-                "path": "/etc/origin/node/system:node:m01.example.com.kubeconfig"
-            },
-            {
-                "cert_cn": "O:system:cluster-admins, CN:system:admin",
-                "days_remaining": 715,
-                "expiry": "2018-09-22 17:04:40",
-                "health": "warning",
-                "path": "/etc/origin/master/admin.kubeconfig"
-            }
-        ],
-        "meta": {
-            "checked_at_time": "2016-10-07 15:26:47.608192",
-            "show_all": "True",
-            "warn_after_date": "2020-11-15 15:26:47.608192",
-            "warning_days": 1500
-        },
-        "ocp_certs": [
-            {
-                "cert_cn": "CN:172.30.0.1, DNS:kubernetes, DNS:kubernetes.default, DNS:kubernetes.default.svc, DNS:kubernetes.default.svc.cluster.local, DNS:m01.example.com, DNS:openshift, DNS:openshift.default, DNS:openshift.default.svc, DNS:openshift.default.svc.cluster.local, DNS:172.30.0.1, DNS:192.168.124.148, IP Address:172.30.0.1, IP Address:192.168.124.148",
-                "days_remaining": 715,
-                "expiry": "2018-09-22 17:04:39",
-                "health": "warning",
-                "path": "/etc/origin/master/master.server.crt"
-            },
-            {
-                "cert_cn": "CN:openshift-signer@1474563878",
-                "days_remaining": 1810,
-                "expiry": "2021-09-21 17:04:38",
-                "health": "ok",
-                "path": "/etc/origin/node/ca.crt"
-            }
-        ],
-        "registry": [
-            {
-                "cert_cn": "CN:172.30.101.81, DNS:docker-registry-default.router.default.svc.cluster.local, DNS:docker-registry.default.svc.cluster.local, DNS:172.30.101.81, IP Address:172.30.101.81",
-                "days_remaining": 728,
-                "expiry": "2018-10-05 18:54:29",
-                "health": "warning",
-                "path": "/api/v1/namespaces/default/secrets/registry-certificates"
-            }
-        ],
-        "router": [
-            {
-                "cert_cn": "CN:router.default.svc, DNS:router.default.svc, DNS:router.default.svc.cluster.local",
-                "days_remaining": 715,
-                "expiry": "2018-09-22 17:48:23",
-                "health": "warning",
-                "path": "/api/v1/namespaces/default/secrets/router-certs"
-            }
-        ]
+            "ocp_certs": [
+                {
+                    "cert_cn": "CN:172.30.0.1, DNS:kubernetes, DNS:kubernetes.default, DNS:kubernetes.default.svc, DNS:kubernetes.default.svc.cluster.local, DNS:m01.example.com, DNS:openshift, DNS:openshift.default, DNS:openshift.default.svc, DNS:openshift.default.svc.cluster.local, DNS:172.30.0.1, DNS:192.168.124.148, IP Address:172.30.0.1, IP Address:192.168.124.148",
+                    "days_remaining": 715,
+                    "expiry": "2018-09-22 17:04:39",
+                    "health": "warning",
+                    "path": "/etc/origin/master/master.server.crt"
+                },
+                {
+                    "cert_cn": "CN:openshift-signer@1474563878",
+                    "days_remaining": 1810,
+                    "expiry": "2021-09-21 17:04:38",
+                    "health": "ok",
+                    "path": "/etc/origin/node/ca.crt"
+                }
+            ],
+            "registry": [
+                {
+                    "cert_cn": "CN:172.30.101.81, DNS:docker-registry-default.router.default.svc.cluster.local, DNS:docker-registry.default.svc.cluster.local, DNS:172.30.101.81, IP Address:172.30.101.81",
+                    "days_remaining": 728,
+                    "expiry": "2018-10-05 18:54:29",
+                    "health": "warning",
+                    "path": "/api/v1/namespaces/default/secrets/registry-certificates"
+                }
+            ],
+            "router": [
+                {
+                    "cert_cn": "CN:router.default.svc, DNS:router.default.svc, DNS:router.default.svc.cluster.local",
+                    "days_remaining": 715,
+                    "expiry": "2018-09-22 17:48:23",
+                    "health": "warning",
+                    "path": "/api/v1/namespaces/default/secrets/router-certs"
+                }
+            ]
+        }
+    },
+    "summary": {
+        "warning": 6,
+        "expired": 0
     }
 }
 ```
 
 
+Requirements
+------------
+
+* None
+
+
+Dependencies
+------------
+
+* None
+
 
 License
 -------
diff --git a/roles/openshift_certificate_expiry/defaults/main.yml b/roles/openshift_certificate_expiry/defaults/main.yml
index c05617a75..6d7b19298 100644
--- a/roles/openshift_certificate_expiry/defaults/main.yml
+++ b/roles/openshift_certificate_expiry/defaults/main.yml
@@ -1,8 +1,8 @@
 ---
-config_base: "/etc/origin"
-warning_days: 30
-show_all: no
-generate_html_report: no
-html_report_path: "/tmp/cert-expiry-report.html"
-save_json_results: no
-json_results_path: "/tmp/cert-expiry-report.json"
+openshift_certificate_expiry_config_base: "/etc/origin"
+openshift_certificate_expiry_warning_days: 30
+openshift_certificate_expiry_show_all: no
+openshift_certificate_expiry_generate_html_report: no
+openshift_certificate_expiry_html_report_path: "/tmp/cert-expiry-report.html"
+openshift_certificate_expiry_save_json_results: no
+openshift_certificate_expiry_json_results_path: "/tmp/cert-expiry-report.json"
diff --git a/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py b/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py
index 3b934d019..6e27a7ebd 100644
--- a/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py
+++ b/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py
@@ -97,7 +97,6 @@ Source: Alex Martelli - http://stackoverflow.com/a/2819788/6490583
 
 
 ######################################################################
-
 def filter_paths(path_list):
     """`path_list` - A list of file paths to check. Only files which exist
 will be returned
@@ -339,7 +338,7 @@ an OpenShift Container Platform cluster
     check_results['meta'] = {}
     check_results['meta']['warning_days'] = warning_days
     check_results['meta']['checked_at_time'] = str(now)
-    check_results['meta']['warn_after_date'] = str(now + expire_window)
+    check_results['meta']['warn_before_date'] = str(now + expire_window)
     check_results['meta']['show_all'] = str(module.params['show_all'])
     # All the analyzed certs accumulate here
     ocp_certs = []
@@ -551,8 +550,6 @@ an OpenShift Container Platform cluster
 
     ######################################################################
     # Now for registry
-    # registry_secrets = subprocess.call('oc get secret registry-certificates -o yaml'.split())
-    # out = subprocess.PIPE
     try:
         registry_secrets_raw = subprocess.Popen('oc get secret registry-certificates -o yaml'.split(),
                                                 stdout=subprocess.PIPE)
@@ -607,11 +604,11 @@ an OpenShift Container Platform cluster
         check_results['registry'] = registry_certs
         check_results['router'] = router_certs
 
-
     # Sort the final results to report in order of ascending safety
     # time. That is to say, the certificates which will expire sooner
     # will be at the front of the list and certificates which will
-    # expire later are at the end.
+    # expire later are at the end. Router and registry certs should be
+    # limited to just 1 result, so don't bother sorting those.
     check_results['ocp_certs'] = sorted(check_results['ocp_certs'], cmp=lambda x, y: cmp(x['days_remaining'], y['days_remaining']))
     check_results['kubeconfigs'] = sorted(check_results['kubeconfigs'], cmp=lambda x, y: cmp(x['days_remaining'], y['days_remaining']))
     check_results['etcd'] = sorted(check_results['etcd'], cmp=lambda x, y: cmp(x['days_remaining'], y['days_remaining']))
diff --git a/roles/openshift_certificate_expiry/tasks/main.yml b/roles/openshift_certificate_expiry/tasks/main.yml
index 88bc02efe..b78209055 100644
--- a/roles/openshift_certificate_expiry/tasks/main.yml
+++ b/roles/openshift_certificate_expiry/tasks/main.yml
@@ -1,9 +1,9 @@
 ---
 - name: Check cert expirys on host
   openshift_cert_expiry:
-    warning_days: "{{ warning_days|int }}"
-    config_base: "{{ config_base }}"
-    show_all: "{{ show_all|bool }}"
+    warning_days: "{{ openshift_certificate_expiry_warning_days|int }}"
+    config_base: "{{ openshift_certificate_expiry_config_base }}"
+    show_all: "{{ openshift_certificate_expiry_show_all|bool }}"
   register: check_results
 
 - name: Generate expiration report HTML
@@ -11,15 +11,15 @@
   run_once: yes
   template:
     src: cert-expiry-table.html.j2
-    dest: "{{ html_report_path }}"
+    dest: "{{ openshift_certificate_expiry_html_report_path }}"
   delegate_to: localhost
-  when: "{{ generate_html_report|bool }}"
+  when: "{{ openshift_certificate_expiry_generate_html_report|bool }}"
 
 - name: Generate expiration results JSON
   become: no
   run_once: yes
   template:
     src: save_json_results.j2
-    dest: "{{ json_results_path }}"
+    dest: "{{ openshift_certificate_expiry_json_results_path }}"
   delegate_to: localhost
-  when: "{{ save_json_results|bool }}"
+  when: "{{ openshift_certificate_expiry_save_json_results|bool }}"