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

Reorganise h's config files #8386

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Reorganise h's config files #8386

merged 1 commit into from
Jan 2, 2024

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Dec 23, 2023

This commit reorganises h's conf/ directory following a schema that I plan to apply consistently to our cookiecutter and the rest of our apps. This is gonna make it easier to apply the cookiecutter to h:

conf/
  alembic.ini           # Alembic config for all environments.

  supervisord.conf      # Supervisor config for QA/staging and production environments.
  gunicorn.conf.py      # Gunicorn config for QA/staging and production environments.
  production.ini        # Pyramid config for QA/staging and production environments.

  supervisord-dev.conf  # Supervisor config for development environment.
  gunicorn-dev.conf.py  # Gunicorn config for development environment.
  development.ini       # Pyramid config for development environment.

h's separate websocket app

Unlike our other repos the h repo actually contains two separate Pyramid apps: there's a separate Pyramid app for the websocket and it has its own set of config files. This is honestly quite a mess (especially when you get to the additional "monolithic" mode below) but it has to do with the un-simple way that the separate websocket app has been split out within the same repo, rather than to do with this PR:

conf/
  gunicorn-websocket.conf.py      # Gunicorn config for websocket (production).
  websocket.ini                   # Pyramid config for websocket (production).

  gunicorn-websocket-dev.conf.py  # Gunicorn config for websocket (dev).
  websocket-dev.ini               # Pyramid config for websocket (dev).

The websocket's "monolithic" mode

The websocket app also has a "monolithic" mode that's used by make run-docker in development and by the Canada production environment (this is not used by make dev nor by the QA/staging or US production environments). This has to have yet another set of separate config files:

conf/
  gunicorn-websocket-monolithic.conf.py
  websocket-monolithic.ini

Other additional config files in h

Finally, h has two more config files that our other apps don't have:

  1. conf/nginx.conf: the config file for h's "internal" NGINX instance in production. (h has no NGINX in development.)
  2. conf/websocket-newrelic.ini: a custom New Relic configuration used by the websocket.

@seanh seanh requested a review from marcospri December 23, 2023 20:03
Comment on lines -25 to -32
[server:main]
use: egg:gunicorn#main
host: 0.0.0.0
port: 5000
proc_name: web
graceful_timeout: 0
timeout: 0
errorlog: -
Copy link
Contributor Author

@seanh seanh Dec 23, 2023

Choose a reason for hiding this comment

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

Renamed development-app.ini to just development.ini, the same as our other apps.

This [server:main] section contained Gunicorn config settings. Gunicorn only reads settings from the [server:main] block of the PasteDeploy config file if Gunicorn is being run indirectly via pserve. We're no longer using pserve, so these Gunicorn settings have now moved into gunicorn-dev.conf.py.

The proc_name setting was unnecessary: the app is already called web in supervisord-dev.conf, so I didn't copy that into gunicorn-dev.conf.py.

The errorlog setting was also unnecessary: - is the default. Didn't copy that either.

Comment on lines +1 to +3
bind = "0.0.0.0:5000"
graceful_timeout = 0
timeout = 0
Copy link
Contributor Author

@seanh seanh Dec 23, 2023

Choose a reason for hiding this comment

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

I think it's better to have separate gunicorn*.conf.py config files for Gunicorn:

  • It's a more obvious place to put the Gunicorn config settings, rather than putting them in a [server:main] block in the PasteDeploy config file (you wouldn't think to look for Gunicorn settings there, and it's not obvious that they are Gunicorn settings even once you've found them).
  • There are some Gunicorn settings (such as server hooks) that can only go in a Gunicorn config file, not a PasteDeploy one.
  • Gunicorn only reads general Gunicorn settings from a PasteDeploy file when run it via pserve, and we're not using pserve anymore

Comment on lines -12 to -15
[server:main]
use: egg:gunicorn#main
bind: unix:/tmp/gunicorn-web.sock
proc_name: web
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed app.ini to production.ini as in our other apps, and again moved the [server:main] section into gunicorn.conf.py. Again the proc_name setting was unnecessary here.

@@ -3,15 +3,15 @@ nodaemon = true
silent = true

[program:web]
command = pserve --reload conf/development-app.ini
command = gunicorn --paste conf/development.ini --config conf/gunicorn-dev.conf.py
Copy link
Contributor Author

@seanh seanh Dec 23, 2023

Choose a reason for hiding this comment

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

h was the only one of our apps that was running Gunicorn indirectly via pserve. I've changed it to run gunicorn directly like our other apps do and to use gunicorn's --paste option so that the Pyramid settings get read (again like we do in other apps).

