newdevice: Added further QNAP Turbo NAS detection #5229 #5804

Merged
merged 3 commits into from Feb 9, 2017

Conversation

Projects
None yet
7 participants
@laf
Member

laf commented Feb 6, 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.

  • Have you signed the Contributors agreement - please do NOT submit a pull request unless you have (signing the agreement in the same pull request is fine). Your commit message for signing the agreement must appear as per the docs.
  • Have you followed our code guidelines?

Fixes: #5229

Adds a new option to os discover, it's a generic check (manufacturer name).

Chose to always have sysMfgName as regex compatible as I doubt people will search for the full thing here.

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Feb 6, 2017

Thank you for submitting a PR @laf! We have found the following @murrant, @jonathon-k and @Rosiak based on the history of these files to review this PR.

Thank you for submitting a PR @laf! We have found the following @murrant, @jonathon-k and @Rosiak based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
includes/functions.php
@@ -101,8 +101,9 @@ function getHostOS($device)
$sysDescr = snmp_get($device, "SNMPv2-MIB::sysDescr.0", "-Ovq");
$sysObjectId = snmp_get($device, "SNMPv2-MIB::sysObjectID.0", "-Ovqn");
+ $sysMfgName = snmp_get($device, "ENTITY-MIB::entPhysicalMfgName.1", "-Ovqn");

This comment has been minimized.

@murrant

murrant Feb 7, 2017

Member

Will this always be .1, can it be .0?

@murrant

murrant Feb 7, 2017

Member

Will this always be .1, can it be .0?

This comment has been minimized.

@laf

laf Feb 7, 2017

Member

Here yes, we also use it in siklu discovery but in other vendors no it can be quit varied, Cisco seems to be [0-9]!

@laf

laf Feb 7, 2017

Member

Here yes, we also use it in siklu discovery but in other vendors no it can be quit varied, Cisco seems to be [0-9]!

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Feb 7, 2017

Member

I'm unsure how I feel about this PR. Something like this seems to be required in this case, but often using manufacturer name is obvious but often the wrong thing. So I don't think we need to encourage it's use by making it easier.

I created a ticket with Qnap for the missing sysObjectID.

Member

murrant commented Feb 7, 2017

I'm unsure how I feel about this PR. Something like this seems to be required in this case, but often using manufacturer name is obvious but often the wrong thing. So I don't think we need to encourage it's use by making it easier.

I created a ticket with Qnap for the missing sysObjectID.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Feb 7, 2017

Member

I'm not happy either but it's the only way I could think of getting detection done without going back to doing a php file.

Member

laf commented Feb 7, 2017

I'm not happy either but it's the only way I could think of getting detection done without going back to doing a php file.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Feb 8, 2017

Member

I feel like I would prefer it as a php file for now. You don't have to remove the sysObjectId detection from the yaml file.

Member

murrant commented Feb 8, 2017

I feel like I would prefer it as a php file for now. You don't have to remove the sysObjectId detection from the yaml file.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Feb 9, 2017

Member

Updated

Member

laf commented Feb 9, 2017

Updated

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Feb 9, 2017

The inspection completed: No new issues

The inspection completed: No new issues

@laf laf merged commit 5e2ef21 into librenms:master Feb 9, 2017

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@laf laf deleted the laf:issue-5229 branch Feb 9, 2017

@filltr

This comment has been minimized.

Show comment
Hide comment
@filltr

filltr Feb 14, 2017

Plase add HDD temperatures.
HDD1 - 1.3.6.1.4.1.24681.1.2.11.1.3.1
HDD2 - 1.3.6.1.4.1.24681.1.2.11.1.3.2
HDD3 - 1.3.6.1.4.1.24681.1.2.11.1.3.3
HDD4 - 1.3.6.1.4.1.24681.1.2.11.1.3.4

filltr commented Feb 14, 2017

Plase add HDD temperatures.
HDD1 - 1.3.6.1.4.1.24681.1.2.11.1.3.1
HDD2 - 1.3.6.1.4.1.24681.1.2.11.1.3.2
HDD3 - 1.3.6.1.4.1.24681.1.2.11.1.3.3
HDD4 - 1.3.6.1.4.1.24681.1.2.11.1.3.4

@NK74

This comment has been minimized.

Show comment
Hide comment
@NK74

NK74 Feb 15, 2017

Contributor

Hi @filltr , it's already mentioned here #5229 (comment)

Contributor

NK74 commented Feb 15, 2017

Hi @filltr , it's already mentioned here #5229 (comment)

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