Discoverying and polling vrf #Issue280 #2820

Merged
merged 47 commits into from Feb 15, 2016

Projects

None yet

6 participants

@HenocKA
Contributor
HenocKA commented Jan 20, 2016

Trying again to add vrf-life adaptation, made by @nicearma.

On this pull request, there is no visual, only code for discoverying and polling vrf-lite :

sql-schema
adding vrf_lite_cisco table, and context name columns for some tables
mibs for work with VRF-Lite
CISCO-BRIDGE-DOMAIN-MIB and CISCO-CONTEXT-MAPPING-MIB 
discovery :
arp-table to work with vrf-lite context (sql changes and foreach for search with the device context array)
bgp-peers for work with vrf-lite context (sql changes and foreach for search with the device context array)
function for work with vrf-lite (sql changes and foreach for search with the device context array)
ipv4-addresses for work with vrf-lite (sql changes and foreach for search with the device context array)
ipv6-addresses for work with vrf-lite (sql changes and foreach for search with the device context array)
polling :
bgp-peers change for work with vrf-lite context and change to librenms logic (no more `` for execute the code, the code now the librenms function for this) and serveral logic fix
ospf change for work with vrf-lite context and change to librenms logic and serveral logic fix
other modifications :
snmp add context if SNMP v3 and if the device have the key context_name (very important!! for work with vrf-lite context)
poller add the vrf-lite array to the device
default add discovery module to the $config array

I will check for rebase regularly. Thanks for reading

