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

Add global element for default group behavior. #2782

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sol1-matt
Copy link

Add Global Director Settings for group behavior so it can be changed from the current default of assignment (=) to append (+=).

  • Add global element for default group behavior.
  • Add default value 'assign' for default group behavior which matches group behavior .
  • Add logic to IcingaObjectGroups so the class initializes the with the global group behavior and sets the group operator based on the class operator.

Partial Fix for #344, #636, #824, #2273

Add default value 'assign' for default group behaviour which matches group behaviour.
Add logic to IcingaObjectGroups so the class initalizes with the global group behaviour and sets the group operator based on the class operator.
Partial Fix for Icinga#344, Icinga#636, Icinga#824, Icinga#2273
@cla-bot
Copy link

cla-bot bot commented Aug 7, 2023

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @sol1-matt

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

  • If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

@sol1-matt
Copy link
Author

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

CLA has been signed

@lippserd
Copy link
Member

lippserd commented Aug 7, 2023

@cla-bot check

@cla-bot cla-bot bot added the cla/signed label Aug 7, 2023
@Thomas-Gelf
Copy link
Contributor

Hi @sol1-matt,

most users probably want it this way, but for compatibility reasons this is hard to change. While I do not really like the idea of a global toggle, this of course addresses the problem. From a user perspective it might be irritating, having on Director behaving this way and another Director behaving another way. Even worse for vendors shipping baskets, that might behave differently in different setups.

To complete your variant, there is one important thing missing: object property resolution must work the same way, to address pre-calculated group memberships and related UI features. But even given this, I would prefer a clean solution, which would have to store the desired behaviour with every assignment.

Cheers,
Thomas

@sol1-matt
Copy link
Author

Hi @Thomas-Gelf,

I did look into individual sync rule properties, it is where the changes in library/Director/Objects/IcingaObjectGroups.php came from originally, but had some problems getting that working, from memory the database needs to be modified to accommodate the use of assign, remove and append.

If the preferred approach here is to implement this at the individual sync rule property level I'll look at what changes are needed to make that happen.

Thanks,
Matt

@Thomas-Gelf
Copy link
Contributor

@sol1-matt: this is about group inheritance behavior, not about Sync - isn't it?

@sol1-matt
Copy link
Author

@Thomas-Gelf

This is about adding the ability for groups to use inheritance.

But I do a lot of automation so my solutions focused on functionality that can be used in Automation Sync Rules to achieved the desired outcome, or the global setting which affects everything.

Your comment is a reminder than there is a whole other UI people use to enter things manually so I'll need to account for that as well which will be complicated. The Automation Sync Rule properties are ordered, the current group allocation UI in director not so much.

Are you sure you don't want to reconsider your stance on a global setting?
I'm managing multiple director installations, many of which I've added a global group append to. I've found the advantages of having a global setting far outweigh the downsides you've listed above in practical use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants