From a06bc964143ac4f6c793c3784ba921c3cf302f96 Mon Sep 17 00:00:00 2001 From: Yifan Gu Date: Wed, 25 Mar 2015 21:25:08 -0700 Subject: [PATCH] kubelet: Add container reference manager. Move the reference managing logic into container reference manager. This enables pluggable container runtime to manage the container references. --- pkg/kubelet/container_reference_manager.go | 87 ++++++++++++++++++++++ pkg/kubelet/kubelet.go | 60 ++------------- pkg/kubelet/kubelet_test.go | 1 + pkg/kubelet/probe.go | 2 +- pkg/kubelet/probe_test.go | 1 + pkg/kubelet/runonce_test.go | 11 +-- 6 files changed, 103 insertions(+), 59 deletions(-) create mode 100644 pkg/kubelet/container_reference_manager.go diff --git a/pkg/kubelet/container_reference_manager.go b/pkg/kubelet/container_reference_manager.go new file mode 100644 index 00000000000..811e9d74b39 --- /dev/null +++ b/pkg/kubelet/container_reference_manager.go @@ -0,0 +1,87 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubelet + +import ( + "sync" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + +// ContainerRefManager manages the references for the containers. +// The references are used for reporting events such as creation, +// failure, etc. This manager is thread-safe, no locks are necessary +// for the caller. +type ContainerRefManager struct { + sync.RWMutex + // TODO(yifan): To use strong type. + containerIDToRef map[string]*api.ObjectReference +} + +// newContainerRefManager creates and returns a container reference manager +// with empty contents. +func newContainerRefManager() *ContainerRefManager { + c := ContainerRefManager{} + c.containerIDToRef = make(map[string]*api.ObjectReference) + return &c +} + +// SetRef stores a reference to a pod's container, associating it with the given container id. +func (c *ContainerRefManager) SetRef(id string, ref *api.ObjectReference) { + c.Lock() + defer c.Unlock() + c.containerIDToRef[id] = ref +} + +// ClearRef forgets the given container id and its associated container reference. +// TODO(yifan): This is currently never called. Consider to remove this function, +// or figure out when to clear the references. +func (c *ContainerRefManager) ClearRef(id string) { + c.Lock() + defer c.Unlock() + delete(c.containerIDToRef, id) +} + +// GetRef returns the container reference of the given id, or (nil, false) if none is stored. +func (c *ContainerRefManager) GetRef(id string) (ref *api.ObjectReference, ok bool) { + c.RLock() + defer c.RUnlock() + ref, ok = c.containerIDToRef[id] + return ref, ok +} + +// GenerateContainerRef returns an *api.ObjectReference which references the given container within the +// given pod. Returns an error if the reference can't be constructed or the container doesn't +// actually belong to the pod. +// TODO: Pods that came to us by static config or over HTTP have no selfLink set, which makes +// this fail and log an error. Figure out how we want to identify these pods to the rest of the +// system. +// TODO(yifan): Revisit this function later, for current case, it does not need to use ContainerRefManager +// as a receiver, and does not need to be exported. +func (c *ContainerRefManager) GenerateContainerRef(pod *api.Pod, container *api.Container) (*api.ObjectReference, error) { + fieldPath, err := fieldPath(pod, container) + if err != nil { + // TODO: figure out intelligent way to refer to containers that we implicitly + // start (like the pod infra container). This is not a good way, ugh. + fieldPath = "implicitly required container " + container.Name + } + ref, err := api.GetPartialReference(pod, fieldPath) + if err != nil { + return nil, err + } + return ref, nil +} diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index ee2b1230abd..44402c85a4b 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -218,7 +218,7 @@ func NewMainKubelet( rootDirectory: rootDirectory, resyncInterval: resyncInterval, podInfraContainerImage: podInfraContainerImage, - containerIDToRef: map[string]*api.ObjectReference{}, + containerRefManager: newContainerRefManager(), runner: dockertools.NewDockerContainerCommandRunner(dockerClient), httpClient: &http.Client{}, pullQPS: pullQPS, @@ -296,8 +296,7 @@ type Kubelet struct { // Needed to report events for containers belonging to deleted/modified pods. // Tracks references for reporting events - containerIDToRef map[string]*api.ObjectReference - refLock sync.RWMutex + containerRefManager *ContainerRefManager // Optional, defaults to simple Docker implementation dockerPuller dockertools.DockerPuller @@ -685,54 +684,9 @@ func fieldPath(pod *api.Pod, container *api.Container) (string, error) { return "", fmt.Errorf("container %#v not found in pod %#v", container, pod) } -// containerRef returns an *api.ObjectReference which references the given container within the -// given pod. Returns an error if the reference can't be constructed or the container doesn't -// actually belong to the pod. -// TODO: Pods that came to us by static config or over HTTP have no selfLink set, which makes -// this fail and log an error. Figure out how we want to identify these pods to the rest of the -// system. -func containerRef(pod *api.Pod, container *api.Container) (*api.ObjectReference, error) { - fieldPath, err := fieldPath(pod, container) - if err != nil { - // TODO: figure out intelligent way to refer to containers that we implicitly - // start (like the pod infra container). This is not a good way, ugh. - fieldPath = "implicitly required container " + container.Name - } - ref, err := api.GetPartialReference(pod, fieldPath) - if err != nil { - return nil, err - } - return ref, nil -} - -// setRef stores a reference to a pod's container, associating it with the given docker id. -func (kl *Kubelet) setRef(id string, ref *api.ObjectReference) { - kl.refLock.Lock() - defer kl.refLock.Unlock() - if kl.containerIDToRef == nil { - kl.containerIDToRef = map[string]*api.ObjectReference{} - } - kl.containerIDToRef[id] = ref -} - -// clearRef forgets the given docker id and its associated container reference. -func (kl *Kubelet) clearRef(id string) { - kl.refLock.Lock() - defer kl.refLock.Unlock() - delete(kl.containerIDToRef, id) -} - -// getRef returns the container reference of the given id, or (nil, false) if none is stored. -func (kl *Kubelet) getRef(id string) (ref *api.ObjectReference, ok bool) { - kl.refLock.RLock() - defer kl.refLock.RUnlock() - ref, ok = kl.containerIDToRef[id] - return ref, ok -} - // Run a single container from a pod. Returns the docker container ID func (kl *Kubelet) runContainer(pod *api.Pod, container *api.Container, podVolumes volumeMap, netMode, ipcMode string) (id dockertools.DockerID, err error) { - ref, err := containerRef(pod, container) + ref, err := kl.containerRefManager.GenerateContainerRef(pod, container) if err != nil { glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err) } @@ -773,7 +727,7 @@ func (kl *Kubelet) runContainer(pod *api.Pod, container *api.Container, podVolum } // Remember this reference so we can report events about this container if ref != nil { - kl.setRef(dockerContainer.ID, ref) + kl.containerRefManager.SetRef(dockerContainer.ID, ref) kl.recorder.Eventf(ref, "created", "Created with docker id %v", dockerContainer.ID) } @@ -993,7 +947,7 @@ func (kl *Kubelet) killContainerByID(ID string) error { kl.readiness.remove(ID) err := kl.dockerClient.StopContainer(ID, 10) - ref, ok := kl.getRef(ID) + ref, ok := kl.containerRefManager.GetRef(ID) if !ok { glog.Warningf("No ref for pod '%v'", ID) } else { @@ -1043,7 +997,7 @@ func (kl *Kubelet) createPodInfraContainer(pod *api.Pod) (dockertools.DockerID, Image: kl.podInfraContainerImage, Ports: ports, } - ref, err := containerRef(pod, container) + ref, err := kl.containerRefManager.GenerateContainerRef(pod, container) if err != nil { glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err) } @@ -1201,7 +1155,7 @@ func (kl *Kubelet) getPodInfraContainer(podFullName string, uid types.UID, func (kl *Kubelet) pullImageAndRunContainer(pod *api.Pod, container *api.Container, podVolumes *volumeMap, podInfraContainerID dockertools.DockerID) (dockertools.DockerID, error) { podFullName := kubecontainer.GetPodFullName(pod) - ref, err := containerRef(pod, container) + ref, err := kl.containerRefManager.GenerateContainerRef(pod, container) if err != nil { glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err) } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 5eec297391c..8069f061733 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -110,6 +110,7 @@ func newTestKubelet(t *testing.T) *TestKubelet { kubelet.cadvisor = mockCadvisor podManager, fakeMirrorClient := newFakePodManager() kubelet.podManager = podManager + kubelet.containerRefManager = newContainerRefManager() return &TestKubelet{kubelet, fakeDocker, mockCadvisor, fakeKubeClient, waitGroup, fakeMirrorClient} } diff --git a/pkg/kubelet/probe.go b/pkg/kubelet/probe.go index 531f4246038..99c77920ad6 100644 --- a/pkg/kubelet/probe.go +++ b/pkg/kubelet/probe.go @@ -65,7 +65,7 @@ func (kl *Kubelet) probeContainer(pod *api.Pod, status api.PodStatus, container glog.V(1).Infof("Readiness probe failed/errored: %v, %v", ready, err) kl.readiness.set(containerID, false) - ref, ok := kl.getRef(containerID) + ref, ok := kl.containerRefManager.GetRef(containerID) if !ok { glog.Warningf("No ref for pod '%v' - '%v'", containerID, container.Name) } else { diff --git a/pkg/kubelet/probe_test.go b/pkg/kubelet/probe_test.go index 60481045a20..10fc272203d 100644 --- a/pkg/kubelet/probe_test.go +++ b/pkg/kubelet/probe_test.go @@ -153,6 +153,7 @@ func makeTestKubelet(result probe.Result, err error) *Kubelet { err: err, }, }, + containerRefManager: newContainerRefManager(), } } diff --git a/pkg/kubelet/runonce_test.go b/pkg/kubelet/runonce_test.go index b952a96a3e3..7a0f4e6c501 100644 --- a/pkg/kubelet/runonce_test.go +++ b/pkg/kubelet/runonce_test.go @@ -74,11 +74,12 @@ func TestRunOnce(t *testing.T) { cadvisor := &cadvisor.Mock{} cadvisor.On("MachineInfo").Return(&cadvisorApi.MachineInfo{}, nil) kb := &Kubelet{ - rootDirectory: "/tmp/kubelet", - recorder: &record.FakeRecorder{}, - cadvisor: cadvisor, - nodeLister: testNodeLister{}, - statusManager: newStatusManager(nil), + rootDirectory: "/tmp/kubelet", + recorder: &record.FakeRecorder{}, + cadvisor: cadvisor, + nodeLister: testNodeLister{}, + statusManager: newStatusManager(nil), + containerRefManager: newContainerRefManager(), } kb.networkPlugin, _ = network.InitNetworkPlugin([]network.NetworkPlugin{}, "", network.NewFakeHost(nil))