HenocKA added some commits Jan 20, 2016
@HenocKA HenocKA Adding sql-schema 255eb9c
@HenocKA HenocKA Adding mibs a7c88be
@HenocKA HenocKA discoverying and polling VRF 831940c
@HenocKA HenocKA Adding sql-schema 286dcdc
@HenocKA HenocKA merging
5d9c6eb
@laf laf commented on an outdated diff Jan 24, 2016
includes/defaults.inc.php
@@ -734,6 +736,7 @@ function set_debug($debug) {
$config['discovery_modules']['cisco-mac-accounting'] = 1;
$config['discovery_modules']['cisco-pw'] = 1;
$config['discovery_modules']['cisco-vrf'] = 1;
+$config['discovery_modules']['cisco-vrf-lite'] = 1;
@laf
laf Jan 24, 2016 Member

Can you align this config item with the others please.

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/arp-table.inc.php
@@ -4,78 +4,90 @@
echo 'ARP Table : ';
-$ipNetToMedia_data = snmp_walk($device, 'ipNetToMediaPhysAddress', '-Oq', 'IP-MIB');
-$ipNetToMedia_data = str_replace('ipNetToMediaPhysAddress.', '', trim($ipNetToMedia_data));
-$ipNetToMedia_data = str_replace('IP-MIB::', '', trim($ipNetToMedia_data));
+if( key_exists('vrf_lite_cisco', $device) && (count($device['vrf_lite_cisco'])!=0) ){
+ $vrfs_lite_cisco = $device['vrf_lite_cisco'];
+}
+else{
@laf
laf Jan 24, 2016 Member

Needs a space after the else so:

else {

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/arp-table.inc.php
- dbUpdate(array('mac_address' => $clean_mac), 'ipv4_mac', 'port_id=? AND ipv4_address=?', array($interface['port_id'], $ip));
- echo '.';
- }
- else if (isset($interface['port_id'])) {
- echo '+';
- // echo("Add MAC $mac\n");
- $insert_data = array(
- 'port_id' => $interface['port_id'],
- 'mac_address' => $clean_mac,
- 'ipv4_address' => $ip,
- );
+ dbUpdate(array('mac_address' => $clean_mac), 'ipv4_mac', 'port_id=? AND ipv4_address=? AND `context_name`= ?', array($interface['port_id'], $ip, $device['context_name']));
+ echo '.';
+ }
+ else if (isset($interface['port_id'])) {
@laf
laf Jan 24, 2016 Member

Can you update this to:

elseif () {

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/bgp-peers.inc.php
@@ -3,232 +3,246 @@
if ($config['enable_bgp']) {
// Discover BGP peers
echo 'BGP Sessions : ';
-
+
+ if( key_exists('vrf_lite_cisco', $device) && (count($device['vrf_lite_cisco'])!=0) ){
+ $vrfs_lite_cisco = $device['vrf_lite_cisco'];
+ }
+ else{
@laf
laf Jan 24, 2016 Member

Same as earlier, just needs to be else {

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/cisco-vrf-lite.inc.php
+
+ //get all vrf_lite_cisco, this will used where the value depend of the context, be careful with the order that you call this module, if the module is disabled the context search will not work
+ $tmpVrfC = dbFetchRows("SELECT * FROM vrf_lite_cisco WHERE device_id = ? ", array(
+ $device ['device_id']));
+
+ $device['vrf_lite_cisco'] = $tmpVrfC;
+
+ //Delete all vrf that chaged
+ foreach ($tmpVrfC as $vrfC) {
+ unset($ids[$vrfC['vrf_lite_cisco_id']]);
+ }
+ if (!empty($ids)) {
+ foreach ($ids as $id) {
+
+ dbDelete('vrf_lite_cisco', 'vrf_lite_cisco_id = ? ', array(
+ $id));
@laf
laf Jan 24, 2016 Member

Can you put this on the line before?

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/cisco-vrf-lite.inc.php
+ $tableVrf[$tmpVrf[0]]['vrf_name'] = $tmpVrf[1];
+ }
+ }
+
+ unset($listVrf);
+
+ $listIntance = snmp_walk($device, "cContextMappingProtoInstName", "-Osq -Ln", $mib, NULL);
+ $listIntance = str_replace("cContextMappingProtoInstName.", "", $listIntance);
+ $listIntance = str_replace('"', "", $listIntance);
+ $listIntance = trim($listIntance);
+
+ if ($debug) {
+ echo ("\n[DEBUG]\nUsing $mib\n[/DEBUG]\n");
+ echo ("\n[DEBUG]\n List Intance only names\n$listIntance\n[/DEBUG]\n");
+ }
+
@laf
laf Jan 24, 2016 Member

You've got a lot of redundant empty lines here, can you clean them down to single lines please? This goes for the rest of the file.

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/ipv4-addresses.inc.php
@@ -1,58 +1,68 @@
<?php
echo 'IPv4 Addresses : ';
+if( key_exists('vrf_lite_cisco', $device) && (count($device['vrf_lite_cisco'])!=0) ){
+ $vrfs_lite_cisco = $device['vrf_lite_cisco'];
+}
+else{
@laf
laf Jan 24, 2016 Member

else {

:)

@laf laf commented on an outdated diff Jan 24, 2016
includes/discovery/ipv6-addresses.inc.php
@@ -1,78 +1,89 @@
<?php
echo 'IPv6 Addresses : ';
+if( key_exists('vrf_lite_cisco', $device) && (count($device['vrf_lite_cisco'])!=0) ){
+ $vrfs_lite_cisco = $device['vrf_lite_cisco'];
+}
+else{
@laf
laf Jan 24, 2016 Member

else {

@laf
Member
laf commented Jan 24, 2016

I've made some inline comments here. Once those have been updated then you have my 👍

Whilst I can't test vrf lite, I can confirm this doesn't break anything Cisco wise.

@HenocKA HenocKA Syntax update
1a21c51
@laf
Member
laf commented Jan 26, 2016

Close.

It actually looks like you've not defined a $config['poller_modules']['vrf_lite_cisco']. you have one for discovery but not polling. It appears you've used:

$config['enable_vrf_lite_cisco'] = 1;

You will need to update this to use poller_module config.

Also the sql-schema is now conflicting. You've got things a bit muddled up there so you'll need to revert those changes. I'd suggest using 097.sql (I know 95 and 96 don't exist but we have two other PRs before this with schema updates so it makes sense to merge in order).

@HenocKA
Contributor
HenocKA commented Jan 26, 2016

I agree with you, but vrf-lite-cisco is a discovery module not a poller module. It is reason why

@laf
Member
laf commented Jan 26, 2016

That's fine then :)

One thing I have found is it has duplicated all of our bgp sessions in the DB. Clearing them out and a re-discover leaves just single items but that needs to be looked into further please.

@HenocKA
Contributor
HenocKA commented Jan 27, 2016

For the duplication, it’s a normal thing.
In our configuration, for a specific case, we use 2 snmp context for 1 vrf.
In this case, we can collect two time the information for bgp processus.
So if we introduce a limitation : « this solution is single context », there is no problem.

@laf
Member
laf commented Jan 27, 2016

I wouldn't say that's a normal thing. Under normal running of your patch it seems fine, however going from without the patch to with it created duplicated bgp entries in bgpPeers table which certainly isn't normal.

@HenocKA HenocKA bug correction on ospf polling module
47c830a
@HenocKA
Contributor
HenocKA commented Jan 28, 2016

I speek about this case, it's the same for you ?

mysql> select * from bgpPeers where device_id=5;
+------------+-----------+-----------+-------------------+-----------------+--------------+--------------------+--------------+-------------------+------------------+-------------------+------------------------+-------------------------+---------------------------+----------------------------+--------------+
| bgpPeer_id | device_id | astext | bgpPeerIdentifier | bgpPeerRemoteAs | bgpPeerState | bgpPeerAdminStatus | bgpLocalAddr | bgpPeerRemoteAddr | bgpPeerInUpdates | bgpPeerOutUpdates | bgpPeerInTotalMessages | bgpPeerOutTotalMessages | bgpPeerFsmEstablishedTime | bgpPeerInUpdateElapsedTime | context_name |
+------------+-----------+-----------+-------------------+-----------------+--------------+--------------------+--------------+-------------------+------------------+-------------------+------------------------+-------------------------+---------------------------+----------------------------+--------------+
| 6 | 5 | | IP A | 65001 | established | start | IP C | | 4 | 6 | 0 | 0 | 10340861 | 0 | Contex1 |
| 7 | 5 | | IP B | 65002 | established | start | IP D | | 5 | 3 | 0 | 0 | 10339747 | 0 | Contex2 |
| 8 | 5 | | IP A | 65001 | established | start | IP C | | 4 | 6 | 0 | 0 | 10340861 | 0 | Contex1 |
| 9 | 5 | | IP B | 65002 | established | start | ÏP D | | 5 | 3 | 0 | 0 | 10339747 | 0 | Contex2 |
+------------+-----------+--- -------+-------------------+-----------------+--------------+--------------------+--------------+-------------------+------------------+-------------------+------------------------+-------------------------+---------------------------+----------------------------+--------------+
4 rows in set (0.00 sec)

You can see 2 doublons, because we pool for context1 and context2

@SaaldjorMike SaaldjorMike removed the Conflict label Jan 28, 2016
@laf
Member
laf commented Jan 30, 2016

No, we aren't using vrfs here. These are just pure bgp sessions seen on some asrs. Adding this patch caused them all to be duplicated with context as NULL.

Deleting all bgpPeers entries and it's rediscovers the correct number but we can't roll this out and ask everyone to delete the bgpPeers table.

@f0o
Member
f0o commented Jan 31, 2016

As @laf seems to have a blocker here, I'd like to release the schema-number in favour for #2881
Any thoughts @librenms/reviewers ?

@f0o f0o added the Blocker label Jan 31, 2016
@laf
Member
laf commented Jan 31, 2016

Go for it.

@HenocKA
Contributor
HenocKA commented Feb 2, 2016

We work for Deleting or remplacing all bgpPeers entries on equipement when we discovring context.

HenocKA added some commits Feb 2, 2016
@HenocKA HenocKA Merge remote-tracking branch 'refs/remotes/librenms/master' into issu…
…e280
1890cfb
@HenocKA HenocKA Correct data duplication on discovering cisco vrf lite 8a3d14f
@HenocKA HenocKA merging 6de58f5
@HenocKA HenocKA update Authors
cfee15f
@HenocKA HenocKA update branch
7a00d91
@f0o
Member
f0o commented Feb 7, 2016

Schema 97 is still being used in this PR....

@HenocKA HenocKA merging
24df0a0
@laf
Member
laf commented Feb 9, 2016

If you can update your sql-schema file to 101.sql then we should be good for a merge.

@HenocKA HenocKA merging - update schema
21e37c2
@laf
Member
laf commented Feb 10, 2016

Re-started scrut check.

@HenocKA
Contributor
HenocKA commented Feb 10, 2016

@laf I don't understand your last message

@laf
Member
laf commented Feb 10, 2016

@HenocKA Scrutinizer which does code check didn't run, I've restarted it that's all.

@HenocKA
Contributor
HenocKA commented Feb 10, 2016

Thanks, i fix this

@HenocKA HenocKA bug correction
6a51746
@laf laf added Polling Discovery and removed Blocker labels Feb 10, 2016
@laf
Member
laf commented Feb 11, 2016

👍 from me.

@librenms/reviewers tagging. Merge in 24 hours.

@laf
Member
laf commented Feb 12, 2016

Really sorry but this has a merge conflict because of the bgp-peers update we did yesterday. Can you rebase and push an update. We can merge asap at that stage.

@HenocKA HenocKA merging
0161260
@HenocKA
Contributor
HenocKA commented Feb 14, 2016

I update the bgp-peers :)

Le ven. 12 févr. 2016 16:20, Neil Lathwood notifications@github.com a
écrit :

Really sorry but this has a merge conflict because of the bgp-peers update
we did yesterday. Can you rebase and push an update. We can merge asap at
that stage.


Reply to this email directly or view it on GitHub
#2820 (comment).

@laf laf merged commit a572bc4 into librenms:master Feb 15, 2016

2 checks passed

Auto-Deploy Build finished. No test results found.
Details
Scrutinizer 21 new issues
Details
@nicearma
Contributor

@HenocKA Congrats

@HenocKA
Contributor
HenocKA commented Feb 15, 2016

Thanks @nicearma !
Tu as fait du bon boulot. Merci

@laf
Member
laf commented Feb 16, 2016

FYI we've had to revert some of the code in includes/polling/bgp-peers.inc.php as it was affecting junos ipv6 peers.

#3011 and #3010

@laf
Member
laf commented Feb 17, 2016

We've had to do another revert #3028 from this PR.

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