Skip to content

Windows: hostconfig on start#14691

Merged
runcom merged 1 commit intomoby:masterfrom
microsoft:10662-start
Jul 20, 2015
Merged

Windows: hostconfig on start#14691
runcom merged 1 commit intomoby:masterfrom
microsoft:10662-start

Conversation

@lowenna
Copy link
Copy Markdown
Member

@lowenna lowenna commented Jul 16, 2015

Signed-off-by: John Howard [email protected]

@swernli

This PR is part of the proposal described in docker/docker issue 10662 to port the docker daemon to Windows. This addresses a backwards compatibility issue which doesn't affect Windows, hence the Windows daemon errors out. (Specifically if a hostConfig is passed to start.)

Signed-off-by: John Howard <[email protected]>
@cpuguy83
Copy link
Copy Markdown
Member

I'd prefer to just remove this for new API versions.

@cpuguy83
Copy link
Copy Markdown
Member

Also assuming we keep this, can't we just have daemon.setHostConfig be a no-op on windows in daemon_windows.go instead?

@lowenna
Copy link
Copy Markdown
Member Author

lowenna commented Jul 17, 2015

@cpuguy83 - I'm not sure what the policy is for deprecation? I'm just trying to make sure it isn't a problem on Windows ;)

@cpuguy83
Copy link
Copy Markdown
Member

@jhowardmsft doh! of course...

@lowenna
Copy link
Copy Markdown
Member Author

lowenna commented Jul 17, 2015

daemon.setHostConfig is called during create (the right place) as well as start. So I don't believe it can be a no-op on Windows.

@cpuguy83
Copy link
Copy Markdown
Member

This LGTM, but I'll probably open an issue about deprecating hostconfig on start for API versions > 1.19

@runcom
Copy link
Copy Markdown
Member

runcom commented Jul 20, 2015

LGMT, creating issue about deprecating hostConfig on start as well

runcom added a commit that referenced this pull request Jul 20, 2015
@runcom runcom merged commit 37d737f into moby:master Jul 20, 2015
@lowenna lowenna deleted the 10662-start branch July 21, 2015 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants