Cleaning up container ID handling inside kubelet

This commit is contained in:
Justin Huff 2014-06-25 16:24:20 -07:00
parent 3432cf6c53
commit fe0066d2e4
5 changed files with 129 additions and 262 deletions

View File

@ -59,13 +59,13 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do
func (f *FakeDockerClient) StartContainer(id string, hostConfig *docker.HostConfig) error {
f.appendCall("start")
return nil
return f.err
}
func (f *FakeDockerClient) StopContainer(id string, timeout uint) error {
f.appendCall("stop")
f.stopped = append(f.stopped, id)
return nil
return f.err
}
type FakeDockerPuller struct {

View File

@ -169,66 +169,39 @@ func (kl *Kubelet) LogEvent(event *api.Event) error {
return err
}
// Does this container exist on this host? Returns true if so, and the name under which the container is running.
// Returns an error if one occurs.
func (kl *Kubelet) ContainerExists(manifest *api.ContainerManifest, container *api.Container) (exists bool, foundName string, err error) {
containers, err := kl.ListContainers()
if err != nil {
return false, "", err
}
for _, name := range containers {
manifestId, containerName := dockerNameToManifestAndContainer(name)
if manifestId == manifest.Id && containerName == container.Name {
// TODO(bburns) : This leads to an extra list. Convert this to use the returned ID and a straight call
// to inspect
data, err := kl.GetContainerByName(name)
return data != nil, name, err
}
}
return false, "", nil
}
// GetContainerID looks at the list of containers on the machine and returns the ID of the container whose name
// matches 'name'. It returns the name of the container, or empty string, if the container isn't found.
// it returns true if the container is found, false otherwise, and any error that occurs.
func (kl *Kubelet) GetContainerID(name string) (string, bool, error) {
containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{})
if err != nil {
return "", false, err
}
for _, value := range containerList {
if strings.Contains(value.Names[0], name) {
return value.ID, true, nil
}
}
return "", false, nil
}
// Get a container by name.
// returns the container data from Docker, or an error if one exists.
func (kl *Kubelet) GetContainerByName(name string) (*docker.Container, error) {
id, found, err := kl.GetContainerID(name)
if err != nil {
return nil, err
}
if !found {
return nil, nil
}
return kl.DockerClient.InspectContainer(id)
}
func (kl *Kubelet) ListContainers() ([]string, error) {
result := []string{}
// Return a map of docker containers that we manage. The map key is the docker container ID
func (kl *Kubelet) getDockerContainers() (map[string]docker.APIContainers, error) {
result := map[string]docker.APIContainers{}
containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{})
if err != nil {
return result, err
}
for _, value := range containerList {
result = append(result, value.Names[0])
// Skip containers that we didn't create to allow users to manually
// spin up their own containers if they want.
if !strings.HasPrefix(value.Names[0], "/"+containerNamePrefix+"--") {
continue
}
result[value.ID] = value
}
return result, err
}
// Return Docker's container ID for a manifest's container. Returns an empty string if it doesn't exist
func (kl *Kubelet) getContainerId(manifest *api.ContainerManifest, container *api.Container) (string, error) {
dockerContainers, err := kl.getDockerContainers()
if err != nil {
return "", err
}
for id, dockerContainer := range dockerContainers {
manifestId, containerName := dockerNameToManifestAndContainer(dockerContainer.Names[0])
if manifestId == manifest.Id && containerName == container.Name {
return id, nil
}
}
return "", nil
}
type dockerPuller struct{}
func MakeDockerPuller() DockerPuller {
@ -346,15 +319,14 @@ func makePortsAndBindings(container *api.Container) (map[docker.Port]struct{}, m
return exposedPorts, portBindings
}
func (kl *Kubelet) RunContainer(manifest *api.ContainerManifest, container *api.Container, netMode string) (name string, err error) {
name = manifestAndContainerToDockerName(manifest, container)
// Run a single container from a manifest. Returns the docker container ID
func (kl *Kubelet) runContainer(manifest *api.ContainerManifest, container *api.Container, netMode string) (id string, err error) {
envVariables := makeEnvironmentVariables(container)
volumes, binds := makeVolumesAndBinds(container)
exposedPorts, portBindings := makePortsAndBindings(container)
opts := docker.CreateContainerOptions{
Name: name,
Name: manifestAndContainerToDockerName(manifest, container),
Config: &docker.Config{
Image: container.Image,
ExposedPorts: exposedPorts,
@ -368,25 +340,18 @@ func (kl *Kubelet) RunContainer(manifest *api.ContainerManifest, container *api.
if err != nil {
return "", err
}
return name, kl.DockerClient.StartContainer(dockerContainer.ID, &docker.HostConfig{
err = kl.DockerClient.StartContainer(dockerContainer.ID, &docker.HostConfig{
PortBindings: portBindings,
Binds: binds,
NetworkMode: netMode,
})
return dockerContainer.ID, err
}
func (kl *Kubelet) KillContainer(name string) error {
id, found, err := kl.GetContainerID(name)
if err != nil {
return err
}
if !found {
// This is weird, but not an error, so yell and then return nil
glog.Infof("Couldn't find container: %s", name)
return nil
}
err = kl.DockerClient.StopContainer(id, 10)
manifestId, containerName := dockerNameToManifestAndContainer(name)
// Kill a docker container
func (kl *Kubelet) killContainer(container docker.APIContainers) error {
err := kl.DockerClient.StopContainer(container.ID, 10)
manifestId, containerName := dockerNameToManifestAndContainer(container.Names[0])
kl.LogEvent(&api.Event{
Event: "STOP",
Manifest: &api.ContainerManifest{
@ -642,19 +607,12 @@ func (kl *Kubelet) WatchEtcd(watchChannel <-chan *etcd.Response, updateChannel c
const networkContainerName = "net"
func (kl *Kubelet) networkContainerExists(manifest *api.ContainerManifest) (string, bool, error) {
pods, err := kl.ListContainers()
if err != nil {
return "", false, err
}
for _, name := range pods {
if strings.Contains(name, containerNamePrefix+"--"+networkContainerName+"--"+escapeDash(manifest.Id)+"--") {
return name, true, nil
}
}
return "", false, nil
// Return the docker ID for a manifest's network container. Returns an empty string if it doesn't exist
func (kl *Kubelet) getNetworkContainerId(manifest *api.ContainerManifest) (string, error) {
return kl.getContainerId(manifest, &api.Container{Name: networkContainerName})
}
// Create a network container for a manifest. Returns the docker container ID of the newly created container
func (kl *Kubelet) createNetworkContainer(manifest *api.ContainerManifest) (string, error) {
var ports []api.Port
// Docker only exports ports from the network container. Let's
@ -669,73 +627,69 @@ func (kl *Kubelet) createNetworkContainer(manifest *api.ContainerManifest) (stri
Ports: ports,
}
kl.DockerPuller.Pull("busybox")
return kl.RunContainer(manifest, container, "")
return kl.runContainer(manifest, container, "")
}
// Sync the configured list of containers (desired state) with the host current state
func (kl *Kubelet) SyncManifests(config []api.ContainerManifest) error {
glog.Infof("Desired: %#v", config)
var err error
desired := map[string]bool{}
dockerIdsToKeep := map[string]bool{}
// Check for any containers that need starting
for _, manifest := range config {
netName, exists, err := kl.networkContainerExists(&manifest)
// Make sure we have a network container
netId, err := kl.getNetworkContainerId(&manifest)
if err != nil {
glog.Errorf("Failed to introspect network container. (%#v) Skipping container %s", err, manifest.Id)
continue
}
if !exists {
if netId == "" {
glog.Infof("Network container doesn't exist, creating")
netName, err = kl.createNetworkContainer(&manifest)
netId, err = kl.createNetworkContainer(&manifest)
if err != nil {
glog.Errorf("Failed to create network container: %#v", err)
glog.Errorf("Failed to create network container: %#v Skipping container %s", err, manifest.Id)
continue
}
// Docker list prefixes '/' for some reason, so let's do that...
netName = "/" + netName
}
desired[netName] = true
for _, element := range manifest.Containers {
var exists bool
exists, actualName, err := kl.ContainerExists(&manifest, &element)
dockerIdsToKeep[netId] = true
for _, container := range manifest.Containers {
containerId, err := kl.getContainerId(&manifest, &container)
if err != nil {
glog.Errorf("Error detecting container: %#v skipping.", err)
continue
}
if !exists {
glog.Infof("%#v doesn't exist, creating", element)
kl.DockerPuller.Pull(element.Image)
if containerId == "" {
glog.Infof("%#v doesn't exist, creating", container)
kl.DockerPuller.Pull(container.Image)
if err != nil {
glog.Errorf("Error pulling container: %#v", err)
continue
}
// netName has the '/' prefix, so slice it off
networkContainer := netName[1:]
actualName, err = kl.RunContainer(&manifest, &element, "container:"+networkContainer)
// For some reason, list gives back names that start with '/'
actualName = "/" + actualName
containerId, err = kl.runContainer(&manifest, &container, "container:"+netId)
if err != nil {
// TODO(bburns) : Perhaps blacklist a container after N failures?
glog.Errorf("Error creating container: %#v", err)
desired[actualName] = true
continue
}
} else {
glog.V(1).Infof("%#v exists as %v", element.Name, actualName)
glog.V(1).Infof("%#v exists as %v", container.Name, containerId)
}
desired[actualName] = true
dockerIdsToKeep[containerId] = true
}
}
existingContainers, _ := kl.ListContainers()
glog.Infof("Existing: %#v Desired: %#v", existingContainers, desired)
for _, container := range existingContainers {
// Skip containers that we didn't create to allow users to manually
// spin up their own containers if they want.
if !strings.HasPrefix(container, "/"+containerNamePrefix+"--") {
continue
}
if !desired[container] {
glog.Infof("Killing: %s", container)
err = kl.KillContainer(container)
// Kill any containers we don't need
existingContainers, err := kl.getDockerContainers()
if err != nil {
glog.Errorf("Error listing containers: %#v", err)
return err
}
for id, container := range existingContainers {
if !dockerIdsToKeep[id] {
glog.Infof("Killing: %s", id)
err = kl.killContainer(container)
if err != nil {
glog.Errorf("Error killing container: %#v", err)
}
@ -772,8 +726,31 @@ func (kl *Kubelet) RunSyncLoop(updateChannel <-chan manifestUpdate, handler Sync
}
}
// getContainerIdFromName looks at the list of containers on the machine and returns the ID of the container whose name
// matches 'name'. It returns the name of the container, or empty string, if the container isn't found.
// it returns true if the container is found, false otherwise, and any error that occurs.
// TODO: This functions exists to support GetContainerInfo and GetContainerStats
// It should be removed once those two functions start taking proper pod.IDs
func (kl *Kubelet) getContainerIdFromName(name string) (string, bool, error) {
containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{})
if err != nil {
return "", false, err
}
for _, value := range containerList {
if strings.Contains(value.Names[0], name) {
return value.ID, true, nil
}
}
return "", false, nil
}
// Returns docker info for a container
func (kl *Kubelet) GetContainerInfo(name string) (string, error) {
info, err := kl.DockerClient.InspectContainer(name)
dockerId, found, err := kl.getContainerIdFromName(name)
if err != nil || !found {
return "{}", err
}
info, err := kl.DockerClient.InspectContainer(dockerId)
if err != nil {
return "{}", err
}
@ -781,16 +758,17 @@ func (kl *Kubelet) GetContainerInfo(name string) (string, error) {
return string(data), err
}
//Returns stats (from Cadvisor) for a container
func (kl *Kubelet) GetContainerStats(name string) (*api.ContainerStats, error) {
if kl.CadvisorClient == nil {
return nil, nil
}
id, found, err := kl.GetContainerID(name)
dockerId, found, err := kl.getContainerIdFromName(name)
if err != nil || !found {
return nil, err
}
info, err := kl.CadvisorClient.ContainerInfo(fmt.Sprintf("/docker/%v", id))
info, err := kl.CadvisorClient.ContainerInfo(fmt.Sprintf("/docker/%v", dockerId))
if err != nil {
return nil, err

View File

@ -35,7 +35,6 @@ type KubeletServer struct {
// kubeletInterface contains all the kubelet methods required by the server.
// For testablitiy.
type kubeletInterface interface {
GetContainerID(name string) (string, bool, error)
GetContainerStats(name string) (*api.ContainerStats, error)
GetContainerInfo(name string) (string, error)
}
@ -78,6 +77,7 @@ func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
s.UpdateChannel <- manifestUpdate{httpServerSource, manifests}
}
case u.Path == "/containerStats":
// NOTE: The master appears to pass a Pod.ID
container := u.Query().Get("container")
if len(container) == 0 {
w.WriteHeader(http.StatusBadRequest)
@ -105,27 +105,23 @@ func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
w.Header().Add("Content-type", "application/json")
w.Write(data)
case u.Path == "/containerInfo":
// NOTE: The master appears to pass a Pod.ID
// The server appears to pass a Pod.ID
container := u.Query().Get("container")
if len(container) == 0 {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprint(w, "Missing container selector arg.")
return
}
id, found, err := s.Kubelet.GetContainerID(container)
if !found {
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, "{}")
return
}
body, err := s.Kubelet.GetContainerInfo(id)
data, err := s.Kubelet.GetContainerInfo(container)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprintf(w, "Internal Error: %#v", err)
return
}
w.Header().Add("Content-type", "application/json")
w.WriteHeader(http.StatusOK)
fmt.Fprint(w, body)
w.Header().Add("Content-type", "application/json")
fmt.Fprint(w, data)
default:
w.WriteHeader(http.StatusNotFound)
fmt.Fprint(w, "Not found.")

View File

@ -32,7 +32,6 @@ import (
type fakeKubelet struct {
infoFunc func(name string) (string, error)
idFunc func(name string) (string, bool, error)
statsFunc func(name string) (*api.ContainerStats, error)
}
@ -40,10 +39,6 @@ func (fk *fakeKubelet) GetContainerInfo(name string) (string, error) {
return fk.infoFunc(name)
}
func (fk *fakeKubelet) GetContainerID(name string) (string, bool, error) {
return fk.idFunc(name)
}
func (fk *fakeKubelet) GetContainerStats(name string) (*api.ContainerStats, error) {
return fk.statsFunc(name)
}
@ -122,12 +117,6 @@ func TestContainers(t *testing.T) {
func TestContainerInfo(t *testing.T) {
fw := makeServerTest()
expected := "good container info string"
fw.fakeKubelet.idFunc = func(name string) (string, bool, error) {
if name == "goodcontainer" {
return name, true, nil
}
return "", false, fmt.Errorf("bad container")
}
fw.fakeKubelet.infoFunc = func(name string) (string, error) {
if name == "goodcontainer" {
return expected, nil

View File

@ -133,7 +133,7 @@ func TestContainerManifestNaming(t *testing.T) {
verifyPackUnpack(t, "_m___anifest", "-_-container")
}
func TestContainerExists(t *testing.T) {
func TestGetContainerId(t *testing.T) {
fakeDocker := FakeDockerClient{
err: nil,
}
@ -149,19 +149,21 @@ func TestContainerExists(t *testing.T) {
}
fakeDocker.containerList = []docker.APIContainers{
{
ID: "foobar",
Names: []string{"/k8s--foo--qux--1234"},
},
{
Names: []string{"/k8s--bar--qux--1234"},
ID: "barbar",
Names: []string{"/k8s--bar--qux--2565"},
},
}
fakeDocker.container = &docker.Container{
ID: "foobar",
}
exists, _, err := kubelet.ContainerExists(&manifest, &container)
verifyCalls(t, fakeDocker, []string{"list", "list", "inspect"})
if !exists {
id, err := kubelet.getContainerId(&manifest, &container)
verifyCalls(t, fakeDocker, []string{"list"})
if id == "" {
t.Errorf("Failed to find container %#v", container)
}
if err != nil {
@ -170,113 +172,24 @@ func TestContainerExists(t *testing.T) {
fakeDocker.clearCalls()
missingManifest := api.ContainerManifest{Id: "foobar"}
exists, _, err = kubelet.ContainerExists(&missingManifest, &container)
id, err = kubelet.getContainerId(&missingManifest, &container)
verifyCalls(t, fakeDocker, []string{"list"})
if exists {
t.Errorf("Failed to not find container %#v, missingManifest")
if id != "" {
t.Errorf("Failed to not find container %#v", missingManifest)
}
}
func TestGetContainerID(t *testing.T) {
fakeDocker := FakeDockerClient{
err: nil,
}
kubelet := Kubelet{
DockerClient: &fakeDocker,
DockerPuller: &FakeDockerPuller{},
}
fakeDocker.containerList = []docker.APIContainers{
{
Names: []string{"foo"},
ID: "1234",
},
{
Names: []string{"bar"},
ID: "4567",
},
}
id, found, err := kubelet.GetContainerID("foo")
verifyBoolean(t, true, found)
verifyStringEquals(t, id, "1234")
verifyNoError(t, err)
verifyCalls(t, fakeDocker, []string{"list"})
fakeDocker.clearCalls()
id, found, err = kubelet.GetContainerID("bar")
verifyBoolean(t, true, found)
verifyStringEquals(t, id, "4567")
verifyNoError(t, err)
verifyCalls(t, fakeDocker, []string{"list"})
fakeDocker.clearCalls()
id, found, err = kubelet.GetContainerID("NotFound")
verifyBoolean(t, false, found)
verifyNoError(t, err)
verifyCalls(t, fakeDocker, []string{"list"})
}
func TestGetContainerByName(t *testing.T) {
fakeDocker := FakeDockerClient{
err: nil,
}
kubelet := Kubelet{
DockerClient: &fakeDocker,
DockerPuller: &FakeDockerPuller{},
}
fakeDocker.containerList = []docker.APIContainers{
{
Names: []string{"foo"},
},
{
Names: []string{"bar"},
},
}
fakeDocker.container = &docker.Container{
ID: "foobar",
}
container, err := kubelet.GetContainerByName("foo")
verifyCalls(t, fakeDocker, []string{"list", "inspect"})
if container == nil {
t.Errorf("Unexpected nil container")
}
verifyStringEquals(t, container.ID, "foobar")
verifyNoError(t, err)
}
func TestListContainers(t *testing.T) {
fakeDocker := FakeDockerClient{
err: nil,
}
kubelet := Kubelet{
DockerClient: &fakeDocker,
DockerPuller: &FakeDockerPuller{},
}
fakeDocker.containerList = []docker.APIContainers{
{
Names: []string{"foo"},
},
{
Names: []string{"bar"},
},
}
containers, err := kubelet.ListContainers()
verifyStringArrayEquals(t, containers, []string{"foo", "bar"})
verifyNoError(t, err)
verifyCalls(t, fakeDocker, []string{"list"})
}
func TestKillContainerWithError(t *testing.T) {
fakeDocker := FakeDockerClient{
err: fmt.Errorf("sample error"),
containerList: []docker.APIContainers{
{
Names: []string{"foo"},
ID: "1234",
Names: []string{"/k8s--foo--qux--1234"},
},
{
Names: []string{"bar"},
ID: "5678",
Names: []string{"/k8s--bar--qux--5678"},
},
},
}
@ -284,9 +197,9 @@ func TestKillContainerWithError(t *testing.T) {
DockerClient: &fakeDocker,
DockerPuller: &FakeDockerPuller{},
}
err := kubelet.KillContainer("foo")
err := kubelet.killContainer(fakeDocker.containerList[0])
verifyError(t, err)
verifyCalls(t, fakeDocker, []string{"list"})
verifyCalls(t, fakeDocker, []string{"stop"})
}
func TestKillContainer(t *testing.T) {
@ -299,19 +212,21 @@ func TestKillContainer(t *testing.T) {
}
fakeDocker.containerList = []docker.APIContainers{
{
Names: []string{"foo"},
ID: "1234",
Names: []string{"/k8s--foo--qux--1234"},
},
{
Names: []string{"bar"},
ID: "5678",
Names: []string{"/k8s--bar--qux--5678"},
},
}
fakeDocker.container = &docker.Container{
ID: "foobar",
}
err := kubelet.KillContainer("foo")
err := kubelet.killContainer(fakeDocker.containerList[0])
verifyNoError(t, err)
verifyCalls(t, fakeDocker, []string{"list", "stop"})
verifyCalls(t, fakeDocker, []string{"stop"})
}
func TestResponseToContainersNil(t *testing.T) {
@ -491,14 +406,7 @@ func TestSyncManifestsDoesNothing(t *testing.T) {
},
})
expectNoError(t, err)
if len(fakeDocker.called) != 5 ||
fakeDocker.called[0] != "list" ||
fakeDocker.called[1] != "list" ||
fakeDocker.called[2] != "list" ||
fakeDocker.called[3] != "inspect" ||
fakeDocker.called[4] != "list" {
t.Errorf("Unexpected call sequence: %#v", fakeDocker.called)
}
verifyCalls(t, fakeDocker, []string{"list", "list", "list"})
}
func TestSyncManifestsDeletes(t *testing.T) {
@ -527,15 +435,11 @@ func TestSyncManifestsDeletes(t *testing.T) {
}
err := kubelet.SyncManifests([]api.ContainerManifest{})
expectNoError(t, err)
if len(fakeDocker.called) != 5 ||
fakeDocker.called[0] != "list" ||
fakeDocker.called[1] != "list" ||
fakeDocker.called[2] != "stop" ||
fakeDocker.called[3] != "list" ||
fakeDocker.called[4] != "stop" ||
verifyCalls(t, fakeDocker, []string{"list", "stop", "stop"})
if len(fakeDocker.stopped) != 2 ||
fakeDocker.stopped[0] != "1234" ||
fakeDocker.stopped[1] != "9876" {
t.Errorf("Unexpected call sequence: %#v %s", fakeDocker.called, fakeDocker.stopped)
t.Errorf("Unexpected sequence of stopped containers: %s", fakeDocker.stopped)
}
}