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

Issue #5077 - Treat Ubiquiti EdgePoint Switch models the same as EdgeSwitch #5079

Merged
merged 3 commits into from Dec 1, 2016

Conversation

Projects
None yet
4 participants
@twilley
Copy link
Contributor

twilley commented Nov 28, 2016

Please note

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

Treat Ubiquiti EdgePoint Switch models the same as EdgeSwitch and UnifiSwitch models when polling.

@LibreNMS-CI

This comment has been minimized.

Copy link

LibreNMS-CI commented Nov 28, 2016

Auto-Deploy finished, Test PR at http://5079.ci.librenms.org or https://5079.ci.librenms.org

@laf

This comment has been minimized.

Copy link
Member

laf commented Nov 29, 2016

I'm assuming that edgepoint switches run the same OS as edgeswitch?

We also need OS Test units adding please: http://docs.librenms.org/Developing/Support-New-OS/#os-test-units

@laf laf added the Blocker 🚫 label Nov 29, 2016

@twilley

This comment has been minimized.

Copy link
Contributor Author

twilley commented Nov 29, 2016

Yeah, they run the same OS. The EdgePoint Switch 16-Port and the EdgeSwitch 24-Port 250W use the same firmware image file too. I'll look at the OS Test units and get back to you on that.

@twilley

This comment has been minimized.

Copy link
Contributor Author

twilley commented Nov 30, 2016

It looks like the OS Unit Test is already in place for Edgeswitch OS:

from the tests/OSDiscoveryTest.php file:

    public function testEdgeswitch()
    {
        $this->checkOS('edgeswitch');
    }

When discovered, the Edgepoint Switch is correctly classified as running the edgeswitch OS (EdgePoint is just the branding Ubiquiti chose for their outdoor EdgeSwitches and EdgeRouters - Thanks for that, Ubiquiti), but when polled, the Hardware, Version, and Serial [Number] info was not being populated into the database.

When I run the pre-commit.php script, everything seems to be in order:

librenms@librenms:~$ ./scripts/pre-commit.php -u
Running unit tests... success
librenms@librenms:~$

This is my second time contributing, so if I'm missing anything else, I'm hoping you'll point me in the right direction.

I have the output from running addhost.php, discovery.php and poller.php as well as mysql data for the following query: select hardware,version,serial from librenms.devices where device_id=<ID of the Edgepoint>. For the poller output and mysql query, I have pre-patched and post-patched examples. I haven't included this info here yet, because it's quite verbose, and I'm not sure you even want it.

@laf

This comment has been minimized.

Copy link
Member

laf commented Nov 30, 2016

You can have more than one test unit per OS type, look at some of the others which list files like X1, X2, X3, etc.

Just add:

$this->checkOS('edgeswitch', 'edgeswitch1');

Then create a new file called edgeswitch1.snmprec and populate it with the required info.

@LibreNMS-CI

This comment has been minimized.

Copy link

LibreNMS-CI commented Dec 1, 2016

Auto-Deploy finished, Test PR at http://5079.ci.librenms.org or https://5079.ci.librenms.org

1 similar comment
@LibreNMS-CI

This comment has been minimized.

Copy link

LibreNMS-CI commented Dec 1, 2016

Auto-Deploy finished, Test PR at http://5079.ci.librenms.org or https://5079.ci.librenms.org

@twilley twilley force-pushed the twilley:5077 branch from 8f62579 to da25b50 Dec 1, 2016

@LibreNMS-CI

This comment has been minimized.

Copy link

LibreNMS-CI commented Dec 1, 2016

Auto-Deploy finished, Test PR at http://5079.ci.librenms.org or https://5079.ci.librenms.org

Add OS Discovery Test units for Edgeswitch 24-Port 250W and EdgePoint…
… Switch 16-port devices (running 1.6.0 firmware only)
@LibreNMS-CI

This comment has been minimized.

Copy link

LibreNMS-CI commented Dec 1, 2016

Auto-Deploy finished, Test PR at http://5079.ci.librenms.org or https://5079.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Copy link

LibreNMS-CI commented Dec 1, 2016

Auto-Deploy finished, Test PR at http://5079.ci.librenms.org or https://5079.ci.librenms.org

@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented Dec 1, 2016

The inspection completed: No new issues

@twilley

This comment has been minimized.

Copy link
Contributor Author

twilley commented Dec 1, 2016

OK, it's making more sense now.

I've added two tests: one for the EdgePoint 16-port Switch, and one for the EdgeSwitch 24-Port 250W.

Is it preferable to keep the version info, etc in the snmprec file, or to remove it? I see varying levels of specificity in the other snmprec files.

For now, I have included all the info for the 1.3.6.1.2.1.1.1.0 OID:

1.3.6.1.2.1.1.1.0|4|EdgePoint Switch 16-Port, 1.6.0.4900860, Linux 3.6.5-f4a26ed5, 1.0.0.4857129

@laf laf added Device 🖥 and removed Blocker 🚫 labels Dec 1, 2016

@laf

This comment has been minimized.

Copy link
Member

laf commented Dec 1, 2016

image

@laf laf merged commit 694c3bc into librenms:master Dec 1, 2016

2 checks passed

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

VimCommando added a commit to VimCommando/librenms that referenced this pull request Jan 4, 2017

@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.