Skip to content
This repository was archived by the owner on Jul 18, 2025. It is now read-only.

docs: Update compatibility.md#385

Merged
chris-crone merged 1 commit intodocker-archive-public:masterfrom
Dimrok:feature/docs
Oct 2, 2018
Merged

docs: Update compatibility.md#385
chris-crone merged 1 commit intodocker-archive-public:masterfrom
Dimrok:feature/docs

Conversation

@Dimrok
Copy link
Copy Markdown
Contributor

@Dimrok Dimrok commented Sep 17, 2018

- What I did

  • I merged the two matrices to get a clearer view.
  • I added docker-compose
  • I added version 2.x

- Description for the changelog

Update documentation regarding composefile version compatibility.

- A picture of a cute animal (not mandatory but encouraged)

image
(credits: u/kaitlynsara06 from reddit)

Comment thread docs/compatibility.md Outdated
| - order | SK | SK | SK | SK | | | | | | | | | |
| - parallelism | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - devices | C | C | C | C | C | C | C | C | C | C | C | C | C |
| - depends_on | CS | CS | CS | CS | CS | CS | CS | CS | CS | CS | CS | CS | CS |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't work on swarm, there is no notion of depends_on in swarm, the key is ignored (should be in UnsupportedProperties, not sur why it's not…)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The depends_on option is ignored when deploying a stack in swarm mode with a version 3 Compose file.

Good catch.

Comment thread docs/compatibility.md Outdated
| - timeout | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - hostname | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - image | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - init | * | | | | | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably not support in Kubernetes if it's in 3.7, might be wrong though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread docs/compatibility.md Outdated
| - ipc | CK | CK | CK | CK | CK | CK | CK | CK | CK | CK | CK | CK | CK |
| - isolation | CS | CS | CS | | | | | | | | | | |
| - labels | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - links | | | | | | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be supported in C (compose) I think

Comment thread docs/compatibility.md Outdated
| - working_dir | Y | Y | Y | Y | Y | Y | Y |
| Features | 3.7 | 3.6 | 3.5 | 3.4 | 3.3 | 3.2 | 3.1 | 3.0 | 2.4 | 2.3 | 2.2 | 2.1 | 2.0 |
|------------------------------|-----|-----|-----|-----|-----|-----|-----|-----|-----|-----|-----|-----|-----|
| configs | * | * | * | * | * | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Config should work the same way secret are…

Comment thread docs/compatibility.md Outdated
| - ports | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - mode: | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - host | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? |
| - ingress | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? | CS? |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ingress is swarm only

Comment thread docs/compatibility.md Outdated
| - volume | * | * | * | * | * | * | | | | | | | |
| - bind | * | * | * | * | * | * | | | | | | | |
| - tmpfs | * | * | * | * | * | * | | | | | | | |
| - bind | CS | CS | CS | CS | CS | CS | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Didn't we had support for bind mount in k8s ? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@shin- shin- left a comment

Choose a reason for hiding this comment

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

A few notes on Compose support. Also, indentation doesn't work inside table cells.

Comment thread docs/compatibility.md Outdated
| - working_dir | Y | Y | Y | Y | Y | Y | Y |
| Features | 3.7 | 3.6 | 3.5 | 3.4 | 3.3 | 3.2 | 3.1 | 3.0 | 2.4 | 2.3 | 2.2 | 2.1 | 2.0 |
|------------------------------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
| configs.<name> (long syntax) | * | * | * | * | * | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

configs is SK - Compose doesn't have support: docker/compose#5739

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad, I saw

    def get_configs(self):
        return {} if self.version < const.COMPOSEFILE_V3_3 else self.config.get('configs', {})

and the doc says

Defining and using configs in compose files
Both the `docker compose` and `docker stack` commands support defining configs in a compose file. See the Compose file reference for details.

I assumed it was supported.

