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

Modified the PID being inserted to /data/<app>/shared/pids/passenger.<port>.pid #477

Open
wants to merge 1 commit into
base: next-release
Choose a base branch
from

Conversation

kvbautista
Copy link

@kvbautista kvbautista commented Jan 18, 2022

Description of your patch

This is a fix for the Passenger5 bug documented in FB-10179.

The script in /engineyard/bin/app_<app_name> has a bug. It leads to monit looking for the incorrect Passenger PID which results to all attemps at stop/restart failing.

Previous script logic was:

    NGINX_PID=`sudo netstat -antp | grep -w '<%= @port %>' | grep LISTEN | awk '{split($7,a,"/"); print a[1]}'`
    PASSENGER_PID=`sudo ps -ef | grep passenger-standalone | grep <%= @port %> | awk '{print $2};'`
    PIDFILE='/data/<%= @app_name %>/shared/pids/passenger.<%= @port %>.pid'
    if [ -n $NGINX_PID ]; then
      if [ ! -z $PASSENGER_PID ]; then
        if [ ! -f $PIDFILE ]; then
          echo $PASSENGER_PID > $PIDFILE
          echo "Passenger: PIDFILE $PIDFILE was recreated with PID $PASSENGER_PID"
        fi
      else
        echo "Passenger: Nginx is running with PID $NGINX_PID, but the standalone Passenger process is missing."
        echo "Passenger: Killing Nginx with PID $NGINX_PID"
        sudo kill -9 $NGINX_PID
      fi
    fi

Meaning that if the Passenger Nginx process is running and the Passenger process is running then the Passenger PID is put into the pidfile. Whereas on a usual start the Nginx PID would be put into the pidfile.

The suggested change to the script is to output $NGINX_PID to $PIDFILE instead of $PASSENGER_PID as shown below:

    NGINX_PID=`sudo netstat -antp | grep -w '<%= @port %>' | grep LISTEN | awk '{split($7,a,"/"); print a[1]}'`
    PASSENGER_PID=`sudo ps -ef | grep passenger-standalone | grep <%= @port %> | awk '{print $2};'`
    PIDFILE='/data/<%= @app_name %>/shared/pids/passenger.<%= @port %>.pid'
    if [ -n $NGINX_PID ]; then
      if [ ! -z $PASSENGER_PID ]; then
        if [ ! -f $PIDFILE ]; then
          echo $NGINX_PID > $PIDFILE
          echo "Passenger: PIDFILE $PIDFILE was recreated with PID $NGINX_PID"
        fi
      else
        echo "Passenger: Nginx is running with PID $NGINX_PID, but the standalone Passenger process is missing."
        echo "Passenger: Killing Nginx with PID $NGINX_PID"
        sudo kill -9 $NGINX_PID
      fi
    fi

Recommended Release Notes

Modified Passenger5 app control script to indicate Nginx PID instead of Passenger PID to resolve known issue in stop/restart attempts failures

Estimated risk

High. This changes the Passenger5 app control script. If this is broken then the application may fail to start.

Components involved

Passenger5 recipe

Description of testing done

See QA instructions - tests were done on a staging environment.

QA Instructions

Boot a v5 Passenger environment with 1 app instance (not Solo), then boot an additional application instance and check the PID in the pidfile to see which process it is monitoring.

Review the current behavior

  • Check the PID in the pidfile and do a cross-check on the running PIDs for nginx and Passenger by executing `ps -ef | grep passenger-standalone
  • It should show the PID for the Passenger /home/deploy/.passenger/support-binaries/5.1.8/PassengerAgent
  • All attempts at stop/start/restart will fail as the PID indicated in the pidfile is incorrect
  • If the PID in pidfile is showing the correct PID for nginx, do delete pidfile and immediately execute monit reload, this will recreate the PID file by rerunning the script in /engineyard/bin/
  • Observe if the following actions and their result is true (please do the previous step to refresh PID file for each item below):
    • monit stop all - Stops /home/deploy/.passenger/support-binaries/5.1.8/PassengerAgent process only (Kill nginx process to recover both processes)
    • monit restart all - Functions as intended and creates pidfile with correct PID
    • Delete PID file - Will be able to recreate the PID file but with incorrect PID

Update to the QA stack and verify that the bug is fixed

After the modification of the script, test the new behavior, following the same procedure as above:

  • Observe if the following actions and their result is true (please do the previous step to refresh PID file for each item below):
    • monit stop all - Stops both processes (Execute monit start all to recover)
    • monit restart all - Functions as intended and creates pidfile with correct PID
    • Delete PID file - Will be able to recreate the PID file and with the correct PID

Verify that previous functionality still works

Deployment

  • Run passenger-status as root. Note down the uptime of passenger
  • Execute a passenger start/restart. sudo monit start all -g <app> or sudo monit restart all -g <app>

Deploy

  • After deploy has finished, run passenger-status again. You should see the updated uptime.
  • Verify that Passenger is running and processing HTTP requests

…<port>/pid to resolve the issue of stop and restart attempts failure
@kvbautista kvbautista requested a review from mushyy January 18, 2022 08:30
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.

1 participant