New module tower_workflow_template. #37520

fleu42 · 2018-03-16T14:43:44Z

SUMMARY

There was no way to manage Tower workflow with Ansible so here is that module.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

tower_workflow_template

ANSIBLE VERSION

ansible 2.6.0 (feature/add-new-module-tower_workflow_template 4355612494) last updated 2018/03/16 15:15:44 (GMT +200)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/fleu42/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/fleu42/tmp/ansible/lib/ansible
  executable location = /home/fleu42/tmp/ansible/bin/ansible
  python version = 2.7.12 (default, Dec  4 2017, 14:50:18) [GCC 5.4.0 20160609]

ADDITIONAL INFORMATION

$ ansible-playbook -i "localhost," -c local ~/playbooks/ansible_tower.yml --tag workflow

PLAY [localhost] ***************************************************************************************************************************************************************************************************

TASK [include_vars] ************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [set_fact] ****************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Create workflow job template] ********************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP *********************************************************************************************************************************************************************************************************
localhost                  : ok=3    changed=1    unreachable=0    failed=0

ansibot · 2018-03-16T15:00:07Z

@AlanCoding @ghjm @jlaska @matburt @rooftopcellist @ryanpetrello @simfarm @wwitzel3

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

ansibot · 2018-03-16T15:00:12Z

cc @AlanCoding @ghjm @jlaska @matburt @rooftopcellist @ryanpetrello @simfarm @wwitzel3
click here for bot help

AlanCoding · 2018-03-16T15:19:32Z

lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py

+
+            if module.params.get('schema'):
+                wfjt_res.schema(
+                    wfjt_res.get(name=params['name'])['id'],


nit-picky optimization, but if the prior step was successful then 'id' should already be in result

Also, if the state is "absent", then you need to be sure that you don't hit this code. Either that, or it should throw an error if schema is provided and state is absent.

Ok, I've fixed that.

AlanCoding · 2018-03-16T15:19:49Z

lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py

+                )
+
+        except (exc.ConnectionError, exc.BadRequest) as excinfo:
+            module.fail_json(msg='Failed to update inventory source: \


failed to update workflow?

Sorry, I've badly copy/paste.

ryanpetrello · 2018-03-16T19:26:46Z

Thanks for contributing this, @fleu42. I've recently added support for integration tests for the various Tower modules:

#37421

Would you mind adding similar tests for this new module so we can prove it works (and to avoid regressions)? Thanks!

~~needs_reviision~~

fleu42 · 2018-03-19T11:03:20Z

@ryanpetrello, I've added the required integration test but they fail due to a licence error:

2018-03-19 09:48:17 TASK [tower_workflow_template : Create a workflow job template] ****************
2018-03-19 09:48:18 fatal: [testhost]: FAILED! => {"changed": false, "msg": "Failed to update workflow template:                     The Tower server claims it was sent a bad request.\n\nPOST https://ec2-54-172-116-177.compute-1.amazonaws.com/api/v2/workflow_job_templates/\nParams: None\nData: {\"name\": \"my-workflow\"}\n\nResponse: {\"detail\":\"Your license does not allow use of workflows.\"}"}

I don't see any way to overcome that inside the CI.

ryanpetrello · 2018-03-19T13:23:00Z

Ah, you're right 😢 due to licensing restrictions, this may be one we just can't test in our public CI (give me a bit to see what I can do here).

ryanpetrello · 2018-03-19T19:37:09Z

@fleu42 we're almost there; your test is a little bit unstable (it fails periodically) because it's not giving the Tower VM enough time to finish the workflow job:

Let's bump this to a 60 second delay similar to:

ansible/test/integration/targets/tower_job_wait/tasks/main.yml

Line 16 in 872a7b4

timeout: 60

...and see if we have better luck: https://github.com/ansible/ansible/pull/37520/files#diff-f8ce5023269e6a4d72463a3582374814R39

fleu42 · 2018-03-20T10:05:33Z

@ryanpetrello, I've changed the number of retries as you asked. That should make the testing more stable.

ryanpetrello · 2018-03-20T12:20:06Z

shipit

ryanpetrello · 2018-08-22T12:46:55Z

shipit

abadger · 2018-08-28T13:47:33Z

lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py

+        state=dict(choices=['present', 'absent'], default='present'),
+    )
+
+    module = TowerModule(argument_spec=argument_spec, supports_check_mode=True)


