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.

Sign up for GitHub

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 root-required bits of drama-free-django build into Dockerfile #5145

Merged
merged 16 commits into from
Jul 29, 2019
Merged

Move root-required bits of drama-free-django build into Dockerfile #5145

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fb34088
Moves OS dependencies into Dockerfile
hkeeler Jul 23, 2019
f2269c6
Merge branch 'master' of github.com:cfpb/cfgov-refresh into dfd-docke…
hkeeler Jul 25, 2019
8478002
Converts dfd scripts to use docker build, then run
hkeeler Jul 25, 2019
270e1a6
Revert back to centos:6 Docker image
hkeeler Jul 26, 2019
a9a63bb
Revert to original `static_out` path
hkeeler Jul 26, 2019
a6e09f5
Override PIP_NO_CACHE_DIR on pip upgrade
hkeeler Jul 26, 2019
7431fba
Removes Mac-specific `cached` volume attribute
hkeeler Jul 26, 2019
490fe09
Merge branch 'master' into dfd-dockerfile
hkeeler Jul 26, 2019
3cd92f0
Removes unneeded `which` package
hkeeler Jul 26, 2019
139f8d8
Merge branch 'dfd-dockerfile' of github.com:hkeeler/cfgov-refresh int…
hkeeler Jul 26, 2019
e936ae7
Removes unneeded question comment
hkeeler Jul 26, 2019
4433633
Add headings and "Notes" section to DFD README.md
hkeeler Jul 26, 2019
44fef31
Add yarn cache warning to DFD Docker README
hkeeler Jul 26, 2019
5b963b4
Fix yarn warnings by setting $HOME in Dockerfile
hkeeler Jul 27, 2019
8bf713f
Fix typo in drama-free-django/Dockerfile
hkeeler Jul 29, 2019
72e7382
Merge branch 'master' into dfd-dockerfile
hkeeler Jul 29, 2019
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
23 changes: 23 additions & 0 deletions docker/drama-free-django/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
FROM centos:7
Copy link
Member

Choose a reason for hiding this comment

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

The current versions of the build and test scripts use centos:6(also documented here). I did this deliberately to try to match the current setup. Is there a particular reason to change that version here?

Copy link
Member Author

Choose a reason for hiding this comment

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

That was not intentional. Will fix.


ENV SCL_PYTHON_VERSION python27

ENV DFD_DIR /src/cfgov-refresh

# Must be world writable since alternate uid:gid may be patched in at `docker run` time.
RUN mkdir -p ${DFD_DIR} && chmod 777 ${DFD_DIR}
WORKDIR ${DFD_DIR}

# Install dependencies
RUN yum install -y centos-release-scl && \
curl -sL https://rpm.nodesource.com/setup_10.x | bash - && \
curl -sL https://dl.yarnpkg.com/rpm/yarn.repo | tee /etc/yum.repos.d/yarn.repo && \
yum install -y ${SCL_PYTHON_VERSION} gcc git nodejs which yarn && \
Copy link
Member

Choose a reason for hiding this comment

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

Is the inclusion of which here leftover from debugging?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I can remove that. I find it strange that CentOS doesn't have which in its base distribution, but yeah.

echo "source scl_source enable ${SCL_PYTHON_VERSION}" > /etc/profile.d/scl_python.sh && \
source /etc/profile && \
pip install -U pip && \
pip install -U git+https://github.com/cfpb/drama-free-django.git

COPY _build.sh _test.sh docker-entrypoint.sh ./

