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

F5 gtm support #8161

Merged
merged 7 commits into from Feb 2, 2018

Conversation

Projects
None yet
4 participants
@dnapper
Contributor

dnapper commented Jan 29, 2018

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

Adds discovery and polling of F5 GTM module Wide IP and Pools discovery.

@laf

This comment has been minimized.

Member

laf commented Jan 29, 2018

Thanks for this. Before we review the code can you share some screenshots of what the various new pages look like please.

Could you also show the impact on poll time for your devices and indicate roughly how big they are? The f5 module already can have a big impact on users so we need to understand the risk to existing installs here.

@laf laf added the Needs-Info label Jan 29, 2018

@dnapper

This comment has been minimized.

Contributor

dnapper commented Jan 29, 2018

Sure.

In our environment we don't run GTM services on the same devices as we run LTM so for me it will have no impact on poll times on existing devices.

The code checks to see if the GTM module is running prior to discovering any new entities so I don't expect this will impact any devices that have the module disabled.

This adds two new tabs to the load balancer page if GTM objects are discovered.
gtm-newtabs

The pages look virtually the same as the current VS and Pool pages but the graphs are based on DNS requests handled not data throughput.
gtm-wideipdetail
gtm-wideipdetail

I'll leave these pre devices with this running for a few days so you can see what the data looks like (and that it's actually polling :) )

Edit:
Here is the detail with some polling
gtm-widealldetail

One thing i've been struggling with is how to handle representing a service that has multiple pools associated. In the end I decided that if I can sort out a way I can always add that later.

This is with a pre-prod devices with ~30 objects on it.

While on the main branch the discovery time and polling time is ~7s.
Discovered in 7.116 seconds
SNMP [45/5.64s]: Get[10/0.77s] Getnext[1/0.07s] Walk[34/4.80s]
MySQL [158/0.06s]: Cell[30/0.00s] Row[25/0.00s] Rows[70/0.01s] Column[1/0.00s] Update[26/0.03s] Insert[5/0.01s] Delete[1/0.00s]
RRD [0/0.00s]: Update[0/0.00s] Create [0/0.00s] Other[0/0.00s]

Polled in 7.028 seconds
UPDATED!

Start Alerts

End Alerts

SNMP [57/5.61s]: Get[19/1.41s] Getnext[0/0.00s] Walk[38/4.20s]
MySQL [63/0.07s]: Cell[3/0.00s] Row[2/0.00s] Rows[20/0.00s] Column[3/0.00s] Update[33/0.06s] Insert[2/0.00s] Delete[0/0.00s]
RRD [75/0.00s]: Update[75/0.00s] Create [0/0.00s] Other[0/0.00s]

With the new additions discovery takes ~10s
Discovered in 10.139 seconds

SNMP [49/8.12s]: Get[11/0.87s] Getnext[1/0.07s] Walk[37/7.17s]
MySQL [704/0.60s]: Cell[30/0.01s] Row[26/0.00s] Rows[73/0.01s] Column[1/0.00s] Update[86/0.09s] Insert[487/0.48s] Delete[1/0.00s]
RRD [0/0.00s]: Update[0/0.00s] Create [0/0.00s] Other[0/0.00s]

Polling takes ~8s

Polled in 8.377 seconds
UPDATED!

Start Alerts

End Alerts

SNMP [64/6.76s]: Get[19/1.40s] Getnext[0/0.00s] Walk[45/5.36s]
MySQL [65/0.07s]: Cell[3/0.00s] Row[2/0.00s] Rows[22/0.01s] Column[3/0.00s] Update[33/0.06s] Insert[2/0.00s] Delete[0/0.00s]
RRD [195/0.01s]: Update[135/0.00s] Create [60/0.00s] Other[0/0.00s]

@dnapper

This comment has been minimized.

Contributor

dnapper commented Jan 30, 2018

Update:
I found an issue where once I added an LTM to my test server the LTM discovery process was deleting the GTM objects. In order to fix it I split the GTM polling into it's own load balancer module and it appears to be working.