This is unusual for a Pyramid app: the Pyramid docs tell you to use pserve. But we've always run gunicorn directly (in all apps but h) and I'm not aware of anything that we're losing by not using pserve. May as well remove an unnecessary layer?

I've gone for a gunicorn --paste conf/*.ini --config conf/gunicorn*.conf.py command format that gives us both a PasteDeploy config file and a Gunicorn config file. Even though we (rightly) tend to use envvars most often for config settings, I do think we want both config files. Examples in the Pyramid docs and many Pyramid extensions tell you to use the PasteDeploy config file, and it's where we put our logging config. And the Gunicorn config file is the only Gunicorn config source that supports all Gunicorn config settings. There are separate config files for dev because it tends to differ a lot, but QA/staging/production share the same config file (so any differences need to go in envvars).

The cookiecutter will put these gunicorn --paste conf/*.ini --config conf/gunicorn*.conf.py commands in the supervisord[-dev].conf files and then apps will be able to customise the configs by overriding the conf/* files and/or by using environment variables (including the GUNICORN_CMD_ARGS envvar).

Notes on Gunicorn configuration

To understand Gunicorn configuration you need to read the Running Gunicorn, Configuration Overview and Settings pages in the Gunicorn docs and combine them in your head. Here's a short summary:

Gunicorn reads configuration from five locations and merges them. You can use all five of these at once, each successive location overriding earlier ones:

  1. Certain specific environment variables for specific settings. For example PORT (for the port part of the bind setting), WEB_CONCURRENCY (for the workers setting), and a few others. We use some of these (e.g. WEB_CONCURRENCY) in our QA and production environments. Not all settings have environment variables, only a small handful do. The environment variables aren't all listed in one place in the Gunicorn docs, mentions of them are spread throughout the Settings page.

  2. The PasteDeploy config file (if any), e.g. development.ini or production.ini. There are two variations of this:

    1. You can run a command like pserve *.ini which uses Pyramid's pserve to run Gunicorn indirectly. The [server:main] block in the *.ini PasteDeploy config file tells Pyramid to use Gunicorn (instead of Waitress, which is Pyramid's default):

      [server:main]
      use = egg:gunicorn#main
      bind = unix:/tmp/gunicorn-web.sock
      proc_name = web
      workers = 3

      In this case Gunicorn can read some Gunicorn config settings from the [server:main] block in the PasteDeploy config file (such as proc_name and workers = 3 in the example above). Some Gunicorn settings (e.g. server hooks) can't go in a PasteDeploy config file.

      Some Gunicorn features such as hot-reloading don't work when running Gunicorn via pserve (you have to use pserve's reloading instead).

      See Running the Project Application in Pyramid's docs and Paste Deployment in Gunicorn's docs and also Paster Applications in the Gunicorn docs.

    2. You can run Gunicorn with the --paste or --paster option:

      gunicorn --paste development.ini [GUNICORN_OPTIONS]
      

      In this case you're running Gunicorn directly and not using pserve. Gunicorn will read certain settings from the PasteDeploy config file such as the path to the WSGI application function and the logging config. But when run in this way Gunicorn will not read general Gunicorn settings from the PasteDeploy config file's [server:main] section, so you have to put your Gunicorn settings somewhere else. Gunicorn hot reloading will work.

  3. The Gunicorn config file (if any). By default this is gunicorn.conf.py in the current working directory but it can be changed with the -c / --config argument to the gunicorn command. A Gunicorn config file is the only source that gives you access to all Gunicorn settings.

  4. The GUNICORN_CMD_ARGS environment variable which can contain gunicorn command line arguments, e.g. GUNICORN_CMD_ARGS="--bind=127.0.0.1 --workers=3".

  5. Command line arguments passed directly to the gunicorn command itself. These override everything else. Not all Gunicorn settings have command line arguments.

Comment on lines +31 to +38
[program:websocket-monolithic]
command=newrelic-admin run-program gunicorn --paste conf/websocket-monolithic.ini --config conf/gunicorn-websocket-monolithic.conf.py
stdout_logfile=NONE
stderr_logfile=NONE
stdout_events_enabled=true
stderr_events_enabled=true
autostart = %(ENV_ENABLE_WEBSOCKET_MONOLITHIC)s
process_name = websocket
Copy link
Contributor Author

@seanh seanh Dec 23, 2023

Choose a reason for hiding this comment

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

Previously there was a single websocket "program" in supervisord.conf and a %(ENV_WEBSOCKET_CONFIG)s envvar was used to make the websocket config file vary between different QA/staging/production environments: conf/websocket-separate.ini (now renamed to just websocket.ini) or conf/websocket-monolithic.ini.

This will no longer work now that we have separate PasteDeploy and Gunicorn config files. You'd need two separate envvars WEBSOCKET_PASTEDEPLOY_CONFIG and WEBSOCKET_GUNICORN_CONFIG or some other scheme.

I've changed this to instead define two separate websocket and websocket-monolithic processes in supervisord.conf. We'll then set the ENABLE_WEBSOCKET and ENABLE_WEBSOCKET_MONOLITHIC envvars as necessary in each environment.

These ENABLE_{PROGRAM} envvars are a pattern that's available for all our programs/processes in all our repos. We already use ENABLE_WEBSOCKET=false in h's QA and Production (US) environments and then they have separate QA (WebSocket) and Production (WebSocket) environments that have ENABLE_WEB=false and ENABLE_WEBSOCKET=true. These ENABLE_*'s could also be used in h-periodic which needs to disable some of its beat processes in its Canada environment. Having a separate environment for Celery workers would be another example (ENABLE_WEB=false and ENABLE_WORKER=true). So it makes sense to also use the ENABLE_* config to toggle between websocket and websocket-monolithic in different environments.

@seanh seanh marked this pull request as ready for review December 23, 2023 21:05
Copy link
Member

@marcospri marcospri left a comment

Choose a reason for hiding this comment

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

This looks sensible but I'd be careful with the QA deploy of this one in case we missed something. Maybe just double checking that WS still up

This commit reorganises h's `conf/` directory following a schema that I
plan to apply to our cookiecutter and the rest of our apps.

h's `conf/` directory presents some unique problems that don't apply to
our other apps because it has some extra configuration files that our
other apps don't have:

* Several `conf/*websocket*` config files for h's separate websocket
  app, which even needs an extra set of config files for when it's
  running in "monolithic" mode (used by `make run-docker` in development
  and in the Canada production environment).
* `websocket-newrelic.ini`: a New Relic configuration used by h's
  separate websocket app
* An `nginx.conf` file used by h's "internal" NGINX instance in
  production (there's no NGINX in h's development environment)
@seanh
Copy link
Contributor Author

seanh commented Jan 2, 2024

Deployment plan:

  • Set ENABLE_WEBSOCKET_MONOLITHIC=false in QA
  • Set ENABLE_WEBSOCKET_MONOLITHIC=false in QA (WebSocket)
  • Set ENABLE_WEBSOCKET=false in Staging
  • Set ENABLE_WEBSOCKET_MONOLITHIC=true in Staging
  • Set ENABLE_WEBSOCKET_MONOLITHIC=false in Production
  • Set ENABLE_WEBSOCKET_MONOLITHIC=false in Production (WebSocket)
  • Set ENABLE_WEBSOCKET=false in Production (Canada)
  • Set ENABLE_WEBSOCKET_MONOLITHIC=true in Production (Canada)
  • Deploy this PR to QA
  • Test this PR on QA (including testing the WebSocket)
  • Deploy this PR to Production (Canada)
  • Test this PR on Production (Canada) (including testing the WebSocket)
  • Deploy this PR to Production
  • Test this PR on Production (including testing the WebSocket)
  • Remove WEBSOCKET_CONFIG envvar from QA
  • Remove WEBSOCKET_CONFIG envvar from QA (WebSocket)
  • Remove WEBSOCKET_CONFIG envvar from Staging
  • Remove WEBSOCKET_CONFIG envvar from Production
  • Remove WEBSOCKET_CONFIG envvar from Production (WebSocket)
  • Remove WEBSOCKET_CONFIG envvar from Production (Canada)

@seanh seanh merged commit 81459f1 into main Jan 2, 2024
9 checks passed
@seanh seanh deleted the reorganise-config branch January 2, 2024 13:09
seanh added a commit that referenced this pull request Jan 2, 2024
#8386 changed the WebSocket command
in `supervisord.conf` to be prefixed by `newrelic-admin run-program ...`
which it wasn't before. I suspect that might have broken the WebSocket.
If I'm right, this will hopefully fix it.

Slack thread: https://hypothes-is.slack.com/archives/C4K6M7P5E/p1704201384991689?thread_ts=1704200985.822419&cid=C4K6M7P5E
seanh added a commit that referenced this pull request Jan 2, 2024
#8386 changed the WebSocket command
in `supervisord.conf` to be prefixed by `newrelic-admin run-program ...`
which it wasn't before. I suspect that might have broken the WebSocket.
If I'm right, this will hopefully fix it.

Slack thread: https://hypothes-is.slack.com/archives/C4K6M7P5E/p1704201384991689?thread_ts=1704200985.822419&cid=C4K6M7P5E
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.

None yet

2 participants