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

Move common docker compose configs to common repository #6539

Merged
merged 3 commits into from Jan 24, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Jan 18, 2020

Most of the code for community and commercial regarding docker configs can be shared.

This PR moves out from this repository all these configs and keep the minimal configuration that needs to be overwritten in this repo.

Notes:

  • Added a new environment variable to change the prefix of the containers (community_web). We can't use the name of the directory anymore since the docker-compose file is in a shared directory under common
  • Download the requirements file directly instead of copying it from the context. We don't have access to from the context anymore since it's in an upper directory
  • YAML for search is completely moved to common

Requires: readthedocs/common#43

@humitos humitos requested a review from Jan 18, 2020
@humitos humitos force-pushed the humitos/merge-common-docker-compose branch from 0907ea6 to cf94476 Compare Jan 18, 2020
@humitos
Copy link
Member Author

@humitos humitos commented Jan 20, 2020

While playing with these changes, I got stuck because there were networks assigned with the same pool of IPs and docker does not prune them automatically by calling docker system prune sometimes.

So, I had to do it manually with docker network disconnect -f {network} {container}. See moby/moby#17217 (comment)

@humitos
Copy link
Member Author

@humitos humitos commented Jan 22, 2020

Added a new environment variable to change the prefix of the containers (community_web). We can't use the name of the directory anymore since the docker-compose file is in a shared directory under common

This will end up using a different volume (from readthedocsorg_database to community_database) which implies that your new "instance" will be empty. In case you don't want to loose your local, data you can copy the volume

.env Show resolved Hide resolved
@humitos humitos changed the title Move common docker compose configs to common repository Merge pull request #6539 from readthedocs/humitos/merge-common-docker-compose Jan 24, 2020
@humitos humitos merged commit 1445011 into master Jan 24, 2020
3 checks passed
@humitos humitos deleted the humitos/merge-common-docker-compose branch Jan 24, 2020
@humitos humitos changed the title Merge pull request #6539 from readthedocs/humitos/merge-common-docker-compose Move common docker compose configs to common repository Jan 24, 2020
humitos added a commit that referenced this issue Jan 24, 2020
I introduce this bug in #6539
WORKDIR /tmp

RUN curl -O https://raw.githubusercontent.com/readthedocs/readthedocs.org/master/requirements/pip.txt
RUN curl -O https://raw.githubusercontent.com/readthedocs/readthedocs.org/master/requirements/docker.txt
Copy link
Contributor

@davidfischer davidfischer Jan 31, 2020

Choose a reason for hiding this comment

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

These two lines are going to be a nightmare for testing different versions of things.

Copy link
Member Author

@humitos humitos Feb 2, 2020

Choose a reason for hiding this comment

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

I'm not following your point here. What would be the problem?

Copy link
Member

@stsewd stsewd Feb 3, 2020

Choose a reason for hiding this comment

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

I think David means we should install deps the first time the container is run (or when init=true) instead of installing them when building the image.

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

4 participants