Skip to content

Enable Hotadd support for windows#1810

Closed
madhanrm wants to merge 1 commit intomoby:masterfrom
madhanrm:hotadd
Closed

Enable Hotadd support for windows#1810
madhanrm wants to merge 1 commit intomoby:masterfrom
madhanrm:hotadd

Conversation

@madhanrm
Copy link
Copy Markdown

@madhanrm madhanrm commented Jun 16, 2017

Hot add for windows requires containerID to be passed down to hcsshim, which interally finds the right container and connects the nic.

@madhanrm
Copy link
Copy Markdown
Author

ping @mavenugo @msabansal

@madhanrm
Copy link
Copy Markdown
Author

madhanrm commented Jul 7, 2017

ping @mavenugo

Signed-off-by: Madhan Raj Mookkandy <madhanm@microsoft.com>
@madhanrm
Copy link
Copy Markdown
Author

ping @msabansal @mavenugo Can you look into this? It has been pending for a long time.

@msabansal
Copy link
Copy Markdown
Contributor

LGTM

}

// This is just a stub for now
endpoint.containerID = sboxKey
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.

As per the design, containerID is internal to libnetwork core while the drivers shouldnt assume the presence of container-id. SandboxKey is the generic concept that can be trusted without assuming any changes to the core.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, I understand that.
But libnetwork's design principle will not work well with Windows, because there is a service "HCS" that manages all the containers. So for any changes to the container, HCS has to be notified of that change. For this the ID of the container is required. This is the basic design principle of Windows Containers.

To explain the internals more, Like CreateComputeSystem HCS call, for Hot Add/Remove of network endpoints, ModifyCompyteSystem HCS call has to be invoked.

Unlike Linux, on Windows, Containers and Networks are not yet decoupled completely to fit into LibNetwork design principle. Once that is done (am not sure when), we could drop the containerID.

Till then windows needs containerID for hot add to work.

Comment thread sandbox_windows.go
if sb.config.useDefaultSandBox {
return osl.GenerateKey("default")
}
return osl.GenerateKey(sb.containerID)
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.

As explained above, this change goes against the current design

@pradipd
Copy link
Copy Markdown
Contributor

pradipd commented Oct 9, 2017

We can close this request.
The change was merged in a separate PR: #1964

@fcrisciani
Copy link
Copy Markdown

as per request from @pradipd

@fcrisciani fcrisciani closed this Oct 9, 2017
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.

6 participants