Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only add 'credential' parameter to the lookup fields if configured while using tower_job_launch module #58367

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@rawat-he
Copy link

commented Jun 26, 2019

SUMMARY

When using tower_job_launch module, if parameter credential is not passed, don't add to the lookup fields list.

By default it is added and hence invoking credential resource lookup. Due to this GET credentials request is initiated (shown below in section Before Change Output )

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

tower_job_launch

ADDITIONAL INFORMATION
  1. Create a sample job template with credentials and unchecked prompt on launch option
  2. Create a sample project for inventory and inventory with sources.
  3. Create a inventory with sources as type = source from a project (2).
  4. A sample Ansible playbook with below task
- name: Launch the sample job template
  tower_job_launch:
    job_template: "my-sample-job-template"
    limit: "my-host"
    inventory: "my-sample-inventory"
    tower_verify_ssl: no
Before Change Output
  module_stderr: |-
    *** DETAILS: Getting the record. **********************************************
    GET https://woah-nonprod-tower.sdpamp.com/api/v2/job_templates/
    Params: [('name', 'my-sample-job-template')]
  
    *** DETAILS: Getting the record. **********************************************
    GET https://woah-nonprod-tower.sdpamp.com/api/v2/inventories/
    Params: [('name', 'my-sample-inventory')]
  
    *** DETAILS: Getting the record. **********************************************
    GET https://woah-nonprod-tower.sdpamp.com/api/v2/credentials/
    Params: []
  
    Traceback (most recent call last):
      File "/root/.ansible/tmp/ansible-tmp-1561513427.78-182021178072793/AnsiballZ_tower_job_launch.py", line 113, in <module>
        _ansiballz_main()
      File "/root/.ansible/tmp/ansible-tmp-1561513427.78-182021178072793/AnsiballZ_tower_job_launch.py", line 105, in _ansiballz_main
        invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
      File "/root/.ansible/tmp/ansible-tmp-1561513427.78-182021178072793/AnsiballZ_tower_job_launch.py", line 48, in invoke_module
        imp.load_module('__main__', mod, module, MOD_DESC)
      File "/tmp/ansible_tower_job_launch_payload_5Ljs29/__main__.py", line 149, in <module>
      File "/tmp/ansible_tower_job_launch_payload_5Ljs29/__main__.py", line 133, in main
      File "/data/.venv/lib/python2.7/site-packages/tower_cli/models/base.py", line 499, in get
        response = self.read(pk=pk, fail_on_no_results=True, fail_on_multiple_results=True, **kwargs)
      File "/data/.venv/lib/python2.7/site-packages/tower_cli/models/base.py", line 325, in read
        'fields. Please tighten your criteria.' % resp['count'])
    tower_cli.exceptions.MultipleResults: Expected one result, got 2. Possibly caused by not providing required fields. Please tighten your criteria.
  module_stdout: ''
  msg: |-
    MODULE FAILURE
    See stdout/stderr for the exact error
  rc: 1
After Change Output
No error and job is successfully launched in Ansible Tower

@ansibot ansibot removed the small_patch label Jun 26, 2019

lookup_fields = ('job_template', 'inventory', 'credential')
lookup_fields = ['job_template', 'inventory']
if credential is not None:
lookup_fields.append('credential')

This comment has been minimized.

Copy link
@AlanCoding

AlanCoding Jun 26, 2019

Member

I'm having a hard time seeing how this code will behave any differently. If the credential field is not provided, it looks like it should be None, in which case L145 name = params.pop(field) will return that and if name: resolves to False, meaning that it already isn't added to params, which is passed to the launch method.

The only functional difference I would predict is that after the change, params['credential'] has a value of None, whereas before the key was not present.

This comment has been minimized.

Copy link
@rawat-he

rawat-he Jun 27, 2019

Author

Yes you are right. Wouldn't behave differently without changes. Looked in to the history of file and it is fixed in commit

My local ansible fork was not synced and thus was looking at the old code.

Please reject the PR.

@ansibot ansibot added stale_ci and removed needs_triage labels Jul 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.