Skip to content
This repository has been archived by the owner on Mar 20, 2021. It is now read-only.

Feature gh#91 development environment setup with docker #99

Merged
merged 6 commits into from
Mar 9, 2020
Merged

Feature gh#91 development environment setup with docker #99

merged 6 commits into from
Mar 9, 2020

Conversation

dellagustin
Copy link
Contributor

@dellagustin dellagustin commented Feb 21, 2020

This commit enables the usage of Docker for setting up a development
environment.

It is an implementation for
#91

With this development, with the command docker-compose up or docker-compose -f docker-compose-toolbox.yml up (for Docker Toolbox), a docker
image is built and a docker contain is run serving the webpage, that
can be accessed on http://localhost:4000 (or http://<docker machine ip>:4000 for Docker Toolbox)

Files:

  • .dockerignore - ensures that only the necessary files are used in
    the context passed to the docker image build, saving build time
  • Dockerfile - The commands used to build a docker image
  • docker-compose.yml - Configuration for docker-compose, that enables
    a simple development environment setup, automating the image build and
    container orchestration
  • docker-compose-toolbox.yml - docker-compose file with tweaks for Docker Toolbox (see comments on files)
  • _docker_dev_toolbox.yml - overrides url configuration for Docker Toolbox (see comments on file)

Known limitations:

  • Website is not rebuilt automatically when a file is changed, requiring
    a container restart (this was expected to work out of the box with
    jekyll serve, but testing showed it did not work)
    • It was already working for Docker for Mac, implemented for Docker Toolbox with using the command line option --force_polling
  • Docker Toolbox is not supported - with this version of docker, the
    page can be loaded, but assets are not loaded, as the browser tries to
    fetch them from http://0.0.0.0, and the website must be accessed on the
    IP returned by docker-machine ip. More investigation is necessary.

Explanations:

  • RUN apk add --no-cache g++ gcc make musl-dev - necessary for
    building gem native extensions (eventmachine) - Error message "You have
    to install development tools first."
  • RUN bundle update --bundler - Image build on Docker Toolbox worked
    without this, but when trying to build the image on Docker for Mac, it
    raised the following error: find_spec_for_exe': Could not find 'bundler' (1.16.1) required by your /InnerSource/Gemfile.lock. (Gem::GemNotFoundException) To update to the latest version installed on your system, run `bundle update --bundler`.
  • --host 0.0.0.0 - without this line, jekyll will only accept traffice
    incoming from localhost or 127.0.0.1, which is not the case when
    using Docker for Mac or Docker Toolbox, as it forwards traffic from the host to the
    container. Without this line, the browser will show errors
    ERR_CONNECTION_REFUSED (Toolbox) or ERR_EMPTY_RESPONSE (Mac).

Remarks:

  • Only tested (by the author) on Docker for Mac and Docker Toolbox

