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

Changes in piholeStatus and FTL API calls. #2126

Merged
merged 2 commits into from
Mar 10, 2022
Merged

Changes in piholeStatus and FTL API calls. #2126

merged 2 commits into from
Mar 10, 2022

Conversation

rdwebdesign
Copy link
Member

@rdwebdesign rdwebdesign commented Feb 16, 2022

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)
  • I have Signed Off all commits. (git commit --signoff)

What does this PR aim to accomplish?:

Attempts to minimize the side effects seen in pi-hole/pi-hole#4615.
In addition, this PR adds changes to socket-related functions. Also, some global variables were replace by constants.

How does this PR accomplish the above?:

The piholeStatus() function is called very frequently to assure the dashboard shows the correct status.
The original code uses shell command to retrieve the value (generating a lot of logs).
The new function uses a socket to call FTL directly.

The API is still the same. A few inner changes were introduced.

What documentation changes (if any) are needed to support this PR?:

None (no API calls were changed).

@rdwebdesign rdwebdesign changed the title Test ftl api Changes in piholeStatus and FTL API calls. Feb 16, 2022
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-spamming-auth-log/53637/4

scripts/pi-hole/php/FTL.php Outdated Show resolved Hide resolved
scripts/pi-hole/php/FTL.php Outdated Show resolved Hide resolved
scripts/pi-hole/php/FTL.php Outdated Show resolved Hide resolved
scripts/pi-hole/php/FTL.php Outdated Show resolved Hide resolved
scripts/pi-hole/php/FTL.php Outdated Show resolved Hide resolved
scripts/pi-hole/php/FTL.php Outdated Show resolved Hide resolved
scripts/pi-hole/php/func.php Outdated Show resolved Hide resolved
@rdwebdesign rdwebdesign requested review from yubiuser and a team and removed request for yubiuser February 19, 2022 00:19
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Not sure why, but if you disable via web interface and reload the page the "TOP" statistics will fail to load
Bildschirmfoto zu 2022-02-28 19-51-03

@rdwebdesign
Copy link
Member Author

I'm not sure those 2 errors are related (maybe).

Are you sure the Chrome error and PHP warnings (logs) happened at the same time?

- Using constants for default values
- Remove unnecessary variable
- Replace `piholeStatusAPI()` with a call to FTL API
- Transfer `api.php?status` code from `api.php` to `api_FTL.php`
- Change `piholeStatus()` to use FTL info (no need to use `pihole`).

Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
Signed-off-by: RD WebDesign <github@rdwebdesign.com.br>
@yubiuser
Copy link
Member

After undoing the changes to index.js the errors did not re-occur.

@janwiesemann
Copy link

Are there any news as to when we can except this to be included in a release?

@jfb-pihole
Copy link
Member

Pi-hole has a small team of developers (all of whom have regular jobs outside of Pi-hole). We do not have scheduled release dates and cannot predict accurately when the next release will occur.

@janwiesemann
Copy link

janwiesemann commented Apr 6, 2022

Pi-hole has a small team of developers (all of whom have regular jobs outside of Pi-hole). We do not have scheduled release dates and cannot predict accurately when the next release will occur.

That's what I thought. Still tanks for your reply.

This is fixed in the current Version of Pi-hole. There is no need to use the following fix.

Here is the Fix I'm using (for Debian based systems):

  1. Edit /etc/rsyslog.confby using your favourite editor. Remember to use one you can actually close (i.e. not vim)!

    nano /etc/rsyslog.conf
    
  2. find the line

    auth,authpriv.*                  /var/log/auth.log
    
  3. change it to

    auth,authpriv.warn              /var/log/auth.log
    

    For anyone who is interested and does not know what it does:
    rsyslog the the application that actually handles the writing of system logs. It uses filters to differentiate sources and to redirect them to a specified log file. In this case we are changing the filter for authpriv.*. Everything after the . will be used to filter out different message levels I.e. errors, warning, informations,... The related message dispatched using the notice option. By changing * (any) to warn (warnings, errors, ...) we are filtering our any messages using the notice, info or debug flag.

  4. reload rsyslog by executing

    systemctl restart rsyslog.service
    

How to revert the changes:

you should probably change it back after we receive an update (v. > 5.11). I'm actually not sure what else is tied to this 'fix'. I'm running pihole inside a LXC container and so it does not affect any of my other systems.

  1. edit rsyslog.conf

    nano /etc/rsyslog.conf
    
  2. change this

    auth,authpriv.warn              /var/log/auth.log
    
  3. back to this

    auth,authpriv.*                  /var/log/auth.log
    
  4. reload rsyslog by executing

    systemctl restart rsyslog.service
    

@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-15-web-v5-12-and-core-v5-10-released/54987/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants