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
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -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? :))

!Gemfile.locks
25 changes: 25 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
FROM ruby:2.7.0-alpine3.11
maxcapraro marked this conversation as resolved.
Show resolved Hide resolved

WORKDIR /InnerSource
ADD Gemfile .
ADD Gemfile.lock .

# Required in order to build gem native extenstions
# see https://github.com/docker-library/ruby/issues/163
RUN apk add --no-cache g++ gcc make musl-dev

# Required to build in Docker for Mac, otherwise it will raise the
# error: Could not find 'bundler'
RUN bundle update --bundler

RUN bundle install

# Required so that Jekyll will not override site.url with the host passed by --host
ENV JEKYLL_ENV=docker

EXPOSE 35729
maxcapraro marked this conversation as resolved.
Show resolved Hide resolved
EXPOSE 4000

# On --host 0.0.0.0
# It is required so that Jekyll will accept connections outside of localhost and 127.0.0.1
CMD jekyll serve --host 0.0.0.0 --livereload --config /source/_config.yml,/source/_config_dev.yml -s /source
2 changes: 1 addition & 1 deletion _config_dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ sass:
# trace_selectors: true
# debug_info: true
# FUTURE https://github.com/jekyll/jekyll-sass-converter/issues/12
sourcemap: true
sourcemap: always

# Disable when not in production
google_analytics_tracking_id:
8 changes: 8 additions & 0 deletions _config_dev_toolbox.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# This file is needed for setting up the development environment
# with docker toolbox.
# It reverts the absolute paths configured in _config_dev.yml
# Without this, assets and links build with site.url would fail to
# load or open.

url: ''
urlimg: 'images/'
13 changes: 13 additions & 0 deletions docker-compose-toolbox.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
version: "3.7"
maxcapraro marked this conversation as resolved.
Show resolved Hide resolved
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 👍

build: .
image: innersource-website-devenv
ports:
- "4000:4000" # webpage
- "35729:35729" # live reload
volumes:
- ./:/source
# --force_polling is necessary as the sharing of the filesystem from host to the toolbox vm does not seem to trigger the regular rebuild on changes
# _config_dev_toolbox.yml - see the comments on the file
command: jekyll serve --host 0.0.0.0 --force_polling --livereload --config /source/_config.yml,/source/_config_dev.yml,/source/_config_dev_toolbox.yml -s /source
10 changes: 10 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
version: "3.7"
maxcapraro marked this conversation as resolved.
Show resolved Hide resolved
services:
jekyll:
build: .
image: innersource-website-devenv
ports:
- "4000:4000" # webpage
- "35729:35729" # live reload
volumes:
- ./:/source