Cacti / plugin_thold

Thold Plugin for Cacti
GNU General Public License v2.0
61 stars 60 forks source link

QA: Upgrade to thold 1.8 just went bad #602

Closed arno-st closed 11 months ago

arno-st commented 12 months ago

I just upgrade thold to the latest, 1.8 (from 1.7) with the following script: `

!/bin/bash

git clone https://github.com/Cacti/plugin_thold.git /bin/cp -rpf plugin_thold/* thold /bin/rm -rf plugin_thold chown -R apache:apache thold ` And at first I got this error: 31/07/2023 17:02:39 - PLUGIN WARNING: Plugin 'thold' is attempting to call 'api_plugin_register_hook' improperly in function 'thold_setup_database'

And now I got plenty of page with this: 31/07/2023 17:03:10 - CMDPHP PHP ERROR WARNING Backtrace: (/poller.php[777]:process_poller_output(), /lib/poller.php[706]:process_poller_output(), /lib/poller.php[687]:api_plugin_hook_function(), /lib/plugins.php[149]:api_plugin_run_plugin_hook_function(), /lib/plugins.php[270]:thold_poller_output(), /plugins/thold/includes/polling.php[165]:thold_build_cdef(), /plugins/thold/thold_functions.php[4205]:strpos(), CactiErrorHandler()) 31/07/2023 17:03:10 - ERROR PHP WARNING in Plugin 'thold': strpos(): Empty needle in file: /usr/share/cacti/plugins/thold/thold_functions.php on line: 4205

missing something in the line: if (strpos($cdef['value'], '') !== false) { $cdef['value'] = str_replace('CURRENT_DS_MINIMUM_VALUE', get_current_value($local_data_id, 'rrd_minimum'), $cdef['value']); $found = true; } or it's a copy of the line 4200

Yea that solve the problem, remove this 4 lines

TheWitness commented 12 months ago

Plugin is under heavy changes, but slowing down. Should be pretty stable today...

anarkia1976 commented 12 months ago

@TheWitness we can merge latest patches or we need to wait another cycle of testing.

TheWitness commented 12 months ago

I'll fix it this morning.

TheWitness commented 12 months ago

Okay, fixing this is going to get you a new set of warnings now. Post them once I have the patch in, and then let's look at the Data Source to figure out what to do next.

TheWitness commented 12 months ago

Okay, done. Now test before I go back to sleep. The pace of Thold changes kind of burned me out.

TheWitness commented 12 months ago

Next round of changes are going to be more significant. So, need to take a deep breath.

TheWitness commented 12 months ago

Keep testing. However, the following are not yet implemented

Also, the baseline deviation for average can not go far enough in the past. Was a hack I put in due to writers block, since lifted obviously.

I'll fix that bug next as it won't break anything major, just make it work better. Will be adding a column too for the fix.

arno-st commented 12 months ago

So now I have an error on line 4206 of the same file, and same error than before! and empty needle on strpos:

03/08/2023 07:04:10 - ERROR PHP WARNING in Plugin 'thold': strpos(): Empty needle in file: /usr/share/cacti/plugins/thold/thold_functions.php on line: 4206 03/08/2023 07:04:10 - CMDPHP PHP ERROR WARNING Backtrace: (/poller.php[777]:process_poller_output(), /lib/poller.php[687]:api_plugin_hook_function(), /lib/plugins.php[149]:api_plugin_run_plugin_hook_function(), /lib/plugins.php[270]:thold_poller_output(), /plugins/thold/includes/polling.php[165]:thold_build_cdef(), /plugins/thold/thold_functions.php[4206]:strpos(), CactiErrorHandler())

and the code is: if (strpos($cdef['value'], '') !== false) { $cdef['value'] = str_replace('CURRENT_DS_MINIMUM_VALUE', get_current_value($local_data_id, 'rrd_minimum'), $cdef['value']); $found = true; }

TheWitness commented 12 months ago

Very interesting. Going to convert to a warning so I can trace this back to the bug.

TheWitness commented 12 months ago

Holy crap batman! Just reviewing this one. Fixing now.

TheWitness commented 12 months ago

Fixed now.

arno-st commented 12 months ago

Ok technically it's fixed, but why did you have 2 times the same test line 4201 and 4206


                                        if (strpos($cdef['value'], 'CURRENT_DS_MINIMUM_VALUE') !== false) {
                                                $cdef['value'] = str_replace('CURRENT_DS_MINIMUM_VALUE', get_current_value($local_data_id, 'rrd_minimum'), $cdef['value']);
                                                $found = true;
                                        }

                                        if (strpos($cdef['value'], 'CURRENT_DS_MINIMUM_VALUE') !== false) {
                                                $cdef['value'] = str_replace('CURRENT_DS_MINIMUM_VALUE', get_current_value($local_data_id, 'rrd_minimum'), $cdef['value']);
                                                $found = true;
                                        }
TheWitness commented 12 months ago

Oh, FFS, let me check that. Focusing on baseline now. If you are testing, you need to roll the version back in plugin_config to 1.7 and upgrade the schema again.

TheWitness commented 12 months ago

Thanks for you help guys. Remember, there will be more changes. To apply them, if you run this database command:

UPDATE plugin_config SET version='1.7' WHERE directory='thold';

And then goto the Thold page, your schema will pick up the latest changes.

arno-st commented 12 months ago

You close it to fast, or I need to create a new issue: 04/08/2023 18:16:26 - ERROR PHP WARNING in Plugin 'thold': A non-numeric value encountered in file: /usr/share/cacti/plugins/thold/thold_functions.php on line: 4627 04/08/2023 18:16:26 - CMDPHP PHP ERROR WARNING Backtrace: (/plugins/thold/poller_thold.php[96]:perform_thold_processes(), /plugins/thold/poller_thold.php[139]:thold_check_all_thresholds(), /plugins/thold/includes/polling.php[299]:thold_check_threshold(), /plugins/thold/thold_functions.php[2664]:thold_check_baseline(), /plugins/thold/thold_functions.php[4627]:CactiErrorHandler())

TheWitness commented 12 months ago

New issue.

TheWitness commented 11 months ago

Test with the latest update and let me know if it's fixed now. The error, if it's on the right line, is indicative of the poller_interval variable not being initialized.

arno-st commented 11 months ago

Hello, so other error 07/08/2023 08:11:36 - ERROR PHP WARNING in Plugin 'thold': A non-numeric value encountered in file: /usr/share/cacti/plugins/thold/thold_functions.php on line: 4876 07/08/2023 08:11:36 - CMDPHP PHP ERROR WARNING Backtrace: (/plugins/thold/poller_thold.php[96]:perform_thold_processes(), /plugins/thold/poller_thold.php[139]:thold_check_all_thresholds(), /plugins/thold/includes/polling.php[299]:thold_check_threshold(), /plugins/thold/thold_functions.php[2699]:thold_check_baseline(), /plugins/thold/thold_functions.php[4876]:CactiErrorHandler()) 07/08/2023 08:11:36 - CMDPHP ERROR: A DB Exec Failed!, Error: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'thold_fail_count = '8412' thold_warning_fail_count = 0 WHERE id = '10307'' at line 1 07/08/2023 08:11:36 - CMDPHP SQL Backtrace: (/plugins/thold/poller_thold.php[96]:perform_thold_processes(), /plugins/thold/poller_thold.php[139]:thold_check_all_thresholds(), /plugins/thold/includes/polling.php[299]:thold_check_threshold(), /plugins/thold/thold_functions.php[2347]:db_execute_prepared())

TheWitness commented 11 months ago

So much fun. Thanks for keeping up the testing.

TheWitness commented 11 months ago

Okay, more updates ready for testing.

arno-st commented 11 months ago

so just when I disable/enable the plugin I got this: 08/08/2023 07:31:58 - PLUGIN WARNING: Plugin 'thold' is attempting to call 'api_plugin_register_hook' improperly in function 'thold_setup_database' And then sorry same that before: 08/08/2023 07:33:28 - CMDPHP ERROR: A DB Exec Failed!, Error: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'thold_fail_count = '8412' thold_warning_fail_count = 0 WHERE id = '10307'' at line 1 08/08/2023 07:33:28 - CMDPHP SQL Backtrace: (/plugins/thold/poller_thold.php[96]:perform_thold_processes(), /plugins/thold/poller_thold.php[139]:thold_check_all_thresholds(), /plugins/thold/includes/polling.php[299]:thold_check_threshold(), /plugins/thold/thold_functions.php[2347]:db_execute_prepared())

`

I'm wondering if it's not the single quote who give the trouble

TheWitness commented 11 months ago

I believe this is fixed in Cacti 1.2.25. We've been delaying it's release until we can get the windows installer in better shape.

TheWitness commented 11 months ago

Ope, but that SQL error. Got to fix that.

TheWitness commented 11 months ago

Missing a comma.

TheWitness commented 11 months ago

Okay, fixed now.

arno-st commented 11 months ago

ok it's fixed this time thanks

TheWitness commented 11 months ago

Keep testing from the CHANGELOG if you can spare the time.