ENTRYPOINT [ "./docker-entrypoint.sh"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ENTRYPOINT [ "./docker-entrypoint.sh"]
ENTRYPOINT ["./docker-entrypoint.sh"]

20 changes: 4 additions & 16 deletions docker/drama-free-django/_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,6 @@ if [ ! -d "$cfgov_refresh_volume" ]; then
exit 1
fi

# Install build requirements.
yum install -y centos-release-scl
yum install -y gcc git python27

source /opt/rh/python27/enable

pip install -U pip
pip install -U git+https://github.com/cfpb/drama-free-django.git

curl -sL https://rpm.nodesource.com/setup_10.x | bash -
curl -sL https://dl.yarnpkg.com/rpm/yarn.repo | tee /etc/yum.repos.d/yarn.repo
yum install -y nodejs yarn

# Run the frontend build.
pushd "$cfgov_refresh_volume"
./frontend.sh production
Expand Down Expand Up @@ -62,13 +49,14 @@ no-drama build "${build_args[@]}"
echo "{}" > ./dfd_env.json

# This is used by DFD to set Django's settings.STATIC_ROOT.
echo '{"static_out": "../../../static"}' > ./dfd_paths.json
# Q: Why do we need to override the default?
# echo '{"static_out": "../static"}' > ./dfd_paths.json
Copy link
Member

Choose a reason for hiding this comment

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

This is needed because the default drama-free-django behavior is to set its static_out variable to 'static'. This is used to set Django STATIC_ROOT, which specifies where collectstatic puts its files. The default behavior would have this command collect staticfiles to a static relative path within the DFD deploy.

We instead want these files to go to '../../../static/', in practice going from, say, /srv/cfgov/versions/20190712095508/current to /srv/cfgov/static.

@rosskarchner's open PR #5133 currently includes a change that would set the default static root to /srv/cfgov/static as desired, but that'll only work once cfpb/drama-free-django#25 or something like it removes the DFD behavior that overwrites the static root at runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I'll change it back.


no-drama release \
"./$build_artifact" \
./dfd_env.json \
"$artifact_release" \
--paths ./dfd_paths.json
"$artifact_release" #\
#--paths ./dfd_paths.json

# Copy release artifact to source directory.
cp "$release_artifact" "$cfgov_refresh_volume"
Expand Down
11 changes: 4 additions & 7 deletions docker/drama-free-django/_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ set -x

artifact_filename=cfgov_current_build.zip
artifact_volume=/cfgov
dfd_test_dir=/tmp/dfd-test/release

# Verify that the artifact volume has been mapped.
if [ ! -d "$artifact_volume" ]; then
Expand All @@ -16,15 +17,11 @@ if [ ! -d "$artifact_volume" ]; then
exit 1
fi

# Install runtime requirements.
yum install -y centos-release-scl
yum install -y python27

source /opt/rh/python27/enable

# Extract the artifact in /tmp.
cp "$artifact_volume/$artifact_filename" /tmp
cd /tmp
mkdir -p $dfd_test_dir
Copy link
Member

Choose a reason for hiding this comment

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

Not a big deal, but I'm curious why you made a new directory for this. I figured that working in /tmp was fine since the container is ephemeral.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue was that by having it in /tmp, it resulted in directories being created in the / root directory, and the non-root user didn't have permission to create those directories. An alternative would be to open up permissions on /, but this seems like a better option.

cp "$artifact_volume/$artifact_filename" $dfd_test_dir
cd $dfd_test_dir
python "./$artifact_filename"

cd current
Expand Down
10 changes: 9 additions & 1 deletion docker/drama-free-django/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
#!/usr/bin/env bash

docker run -v `pwd`:/cfgov centos:6 /cfgov/docker/drama-free-django/_build.sh
set -e

docker build -t cfgov-dfd-builder docker/drama-free-django

docker run \
--rm \
-u $(id -u):$(id -g) \
-v $(pwd):/cfgov \
cfgov-dfd-builder ./_build.sh
7 changes: 7 additions & 0 deletions docker/drama-free-django/docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash --login
# This entrypoint is used primarily as means of setting up a consistent
# shell environment no matter which user the process runs as. By using
# --login, it guarantees /etc/profile is always sourced, unlike the
# non-login, non-interactive shell you get by default with `docker run`.

exec "$@"
10 changes: 9 additions & 1 deletion docker/drama-free-django/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
#!/usr/bin/env bash

docker run -v `pwd`:/cfgov centos:6 /cfgov/docker/drama-free-django/_test.sh
set -e

docker build -t cfgov-dfd-builder docker/drama-free-django

docker run \
--rm \
-u $(id -u):$(id -g) \
-v $(pwd):/cfgov:cached \
Copy link
Member

Choose a reason for hiding this comment

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

Why is :cached used here in the test script but not in the build script? Is this Mac-specific, as documented here -- is it a no-op when run on Linux systems?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, it fails currently on Linux. I'll remove it.

cfgov-dfd-builder ./_test.sh
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't hold up this PR, but, if we are going to maintain this DFD testing capability, I wonder if it's worth entertaining the idea of a distinct Dockerfile just for that purpose. We don't really need everything in the "cfgov-dfd-builder" image just to run the DFD image, and it would be nice to actually determine what is needed. But probably thinking more about that should wait until/if we want to think about migrating Ansible code here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it does feel a little awkward to have the two scripts that both build the same image, but my assumption was that generally when you'd run test.sh, you would have run build.sh just before it, and so the docker build part would be fully cached and be pretty instant, but I didn't want to make that a requirement to running test.sh, so it's duplicated in both scripts.

As for a separate image, yeah we could. It just seemed like it'd be better to only maintain one Dockerfile that could do both. But if there's a scenario where we'd be running just test.sh, maybe that starts to make more sense.