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

Only recreate what's changed #1399

Merged
merged 3 commits into from
May 18, 2015
Merged

Only recreate what's changed #1399

merged 3 commits into from
May 18, 2015

Conversation

aanand
Copy link

@aanand aanand commented May 8, 2015

Closes #1345. To try it out:

$ docker-compose up -d --x-smart-recreate

Here's what needs fixing:

  • If the image tag (whether arrived at via image or build) has changed, this should trigger a recreate.
  • When a service is recreated, dependent services via links, volumes_from and net:container should also be recreated, whether or not their configuration has changed.
  • If a container is up-to-date but not running, it should be started.

@aanand
Copy link
Author

aanand commented May 9, 2015

Edit: this is now implemented.

After giving it some thought, it'd be far easier to just label every container with a hash generated from the service dict that was used to create it, and compare hashes to see if a container needs recreating.

This means we can avoid doing any of the configuration diffing stuff, which is (a) hairy to implement (b) full of corner cases (c) going to be a total pain to debug.

Thoughts?

@dnephin
Copy link

dnephin commented May 9, 2015

I agree, I like the idea of using a hash. It should make things a lot easier.

@aanand aanand force-pushed the state branch 4 times, most recently from 9326dbd to 83923d5 Compare May 12, 2015 21:12
@aanand
Copy link
Author

aanand commented May 12, 2015

Pushed an initial implementation using hashes.

@aanand
Copy link
Author

aanand commented May 14, 2015

Downstream dependent services are now recreated when an upstream dependency is recreated.

@aanand aanand force-pushed the state branch 3 times, most recently from 1bdfee9 to 2a8210e Compare May 14, 2015 18:06
@aanand aanand changed the title WIP: Only recreate what's changed Only recreate what's changed May 14, 2015
@aanand aanand force-pushed the state branch 2 times, most recently from 8fefbb2 to b4cd02c Compare May 14, 2015 18:30
@aanand
Copy link
Author

aanand commented May 14, 2015

This is now ready for review.

@aanand
Copy link
Author

aanand commented May 14, 2015

One thing worth noting: the --no-build flag has been rendered somewhat useless, as we now have to hit the API (to determine the image ID) before creating containers. Still, we're calling inspect_image() rather than images(), so perhaps it won't be a huge performance regression.

Compose now shows a warning if you pass --no-build but the image doesn't exist.

If we're just streaming logs from `docker-compose up`, we don't need
to set AttachStdin/out/err, and doing so results in containers with
different configuration depending on whether `up` or `run` were invoked
with `-d` or not.

Signed-off-by: Aanand Prasad <[email protected]>
@aanand
Copy link
Author

aanand commented May 18, 2015

Just rebasing this now that #1356 is in!

and plans[name].action == 'recreate'
]

if updated_dependencies:
Copy link

Choose a reason for hiding this comment

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

When we were chatting about #1395 I suggested to support the feature we'd probably want to store the DAG of the services in compose.project.sort_service_dicts() instead of flattening it immediately.

I think that data structure would make this easier as well. Instead of having to lookup the dependencies for each service, we could immediately add recreate plans for all dependencies. When it's available we could consider using it here.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I think that'd be a good refactor as part of implementing #1395.

@aanand aanand force-pushed the state branch 2 times, most recently from 6f29b7a to a863857 Compare May 18, 2015 17:05
@aanand
Copy link
Author

aanand commented May 18, 2015

Rebased and removed converge() from Service.

@aanand
Copy link
Author

aanand commented May 18, 2015

Whoops, the tests are still using converge(). I'll move it into a test helper in a separate PR.

@aanand
Copy link
Author

aanand commented May 18, 2015

Build is green.

@dnephin
Copy link

dnephin commented May 18, 2015

LGTM

aanand added a commit that referenced this pull request May 18, 2015
Only recreate what's changed
@aanand aanand merged commit 80eaf4c into docker:master May 18, 2015
@aanand aanand deleted the state branch May 18, 2015 18:25
@prologic
Copy link

This feature isn't it any published version of docker-compose yet right?

@wader
Copy link

wader commented Aug 24, 2015

@prologic it was introduced in 1.3

@prologic
Copy link

As a default (no special cil option) feature? :)

@wader
Copy link

wader commented Aug 24, 2015

Sorry, not default, as a option to up, see docker-compose up --help

@prologic
Copy link

dc up --help
    --force-recreate       Recreate containers even if their configuration and
                           image haven't changed. Incompatible with --no-recreate.

This one I guess :)

@wader
Copy link

wader commented Aug 24, 2015

Hmm what version is that?

$ docker-compose up --help | grep recreate
    --x-smart-recreate     Only recreate containers whose configuration or
    --no-recreate          If containers already exist, don't recreate them.

@prologic
Copy link

$ dc --version
docker-compose version: 1.4.0

$ dc up --help | grep recreate
volumes). To prevent Compose from picking up changes, use the `--no-recreate`
If you want to force Compose to stop and recreate all containers, use the
`--force-recreate` flag.
    --force-recreate       Recreate containers even if their configuration and
                           image haven't changed. Incompatible with --no-recreate.
    --no-recreate          If containers already exist, don't recreate them.

@wader
Copy link

wader commented Aug 24, 2015

Your totally right, i'm running 1.3.1 and should probably update :)

From 1.4 changelog https://github.com/docker/compose/blob/master/CHANGELOG.md

The experimental --x-smart-recreate flag which introduced this feature in Compose 1.3.0 has been removed, and a --force-recreate flag has been added for when you want to recreate everything.

@prologic
Copy link

👍 Awesome :)

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.

Only recreate containers whose configuration needs updating
5 participants