redhat-cop / infra.aap_configuration

A collection of roles to manage Ansible Controller and previously Ansible Tower
https://galaxy.ansible.com/infra/controller_configuration
GNU General Public License v3.0
269 stars 141 forks source link

Constructed Inventory colliding with defaults on controller_inventory_sources #601

Closed Klaas- closed 11 months ago

Klaas- commented 1 year ago

Summary

The new defaults are blocking creating a source for a constructed inventory.

Issue Type

Ansible, Collection, Controller details

infra.controller_configuration: core 2.13.3 awx.awx 22.2.0

ansible --version
core 2.13.3

ansible-galaxy collection list
Collection                     Version
------------------------------ -------
ansible.posix                  1.5.1
awx.awx                        22.2.0
azure.azcollection             1.15.0
community.crypto               2.13.0
community.general              6.6.0
containers.podman              1.10.1
fedora.linux_system_roles      1.38.0
infra.controller_configuration dev version from git
kubernetes.core                2.4.0
ovirt.ovirt                    3.1.2
servicenow.itsm                2.1.0

Controller version
awx 22.2.0

OS / ENVIRONMENT

RHEL8.7 VM

Desired Behavior

Creates constructed inventory

Actual Behavior

Fails for inventory sources

failed: [hostname.tld] (item={'failed': 0, 'started': 1, 'finished': 0, 'ansible_job_id': '673456544774.200426', 'results_file': '/tmp/.ansible_async/673456544774.200426', 'changed': False, '__controller_source_item': {'name': 'Auto-created source for: test_constructed', 'inventory': 'test_constructed', 'limit': 'host1,host2'}, 'ansible_loop_var': '__controller_source_item'}) => {"__inventory_source_job_async_results_item": {"__controller_source_item": {"inventory": "test_constructed", "limit": "host1,host2", "name": "Auto-created source for: test_constructed"}, "ansible_job_id": "673456544774.200426", "ansible_loop_var": "__controller_source_item", "changed": false, "failed": 0, "finished": 0, "results_file": "/tmp/.ansible_async/673456544774.200426", "started": 1}, "ansible_job_id": "673456544774.200426", "ansible_loop_var": "__inventory_source_job_async_results_item", "attempts": 2, "changed": false, "finished": 1, "msg": "The specified source project, {}, was not found.", "results_file": "/tmp/.ansible_async/673456544774.200426", "started": 1, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": []}

https://github.com/redhat-cop/controller_configuration/blob/a764ec7a64725ce35662a7dd2f655b1d9b589432/roles/inventory_sources/tasks/main.yml#L25

Change line back to omit if undefined source_project: "{{ controller_source_item.source_project.name | default(__controller_source_item.source_project | default(omit, true)) }}" But maybe this could also be default '' if controller_source_item.source == scm or something like that

Then next error is

failed: [hostname.tld] (item={'failed': 0, 'started': 1, 'finished': 0, 'ansible_job_id': '142609382821.203918', 'results_file': '/tmp/.ansible_async/142609382821.203918', 'changed': False, '__controller_source_item': {'name': 'Auto-created source for: test_constructed', 'inventory': 'test_constructed', 'limit': 'host1,host2'}, 'ansible_loop_var': '__controller_source_item'}) => {"__inventory_source_job_async_results_item": {"__controller_source_item": {"inventory": "test_constructed", "limit": "host1,host2", "name": "Auto-created source for: test_constructed"}, "ansible_job_id": "142609382821.203918", "ansible_loop_var": "__controller_source_item", "changed": false, "failed": 0, "finished": 0, "results_file": "/tmp/.ansible_async/142609382821.203918", "started": 1}, "ansible_job_id": "142609382821.203918", "ansible_loop_var": "__inventory_source_job_async_results_item", "attempts": 2, "changed": false, "finished": 1, "msg": "Unable to update inventory_source Auto-created source for: test_constructed, see response", "response": {"json": {"error": ["Cannot change field 'source' on a constructed inventory source."]}, "status_code": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER"}, "results_file": "/tmp/.ansible_async/142609382821.203918", "started": 1, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": []}

https://github.com/redhat-cop/controller_configuration/blob/a764ec7a64725ce35662a7dd2f655b1d9b589432/roles/inventory_sources/tasks/main.yml#L9

so here source can be different things, but it needs to be omitted for constructed inventories as of now

https://github.com/ansible/awx/blob/e434b1e0f3ff29858058fccf84d84392d4211140/awx_collection/plugins/modules/inventory.py#L104-L131

I am guessing this can be resolved on the awx collection side by also allowing cosntructed as source type, but that is not the case currently.

STEPS TO REPRODUCE

Minimal reproducer for me:

controller_inventories:
  - name: test
    organization: "{{ awx_primary_organization }}"
  - name: test_constructed
    organization: "{{ awx_primary_organization }}"
    kind: constructed
    input_inventories:
      - test

controller_hosts:
  - name: host1
    inventory: test
    enabled: true
  - name: host2
    inventory: test
    enabled: true

#controller_configuration_inventory_sources_enforce_defaults: false
controller_inventory_sources:
  - name: "Auto-created source for: test_constructed"
    inventory: test_constructed
    limit: host1,host2
    overwrite: true # not overwriteable for constructed 
    overwrite_vars: true # not overwriteable for constructed 
    update_on_launch: true # not overwriteable for constructed 

It works if you uncomment controller_configuration_inventory_sources_enforce_defaults: false

adonisgarciac commented 1 year ago

Hi, when you are creating an inventory_source, you have to specify the source parameter. In the default case, it will be scm and it will need other parameters for this kind of source.

I mean, even though source seems to be not mandatory, it only has these choices:

source=dict(choices=["scm", "ec2", "gce", "azure_rm", "vmware", "satellite6", "openstack", "rhv", "controller", "insights"]), And it depends on your choice, there will be more parameters to configure.

Klaas- commented 1 year ago

Hi, when you are creating an inventory_source, you have to specify the source parameter. In the default case, it will be scm and it will need other parameters for this kind of source.

I mean, even though source seems to be not mandatory, it only has these choices:

source=dict(choices=["scm", "ec2", "gce", "azure_rm", "vmware", "satellite6", "openstack", "rhv", "controller", "insights"]), And it depends on your choice, there will be more parameters to configure.

Hi, yes that is my point, it is currently required to not send any of those choices for a constructed inventory. So for constructed inventories you need to omit it because it's none of these choices.

What this means for the role here, it does not work with the new default enforcement unless you turn it off again (controller_configuration_inventory_sources_enforce_defaults: false)

adonisgarciac commented 1 year ago

Constructed inventory seems to be not supported yet:

https://docs.ansible.com/ansible/latest/collections/awx/awx/inventory_module.html#parameter-kind

But we will have to review this point.

Klaas- commented 1 year ago

the docs are outdated, it was released as part of the 22.0.0 collection https://github.com/ansible/awx/releases/tag/22.0.0

Klaas- commented 1 year ago

https://github.com/ansible/awx/commit/e22967d28de40fe40d94fd4818aa9248942f6b4c

djdanielsson commented 1 year ago

is this still an issue in the latest release?

Klaas- commented 1 year ago

yeah -- I'd create a PR but I am unsure how this should be best solved :)

sean-m-sullivan commented 1 year ago

So I went over this, and I think I found the underlying problem. Constructed inventories at first glance do not use inventory sources, however they do use them for sync.

You shouldn't ever edit a constructed inventory source through the api, I get it can Look like you CAN do it, but its not meant to be done. This is signified by the "auto created source" in the name. It would also lead to issues where the 'constructred inventory' and the inventory source have conflicting information, where changing one is reset by the other.

So this would be the playbook that shows how it should be done

---
- name: Playbook to configure constructed inventory
  hosts: localhost
  connection: local
  gather_facts: false
  vars:
    controller_validate_certs: false
    controller_hostname: controller.nas
    controller_username: admin
    controller_password: secret123
    controller_inventories:
      - name: test
        organization: Default
      - name: test_constructed
        organization: Default
        kind: constructed
        input_inventories:
          - test

    controller_hosts:
      - name: host1
        inventory: test
        enabled: true
      - name: host2
        inventory: test
        enabled: true

    #controller_configuration_inventory_sources_enforce_defaults: false
    controller_inventory_sources:
      - name: "Auto-created source for: test_constructed"
        inventory: test_constructed

  collections:
    - awx.awx
    - redhat_cop.controller_configuration

  roles:
    - infra.controller_configuration.inventories
    - infra.controller_configuration.hosts
    - infra.controller_configuration.inventory_source_update
...

Add to this, we are in the process of removing enforced defaults from the roles, Its a kludged together solution at best atm. The idea is sound, but its being set at the wrong level.

This PR to the awx.awx collection https://github.com/ansible/awx/pull/14128 would allow for it to be set at the module level, populated with defaults from the API. in addition this changes the trigger to the state option, which can be set globally with the controller_state variable, and/or overwritten on a per item basis. Once that PR is merged/published. So we can still enforce defaults, just going about it in a better way.

Thats a long way of saying we are removing these from the roles, but its something to think of as well.

So TLDR The question is more of how do we want to address this, Right now with enforced, and the module level enforce, it prevents users from making a change they shouldn't make, which could be a good thing, however with unenforced, they could make changes anyway.

sean-m-sullivan commented 1 year ago

With this, we would need some thing to skip creation of constructed ones called in the inventory sources, so that the functionally with inventory source update can be kept.

sean-m-sullivan commented 11 months ago

@Klaas- was taking another look at this. I've created a PR that will skip constructed inventories based on the source var.

controller_inventory_sources:
  - name: "Auto-created source for: test_constructed"
    inventory: test_constructed
    source: constructed

This is consistent with the export data from the API, that inventory sources for constructed are not meant to be edited, and still allow for inventory source updates to happen.

I am hoping this will fix things.

sean-m-sullivan commented 11 months ago

Closing this issue as I believe it is solved, if you find issues please reopen.

Klaas- commented 10 months ago

Hi, sorry for missing the discussion :)

I am looking at the latest version, this is not yet working or I am missing something :) source: constructed does not mean you don't want to edit the source because you may want to set certain things like in my usecases limitand source_vars. I think I have an idea how to get it working, let me do a few tests and I'll open a PR

adrienmrgn commented 8 months ago

I've just been confronted to the fact that I can no longer edit the source_vars or limit of a constructed inventory by configuration as code. Prior to this PR, we could edit the constructed inventory configuration, now it's no longer possible.

I'm interested in your way to handled constructed inventory configuration and help you on this.

Klaas- commented 2 months ago

So I think the focus here has shifted from this repo to awx repo, for anyone else stumbling over this issue, constucted inventories will need a new module in awx collection and that one can then be used in a new role here (or used by this role when kind=constructed) https://github.com/ansible/awx/issues/15119