Refactors config validation of a service to use a ServiceConfig data object.
Instead of passing around a bunch of related scalars, we can use the
ServiceConfig object as a parameter to most of the service validation functions.
This allows for a fix to the config schema, where the name is a field in the
schema, but not actually in the configuration. My passing the name around as
part of the ServiceConfig object, we don't need to add it to the config options.
Fixes#2299
validate_against_service_schema() is moved from a conditional branch in
ServiceExtendsResolver() to happen as one of the last steps after all
configuration is merged. This schema only contains constraints which only need
to be true at the very end of merging.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
This bug can be seen by the change to the test case. When the extended service
uses a different name, the error was reported incorrectly.
By fixing this bug we can simplify self.signature and self.detect_cycles to
always use self.service_name.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
ServiceLoader has evolved to be not really all that related to "loading" a
service. It's responsibility is more to do with handling the `extends`
field, which is only part of loading. The class and its primary method
(make_service_dict()) were renamed to better reflect their responsibility.
As part of that change process_container_options() was removed from
make_service_dict() and renamed to process_service(). It contains logic for
handling the non-extends options.
This change allows us to remove the hacks from testcase.py and only call
the functions we need to format a service dict correctly for integration tests.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Consolidates all the top level config handling into `process_config_file` which
is now used for both files and merge sources.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
It's a flag passed to docker build that removes the intermediate
containers left behind on fail builds.
Signed-off-by: Adrian Budau <budau.adi@gmail.com>
Ensure config_hash is updated when volumes_from mode is changed, and
service is recreated on next up as a result.
Signed-off-by: Joffrey F <joffrey@docker.com>
- make it a positional arg, since it's required
- make it the first argument for all functions that require it
- remove an unnecessary one-line function that was only called in one place
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Includes some refactoring of log_printer_test to support checking for flush(), and so that each test calls the unit-under-test directly, instead of through a helper function.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
It has been an unnecessary wrapper around container.start() for a little while now, so we can call it directly.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
These cli tests are now a different kind of that that run the compose binary. They are not the same as integration tests that test some internal interface.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
So we're not displaying output of all previous logs for a container, we attach,
if possible, to a container before the container is started.
LogPrinter checks if a container has a log_stream already attached and
print from that rather than always attempting to attach one itself.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Remove some unnecessary newlines.
Remove a unittest that was attempting to test behaviour that was removed a while ago, so isn't testing anything.
Updated some unit tests to use mocks instead of a custom fake.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Environment keys that contain no value, get populated with values taken
from the environment not from the build phase but from running the command `up`.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
This change adds cgroup-parent support to compose project. It allows
each service to specify a 'cgroup_parent' option.
Signed-off-by: Mohit Soni <mosoni@paypal.com>
splitdrive doesn't handle relative paths, so if volume_path contains
a relative path, we handle that differently and manually set drive to ''.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Including examples of more boolean types, eg yes/N as it's not
always immediately clear that they are treated as booleans.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Rather than inefficiently looping through all the containers that a
service has and overriding each volumes_from value, pick the first
one and return that.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
An expanded windows path of c:\shiny\thing:\shiny:rw
needs to be changed to be linux style path, including the drive,
like /c/shiny/thing /shiny
to be mounted successfully by the engine.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
When a relative path is expanded and we're on a windows platform,
it expands to include the drive, eg C:\ , which was causing a ConfigError
as we split on ":" in parse_volume_spec and that was giving too many parts.
Use os.path.splitdrive instead of manually calculating the drive.
This should help us deal with windows drives as part of the volume
path better than us doing it manually.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Only expand volume host paths if they begin with a dot.
This is a breaking change. The deprecation warning preparing users for
this change has been removed.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
It was mocking self.client but relying on the call to
utils.create_host_config which was not mocked. So now that function
has moved to also be on self.client we need to redefine the test
boundary, up to where we would call docker-py, not the result of
docker-py.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
This is a unit test and we are mocking the client. The method to get
the create_config_host now lives on the client, so we mock that too.
So we can test to the boundary that the method is called with the
arguments we expect.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
create_host_config from docker.utils will be deprecated so that
the new create_host_config has access to the _version so
we can ensure that network_mode only gets set to 'default' by
default if the version is high enough and won't explode.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Now docker-py isn't hardcoding a list of valid log_drivers, we
can expect an APIError in response rather than a ValueError
if we send an invalid log_driver.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
One of the use cases is swarm requires at least : character, so going
from conservative to relaxed.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
We're going to warn people that allowing a boolean in the environment is
being deprecated, so in a future release we can disallow it. This is to
ensure boolean variables are quoted in strings to ensure they don't get
mis-parsed by YML.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
When users were putting true/false/yes/no in the environment key,
the YML parser was converting them into True/False, rather than leaving
them as a string.
This change will force people to put them in quotes, thus ensuring
that the value gets passed through as intended.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
oneOf schema ValidationError takes a little more work to parse and
pull out more detail so we can give a better error message back to
the user.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
English language is a tricky old thing and I've pulled out the validator type
parsing so that we can prefix our validator types with the correct article,
'an' or 'a'.
Doing a bit of extra hard work to ensure the error message is clear and
well constructed english.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
'~/' in a path currently doesnt work, you get the following error:
[Errno 2] No such file or directory: u'/home/USER/folder/~/some/path/.yml'
Signed-off-by: Nick H <nick.humrich@gmail.com>
The command value can be a list, which would be a Unix command-line
invocation broken up into individual values, thus needing the ability to
have non unique values.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Some were missing build '.' from their dicts, others were the
incorrect type and one I've moved from integration to unit.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
This is minimal disruptive change I could make to ensure
the service integration tests worked, now we have some validation
happening.
There is some coupling/entanglement/assumption going on here.
Project when creating a service, using it's class method from_dicts
performs some transformations on links, net & volume_from, which
get passed on to Service when creating. Service itself, then performs
some transformation on those values. This worked fine in the tests before
because those options were merely passed on via make_service_dict.
This is no longer true with our validation in place. You can't pass to
ServiceLoader [(obj, 'string')] for links, the validation expects it to be
a list of strings. Which it would be when passed into Project.from_dicts
method.
I think the tests need some re-factoring but for now, manually deleting
keys out of the kwargs and then putting them back in for Service Creation
allows the tests to continue.
I am not super happy about this approach. Hopefully we can come back and
improve it.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
We want to give feedback to the user as soon as possible about the
validity of the config supplied for the services.
When extending a service, we can validate that the fields are
correct against our schema but we must wait until the *end* of
the extends cycle once all of the extended dicts have been merged
into the service dict, to perform the final validation check on the
config to ensure it is a complete valid service.
Doing this before that had happened resulted in false reports of
invalid config, as common config when split out, by itself, is not
a valid service but *is* valid config to be included.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
This refactoring is now really coming together. Construction is
happening in the __init__, which is a constructor and helps
clean up the design and clarity of intent of the code. We can now
see (nearly) everything that is being constructed when a ServiceLoader
is created. It needs all of these data constructs to perform the
domain logic and actions. Which are now clearer to see and moving
more towards the principle of functions doing (mostly)one thing and
function names being more descriptive.
resolve_extends is now concerned with the resolving of extends, rather
than the construction, validation, pre processing and *then* resolving
of extends.
Happy days :)
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Moving service name and dict out of the function make_service_dict
and into __init__. We always call make_service_dict with those so
let's put them in the initialiser. Slightly cleaner design intent.
The whole purpose of the ServiceLoader is to take a
service name&service dictionary then validate, process and return
service dictionaries ready to be created.
This is also another step towards cleaning the code up so we can
interpolate and validate an extended dictionary.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
While it can be set to ultimately a value of None, when a
config file is read in from stdin, it is not optional.
We kinda make use of it's ability to be set to None in our
tests but functionally and design wise, it is required.
If filename is not set, extends does not work.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
In particular it includes:
- some extension of CONTRIBUTING.md
- one fix for Python 2.6 in tests/integration/cli_test.py
- one fix for Python 3.3 in tests/integration/service_test.py
- removal of unused imports
Make stream_output Python 3-compatible
Signed-off-by: Frank Sachsenheim <funkyfuture@riseup.net>
If we get back an error that wasn't an APIError, it was causing the
thread to hang. This catch all, while I appreciate feels risky to
have a catch all, is better than not catching and silently failing,
with a never ending thread.
If something worse than an APIError has gone wrong, we want to stop
the incredible journey of what we're doing.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
In order to validate a service name that has been specified as an
integer we need to run that as a pre-process validation step
*before* we pass the config to be validated against the schema.
It is not possible to validate it *in* the schema, it causes a
type error. Even though a number is a valid service name, it
must be a cast as a string within the yaml to avoid type error.
Taken this opportunity to move the code design in a direction
towards:
1. pre-process
2. validate
3. construct
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
We use $ref in the schema to allow us to specify multiple type, eg
command, it can be a string or a list of strings.
It required some extra parsing to retrieve a helpful type to display
in our error message rather than 'string or string'. Which while
correct, is not helpful. We value helpful.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
When a schema type is set as unique, we should display the validation
error to indicate that non-unique values have been provided for a key.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
The validation message was confusing by displaying only 1 level of
property of the service, even if the error was another level down.
Eg. if the 'files' property of 'extends' was the incorrect format,
it was displaying 'an invalid value for 'extends'', rather than
correctly retrieving 'files'.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Unfortunately the way that jsonschema is calling %r on its property
and then encoding the complete message means I've had to do this
manual way of removing the literal string prefix, u'.
eg:
key = 'extends'
message = "Invalid value for %r" % key
error.message = message.encode("utf-8")"
results in:
"Invalid value for u'extends'"
Performing a replace to strip out the extra "u'", does not change the
encoding of the string, it is at this point the character u followed
by a '.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
While it was intended as a positive to be stricter in validation
it would in fact break backwards compatibility, which we do not
want to be doing.
Consider re-visiting this later and include a deprecation warning if
we want to be stricter.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
If a container is in the process of being removed, or removal has
failed, it can sometimes appear in the output of GET /containers/json
but not have a 'Name' key. In that case, rather than crashing, we can
ignore it.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Rather than implement the logic a second time, use docker-py
split_port function to test if the ports is valid.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Move validation out into its own file without causing circular
import errors.
Fix some of the tests to import from the right place.
Also fix tests that were not using valid test data, as the validation
schema is now firing telling you that you couldn't "just" have this
dict without a build/image config key.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
jsonschema provides a rich error tree of info, by parsing each error
we can pull out relevant info and re-write the error messages.
This covers current error handling behaviour.
This includes new error handling behaviour for types and formatting of
the ports field.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
We validate the config against our schema before a service is created
so checking whether a service name is valid at time of instantiation of
the Service class is not needed.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Define a schema that we can pass to jsonschema to validate against the
config a user has supplied. This will help catch a wide variety of common
errors that occur.
If the config does not pass schema validation then it raises an exception
and prints out human readable reasons.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
- Better method names.
- Environment variable syntax in volume paths, even when a driver is
specified, now *will* be processed (the test wasn't testing it
properly). However, `~` will still *not* expand to the user's home
directory.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
* Add support for volume_driver parameter in compose yml
* Don't expand volume host paths if a volume_driver is specified
(i.e., disable compose feature "relative to absolute path
transformation" when volume drivers are in use, since volume drivers
can use name where host path is normally specified; this is a
heuristic)
Signed-off-by: Luke Marsden <luke@clusterhq.com>
When an image declares a volume such as `/var/lib/mysql`, and a Compose
file has a line like `./data:/var/lib/mysql/` (note the trailing slash),
Compose creates duplicate volume binds when *recreating* the container.
(The first container is created without a hitch, but contains multiple
entries in its "Volumes" config.)
Fixed by normalizing all paths in volumes config.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
It doesn't do much other than cause the remainder of the test suite to
generate lots of junk output.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Rather than creating a docker client within each test, create one
at setup and make it accessible to the whole class.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
When specifying a log_driver you want to specify some options for
the logger as per the docker run --log-opt option. The logger
options are key value pairs.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
By allowing the memswap_limit option to be defined we also need to
check that mem_limit is set, you can't have swap without a limit.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Also warn the user about the one-off containers in the standard error
message about legacy containers.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
This top level function is a test helper, so I've moved it into the
config_test file and updated accordingly.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
When building test data using make_service_dict, we need to include
working_dir as it is core to some of the functionality of
ServiceLoader.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
If you have an alternate YAML file with different services defined,
containers for those services will be shown in `docker-compose ps` even
if you don't pass that file in.
Furthermore, `docker-compose rm` will claim that it's going to remove
them, but actually won't.
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
When specifying the 'file' key to a value of it's own name, test
that this works and does not cause a false positive circular reference
error.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
This refactoring allows us to raise an error when there is no
'file' key specified in the .yml and no self.filename set. This
error was specific to the tests, as the tests are
the only place that constructs service dicts without sometimes
setting a filename.
Moving the function within the class as well as it is code that
is exclusively for the use of validating properties for the
ServiceLoader class.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Split them out into individual validation tests so it is clearer
to see what is going on and to enable adding further validation
tests.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
If the 'file' key is not set in the extends_options dict then we
look for the 'service' from within the same file.
Fixes this issue: https://github.com/docker/compose/issues/1237
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
This commit adds environment variable parsing to the container side
of the volume mapping in configs. The common use case for this is
mounting SSH agent sockets in a container, using code like:
volumes:
- $SSH_AUTH_SOCK:$SSH_AUTH_SOCK
environment:
- SSH_AUTH_SOCK
Signed-off-by: Jeff Kramer <jeff.kramer@voxmedia.com>
As VALID_CHARS is shared with project names, these chars are also
now allowed within project names.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
These tests were indeed raising a config error, but not for the reason
intended/tested for. I've added in the image name so the config error
raise is correctly testing the Service name.
Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
Daniel Nephin
Yves Peter
Aanand Prasad
Daniel Nephin
Kevin Greene
Adrian Budau
Joffrey F
Joffrey F
Mazz Mosley
mnowster
Viranch Mehta
Mohit Soni
Karol Duleba
Vincent Demeester
Vojta Orgon
Christophe Labouisse
Nick H
funkyfuture
Joel Hansson
Ben Firshman
Veres Lajos
Yuval Kohavi
Luke Marsden
Jeff Kramer
Sébastien Gruchet