Update balrog to fit Dockerflow

RESOLVED FIXED

Status

Release Engineering
Balrog: Backend
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: mostlygeek, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The balrog application needs to be updated to fit the Dockerflow[1] specification.

[1] https://github.com/mozilla-services/Dockerflow
(Assignee)

Comment 1

2 years ago
Looks like there's a few specific things that to happen:
1) Update the main Dockerfile to build a production-ready image. This means pulling out things like npm, and making sure all the necessary data is installed (instead of relying on bind mounts). We also need to make sure that changes to Python dependencies (currently specified in setup.py) will cause the image to be rebuilt.
2) Add __version__, __heartbeat__, and __lbheartbeat__ endpoints per the docs.
3) Tweak uwsgi configs to listen on $PORT instead of a hardcoded port.
4) Get rid of uwsgi .ini files or bake them into the image.
5) Ensure app logs go to stdout and stderr, not a file, and use the mozlog format (https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=42895640)

Other things will probably come up, this is a start though.
(Assignee)

Comment 2

2 years ago
Quick update here:
(In reply to Ben Hearsum (:bhearsum) from comment #1)
> Looks like there's a few specific things that to happen:
> 1) Update the main Dockerfile to build a production-ready image. This means
> pulling out things like npm, and making sure all the necessary data is
> installed (instead of relying on bind mounts). We also need to make sure
> that changes to Python dependencies (currently specified in setup.py) will
> cause the image to be rebuilt.

I've got a patch in progress for this: https://github.com/mozilla/balrog/compare/master...bhearsum:production-dockerfile

I think the current state of the Dockerfile in that branch will build production-ready Docker images. I'm still trying to sort out whether or not we can push them to dockerhub from Taskcluster. If I can't get that figured out next week I'll probably punt on it and switch to CircleCI.

> 2) Add __version__, __heartbeat__, and __lbheartbeat__ endpoints per the
> docs.

This is also started in https://github.com/mozilla/balrog/compare/master...bhearsum:dockerflow-endpoints

Nothing difficult here AFAICT, just need to do it.

> 4) Get rid of uwsgi .ini files or bake them into the image.

This probably fixed with #1, as I'm copying the entire repository into the image, which includes these files.
Depends on: 1251606
(Reporter)

Comment 3

2 years ago
Took a look at the patch: https://github.com/mozilla/balrog/compare/master...bhearsum:production-dockerfile

- are you planning on turning this into a PR? 
- why requirements.txt and then use setup.py to install it?
  - the convention is usually to install with pip. pip is already on the base container 
    and in Dockerfile we can copy in requirements.txt, pip install ... and get 
    all the requirements in and cached as a layer. This way new builds where
    requirements.txt don't change will use the cached layer.
(Reporter)

Comment 4

2 years ago
Here are some examples you can work from. I've been working with the TestPilot guys and I think they have the best/most advanced Dockerflow implementation so far: 

* Dockerfile: https://github.com/mozilla/testpilot/blob/master/Dockerfile
* CircleCI: https://github.com/mozilla/testpilot/blob/master/circle.yml

With circle.yml: 

* Static frontend assets are built by the CI and tested using a headless browser
* Copied into the container as static assets
* Container is built and tested
* Container is shipped to DockerHub

It sounds pretty similar to what balrog's needs are.
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1189379
(Assignee)

Comment 6

2 years ago
Sorry, missed these comments until just now.

(In reply to Benson Wong [:mostlygeek] from comment #3)
> Took a look at the patch:
> https://github.com/mozilla/balrog/compare/master...bhearsum:production-
> dockerfile
> 
> - are you planning on turning this into a PR? 

Should have one ready tomorrow. Any progress on getting the "mozillabalrog" dockerhub account access to push images? I was hoping to test that out first.

> - why requirements.txt and then use setup.py to install it?
>
>   - the convention is usually to install with pip. pip is already on the
> base container 
>     and in Dockerfile we can copy in requirements.txt, pip install ... and
> get 
>     all the requirements in and cached as a layer. This way new builds where
>     requirements.txt don't change will use the cached layer.

I originally did this because it's what shavar does (https://github.com/mozilla-services/shavar/blob/master/setup.py). I think it's proper to include the requirements in setup.py, because the normal way to install Python modules is "python setup.py install", which should pull in required dependencies. However, there's no reason we can't run pip before setup.py in the Dockerfile to have the benefit of caching without breaking setup.py, I think?
(Reporter)

Comment 7

2 years ago
- mozillabalrog should have write access to https://hub.docker.com/r/mozilla/balrog now. 

re: setup.py

- could we copy in setup.py first and run `python setup.py install` without the rest of the app?
- if we can, then I'm +1 on doing it this way, otherwise the requirements.txt way is preferred
  since it will make use of the docker cache
(Assignee)

Comment 8

2 years ago
Created attachment 8725887 [details] [review]
add support for dockerflow requirements, minus mozlog

More details in the PR.
Assignee: nobody → bhearsum
Status: NEW → ASSIGNED
Attachment #8725887 - Flags: review?(nthomas)
Attachment #8725887 - Flags: review?(bwong)
(Reporter)

Comment 9

2 years ago
Comment on attachment 8725887 [details] [review]
add support for dockerflow requirements, minus mozlog

commented in the PR
Attachment #8725887 - Flags: review?(bwong)
(Reporter)

Comment 10

2 years ago
Balrog's Dockerflow PR: https://github.com/mozilla/balrog/pull/55
(Assignee)

Updated

2 years ago
Depends on: 1252895
Comment on attachment 8725887 [details] [review]
add support for dockerflow requirements, minus mozlog

f+ with some comments on github where we can improve the patch.
Attachment #8725887 - Flags: review?(nthomas) → feedback+
(Assignee)

Updated

2 years ago
Depends on: 1255439
(Assignee)

Comment 12

2 years ago
Comment on attachment 8725887 [details] [review]
add support for dockerflow requirements, minus mozlog

I think this is ready for review now, details in https://github.com/mozilla/balrog/pull/55#issuecomment-194936702.
Attachment #8725887 - Flags: review?(nthomas)
Attachment #8725887 - Flags: review?(bwong)
(Reporter)

Updated

2 years ago
Attachment #8725887 - Flags: review?(bwong)
Attachment #8725887 - Flags: review?(nthomas) → review+

Comment 13

a year ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/efe00da440b91f15957e7d9ecb67acde67734bf3
Merge pull request #55 from bhearsum/dockerflow-endpoints

bug 1248751: Add support for Dockerflow requirements (minus mozlog). r=nthomas,mostlygeek
(Assignee)

Comment 14

a year ago
Comment on attachment 8725887 [details] [review]
add support for dockerflow requirements, minus mozlog

It required a few follow-ups to deal with some issues with the commands in the Docker script, but this is now landed and pushing images to https://hub.docker.com/r/mozilla/balrog/tags/
Attachment #8725887 - Flags: checked-in+
(Assignee)

Comment 15

a year ago
I think we're done with this bug. bug 1251335 will deal with mozlog.
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.