ansible-middleware / amq

A collection to manage AMQ brokers
Apache License 2.0
20 stars 11 forks source link

Issue with restart handler #190

Closed cmasopust closed 1 month ago

cmasopust commented 1 month ago
SUMMARY

Restart handler results in error when amq_broker_configuration_file_refresh_period is defined via Jinja2.

We are defining amq_broker_configuration_file_refresh_period in such way:

- set_fact:
     amq_broker_configuration_file_refresh_period: "{{ ... }}"

which makes amq_broker_configuration_file_refresh_period being of type str

I know, this may be special to our environment, but a simple fix/mitigation could be changing the when condition for the restart handler.

- name: "Restart handler"
  ansible.builtin.include_tasks: restart.yml
  when: amq_broker_configuration_file_refresh_period | int < 1
  listen: "restart amq_broker with no config refresh"

I also tried to set jinja2_native=True in my ansible.cfg but that lead then to lots of errors at "Set journal configuration", error were:

TypeError: Argument must be bytes or unicode, got 'int'

ISSUE TYPE
ANSIBLE VERSION
ansible [core 2.14.14]
  config file = /home/cmaso/.ansible.cfg
  configured module search path = ['/home/cmaso/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  ansible collection location = /home/cmaso/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.9.18 (main, Jul  3 2024, 00:00:00) [GCC 11.4.1 20231218 (Red Hat 11.4.1-3)] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True
COLLECTION VERSION
redhat.amq_broker                        2.2.5
guidograzioli commented 1 month ago

Why not:

- set_fact:
    amq_broker_configuration_file_refresh_period: "{{ ...  | int }}"

?

Anyway I'll add the type conversion around too

cmasopust commented 1 month ago

because " | int" here would be a no-op... the fact still would be of type str

- hosts: localhost
  become: no
  gather_facts: no

  vars:
    var1: 25

  tasks:
    - ansible.builtin.set_fact:
        test1: "{{ var1 | int }}"

    - ansible.builtin.debug:
        msg: "{{ var1 }}"
    - ansible.builtin.debug:
        msg: "{{ var1 | type_debug }}"
    - ansible.builtin.debug:
        msg: "{{ test1 }}"
    - ansible.builtin.debug:
        msg: "{{ test1 | type_debug }}"

would result in:

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

TASK [ansible.builtin.set_fact] *************************************************************************************************************
ok: [localhost]

TASK [ansible.builtin.debug] ****************************************************************************************************************
ok: [localhost] => {
    "msg": 25
}

TASK [ansible.builtin.debug] ****************************************************************************************************************
ok: [localhost] => {
    "msg": "int"
}

TASK [ansible.builtin.debug] ****************************************************************************************************************
ok: [localhost] => {
    "msg": "25"
}

TASK [ansible.builtin.debug] ****************************************************************************************************************
ok: [localhost] => {
    "msg": "str"
}

PLAY RECAP **********************************************************************************************************************************
localhost                  : ok=5    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

thanks for adding the type conversion to the when !