search

lucasheld / ansible-uptime-kuma

Ansible collection of modules to configure Uptime Kuma
GNU General Public License v3.0
144 stars 19 forks source link

Monitors including notifications are not idempotent #3

Closed spkenney closed 2 years ago

spkenney commented 2 years ago

If a monitor includes notifications, then that task always returns changed when re-run, even if the nothing in the monitor configuration has changed (i.e., these tasks are non-idempotent).

This issue is somewhat related to a previous issue I filed on the uptime-kuma-api in that the need to translate the simple array of integer notification IDs into the dict of {<id>: True, etc...} looks to be the cause. I haven't looked into the Uptime Kuma source to try to understand why it needs that format, but maybe we can get that changed to avoid this translation entirely :)

Anyways, the issue is that the object_changed function in module_utils/common.py detects a "change" in notifications because it is comparing the object that looks like {'14': True, '15': True} returned from the existing monitor with the array of integers ([14, 15]) from the task's attributes, which obviously don't match, even though they represent the same notifications already being configured as the playbook is requested be configured.

I'll share my current workaround as a PR, but I am not sure it's the best way to solve this problem.

lucasheld commented 2 years ago

Thanks for the report and the PR.

Yes, in reflection it was not useful to change the structure of the Uptime Kuma API at some places. The reason for this conversion was just to optimize the data structure.

Currently, I will not remove this conversion to not cause a breaking change. Because it doesn't seem to be possible to accept an argument with two different types in the Ansible module for backward compatibility reasons.

Instead, I added the missing conversion to uptime-kuma-api so that ansible can compare the differences (https://github.com/lucasheld/uptime-kuma-api/commit/314f07c93d87e04cce9cb947efd19ac01a33dc28).