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

feature: Host dependencies #7332

Merged
merged 53 commits into from Dec 20, 2017

Conversation

Projects
None yet
10 participants
@aldemira
Contributor

aldemira commented Sep 13, 2017

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

I felt obliged to produce this code :) . It obviously needs vigorous testing, I'll ask it on the forum thread. I really wouldn't want to break the alerts!

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Sep 13, 2017

Member

let us know when it's ready for testing. I can test it out on one of my dev servers.

Member

kkrumm1 commented Sep 13, 2017

let us know when it's ready for testing. I can test it out on one of my dev servers.

aldemira added some commits Sep 13, 2017

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Sep 13, 2017

Contributor

I think it is ready for testing now.

Contributor

aldemira commented Sep 13, 2017

I think it is ready for testing now.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Sep 13, 2017

Member

Wow, awesome work :)

No code review yet but a quick play around:

  • Delete parent host button not working for me due to mysql strict (we should always aim to support this with new queries: MySQL Error: Incorrect integer value: '' for column 'parent_id' at row 1 (UPDATE devices set parent_id ='' WHERE device_id = '173'))
  • We should definitely add the ability to show the bulk host dependancies.
  • Make the devices in the /host-dependencies/ section clickable.
  • Whilst the /host-dependencies/ page has been bootgrid enabled (someone is actually converting these to Datatables which should then be done here, it does however need server side processing done. I have 4k+ devices so this page won't work well for me otherwise :)
  • Do we need to index the new column?
Member

laf commented Sep 13, 2017

Wow, awesome work :)

