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

Automation discovery : New devices added by automation discovery have empty SNMP community field #1300

Closed
g1augusto opened this issue Feb 2, 2018 · 20 comments

Comments

@g1augusto
Copy link

g1augusto commented Feb 2, 2018

Hi Everyone,

Since last version update I noticed that for Automation discovery all our newly added devices discovered by a scan are added to CACTI but without the SNMP community

Before it was correctly adding the SNMP community from the Automation discovery SNMP Option matching SNMP community

@netniV
Copy link
Member

netniV commented Feb 2, 2018

In your network discovery, is it using the correct option set?

@g1augusto
Copy link
Author

Yes, the option were not changed and I had these discoveries working well before the 1.1.33 update

@netniV
Copy link
Member

netniV commented Feb 2, 2018

What version were you on prior to the 1.1.33 update?

@g1augusto
Copy link
Author

1.1.28

@netniV
Copy link
Member

netniV commented Feb 2, 2018

OK, I'll check out the differences between the two release tags later today and see if anything is likely to impact on this issue.

@g1augusto
Copy link
Author

thanks a lot, let me know if I can help with the testing anyway

@netniV
Copy link
Member

netniV commented Feb 2, 2018

So I can see a lot of changes to the automation to move from snmp_readstring to snmp_community. I think it may be related to this.

@netniV
Copy link
Member

netniV commented Feb 2, 2018

One thing, can you make sure that your automation tables are right. Do a SHOW CREATE TABLE tablename \G against each fo them and paste here.

@g1augusto
Copy link
Author

If it's OK with you I ran DESCRIBE instead (let me know if you need the other command):

automation_tables.txt

@netniV
Copy link
Member

netniV commented Feb 2, 2018

OK, I believe that I have found the problem. It's in lib/api_automation.php and it assigns automation_snmp_items.snmp_readstring to automation_devices.snmp_community

This is wrong on two levels because for starters, snmp_readstring on automation_snmp_items is now snmp_community; and because snmp_community is just community on automation_devices;

I think if you look at automation_devices, you'll find that community is not populated for your devices.

@g1augusto
Copy link
Author

TBH the automation automatically adds the devices to CACTI in my installation, so I found out looking at new devices with missing SNMP community, but I guess it's the same as you are looking at.

Funny is that the device is queried with the right SNMP community and then it's not saved with any community, as you pointed out.

@netniV
Copy link
Member

netniV commented Feb 2, 2018

Do this SQL query:

select community, count(*) from automation_devices;

@g1augusto
Copy link
Author

image

@netniV
Copy link
Member

netniV commented Feb 2, 2018

So they all have a community. OK, let me check the code again to see if that is what I expected 👍

@netniV
Copy link
Member

netniV commented Feb 2, 2018

Actually first, lets correct the SQL statement. I was surprised by the result so I thought I'd test it against my own data and realised I've missed off the group by statement.

mysql> select community, count(*) from automation_devices;
+-----------+----------+
| community | count(*) |
+-----------+----------+
|           |      542 |
+-----------+----------+
1 row in set (0.00 sec)

mysql> select community, count(*) from automation_devices group by community;
+-----------+----------+
| community | count(*) |
+-----------+----------+
|           |       83 |
| <name>    |      459 |
+-----------+----------+
2 rows in set (0.00 sec)

Try using that second SQL statement and let me know results. Also, if you know the name of the device that you are looking at as an example, replace <name> with what you are looking for

select id, hostname, community from automation_devices where hostname like '%<name>%'

@g1augusto
Copy link
Author

g1augusto commented Feb 2, 2018

To run this test I deleted one of the devices that was in CACTI, created a test automation network to discover it and when I ran it. I can see that the community is in the automation_devices table entry

image

Then I purged the discovery automation_devices table, I modified the automation discovery test adding the option : Automatically Add to Cacti and when I ran again the discovery I could see that the device added to cacti is missing the Community.

Seems that the issue is on the portion of code that creates a device in cacti

Also I can see the following logs into that specific poller logs :

02/Feb/2018 21:13:51 - CMDPHP PHP ERROR NOTICE Backtrace: (/poller_automation.php: 345 discoverDevices)(/poller_automation.php: 612 automation_add_device)(/lib/api_automation.php: 2656 CactiErrorHandler)(/lib/functions.php: 4482 cacti_debug_backtrace)

02/Feb/2018 21:13:51 - ERROR PHP NOTICE: Undefined index: community in file: /var/www/html/cacti/lib/api_automation.php on line: 2656

@netniV
Copy link
Member

netniV commented Feb 2, 2018

I looked at that section of code. But with your backtrace, this makes things a bit easier. It depends on whether $device is from the host table or the automation_devices table. I really think we should make a mod to rename the automation_devices table to be inline with every other table's snmp options and I'm hoping @cigamit would agree with that assessment.

@netniV
Copy link
Member

netniV commented Feb 2, 2018

The poller_automation.php is setting everything as snmp_community, so is automation_valid_snmp_device() in api_automation.php. The problem is that (also in api_automation.php) is automation_add_device that is using community not snmp_community.

Now, if the $device variable comes from row directly from automation_devices, that would have that column. However, poller_automation never sets it thus you get a blank version.

This furthers my resolve that we should be renaming that column to avoid conflict/confusion.

@cigamit
Copy link
Member

cigamit commented Feb 3, 2018

Totally agree with the renaming. I've been trying to get some of the older plugins (especially thold) to use proper schema naming for ever. The update in 1.1.32 attempted to handle all the snmp stuff in the core Cacti (excluding username and password due to numerous user scripts using those columns directly). It appears however, that we missed this one in the automation table/files, which of course leads to errors.

As developers, we try to minimize variation so that we can get into the habit of assuming things when writing code, and then of course, as a consequence, we don't catch the fact that assumption is the mother of all screw ups until regression testing. And as a small open source project, we often times skip that part of the release process.

Either way, we are making great progress here.

@cigamit cigamit changed the title [1.1.33] Automation discovery : New devices added by automation discovery have empty SNMP community field Automation discovery : New devices added by automation discovery have empty SNMP community field Feb 3, 2018
cigamit added a commit that referenced this issue Feb 3, 2018
- Automation discovery : New devices added by automation discovery have
empty SNMP community field.
- Correct two missing i18n issues
- Convert htmlspecialchars() use to html_escape() instead
@cigamit
Copy link
Member

cigamit commented Feb 3, 2018

Resolved. Thanks for reporting.

@cigamit cigamit closed this as completed Feb 3, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants