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

Revamp of debug script with logic and colours #1565

Merged
merged 49 commits into from Jul 5, 2017

Conversation

Projects
None yet
8 participants
@jacobsalmela
Copy link
Member

jacobsalmela commented Jun 27, 2017

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

5

This is an attempt to make the debug log easier to read and to disseminate the information it contains--with the end goal being that users will be able to help themselves based on the output of the script. As much as we try to help every user who has a question, we're all volunteering our time, so if the user can run the debug script and self-diagnose the issue, not only will they learn something, but can hopefully fix any issue they may run into by following the URLs the script provides.

The script will:

  • display the same information to the user that can be found in the log file
  • display common information the developers need when assisting a user
  • display important information first
  • display information in a logical order
  • display any caution or warnings in yellow
  • display any errors in red
  • link to an FAQ if the script detects any warnings or errors
  • make the debug token highly-visible so users can clearly see they need to let us know what it

In addition to the revamp, detailed comments are included on nearly every line. Pi-hole is a great beginners project (especially in the RPi-realm) and we want Pi-hole to be a project people feel they can contribute to. To that end, each portion of code is explained so someone trying to learn about shell scripts can use our project as a springboard.

Intensive testing should be done on this before merging as the variety of systems and issues people may run into are difficult for one person to recreate.

And it's been a while since I've coded anything so forgive any rustiness you may find... and feel free to add any changes you see fit.

TODO

  • Strip off the color codes before uploading to tricorder since the colors show up like \e[0;36m some more information here, which makes it difficult to read once on the server
  • Add the disclaimer at the beginning of the debugger
  • Search the forums for more common issues to test for and create FAQs if necessary

This template was created based on the work of udemy-dl.

jacobsalmela added some commits May 20, 2017

@PromoFaux

This comment has been minimized.

Copy link
Member

PromoFaux commented on advanced/Scripts/piholeDebug.sh in 6fa00e7 May 23, 2017

Just a note on this bit, the if/else can probably be left out of debug script, as by the point the install script will have copied COL_TABLE into /opt/pihole. It's only in the install script for first-runs

jacobsalmela added some commits May 24, 2017

comment out dir checks for lighttpd, cronm and http as they have a lo…
…t of other files that need parsing through. May need to increase the logic there if this is information we really need to know.

@pi-hole pi-hole deleted a comment from codacy-bot Jun 29, 2017

@pi-hole pi-hole deleted a comment from codacy-bot Jun 29, 2017

@pi-hole pi-hole deleted a comment from codacy-bot Jun 29, 2017

@pi-hole pi-hole deleted a comment from codacy-bot Jun 29, 2017

@pi-hole pi-hole deleted a comment from codacy-bot Jun 29, 2017

PIHOLE_WILDCARD_CONFIG_FILE="${DNSMASQ_D_DIRECTORY}/03-wildcard.conf"

WEB_SERVER_CONFIG_FILE="${WEB_SERVER_CONFIG_DIRECTORY}/lighttpd.conf"
WEB_SERVER_CUSTOM_CONFIG_FILE="${WEB_SERVER_CONFIG_DIRECTORY}/external.conf"

This comment has been minimized.

@DL6ER

DL6ER Jul 2, 2017

Member

This variable doesn't seem to be used at all.

This comment has been minimized.

@jacobsalmela

jacobsalmela Jul 5, 2017

Author Member

Yeah, some are unused right now, but we may use them in the future.


# An array of operating system "pretty names" that we officialy support
# We can loop through the array at any time to see if it matches a value
SUPPORTED_OS=("Raspbian" "Ubuntu" "Fedora" "Debian" "CentOS")

This comment has been minimized.

@DL6ER

DL6ER Jul 2, 2017

Member

This variable doesn't seem to be used at all.

${BLOCK_PAGE_DIRECTORY})

# Store the required directories in an array so it can be parsed through
mapfile -t array <<< "$var"

This comment has been minimized.

@DL6ER

DL6ER Jul 2, 2017

Member

I don't see what this line is supposed to do here.

# Strip just the base name of the system using sed
the_os=$(echo ${os_to_check} | sed 's/ .*//')
# If the variable is one of our supported OSes,
case "${the_os}" in

This comment has been minimized.

@DL6ER

DL6ER Jul 2, 2017

Member

Ah, you probably want to use $SUPPORTED_OS here

This comment has been minimized.

@jacobsalmela

jacobsalmela Jul 5, 2017

Author Member

Yes, that is correct. Sorry for the confusion. It' a beastly script akin to the installer....

Merge branch 'development' of https://github.com/pi-hole/pi-hole into…
… revamp/debug

merge in development to avoid shellcheck from travis
OLD_IFS="$IFS"
# Store the distro info in an array and make it global since the OS won't change,
# but we'll keep it within the function for better unit testing
IFS=$'\r\n' command eval 'distro_info=( $(cat /etc/*release) )'
local search_term="Pi-hole"
elif [[ "${pihole_component}" == "Web" ]]; then
# We need to search for "AdminLTE" so store it in a variable as well
local search_term="AdminLTE"

This comment has been minimized.

is_os_supported() {
local os_to_check="${1}"
# Strip just the base name of the system using sed
the_os=$(echo ${os_to_check} | sed 's/ .*//')

This comment has been minimized.

${BLOCK_PAGE_DIRECTORY})

# Store the required directories in an array so it can be parsed through
mapfile -t array <<< "$var"

This comment has been minimized.

# IP address to check for
local ip_address="${2}"
# See what IP is in the setupVars.conf file
local setup_vars_ip=$(cat ${PIHOLE_SETUP_VARS_FILE} | grep IPV${protocol}_ADDRESS | cut -d '=' -f2)

This comment has been minimized.

PIHOLE_WILDCARD_CONFIG_FILE="${DNSMASQ_D_DIRECTORY}/03-wildcard.conf"

WEB_SERVER_CONFIG_FILE="${WEB_SERVER_CONFIG_DIRECTORY}/lighttpd.conf"
WEB_SERVER_CUSTOM_CONFIG_FILE="${WEB_SERVER_CONFIG_DIRECTORY}/external.conf"
TICK="[${COL_LIGHT_GREEN}${COL_NC}]"
CROSS="[${COL_LIGHT_RED}${COL_NC}]"
INFO="[i]"
DONE="${COL_LIGHT_GREEN} done!${COL_NC}"

This comment has been minimized.

PIHOLE_PROCESSES=( "dnsmasq" "lighttpd" "pihole-FTL" )

# Store the required directories in an array so it can be parsed through
REQUIRED_DIRECTORIES=(${CORE_GIT_DIRECTORY}

This comment has been minimized.

# Put the current Internal Field Separator into another variable so it can be restored later
OLD_IFS="$IFS"
# Get the lines that are in the file(s) and store them in an array for parsing later
IFS=$'\r\n' command eval 'file_info=( $(cat "${filename}") )'
CROSS="[${COL_LIGHT_RED}${COL_NC}]"
INFO="[i]"
DONE="${COL_LIGHT_GREEN} done!${COL_NC}"
OVER="\r\033[K"

This comment has been minimized.


# An array of operating system "pretty names" that we officialy support
# We can loop through the array at any time to see if it matches a value
SUPPORTED_OS=("Raspbian" "Ubuntu" "Fedora" "Debian" "CentOS")

This comment has been minimized.

@jacobsalmela jacobsalmela changed the title [WIP] Revamp of debug script with logic and colours Revamp of debug script with logic and colours Jul 5, 2017

@dschaper dschaper merged commit c3ed710 into development Jul 5, 2017

5 checks passed

codacy/pr Good work! A positive pull request.
Details
code-review/pullapprove Approved by dschaper, jacobsalmela
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@Mcat12 Mcat12 deleted the revamp/debug branch Jul 5, 2017

@pralor

This comment has been minimized.

Copy link

pralor commented Feb 27, 2018

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

https://discourse.pi-hole.net/t/pi-hole-v3-2-introduces-long-term-statistics-an-audit-log-colours-and-more/5772/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.