[BUG] Unexpected host-update behavior about ansible-collection-checkmk.general HOT 16 CLOSED

Hi Oliver,
fair enough, I hear you loud and clear.
I still deem my argument valid, but yours is just as valid.
We need to take a look at this, and I will think about making the documentation clearer - if possible - in the meantime.

Regarding the lookup plugin you suggest: I already thought about that too. But the problem is resources: We need to prioritize the little time we have and focus on the most valuable features for now. So every contribution toward that will be appreciated, even if it is a work in progress PR.

from ansible-collection-checkmk.general.

The API supports the three modes "attributes", "update_attributes" and "remove_attributes". Robin and I were recently talking about using these in the collection, as well.

from ansible-collection-checkmk.general.

@ogaida , @robin-tribe29, @lgetwan

It might be that I have misunderstood Oliver Gaida's proposal and he exactly proposes the following

What's about state dependent default value for folder?

• in case host is not registered and folder is not specified it defaults to / as it is now
• in case host is registered and folder is not specified it defaults to the folder name where it is already sitting.

from ansible-collection-checkmk.general.

@msekania
That's the behavior that I would expect, too.

from ansible-collection-checkmk.general.

@ogaida

regarding Dec 8, 2022 comment:
someone might be already using default value of folder which is "/" to move already existing hosts to "/" (origin).

Anyway, what you meant (and Dec 9, 2022 my comment) , is definitely more useful approach!
and if one wants to move already registered hosts to "/", one should specify folder: "/" explicitly.

from ansible-collection-checkmk.general.

@ogaida sorry, this one seems to have slipped by. Judging on the lack of affirmative feedback, this seems to be not a huge issue for many, so it was not a pressing matter. Anyway, we want to improve this.

To get this straight: The target state would be this:

• in case host is not registered and folder is not specified it defaults to / as it is now
• in case host is registered and folder is not specified it defaults to the folder name where it is already sitting.

@msekania and @ogaida can you please confirm this with a thumbs up?
Then we will put this on top of our priorities (which can still mean some wait time).
Otherwise, we would be very happy to see a PR from you. ✌️

from ansible-collection-checkmk.general.

I took a look into the openapi documentation. There are three different possibilities to change attributes:

1. overwrite: use attributes
2. keep existing and only update given attributes: use update_attributes
3. remove only given attributes: use remove_attributes

For me it seems the best solution to take this parameters as module parameters too. So anyone could decide what is the best option.

And about the folder, we should differentiate two cases:

1. if host does not exist, then folder defaults to '/'
2. if host already exist, leave folder untouched except if folder was explicit set to a new value.

from ansible-collection-checkmk.general.

Hi Oliver and thanks for your input!
I do want to say this: Ansible is a tool, which makes sure a certain state is there. So taking your example, I would in general actually expect this behavior, as Ansible cares less about what is already there, but rather about what you want to have there.

That philosophical detour aside, I understand your desire perfectly well and we will take a look at how to solve this.
Just do not hold your breath, because I see this as an enhancement rather than a bug, so it does not have the highest priority.
Feel free to propose a PR however, even if it is just a work in progress.

from ansible-collection-checkmk.general.

Hi Robin,
there are many ansible-modules out there, which care about what is already there. For example take the user module:

- name: Set maximum expiration date for password
  user:
    name: ram19
    password_expire_max: 10

this will not reset the password or the shell or anything else. And yes, the desired state will be activated.

The great danger with the actual handling is, you remove tons of attributes if you dont know it. And here comes a first step... You should comment this problem in the main README.md and in the documentation of the host module. For example:

- name: update host, set the alias, delete all other explicit attributes and move back to folder /
  tribe29.checkmk.host:
    server_url: "http://localhost/"
    site: "cmk"
    automation_user: "automation"
    automation_secret: "12345"
    host_name: "myhost"
    state: "present"
    attributes:
      alias: my alias

from ansible-collection-checkmk.general.

Another point is, there is no info module for hosts. Nothing to gather facts for existing hosts. So there is no easy way to update a host at all. How should i set the old attributes in a playbook, if i do not have a module for that. Should i do an API-Request to fetch the data first and then fumble them apart with ansible_json_query, really?

from ansible-collection-checkmk.general.

Excellent, please set update_attributes as the default update_method and please remember to not overwrite the folder if the host already exists. :-)

from ansible-collection-checkmk.general.

and please remember to not overwrite the folder if the host already exists. :-)

@ogaida, if one will not overwrite the folder, then, unfortunately, it will break already existing feature, namely moving existing host from current folder to another folder!

from ansible-collection-checkmk.general.

@msekania
your comment from Dec 9, 2022 is right. that's what I meant. But the comment from Dec 8, I do not know what you mean.

from ansible-collection-checkmk.general.

@robin-tribe29 Hi Robin, is there any hope that this bug will be fixed soon?

from ansible-collection-checkmk.general.

Hi Robin,

thank you very much for your quick answer. one point is very important to make the module idempotent. as we wrote about in November here in this issue, please set update_attributes as the default update_method.

Thanks in advance and best regards from Duderstadt ✌️

from ansible-collection-checkmk.general.

i made a pr last night. it passed the sanity checks. @robin-tribe29 & @lgetwan please give me some feedback to my changes.

I brought the attributes update-behavior from the API to the module. So if the host already exists, you can take the arg update_attributes to just update what you want and leave all other attributes and folder untouched. It also supports remove_attributes. On create the update_attributes works like an alias for attributes and it is idempotent. remove_attributes will be ignored on create.

from ansible-collection-checkmk.general.