Letf overs and follow ups:

  • Documentation on how to use the Docker environment is missing
  • Documentation on the overall solution for Docker environment is missing
    • The number of references, development iterations (until ready) and questions during review shows that even though the final solution is concise, there is a lot of non trivial aspects to it, therefore I would like to include some kind of documentation that explains how it works and more details on the whys of some things (decisions), and unused alternatives, this should help future refactoring - even though there is extensive information on the github issue Development environment setup with docker #91 and this pr (Feature gh#91 development environment setup with docker #99) I have the strong feeling git repos should be self contained regarding their documentation (i.e. portable to other git managers such as bitbucket or gitlab)
  • Decision Needed: Implement the left overs in this PR or approve and Merge the PR as is and document the leftovers with follow up issues - DONE - Document how to use in this PR, document overall Docker environment solution in a separate PR.
  • Create follow up issue to review Contributing instructions with respect to test the build locally (see Google Indexing innersourcecommons.org#99 (comment))
  • Create follow up issue to reduce the technical debt introduce with this PR
    • Separate docker-compose files for regular docker and toolbox
    • Unexplained need for RUN bundle update --bundler only on Docker Desktop, but not on toolbox
    • Not using jekyll/jekyll image

References:

PS: I carried InnerSourceCommons/innersourcecommons.org@f4edc9f to this PR by mistake, but I imagine #98 will be approved before this one, so I will not undo that.

When trying to run the command `jekyll serve` using `_config_dev.yml`,
it failed with the following error:
  Conversion error: Jekyll::Converters::Scss encountered an error while
  converting 'assets/css/atom.scss': undefined method `to_sym' for true:TrueClass Did
you mean? to_s
------------------------------------------------
Jekyll 4.0.0   Please append `--trace` to the `serve` command
               for any additional information or backtrace.
------------------------------------------------
This happens because in `_config_dev.yml` sourcemap is set to `true`,
while the documentation at
https://github.com/jekyll/jekyll-sass-converter stated the allowed
values are `never`, `always` and `development`.
This commit enables the usage of Docker for setting up a development
environment.

It is an implementation for
InnerSourceCommons/innersourcecommons.org#91

With this development, with the command `docker-compose up`, a docker
image is built and a docker contain is run serving the webpage, that
can be accessed on https://localhost:4000

Files:
- `.dockerignore` - ensures that only the necessary files are used in
the context passed to the docker image build, saving build time
- `Dockerfile` - The commands used to build a docker image
- `docker-compose.yml` - Configuration for docker-compose, that enables
a simple development environment setup, automating the image build and
container orchestration

Known limitations:
- Website is not rebuilt automatically when a file is changed, requiring
a container restart (this was expected to work out of the box with
`jekyll serve`, but testing showed it did not work)
- Docker Toolbox is not supported - with this version of docker, the
page can be loaded, but assets are not loaded, as the browser tries to
fetch them from http://0.0.0.0, and the website must be accessed on the
IP returned by `docker-machine ip`. More investigation is necessary.

Explanations:
- `RUN apk add --no-cache g++ gcc make musl-dev` - necessary for
building gem native extensions (eventmachine) - Error message "You have
to install development tools first."
- `RUN bundle update --bundler` - Image build on Docker Toolbox worked
without this, but when trying to build the image on Docker for Mac, it
raised the following error: `find_spec_for_exe': Could not find
'bundler' (1.16.1) required by your /InnerSource/Gemfile.lock.
(Gem::GemNotFoundException)
To update to the latest version installed on your system, run `bundle
update --bundler`.
- `--host 0.0.0.0` - without this line, jekyll will only accept traffice
incoming from `localhost` or `127.0.0.1`, which is not the case when
using Docker for Mac or Docker Toolbox, as it forwards traffic from the host to the
container. Without this line, the browser will show errors
ERR_CONNECTION_REFUSED (Toolbox) or ERR_EMPTY_RESPONSE (Mac).

Remarks:
- Only tested on Docker for Mac and Docker Toolbox

References:
- https://hub.docker.com/_/ruby
- https://docs.docker.com/engine/reference/builder/#dockerignore-file
- https://docs.docker.com/engine/reference/builder/
- docker-library/ruby#163 (How to install
eventmachine)
-
https://stackoverflow.com/questions/41485217/mount-current-directory-as-a-volume-in-docker-on-windows-10
-
https://stackoverflow.com/questions/42893475/how-to-run-jekyll-serve-pointing-to-another-directory
-
https://stackoverflow.com/questions/50540721/docker-toolbox-error-response-from-daemon-invalid-mode-root-docker
-
https://medium.com/@Charles_Stover/fixing-volumes-in-docker-toolbox-4ad5ace0e572
-
https://sashabrava.github.io/2018/making-Jekyll-available-on-local-network.html
- microsoft/vscode-remote-release#764
-
https://stackoverflow.com/questions/16608466/connect-to-a-locally-built-jekyll-server-using-mobile-devices-in-the-lan/16608698
@maxcapraro
Copy link
Member

maxcapraro commented Feb 23, 2020

Thanks a lot for contributing this work, @dellagustin! 🙏

I think I might not be able to review it within the next 24h - but anyway: I am really looking forward to playing around with your implementation and will happily provide review feedback asap :)

@rrrutledge rrrutledge removed their request for review February 24, 2020 13:57
dellagustin-sap and others added 3 commits February 24, 2020 21:48
Added future proof support to Docker for windows
with setting `JEKYLL_ENV` to a value different than
`development`.
With this setting, the ip used in `--host` is not
going to override site.url.
The alternative would be use `build --watch` + a separate
web server for static files, but I did not see any advantage,
and also I wanted to have the livereload option of `serve`.

Added comments on the Dockerfile to explain some
of the non-intuitive parts.

Added support for livereload.

Added EXPOSE statements to the Dockerfile to document ports in use.

References:
- https://jekyllrb.com/docs/configuration/options/#serve-command-options
- https://tonyho.net/jekyll-docker-windows-and-0-0-0-0/
- Cannot set site.url to a different address than the --host flag address in development
  - jekyll/jekyll#5743
- https://docs.docker.com/engine/reference/builder/#expose
…-environment-setup-with-docker

Docker support enhancement
This commit enables the usage of Docker Toolbox with the command
`docker-compose -f docker-compose-toolbox.yml up`.

A similar result could be achieved by editing `_config_dev.yml`
to have 'url':'' and tweak the Dockerfile command to include
the --force_polling option, but I did not want to change the
`_config_dev.yml` without being sure of the consequences for
non docker users.
@dellagustin
Copy link
Contributor Author

Hi @maxcapraro Did you have the chance to review this PR?
If you need any help with the setup, please let me know.

@maxcapraro
Copy link
Member

Thanks for your patience with this one :) I'll look into it today

Copy link
Member

@maxcapraro maxcapraro left a comment

Choose a reason for hiding this comment

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

This is an excellent contribution, @dellagustin! Thanks a lot for this 🎖🥇

With this review, I propose a few minor changes to a) have less files in repo, b) document things, c) and potentially use a ready-made image.

@@ -0,0 +1,2 @@
!Gemfile
Copy link
Member

Choose a reason for hiding this comment

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

(Not specific to the .dockerignore file)

I suggest we also adapt the CONTRIBUTING.md file to explain the new development and contribution workflow. This way prospective contributors will learn that (and how) they can make use of this setup.

Copy link
Member

Choose a reason for hiding this comment

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

