Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Director 1.10 - Error when modifying service apply rules #190

Closed
kastybel opened this issue Nov 10, 2022 · 9 comments · Fixed by #239
Closed

Director 1.10 - Error when modifying service apply rules #190

kastybel opened this issue Nov 10, 2022 · 9 comments · Fixed by #239
Labels
bug Something isn't working

Comments

@kastybel
Copy link

kastybel commented Nov 10, 2022

Description

Apparently, director versions later than 1.8.1 introduced a change in the API endpoint "director/serviceapplyrules".

This change broke functionality in module "icinga_service_apply.py"

Broken function:

  • Existing service apply rule objects cannot be modified.

Reproduction steps

  1. Create a service apply rule object using ansible.
  2. Run the same playbook again.
  3. Receive an error.
TASK [Add service apply rule to icinga] ********************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "exception when deleting: 'id'"}

Current Behavior

Error displayed when modifying an object.

Expected Behavior

Object is modified, no error required.

Additional information

The module "icinga_service_apply.py" looks for "id" key in the returned JSON object. The key "id" was removed from the API endpoint JSON output in Director 1.10 or even earlier.

Director 1.8.1

./director-curl GET director/serviceapplyrules
{
    "assign_filter": "host.vars.cisco_check_config_nwc=true",
    "check_interval": "600",
    "id": "16",                                                                       ####### !!! #######
    "imports": [
        "__cisco-check-config__"
    ],
    "object_name": "__nwc__cisco_check_config",
    "object_type": "apply",
    "retry_interval": "300"
},
...

Director 1.10.1 / 1.10.2

./director-curl GET director/serviceapplyrules
{
    "assign_filter": "host.vars.cisco_check_config_nwc=true",
    "check_interval": "600",
    "fields": [],
    "imports": [
        "__cisco-check-config__"
    ],
    "object_name": "__nwc__cisco_check_config",
    "object_type": "apply",
    "retry_interval": "300"
}
...

A fix that seems to do the trick (taken from service_template.py):

# diff icinga_service_apply.py_mod icinga_service_apply.py_orig
325c354
<     icinga_object = Icinga2APIObject(module=module, path="/service", data=data)
---
>     icinga_object = ServiceApplyRule(module=module, data=data)

This also makes Class ServiceApplyRule obsolete, but service template also does not have one...

@kastybel kastybel added the bug Something isn't working label Nov 10, 2022
@kastybel kastybel changed the title [Bug] Error when modifying service apply rules after director update 1.8 > 1.10 Error when modifying service apply rules after director update 1.8 > 1.10 Nov 10, 2022
@rndmh3ro
Copy link
Collaborator

I don't think your trick will work.

When you look at /service you'll get the defined service objects, for example:

> curl -u "icingaadmin:icinga" -H "Accept: application/json" http://localhost:80/icingaweb2/director/services
{ "objects": [ {
    "check_command": "hostalive",
    "display_name": "foo service",
    "fields": [],
    "host": "foohost",
    "notes": "example note",
    "notes_url": "'http://url1' 'http://url2'",
    "object_name": "foo service",
    "object_type": "object",
    "use_agent": false,
    "vars": {
        "procs_argument": "consul",
        "procs_critical": "1:",
        "procs_warning": "1:"
    }
}] }

And here are the service apply rules:

> curl -u "icingaadmin:icinga" -H "Accept: application/json" http://localhost:80/icingaweb2/director/serviceapplyrules
{ "objects": [ {
    "assign_filter": "host.name=%22foohost%22",
    "check_command": "hostalive",
    "check_interval": "600",
    "check_period": "24/7",
    "check_timeout": "60",
    "command_endpoint": "fooendpoint",
    "display_name": "dummy process",
    "enable_active_checks": true,
    "enable_event_handler": true,
    "enable_notifications": true,
    "enable_passive_checks": false,
    "enable_perfdata": true,
    "event_command": "restart_httpd",
    "fields": [],
    "groups": [
        "fooservicegroup"
    ],
    "imports": [
        "fooservicetemplate"
    ],
    "max_check_attempts": "5",
    "object_name": "SERVICE_dummy",
    "object_type": "apply",
    "retry_interval": "180"
}] }

Note the different object types.

It works for service templates, because service templates and services both can be found under /service (with differing object types), however the same is not true for services and service applies.

I opened an upstream-issue here: Icinga/icingaweb2-module-director#2642

@rndmh3ro
Copy link
Collaborator

Using the name doesn't work either according to the docs:

