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

Restore ability to override an included resource in compose.override.yaml #559

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Feb 4, 2024

This restore the (de facto) feature to override an included resource definition using a compose.override.yaml file. Also need to document this pattern so that this doesn't just work by chance and users can safely rely on it.

closes docker/compose#11404

image

@luciangabor
Copy link

Sorry to intervene here, but, generalizing the original bug report, I think that there's a simpler solution. Please bear with me.

To begin with, I have v2.33.3 installed as a plugin (and used as reference), and v.2.24.5 installed in the PATH:

$ docker compose version
Docker Compose version v2.23.3

$ docker-compose version
Docker Compose version v2.24.5

then, let's say I have an included service defined in ... include.yaml:

services:
  included:
    image: included

and it's ... included in compose.yaml, alongside a default service:

include:
  - path:
      - included.yaml

services:
  default:
    image: default

nothing special yet, with both versions, I get the same result:

$ # this diffs the outputs of the config command for both versions,
$ # while mirroring on stderr the output of the reference version

$ diff -q <(docker compose -f compose.yaml config | tee /dev/stderr) <(docker-compose -f compose.yaml config)
name: inc
services:
  default:
    image: default
    networks:
      default: null
  included:
    image: included
    networks:
      default: null
networks:
  default:
    name: inc_default

now, let's say that I have an optional service defined in ... yep ... optional.yaml:

services:
  optional:
    image: optional

and, when merged using -fs, it looks like this:

$ diff -q <(docker compose -f compose.yaml -f optional.yaml config | tee /dev/stderr) <(docker-compose -f compose.yaml -f optional.yaml config)
name: inc
services:
  default:
    image: default
    networks:
      default: null
  included:
    image: included
    networks:
      default: null
  optional:
    image: optional
    networks:
      default: null
networks:
  default:
    name: inc_default

now, if I want to automate this I would include the optional service using compose.override.yaml,
but, and here's the generalization, I'll also include an override.yaml,
which I'll use to override (e.g. by adding an image tag) all services:

services:
  default:
    image: default:override
  included:
    image: included:override
  optional:
    image: optional:override

and here comes the test case:

$ ls -1 *.yaml
compose.override.yaml
compose.yaml
included.yaml
optional.yaml
override.yaml

$ diff -q <(docker compose config | tee /dev/stderr) <(docker-compose config)
services.default conflicts with imported resource
name: inc
services:
  default:
    image: default:override
    networks:
      default: null
  included:
    image: included:override
    networks:
      default: null
  optional:
    image: optional:override
    networks:
      default: null
networks:
  default:
    name: inc_default
Files /dev/fd/63 and /dev/fd/62 differ

$ # to make it clearer

$ docker compose config
name: inc
services:
  default:
    image: default:override
    networks:
      default: null
  included:
    image: included:override
    networks:
      default: null
  optional:
    image: optional:override
    networks:
      default: null
networks:
  default:
    name: inc_default

$ docker-compose config
services.default conflicts with imported resource

So, I think that the include mechanism, can be viewed as a special case of using multiple -fs.

That is:

  • it's similar with docker compose -f compose.yaml -f include.yaml,
  • with the additional constraint that include.yaml may not "touch" anything from compose.yaml,
  • since the tail can't wag the dog, only the other way around,
  • but everything has to look as if the contents of include.yaml
  • is "pasted" after the contents of compose.yaml,
  • thus it's impossible to override anything from the included file,
  • since that would "look" like a double definition of the same service.

Yet, I think that another solution would be to give up on the additional constraint,
and "see" the mechanism a bit differently (arguably, simpler too):

  • as similar with docker compose -f include.yaml -f compose.yaml (please note the reversed order),
  • with the "tweak" that the project's defaults are determined by the including file.

But, it's just an opinion, and it conflicts with the view that the included services are sealed.
(Well, they are not really sealed as it can be viewed from the test case above.)

And to finish, I really appreciate your work, and (but I can't recollect where)
I remember reading that having your software being used in an unexpected way is a Good Thing (TM) :D.

@luciangabor
Copy link

Oh, here's the content of compose.override.yaml:

include:
  - path:
      - optional.yaml
      - override.yaml

@ndeloof ndeloof marked this pull request as ready for review February 5, 2024 13:43
@ndeloof
Copy link
Collaborator Author

ndeloof commented Feb 5, 2024

@luciangabor what you describe here would make include just a higher-level extends. Maybe we will consider this feasible at some point, but then this would deserve a dedicated keyword (maybe can just reuse extends as top-level element, as the merge rules would be comparable)

Copy link
Collaborator

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@glours glours enabled auto-merge (rebase) February 6, 2024 10:41
@glours glours merged commit 58f2fbb into compose-spec:main Feb 6, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Got services.NAME conflicts with imported resource after update from 2.21.0 to 2.24.1
3 participants