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

Adjust logs for containerized and non-containerized services #7043

Merged
merged 26 commits into from
Jul 7, 2022

Conversation

nqb
Copy link
Contributor

@nqb nqb commented Jun 28, 2022

Description

  • rsyslog will look into systemd-journal to get logs for containerized and non-containerized services
  • disable docker logs for all containerized-services: everything is in journald
  • adjust rsyslog rules for services (old, new, containerized)
  • remove management of permissions on log directory and files using fixpermissions:
    • log directory is installed by packaging
    • rsyslog will create/update files inside log directory (owned by root and group pf)
    • logrotate will handle log rotation
    • pfcmd checkup ensure rights are correct on log files
  • innobackup.log is handle separately

Issues

fixes #3835

Impacts

  • rsyslog
  • logrotate
  • permissions on logs/ and log files

Delete branch after merge

YES

NEWS file entries

Enhancements

  • Adjus logs for containerized and non-containerized services (#FIXME)

UPGRADE file entries

=== Log files names updated

The name of some log files have changed. You can find a list below:

.Mapping between old and new log files
|===
|Service |Old log file(s) |New log file(s)

|MariaDB
|mariadb_error.log
|mariadb.log

|httpd.aaa (Apache requests)
|httpd.aaa.access and httpd.aaa.error
|httpd.apache

|httpd.collector (Apache requests)
|httpd.collector.log and httpd.collector.error
|httpd.apache

|httpd.portal (Apache requests)
|httpd.portal.access, httpd.portal.error, httpd.portal.catalyst
|httpd.apache

|httpd.proxy (Apache requests)
|httpd.proxy.error and httpd.proxy.access
|httpd.apache

|httpd.webservices (Apache requests)
|httpd.webservices.error and httpd.webservices.access
|httpd.apache

|api-frontend (Apache requests)
|httpd.api-frontend.access 
|httpd.apache

|HAProxy (all services)
|/var/log/syslog or /var/log/messages
|haproxy.log


If you use a Syslog Forwarder to send log files to a remote Syslog server, you should run following script:

[code,bash]
----
/usr/local/pf/addons/upgrade/to-12.0-rename-log-files.pl
----

@nqb nqb requested a review from JeGoi June 28, 2022 12:56
@nqb nqb added this to the PacketFence-12.0 milestone Jun 28, 2022
@nqb
Copy link
Contributor Author

nqb commented Jun 28, 2022

I ran following pipeline to start testing with new packages: https://gitlab.com/inverse-inc/packetfence/-/pipelines/575065134

Copy link
Contributor

@JeGoi JeGoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks fine for me.
Questions answered.

@nqb nqb changed the title Adjus logs for containerized and non-containerized services Adjust logs for containerized and non-containerized services Jun 30, 2022
@nqb nqb force-pushed the feature/log-containers branch 3 times, most recently from 6ddc228 to 7ec9bed Compare June 30, 2022 11:42
@nqb
Copy link
Contributor Author

nqb commented Jun 30, 2022

I finished to fix some issues. Running a new pipeline (https://gitlab.com/inverse-inc/packetfence/-/pipelines/576942480) to test new packages.

@nqb nqb force-pushed the feature/log-containers branch 2 times, most recently from 9182c8b to a2d2b22 Compare July 4, 2022 16:04
@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

Previously, it was possible to forward or live view /var/log/syslog or /var/log/messages (depending on OS). Currently, it doesn't work due to #7049 (comment).

Also, it would mean we will need to expose /var/log inside api-frontend container. Not sure this is what we want.

@nqb nqb force-pushed the feature/log-containers branch 3 times, most recently from cc8e64d to 7967c8c Compare July 5, 2022 10:28
@julsemaan
Copy link
Collaborator

Previously, it was possible to forward or live view /var/log/syslog or /var/log/messages (depending on OS). Currently, it doesn't work due to #7049 (comment).

Also, it would mean we will need to expose /var/log inside api-frontend container. Not sure this is what we want.

I don't think we need to live view the syslog file anymore as long as we make sure all the PF services log in files in /usr/local/pf/logs. Historically, some services like keepalived and haproxy didn't log there and that's why that file can be viewed in the log viewer.

We could also mount this directory in read-only on the api-frontend container

@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

I added a new rule to redirect [GIN] messages (related to fingerbank-collector) into fingerbank.log. They previously go into /var/log/{syslog,messages}. However, there is still some messages related to fingerbank-collector which go into /var/log/{syslog,messages} but they are not easy to distinguish from others.

Example:

Jul 05 10:53:34 pfdeb11dev bash[17072]: export PORT=4723
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_ENDPOINTS_CACHE_PATH=/usr/local/fingerbank/db/collector_endpoints_cache.db
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_DB_PERSISTENCE_INTERVAL=60s
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_IP_MAPS_DB_PATH=/usr/local/fingerbank/db/collector_ip_maps.db
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_CLUSTERED=true
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_CLUSTER_RESYNC_INTERVAL=120s
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_CONTACT_PEERS=
Jul 05 10:53:34 pfdeb11dev bash[17072]: export GOGC=100
Jul 05 10:53:34 pfdeb11dev bash[17072]: export COLLECTOR_DELETE_INACTIVE_ENDPOINTS=168h

I don't think it's critical to have this ones in fingerbank.log but I remembered to see some "monitoring" messages related to bash scripts coming from fingerbank-collector in /var/log/{syslog,messages}.

@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

@julsemaan, I'm talking about:

Jul  5 11:55:35 pfdeb11dev bash[1315]: Failed check cb8f0450-e1b7-11e7-80c1-9a214cf093ae:w
Jul  5 11:55:35 pfdeb11dev bash[1315]: The collector doesn't seem to be running alongside PacketFence.

@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

Unless the /var/log/{syslog,messages] decision, nothing more to add on my side. Tested on Debian and EL systems

@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

As discussed internally, I exposed /var/log directory inside api-frontend Docker container in read-only. It's hard to expose only /var/log/messages or /var/log/syslog at the moment before we handle properly os_detection inside containers.

Please review and merge if everything is good for you.

@julsemaan
Copy link
Collaborator

@nqb, do you think we could have an upgrade note we can add that would state what changed to where. Perhaps not for all the smaller services but like saying mariadb_error.log is in mariadb.log looks important enough to be included

@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

@nqb, do you think we could have an upgrade note we can add that would state what changed to where. Perhaps not for all the smaller services but like saying mariadb_error.log is in mariadb.log looks important enough to be included

100% agree, will do that tomorrow morning first thing.

@nqb
Copy link
Contributor Author

nqb commented Jul 5, 2022

In fact, I already add an upgrade note in PR description but it can be improved.

@nqb
Copy link
Contributor Author

nqb commented Jul 6, 2022

Upgrade note updated and I also push a small fix to avoid keeping old httpd.*.access,error,catalyst files after upgrade.

@nqb
Copy link
Contributor Author

nqb commented Jul 7, 2022

@JeGoi and @julsemaan, could you merge if all is fine ?
Thanks!

Copy link
Contributor

@JeGoi JeGoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow looks great!

@JeGoi JeGoi merged commit c9b7547 into devel Jul 7, 2022
@JeGoi JeGoi deleted the feature/log-containers branch July 7, 2022 11:59
JeGoi added a commit that referenced this pull request Jul 7, 2022
Also add script to remove tenant and option to use proxysql
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fixpermissions: change rights on logs dir
3 participants