Skip to content

Commit

Permalink
images: Register the meteringconfig_default_values as a fact.
Browse files Browse the repository at this point in the history
In 2.9.6, changes were made to how the template caching mechanism works. In previous versions, all variables (i.e. jinja2 expressions) were being cached, instead of the intended behavior which is a single variable gets cached. This was obviously problematic, especially in the case where you're trying to generate a password a couple of times, which meant the first password result gets cached, and any subsequent calls to generating passwords would result in the value of the first, cached password being assigned to the corresponding variable.

In the case of Metering, our Ansible role heavily relies on the notion of lazy evaluation - Ansible evaluates any variables at the last possible second. When this change to 2.9.6 was made, Metering saw a significant performance degradation, going from an average of 3-7 minutes to finish role execution, to 45+ minutes.

This was because we were relying on a buggy implementation of the templating caching mechanism. In our current implementation, we pass a variable to the name field of the module. This meant that this module needed to make a filesystem lookup call many, many times for a particular value stored in this meteringconfig_spec variable, which is a large value dictionary and essentially serves as the single source of truth that Metering references.

A more concrete example:
```yaml
ASK [meteringconfig : Check for the existence of the Presto TLS secret] *******
task path: /opt/ansible/roles/meteringconfig/tasks/configure_presto_tls.yml:15

Wednesday 29 April 2020  21:43:56 +0000 (0:00:00.380)       0:00:38.695 *******
File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file

File lookup using /opt/ansible/charts/openshift-metering/values.yaml as file
...
```

That variable in turn also relies on the value of the `meteringconfig_default_values`, which is a dictionary containing the default helm chart values. Previously, the result of that expression was cached. Now, due to how lazy evaluation works, the `meteringconfig_default_values` needed to be re-evaluated every time it's used, causing the aforementioned performance issues. The workaround is to cache or finalize, for a lack of a better phrase, the resultant of this `meteringconfig_default_values` expression. This would help performance as we're no longer re-evaluating this variable every time we want to template something, and all changes made go through the meteringconfig_spec dictionary so there's no risk of creating this variable early in the role.

Also included in the changeset is the switch to the  [`k8s_info` module](https://docs.ansible.com/ansible/latest/modules/k8s_info_module.html), as the `k8s_facts` module was depreciated in 2.9, and the `k8s_info` has the same usage.
  • Loading branch information
timflannagan committed May 1, 2020
1 parent 5a552f8 commit f578925
Showing 1 changed file with 4 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---

- name: Finalize the set of meteringconfig default values
set_fact:
meteringconfig_default_values: "{{ meteringconfig_default_values }}"

- name: Validate Configurations
include_tasks: validate.yml

Expand Down

0 comments on commit f578925

Please sign in to comment.