Please note that Service Apply Rule names are not unique in Icinga 2. They are not real objects, they are creating other objects in a loop. This makes it impossible to distinct them by name.

This part from the docs isn't true anymore;

Therefore, a dedicated REST API endpoint director/serviceapplyrules ships all Service Apply Rules combined with their internal ID. This ID can then be used to modify or delete a Rule via director/service.

@kastybel
Copy link
Author

kastybel commented Nov 11, 2022

Thanks for looking into this and highlighting the Director documentation. This needs to be addressed in Director then.

The endpoint /service is referenced directly in the class ServiceApplyRule. Service, Service Template and Service Apply all use the same endpoint. That is why the workaround works.

The class ServiceApplyRule is built on top of Icinga2APIObject, adds no other fields. It uses additional endpoint /serviceapplyrules only to find the id of the object, by filtering the list by object_name (which, however, as pointed out might not be unique).

### file: icinga_service_apply.yml
# ===========================================
# Icinga2 API class
#
class ServiceApplyRule(Icinga2APIObject):
    def __init__(self, module, data):
        path = "/service"
        super(ServiceApplyRule, self).__init__(module, path, data)

    def exists(self):
        ret = self.call_url(path="/serviceapplyrules")
        if ret["code"] == 200:
            for existing_rule in ret["data"]["objects"]:
                if existing_rule["object_name"] == self.data["object_name"]:
                    self.object_id = existing_rule["id"]
                    return self.object_id
        return False

    def delete(self):
        return super(ServiceApplyRule, self).delete(find_by="id")

    def modify(self):
        return super(ServiceApplyRule, self).modify(find_by="id")

    def diff(self):
        return super(ServiceApplyRule, self).diff(find_by="id")

Generally this still implies that one should use unique object names for the service apply rules when working with the ansible collection. As the 'id' for the service apply rule currently cannot be assigned using ansible module "t_systems_mms.icinga_director.icinga_service_apply", deploying different service apply rules with the same name will also not work.

@rndmh3ro
Copy link
Collaborator

So what's your preferred solution? Implementing the workaround so we check for name and assign_filter parameters for uniqueness?
I'd rather wait for an upstream fix - if no id will be provided, I'll have to implement the check for these two parameters.

@kastybel
Copy link
Author

kastybel commented Nov 15, 2022

Checking the name and the assign_filter parameters for uniqueness would probably not work. If you modify the assign_filter in the task, the module would still have no clue if that is a new object or a modified one. Enforcing unique object names might even be a better solution. Just need to add an explanation for that.

I believe the service apply rules, as other objects, should have unique names. The display_name should be available in the Director UI for the service apply rules as it is currently available via the API. Then one could choose to use unique object names and all the problems are gone. As mentioned the display_name is actually available via the director API and also when using this collection but it is missing in the UI.

So actually, when using the ansible collection, it is already not necessary to have non-unique object_name for the service apply rules. That is why the module could enforce unique names from my perspective.

Director should have display_name field in the UI too.

Added an upstream request for that Icinga/icingaweb2-module-director#2659

And till the one or another bugfix comes I'll have to use the workaround and will also maintain the objects with unique names.

@rndmh3ro rndmh3ro pinned this issue Nov 13, 2023
@rndmh3ro rndmh3ro changed the title Error when modifying service apply rules after director update 1.8 > 1.10 Director 1.10 - Error when modifying service apply rules Nov 13, 2023
@gianmarco-mameli
Copy link
Contributor

Hi all, any news on this bug? The error is still present when I try to pass on an already present service_apply

Thanks

@rndmh3ro
Copy link
Collaborator

rndmh3ro commented Jan 3, 2024

The upstream-issue is unanswered, so I don't know if this will ever be fixed.

I'm open to accepting a PR to workaround this issue.

@gianmarco-mameli
Copy link
Contributor

Hi, I've forked the repo and I'm going to take a look for a workaround

Thanks

rndmh3ro added a commit that referenced this issue Jan 18, 2024
* working version

* test for old director version with id

* lint and bugfix

* Update icinga_service_apply.py

* fix lint and cleanup

---------

Co-authored-by: Sebastian Gumprich <rndmh3ro@users.noreply.github.com>
@ww-bro
Copy link

ww-bro commented Feb 7, 2024

Thank you very much @gianmarco-mameli - I can confirm that this fixes the issue with the latest director verison 1.11.0

@rndmh3ro rndmh3ro unpinned this issue Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants