nsf-open / nsf

Repo for the NSF Site Redesign & Content Discovery project
https://nsf-demo.app.cloud.gov
GNU General Public License v2.0
4 stars 4 forks source link

Switch to s3fs so we can have multiple instances #223

Closed cmc333333 closed 6 years ago

cmc333333 commented 6 years ago

Drupal writes "optimized" css/js to the "public" file system, which defaults to the local disk. This is problematic when we have multiple instances, as the files will only be written to the disk of one. It'd also be an issue between deployments/restages, except that we clear caches on the first instance.

Unfortunately, flysystem doesn't override the "public" filesystem, so it couldn't be used to store those files in s3. s3fs can, however, with enough configuration. Notably, it requires apache be configured to proxy s3 files, which requires some extra lifting in our system config.

This PR switches to the s3fs module, handles the apache configs, turns on optimized css/js and bumps up our instance count. It's working in cloud.gov, but I'd really appreciate another test of the Docker version (remember to docker-compose build).

This PR's probably easiest to review changeset by changeset, but let me know if a walkthrough'd be better.

fureigh commented 6 years ago

@cmc333333 I'm running into a couple of issues locally. Here's the primary one:

I ran docker-compose build and docker-compose up and got this:

web_1       |  [ERROR] An error occurred while trying to write the config file: "Unable to    
web_1       |          install the <em class="placeholder">s3fs</em> module since it does     
web_1       |          not exist."

So far I've confirmed that s3fs is present in both composer.json and composer.lock. I didn't immediately spot any clues in docker-compose logs but am taking a closer look.

Also, in case this is related: When I ran docker-compose build, the terminal output included the following:

To activate the new configuration, you need to run:
  service apache2 restart

I'm getting a "command not found" error for service; presumably that's a local shell config issue, so I'll proceed accordingly.

fureigh commented 6 years ago

@cmc333333 noticed that settings.cf.php hadn't come through quite right. chmod a+w web/sites/default and then re-checking-out that file got it running locally! 🎉

I'm still testing locally, but we may end up needing to change (and subsequently revert) some file permissions in bootstrap.sh. For security reasons, we wouldn't want that directory to be writable in any non-local environment.

fureigh commented 6 years ago

Upon further consideration: because the permissions issue is an artifact of local development (Drupal's install script helpfully removing writability from certain files), I'll leave bootstrap.sh alone and try sorting it out in a Git hook or two instead. No need for that to hold up a merge of this pull request.

In the meantime, I've confirmed that this is working on staging. We'll want to turn on the page cache too (/admin/config/development/performance), but it makes sense to handle that in a separate pull request.

screen shot 2018-04-20 at 3 50 21 pm

Now testing mimicking locally.

fureigh commented 6 years ago

Beautiful! I was able to access remotely cached CSS and JS files via localhost without any issues. Thanks for all of your work on this, @cmc333333! I say rebase and :shipit:.

fureigh commented 6 years ago

One more note: because /web/.htaccess does get rewritten to include the actual environment variables, there's a danger of a developer accidentally committing those values to the repo. Worth thinking about how to guard against that (rather than relying on individuals' git hygiene).

cmc333333 commented 6 years ago

Thanks @fureigh ! Added a note about committing secrets and we can expand if desired.