search

telekom-mms / ansible-collection-icinga-director

An Ansible collection that contains modules to change objects in Icinga 2 using the director API.
GNU General Public License v3.0
81 stars 30 forks source link

Fixes #190 - Workaround for service apply bug #239

Closed gianmarco-mameli closed 10 months ago

gianmarco-mameli commented 10 months ago

Fixes #190

I've found that changing all instances of 'find_by="id"' field with 'find_by="uuid"', in the ServiceApplyClass, actually made the error disappear. Unfortunately at this time I've only a completely empty Icinga installation, so I can't use the service apply created to any host, but 'm able to perform operations on Service Apply If you run some test with my fix the error is not present and you are now able to Add/Delete service apply in idempotent way

What do you think?

Thanks

rndmh3ro commented 10 months ago

The implementation looks good from first sight and the idea is great! Now the tests have to pass, too. There seems to be a problem when deleting objects. I don't know yet what is the problem.

gianmarco-mameli commented 10 months ago

you're right, something strange beacause the tests at the first run fail at the 'Add service apply rule with command_endpoint', but at the second run with verbosity fail at the previous task 'Add service apply rule to icinga'. I'll try to fix it

rndmh3ro commented 10 months ago

I just checked why it's failing and the answer is easy: we're testing against the director version 1.9.0, where uuid's don't exist, yet. So we probably have to check which director-version is used so we can use either uuid's or the old id-approach. However I didn't find an API-endpoint on the director-side that tells me what version is running.

gianmarco-mameli commented 10 months ago

Take a look at my last commit, i understand that is a workaround, and I'm glad if anyone find a better solution, but maybe it's a temp fix to the bug, let's see if the tests fails again

gianmarco-mameli commented 10 months ago

Ok, sorry I see some pylint errors, I'm going to check and resolve

rndmh3ro commented 10 months ago

This seems to work now, at least for the tested director-version.

What I'd like to do now is update our testing-strategy to test more icinga-director versions, starting from 1.8 to 1.11. But that's nothing for you to do, I'll do this. :)

Anyway, thanks for this PR!

gianmarco-mameli commented 10 months ago

You're welcome, very good, I've pushed the lint fix and cleaned the comments

rndmh3ro commented 10 months ago

Tested here: https://github.com/telekom-mms/ansible-collection-icinga-director/pull/242/checks

works for 1.8.1 - that's enough for me.