Skip to content

Add support for 'Dependency Redundancy Groups' #2860

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

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

Conversation

Donien
Copy link

@Donien Donien commented Jan 18, 2024

Icinga2 version 2.14.0 introduced dependency redundancy groups which allow for more flexiblity when it comes to determining the "reachability" of a host (e.g. multiple DNS servers as redundant dependencies).

This PR adds a new attribute "redundancy_group" to dependencies and adjusts the web forms accordingly.

form

rendered


If any changes need to be made, please let me know.

@cla-bot cla-bot bot added the cla/signed label Jan 18, 2024
@widhalmt
Copy link
Member

ref/NC/805863

@nilmerg nilmerg added this to the Dependencies milestone Nov 4, 2024
@raviks789
Copy link
Collaborator

raviks789 commented Feb 10, 2025

@Donien Please rebase with master and resolve conflicts

@Donien Donien force-pushed the add-dependency-redundancy-group branch from 4feb84f to 13b93d1 Compare February 10, 2025 10:17
@Donien
Copy link
Author

Donien commented Feb 10, 2025

@raviks789 Should be okay now.
I also made it one clean commit.

@Donien Donien force-pushed the add-dependency-redundancy-group branch from 13b93d1 to 01c6c0c Compare February 24, 2025 07:33
@nilmerg nilmerg requested a review from raviks789 May 19, 2025 08:21
Copy link
Collaborator

@raviks789 raviks789 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been decided that this PR will be merged to master, so please make the necessary updates based on the comments. Once the changes are done, please rebase with master.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restore this file.


INSERT INTO director_schema_migration
(schema_version, migration_time)
VALUES (189, NOW());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise the schema version number to 190. And change the file name to upgrade_190.sql.

@Donien Donien force-pushed the add-dependency-redundancy-group branch from 01c6c0c to 9a11ed5 Compare June 2, 2025 09:43
@Donien
Copy link
Author

Donien commented Jun 2, 2025

Hi @raviks789,

please have another look. Everything should be fine now after rebase and suggested changes.

Copy link
Collaborator

@raviks789 raviks789 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just address that one change I have requested, otherwise it looks good to me. Also rebase with master once again :)

. ' whole dependency to be fulfilled.'
),
'class' => "director-suggest",
'data-suggestion-context' => 'dependencyredundancygroups',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'data-suggestion-context' => 'dependencyredundancygroups',
'data-suggestion-context' => 'dependencyRedundancyGroups',

I know that the methods are case insensitive in PHP, but it is still desirable to match cases for consistency and readability reasons.

@Donien Donien force-pushed the add-dependency-redundancy-group branch from 9a11ed5 to 2102c17 Compare June 18, 2025 11:01
@Donien
Copy link
Author

Donien commented Jun 18, 2025

Done.

Thanks again for your continued support on this, @raviks789 :)

Copy link
Collaborator

@raviks789 raviks789 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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.

4 participants