redhat-cop / openshift-applier

Used to apply OpenShift objects to an OpenShift Cluster
Apache License 2.0
102 stars 61 forks source link

Add support for Jinja templates, closes #119 #120

Closed pabrahamsson closed 4 years ago

pabrahamsson commented 5 years ago

What does this PR do?

Adds support for using Jinja templates

How should this be tested?

ansible-playbook playbooks/openshift-cluster-seed.yml -i tests/inventories/jinja-templates

This should create 6 new projects

$ oc get project
NAME                                  DISPLAY NAME                                                               STATUS
...
oa-ci-jinja-templates-file-dev        OpenShift Applier Jinja Templates Test 1 (displayName) (file) - dev        Active
oa-ci-jinja-templates-file-prod       OpenShift Applier Jinja Templates Test 1 (displayName) (file) - prod       Active
oa-ci-jinja-templates-file-test       OpenShift Applier Jinja Templates Test 1 (displayName) (file) - test       Active
oa-ci-jinja-templates-template-dev    OpenShift Applier Jinja Templates Test 1 (displayName) (template )- dev    Active
oa-ci-jinja-templates-template-prod   OpenShift Applier Jinja Templates Test 1 (displayName) (template )- prod   Active
oa-ci-jinja-templates-template-test   OpenShift Applier Jinja Templates Test 1 (displayName) (template )- test   Active
...

Is there a relevant Issue open for this?

119

Who would you like to review this?

cc: @redhat-cop/openshift-applier

malacourse commented 5 years ago

Any thoughts on just using a .j2 extension rather than setting the jinja variable?

pabrahamsson commented 5 years ago

This sounds fine to me @malacourse but I'd like to hear from @oybed, @etsauer and @makentenza before proceedeing. I know they had some thoughts around this when jinja2 support was first suggested.

oybed commented 5 years ago

@pabrahamsson thank you for this - looks like a good start. I do like the suggestion @malacourse had above about using the .j2 extension to determine if a "pre-processing" is needed. However, I'd like to take it even one step further. The Jinja processing is pretty much the same regardless of file or template, so just make a process_jinja file that handles this - and it only does so if the file or template ends in .j2. In that way, a sample inventory could be:

openshift_cluster_content:
- object: mycontent
  content:
  - template: "{{ inventory_dir }}/../../files/templates/projectrequest.yml"
    params: "..."
    action: create
- object: my-jina-content
  content:
  - template: "{{ inventory_dir }}/../jinja-template.j2"
    params: "..."
  - file: "{{ inventory_dir }}/../jinja-file.j2"

In the inventory above, the file and template steps are handled as-is today, but because the filename ends in .j2 there's a pre-processing step for Jinja.

This process_jinja step could be injected around line 38 in the process-one-entry.yml file. https://github.com/redhat-cop/openshift-applier/blob/master/roles/openshift-applier/tasks/process-one-entry.yml#L38

Please let me know if anything is unclear.

oybed commented 5 years ago

@pabrahamsson looks good. However, as the CI indicates, it's failing due to not finding the file correctly. Basically, the logic to determine if this is a URL, local file or existing openshift template isn't aware of this new .j2 processing (and hence the "generated file"). This logic needs a bit of refactoring anyway, so I can assist with this when I can find some time.

oybed commented 4 years ago

@pabrahamsson I finally had a chance to look into the issue and review your PR. So the CI job is correct that this doesn't work as-is for remote hosts. While I still believe we should look to refactor the file location detection, the issue in your PR can be worked around by adding delegate_to: localhost in the process-jinja.yml file. Since you cannot apply delegate_to to the actual include_tasks, you'll have to just create a block in the file and apply it within the file. This will take care of the CI issue. I.e.:


- block:
  - name: Set file or template fact
   :
   : 
  - name: Update path
    set_fact: {"{{ process_file_or_template }}":"{{ dest_path }}"}

  # applies to the entire file
  delegate_to: localhost 
etsauer commented 4 years ago

We have a winner!!!