I feel the CONTRIBUTING.md is a better fit than README.md because nobody wants to run the website locally for fun, but rather as a pre-step to contributing to it?

Copy link
Member

Choose a reason for hiding this comment

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

The section should probably include information like:

  • Build an image from the docker file, tag it as innersource-website-devenv
  • Run the docker-compose environment
  • Find your preview at http://localhost:4000

Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation also part of #91 , you will see a unchecked task there exactly because I wanted to follow this up, but I wanted to have a review of the functionality before documenting it :D

Copy link
Member

Choose a reason for hiding this comment

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

That's a legit decision. Now that most discussions are resolved, let's add the documentation? :))

Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose-toolbox.yml Show resolved Hide resolved
@@ -0,0 +1,13 @@
version: "3.7"
services:
jekyll:
Copy link
Member

Choose a reason for hiding this comment

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

Both docker-compose files worked (at least for me).

My suggestion is to not have two nearly redundant setups but rather use only one compose-file and scrap the other from this pull request. I'd delete docker-compose.yml (and corresponding config files) and keep and rename docker-compose-toolbox.yml). What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Saw this too late:

  • docker-compose.yml - Configuration for docker-compose, that enables
    a simple development environment setup, automating the image build and
    container orchestration
  • docker-compose-toolbox.yml - docker-compose file with tweaks for Docker Toolbox (see comments on files)

For me it worked well with both (just, obviously without the live reload). Do you expect any adverse effects when only having this compose environment?

Copy link
Contributor

@dellagustin-sap dellagustin-sap Mar 4, 2020

Choose a reason for hiding this comment

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

May I ask if you are testing with Docker for Windows (https://docs.docker.com/docker-for-windows/) or Docker Toolbox (https://docs.docker.com/toolbox/toolbox_install_windows/)?

As your screenshots show https://localhost:4000, unless you tweaked something, I guess you are using Docker for Windows.

The first one is not support on windows home version.

I will elaborate on the difference and why there are separate files on my next comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so, in more details now.

When you are using docker toolbox (not to be confused with docker for windows), the services running on docker will not be available at localhost or 127.0.0.1, but at the IP of the virtual machine running docker (normally 192.168.99.100).

We have many links that are being generated, for no apparent reason, with an absolute url (site.url + something) - see #102 .

If you see the content of _config_dev.yml, you will see url: 'http://localhost:4000', which means that running jekyll with the command line jekyl serve --config _config.yml,_config_dev.yml will render a page with many absolute links as http://localhost:4000/path/to/resource, which are not reachable when serving from docker toolbox.

I could not understand why _config_dev.yml needs this setting (or if it is needed at all) - blaming the file reveals this is there since the beginning, but I think this is only necessary if the url setting in _config.yml sets url to something different than '', which is not our case.

Removing the url configuration from _config.yml would reduce the complexity and enable us to have a single docker setup for toolbox and non-toolbox (if we accepts the superfluous ----force_polling), but I could not predict the consequences. We can create a follow up issue to clean this up and test it with more time.

To wrap it up, this is the main reason to have separate docker-compose files and an additional _config_dev_toolbox.yml (see the comments inside this file).

This is rather confusing, I hope to have explained it in a minimally understandable format 😃 .

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thanks for the very detailed explanation :)

I would like to enable more people to contribute to the website with minimal fear. That's why I would follow your suggestion:

Copy link
Member

Choose a reason for hiding this comment

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

@cewilliams @lenucksi Is that approach allright with you?

Copy link
Member

Choose a reason for hiding this comment

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

Cleaning non-relative links and other cases of hard-coded stuff up is always a good idea.
As long as this approach will not be mandatory, which it doesn't right now, everything should be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @lenucksi , I am not sure what you mean with this approach will not be mandatory, but removing all the non-relative links are not a pre-condition to make this PR work.

Copy link
Member

Choose a reason for hiding this comment

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

I was refering to "Editing only using Docker" which still works without, I give this 👍

@dellagustin
Copy link
Contributor Author

Hi @maxcapraro , thanks for the review 👍
I answered your comments, there are some decisions to be made (solve now or accept and track technical debt).
Can you take a look?
The docu I can write and push a new commit.

@maxcapraro
Copy link
Member

Thanks for the clarifications, @dellagustin! So far: looks good to me (except for the one comment re: documentation). Let's add that and then (if Jo and Cedrid haven't raised an opinion re: the two alternative docker files by then) let's merge :)

I added a section in CONTRIBUTING.md on how to test the page
edits locally, with and without docker.

So far the instructions only covered testing the build locally,
but not how to serve the page while editing.
@dellagustin
Copy link
Contributor Author

Hi @maxcapraro , I added documentation.
I created a new section for testing the page edits locally.
I think the build part is a bit confusing (i.e. general flow vs steps for ubuntu, no mention of the htmlproofer step), and as it is, I am not sure it adds much value, if the contributor is already using jekyll serve, but I did not want to change this for now.

@maxcapraro
Copy link
Member

It's beautiful. Thank you @dellagustin!

@maxcapraro maxcapraro merged commit c7455d7 into InnerSourceCommons:master Mar 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants