Skip to content

Add init support in 3.7 schema - #1129

Merged
silvin-lubecki merged 2 commits into
docker:masterfrom
vdemeester:init-in-composefile
Jun 25, 2018
Merged

Add init support in 3.7 schema#1129
silvin-lubecki merged 2 commits into
docker:masterfrom
vdemeester:init-in-composefile

Conversation

@vdemeester

@vdemeestervdemeester commented Jun 18, 2018

Copy link
Copy Markdown
Collaborator

Run an init inside the container that forwards signals and reaps
processes

This is supported on run and now on Swarm services too, so it's also
possible to have in on a composefile :).

Signed-off-by: Vincent Demeester vincent@sbr.pm

follow up to #479
fixes#51
fixesmoby/moby#34639
fixesmoby/swarmkit#2173

- What I did

- How I did it

- How to verify it

- Description for the changelog

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

Comment threadcli/compose/loader/loader_test.go Outdated
image: alpine
init: true`)
assert.NilError(t, err)
assert.Check(t, is.DeepEqual(config, &types.Config{

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.

I think you can just check the Init value, checking the equality on all the Config is a little bit extreme here:

assert.Equal(t, isconfig.Services[0].Init, &booleanTrue)

You may argue that then you have to check Services length before, but I think we can skip it 😇

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.

Then you can refactor the test with these 3 cases using Table Tests.

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

but agree that just checking for the boolean may be sufficient in the tests

@thaJeztah

Copy link
Copy Markdown
Member

ping @silvin-lubecki PTAL; is the test a blocker for you, or ok to merge as-is?

vdemeesterand others added 2 commits June 25, 2018 11:13
> Run an init inside the container that forwards signals and reaps
processes
This is supported on `run` and now on Swarm services too, so it's also
possible to have in on a composefile :).
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>

@vdemeestervdemeester left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

LGTM on @silvin-lubecki commit 👼

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants

@vdemeester@thaJeztah@silvin-lubecki@GordonTheTurtle