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

Add Laravel to LibreNMS #8318

Merged
merged 59 commits into from May 9, 2018

Conversation

Projects
None yet
3 participants
@murrant
Copy link
Member

murrant commented Mar 2, 2018

Intercepts all routes and falls back to existing pages/api.

Discovery/Poller should continue to work, if not updating via daily.sh, you will need to run:

composer install --no-dev

.env will be created with some base settings. Settings in .env are generally settings we need to start the application. If you can think of anything else that should be in .env, please leave a note so we can add it before we merge this. When running composer install, we only create .env once, so updating in the future will likely be manual.

If you have permissions issues, you should get a message telling you how to fix it.

Visit any page or /laravel. Test API.

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 8318

@murrant murrant added the Blocker 🚫 label Mar 2, 2018

@laf

This comment has been minimized.

Copy link
Member

laf commented Mar 19, 2018

composer install --no-dev

This would happen with ./daily.sh though right?

setfacl -R -m g::rwx rrd/ logs/ storage/ bootstrap/cache/
setfacl -d -m g::rwx rrd/ logs/ storage/ bootstrap/cache/

Only needed for webui so at least we don't break polling.

By chance. I get this error:

[2018-03-19 22:11:11] production.ERROR: Symfony\Component\Debug\Exception\FatalThrowableError: [] operator not supported for strings in /opt/librenms/includes/defaults.inc.php:124

I had $config['transit_descr'] = 'boom'; set for testing. This does indicate that config.php can cause the webui to break now if the config values deviate from what we expect.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Mar 19, 2018

Probably different error settings.

Thanks for testing. I'm not sure we can fix the folder permissions without manual intervention.

@laf

This comment has been minimized.

Copy link
Member

laf commented Mar 20, 2018

Thanks for testing. I'm not sure we can fix the folder permissions without manual intervention.

Yeah possibly although it doesn't matter too much if it doesn't break poller/discovery and only affects the webui.

@murrant murrant force-pushed the murrant:laravel branch from 34af827 to 3a458a3 Mar 20, 2018

@murrant murrant referenced this pull request Mar 22, 2018

Closed

PostgreSQL Support #619

@murrant murrant force-pushed the murrant:laravel branch 2 times, most recently from 1249f0f to 026b5d3 Mar 22, 2018

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 2, 2018

Tag me when this needs testing.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 2, 2018

@laf I believe you can test it now. I've been working on replicating the LibreNMS (v1) layout so we could blend new and old pages. Mostly that is to see what issues we have in the future. Right now I am stuck on some database queries, I may just take the easy way out on those for now.

Accessing existing pages and migrating from existing set up is working as well as I can get it for now. There may still be permissions issues, but we try to avoid those and we should give messages on how to manually fix those. I can't remember if I tested what happens if git pull is done without composer install --no-dev

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 3, 2018

The web pages seem to load ok. I've not tested anything on the API yet.

I did manage to break it but I'm not sure how. You can see the second time I re-ran daily.sh, I ended up with no .env and some errors but it worked after resetting again. Will see if I can replicate it.

-bash-4.2$ git checkout laravel
gBranch 'laravel' set up to track remote branch 'laravel' from 'murrant'.
Switched to a new branch 'laravel'
-bash-4.2$ git pull murrant laravel
git From github.com:murrant/librenms
 * branch                laravel    -> FETCH_HEAD
Already up to date.
-bash-4.2$ git reset --hard 97d58cc1214e68253220b1bc06678d7af449fa40
HEAD is now at 97d58cc12 fix:: Fixed display of WD drives for smart application (#8471)
-bash-4.2$ vi config.php
-bash-4.2$ ./daily.sh
Updating to latest codebase                        OK
Updating Composer packages                         OK
Updated from 97d58cc12 to 427c9b291                OK
Updating SQL-Schema                                OK
Updating submodules                                OK
Cleaning up DB                                     OK
Fetching notifications                             OK
Caching PeeringDB data                             OK
-bash-4.2$ git checkout^C
-bash-4.2$ vi config.php
-bash-4.2$ vi .env
.env          .env.example
-bash-4.2$ vi .env
-bash-4.2$ rm .env
-bash-4.2$ git reset --hard 97d58cc1214e68253220b1bc06678d7af449fa40
HEAD is now at 97d58cc12 fix:: Fixed display of WD drives for smart application (#8471)
-bash-4.2$ ./daily.sh
PHP Fatal error:  Cannot redeclare array_sort() (previously declared in /opt/librenms/vendor/laravel/framework/src/Illuminate/Support/helpers.php:287) in /opt/librenms/includes/functions.php on line 69
PHP Stack trace:
PHP   1. {main}() /opt/librenms/daily.php:0
PHP   2. require() /opt/librenms/daily.php:14
PHP Fatal error:  Cannot redeclare array_sort() (previously declared in /opt/librenms/vendor/laravel/framework/src/Illuminate/Support/helpers.php:287) in /opt/librenms/includes/functions.php on line 69
PHP Stack trace:
PHP   1. {main}() /opt/librenms/daily.php:0
PHP   2. require() /opt/librenms/daily.php:14
Updating Composer packages                         OK
Updated from unset to                              OK
Updating SQL-Schema                                OK
Updating submodules                                OK
Cleaning up DB                                     OK
Fetching notifications                             OK
Caching PeeringDB data                             OK
-bash-4.2$ vi .en
-bash-4.2$ vi .en^C
-bash-4.2$
-bash-4.2$
-bash-4.2$ ll .en^C
-bash-4.2$ ./daily.sh
Updating to latest codebase                       ^C
-bash-4.2$ git reset --hard 97d58cc1214e68253220b1bc06678d7af449fa40
HEAD is now at 97d58cc12 fix:: Fixed display of WD drives for smart application (#8471)
-bash-4.2$ ./daily.sh
Updating to latest codebase                        OK
Updating Composer packages                         OK
Updated from 97d58cc12 to 427c9b291                OK
Updating SQL-Schema                                OK
Updating submodules                                OK
Cleaning up DB                                     OK
Fetching notifications                             OK
Caching PeeringDB data                             OK
-bash-4.2$ cat .env

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 3, 2018

API looks good as well

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 3, 2018

So I can reliably replicate the issue with daily.sh but not when starting from a clean, i.e no .env setup so it probably won't impact anyone.

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 3, 2018

We will need to update the install docs to install php72w-mysqlnd instead of php72w-mysql otherwise you get:

[2018-04-03 13:40:37] production.ERROR: Error in config.php!
mysqli_connect(): Headers and client library minor version mismatch. Headers:50556 Library:100130

@murrant murrant force-pushed the murrant:laravel branch from 9c8b5cc to f2c765a Apr 4, 2018

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 4, 2018

@laf Odd about the php72w-mysqlnd, not sure why you would need that...

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 4, 2018

@laf After some research are you sure that isn't an OS level issue? Did LibreNMS without this PR merged work ok?

@murrant murrant force-pushed the murrant:laravel branch from e8cb223 to 9599aee Apr 10, 2018

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 10, 2018

@laf After some research are you sure that isn't an OS level issue? Did LibreNMS without this PR merged work ok?

It works both with and without this PR, it's the warning that gets printed everywhere which drew me to it.

Seems mysqlnd is the recommended approach anyway: https://secure.php.net/manual/en/mysqlnd.overview.php

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 10, 2018

Probably update the install docs then. We might have to add a FAQ for it.

@murrant murrant force-pushed the murrant:laravel branch from 5e8fc2c to e8bbec8 Apr 12, 2018

@murrant murrant removed the Blocker 🚫 label Apr 13, 2018

@laf laf added the Needs Testing label Apr 13, 2018

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 15, 2018

So I now get a 500 with this when not running mysqlnd

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 15, 2018

Disco and polling is fine. Tested quite a lot of the webui with no issues.

Just need to test the api now.

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 15, 2018

API seems good as well.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 16, 2018

Not sure how to handle the mysqlnd, we could maybe make it ignore it somehow, but I can't reproduce.

Laravel is set to show all messages by default.

If you set APP_DEBUG=true in your .env you will get more info. Speaking of which, we could probably use some sort of doc updates.

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 21, 2018

Not sure how to handle the mysqlnd, we could maybe make it ignore it somehow, but I can't reproduce.

Laravel is set to show all messages by default.

If you set APP_DEBUG=true in your .env you will get more info. Speaking of which, we could probably use some sort of doc updates.

I had debug on which is how I found it. Without it it just showed a 500 generic error.

I doubt I'll be the only one caught out by this, it's just simple yum install for php/mysql from the repos we use in our docs.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 22, 2018

I'm saying it's not a problem with Laravel per-se. Just that Laravel reports all errors. You were probably getting an error before and it is suppressed.

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 22, 2018

I'm saying it's not a problem with Laravel per-se. Just that Laravel reports all errors. You were probably getting an error before and it is suppressed.

I know that, however this PR causes a 500 without any other changes which shouldn't happen. Luckily it doesn't affect the poller but still.

@murrant murrant force-pushed the murrant:laravel branch from 5ba15cd to d1d3800 Apr 27, 2018

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 27, 2018

Nice catch @laf. You should look at the fix, you might get a chuckle.

@laf

This comment has been minimized.

Copy link
Member

laf commented Apr 27, 2018

Nice catch @laf. You should look at the fix, you might get a chuckle.

It's always the simple things :)

@murrant murrant added WebUI Feature and removed Blocker 🚫 labels Apr 30, 2018

murrant and others added some commits May 1, 2018

Add SELinux configuration steps
More detailed permissions check.
Check all and give complete corrective commands in one step.
@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented May 8, 2018

The inspection completed: 96 new issues, 237 updated code elements

@laf

This comment has been minimized.

Copy link
Member

laf commented May 9, 2018

@murrant Good to go now?

@murrant

This comment has been minimized.

Copy link
Member

murrant commented May 9, 2018

Good to go and I'm up for the day :-)

@murrant murrant removed the Needs Testing label May 9, 2018

@murrant murrant merged commit 1ad7f31 into librenms:master May 9, 2018

2 checks passed

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

@laf laf deleted the murrant:laravel branch May 9, 2018

TheMysteriousX added a commit to TheMysteriousX/librenms that referenced this pull request May 20, 2018

Add Laravel to LibreNMS (librenms#8318)
* Add Laravel to LibreNMS.

* Try to set permissions during initial install and first composer update to Laravel.

* Fix composer.lock
Fix missing db config keys

* Start building v1 layout
Port ajax_setresolution, inject csrf into jquery ajax calls
Layout works, building menu
Partially done.

* Fix device group list
remove stupid count relationships

* Print messages for common boot errors.
Don't log to laravel.log file.
Log to error_log until booted, then librenms.log

* Fix up some issues with Config loading
Start of custom directives

* Custom blade directives: config, notconfig, admin

* Preflight checks
Only load config files once.

* Update the composer.lock for php 5.6

* Menu through routing

* Start of alert menu

* Better alert scopes

* reduce cruft in models

* Alerting menu more or less working :D

* Fix style

* Improved preflight

* Fix chicken-eggs!

* Remove examples

* Better alert_rule status queries
Debugbar

* fix app.env check

* User Menu

* Settings bar (dropped refresh)
Search JS

* Toastr messages

* Rename preflight

* Use hasAccess(User) on most models.
Add port counts

* Missed a Preflight -> Checks rename

* Fix some formatting

* Boot Eloquent outside of Laravel
Use Eloquent for Config and Plugins so we don't have to connect with dbFacile inside Laravel.
Move locate_binary() into Config class

* Config WIP

* Try to fix a lot of config loading issues.

* Improve menu for non-admins removing unneeded menus
url() for all in menu

* Only use eloquent if it exists

* Include APP_URL in initial .env settings

* Implement Legacy User Provider

* Helper class for using Eloquent outside of Laravel.
Allows access to DB style queries too and checking the connection status.

* Fix up tests

* Fix device groups query

* Checking Travis

* copy config.test.php earlier

* dbFacile check config before connecting
Don't use exception to check if eloquent is connected, it gets grabbed by the exception handler.
Ignore missing config.php error.

* Fix config load with database is not migrated yet.

* Remove Config::load() from early boot.

* Use laravel config settings to init db (this prefers .env settings)
Fix bgp vars not set in menu
add _ide_helper.php to .gitignore

* Restrict dependencies to versions that support php 5.6

* Update ConfigTest

* Fix a couple of installation issues

* Add unique NODE_ID to .env

* Correct handling of title image

* Fix database config not loading. Thanks @laf

* Don't prepend /

* add class_exists checks for development service providers

* Fix config value casting

* Don't use functions that may not exist

* Update dbFacile.php

* d_echo may not be defined when Config used called.

* Add SELinux configuration steps
More detailed permissions check.
Check all and give complete corrective commands in one step.

* Ignore node_modules directory

* Re-add accidetal removal

@lock lock bot locked as resolved and limited conversation to collaborators Jul 8, 2018

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