search

theforeman / foreman-ansible-modules

Ansible modules for interacting with the Foreman API and various plugin APIs such as Katello
GNU General Public License v3.0
147 stars 163 forks source link

Apypie modules does not support changing associated resources #290

Closed ephracis closed 5 years ago

ephracis commented 5 years ago
SUMMARY

New modules based on Apypie does not seem to support changing associated resources such as organizations or locations. One example would be foreman_domain.

This is because the API wants the POST/PUT params to be named organization_ids but the GET request returns organizations. This causes the comparison in update_resource (which iterates the params from GET) to miss the updated IDs.

ISSUE TYPE
ANSIBLE VERSION
ansible 2.8.0
  config file = /Users/chbr/repos/work/github/foreman-ansible-modules/ansible.cfg
  configured module search path = ['/Users/chbr/repos/work/github/foreman-ansible-modules/modules']
  ansible python module location = /usr/local/lib/python3.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.7.3 (default, Mar 27 2019, 09:23:15) [Clang 10.0.1 (clang-1001.0.46.3)]
KATELLO/FOREMAN VERSION
tfm-rubygem-katello-3.11.1-1.el7.noarch
foreman-1.21.3-1.el7.noarch
NAILGUN VERSION
Version: 0.32.0
STEPS TO REPRODUCE
- hosts: tests
  gather_facts: false
  tasks:
  - name: Load server config
    include_vars:
      file: server_vars.yml
  - name: Load domain test-config
    include_vars:
      file: domain_vars.yml
  - include_tasks: tasks/domain.yml
    vars:
      domain_locations: []
      domain_organizations: []
      domain_state: "present"
      expected_change: true
  - include_tasks: tasks/domain.yml
    vars:
      domain_state: "present"
      expected_change: true
EXPECTED RESULTS

The tests should pass.

ACTUAL RESULTS

The tests fails because the module did not report changed when the list of associated organisations and locations were changed. Ocular inspection of Foreman shows that no change occurred and the domain remains unassociated.

evgeni commented 5 years ago

Thanks for the reproducer steps. I roughly know why this is happening and am trying to get a fix for that ASAP.

evgeni commented 5 years ago

@ephracis can you test #291 too please?

ephracis commented 5 years ago

Tried it with #286 and the following tests:

- hosts: tests
  gather_facts: false
  tasks:
  - name: Load server config
    include_vars:
      file: server_vars.yml
  - include: tasks/auth_source_ldap.yml
    vars:
      auth_source_ldap_state: present
      expected_change: true
  - include: tasks/auth_source_ldap.yml
    vars:
      auth_source_ldap_state: present
      auth_source_ldap_organizations: []
      auth_source_ldap_locations: []
      expected_change: true
  - include: tasks/auth_source_ldap.yml
    vars:
      auth_source_ldap_state: present
      expected_change: true
  - include: tasks/auth_source_ldap.yml
    vars:
      auth_source_ldap_state: present
      expected_change: false
  - include: tasks/auth_source_ldap.yml
    vars:
      auth_source_ldap_state: absent
      expected_change: true
  - include: tasks/auth_source_ldap.yml
    vars:
      auth_source_ldap_state: absent
      expected_change: false

It worked fine!

ephracis commented 5 years ago

If you get #291 merged I can rebase #286 onto it with the extended tests.