No code review yet but a quick play around:

  • Delete parent host button not working for me due to mysql strict (we should always aim to support this with new queries: MySQL Error: Incorrect integer value: '' for column 'parent_id' at row 1 (UPDATE devices set parent_id ='' WHERE device_id = '173'))
  • We should definitely add the ability to show the bulk host dependancies.
  • Make the devices in the /host-dependencies/ section clickable.
  • Whilst the /host-dependencies/ page has been bootgrid enabled (someone is actually converting these to Datatables which should then be done here, it does however need server side processing done. I have 4k+ devices so this page won't work well for me otherwise :)
  • Do we need to index the new column?

@murrant murrant changed the title from newfeature: Host dependencies to feature: Host dependencies Sep 14, 2017

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Sep 14, 2017

Contributor

@laf
Yes, I'm not a big fan of bootgrid, so to do the conversion, I'd like to wait until datatable pr is merged, and that will save me from any possible redundant work.
For the db index, I'm using join to get the parent hostname from the devices table, so I thought it would make things faster.
I'm working on the rest meanwhile

Contributor

aldemira commented Sep 14, 2017

@laf
Yes, I'm not a big fan of bootgrid, so to do the conversion, I'd like to wait until datatable pr is merged, and that will save me from any possible redundant work.
For the db index, I'm using join to get the parent hostname from the devices table, so I thought it would make things faster.
I'm working on the rest meanwhile

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Sep 19, 2017

Member

@aldemira You can prepend EXPLAIN to your sql queries to check the execution plan and see if it uses the correct indexes.

Member

murrant commented Sep 19, 2017

@aldemira You can prepend EXPLAIN to your sql queries to check the execution plan and see if it uses the correct indexes.

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Sep 21, 2017

Contributor

Yes I checked it, and it's not being used. So I'm removing the index line.

Contributor

aldemira commented Sep 21, 2017

Yes I checked it, and it's not being used. So I'm removing the index line.

@barryodonovan

This comment has been minimized.

Show comment
Hide comment
@barryodonovan

barryodonovan Sep 22, 2017

Contributor

Hi,

Following code (includes/alerts.inc.php lines 889-892), $result will always be falsy since 'status' == 0 when 'status_reason' = 'icmp' therefore function will never return true

 $result = dbFetchCell("SELECT `status` from `devices` WHERE device_id = ? and status_reason = 'icmp'", array($parent_id));
if ($result) {
    return true;
}
Contributor

barryodonovan commented Sep 22, 2017

Hi,

Following code (includes/alerts.inc.php lines 889-892), $result will always be falsy since 'status' == 0 when 'status_reason' = 'icmp' therefore function will never return true

 $result = dbFetchCell("SELECT `status` from `devices` WHERE device_id = ? and status_reason = 'icmp'", array($parent_id));
if ($result) {
    return true;
}
@barryodonovan

This comment has been minimized.

Show comment
Hide comment
@barryodonovan

barryodonovan Sep 25, 2017

Contributor

Hi,

Unfortunately the query is still incorrect.

It needs to be

    $result = dbFetchCell("SELECT `device_id` from `devices` WHERE device_id = ? and status_reason = 'icmp'", array($parent_id));
    if ($result) {
        return true;
    }

Also there appears to be some issue with the diff generated as a result of changing 207.sql to 208.sql
I had to remove that section of the diff and apply the sql by hand.

On a positive note, recursive parent dependencies seem to work really well, I went to 4 levels on my dev box and only got one notice (once everything settled down after a full poller cycle)

Barry

Contributor

barryodonovan commented Sep 25, 2017

Hi,

Unfortunately the query is still incorrect.

It needs to be

    $result = dbFetchCell("SELECT `device_id` from `devices` WHERE device_id = ? and status_reason = 'icmp'", array($parent_id));
    if ($result) {
        return true;
    }

Also there appears to be some issue with the diff generated as a result of changing 207.sql to 208.sql
I had to remove that section of the diff and apply the sql by hand.

On a positive note, recursive parent dependencies seem to work really well, I went to 4 levels on my dev box and only got one notice (once everything settled down after a full poller cycle)

Barry

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Sep 25, 2017

Member

If you don't care about the result, you can just do:

"SELECT 1 from `devices` WHERE device_id = ? and status_reason = 'icmp'"
Member

murrant commented Sep 25, 2017

If you don't care about the result, you can just do:

"SELECT 1 from `devices` WHERE device_id = ? and status_reason = 'icmp'"
@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Oct 6, 2017

Contributor

Is datatables pr withdrawn? If so, I'll fix the SQL problems and we'll be good to go.

Contributor

aldemira commented Oct 6, 2017

Is datatables pr withdrawn? If so, I'll fix the SQL problems and we'll be good to go.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Oct 6, 2017

Member

@aldemira It was, no idea why so will have to stick with what we have for now.

Member

laf commented Oct 6, 2017

@aldemira It was, no idea why so will have to stick with what we have for now.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Oct 7, 2017

Member

@aldemira You all done ready for a code review?

Member

laf commented Oct 7, 2017

@aldemira You all done ready for a code review?

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Oct 8, 2017

Contributor

@laf Yes please!

Contributor

aldemira commented Oct 8, 2017

@laf Yes please!

@laf

Some comments left inline in the code.

Can we expect that a device will only have one dependancy or should we support multiple dependencies? It will get complicated as you may then need to allow for setting all dependancies to be down or just one, etc. Thoughts?

Absolutely great start. As far as I'm concerned if the suggestions I've made are fixed then we should merge.

@librenms/reviewers anyone else?

@@ -0,0 +1,7 @@
source: Alerting/Hostdepencies.md

This comment has been minimized.

@laf

laf Oct 15, 2017

Member

This file won't get generated until you add it mkdocs.yaml. As it's a hidden one (part of another heading) then add it to hidden at the bottom and give it a good title.

@laf

laf Oct 15, 2017

Member

This file won't get generated until you add it mkdocs.yaml. As it's a hidden one (part of another heading) then add it to hidden at the bottom and give it a good title.

This comment has been minimized.

@aldemira

aldemira Oct 17, 2017

Contributor

Is mkdocs.yaml in another repo? I couldn't find it.

@aldemira

aldemira Oct 17, 2017

Contributor

Is mkdocs.yaml in another repo? I couldn't find it.

This comment has been minimized.

@laf

laf Oct 17, 2017

Member

Nope, should be in the root of this one.

@laf

laf Oct 17, 2017

Member

Nope, should be in the root of this one.

Show outdated Hide outdated html/includes/forms/delete-host-dependency.inc.php Outdated
Show outdated Hide outdated html/includes/forms/get-host-dependencies.inc.php Outdated
Show outdated Hide outdated html/includes/forms/save-host-dependency.inc.php Outdated
Show outdated Hide outdated html/includes/forms/save-host-dependency.inc.php Outdated
Show outdated Hide outdated html/includes/modal/edit_host_dependency.inc.php Outdated
Show outdated Hide outdated html/includes/modal/manage_host_dependencies.inc.php Outdated
Show outdated Hide outdated html/includes/modal/manage_host_dependencies.inc.php Outdated
Show outdated Hide outdated html/includes/modal/manage_host_dependencies.inc.php Outdated
Show outdated Hide outdated includes/alerts.inc.php Outdated

@laf laf added this to the 1.33 milestone Oct 15, 2017

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 14, 2017

Member

Also, when editing host dependencies and you click save, the page should refresh (or at least the table should).

In fact for me, editing them doesn't actually update / edit them it just says it has done but refreshing shows the old data.

Member

laf commented Dec 14, 2017

Also, when editing host dependencies and you click save, the page should refresh (or at least the table should).

In fact for me, editing them doesn't actually update / edit them it just says it has done but refreshing shows the old data.

Show outdated Hide outdated includes/alerts.inc.php Outdated
Show outdated Hide outdated includes/alerts.inc.php Outdated
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 14, 2017

Member

I think this fixes it for me:

https://p.libren.ms/view/raw/b0bbe5db

Member

laf commented Dec 14, 2017

I think this fixes it for me:

https://p.libren.ms/view/raw/b0bbe5db

Show outdated Hide outdated includes/alerts.inc.php Outdated
@laf

Couple of more changes from my side.

@@ -0,0 +1 @@
.select2-container{box-sizing:border-box;display:inline-block;margin:0;position:relative;vertical-align:middle}.select2-container .select2-selection--single{box-sizing:border-box;cursor:pointer;display:block;height:28px;user-select:none;-webkit-user-select:none}.select2-container .select2-selection--single .select2-selection__rendered{display:block;padding-left:8px;padding-right:20px;overflow:hidden;text-overflow:ellipsis;white-space:nowrap}.select2-container .select2-selection--single .select2-selection__clear{position:relative}.select2-container[dir="rtl"] .select2-selection--single .select2-selection__rendered{padding-right:8px;padding-left:20px}.select2-container .select2-selection--multiple{box-sizing:border-box;cursor:pointer;display:block;min-height:32px;user-select:none;-webkit-user-select:none}.select2-container .select2-selection--multiple .select2-selection__rendered{display:inline-block;overflow:hidden;padding-left:8px;text-overflow:ellipsis;white-space:nowrap}.select2-container .select2-search--inline{float:left}.select2-container .select2-search--inline .select2-search__field{box-sizing:border-box;border:none;font-size:100%;margin-top:5px;padding:0}.select2-container .select2-search--inline .select2-search__field::-webkit-search-cancel-button{-webkit-appearance:none}.select2-dropdown{background-color:white;border:1px solid #aaa;border-radius:4px;box-sizing:border-box;display:block;position:absolute;left:-100000px;width:100%;z-index:1051}.select2-results{display:block}.select2-results__options{list-style:none;margin:0;padding:0}.select2-results__option{padding:6px;user-select:none;-webkit-user-select:none}.select2-results__option[aria-selected]{cursor:pointer}.select2-container--open .select2-dropdown{left:0}.select2-container--open .select2-dropdown--above{border-bottom:none;border-bottom-left-radius:0;border-bottom-right-radius:0}.select2-container--open .select2-dropdown--below{border-top:none;border-top-left-radius:0;border-top-right-radius:0}.select2-search--dropdown{display:block;padding:4px}.select2-search--dropdown .select2-search__field{padding:4px;width:100%;box-sizing:border-box}.select2-search--dropdown .select2-search__field::-webkit-search-cancel-button{-webkit-appearance:none}.select2-search--dropdown.select2-search--hide{display:none}.select2-close-mask{border:0;margin:0;padding:0;display:block;position:fixed;left:0;top:0;min-height:100%;min-width:100%;height:auto;width:auto;opacity:0;z-index:99;background-color:#fff;filter:alpha(opacity=0)}.select2-hidden-accessible{border:0 !important;clip:rect(0 0 0 0) !important;height:1px !important;margin:-1px !important;overflow:hidden !important;padding:0 !important;position:absolute !important;width:1px !important}.select2-container--default .select2-selection--single{background-color:#fff;border:1px solid #aaa;border-radius:4px}.select2-container--default .select2-selection--single .select2-selection__rendered{color:#444;line-height:28px}.select2-container--default .select2-selection--single .select2-selection__clear{cursor:pointer;float:right;font-weight:bold}.select2-container--default .select2-selection--single .select2-selection__placeholder{color:#999}.select2-container--default .select2-selection--single .select2-selection__arrow{height:26px;position:absolute;top:1px;right:1px;width:20px}.select2-container--default .select2-selection--single .select2-selection__arrow b{border-color:#888 transparent transparent transparent;border-style:solid;border-width:5px 4px 0 4px;height:0;left:50%;margin-left:-4px;margin-top:-2px;position:absolute;top:50%;width:0}.select2-container--default[dir="rtl"] .select2-selection--single .select2-selection__clear{float:left}.select2-container--default[dir="rtl"] .select2-selection--single .select2-selection__arrow{left:1px;right:auto}.select2-container--default.select2-container--disabled .select2-selection--single{background-color:#eee;cursor:default}.select2-container--default.select2-container--disabled .select2-selection--single .select2-selection__clear{display:none}.select2-container--default.select2-container--open .select2-selection--single .select2-selection__arrow b{border-color:transparent transparent #888 transparent;border-width:0 4px 5px 4px}.select2-container--default .select2-selection--multiple{background-color:white;border:1px solid #aaa;border-radius:4px;cursor:text}.select2-container--default .select2-selection--multiple .select2-selection__rendered{box-sizing:border-box;list-style:none;margin:0;padding:0 5px;width:100%}.select2-container--default .select2-selection--multiple .select2-selection__rendered li{list-style:none}.select2-container--default .select2-selection--multiple .select2-selection__placeholder{color:#999;margin-top:5px;float:left}.select2-container--default .select2-selection--multiple .select2-selection__clear{cursor:pointer;float:right;font-weight:bold;margin-top:5px;margin-right:10px}.select2-container--default .select2-selection--multiple .select2-selection__choice{background-color:#e4e4e4;border:1px solid #aaa;border-radius:4px;cursor:default;float:left;margin-right:5px;margin-top:5px;padding:0 5px}.select2-container--default .select2-selection--multiple .select2-selection__choice__remove{color:#999;cursor:pointer;display:inline-block;font-weight:bold;margin-right:2px}.select2-container--default .select2-selection--multiple .select2-selection__choice__remove:hover{color:#333}.select2-container--default[dir="rtl"] .select2-selection--multiple .select2-selection__choice,.select2-container--default[dir="rtl"] .select2-selection--multiple .select2-selection__placeholder,.select2-container--default[dir="rtl"] .select2-selection--multiple .select2-search--inline{float:right}.select2-container--default[dir="rtl"] .select2-selection--multiple .select2-selection__choice{margin-left:5px;margin-right:auto}.select2-container--default[dir="rtl"] .select2-selection--multiple .select2-selection__choice__remove{margin-left:2px;margin-right:auto}.select2-container--default.select2-container--focus .select2-selection--multiple{border:solid black 1px;outline:0}.select2-container--default.select2-container--disabled .select2-selection--multiple{background-color:#eee;cursor:default}.select2-container--default.select2-container--disabled .select2-selection__choice__remove{display:none}.select2-container--default.select2-container--open.select2-container--above .select2-selection--single,.select2-container--default.select2-container--open.select2-container--above .select2-selection--multiple{border-top-left-radius:0;border-top-right-radius:0}.select2-container--default.select2-container--open.select2-container--below .select2-selection--single,.select2-container--default.select2-container--open.select2-container--below .select2-selection--multiple{border-bottom-left-radius:0;border-bottom-right-radius:0}.select2-container--default .select2-search--dropdown .select2-search__field{border:1px solid #aaa}.select2-container--default .select2-search--inline .select2-search__field{background:transparent;border:none;outline:0;box-shadow:none;-webkit-appearance:textfield}.select2-container--default .select2-results>.select2-results__options{max-height:200px;overflow-y:auto}.select2-container--default .select2-results__option[role=group]{padding:0}.select2-container--default .select2-results__option[aria-disabled=true]{color:#999}.select2-container--default .select2-results__option[aria-selected=true]{background-color:#ddd}.select2-container--default .select2-results__option .select2-results__option{padding-left:1em}.select2-container--default .select2-results__option .select2-results__option .select2-results__group{padding-left:0}.select2-container--default .select2-results__option .select2-results__option .select2-results__option{margin-left:-1em;padding-left:2em}.select2-container--default .select2-results__option .select2-results__option .select2-results__option .select2-results__option{margin-left:-2em;padding-left:3em}.select2-container--default .select2-results__option .select2-results__option .select2-results__option .select2-results__option .select2-results__option{margin-left:-3em;padding-left:4em}.select2-container--default .select2-results__option .select2-results__option .select2-results__option .select2-results__option .select2-results__option .select2-results__option{margin-left:-4em;padding-left:5em}.select2-container--default .select2-results__option .select2-results__option .select2-results__option .select2-results__option .select2-results__option .select2-results__option .select2-results__option{margin-left:-5em;padding-left:6em}.select2-container--default .select2-results__option--highlighted[aria-selected]{background-color:#5897fb;color:white}.select2-container--default .select2-results__group{cursor:default;display:block;padding:6px}.select2-container--classic .select2-selection--single{background-color:#f7f7f7;border:1px solid #aaa;border-radius:4px;outline:0;background-image:-webkit-linear-gradient(top, #fff 50%, #eee 100%);background-image:-o-linear-gradient(top, #fff 50%, #eee 100%);background-image:linear-gradient(to bottom, #fff 50%, #eee 100%);background-repeat:repeat-x;filter:progid:DXImageTransform.Microsoft.gradient(startColorstr='#FFFFFFFF', endColorstr='#FFEEEEEE', GradientType=0)}.select2-container--classic .select2-selection--single:focus{border:1px solid #5897fb}.select2-container--classic .select2-selection--single .select2-selection__rendered{color:#444;line-height:28px}.select2-container--classic .select2-selection--single .select2-selection__clear{cursor:pointer;float:right;font-weight:bold;margin-right:10px}.select2-container--classic .select2-selection--single .select2-selection__placeholder{color:#999}.select2-container--classic .select2-selection--single .select2-selection__arrow{background-color:#ddd;border:none;border-left:1px solid #aaa;border-top-right-radius:4px;border-bottom-right-radius:4px;height:26px;position:absolute;top:1px;right:1px;width:20px;background-image:-webkit-linear-gradient(top, #eee 50%, #ccc 100%);background-image:-o-linear-gradient(top, #eee 50%, #ccc 100%);background-image:linear-gradient(to bottom, #eee 50%, #ccc 100%);background-repeat:repeat-x;filter:progid:DXImageTransform.Microsoft.gradient(startColorstr='#FFEEEEEE', endColorstr='#FFCCCCCC', GradientType=0)}.select2-container--classic .select2-selection--single .select2-selection__arrow b{border-color:#888 transparent transparent transparent;border-style:solid;border-width:5px 4px 0 4px;height:0;left:50%;margin-left:-4px;margin-top:-2px;position:absolute;top:50%;width:0}.select2-container--classic[dir="rtl"] .select2-selection--single .select2-selection__clear{float:left}.select2-container--classic[dir="rtl"] .select2-selection--single .select2-selection__arrow{border:none;border-right:1px solid #aaa;border-radius:0;border-top-left-radius:4px;border-bottom-left-radius:4px;left:1px;right:auto}.select2-container--classic.select2-container--open .select2-selection--single{border:1px solid #5897fb}.select2-container--classic.select2-container--open .select2-selection--single .select2-selection__arrow{background:transparent;border:none}.select2-container--classic.select2-container--open .select2-selection--single .select2-selection__arrow b{border-color:transparent transparent #888 transparent;border-width:0 4px 5px 4px}.select2-container--classic.select2-container--open.select2-container--above .select2-selection--single{border-top:none;border-top-left-radius:0;border-top-right-radius:0;background-image:-webkit-linear-gradient(top, #fff 0%, #eee 50%);background-image:-o-linear-gradient(top, #fff 0%, #eee 50%);background-image:linear-gradient(to bottom, #fff 0%, #eee 50%);background-repeat:repeat-x;filter:progid:DXImageTransform.Microsoft.gradient(startColorstr='#FFFFFFFF', endColorstr='#FFEEEEEE', GradientType=0)}.select2-container--classic.select2-container--open.select2-container--below .select2-selection--single{border-bottom:none;border-bottom-left-radius:0;border-bottom-right-radius:0;background-image:-webkit-linear-gradient(top, #eee 50%, #fff 100%);background-image:-o-linear-gradient(top, #eee 50%, #fff 100%);background-image:linear-gradient(to bottom, #eee 50%, #fff 100%);background-repeat:repeat-x;filter:progid:DXImageTransform.Microsoft.gradient(startColorstr='#FFEEEEEE', endColorstr='#FFFFFFFF', GradientType=0)}.select2-container--classic .select2-selection--multiple{background-color:white;border:1px solid #aaa;border-radius:4px;cursor:text;outline:0}.select2-container--classic .select2-selection--multiple:focus{border:1px solid #5897fb}.select2-container--classic .select2-selection--multiple .select2-selection__rendered{list-style:none;margin:0;padding:0 5px}.select2-container--classic .select2-selection--multiple .select2-selection__clear{display:none}.select2-container--classic .select2-selection--multiple .select2-selection__choice{background-color:#e4e4e4;border:1px solid #aaa;border-radius:4px;cursor:default;float:left;margin-right:5px;margin-top:5px;padding:0 5px}.select2-container--classic .select2-selection--multiple .select2-selection__choice__remove{color:#888;cursor:pointer;display:inline-block;font-weight:bold;margin-right:2px}.select2-container--classic .select2-selection--multiple .select2-selection__choice__remove:hover{color:#555}.select2-container--classic[dir="rtl"] .select2-selection--multiple .select2-selection__choice{float:right}.select2-container--classic[dir="rtl"] .select2-selection--multiple .select2-selection__choice{margin-left:5px;margin-right:auto}.select2-container--classic[dir="rtl"] .select2-selection--multiple .select2-selection__choice__remove{margin-left:2px;margin-right:auto}.select2-container--classic.select2-container--open .select2-selection--multiple{border:1px solid #5897fb}.select2-container--classic.select2-container--open.select2-container--above .select2-selection--multiple{border-top:none;border-top-left-radius:0;border-top-right-radius:0}.select2-container--classic.select2-container--open.select2-container--below .select2-selection--multiple{border-bottom:none;border-bottom-left-radius:0;border-bottom-right-radius:0}.select2-container--classic .select2-search--dropdown .select2-search__field{border:1px solid #aaa;outline:0}.select2-container--classic .select2-search--inline .select2-search__field{outline:0;box-shadow:none}.select2-container--classic .select2-dropdown{background-color:#fff;border:1px solid transparent}.select2-container--classic .select2-dropdown--above{border-bottom:none}.select2-container--classic .select2-dropdown--below{border-top:none}.select2-container--classic .select2-results>.select2-results__options{max-height:200px;overflow-y:auto}.select2-container--classic .select2-results__option[role=group]{padding:0}.select2-container--classic .select2-results__option[aria-disabled=true]{color:grey}.select2-container--classic .select2-results__option--highlighted[aria-selected]{background-color:#3875d7;color:#fff}.select2-container--classic .select2-results__group{cursor:default;display:block;padding:6px}.select2-container--classic.select2-container--open .select2-dropdown{border-color:#5897fb}

This comment has been minimized.

@laf

laf Dec 15, 2017

Member

Can you add the license and link to the project for this please to: https://github.com/librenms/librenms/blob/master/doc/General/Acknowledgement.md

@laf

laf Dec 15, 2017

Member

Can you add the license and link to the project for this please to: https://github.com/librenms/librenms/blob/master/doc/General/Acknowledgement.md

Show outdated Hide outdated html/includes/forms/save-host-dependency.inc.php Outdated
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 16, 2017

Member

I've done an intro video for this: https://www.youtube.com/watch?v=KMAarVS9QQ8

When starting out doing it, I realised that 'host' isn't standardised and we use device more (we still have some use of 'host') so I updated the references to this for the video. Patch is here: https://p.libren.ms/view/raw/c6a16990

Also moved: git mv html/pages/host-dependencies.inc.php html/pages/device-dependencies.inc.php

Member

laf commented Dec 16, 2017

I've done an intro video for this: https://www.youtube.com/watch?v=KMAarVS9QQ8

When starting out doing it, I realised that 'host' isn't standardised and we use device more (we still have some use of 'host') so I updated the references to this for the video. Patch is here: https://p.libren.ms/view/raw/c6a16990

Also moved: git mv html/pages/host-dependencies.inc.php html/pages/device-dependencies.inc.php

aldemira added some commits Dec 16, 2017

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Dec 16, 2017

Contributor

@laf added your patch and the bulkinsert. Great video btw.

Contributor

aldemira commented Dec 16, 2017

@laf added your patch and the bulkinsert. Great video btw.

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Dec 16, 2017

The inspection completed: No new issues

scrutinizer-notifier commented Dec 16, 2017

The inspection completed: No new issues

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Dec 18, 2017

Member

Do we need to use device dependencies to determine poller order? I could see a situation where we poll a child (especially a distant child) and the parent hasn't been polled yet. Alerts will not behave as expected here.

Member

murrant commented Dec 18, 2017

Do we need to use device dependencies to determine poller order? I could see a situation where we poll a child (especially a distant child) and the parent hasn't been polled yet. Alerts will not behave as expected here.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 18, 2017

Member

Do we need to use device dependencies to determine poller order? I could see a situation where we poll a child (especially a distant child) and the parent hasn't been polled yet. Alerts will not behave as expected here.

It would be helpful but can we easily work that out, how many levels down do you go before saying enough. I.e child device has 4 parents, one of those parents has 4 parents and so on. Also if someone accidentally sets the last parent to have a parent which is the first child.

I had thought that maybe in alerts we do an icmp check against the parents but would that slow it down too much. What about a daemon that ran doing icmp status checks all the time? We may be able to leverage fping for this.

For now I'm not sure this matters, the alerts will be suppressed once the parent host is down. Might just be worth a warning in the doc.

Member

laf commented Dec 18, 2017

Do we need to use device dependencies to determine poller order? I could see a situation where we poll a child (especially a distant child) and the parent hasn't been polled yet. Alerts will not behave as expected here.

It would be helpful but can we easily work that out, how many levels down do you go before saying enough. I.e child device has 4 parents, one of those parents has 4 parents and so on. Also if someone accidentally sets the last parent to have a parent which is the first child.

I had thought that maybe in alerts we do an icmp check against the parents but would that slow it down too much. What about a daemon that ran doing icmp status checks all the time? We may be able to leverage fping for this.

For now I'm not sure this matters, the alerts will be suppressed once the parent host is down. Might just be worth a warning in the doc.

@Rosiak

This comment has been minimized.

Show comment
Hide comment
@Rosiak

Rosiak Dec 19, 2017

Contributor

A note in the docs is sufficient for now I think.
We should aim to have this PR merged soon-ish, it's been alive for 3 months and a-lot of work from @aldemira .

Contributor

Rosiak commented Dec 19, 2017

A note in the docs is sufficient for now I think.
We should aim to have this PR merged soon-ish, it's been alive for 3 months and a-lot of work from @aldemira .

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 19, 2017

Member

I might just drop the travis requirements to merge this in. If @aldemira hasn't rebased tomorrow I'll do that tomorrow night.

Member

laf commented Dec 19, 2017

I might just drop the travis requirements to merge this in. If @aldemira hasn't rebased tomorrow I'll do that tomorrow night.

@laf

laf approved these changes Dec 20, 2017

@laf laf merged commit 56561aa into librenms:master Dec 20, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
license/cla Contributor License Agreement is signed.
Details

@laf laf added the Feature label Dec 20, 2017

@brettmarl

This comment has been minimized.

Show comment
Hide comment
@brettmarl

brettmarl Jan 9, 2018

we just started using this feature on our deployment - super excited. @aldemira - is there any way to get the dependencies list back as a property via the /devices/ API?

brettmarl commented Jan 9, 2018

we just started using this feature on our deployment - super excited. @aldemira - is there any way to get the dependencies list back as a property via the /devices/ API?

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Jan 9, 2018

Contributor

@brettmarl Sure I'll have a look!

Contributor

aldemira commented Jan 9, 2018

@brettmarl Sure I'll have a look!

@aldemira aldemira deleted the aldemira:hostdeps branch Jan 9, 2018

@olds463

This comment has been minimized.

Show comment
Hide comment
@olds463

olds463 Jan 10, 2018

@aldemira anyway to reflect in alert template when parent is down what the child devices of the parent is?

also when clicking "manage host dependencies" maybe adding option to clear child list? When you have a lot of devices its very time consuming to add relationships.

But thanks for this, it's very helpful.

olds463 commented Jan 10, 2018

@aldemira anyway to reflect in alert template when parent is down what the child devices of the parent is?

also when clicking "manage host dependencies" maybe adding option to clear child list? When you have a lot of devices its very time consuming to add relationships.

But thanks for this, it's very helpful.

@aldemira

This comment has been minimized.

Show comment
Hide comment
@aldemira

aldemira Jan 11, 2018

Contributor

@olds463 Do you mind posting this on https://community.librenms.org ? Let's discuss it in there, thanks.

Contributor

aldemira commented Jan 11, 2018

@olds463 Do you mind posting this on https://community.librenms.org ? Let's discuss it in there, thanks.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jan 14, 2018

Member

@brettmarl @aldemira has added this information now to the API for both list_device and get_device.

Member

laf commented Jan 14, 2018

@brettmarl @aldemira has added this information now to the API for both list_device and get_device.

@brettmarl

This comment has been minimized.

Show comment
Hide comment
@brettmarl

brettmarl Jan 14, 2018

brettmarl commented Jan 14, 2018

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.