postgresql_set on log_rotation_size reports change with --check mode

[eb4x]

SUMMARY

The postgresql_set reports a change to log_rotation_size when running in --check mode.

ISSUE TYPE

• Bug Report

COMPONENT NAME

community.postgresql.postgresql_set

ANSIBLE VERSION

ansible [core 2.16.4]
  config file = /home/username/src/dia-ansible/ansible.cfg
  configured module search path = ['/home/username/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/username/.local/lib/python3.12/site-packages/ansible
  ansible collection location = /home/username/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/username/.local/bin/ansible
  python version = 3.12.5 (main, Aug  7 2024, 00:00:00) [GCC 13.3.1 20240522 (Red Hat 13.3.1-1)] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True

COLLECTION VERSION

Collection           Version
-------------------- -------
community.postgresql 3.5.0  

CONFIGURATION

CONFIG_FILE() = /home/username/src/dia-ansible/ansible.cfg
DEFAULT_HOST_LIST(/home/username/src/dia-ansible/ansible.cfg) = ['/home/username/src/dia-ansible/inventory']
DEFAULT_VAULT_IDENTITY_LIST(/home/username/src/dia-ansible/ansible.cfg) = ['app1@~/.ansible/vault/app1', 'app2@~/.ansible/vault/app2']
DEFAULT_VAULT_ID_MATCH(/home/username/src/dia-ansible/ansible.cfg) = True
EDITOR(env: EDITOR) = nvim

OS / ENVIRONMENT

Target: AlmaLinux 8.10 (Cerulean Leopard)

STEPS TO REPRODUCE

Run a playbook to set the log_rotation_size for your postgresql server, then run the same playbook with --check to see if it needs to change anything.

---
- hosts: somehost
  tasks:
    - community.postgresql.postgresql_set:
        name: "{{ item.name }}"
        value: "{{ item.value }}"
      become: true
      become_user: postgres
      loop:
        - { name: log_rotation_size, value: 10240 }

I.e.

ansible-playbook --diff pgsql_set.yml  # Acutal change
ansible-playbook --diff --check pgsql_set.yml # Reports change still needed
ansible-playbook --diff pgsql_set.yml # No change during run

It'll report back it's gonna change the value. But if you run it without the check, there's no change reported.

EXPECTED RESULTS

No change reported during --check mode.

ACTUAL RESULTS

It reports back a change is needed.

TASK [Configure postgresql] ****
changed: [somehost] => (item={'name': 'log_rotation_size', 'value': 10240})

I don't know how many other options are affected by this, but of all the changes I set here, only log_rotation_size reports back a need for change.

pgsql_server_postgresql_conf:
  - { name: max_connections, value: 600 }
  - { name: unix_socket_permissions, value: "0770" }
  - { name: shared_buffers, value: 2048MB }
  - { name: work_mem, value: 32MB }
  - { name: maintenance_work_mem, value: 512MB }
  - { name: max_stack_depth, value: 4MB }
  - { name: bgwriter_delay, value: 50ms }
  - { name: bgwriter_lru_maxpages, value: 1000 }
  - { name: bgwriter_lru_multiplier, value: 10 }
  - { name: effective_io_concurrency, value: 2 }
  - { name: wal_level, value: minimal }
  - { name: synchronous_commit, value: "off" }
  - { name: checkpoint_timeout, value: 30min }
  - { name: checkpoint_completion_target, value: 0.9 }
  - { name: max_wal_senders, value: 0 }
  - { name: random_page_cost, value: 1.1 }
  - { name: log_directory, value: pg_log }
  - { name: log_rotation_size, value: 1000000 }
  - { name: log_line_prefix, value: "< %m > " }
  - { name: log_timezone, value: Europe/Oslo }
  - { name: autovacuum_max_workers, value: 6 }
  - { name: autovacuum_vacuum_scale_factor, value: 0.01 }
  - { name: autovacuum_vacuum_cost_delay, value: 20ms }
  - { name: autovacuum_vacuum_cost_limit, value: 3000 }
  - { name: TimeZone, value: Europe/Oslo }
  - { name: lc_messages, value: C }
  - { name: lc_monetary, value: C }
  - { name: lc_numeric, value: C }
  - { name: lc_time, value: C }

[hunleyd]

Thanks for the report @eb4x !

[hunleyd]

@eb4x is this something you might feel like tackling yourself? if not, that's OK; someone will pick it up. if so, a couple of (probably obvious) things worth mentioning:

• There's a short quick start dev guide that can help
• Please add a changelog fragment to the PR
• Any questions/concerns/circumstances change - please let us know so that someone else can pick it up

thanks!

[cameronmurdoch]

I've had a look at this and I think it effects at least all settings that have a unit of kB or MB if the value is supplied without units.

I think the bug is here:

community.postgresql/plugins/modules/postgresql_set.py

Line 492 in c0db2c8

if pretty_to_bytes(value) == pretty_to_bytes(current_val):

In the example given above this check ends up being:

if pretty_to_bytes('10240') == pretty_to_bytes('10MB'):

which evaluates to false.

I can try to fix this, but am unsure how much time I will have over the next few days.

[Andersson007]

@cameronmurdoch please go ahead and thanks! i've assigned it to you. If the circumstances change, please let us know

[cameronmurdoch]

@Andersson007 another part of this bug is that for this case (the setting has a unit but is supplied without units) the return value of value_pretty is different dependent on check_mode.

For example, using the values from above and no check mode we get:

"changed": false,
"name": "log_rotation_size",
"prev_val_pretty": "10MB",
"value_pretty": "10MB"

But with check mode we get:

"changed": true,
"name": "log_rotation_size",
"prev_val_pretty": "10MB",
"value_pretty": "10240"

The doc says that value_pretty is "Information about current state of the parameter." but in check mode it is the value that the user supplied, whereas in normal mode it comes from postgresql (after reconnecting to check if the setting changed).

How important do you think is it that this bit of the problem is also fixed? I would have thought that being consistent between check mode and normal mode is better?

Edit: I've also researched this bug a bit more and it seems to affect all settings that have units of BLOCKSIZE,kB,MB,min,s, and ms.

[Andersson007]

@cameronmurdoch yes, consistent work would be great

[cameronmurdoch]

Which file should new integration tests go in? postgresql_set/tasks/options_coverage.yml or postgresql_set/tasks/postgresql_set_initial.yml?

[Andersson007]

Which file should new integration tests go in? postgresql_set/tasks/options_coverage.yml or postgresql_set/tasks/postgresql_set_initial.yml?

@cameronmurdoch in this case, i think, yes

[cameronmurdoch]

@Andersson007 That's great, but which of the two files? :-)

[Andersson007]

@Andersson007 That's great, but which of the two files? :-)

Oh, sorry, i saw only one somehow, now i see both - use any:)