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

dockertools: call TearDownPod when GC-ing infra pods #37036

Merged
merged 7 commits into from
Feb 17, 2017
2 changes: 1 addition & 1 deletion pkg/kubelet/dockershim/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ go_test(
"//pkg/kubelet/dockertools:go_default_library",
"//pkg/kubelet/dockertools/securitycontext:go_default_library",
"//pkg/kubelet/network:go_default_library",
"//pkg/kubelet/network/mock_network:go_default_library",
"//pkg/kubelet/network/testing:go_default_library",
"//pkg/kubelet/types:go_default_library",
"//pkg/kubelet/util/cache:go_default_library",
"//pkg/security/apparmor:go_default_library",
Expand Down
12 changes: 6 additions & 6 deletions pkg/kubelet/dockershim/docker_sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (ds *dockerService) RunPodSandbox(config *runtimeapi.PodSandboxConfig) (str
// on the host as well, to satisfy parts of the pod spec that aren't
// recognized by the CNI standard yet.
cID := kubecontainer.BuildContainerID(runtimeName, createResp.ID)
err = ds.networkPlugin.SetUpPod(config.GetMetadata().Namespace, config.GetMetadata().Name, cID)
err = ds.network.SetUpPod(config.GetMetadata().Namespace, config.GetMetadata().Name, cID)
// TODO: Do we need to teardown on failure or can we rely on a StopPodSandbox call with the given ID?
return createResp.ID, err
}
Expand Down Expand Up @@ -162,8 +162,8 @@ func (ds *dockerService) StopPodSandbox(podSandboxID string) error {
errList := []error{}
if needNetworkTearDown {
cID := kubecontainer.BuildContainerID(runtimeName, podSandboxID)
if err := ds.networkPlugin.TearDownPod(namespace, name, cID); err != nil {
errList = append(errList, fmt.Errorf("failed to teardown sandbox %q for pod %s/%s: %v", podSandboxID, namespace, name, err))
if err := ds.network.TearDownPod(namespace, name, cID); err != nil {
errList = append(errList, err)
}
}
if err := ds.client.StopContainer(podSandboxID, defaultSandboxGracePeriod); err != nil {
Expand Down Expand Up @@ -199,12 +199,12 @@ func (ds *dockerService) getIPFromPlugin(sandbox *dockertypes.ContainerJSON) (st
}
msg := fmt.Sprintf("Couldn't find network status for %s/%s through plugin", metadata.Namespace, metadata.Name)
cID := kubecontainer.BuildContainerID(runtimeName, sandbox.ID)
networkStatus, err := ds.networkPlugin.GetPodNetworkStatus(metadata.Namespace, metadata.Name, cID)
networkStatus, err := ds.network.GetPodNetworkStatus(metadata.Namespace, metadata.Name, cID)
if err != nil {
// This might be a sandbox that somehow ended up without a default
// interface (eth0). We can't distinguish this from a more serious
// error, so callers should probably treat it as non-fatal.
return "", fmt.Errorf("%v: %v", msg, err)
return "", err
}
if networkStatus == nil {
return "", fmt.Errorf("%v: invalid network status for", msg)
Expand Down Expand Up @@ -408,7 +408,7 @@ func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig
}
hc.CgroupParent = cgroupParent
// Apply security context.
applySandboxSecurityContext(lc, createConfig.Config, hc, ds.networkPlugin, separator)
applySandboxSecurityContext(lc, createConfig.Config, hc, ds.network, separator)

return nil
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/kubelet/dockershim/docker_sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/kubelet/network"
"k8s.io/kubernetes/pkg/kubelet/types"
)

Expand Down Expand Up @@ -146,7 +147,7 @@ func TestSandboxStatus(t *testing.T) {
func TestNetworkPluginInvocation(t *testing.T) {
ds, _, _ := newTestDockerService()
mockPlugin := newTestNetworkPlugin(t)
ds.networkPlugin = mockPlugin
ds.network = network.NewPluginManager(mockPlugin)
defer mockPlugin.Finish()

name := "foo0"
Expand All @@ -158,6 +159,7 @@ func TestNetworkPluginInvocation(t *testing.T) {
)
cID := kubecontainer.ContainerID{Type: runtimeName, ID: fmt.Sprintf("/%v", makeSandboxName(c))}

mockPlugin.EXPECT().Name().Return("mockNetworkPlugin").AnyTimes()
setup := mockPlugin.EXPECT().SetUpPod(ns, name, cID)
// StopPodSandbox performs a lookup on status to figure out if the sandbox
// is running with hostnetworking, as all its given is the ID.
Expand All @@ -175,7 +177,7 @@ func TestNetworkPluginInvocation(t *testing.T) {
func TestHostNetworkPluginInvocation(t *testing.T) {
ds, _, _ := newTestDockerService()
mockPlugin := newTestNetworkPlugin(t)
ds.networkPlugin = mockPlugin
ds.network = network.NewPluginManager(mockPlugin)
defer mockPlugin.Finish()

name := "foo0"
Expand Down
10 changes: 5 additions & 5 deletions pkg/kubelet/dockershim/docker_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func NewDockerService(client dockertools.DockerInterface, seccompProfileRoot str
if err != nil {
return nil, fmt.Errorf("didn't find compatible CNI plugin with given settings %+v: %v", pluginSettings, err)
}
ds.networkPlugin = plug
ds.network = network.NewPluginManager(plug)
glog.Infof("Docker cri networking managed by %v", plug.Name())

// NOTE: cgroup driver is only detectable in docker 1.11+
Expand Down Expand Up @@ -224,7 +224,7 @@ type dockerService struct {
podSandboxImage string
streamingRuntime *streamingRuntime
streamingServer streaming.Server
networkPlugin network.NetworkPlugin
network *network.PluginManager
containerManager cm.ContainerManager
// cgroup driver used by Docker runtime.
cgroupDriver string
Expand Down Expand Up @@ -270,10 +270,10 @@ func (ds *dockerService) UpdateRuntimeConfig(runtimeConfig *runtimeapi.RuntimeCo
return
}
glog.Infof("docker cri received runtime config %+v", runtimeConfig)
if ds.networkPlugin != nil && runtimeConfig.NetworkConfig.PodCidr != "" {
if ds.network != nil && runtimeConfig.NetworkConfig.PodCidr != "" {
event := make(map[string]interface{})
event[network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE_DETAIL_CIDR] = runtimeConfig.NetworkConfig.PodCidr
ds.networkPlugin.Event(network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE, event)
ds.network.Event(network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE, event)
}
return
}
Expand Down Expand Up @@ -339,7 +339,7 @@ func (ds *dockerService) Status() (*runtimeapi.RuntimeStatus, error) {
runtimeReady.Reason = "DockerDaemonNotReady"
runtimeReady.Message = fmt.Sprintf("docker: failed to get docker version: %v", err)
}
if err := ds.networkPlugin.Status(); err != nil {
if err := ds.network.Status(); err != nil {
networkReady.Status = false
networkReady.Reason = "NetworkPluginNotReady"
networkReady.Message = fmt.Sprintf("docker: network plugin is not ready: %v", err)
Expand Down
11 changes: 6 additions & 5 deletions pkg/kubelet/dockershim/docker_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,21 @@ import (
containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
"k8s.io/kubernetes/pkg/kubelet/dockertools"
"k8s.io/kubernetes/pkg/kubelet/network"
"k8s.io/kubernetes/pkg/kubelet/network/mock_network"
nettest "k8s.io/kubernetes/pkg/kubelet/network/testing"
"k8s.io/kubernetes/pkg/kubelet/util/cache"
)

// newTestNetworkPlugin returns a mock plugin that implements network.NetworkPlugin
func newTestNetworkPlugin(t *testing.T) *mock_network.MockNetworkPlugin {
func newTestNetworkPlugin(t *testing.T) *nettest.MockNetworkPlugin {
ctrl := gomock.NewController(t)
return mock_network.NewMockNetworkPlugin(ctrl)
return nettest.NewMockNetworkPlugin(ctrl)
}

func newTestDockerService() (*dockerService, *dockertools.FakeDockerClient, *clock.FakeClock) {
fakeClock := clock.NewFakeClock(time.Time{})
c := dockertools.NewFakeDockerClient().WithClock(fakeClock).WithVersion("1.11.2", "1.23")
return &dockerService{client: c, os: &containertest.FakeOS{}, networkPlugin: &network.NoopNetworkPlugin{},
pm := network.NewPluginManager(&network.NoopNetworkPlugin{})
return &dockerService{client: c, os: &containertest.FakeOS{}, network: pm,
legacyCleanup: legacyCleanupFlag{done: 1}, checkpointHandler: NewTestPersistentCheckpointHandler()}, c, fakeClock
}

Expand Down Expand Up @@ -95,7 +96,7 @@ func TestStatus(t *testing.T) {

// Should not report ready status is network plugin returns error.
mockPlugin := newTestNetworkPlugin(t)
ds.networkPlugin = mockPlugin
ds.network = network.NewPluginManager(mockPlugin)
defer mockPlugin.Finish()
mockPlugin.EXPECT().Status().Return(errors.New("network error"))
status, err = ds.Status()
Expand Down
17 changes: 8 additions & 9 deletions pkg/kubelet/dockershim/security_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (
"k8s.io/kubernetes/pkg/api/v1"
runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
"k8s.io/kubernetes/pkg/kubelet/dockertools/securitycontext"
"k8s.io/kubernetes/pkg/kubelet/network"
knetwork "k8s.io/kubernetes/pkg/kubelet/network"
)

// applySandboxSecurityContext updates docker sandbox options according to security context.
func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *dockercontainer.Config, hc *dockercontainer.HostConfig, networkPlugin network.NetworkPlugin, separator rune) {
func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *dockercontainer.Config, hc *dockercontainer.HostConfig, network *knetwork.PluginManager, separator rune) {
if lc == nil {
return
}
Expand All @@ -47,8 +47,7 @@ func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *d

modifyContainerConfig(sc, config)
modifyHostConfig(sc, hc, separator)
modifySandboxNamespaceOptions(sc.GetNamespaceOptions(), hc, networkPlugin)

modifySandboxNamespaceOptions(sc.GetNamespaceOptions(), hc, network)
}

// applyContainerSecurityContext updates docker container options according to security context.
Expand Down Expand Up @@ -109,9 +108,9 @@ func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig *
}

// modifySandboxNamespaceOptions apply namespace options for sandbox
func modifySandboxNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, hostConfig *dockercontainer.HostConfig, networkPlugin network.NetworkPlugin) {
func modifySandboxNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, hostConfig *dockercontainer.HostConfig, network *knetwork.PluginManager) {
modifyCommonNamespaceOptions(nsOpts, hostConfig)
modifyHostNetworkOptionForSandbox(nsOpts.HostNetwork, networkPlugin, hostConfig)
modifyHostNetworkOptionForSandbox(nsOpts.HostNetwork, network, hostConfig)
}

// modifyContainerNamespaceOptions apply namespace options for container
Expand All @@ -137,18 +136,18 @@ func modifyCommonNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, hostConfig
}

// modifyHostNetworkOptionForSandbox applies NetworkMode/UTSMode to sandbox's dockercontainer.HostConfig.
func modifyHostNetworkOptionForSandbox(hostNetwork bool, networkPlugin network.NetworkPlugin, hc *dockercontainer.HostConfig) {
func modifyHostNetworkOptionForSandbox(hostNetwork bool, network *knetwork.PluginManager, hc *dockercontainer.HostConfig) {
if hostNetwork {
hc.NetworkMode = namespaceModeHost
return
}

if networkPlugin == nil {
if network == nil {
hc.NetworkMode = "default"
return
}

switch networkPlugin.Name() {
switch network.PluginName() {
case "cni":
fallthrough
case "kubenet":
Expand Down
1 change: 0 additions & 1 deletion pkg/kubelet/dockertools/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ go_test(
"//pkg/kubelet/events:go_default_library",
"//pkg/kubelet/images:go_default_library",
"//pkg/kubelet/network:go_default_library",
"//pkg/kubelet/network/mock_network:go_default_library",
"//pkg/kubelet/network/testing:go_default_library",
"//pkg/kubelet/prober/results:go_default_library",
"//pkg/kubelet/types:go_default_library",
Expand Down