-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Add support of hyperv isolation for windows containers #58751
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,9 +26,19 @@ import ( | |
dockercontainer "github.com/docker/docker/api/types/container" | ||
dockerfilters "github.com/docker/docker/api/types/filters" | ||
"github.com/golang/glog" | ||
|
||
utilfeature "k8s.io/apiserver/pkg/util/feature" | ||
"k8s.io/kubernetes/pkg/features" | ||
runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" | ||
) | ||
|
||
const ( | ||
hypervIsolationAnnotationKey = "experimental.windows.kubernetes.io/isolation-type" | ||
|
||
// Refer https://aka.ms/hyperv-container. | ||
hypervIsolation = "hyperv" | ||
) | ||
|
||
func DefaultMemorySwap() int64 { | ||
return 0 | ||
} | ||
|
@@ -40,18 +50,40 @@ func (ds *dockerService) getSecurityOpts(seccompProfile string, separator rune) | |
return nil, nil | ||
} | ||
|
||
func shouldIsolatedByHyperV(annotations map[string]string) bool { | ||
if !utilfeature.DefaultFeatureGate.Enabled(features.HyperVContainer) { | ||
return false | ||
} | ||
|
||
v, ok := annotations[hypervIsolationAnnotationKey] | ||
return ok && v == hypervIsolation | ||
} | ||
|
||
// applyExperimentalCreateConfig applys experimental configures from sandbox annotations. | ||
func applyExperimentalCreateConfig(createConfig *dockertypes.ContainerCreateConfig, annotations map[string]string) { | ||
if shouldIsolatedByHyperV(annotations) { | ||
createConfig.HostConfig.Isolation = hypervIsolation | ||
|
||
if networkMode := os.Getenv("CONTAINER_NETWORK"); networkMode == "" { | ||
createConfig.HostConfig.NetworkMode = dockercontainer.NetworkMode("none") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean that hyperV isolated containers will not have any networking by default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, hyperV container's network will be set by CNI plugins |
||
} | ||
} | ||
} | ||
|
||
func (ds *dockerService) updateCreateConfig( | ||
createConfig *dockertypes.ContainerCreateConfig, | ||
config *runtimeapi.ContainerConfig, | ||
sandboxConfig *runtimeapi.PodSandboxConfig, | ||
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error { | ||
if networkMode := os.Getenv("CONTAINER_NETWORK"); networkMode != "" { | ||
createConfig.HostConfig.NetworkMode = dockercontainer.NetworkMode(networkMode) | ||
} else { | ||
} else if !shouldIsolatedByHyperV(sandboxConfig.Annotations) { | ||
// Todo: Refactor this call in future for calling methods directly in security_context.go | ||
modifyHostNetworkOptionForContainer(false, podSandboxID, createConfig.HostConfig) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For Hyper-V container, no need to call modifyHostNetworkOptionForContainer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack, will fix |
||
} | ||
|
||
applyExperimentalCreateConfig(createConfig, sandboxConfig.Annotations) | ||
|
||
return nil | ||
} | ||
|
||
|
@@ -87,8 +119,17 @@ func (ds *dockerService) determinePodIPBySandboxID(sandboxID string) string { | |
// Todo: Add a kernel version check for more validation | ||
|
||
if networkMode := os.Getenv("CONTAINER_NETWORK"); networkMode == "" { | ||
// Do not return any IP, so that we would continue and get the IP of the Sandbox | ||
ds.getIP(sandboxID, r) | ||
if r.HostConfig.Isolation == hypervIsolation { | ||
// Hyper-V only supports one container per Pod yet and the container will have a different | ||
// IP address from sandbox. Return the first non-sandbox container IP as POD IP. | ||
// TODO(feiskyer): remove this workaround after Hyper-V supports multiple containers per Pod. | ||
if containerIP := ds.getIP(c.ID, r); containerIP != "" { | ||
return containerIP | ||
} | ||
} else { | ||
// Do not return any IP, so that we would continue and get the IP of the Sandbox | ||
ds.getIP(sandboxID, r) | ||
} | ||
} else { | ||
// On Windows, every container that is created in a Sandbox, needs to invoke CNI plugin again for adding the Network, | ||
// with the shared container name as NetNS info, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need apply the isolation specific config to sandbox? For Windows Hyper-V container, the POD infra container is not used and needs to be minimal, and therefore process container is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not used now, but not future. I think we'd better keep sandbox same with containers, WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine with me. We can reserve it as place holder.