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

Add docker image from the YAML config integration #3339

Merged
merged 12 commits into from
Dec 28, 2017
15 changes: 8 additions & 7 deletions docs/yaml-config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,20 @@ The ``build`` block configures specific aspects of the documentation build.
build.image
```````````

* Default: ``2.0``

* Default: :djangosetting:`DOCKER_IMAGE`
* Options: ``1.0``, ``2.0``, ``latest``
Copy link
Contributor

Choose a reason for hiding this comment

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

With settings re-enabled, this can be pulled from settings. See settings.rst and doc_extensions.py in conf/


The docker image to use for specific builds.
This lets users specify a more experimental builder,
The build image to use for specific builds.
This lets users specify a more experimental build image,
if they want to be on the cutting edge.
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency and clarity:

"docker image" -> "build image"
"builder" -> "build image"


Certain Python versions require a certain builder,
Certain Python versions require a certain build image,
as defined here::
Copy link
Contributor

Choose a reason for hiding this comment

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

Same on "builder" here, builder is an internal term, this refers to the build image instead.


* '1.0': 2, 2.7, 3, 3.4
* '2.0': 2, 2.7, 3, 3.5
* 'latest': 2, 2.7, 3, 3.3, 3.4, 3.5, 3.6
* `'1.0': 2, 2.7, 3, 3.4`
* `'2.0': 2, 2.7, 3, 3.5`
* `'latest': 2, 2.7, 3, 3.3, 3.4, 3.5, 3.6`

.. code-block:: yaml

Expand Down
29 changes: 22 additions & 7 deletions readthedocs/doc_builder/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from readthedocs_build.config import load as load_config
from readthedocs_build.config import BuildConfig, ConfigError, InvalidConfig

from .constants import DOCKER_IMAGE_SETTINGS, DOCKER_IMAGE


class ConfigWrapper(object):

Expand Down Expand Up @@ -57,7 +59,7 @@ def python_interpreter(self):
list(
filter(
lambda x: x < ver + 1,
self._yaml_config.PYTHON_SUPPORTED_VERSIONS,
self._yaml_config.get_valid_python_versions(),
)))
return 'python{0}'.format(ver)

Expand Down Expand Up @@ -110,10 +112,8 @@ def formats(self):
def build_image(self):
if self._project.container_image:
# Allow us to override per-project still
assert 'readthedocs/build' in self._project.container_image, (
'container image must be fully qualified')
return self._project.container_image
return 'readthedocs/build:{}'.format(self._yaml_config['build']['image'])
return self._yaml_config['build']['image']

# Not implemented until we figure out how to keep in sync with the webs.
# Probably needs to be version-specific as well, not project.
Expand All @@ -133,8 +133,24 @@ def load_yaml_config(version):
parsing consistent between projects.
"""
checkout_path = version.project.checkout_path(version.slug)
env_config = {}
Copy link
Member

Choose a reason for hiding this comment

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

This one is not needed. You are re-creating it some lines below.


# Get build image to set up the python version validation. Pass in the
# build image python limitations to the loaded config so that the versions
# can be rejected at validation

img_name = version.project.container_image or DOCKER_IMAGE
env_config = {
'build': {
'image': img_name,
}
}
img_settings = DOCKER_IMAGE_SETTINGS.get(img_name, None)
if img_settings:
env_config.update(img_settings)

try:
sphinx_env_config = {}
sphinx_env_config = env_config.copy()
sphinx_env_config.update({
Copy link
Member

Choose a reason for hiding this comment

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

You can just create the dict at this line instead of .update :)

'output_base': '',
'type': 'sphinx',
Expand All @@ -148,9 +164,8 @@ def load_yaml_config(version):
# This is a subclass of ConfigError, so has to come first
raise
except ConfigError:
# Just fall back to defaults
config = BuildConfig(
env_config={},
env_config=env_config,
raw_config={},
source_file='empty',
source_position=0,
Expand Down
10 changes: 10 additions & 0 deletions readthedocs/doc_builder/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
from __future__ import (
absolute_import, division, print_function, unicode_literals)

import logging
import os
import re

from django.conf import settings

log = logging.getLogger(__name__)

SPHINX_TEMPLATE_DIR = os.path.join(
settings.SITE_ROOT,
'readthedocs',
Expand All @@ -33,6 +36,13 @@
)
DOCKER_VERSION = getattr(settings, 'DOCKER_VERSION', 'auto')
DOCKER_IMAGE = getattr(settings, 'DOCKER_IMAGE', 'readthedocs/build:2.0')
DOCKER_IMAGE_SETTINGS = getattr(settings, 'DOCKER_IMAGE_SETTINGS', {})

old_config = getattr(settings, 'DOCKER_BUILD_IMAGES', None)
if old_config:
log.warning('Old config detected, DOCKER_BUILD_IMAGES->DOCKER_IMAGE_SETTINGS')
DOCKER_IMAGE_SETTINGS.update(old_config)
Copy link
Contributor

Choose a reason for hiding this comment

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

This warning could be a good use of a django system check warning as well.


DOCKER_LIMITS = {'memory': '200m', 'time': 600}
DOCKER_LIMITS.update(getattr(settings, 'DOCKER_LIMITS', {}))

Expand Down
40 changes: 40 additions & 0 deletions readthedocs/rtd_tests/tests/test_config_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,46 @@ def setUp(self):
install_project=False, requirements_file='urls.py')
self.version = get(Version, project=self.project)

def test_python_supported_versions_default_image_1_0(self, load_config):
load_config.side_effect = create_load()
self.project.container_image = 'readthedocs/build:1.0'
self.project.save()
config = load_yaml_config(self.version)
self.assertEqual(load_config.call_count, 1)
load_config.assert_has_calls([
mock.call(path=mock.ANY, env_config={
'build': {'image': 'readthedocs/build:1.0'},
'type': 'sphinx',
'output_base': '',
'name': mock.ANY
}),
])
self.assertEqual(config.python_version, 2)

def test_python_supported_versions_image_1_0(self, load_config):
load_config.side_effect = create_load()
self.project.container_image = 'readthedocs/build:1.0'
self.project.save()
config = load_yaml_config(self.version)
self.assertEqual(config._yaml_config.get_valid_python_versions(),
[2, 2.7, 3, 3.4])

def test_python_supported_versions_image_2_0(self, load_config):
load_config.side_effect = create_load()
self.project.container_image = 'readthedocs/build:2.0'
self.project.save()
config = load_yaml_config(self.version)
self.assertEqual(config._yaml_config.get_valid_python_versions(),
[2, 2.7, 3, 3.5])

def test_python_supported_versions_image_latest(self, load_config):
load_config.side_effect = create_load()
self.project.container_image = 'readthedocs/build:latest'
self.project.save()
config = load_yaml_config(self.version)
self.assertEqual(config._yaml_config.get_valid_python_versions(),
[2, 2.7, 3, 3.3, 3.4, 3.5, 3.6])

def test_python_default_version(self, load_config):
load_config.side_effect = create_load()
config = load_yaml_config(self.version)
Expand Down
2 changes: 1 addition & 1 deletion requirements/pip.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ six==1.10.0
future==0.16.0
#readthedocs-build==2.0.8
# For testing
git+https://github.com/rtfd/readthedocs-build.git@d93c81c#egg=readthedocs_build
git+https://github.com/rtfd/readthedocs-build.git@79f78d23d367f71#egg=readthedocs_build

django-tastypie==0.13.0
django-haystack==2.6.0
Expand Down