Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

add the openshift-provision template processor#147

Merged
seanmalloy merged 2 commits intomasterfrom
issue-108
Dec 3, 2019
Merged

add the openshift-provision template processor#147
seanmalloy merged 2 commits intomasterfrom
issue-108

Conversation

@vinny-sabatini
Copy link
Copy Markdown
Member

@vinny-sabatini vinny-sabatini commented Nov 9, 2019

Description

Introduce a new template processor based on the openshift-provision Ansible role.
Ref: https://github.com/gnuthought/ansible-role-openshift-provision

Remaining items for PR:

- [ ] Add tests for the template processor This will be done in #186 when #185 is complete.

  • Add an example for the template processor

Fixes #108

Type of change

  • New feature (non-breaking change which adds functionality)

@vinny-sabatini
Copy link
Copy Markdown
Member Author

I wanted to get the code out there for initial review, I will be brushing up the documentation/example for this, as well as add tests.

I was able to manually test the template processor so this code is in a working state

@vinny-sabatini vinny-sabatini added the wip Work In Progress label Nov 9, 2019
@vinny-sabatini vinny-sabatini force-pushed the issue-108 branch 2 times, most recently from 3a21979 to 1420e23 Compare November 11, 2019 21:44
Comment thread examples/openshift-provision/README.md Outdated
Comment thread examples/README.md Outdated
Comment thread examples/openshift-provision/README.md
Comment thread examples/openshift-provision/README.md Outdated
Comment thread examples/openshift-provision/README.md Outdated
Comment thread template-processors/openshift-provision/files/processParameters.yml Outdated
Comment thread template-processors/openshift-provision/files/processParameters.yml Outdated
Comment thread template-processors/openshift-provision/files/processTemplates.yml Outdated
Comment thread template-processors/openshift-provision/files/processTemplates.yml Outdated
Comment thread template-processors/openshift-provision/files/processTemplates.yml
- name: Define dynamic hierarchy_paths
set_fact:
hierarchy_paths: "{{ hierarchy_paths }} + [ '{{ openshift_cluster_config }}/{{ item.key }}/{{ item.value }}' ]"
with_dict: "{{ hierarchy }}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to have tests verifying that the overriding here works as intended

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will be adding the tests for this and an example for this very soon. I wanted to get the initial code out there so people could start looking in advanced.

Comment thread template-processors/openshift-provision/files/filter_plugins/cpu_cores_filter.py Outdated
@vinny-sabatini
Copy link
Copy Markdown
Member Author

I have cleaned up some of the changes requested, I will take care of more of them tomorrow when I have more time to test!

Comment thread examples/README.md Outdated
Comment thread examples/openshift-provision/README.md Outdated
@vinny-sabatini
Copy link
Copy Markdown
Member Author

@akavel I think I've addressed all of your concerns, please let me know if there's anything else that's unclear/could use improvement

Comment thread template-processors/openshift-provision/Dockerfile.in Outdated
Comment thread template-processors/openshift-provision/files/processParameters.yml
Copy link
Copy Markdown

@akavel akavel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not good enough with Kubernetes, so just doing a cursory overview of the example. To that extent, generally looks good to me!

Comment thread examples/openshift-provision/cluster/my_cluster_name/base.yml Outdated
Comment thread examples/openshift-provision/cluster/my_cluster_name/vars/main.yml Outdated
akavel
akavel previously approved these changes Dec 3, 2019
Copy link
Copy Markdown

@akavel akavel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand from our talk on chat that it's actually hard to do tests with openshift, so personally I think I'm ok with this being merged without tests. I guess we could open a separate issue for adding some minishift-based tests? if one's not open yet?

@vinny-sabatini
Copy link
Copy Markdown
Member Author

@akavel I chatted with @seanmalloy today and he agreed. He will review this when he has time.
I have created two issues addressing the lack of tests on this PR (#185) and the inability to test OpenShift specific template processors (#186).

Thanks for all of your feedback on this PR!!

Vinny Sabatini added 2 commits December 3, 2019 14:27
This template processor utilizes the openshift-provision ansible role
Ref: https://github.com/gnuthought/ansible-role-openshift-provision

openshift-provision role was pinned to a commit to ensure upstream
changes do not break the template processor.

Closes #108
@seanmalloy seanmalloy merged commit 0b897c4 into master Dec 3, 2019
@seanmalloy seanmalloy deleted the issue-108 branch December 3, 2019 21:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Create cluster-ansible template processor image

3 participants