This should be supports_check_mode=False.

abadger · 2018-08-28T13:53:47Z

lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py

+                organization_res = tower_cli.get_resource('organization')
+                try:
+                    organization = organization_res.get(
+                        name=module.params.get('organization'))


is it okay that the organization can be None here?

Okay, so alancoding says that this could throw an error that it can't figure out which organization you meant if there is more than one organization.

Perhaps what we need here is:

if organization is not None: try: organization = organization_res.get([....]

Isn't that covered by if module.params.get('organization'):?

Oops, sorry, you are correct.

abadger · 2018-08-28T14:56:07Z

Okay, I noticed one thing that needs to be changed (check_mode set to False) and one set of related questions. The argument spec and documentation are saying that many of the arguments are not required but they have no explicit default set. This means that if the user does not specify those parameters they will default to None. The None value is then passed into the tower API. Is that what you want or should you either make those a required user parameter (organization may fall in this category?) or set a default (the booleans seem like they might need to be default=True or default=False?), or not include them in the parameters if they're set to None? I don't know the tower API so None may be fine for some or all of these but figured I should check.

Last question, does delete() take all the same parameters as modify() or will you end up with an error if the user uses state=absent but fills in the rest of the parameters with values?

fleu42 · 2018-08-29T10:03:01Z

Thank you for reviewing my module.

I agree with the supports_check_mode = False even though all the other modules but tower_settings.py in the ansible_tower section have it set to True. That might be something to address. I will correct my module.
I've set many of the arguments as not required because the AWX/Tower API requires only the name field to create or delete a workflow. All other values are optional (see: https://tower-cli.readthedocs.io/en/latest/api_ref/resources/workflow.html for more informations).
So, yes, you can create a workflow that doesn't belong to any organization. Also, a None value is fine for the Tower API. That means that that field is left blank.
delete() can take all the same parameters as modify(). The function will call a _lookup() function using these parameters to retrieve the workflow id and then delete it.
I've added a test to cover a simple delete() case.

Manage Tower workflows and their schemas.

Add a test to cover the deletion of a template.

abadger

Still what happens if a value is None? The value is left unchanged in tower?

abadger · 2018-08-29T13:14:14Z

lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py

+    with settings.runtime_values(**tower_auth):
+        tower_check_mode(module)
+        wfjt_res = tower_cli.get_resource('workflow')
+        try:


Try except blocks souls he as small as possible

Thanks for pointing that out. I've fixed it.

abadger · 2018-08-29T13:18:50Z

lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py

+                wfjt_res.schema(
+                    result['id'],
+                    module.params.get('schema')
+                )


Two things standout to me about this code: (1) merge it into the above if state == "present" block. (2) you should have a check after the ansible module is created that errors if schema is set and state==absent

I've relocated the schema association to the workflow template

I've added a condition for that

AlanCoding · 2018-08-29T13:34:04Z

Still what happens if a value is None? The value is left unchanged in tower?

For setting expectations, it should be possible to change a non-null value to a null value. For workflows, this is allowed by the API just fine too. Because we take values as strings, and have to abide by the click library practices, tower-cli introduced "null" to be a keyword

(cli) arominge-OSX:tower-cli alancoding$ tower-cli workflow create --name=bar --organization=Default
Resource changed.
== ==== ============ 
id name organization 
== ==== ============ 
30 bar            13
== ==== ============ 
(cli) arominge-OSX:tower-cli alancoding$ tower-cli workflow modify --name=bar --organization=null
Resource changed.
== ==== ============ 
id name organization 
== ==== ============ 
30 bar  None
== ==== ============ 
(cli) arominge-OSX:tower-cli alancoding$ tower-cli workflow get --name=bar
== ==== ============ 
id name organization 
== ==== ============ 
30 bar  None
== ==== ============

However, this keyword of "null" does not impact the modules, because it is implemented in tower-cli types, which these modules bypass. A null value should be interpreted as setting the organization to null, which is different from the parameter being absent. Granted, this isn't an important use-case, and there may be general Ansible module practices that I am unaware of.

abadger · 2018-08-29T13:38:50Z

Alright... then, at least for now, we probably need this module to see that the user did not specify a value for a parameter and not try to forward on None to the API and later, figure out how the user should specify that the value can be nulled by the user? As coded right now, this module will set the optional fields to None if the user didn't specify them.

AlanCoding · 2018-08-29T13:55:57Z

As coded right now, this module will set the optional fields to None if the user didn't specify them.

On creation, I think that's perfectly fine. If I have a task to create a workflow and no organization specified, I would be happy to see that field is null. It's modification where that becomes more challenging. My gut thinking is that if it's not provided it should not actively remove the organization from the workflow of that name, but I don't know if this is standard behavior.

Relocate the schema association to the workflow template.

abadger · 2018-08-29T14:26:40Z

Your gut matches up with the standard behaviour of modules: if an optional field is not specified then we usually do not modify it.

Errors if state is absent and schema is present.

AlanCoding · 2018-08-29T15:25:14Z

I pulled the code, everything looks good. Here is me doing some testing on managing the blank-ness or null-ness of the related organization.

https://gist.github.com/AlanCoding/999b415be68f7f419d95ded8c92404f1

The only thing which doesn't work as expected is that there's no real way to remove the organization, see foo4 example. I was talking with @abadger with this, but I'm still not sure what means of specifying null would be expected.

AlanCoding · 2018-08-29T15:44:11Z

If there was a very strong desire to address the set-null case, then we could carry through the practice of using "null" string as a keyword, as tower-cli did.

diff --git a/lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py b/lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py
index 3454f432d9..fddead2803 100644
--- a/lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py
+++ b/lib/ansible/modules/web_infrastructure/ansible_tower/tower_workflow_template.py
@@ -148,7 +148,9 @@ def main():
         if module.params.get('description'):
             params['description'] = module.params.get('description')
 
-        if module.params.get('organization'):
+        if module.params.get('organization') == "null":
+            params['organization'] = "null"
+        elif module.params.get('organization'):
             organization_res = tower_cli.get_resource('organization')
             try:
                 organization = organization_res.get(

Then the tasks

    - name: remove org from foo4
      tower_workflow_template:
        name: foo4
        organization: "null"

Would be able to remove it.

This is a very general problem, and I would rather delay addressing it, since I'm not thrilled about the API structure for this.

abadger · 2018-08-29T17:58:22Z

@AlanCoding Sounds good. Thanks for also pointing out that if module.params.get('organization'): (and similar) covers the problem of None sneaking into a parameter to tower_cli via defaults. I'll squash and merge this to devel for 2.7.0.

mkrizek removed the needs_triage Needs a first human triage before being processed. label Mar 16, 2018

AlanCoding reviewed Mar 16, 2018

View reviewed changes

fleu42 force-pushed the feature/add-new-module-tower_workflow_template branch 2 times, most recently from 97f7952 to 9022629 Compare March 16, 2018 15:32

This was referenced Mar 16, 2018

New module : tower_credential_type #37243

Merged

Module: Tower inventory source module #37110

Merged

fleu42 force-pushed the feature/add-new-module-tower_workflow_template branch from abc774b to 76892ec Compare March 19, 2018 09:44

ansibot removed the needs_info This issue requires further information. Please answer any outstanding questions. label Mar 19, 2018

fleu42 force-pushed the feature/add-new-module-tower_workflow_template branch 2 times, most recently from 9654608 to 1cb92df Compare March 20, 2018 09:45

abadger reviewed Aug 28, 2018

View reviewed changes

fleu42 force-pushed the feature/add-new-module-tower_workflow_template branch from 0a6017d to 3e7aef1 Compare August 29, 2018 12:10

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed shipit This PR is ready to be merged by Core labels Aug 29, 2018

fleu42 added 2 commits August 29, 2018 14:15

Add new module *tower_workflow_template*

0188d52

Manage Tower workflows and their schemas.

Set support_check_module to False.

1e32867

Add a test to cover the deletion of a template.

fleu42 force-pushed the feature/add-new-module-tower_workflow_template branch from 3e7aef1 to 1e32867 Compare August 29, 2018 12:19

abadger reviewed Aug 29, 2018

View reviewed changes

Condense try/except block

6eb7ff8

Relocate the schema association to the workflow template.

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 29, 2018

Add a condition on schema when state is absent

3f991c3

Errors if state is absent and schema is present.

fleu42 force-pushed the feature/add-new-module-tower_workflow_template branch from 175e888 to 3f991c3 Compare August 29, 2018 15:03

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 29, 2018

abadger merged commit aaa157f into ansible:devel Aug 29, 2018

dagwieers added the tower Tower community label Feb 8, 2019

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New module tower_workflow_template. #37520

New module tower_workflow_template. #37520

fleu42 commented Mar 16, 2018

ansibot commented Mar 16, 2018

ansibot commented Mar 16, 2018

AlanCoding Mar 16, 2018

fleu42 Mar 16, 2018

AlanCoding Mar 16, 2018

fleu42 Mar 16, 2018

ryanpetrello commented Mar 16, 2018 •

edited

fleu42 commented Mar 19, 2018

ryanpetrello commented Mar 19, 2018 •

edited

ryanpetrello commented Mar 19, 2018 •

edited

fleu42 commented Mar 20, 2018

ryanpetrello commented Mar 20, 2018

ryanpetrello commented Aug 22, 2018

abadger Aug 28, 2018

abadger Aug 28, 2018

abadger Aug 29, 2018 •

edited

AlanCoding Aug 29, 2018

abadger Aug 29, 2018

abadger commented Aug 28, 2018

fleu42 commented Aug 29, 2018

abadger left a comment

abadger Aug 29, 2018

fleu42 Aug 29, 2018

abadger Aug 29, 2018 •

edited

fleu42 Aug 29, 2018

AlanCoding commented Aug 29, 2018

abadger commented Aug 29, 2018

AlanCoding commented Aug 29, 2018

abadger commented Aug 29, 2018

AlanCoding commented Aug 29, 2018

AlanCoding commented Aug 29, 2018

abadger commented Aug 29, 2018

New module tower_workflow_template. #37520

New module tower_workflow_template. #37520

Conversation

fleu42 commented Mar 16, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Mar 16, 2018

ansibot commented Mar 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanpetrello commented Mar 16, 2018 • edited

fleu42 commented Mar 19, 2018

ryanpetrello commented Mar 19, 2018 • edited

ryanpetrello commented Mar 19, 2018 • edited

fleu42 commented Mar 20, 2018

ryanpetrello commented Mar 20, 2018

ryanpetrello commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abadger Aug 29, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abadger commented Aug 28, 2018

fleu42 commented Aug 29, 2018

abadger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abadger Aug 29, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlanCoding commented Aug 29, 2018

abadger commented Aug 29, 2018

AlanCoding commented Aug 29, 2018

abadger commented Aug 29, 2018

AlanCoding commented Aug 29, 2018

AlanCoding commented Aug 29, 2018

abadger commented Aug 29, 2018

ryanpetrello commented Mar 16, 2018 •

edited

ryanpetrello commented Mar 19, 2018 •

edited

ryanpetrello commented Mar 19, 2018 •

edited

abadger Aug 29, 2018 •

edited

abadger Aug 29, 2018 •

edited