From 62f629137728fed18673026f7a6356e9bde0b047 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 9 Jun 2014 16:43:55 -0700 Subject: [PATCH 1/4] Fix a bug if the container is non-existent. --- pkg/kubelet/kubelet.go | 20 ++++++++++++++------ pkg/kubelet/kubelet_server.go | 9 +++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index de375a3f2b..0ce732e475 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -152,26 +152,29 @@ func (kl *Kubelet) ContainerExists(manifest *api.ContainerManifest, container *a return false, "", nil } -func (kl *Kubelet) GetContainerID(name string) (string, error) { +func (kl *Kubelet) GetContainerID(name string) (string, bool, error) { containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{}) if err != nil { - return "", err + return "", false, err } for _, value := range containerList { if strings.Contains(value.Names[0], name) { - return value.ID, nil + return value.ID, true, nil } } - return "", fmt.Errorf("couldn't find name: %s", name) + 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, err := kl.GetContainerID(name) + id, found, err := kl.GetContainerID(name) if err != nil { return nil, err } + if !found { + return nil, nil + } return kl.DockerClient.InspectContainer(id) } @@ -300,10 +303,15 @@ func (kl *Kubelet) RunContainer(manifest *api.ContainerManifest, container *api. } func (kl *Kubelet) KillContainer(name string) error { - id, err := kl.GetContainerID(name) + 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 + log.Printf("Couldn't find container: %s", name) + return nil + } err = kl.DockerClient.StopContainer(id, 10) manifestId, containerName := dockerNameToManifestAndContainer(name) kl.LogEvent(&api.Event{ diff --git a/pkg/kubelet/kubelet_server.go b/pkg/kubelet/kubelet_server.go index 54fadd965f..73cd8171ea 100644 --- a/pkg/kubelet/kubelet_server.go +++ b/pkg/kubelet/kubelet_server.go @@ -34,7 +34,7 @@ type KubeletServer struct { // kubeletInterface contains all the kubelet methods required by the server. // For testablitiy. type kubeletInterface interface { - GetContainerID(name string) (string, error) + GetContainerID(name string) (string, bool, error) GetContainerInfo(name string) (string, error) } @@ -71,7 +71,12 @@ func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { fmt.Fprint(w, "Missing container query arg.") return } - id, err := s.Kubelet.GetContainerID(container) + id, found, err := s.Kubelet.GetContainerID(container) + if (!found) { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, "{}") + return + } body, err := s.Kubelet.GetContainerInfo(id) if err != nil { w.WriteHeader(http.StatusInternalServerError) From d83f407e437624be5b7b214cc4a1ccb4e0cc3db3 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 9 Jun 2014 16:50:44 -0700 Subject: [PATCH 2/4] Fix tests. --- pkg/kubelet/kubelet_server_test.go | 10 +++++----- pkg/kubelet/kubelet_test.go | 17 +++++++++++++---- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/kubelet/kubelet_server_test.go b/pkg/kubelet/kubelet_server_test.go index 32f3978205..ac9ddd4c3a 100644 --- a/pkg/kubelet/kubelet_server_test.go +++ b/pkg/kubelet/kubelet_server_test.go @@ -16,14 +16,14 @@ import ( type fakeKubelet struct { infoFunc func(name string) (string, error) - idFunc func(name string) (string, error) + idFunc func(name string) (string, bool, error) } func (fk *fakeKubelet) GetContainerInfo(name string) (string, error) { return fk.infoFunc(name) } -func (fk *fakeKubelet) GetContainerID(name string) (string, error) { +func (fk *fakeKubelet) GetContainerID(name string) (string, bool, error) { return fk.idFunc(name) } @@ -105,11 +105,11 @@ func TestContainer(t *testing.T) { func TestContainerInfo(t *testing.T) { fw := makeServerTest() expected := "good container info string" - fw.fakeKubelet.idFunc = func(name string) (string, error) { + fw.fakeKubelet.idFunc = func(name string) (string, bool, error) { if name == "goodcontainer" { - return name, nil + return name, true, nil } - return "", fmt.Errorf("bad container") + return "", false, fmt.Errorf("bad container") } fw.fakeKubelet.infoFunc = func(name string) (string, error) { if name == "goodcontainer" { diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index e4adc8b5d3..80e67280e4 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -150,6 +150,12 @@ func verifyPackUnpack(t *testing.T, manifestId, containerName string) { } } +func verifyBoolean(t *testing.T, expected, value bool) { + if expected != value { + t.Errorf("Unexpected boolean. Expected %s. Found %s", expected, value) + } +} + func TestContainerManifestNaming(t *testing.T) { verifyPackUnpack(t, "manifest1234", "container5678") verifyPackUnpack(t, "manifest--", "container__") @@ -211,20 +217,23 @@ func TestGetContainerID(t *testing.T) { }, } - id, err := kubelet.GetContainerID("foo") + 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, err = kubelet.GetContainerID("bar") + 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, err = kubelet.GetContainerID("NotFound") - verifyError(t, err) + id, found, err = kubelet.GetContainerID("NotFound") + verifyBoolean(t, false, found) + verifyNoError(t, err) verifyCalls(t, fakeDocker, []string{"list"}) } From 83c5b2c57868c8088418673fd0ab7fa343bf4097 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Tue, 10 Jun 2014 09:43:04 -0700 Subject: [PATCH 3/4] Add some extra log info. --- cmd/cloudcfg/cloudcfg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloudcfg/cloudcfg.go b/cmd/cloudcfg/cloudcfg.go index f495de79fc..92fde8d0a0 100644 --- a/cmd/cloudcfg/cloudcfg.go +++ b/cmd/cloudcfg/cloudcfg.go @@ -133,7 +133,7 @@ func main() { var body string body, err = cloudcfg.DoRequest(request, auth.User, auth.Password) if err != nil { - log.Fatalf("Error: %#v", err) + log.Fatalf("Error: %#v %s", err, body) } err = printer.Print(body, os.Stdout) if err != nil { From c36a7896fd463a5b3241291ebe553c014e7cff11 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 12 Jun 2014 11:27:50 -0700 Subject: [PATCH 4/4] Added a comment explaining the function. --- pkg/kubelet/kubelet.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 0ce732e475..7e0a67d8ab 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -152,6 +152,9 @@ func (kl *Kubelet) ContainerExists(manifest *api.ContainerManifest, container *a 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 {