-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Add support for declaring named volumes in compose files #2421
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
Changes from all commits
73de81b
3bdcc9d
afab5c7
c64b7cb
97fe2ee
4bf2f8c
b4be7b8
b253efd
abe145b
df6877a
ecef5d3
ec5111f
661519a
f3a9533
a7689f3
1dcdd98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| import six | ||
| import yaml | ||
|
|
||
| from ..const import COMPOSEFILE_VERSIONS | ||
| from .errors import CircularReference | ||
| from .errors import ComposeFileNotFound | ||
| from .errors import ConfigurationError | ||
|
|
@@ -24,6 +25,7 @@ | |
| from .validation import validate_against_service_schema | ||
| from .validation import validate_extends_file_path | ||
| from .validation import validate_top_level_object | ||
| from .validation import validate_top_level_service_objects | ||
|
|
||
|
|
||
| DOCKER_CONFIG_KEYS = [ | ||
|
|
@@ -116,6 +118,20 @@ class ConfigFile(namedtuple('_ConfigFile', 'filename config')): | |
| def from_filename(cls, filename): | ||
| return cls(filename, load_yaml(filename)) | ||
|
|
||
| def get_service_dicts(self, version): | ||
| return self.config if version == 1 else self.config.get('services', {}) | ||
|
|
||
|
|
||
| class Config(namedtuple('_Config', 'version services volumes')): | ||
| """ | ||
| :param version: configuration version | ||
| :type version: int | ||
| :param services: List of service description dictionaries | ||
| :type services: :class:`list` | ||
| :param volumes: List of volume description dictionaries | ||
| :type volumes: :class:`list` | ||
| """ | ||
|
|
||
|
|
||
| class ServiceConfig(namedtuple('_ServiceConfig', 'working_dir filename name config')): | ||
|
|
||
|
|
@@ -148,6 +164,34 @@ def find(base_dir, filenames): | |
| [ConfigFile.from_filename(f) for f in filenames]) | ||
|
|
||
|
|
||
| def get_config_version(config_details): | ||
| def get_version(config): | ||
| if config.config is None: | ||
| return 1 | ||
| version = config.config.get('version', 1) | ||
| if isinstance(version, dict): | ||
| # in that case 'version' is probably a service name, so assume | ||
| # this is a legacy (version=1) file | ||
| version = 1 | ||
| return version | ||
|
|
||
| main_file = config_details.config_files[0] | ||
| validate_top_level_object(main_file) | ||
| version = get_version(main_file) | ||
| for next_file in config_details.config_files[1:]: | ||
| validate_top_level_object(next_file) | ||
| next_file_version = get_version(next_file) | ||
|
|
||
| if version != next_file_version and next_file_version is not None: | ||
| raise ConfigurationError( | ||
| "Version mismatch: main file {0} specifies version {1} but " | ||
| "extension file {2} uses version {3}".format( | ||
| main_file.filename, version, next_file.filename, next_file_version | ||
| ) | ||
| ) | ||
| return version | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we will probably need to do a similar version checking for extends, but I think we should wait until this is merged since it's already so large. |
||
|
|
||
|
|
||
| def get_default_config_files(base_dir): | ||
| (candidates, path) = find_candidates_in_parent_dirs(SUPPORTED_FILENAMES, base_dir) | ||
|
|
||
|
|
@@ -194,14 +238,52 @@ def load(config_details): | |
|
|
||
| Return a fully interpolated, extended and validated configuration. | ||
| """ | ||
| version = get_config_version(config_details) | ||
| if version not in COMPOSEFILE_VERSIONS: | ||
| raise ConfigurationError('Invalid config version provided: {0}'.format(version)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By checking What do you think about removing this check and moving the error to an
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was doing this before but I think that moving it to the else was too late for some reason. I'm gonna test it again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, if for some reason this needs to happen after processing files , the block below that iterates over the config_details could be moved into a function and called from both branches. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think this is an issue, but not enough to block this branch, so I guess we can discuss it after this is merged. |
||
|
|
||
| processed_files = [] | ||
| for config_file in config_details.config_files: | ||
| processed_files.append( | ||
| process_config_file(config_file, version=version) | ||
| ) | ||
| config_details = config_details._replace(config_files=processed_files) | ||
|
|
||
| if version == 1: | ||
| service_dicts = load_services( | ||
| config_details.working_dir, config_details.config_files, | ||
| version | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this function takes both parts of a service_dicts = load_services(config_details) |
||
| volumes = {} | ||
| elif version == 2: | ||
| config_files = [ | ||
| ConfigFile(f.filename, f.config.get('services', {})) | ||
| for f in config_details.config_files | ||
| ] | ||
| service_dicts = load_services( | ||
| config_details.working_dir, config_files, version | ||
| ) | ||
| volumes = load_volumes(config_details.config_files) | ||
|
|
||
| return Config(version, service_dicts, volumes) | ||
|
|
||
|
|
||
| def load_volumes(config_files): | ||
| volumes = {} | ||
| for config_file in config_files: | ||
| for name, volume_config in config_file.config.get('volumes', {}).items(): | ||
| volumes.update({name: volume_config}) | ||
| return volumes | ||
|
|
||
|
|
||
| def load_services(working_dir, config_files, version): | ||
| def build_service(filename, service_name, service_dict): | ||
| service_config = ServiceConfig.with_abs_paths( | ||
| config_details.working_dir, | ||
| working_dir, | ||
| filename, | ||
| service_name, | ||
| service_dict) | ||
| resolver = ServiceExtendsResolver(service_config) | ||
| resolver = ServiceExtendsResolver(service_config, version) | ||
| service_dict = process_service(resolver.run()) | ||
|
|
||
| # TODO: move to validate_service() | ||
|
|
@@ -227,20 +309,28 @@ def merge_services(base, override): | |
| for name in all_service_names | ||
| } | ||
|
|
||
| config_file = process_config_file(config_details.config_files[0]) | ||
| for next_file in config_details.config_files[1:]: | ||
| next_file = process_config_file(next_file) | ||
|
|
||
| config_file = config_files[0] | ||
| for next_file in config_files[1:]: | ||
| config = merge_services(config_file.config, next_file.config) | ||
| config_file = config_file._replace(config=config) | ||
|
|
||
| return build_services(config_file) | ||
|
|
||
|
|
||
| def process_config_file(config_file, service_name=None): | ||
| validate_top_level_object(config_file) | ||
| processed_config = interpolate_environment_variables(config_file.config) | ||
| validate_against_fields_schema(processed_config, config_file.filename) | ||
| def process_config_file(config_file, version, service_name=None): | ||
| service_dicts = config_file.get_service_dicts(version) | ||
| validate_top_level_service_objects( | ||
| config_file.filename, service_dicts | ||
| ) | ||
| interpolated_config = interpolate_environment_variables(service_dicts) | ||
| if version == 2: | ||
| processed_config = dict(config_file.config) | ||
| processed_config.update({'services': interpolated_config}) | ||
| if version == 1: | ||
| processed_config = interpolated_config | ||
| validate_against_fields_schema( | ||
| processed_config, config_file.filename, version | ||
| ) | ||
|
|
||
| if service_name and service_name not in processed_config: | ||
| raise ConfigurationError( | ||
|
|
@@ -251,10 +341,11 @@ def process_config_file(config_file, service_name=None): | |
|
|
||
|
|
||
| class ServiceExtendsResolver(object): | ||
| def __init__(self, service_config, already_seen=None): | ||
| def __init__(self, service_config, version, already_seen=None): | ||
| self.service_config = service_config | ||
| self.working_dir = service_config.working_dir | ||
| self.already_seen = already_seen or [] | ||
| self.version = version | ||
|
|
||
| @property | ||
| def signature(self): | ||
|
|
@@ -283,7 +374,8 @@ def validate_and_construct_extends(self): | |
|
|
||
| extended_file = process_config_file( | ||
| ConfigFile.from_filename(config_path), | ||
| service_name=service_name) | ||
| version=self.version, service_name=service_name | ||
| ) | ||
| service_config = extended_file.config[service_name] | ||
| return config_path, service_config, service_name | ||
|
|
||
|
|
@@ -294,6 +386,7 @@ def resolve_extends(self, extended_config_path, service_dict, service_name): | |
| extended_config_path, | ||
| service_name, | ||
| service_dict), | ||
| self.version, | ||
| already_seen=self.already_seen + [self.signature]) | ||
|
|
||
| service_config = resolver.run() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| { | ||
| "$schema": "http://json-schema.org/draft-04/schema#", | ||
| "type": "object", | ||
| "id": "fields_schema_v2.json", | ||
|
|
||
| "properties": { | ||
| "version": { | ||
| "enum": [2] | ||
| }, | ||
| "services": { | ||
| "id": "#/properties/services", | ||
| "type": "object", | ||
| "patternProperties": { | ||
| "^[a-zA-Z0-9._-]+$": { | ||
| "$ref": "fields_schema_v1.json#/definitions/service" | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm hoping we'll be able to refactor the schema so that this service name constraint only exists in once place (instead of both here and the v1 schema). I think it can wait until later though. |
||
| }, | ||
| "additionalProperties": false | ||
| }, | ||
| "volumes": { | ||
| "id": "#/properties/volumes", | ||
| "type": "object", | ||
| "patternProperties": { | ||
| "^[a-zA-Z0-9._-]+$": { | ||
| "$ref": "#/definitions/volume" | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
| }, | ||
|
|
||
| "definitions": { | ||
| "volume": { | ||
| "id": "#/definitions/volume", | ||
| "type": "object", | ||
| "properties": { | ||
| "driver": {"type": "string"}, | ||
| "driver_opts": { | ||
| "type": "object", | ||
| "patternProperties": { | ||
| "^.+$": {"type": ["string", "number"]} | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what case would
config.config['version']be a dict?Isn't this either
None(because theversionkey doesn't exist) or an int?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user for some reason has a service called version in a legacy compose file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, right! Mind adding an inline comment about that? I'm sure I will forget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will do.