I'll leave it for a while to see if the problem comes back.

As a side note here is a comparison of LTM poling before and after the patch.
The polling time is quite a bit faster on my dev server but that's probably due to load.

DEV
polling-dev

PRE
polling-pre

dnapper added some commits Jan 31, 2018

@laf

Some in line comments that need looking at. Thanks again for submitting this. @dnapper

}
$components = $keep;
global $config;

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

You shouldn't need this.

global $config;
if (is_file('pages/device/loadbalancer/'.mres($vars['subtype']).'.inc.php')) {

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

You can drop the mres from these two lines, $vars has already had that applied.

This comment has been minimized.

@dnapper

dnapper Jan 31, 2018

Contributor

Do you want me to remove it from the ltm module pages as well?

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

No, leave that. You can remove them in another PR but let's not mix things up here.

global $config;
if (is_file('pages/device/loadbalancer/'.mres($vars['subtype']).'.inc.php')) {

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

You don't need to use mres on these two lines.

* null == timeout or something else that caused an error, OID may exist but we couldn't get it.
*/
if (!is_null($gtmWideIPEntry) || !is_null($gtmWideStatusEntry) || !is_null($gtmPoolEntry)) {

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

Don't these need to be && if you want them all to match?

This comment has been minimized.

@dnapper

dnapper Jan 31, 2018

Contributor

You don't need to have a pool attached to a wide IP for it to function so there could be situations where people are using the device to service static DNS records as well as dynamic ones based on pool conditions.

//Check if GTM modeul is running and run object discovery.
if ((snmp_get($device, 'sysModuleAllocationProvisionLevel.3.103.116.109', '-Ovqs', 'F5-BIGIP-SYSTEM-MIB')) !=none) {
$gtmWideIPEntry = snmpwalk_array_num($device, '1.3.6.1.4.1.3375.2.3.12.1.2.1', 0);

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

If we need all of these walks then it would be worth checking if each contains the data before trying the next and only carry on if we have them all (moving the check from below to here).

This comment has been minimized.

@dnapper

dnapper Jan 31, 2018

Contributor

Sure, there is no point moving onto look for status and pools if there are no wide IPs.
Do you want me to do the same in the LTM discovery? We could skip trying to get pools and status codes if there are no virtual servers objects.

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

Any saving is good, but again I'd keep it to another PR for LTM.

This comment has been minimized.

@dnapper

dnapper Jan 31, 2018

Contributor

Can do.

@@ -51,6 +51,13 @@
$ltmPoolMemberEntry = snmpwalk_array_num($device, '1.3.6.1.4.1.3375.2.2.5.3.2.1', 0);
$ltmPoolMbrStatusEntry = snmpwalk_array_num($device, '1.3.6.1.4.1.3375.2.2.5.6.2.1', 0);
//Check if GTM modeul is running and run object discovery.

This comment has been minimized.

@laf

laf Jan 31, 2018

Member

Is this supposed to be here?

This comment has been minimized.

@dnapper

dnapper Jan 31, 2018

Contributor

no... whoops.

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Feb 1, 2018

The inspection completed: No new issues

@laf

laf approved these changes Feb 1, 2018

@murrant

This comment has been minimized.

Member

murrant commented Feb 2, 2018

Looks good, thanks @dnapper :)

@murrant murrant merged commit a347e7c into librenms:master Feb 2, 2018

2 checks passed

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

@dnapper dnapper deleted the dnapper:f5-gtm-support branch Feb 2, 2018

inetAnt added a commit to criteo-forks/librenms that referenced this pull request Mar 19, 2018

F5 gtm support (librenms#8161)
* initial

* commited correct version of f5-ltm-inc.php

* cleaned up comments

* cleaned up formating for travis

* split GTM discovery from LTM discovery module

* removed blank line at EOF

* updated per comments
@lock

This comment has been minimized.

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.