Use docker-compose to run Solr in CI by mjgiarlo · Pull Request #2263 · projectblacklight/blacklight

mjgiarlo · 2020-03-13T16:12:35Z

Summary

In this way, we are using docker-compose for development and testing of the Blacklight gem itself, not for downstream purposes such as generation of new Blacklight applications, which will continue to use solr_wrapper. Also includes: using a lighter base image for the app.

This supersedes #2256.

Open Question (Now Closed)

I put the documentation here: https://github.com/projectblacklight/blacklight/wiki/Testing-and-Developing-Blacklight

It is not clear to me where information like this is currently documented. The README is short and sweet, and for this reason I suspect it is not a good place for such internal development documentation to live. Other options include CONTRIBUTING.md, but that is more focused on higher-level contribution process than particular technical steps.

Note, I also don't think the Quickstart is the right place, so this ticket has nothing to do with #2183, which I think we should either close as a wontfix per my comment or discuss what the right way to move forward is particularly w/r/t how much we continue to generate for downstream BL apps: i.e., if we don't give them a working SolrWrapper, do we generate/ship a working docker-compose configuration? Or do we get out of that business entirely? This is beyond the scope of this PR.

~~For this reason, perhaps the best place is somewhere on the project wiki. Happy to take suggestions on this.~~

Webpack(er) Broken (But Not Related)

I believe this may be solved by #2266. Thanks, @mejackreed!

Note that this PR changes the blacklight:server rake task to use docker-compose for Solr but I was unable to see this working---with Ruby 2.6 and Rails 6, but other combinations will yield different results---because when Blacklight uses EngineCart to generate the test app, there's a problem with webpack(er). If you generate without the --skip-webpack-install flag being passed to the rails new command, the Rails webpack generator runs and fails because the dependency is not declared. If you skip webpack installation, Rails generates an app that still contains webpacker in its Gemfile, and it's a multi-step process to strip out webpack(er) and use Sprockets in its stead (1, 2, 3). That said, I did not alter this behavior or attempt to fix the underlying problem since it is orthogonal to how we spin up Solr for CI/testing of Blacklight.

mjgiarlo · 2020-03-13T16:24:47Z

.travis.yml

  chrome: stable
-language: ruby

-notifications:


This is dead configuration, overridden by lines 34-37 below, and has been cruft for 7 years.

.travis.yml

mjgiarlo · 2020-03-13T16:25:54Z

.travis.yml

-
-jdk: openjdk11
+env:
+  global:


This appears to be the modern way to specify global environment variables.

mjgiarlo · 2020-03-13T16:26:14Z

.travis.yml

-  - ENGINE_CART_RAILS_OPTIONS='--skip-git --skip-listen --skip-spring --skip-keeps --skip-action-cable --skip-coffee --skip-test'
-  - CC_TEST_REPORTER_ID=5042c7358c96b0b926088a4cda3e132fffe7a66ce8047cdb1dc6f0b4b6676b79
-
-jdk: openjdk11


Don't need JDK anymore if spinning up Solr via Travis's docker infrastructure and our docker-compose configuration.

mjgiarlo · 2020-03-13T16:28:23Z

tasks/blacklight.rake

    Rake::Task["spec"].invoke
  end

-  desc "Create the test rails app"


Unclear why this is needed; looks like cruft from the pre-EngineCart days. Found no references to it anywhere on the open web, so I removed it.

I think this is needed because blacklight is a rails engine too and to test that functionality the engine_cart is used.

Removing this doesn't seem to have caused any ill effects, FWIW.

mjgiarlo · 2020-03-13T16:28:47Z

tasks/blacklight.rake

    task seed: ['engine_cart:generate'] do
      within_test_app do
-        system "bundle exec rake blacklight:index:seed"
+        system "bin/rake blacklight:index:seed"


Use generated binstubs. See https://github.com/rails/rails/blob/v6.0.2.1/railties/lib/rails/app_loader.rb#L13-L14

Not necessary for this work, so I'm happy to undo this if folks would prefer.

jcoyne

👍

dkinzer · 2020-03-14T02:05:58Z

docker-compose.yml

      - $PWD/lib/generators/blacklight/templates/solr/conf:/opt/solr/conf
    ports:
-      - "8983:8983"
+      - "${SOLR_PORT}:8983"


I really like the use of SOLR_PORT environment variable here.

dkinzer · 2020-03-14T18:56:35Z

@mjgiarlo I wonder whether (if it's possible) for us to also incorporate the docker app in the travis build in order to avoid issues where the Dockerfile gets broken or stale.

mjgiarlo · 2020-03-14T19:13:05Z

Totally open to that, @dkinzer. Do you have any ideas for what to test? Could try to build the image and fail unless it returns a zero exit code? Or maybe there's a smarter thing to do?

dkinzer · 2020-03-14T19:30:25Z

@mjgiarlo , maybe we can run the blacklight tests within the app for one of the ruby versions?

Unfortunately when I try that on my local I get an error so it's likely not an easy thing to setup:

 docker-compose exec app rake ci
WARN: Unresolved or ambigious specs during Gem::Specification.reset:
      minitest (~> 5.1)
      Available/installed versions of this gem:
      - 5.14.0
      - 5.13.0
      - 5.11.3
      loofah (~> 2.3)
      Available/installed versions of this gem:
      - 2.4.0
      - 2.3.1
      nokogiri (>= 1.6)
      Available/installed versions of this gem:
      - 1.10.9
      - 1.10.5
      builder (~> 3.1)
      Available/installed versions of this gem:
      - 3.2.4
      - 3.2.3
WARN: Clearing out unresolved specs. Try 'gem cleanup <gem>'
Please report a bug if this causes problems.
You must `gem install bundler` and `bundle install` to run rake tasks
rake aborted!
LoadError: cannot load such file -- engine_cart/rake_task
tasks/blacklight.rake:3:in `require'
tasks/blacklight.rake:3:in `<top (required)>'
/app/rakefile:13:in `load'
/app/rakefile:13:in `<top (required)>'
/usr/local/bundle/gems/rake-13.0.1/exe/rake:27:i```

dkinzer · 2020-03-14T19:35:07Z

I was able to run the tests using docker-compose exec app bundle exec rspec but now that I think about it, I tend to agree with you that just doing docker-compose up should suffice.

mjgiarlo · 2020-03-17T00:15:17Z

bin/start.sh

@@ -1,5 +0,0 @@
-#!/usr/bin/env bash


I essentially moved this file into .docker/app/ and renamed it to make its intent clearer.

.docker/app/Dockerfile

cdmo · 2020-03-17T13:23:01Z

@mjgiarlo and @dkinzer do you have any recommendations for learning more about docker, docker compose, etc? Book? Good docs somewhere? Video? Thanks.

Anything that you need tested by developers here at the moment?

cdmo · 2020-03-17T13:32:14Z

This will need to be rebased too I think to get the latest changes that were merged after this was created.

dkinzer · 2020-03-17T13:38:05Z

@cdmo my resource of choice are the docker docs (dark mode theme turned on of course): https://docs.docker.com/compose/

jcoyne · 2020-03-17T14:26:12Z

@cdmo some resources I recommend:
https://LearnDocker.online: Free resource with 11+ hours of video content teaching you everything about Docker and Containers.
https://RailsWithDocker.com: Free online resource teaching you how to dockerize a Rails app.

cdmo · 2020-03-17T14:50:19Z

Slick. Thanks @jcoyne

mjgiarlo · 2020-03-17T15:34:18Z

@cdmo 💬

@mjgiarlo and @dkinzer do you have any recommendations for learning more about docker, docker compose, etc? Book? Good docs somewhere? Video? Thanks.

Anything that you need tested by developers here at the moment?

I'm very nearly ready to mark this PR as ready for review but the big need I see is some shared sense of where to document this stuff. See the Open Question section above. I welcome your thoughts!

mjgiarlo · 2020-03-17T15:49:32Z

.docker/app/Dockerfile

-      nodejs yarn build-essential libpq-dev \
-      && apt-get clean
-RUN gem install bundler
+ARG ALPINE_RUBY_VERSION


I'm using ALPINE_RUBY_VERSION because RUBY_VERSION is already in use elsewhere, e.g., by RVM. This allows switching the version of Ruby used in the app image via environment variable:

$ ALPINE_RUBY_VERSION=2.7 docker-compose up --build app

.env

mejackreed

I've left a few comments on here, but overall it feels like its shaping up nicely.

It is not clear to me where information like this is currently documented. The README is short and sweet, and for this reason I suspect it is not a good place for such internal development documentation to live. Other options include CONTRIBUTING.md, but that is more focused on higher-level contribution process than particular technical steps.

Maybe we could/should just add a new wiki page for Blacklight development contribution?

The webpacker thing also has me :sad_trombone: (with regard to developer experience). Maybe we can look at that somewhere else to get that resolved?

.env

mejackreed · 2020-03-17T23:12:47Z

tasks/blacklight.rake

 desc "Run test suite"
-task ci: ['blacklight:generate'] do
+task :ci do
+  system "docker-compose up -d solr"


I ran into an issue where I pulled this branch and all of the feature tests failed for me. bundle exec rake ci

mejackreed · 2020-03-17T23:17:56Z

tasks/blacklight.rake

 desc "Run test suite"
-task ci: ['blacklight:generate'] do
+task :ci do
+  system "docker-compose up -d solr"


Also, I noticed that this is both in the ci task and the travis file. Is this needed in both placed?

mejackreed · 2020-03-17T23:31:55Z

Sorry adding one more thing I noticed in another comment. Strange, when I run rails s seperately I get the error 500 page as if it is running in RAILS_ENV=production. Not sure what is happening there but happy to keep digging. 👏 thanks again @mjgiarlo for moving this forward.

dkinzer · 2020-03-18T01:22:15Z

@mjgiarlo I ran bundle exec rake ci a second time after first making sure I first ran docker-compose down and also completely deleted the local .inernal_test_app folder and this time all tests passed with no errors. 🤷‍♂

The errors I ran into earlier were all related to solr not being available

 Autocomplete suggestions search form has suggest path
       Failure/Error: raise Blacklight::Exceptions::ECONNREFUSED, "Unable to connect to Solr instance using #{connection.inspect}: #{e.inspect}"

       Blacklight::Exceptions::ECONNREFUSED:

dkinzer · 2020-03-20T23:05:04Z

FYI, we were having some webpacker issues on our docker instance and moving to node 8.x fixed it for us.

.travis.yml

cdmo · 2020-03-21T14:32:30Z

Did I miss something, what's the webpacker issue? Or is this a reference to the feature specs not working thing?

cdmo · 2020-03-21T14:35:08Z

The combination of the dockerized app + engine_cart makes for a very awkward pairing. I'm not sure whether this is a scenario we can or should fix with code changes, or whether to address it via documentation? (In which case, see open question above.) FWIW, I would also be OK with dumping the Dockerfile entirely and using docker-compose only for Solr in CI.

I was going to ask about this. In Travis engine_cart runs the various versions of ruby and rails. Locally it does too. Will this PR replace engine_cart for local and just test against one version of ruby? Thanks for all the contributions and conversations.

dkinzer · 2020-03-22T13:12:17Z

I think this PR should be merged ASAP since the current state of the docker environment on the master branch is broken.

We can iterate on the merged changes in the future if need be.

dkinzer · 2020-03-22T13:18:20Z

@cdmo yes, that was a reference to the features specs not working (when testing locally)... I'm pretty sure issue happens because app fails to load due to webpacker problems. Also we are not seeing the errors that cause this because they are being hidden.

If I have time later on today, I'll try to push a PR against this branch.

mjgiarlo · 2020-03-27T16:04:19Z

@mejackreed

It is not clear to me where information like this is currently documented. The README is short and sweet, and for this reason I suspect it is not a good place for such internal development documentation to live. Other options include CONTRIBUTING.md, but that is more focused on higher-level contribution process than particular technical steps.

Maybe we could/should just add a new wiki page for Blacklight development contribution?

I renamed an existing page, updated its link on the main wiki page, and will use it to stash this information: https://github.com/projectblacklight/blacklight/wiki/Testing-and-Developing-Blacklight

🆒 ❓

mjgiarlo · 2020-03-27T16:23:51Z

@cdmo 💬

I was going to ask about this. In Travis engine_cart runs the various versions of ruby and rails. Locally it does too. Will this PR replace engine_cart for local and just test against one version of ruby? Thanks for all the contributions and conversations.

No, I don't think it will. That's part of why it's an awkward pairing. But I believe the docker componentry does give you the ability to specify Ruby and Rails versions to do whatever you'd like. For a working set of steps, here's what I just did:

https://github.com/projectblacklight/blacklight/wiki/Testing-and-Developing-Blacklight

mjgiarlo · 2020-03-27T16:27:20Z

@mejackreed can you try again following the guide I wrote up here: https://github.com/projectblacklight/blacklight/wiki/Testing-and-Developing-Blacklight

Fixes #2195 Fixes #2231 Closes #2256 Supersedes #2256 In this way, we are using docker-compose for development and testing of the Blacklight gem itself, not for downstream purposes such as generation of new Blacklight applications, which will continue to use solr_wrapper.

mejackreed

Thanks for the work on this @mjgiarlo . Just for others who come across this, locally creating and running an app with Rails > 6 will still fail. But we should deal with that separately.

👏

mjgiarlo · 2020-03-28T01:41:33Z

May I ask @jcoyne and @cdmo for a final review?

cdmo · 2020-03-28T02:48:23Z

Heh 😅 I dont know if I really ought to be reviewing this, kinda just wanted to see how I could help / be a tester. It all seems fine to me I suppose! I am checking out learndocker.online, thanks again for that.

I ran the wiki page instructions and hit a couple minor snags. First, running just rake and rspec lead to problems for me because I have multiple versions of these gems locally, specifying bundle exec as a prefix worked fine and got the right version. I also had one failing spec, sadly a spec I myself wrote (maybe the only one!). facets_spec.rb:83 the expectation's left side was nil, as in the focus was not set as expected. I did confirm that in an actual FF browser the focus setting does work there. Spent some time trying to figure out why that's happening, but I think it's safe to say it's unrelated to what you are working on!

mjgiarlo · 2020-03-28T16:41:09Z

@cdmo 💬

Heh I dont know if I really ought to be reviewing this, kinda just wanted to see how I could help / be a tester. It all seems fine to me I suppose! I am checking out learndocker.online, thanks again for that.

OK, no problem. Cool!

I ran the wiki page instructions and hit a couple minor snags. First, running just rake and rspec lead to problems for me because I have multiple versions of these gems locally, specifying bundle exec as a prefix worked fine and got the right version. I also had one failing spec, sadly a spec I myself wrote (maybe the only one!). facets_spec.rb:83 the expectation's left side was nil, as in the focus was not set as expected. I did confirm that in an actual FF browser the focus setting does work there. Spent some time trying to figure out why that's happening, but I think it's safe to say it's unrelated to what you are working on!

I just added some text to the Testing & Developing Blacklight wiki page about prefixing with bundle exec if needed. (I am still a big rvm user, so I never use bundle exec, and this will vary across developers.) Thanks for that feedback!

No idea about the failing spec. That's weird that it doesn't also fail in CI. Agree w/ you that it's unrelated to this PR, at least.

mjgiarlo · 2020-03-28T16:41:43Z

Let's wait for one more ✔️ before merging. @jcoyne ❓

mjgiarlo commented Mar 13, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

mjgiarlo commented Mar 13, 2020

View reviewed changes

mjgiarlo force-pushed the streamline-docker branch from 67ae211 to a2b5b18 Compare March 13, 2020 16:36

mjgiarlo mentioned this pull request Mar 13, 2020

Revert "Test using Docker based Solr rather than solr_wrapper" #2256

Closed

jcoyne reviewed Mar 13, 2020

View reviewed changes

dkinzer reviewed Mar 14, 2020

View reviewed changes

mjgiarlo force-pushed the streamline-docker branch from 681c5d6 to 4dee61f Compare March 17, 2020 00:12

mjgiarlo commented Mar 17, 2020

View reviewed changes

mjgiarlo force-pushed the streamline-docker branch 2 times, most recently from 41ed549 to 777f8b7 Compare March 17, 2020 00:32

cdmo mentioned this pull request Mar 17, 2020

Fix Dockerfile build error. #2265

Closed

cdmo reviewed Mar 17, 2020

View reviewed changes

.docker/app/Dockerfile Outdated Show resolved Hide resolved

mjgiarlo force-pushed the streamline-docker branch from 777f8b7 to 2e184dd Compare March 17, 2020 15:10

mjgiarlo force-pushed the streamline-docker branch from 2e184dd to eefb6b1 Compare March 17, 2020 15:35

mjgiarlo commented Mar 17, 2020

View reviewed changes

.env Show resolved Hide resolved

mejackreed reviewed Mar 17, 2020

View reviewed changes

cdmo reviewed Mar 21, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

mjgiarlo mentioned this pull request Mar 27, 2020

Implement similar changes that @cbeer made in https://github.com/sul-… #2266

Closed

mjgiarlo force-pushed the streamline-docker branch from eefb6b1 to f00337b Compare March 27, 2020 16:06

mjgiarlo force-pushed the streamline-docker branch 2 times, most recently from 446a58d to 47a5302 Compare March 27, 2020 19:00

mjgiarlo requested review from cdmo, jcoyne and mejackreed March 27, 2020 19:02

mjgiarlo force-pushed the streamline-docker branch from 47a5302 to 49599a2 Compare March 27, 2020 20:40

mejackreed approved these changes Mar 28, 2020

View reviewed changes

mjgiarlo mentioned this pull request Mar 28, 2020

Generate Rails 6 applications with Sprockets 4-style assets #2267

Merged

jcoyne approved these changes Mar 30, 2020

View reviewed changes

jcoyne merged commit 80ddfa7 into master Mar 30, 2020

jcoyne deleted the streamline-docker branch March 30, 2020 15:44

Conversation

mjgiarlo commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Open Question (Now Closed)

Webpack(er) Broken (But Not Related)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mjgiarlo Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjgiarlo Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcoyne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkinzer commented Mar 14, 2020

Uh oh!

mjgiarlo commented Mar 14, 2020

Uh oh!

dkinzer commented Mar 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkinzer commented Mar 14, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cdmo commented Mar 17, 2020

Uh oh!

cdmo commented Mar 17, 2020

Uh oh!

dkinzer commented Mar 17, 2020

Uh oh!

jcoyne commented Mar 17, 2020

Uh oh!

cdmo commented Mar 17, 2020

Uh oh!

mjgiarlo commented Mar 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mejackreed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejackreed commented Mar 17, 2020

Uh oh!

dkinzer commented Mar 18, 2020

Uh oh!

dkinzer commented Mar 20, 2020

Uh oh!

Uh oh!

cdmo commented Mar 21, 2020

Uh oh!

cdmo commented Mar 21, 2020

Uh oh!

dkinzer commented Mar 22, 2020

mjgiarlo commented Mar 13, 2020 •

edited

Loading

mjgiarlo Mar 13, 2020 •

edited

Loading

mjgiarlo Mar 13, 2020 •

edited

Loading

dkinzer commented Mar 14, 2020 •

edited

Loading