ansible / awx-resource-operator

41 stars 34 forks source link

Rename job_template_ parameters to be consistent with AWX API #111

Closed rooftopcellist closed 1 year ago

rooftopcellist commented 1 year ago

The parameters on the JobTemplate spec are named in a way that is not consistent with the AWX API. For example:

              job_template_inventory:
              job_template_playbook:
              job_template_project:
              job_template_ask_vars:
              job_template_ask_inventory:

I think we should deprecate these parameters and introduce and document new ones that align with the naming used in the AWX API.

job_template_inventory
job_template_playbook
job_template_project
ask_variables_on_launch
ask_inventory_on_launch

The above are just the ones in the CRD, however, there are a lot of "passthrough" JobTemplate parameters which we may want to deprecate and shim for a release in case anyone is relying on them.

job_type: "{{ job_template_type | default('run') }}"
credentials: "{{ job_template_credentials | default(omit) }}"
vault_credential: "{{ job_template_vault_credentials | default(omit) }}"
forks: "{{ job_template_forks | default(omit) }}"
limit: "{{ job_template_limit | default(omit) }}"
verbosity: "{{ job_template_verbosity | default(omit) }}"
extra_vars: "{{ job_template_extra_vars | default(omit) }}"
become_enabled: "{{ job_template_become_enabled | default(omit) }}"
allow_simultaneous: "{{ job_template_allow_simultaneous | default(omit) }}"
state: "{{ tower_resource_state if tower_resource_state is defined else 'present' }}"
ask_credential_on_launch: "{{ job_template_ask_credential | default(omit) }}"
ask_job_type_on_launch: "{{ job_template_ask_job_type | default(omit) }}"
ask_limit_on_launch: "{{ job_template_ask_limit | default(omit) }}"
ask_scm_branch_on_launch: "{{ job_template_ask_scm | default(omit) }}"
ask_skip_tags_on_launch: "{{ job_template_ask_skip_tags | default(omit) }}"
ask_tags_on_launch: "{{ job_template_ask_tags | default(omit) }}"
ask_verbosity_on_launch: "{{ job_template_ask_verbosity | default(omit) }}"
custom_virtualenv: "{{ job_template_venv | default(omit) }}"
force_handlers: "{{ job_template_force_handlers | default('no') }}"
host_config_key: "{{ job_template_host_config_key | default(omit) }}"
instance_groups: "{{ job_template_instance_groups | default(omit) }}"
job_slice_count: "{{ job_template_job_slice_count | default('1') }}"
job_tags: "{{ job_template_job_tags | default(omit) }}"
organization: "{{ job_template_organization | default(omit) }}"
scm_branch: "{{ job_template_scm_branch | default(omit) }}"
skip_tags: "{{ job_template_skip_tags | default(omit) }}"
start_at_task: "{{ job_template_start_at_task | default(omit) }}"
use_fast_cache: "{{ job_template_use_fast_cache | default(omit) }}"
webhook_credential: "{{ job_template_webhook_credential | default(omit) }}"
webhook_service: "{{ job_template_webhook_service | default(omit) }}"


*Backwards Compatibility*

We'll also need to add backwards compatibility shims in the role. Basically a task that:
* preferentially uses extra_vars_prompt_on_launch if set
* falls back to job_template_ask_vars
* If neither are set, omit it or set a default where applicable

### Checklist

- [ ] Deprecate the existing params we wish to change (done in the CRD)
- [ ] Create new entries for the new name
- [ ] Update the clusterServiceVersion UI form spec.descriptors so that the deprecated ones are hidden
- [ ] Also in the CSV base template, add spec.descriptor entries for the new params
- [ ] Add logic in the job_template role to check for the old param name and use it if the preferred (new) on is not set. This can be done [here](https://github.com/ansible/awx-resource-operator/blob/eef331ef0e0cba2002b6419542a1b7f5fc9914d4/roles/jobtemplate/tasks/main.yml#L21-L58) I think.
- [ ] Add documentation in the top level README.md
- [ ] Talk to other teams and customers who may be using this communicate this change and determine if it will be breaking given the changes above.
rlopez133 commented 1 year ago

@rooftopcellist - I may take a crack at this. But since I have never done this before I did a have question regarding the variables. For example job_template_inventory, is the expectation for it to be just inventory?

Based on job template, looking at the API I see

{
    "name": "",
    "description": "",
    "job_type": "run",
    "inventory": null,
    "project": null,
    "playbook": "",
    "scm_branch": "",
    "forks": 0,
    "limit": "",
    "verbosity": 0,
    "extra_vars": "",
    "job_tags": "",
    "force_handlers": false,
    "skip_tags": "",
    "start_at_task": "",
    "timeout": 0,
    "use_fact_cache": false,
    "execution_environment": null,
    "host_config_key": "",
    "ask_scm_branch_on_launch": false,
    "ask_diff_mode_on_launch": false,
    "ask_variables_on_launch": false,
    "ask_limit_on_launch": false,
    "ask_tags_on_launch": false,
    "ask_skip_tags_on_launch": false,
    "ask_job_type_on_launch": false,
    "ask_verbosity_on_launch": false,
    "ask_inventory_on_launch": false,
    "ask_credential_on_launch": false,
    "ask_execution_environment_on_launch": false,
    "ask_labels_on_launch": false,
    "ask_forks_on_launch": false,
    "ask_job_slice_count_on_launch": false,
    "ask_timeout_on_launch": false,
    "ask_instance_groups_on_launch": false,
    "survey_enabled": false,
    "become_enabled": false,
    "diff_mode": false,
    "allow_simultaneous": false,
    "job_slice_count": 1,
    "webhook_service": null,
    "webhook_credential": null,
    "prevent_instance_group_fallback": false
}

should variable names match exactly as seen here? Or should they have a prefix like job_template_X, where X is the variable name from that list?