From 49e762ab3ae2618625439e7b6142cd7039096556 Mon Sep 17 00:00:00 2001 From: Alin-Gheorghe Balutoiu Date: Wed, 23 May 2018 09:52:37 +0200 Subject: [PATCH] Fix Windows CNI for the sandbox case Windows supports both sandbox and non-sandbox cases. The non-sandbox case is for Windows Server 2016 and for Windows Server version greater than 1709 which use Hyper-V containers. Currently, the CNI on Windows fetches the IP from the containers within the pods regardless of the mode. This should be done only in the non-sandbox mode where the IP of the actual container will be different than the IP of the sandbox container. In the case where the sandbox container is supported, all the containers from the same pod will share the network details of the sandbox container. This patch updates the CNI to fetch the IP from the sandbox container when this mode is supported. Signed-off-by: Alin Balutoiu --- pkg/kubelet/dockershim/docker_sandbox.go | 3 +- pkg/kubelet/dockershim/helpers_linux.go | 2 +- pkg/kubelet/dockershim/helpers_unsupported.go | 2 +- pkg/kubelet/dockershim/helpers_windows.go | 64 +++++++------------ 4 files changed, 28 insertions(+), 43 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index 13e9c42366e..6806232e115 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -412,8 +412,9 @@ func (ds *dockerService) PodSandboxStatus(ctx context.Context, req *runtimeapi.P var IP string // TODO: Remove this when sandbox is available on windows + // Currently windows supports both sandbox and non-sandbox cases. // This is a workaround for windows, where sandbox is not in use, and pod IP is determined through containers belonging to the Pod. - if IP = ds.determinePodIPBySandboxID(podSandboxID); IP == "" { + if IP = ds.determinePodIPBySandboxID(podSandboxID, r); IP == "" { IP = ds.getIP(podSandboxID, r) } diff --git a/pkg/kubelet/dockershim/helpers_linux.go b/pkg/kubelet/dockershim/helpers_linux.go index 5d231ea82a2..9b4cd9f02a8 100644 --- a/pkg/kubelet/dockershim/helpers_linux.go +++ b/pkg/kubelet/dockershim/helpers_linux.go @@ -136,7 +136,7 @@ func (ds *dockerService) updateCreateConfig( return nil } -func (ds *dockerService) determinePodIPBySandboxID(uid string) string { +func (ds *dockerService) determinePodIPBySandboxID(uid string, sandbox *dockertypes.ContainerJSON) string { return "" } diff --git a/pkg/kubelet/dockershim/helpers_unsupported.go b/pkg/kubelet/dockershim/helpers_unsupported.go index 2867898f301..0abc271b1d0 100644 --- a/pkg/kubelet/dockershim/helpers_unsupported.go +++ b/pkg/kubelet/dockershim/helpers_unsupported.go @@ -45,7 +45,7 @@ func (ds *dockerService) updateCreateConfig( return nil } -func (ds *dockerService) determinePodIPBySandboxID(uid string) string { +func (ds *dockerService) determinePodIPBySandboxID(uid string, sandbox *dockertypes.ContainerJSON) string { glog.Warningf("determinePodIPBySandboxID is unsupported in this build") return "" } diff --git a/pkg/kubelet/dockershim/helpers_windows.go b/pkg/kubelet/dockershim/helpers_windows.go index 0c54bea03d4..6b3665e876f 100644 --- a/pkg/kubelet/dockershim/helpers_windows.go +++ b/pkg/kubelet/dockershim/helpers_windows.go @@ -83,7 +83,28 @@ func (ds *dockerService) updateCreateConfig( return nil } -func (ds *dockerService) determinePodIPBySandboxID(sandboxID string) string { +func (ds *dockerService) determinePodIPBySandboxID(sandboxID string, sandbox *dockertypes.ContainerJSON) string { + // Versions and feature support + // ============================ + // Windows version >= Windows Server, Version 1709, Supports both sandbox and non-sandbox case + // Windows version == Windows Server 2016 Support only non-sandbox case + // Windows version < Windows Server 2016 is Not Supported + + // Sandbox support in Windows mandates CNI Plugin. + // Presence of CONTAINER_NETWORK flag is considered as non-Sandbox cases here + // Hyper-V isolated containers are also considered as non-Sandbox cases + + // Todo: Add a kernel version check for more validation + + // Hyper-V only supports one container per Pod yet and the container will have a different + // IP address from sandbox. Retrieve the IP from the containers as this is a non-Sandbox case. + // TODO(feiskyer): remove this workaround after Hyper-V supports multiple containers per Pod. + if networkMode := os.Getenv("CONTAINER_NETWORK"); networkMode == "" && sandbox.HostConfig.Isolation != kubeletapis.HypervIsolationValue { + // Sandbox case, fetch the IP from the sandbox container. + return ds.getIP(sandboxID, sandbox) + } + + // Non-Sandbox case, fetch the IP from the containers within the Pod. opts := dockertypes.ContainerListOptions{ All: true, Filters: dockerfilters.NewArgs(), @@ -103,45 +124,8 @@ func (ds *dockerService) determinePodIPBySandboxID(sandboxID string) string { continue } - // Versions and feature support - // ============================ - // Windows version == Windows Server, Version 1709,, Supports both sandbox and non-sandbox case - // Windows version == Windows Server 2016 Support only non-sandbox case - // Windows version < Windows Server 2016 is Not Supported - - // Sandbox support in Windows mandates CNI Plugin. - // Presence of CONTAINER_NETWORK flag is considered as non-Sandbox cases here - - // Todo: Add a kernel version check for more validation - - if networkMode := os.Getenv("CONTAINER_NETWORK"); networkMode == "" { - // 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, - // This is passed down to the platform to replicate some necessary information to the new container - - // - // This place is chosen as a hack for now, since ds.getIP would end up calling CNI's addToNetwork - // That is why addToNetwork is required to be idempotent - - // Instead of relying on this call, an explicit call to addToNetwork should be - // done immediately after ContainerCreation, in case of Windows only. TBD Issue # to handle this - - if r.HostConfig.Isolation == kubeletapis.HypervIsolationValue { - // 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 { - // ds.getIP will call the CNI plugin to fetch the IP - if containerIP := ds.getIP(c.ID, r); containerIP != "" { - return containerIP - } + if containerIP := ds.getIP(c.ID, r); containerIP != "" { + return containerIP } }