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

docker commit should not drop configuration #1141

Closed
mhennings opened this issue Jul 5, 2013 · 22 comments · Fixed by #4000
Closed

docker commit should not drop configuration #1141

mhennings opened this issue Jul 5, 2013 · 22 comments · Fixed by #4000

Comments

@mhennings
Copy link
Contributor

Currently when doing a commit on a container to update an image, the configuration the image had is not kept.

I think there should be an option to drop the previous config and a default to keep it.

@fkautz
Copy link
Contributor

fkautz commented Aug 16, 2013

What configuration are you thinking of?

E.g. dns? mounted volumes?

@mhennings
Copy link
Contributor Author

i had volumes and the cmd in mind.

@asbjornenge
Copy link
Contributor

👍 this hit me hard today :-/ Built up quite a huge Dockerfile with lots of ports EXPOSEd. If I do some work in the container and want to commit as a new image, all ports and also CMD is lost.

@jpetazzo
Copy link
Contributor

+1

On Fri, Sep 13, 2013 at 9:55 AM, asbjornenge notifications@github.comwrote:

[image: 👍] this hit me hard today :-/ Built up quite a huge Dockerfile
with lots of ports EXPOSEd. If I do some work in the container and want to
commit as a new image, all ports and also CMD is lost.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1141#issuecomment-24408559
.

@jpetazzo https://twitter.com/jpetazzo
Latest blog post: http://blog.docker.io/2013/09/docker-joyent-openvpn-bliss/

@taylortrimble
Copy link

This was a big one for me today as well!

@asbjornenge
Copy link
Contributor

A friendly tip (might be obvious to most):

As a workaround I just keep a separate Dockerfile with only the configuration stuff that is lost, and the image I'm committing to as FROM. It's not awesome, but it works.

@taylortrimble
Copy link

Haha! A dirty hack, but I will use it 'til we see this closed!

Thanks. :)

@vampolo
Copy link

vampolo commented Sep 30, 2013

+1

@icook
Copy link

icook commented Oct 12, 2013

+1

1 similar comment
@athieriot
Copy link

+1

@BMorearty
Copy link

+1

In addition, docker history imagename should show the configuration overrides that were passed via the -run option to docker commit. So you can see the run configuration.

I know you can do docker inspect imagename but that's all the configuration, not just the overrides from the command line. It's hard to see what the deltas are.

@creack
Copy link
Contributor

creack commented Nov 6, 2013

ping @shykes, any thoughts?

@parente
Copy link

parente commented Dec 4, 2013

+1 on preserving cmd, ports, env, etc.

@shykes
Copy link
Contributor

shykes commented Jan 6, 2014

I believe this has been fixed, @creack @crosbymichael @vieux can you confirm?

@crosbymichael
Copy link
Contributor

No this is still and issue with docker commit. It was been fixed with docker build but still an issue with commit.

Tagging as easy fix

@cap10morgan
Copy link
Contributor

Any progress on this? Got bit by this today on 0.7.6.

@cap10morgan
Copy link
Contributor

I'm working on this issue, and it is indeed mostly an easy fix. But the wrinkle I've run into is figuring out when the user has specified a new config via commit's -run option. Hopefully there's an easy solution for someone who's been using Go for more than a few minutes.

Here's my progress so far: https://github.com/cap10morgan/docker/compare/fix-issue-1141

The if false is where I need to figure out a way to tell if the user submitted a new config (and then do we merge or replace?).

I've tried comparing the Config struct to an empty one (Config{}), but that doesn't work because there are slice members and they don't support equality comparison.

I've also tried checking for no incoming config JSON, but something in the command -> API -> server cycle is putting an empty struct's JSON representation in there anyway, so that's stuck at the same place.

So does anyone have any tips for a robust way of detecting when the user has submitted a new config on the commit command line?

OR

Should I just work on a shallow merge function that leaves the original container's config values there when there is no value in the request's config but otherwise clobbers it with the request's config values? This would be more work, I think, but if it's the right thing to do (and it seems like it is), then it would obviate the need to detect whether someone had given any config values via the -run option (since they'll all be empty and thus won't override the original container's config).

@cap10morgan
Copy link
Contributor

Update: I just found CompareConfig and MergeConfig in utils.go. I'll just use MergeConfig.

cap10morgan added a commit to cap10morgan/docker that referenced this issue Feb 1, 2014
@cap10morgan
Copy link
Contributor

Pull request here: #3889

@prologic
Copy link
Contributor

prologic commented Feb 1, 2014

+1

@cap10morgan
Copy link
Contributor

I closed that PR so I could submit a new one that follows the Docker contrib guidelines a bit better and adds a test. However, I'm a bit stuck on the best way to test that the merged config is correct: cap10morgan@2cf4727

Any tips would be most welcome.

cap10morgan added a commit to cap10morgan/docker that referenced this issue Feb 28, 2014
Fixes moby#1141

Docker-DCO-1.1-Signed-off-by: Wes Morgan <cap10morgan@gmail.com> (github: cap10morgan)
@mattva01
Copy link

mattva01 commented Mar 9, 2014

+1

unclejack pushed a commit to unclejack/moby that referenced this issue Mar 18, 2014
Fixes moby#1141

Docker-DCO-1.1-Signed-off-by: Wes Morgan <cap10morgan@gmail.com> (github: cap10morgan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.