Comment thread docs/compatibility.md
Comment thread docs/compatibility.md Outdated
| - configs | * | * | * | * | * | | | | | | | | |
| - cgroup_parent | C | C | C | C | C | C | C | C | C | C | C | C | C |
| - container_name | C | C | C | C | C | C | C | C | C | C | C | C | C |
| - credential_spec | * | * | * | * | * | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that support for credential_spec in Compose is pending: docker/compose#5488

Comment thread docs/compatibility.md Outdated
| - labels | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - mode | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - placement | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - replicas | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Supported in Compose if compatibility mode is enabled: https://docs.docker.com/compose/compose-file/compose-versioning/#compatibility-mode

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread docs/compatibility.md Outdated
| - resources | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - limits | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - reservations | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - restart_policy | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Partially supported in Compose if compatibility mode is enabled: https://docs.docker.com/compose/compose-file/compose-versioning/#compatibility-mode

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread docs/compatibility.md Outdated
| - mode | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - placement | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - replicas | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
| - resources | SK | SK | SK | SK | SK | SK | SK | SK | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Supported in Compose if compatibility mode is enabled: https://docs.docker.com/compose/compose-file/compose-versioning/#compatibility-mode

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 21, 2018

Codecov Report

Merging #385 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #385   +/-   ##
=======================================
  Coverage   58.34%   58.34%           
=======================================
  Files          59       59           
  Lines        3260     3260           
=======================================
  Hits         1902     1902           
  Misses       1093     1093           
  Partials      265      265

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35db18a...088131f. Read the comment docs.

Comment thread docs/compatibility.md
| - network | C | C | C | C | C | C | C | C | C | C | C | C | C |
| - shm_size | C | C | C | C | C | C | C | C | C | C | C | C | C |
| - target | C | C | C | C | C | C | C | C | C | C | C | C | C |
| - cap_add | CK | CK | CK | CK | CK | CK | CK | C | C | C | C | C | C |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just checked, cap_add/cap_drop are ignored in kube-compose

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just re-checked they are not, just wasn't looking at the right place

Comment thread docs/compatibility.md Outdated
| - timeout | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - hostname | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - image | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - init | CK | | | | | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

seemed to be ignored in kube-compose

@chris-crone
Copy link
Copy Markdown
Contributor

Ping @vdemeester and @simonferquel to update their reviews

Copy link
Copy Markdown
Contributor

@shin- shin- left a comment

Choose a reason for hiding this comment

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

1 last thing

Comment thread docs/compatibility.md Outdated
| - working_dir | Y | Y | Y | Y | Y | Y | Y |
| Features | 3.7 | 3.6 | 3.5 | 3.4 | 3.3 | 3.2 | 3.1 | 3.0 | 2.4 | 2.3 | 2.2 | 2.1 | 2.0 |
|------------------------------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
| configs.<name> (long syntax) | CK | CK | CK | CK | CK | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The whole configs section should be SK, not CK

Signed-off-by: Antony MECHIN <antony.mechin@docker.com>
@chris-crone
Copy link
Copy Markdown
Contributor

Ping @shin- for a review update

Copy link
Copy Markdown
Contributor

@shin- shin- left a comment

Choose a reason for hiding this comment

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

1 instance of configs was missed

Comment thread docs/compatibility.md
| - cpu_quota | | | | | | | | | C | C | C | C | C |
| - cpuset | | | | | | | | | C | C | C | C | C |
| - command | * | * | * | * | * | * | * | * | * | * | * | * | * |
| - configs | CK | CK | CK | CK | CK | | | | | | | | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since @Dimrok is on PTO, can we merge and follow up?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, intentionally left as a comment as to not block this any further. Merge away!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, intentionally left as a comment as to not block this any further. Merge away!

@ijc
Copy link
Copy Markdown
Contributor

ijc commented Oct 2, 2018

This seems like a pretty massive/intractable PR review-wise, given that it clearly contains an improvement over the status quo perhaps we should merge it and correct that last few remaining inaccuracies in a followup (or followups)?

@chris-crone chris-crone merged commit 00b2e92 into docker-archive-public:master Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants