From d523ccb4289c24fbe1ee04b323005216329bad7e Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 30 Jun 2014 19:45:00 -0700 Subject: [PATCH 1/6] Change error printing for easier debugging --- cmd/kubecfg/kubecfg.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/cmd/kubecfg/kubecfg.go b/cmd/kubecfg/kubecfg.go index bb95652972a..ae13408b394 100644 --- a/cmd/kubecfg/kubecfg.go +++ b/cmd/kubecfg/kubecfg.go @@ -74,11 +74,11 @@ func readConfig(storage string) []byte { } data, err := ioutil.ReadFile(*config) if err != nil { - glog.Fatalf("Unable to read %v: %#v\n", *config, err) + glog.Fatalf("Unable to read %v: %v\n", *config, err) } data, err = kubecfg.ToWireFormat(data, storage) if err != nil { - glog.Fatalf("Error parsing %v as an object for %v: %#v\n", *config, storage, err) + glog.Fatalf("Error parsing %v as an object for %v: %v\n", *config, storage, err) } if *verbose { glog.Infof("Parsed config file successfully; sending:\n%v\n", string(data)) @@ -122,7 +122,7 @@ func main() { if secure { auth, err = kubecfg.LoadAuthInfo(*authConfig) if err != nil { - glog.Fatalf("Error loading auth: %#v", err) + glog.Fatalf("Error loading auth: %v", err) } } @@ -175,7 +175,8 @@ func executeAPIRequest(method string, s *kube_client.Client) bool { if method == "create" || method == "update" { r.Body(readConfig(parseStorage())) } - obj, err := r.Do().Get() + result := r.Do() + obj, err := result.Get() if err != nil { glog.Fatalf("Got request error: %v\n", err) return false @@ -191,7 +192,8 @@ func executeAPIRequest(method string, s *kube_client.Client) bool { } if err = printer.PrintObj(obj, os.Stdout); err != nil { - glog.Fatalf("Failed to print: %#v\nRaw received object:\n%#v\n", err, obj) + body, _ := result.Raw() + glog.Fatalf("Failed to print: %v\nRaw received object:\n%#v\n\nBody received: %v", err, obj, string(body)) } fmt.Print("\n") @@ -223,7 +225,7 @@ func executeControllerRequest(method string, c *kube_client.Client) bool { replicas, err := strconv.Atoi(flag.Arg(2)) name := flag.Arg(3) if err != nil { - glog.Fatalf("Error parsing replicas: %#v", err) + glog.Fatalf("Error parsing replicas: %v", err) } err = kubecfg.RunController(image, name, replicas, c, *portSpec, *servicePort) case "resize": @@ -234,14 +236,14 @@ func executeControllerRequest(method string, c *kube_client.Client) bool { name := args[1] replicas, err := strconv.Atoi(args[2]) if err != nil { - glog.Fatalf("Error parsing replicas: %#v", err) + glog.Fatalf("Error parsing replicas: %v", err) } err = kubecfg.ResizeController(name, replicas, c) default: return false } if err != nil { - glog.Fatalf("Error: %#v", err) + glog.Fatalf("Error: %v", err) } return true } From 049bc6b6d47a585b549f5c61938a1287e7584fd0 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 30 Jun 2014 19:46:10 -0700 Subject: [PATCH 2/6] Fix interface{} in api/types.go; plumb through system. --- pkg/api/types.go | 12 ++++- pkg/client/container_info.go | 33 ++++++++---- pkg/client/container_info_test.go | 16 +++--- pkg/kubelet/kubelet.go | 9 ++-- pkg/kubelet/kubelet_server.go | 13 +++-- pkg/kubelet/kubelet_server_test.go | 17 ++++--- pkg/master/pod_cache.go | 17 ++++--- pkg/master/pod_cache_test.go | 38 +++++++------- pkg/registry/pod_registry.go | 75 ++++++++++++++++++--------- pkg/registry/pod_registry_test.go | 82 ++++++++++++++++++++++++++---- 10 files changed, 219 insertions(+), 93 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 2df6fe12cd7..b6d504a0935 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -16,6 +16,10 @@ limitations under the License. package api +import ( + "github.com/fsouza/go-dockerclient" +) + // Common string formats // --------------------- // Many fields in this API have formatting requirements. The commonly used @@ -159,7 +163,13 @@ type PodState struct { Status PodStatus `json:"status,omitempty" yaml:"status,omitempty"` Host string `json:"host,omitempty" yaml:"host,omitempty"` HostIP string `json:"hostIP,omitempty" yaml:"hostIP,omitempty"` - Info interface{} `json:"info,omitempty" yaml:"info,omitempty"` + + // The key of this map is the *name* of the container within the manifest; it has one + // entry per container in the manifest. The value of this map is currently the output + // of `docker inspect`. This output format is *not* final and should not be relied + // upon. + // TODO: Make real decisions about what our info should look like. + Info map[string]docker.Container `json:"info,omitempty" yaml:"info,omitempty"` } type PodList struct { diff --git a/pkg/client/container_info.go b/pkg/client/container_info.go index ce408206f0a..c4ec0c031a2 100644 --- a/pkg/client/container_info.go +++ b/pkg/client/container_info.go @@ -20,15 +20,20 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net" "net/http" + "strconv" + + "github.com/fsouza/go-dockerclient" ) // ContainerInfo is an interface for things that can get information about a container. // Injectable for easy testing. type ContainerInfo interface { // GetContainerInfo returns information about container 'name' on 'host' - // Returns an untyped interface, and an error, if one occurs - GetContainerInfo(host, name string) (interface{}, error) + // Returns a json-formatted []byte (which can be unmarshalled into a + // map[string]interface{}) or an error if one occurs. + GetContainerInfo(host, name string) (*docker.Container, error) } // The default implementation, accesses the kubelet over HTTP @@ -37,8 +42,14 @@ type HTTPContainerInfo struct { Port uint } -func (c *HTTPContainerInfo) GetContainerInfo(host, name string) (interface{}, error) { - request, err := http.NewRequest("GET", fmt.Sprintf("http://%s:%d/containerInfo?container=%s", host, c.Port, name), nil) +func (c *HTTPContainerInfo) GetContainerInfo(host, name string) (*docker.Container, error) { + request, err := http.NewRequest( + "GET", + fmt.Sprintf( + "http://%s/containerInfo?container=%s", + net.JoinHostPort(host, strconv.FormatUint(uint64(c.Port), 10)), + name), + nil) if err != nil { return nil, err } @@ -51,17 +62,21 @@ func (c *HTTPContainerInfo) GetContainerInfo(host, name string) (interface{}, er if err != nil { return nil, err } - var data interface{} - err = json.Unmarshal(body, &data) - return data, err + // Check that this data can be unmarshalled + var container docker.Container + err = json.Unmarshal(body, &container) + if err != nil { + return nil, err + } + return &container, nil } // Useful for testing. type FakeContainerInfo struct { - data interface{} + data *docker.Container err error } -func (c *FakeContainerInfo) GetContainerInfo(host, name string) (interface{}, error) { +func (c *FakeContainerInfo) GetContainerInfo(host, name string) (*docker.Container, error) { return c.data, c.err } diff --git a/pkg/client/container_info_test.go b/pkg/client/container_info_test.go index 1b0904f9596..5d04c361ad6 100644 --- a/pkg/client/container_info_test.go +++ b/pkg/client/container_info_test.go @@ -26,6 +26,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/fsouza/go-dockerclient" ) // TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove. @@ -36,10 +37,12 @@ func expectNoError(t *testing.T, err error) { } func TestHTTPContainerInfo(t *testing.T) { - body := `{"items":[]}` + expectObj := &docker.Container{ID: "myID"} + body, err := json.Marshal(expectObj) + expectNoError(t, err) fakeHandler := util.FakeHandler{ StatusCode: 200, - ResponseBody: body, + ResponseBody: string(body), } testServer := httptest.NewServer(&fakeHandler) @@ -53,10 +56,11 @@ func TestHTTPContainerInfo(t *testing.T) { Client: http.DefaultClient, Port: uint(port), } - data, err := containerInfo.GetContainerInfo(parts[0], "foo") + gotObj, err := containerInfo.GetContainerInfo(parts[0], "foo") expectNoError(t, err) - dataString, _ := json.Marshal(data) - if string(dataString) != body { - t.Errorf("Unexpected response. Expected: %s, received %s", body, string(dataString)) + + // reflect.DeepEqual(expectObj, gotObj) doesn't handle blank times well + if expectObj.ID != gotObj.ID { + t.Errorf("Unexpected response. Expected: %#v, received %#v", expectObj, gotObj) } } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 7bdb48cb4fe..79a4d48a6cf 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -784,17 +784,16 @@ func (kl *Kubelet) getContainerIdFromName(name string) (DockerId, bool, error) { } // Returns docker info for a container -func (kl *Kubelet) GetContainerInfo(name string) (string, error) { +func (kl *Kubelet) GetContainerInfo(name string) (*docker.Container, error) { dockerId, found, err := kl.getContainerIdFromName(name) if err != nil || !found { - return "{}", err + return nil, err } info, err := kl.DockerClient.InspectContainer(string(dockerId)) if err != nil { - return "{}", err + return nil, err } - data, err := json.Marshal(info) - return string(data), err + return info, nil } //Returns stats (from Cadvisor) for a container diff --git a/pkg/kubelet/kubelet_server.go b/pkg/kubelet/kubelet_server.go index 4c7f45ea45c..965cda1d0e1 100644 --- a/pkg/kubelet/kubelet_server.go +++ b/pkg/kubelet/kubelet_server.go @@ -24,6 +24,7 @@ import ( "net/url" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/fsouza/go-dockerclient" "gopkg.in/v1/yaml" ) @@ -36,7 +37,7 @@ type KubeletServer struct { // For testablitiy. type kubeletInterface interface { GetContainerStats(name string) (*api.ContainerStats, error) - GetContainerInfo(name string) (string, error) + GetContainerInfo(name string) (*docker.Container, error) } func (s *KubeletServer) error(w http.ResponseWriter, err error) { @@ -113,7 +114,13 @@ func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { fmt.Fprint(w, "Missing container selector arg.") return } - data, err := s.Kubelet.GetContainerInfo(container) + info, err := s.Kubelet.GetContainerInfo(container) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintf(w, "Internal Error: %v", err) + return + } + data, err := json.Marshal(info) if err != nil { w.WriteHeader(http.StatusInternalServerError) fmt.Fprintf(w, "Internal Error: %v", err) @@ -121,7 +128,7 @@ func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { } w.WriteHeader(http.StatusOK) w.Header().Add("Content-type", "application/json") - fmt.Fprint(w, data) + w.Write(data) default: w.WriteHeader(http.StatusNotFound) fmt.Fprint(w, "Not found.") diff --git a/pkg/kubelet/kubelet_server_test.go b/pkg/kubelet/kubelet_server_test.go index 22891e2a42f..c63df291cf5 100644 --- a/pkg/kubelet/kubelet_server_test.go +++ b/pkg/kubelet/kubelet_server_test.go @@ -28,14 +28,15 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/fsouza/go-dockerclient" ) type fakeKubelet struct { - infoFunc func(name string) (string, error) + infoFunc func(name string) (*docker.Container, error) statsFunc func(name string) (*api.ContainerStats, error) } -func (fk *fakeKubelet) GetContainerInfo(name string) (string, error) { +func (fk *fakeKubelet) GetContainerInfo(name string) (*docker.Container, error) { return fk.infoFunc(name) } @@ -116,12 +117,12 @@ func TestContainers(t *testing.T) { func TestContainerInfo(t *testing.T) { fw := makeServerTest() - expected := "good container info string" - fw.fakeKubelet.infoFunc = func(name string) (string, error) { + expected := &docker.Container{ID: "myContainerID"} + fw.fakeKubelet.infoFunc = func(name string) (*docker.Container, error) { if name == "goodcontainer" { return expected, nil } - return "", fmt.Errorf("bad container") + return nil, fmt.Errorf("bad container") } resp, err := http.Get(fw.testHttpServer.URL + "/containerInfo?container=goodcontainer") if err != nil { @@ -131,7 +132,11 @@ func TestContainerInfo(t *testing.T) { if err != nil { t.Errorf("Error reading body: %v", err) } - if got != expected { + expectedBytes, err := json.Marshal(expected) + if err != nil { + t.Fatalf("Unexpected marshal error %v", err) + } + if got != string(expectedBytes) { t.Errorf("Expected: '%v', got: '%v'", expected, got) } } diff --git a/pkg/master/pod_cache.go b/pkg/master/pod_cache.go index 33b6da00819..d3b20601b4b 100644 --- a/pkg/master/pod_cache.go +++ b/pkg/master/pod_cache.go @@ -17,6 +17,7 @@ limitations under the License. package master import ( + "errors" "sync" "time" @@ -24,6 +25,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/fsouza/go-dockerclient" "github.com/golang/glog" ) @@ -32,7 +34,7 @@ import ( type PodCache struct { containerInfo client.ContainerInfo pods registry.PodRegistry - podInfo map[string]interface{} + podInfo map[string]docker.Container period time.Duration podLock sync.Mutex } @@ -41,21 +43,20 @@ func NewPodCache(info client.ContainerInfo, pods registry.PodRegistry, period ti return &PodCache{ containerInfo: info, pods: pods, - podInfo: map[string]interface{}{}, + podInfo: map[string]docker.Container{}, period: period, } } -// Implements the ContainerInfo interface -// The returned value should be treated as read-only -func (p *PodCache) GetContainerInfo(host, id string) (interface{}, error) { +// Implements the ContainerInfo interface. +func (p *PodCache) GetContainerInfo(host, id string) (*docker.Container, error) { p.podLock.Lock() defer p.podLock.Unlock() value, ok := p.podInfo[id] if !ok { - return nil, nil + return nil, errors.New("No cached pod info") } else { - return value, nil + return &value, nil } } @@ -66,7 +67,7 @@ func (p *PodCache) updateContainerInfo(host, id string) error { } p.podLock.Lock() defer p.podLock.Unlock() - p.podInfo[id] = info + p.podInfo[id] = *info return nil } diff --git a/pkg/master/pod_cache_test.go b/pkg/master/pod_cache_test.go index 9db9d8db4bb..7465041b5e9 100644 --- a/pkg/master/pod_cache_test.go +++ b/pkg/master/pod_cache_test.go @@ -23,16 +23,17 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry" + "github.com/fsouza/go-dockerclient" ) type FakeContainerInfo struct { host string id string - data interface{} + data *docker.Container err error } -func (f *FakeContainerInfo) GetContainerInfo(host, id string) (interface{}, error) { +func (f *FakeContainerInfo) GetContainerInfo(host, id string) (*docker.Container, error) { f.host = host f.id = id return f.data, f.err @@ -41,17 +42,15 @@ func (f *FakeContainerInfo) GetContainerInfo(host, id string) (interface{}, erro func TestPodCacheGet(t *testing.T) { cache := NewPodCache(nil, nil, time.Second*1) - pod := api.Pod{ - JSONBase: api.JSONBase{ID: "foo"}, - } - cache.podInfo["foo"] = pod + expected := docker.Container{ID: "foo"} + cache.podInfo["foo"] = expected info, err := cache.GetContainerInfo("host", "foo") if err != nil { t.Errorf("Unexpected error: %#v", err) } - if !reflect.DeepEqual(info, pod) { - t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", pod, info) + if !reflect.DeepEqual(info, &expected) { + t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", &expected, info) } } @@ -59,8 +58,8 @@ func TestPodCacheGetMissing(t *testing.T) { cache := NewPodCache(nil, nil, time.Second*1) info, err := cache.GetContainerInfo("host", "foo") - if err != nil { - t.Errorf("Unexpected error: %#v", err) + if err == nil { + t.Errorf("Unexpected non-error: %#v", err) } if info != nil { t.Errorf("Unexpected info: %#v", info) @@ -68,11 +67,9 @@ func TestPodCacheGetMissing(t *testing.T) { } func TestPodGetContainerInfo(t *testing.T) { - pod := api.Pod{ - JSONBase: api.JSONBase{ID: "foo"}, - } + expected := docker.Container{ID: "foo"} fake := FakeContainerInfo{ - data: pod, + data: &expected, } cache := NewPodCache(&fake, nil, time.Second*1) @@ -86,8 +83,8 @@ func TestPodGetContainerInfo(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %#v", err) } - if !reflect.DeepEqual(info, pod) { - t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", pod, info) + if !reflect.DeepEqual(info, &expected) { + t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", &expected, info) } } @@ -98,10 +95,13 @@ func TestPodUpdateAllContainers(t *testing.T) { Host: "machine", }, } + pods := []api.Pod{pod} mockRegistry := registry.MakeMockPodRegistry(pods) + + expected := docker.Container{ID: "foo"} fake := FakeContainerInfo{ - data: pod, + data: &expected, } cache := NewPodCache(&fake, mockRegistry, time.Second*1) @@ -115,7 +115,7 @@ func TestPodUpdateAllContainers(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %#v", err) } - if !reflect.DeepEqual(info, pod) { - t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", pod, info) + if !reflect.DeepEqual(info, &expected) { + t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", &expected, info) } } diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index 6b1321366e4..495cee38719 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -27,6 +27,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/scheduler" + "github.com/fsouza/go-dockerclient" "github.com/golang/glog" ) @@ -71,34 +72,62 @@ func (storage *PodRegistryStorage) List(selector labels.Selector) (interface{}, pods, err := storage.registry.ListPods(selector) if err == nil { result.Items = pods - // Get cached info for the list currently. - // TODO: Optionally use fresh info - if storage.podCache != nil { - for ix, pod := range pods { - info, err := storage.podCache.GetContainerInfo(pod.CurrentState.Host, pod.ID) - if err != nil { - glog.Errorf("Error getting container info: %#v", err) - continue - } - result.Items[ix].CurrentState.Info = info - } + for i := range result.Items { + storage.fillPodInfo(&result.Items[i]) } } return result, err } -func makePodStatus(info interface{}) api.PodStatus { - if state, ok := info.(map[string]interface{})["State"]; ok { - if running, ok := state.(map[string]interface{})["Running"]; ok { - if running.(bool) { - return api.PodRunning - } else { - return api.PodStopped +func (storage *PodRegistryStorage) fillPodInfo(pod *api.Pod) { + // Get cached info for the list currently. + // TODO: Optionally use fresh info + if storage.podCache != nil { + pod.CurrentState.Info = map[string]docker.Container{} + infoMap := pod.CurrentState.Info + + for _, container := range pod.DesiredState.Manifest.Containers { + // TODO: clearly need to pass both pod ID and container name here. + info, err := storage.podCache.GetContainerInfo(pod.CurrentState.Host, pod.ID) + if err != nil { + glog.Errorf("Error getting container info: %#v", err) + continue } + infoMap[container.Name] = *info } } - return api.PodPending +} + +func makePodStatus(pod *api.Pod) api.PodStatus { + if pod.CurrentState.Info == nil { + return api.PodPending + } + running := 0 + stopped := 0 + unknown := 0 + for _, container := range pod.DesiredState.Manifest.Containers { + if info, ok := pod.CurrentState.Info[container.Name]; ok { + if info.State.Running { + running++ + } else { + stopped++ + } + } else { + unknown++ + } + } + + switch { + case running > 0 && stopped == 0 && unknown == 0: + return api.PodRunning + case running == 0 && stopped > 0 && unknown == 0: + return api.PodStopped + case running == 0 && stopped == 0 && unknown > 0: + return api.PodPending + default: + return api.PodPending + } } func getInstanceIP(cloud cloudprovider.Interface, host string) string { @@ -130,12 +159,8 @@ func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { return pod, nil } if storage.containerInfo != nil { - info, err := storage.containerInfo.GetContainerInfo(pod.CurrentState.Host, id) - if err != nil { - return pod, err - } - pod.CurrentState.Info = info - pod.CurrentState.Status = makePodStatus(info) + storage.fillPodInfo(pod) + pod.CurrentState.Status = makePodStatus(pod) } pod.CurrentState.HostIP = getInstanceIP(storage.cloud, pod.CurrentState.Host) diff --git a/pkg/registry/pod_registry_test.go b/pkg/registry/pod_registry_test.go index 42505ca876c..c5a42d081ba 100644 --- a/pkg/registry/pod_registry_test.go +++ b/pkg/registry/pod_registry_test.go @@ -26,6 +26,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/scheduler" + "github.com/fsouza/go-dockerclient" ) func expectNoError(t *testing.T, err error) { @@ -153,29 +154,88 @@ func TestGetPodCloud(t *testing.T) { } func TestMakePodStatus(t *testing.T) { - status := makePodStatus(map[string]interface{}{}) + desiredState := api.PodState{ + Manifest: api.ContainerManifest{ + Containers: []api.Container{ + {Name: "containerA"}, + {Name: "containerB"}, + }, + }, + } + pod := &api.Pod{DesiredState: desiredState} + status := makePodStatus(pod) if status != api.PodPending { t.Errorf("Expected 'Pending', got '%s'", status) } - status = makePodStatus(map[string]interface{}{ - "State": map[string]interface{}{ - "Running": false, + runningState := docker.Container{ + State: docker.State{ + Running: true, }, - }) + } + stoppedState := docker.Container{ + State: docker.State{ + Running: false, + }, + } + // All running. + pod = &api.Pod{ + DesiredState: desiredState, + CurrentState: api.PodState{ + Info: map[string]docker.Container{ + "containerA": runningState, + "containerB": runningState, + }, + }, + } + status = makePodStatus(pod) + if status != api.PodRunning { + t.Errorf("Expected 'Running', got '%s'", status) + } + + // All stopped. + pod = &api.Pod{ + DesiredState: desiredState, + CurrentState: api.PodState{ + Info: map[string]docker.Container{ + "containerA": stoppedState, + "containerB": stoppedState, + }, + }, + } + status = makePodStatus(pod) if status != api.PodStopped { t.Errorf("Expected 'Stopped', got '%s'", status) } - status = makePodStatus(map[string]interface{}{ - "State": map[string]interface{}{ - "Running": true, + // Mixed state. + pod = &api.Pod{ + DesiredState: desiredState, + CurrentState: api.PodState{ + Info: map[string]docker.Container{ + "containerA": runningState, + "containerB": stoppedState, + }, }, - }) + } + status = makePodStatus(pod) + if status != api.PodPending { + t.Errorf("Expected 'Pending', got '%s'", status) + } - if status != api.PodRunning { - t.Errorf("Expected 'Running', got '%s'", status) + // Mixed state. + pod = &api.Pod{ + DesiredState: desiredState, + CurrentState: api.PodState{ + Info: map[string]docker.Container{ + "containerA": runningState, + }, + }, + } + status = makePodStatus(pod) + if status != api.PodPending { + t.Errorf("Expected 'Pending', got '%s'", status) } } From 11d6451d2a6d75f1ef8a8ffea440acb73e3dfd75 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Tue, 1 Jul 2014 15:35:56 -0700 Subject: [PATCH 3/6] Change kublet to serve podInfo instead of containerInfo. Plumb through system. --- pkg/api/types.go | 5 ++- pkg/client/{container_info.go => podinfo.go} | 33 ++++++++-------- ...container_info_test.go => podinfo_test.go} | 13 ++++--- pkg/kubelet/kubelet.go | 24 ++++++++---- pkg/kubelet/kubelet_server.go | 15 +++----- pkg/kubelet/kubelet_server_test.go | 16 ++++---- pkg/master/master.go | 2 +- pkg/master/pod_cache.go | 32 ++++++++-------- pkg/master/pod_cache_test.go | 38 +++++++++---------- pkg/registry/pod_registry.go | 31 ++++++--------- 10 files changed, 108 insertions(+), 101 deletions(-) rename pkg/client/{container_info.go => podinfo.go} (60%) rename pkg/client/{container_info_test.go => podinfo_test.go} (82%) diff --git a/pkg/api/types.go b/pkg/api/types.go index b6d504a0935..9e11e52714a 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -157,6 +157,9 @@ const ( PodStopped PodStatus = "Stopped" ) +// PodInfo contains one entry for every container with available info. +type PodInfo map[string]docker.Container + // PodState is the state of a pod, used as either input (desired state) or output (current state) type PodState struct { Manifest ContainerManifest `json:"manifest,omitempty" yaml:"manifest,omitempty"` @@ -169,7 +172,7 @@ type PodState struct { // of `docker inspect`. This output format is *not* final and should not be relied // upon. // TODO: Make real decisions about what our info should look like. - Info map[string]docker.Container `json:"info,omitempty" yaml:"info,omitempty"` + Info PodInfo `json:"info,omitempty" yaml:"info,omitempty"` } type PodList struct { diff --git a/pkg/client/container_info.go b/pkg/client/podinfo.go similarity index 60% rename from pkg/client/container_info.go rename to pkg/client/podinfo.go index c4ec0c031a2..e216662f5fe 100644 --- a/pkg/client/container_info.go +++ b/pkg/client/podinfo.go @@ -24,31 +24,30 @@ import ( "net/http" "strconv" - "github.com/fsouza/go-dockerclient" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) -// ContainerInfo is an interface for things that can get information about a container. +// PodInfoGetter is an interface for things that can get information about a pod's containers. // Injectable for easy testing. -type ContainerInfo interface { - // GetContainerInfo returns information about container 'name' on 'host' - // Returns a json-formatted []byte (which can be unmarshalled into a - // map[string]interface{}) or an error if one occurs. - GetContainerInfo(host, name string) (*docker.Container, error) +type PodInfoGetter interface { + // GetPodInfo returns information about all containers which are part + // Returns an api.PodInfo, or an error if one occurs. + GetPodInfo(host, podID string) (api.PodInfo, error) } // The default implementation, accesses the kubelet over HTTP -type HTTPContainerInfo struct { +type HTTPPodInfoGetter struct { Client *http.Client Port uint } -func (c *HTTPContainerInfo) GetContainerInfo(host, name string) (*docker.Container, error) { +func (c *HTTPPodInfoGetter) GetPodInfo(host, podID string) (api.PodInfo, error) { request, err := http.NewRequest( "GET", fmt.Sprintf( - "http://%s/containerInfo?container=%s", + "http://%s/podInfo?podID=%s", net.JoinHostPort(host, strconv.FormatUint(uint64(c.Port), 10)), - name), + podID), nil) if err != nil { return nil, err @@ -63,20 +62,20 @@ func (c *HTTPContainerInfo) GetContainerInfo(host, name string) (*docker.Contain return nil, err } // Check that this data can be unmarshalled - var container docker.Container - err = json.Unmarshal(body, &container) + info := api.PodInfo{} + err = json.Unmarshal(body, &info) if err != nil { return nil, err } - return &container, nil + return info, nil } // Useful for testing. -type FakeContainerInfo struct { - data *docker.Container +type FakePodInfoGetter struct { + data api.PodInfo err error } -func (c *FakeContainerInfo) GetContainerInfo(host, name string) (*docker.Container, error) { +func (c *FakePodInfoGetter) GetPodInfo(host, podID string) (api.PodInfo, error) { return c.data, c.err } diff --git a/pkg/client/container_info_test.go b/pkg/client/podinfo_test.go similarity index 82% rename from pkg/client/container_info_test.go rename to pkg/client/podinfo_test.go index 5d04c361ad6..7748b7b3996 100644 --- a/pkg/client/container_info_test.go +++ b/pkg/client/podinfo_test.go @@ -25,6 +25,7 @@ import ( "strings" "testing" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/fsouza/go-dockerclient" ) @@ -36,8 +37,10 @@ func expectNoError(t *testing.T, err error) { } } -func TestHTTPContainerInfo(t *testing.T) { - expectObj := &docker.Container{ID: "myID"} +func TestHTTPPodInfoGetter(t *testing.T) { + expectObj := api.PodInfo{ + "myID": docker.Container{ID: "myID"}, + } body, err := json.Marshal(expectObj) expectNoError(t, err) fakeHandler := util.FakeHandler{ @@ -52,15 +55,15 @@ func TestHTTPContainerInfo(t *testing.T) { port, err := strconv.Atoi(parts[1]) expectNoError(t, err) - containerInfo := &HTTPContainerInfo{ + podInfoGetter := &HTTPPodInfoGetter{ Client: http.DefaultClient, Port: uint(port), } - gotObj, err := containerInfo.GetContainerInfo(parts[0], "foo") + gotObj, err := podInfoGetter.GetPodInfo(parts[0], "foo") expectNoError(t, err) // reflect.DeepEqual(expectObj, gotObj) doesn't handle blank times well - if expectObj.ID != gotObj.ID { + if len(gotObj) != len(expectObj) || expectObj["myID"].ID != gotObj["myID"].ID { t.Errorf("Unexpected response. Expected: %#v, received %#v", expectObj, gotObj) } } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 79a4d48a6cf..eac9c225241 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -783,16 +783,26 @@ func (kl *Kubelet) getContainerIdFromName(name string) (DockerId, bool, error) { return "", false, nil } -// Returns docker info for a container -func (kl *Kubelet) GetContainerInfo(name string) (*docker.Container, error) { - dockerId, found, err := kl.getContainerIdFromName(name) - if err != nil || !found { - return nil, err - } - info, err := kl.DockerClient.InspectContainer(string(dockerId)) +// Returns docker info for all containers in the pod/manifest +func (kl *Kubelet) GetPodInfo(podID string) (api.PodInfo, error) { + info := api.PodInfo{} + + containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{}) if err != nil { return nil, err } + + for _, value := range containerList { + manifestID, containerName := parseDockerName(value.Names[0]) + if manifestID != podID { + continue + } + inspectResult, err := kl.DockerClient.InspectContainer(value.ID) + if err != nil { + return nil, err + } + info[containerName] = *inspectResult + } return info, nil } diff --git a/pkg/kubelet/kubelet_server.go b/pkg/kubelet/kubelet_server.go index 965cda1d0e1..b2ab0b8a362 100644 --- a/pkg/kubelet/kubelet_server.go +++ b/pkg/kubelet/kubelet_server.go @@ -24,7 +24,6 @@ import ( "net/url" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/fsouza/go-dockerclient" "gopkg.in/v1/yaml" ) @@ -37,7 +36,7 @@ type KubeletServer struct { // For testablitiy. type kubeletInterface interface { GetContainerStats(name string) (*api.ContainerStats, error) - GetContainerInfo(name string) (*docker.Container, error) + GetPodInfo(name string) (api.PodInfo, error) } func (s *KubeletServer) error(w http.ResponseWriter, err error) { @@ -105,16 +104,14 @@ func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) 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 { + case u.Path == "/podInfo": + podID := u.Query().Get("podID") + if len(podID) == 0 { w.WriteHeader(http.StatusBadRequest) - fmt.Fprint(w, "Missing container selector arg.") + fmt.Fprint(w, "Missing 'podID=' query entry.") return } - info, err := s.Kubelet.GetContainerInfo(container) + info, err := s.Kubelet.GetPodInfo(podID) if err != nil { w.WriteHeader(http.StatusInternalServerError) fmt.Fprintf(w, "Internal Error: %v", err) diff --git a/pkg/kubelet/kubelet_server_test.go b/pkg/kubelet/kubelet_server_test.go index c63df291cf5..6b2b14f86f0 100644 --- a/pkg/kubelet/kubelet_server_test.go +++ b/pkg/kubelet/kubelet_server_test.go @@ -32,11 +32,11 @@ import ( ) type fakeKubelet struct { - infoFunc func(name string) (*docker.Container, error) + infoFunc func(name string) (api.PodInfo, error) statsFunc func(name string) (*api.ContainerStats, error) } -func (fk *fakeKubelet) GetContainerInfo(name string) (*docker.Container, error) { +func (fk *fakeKubelet) GetPodInfo(name string) (api.PodInfo, error) { return fk.infoFunc(name) } @@ -115,16 +115,16 @@ func TestContainers(t *testing.T) { } } -func TestContainerInfo(t *testing.T) { +func TestPodInfo(t *testing.T) { fw := makeServerTest() - expected := &docker.Container{ID: "myContainerID"} - fw.fakeKubelet.infoFunc = func(name string) (*docker.Container, error) { - if name == "goodcontainer" { + expected := api.PodInfo{"goodpod": docker.Container{ID: "myContainerID"}} + fw.fakeKubelet.infoFunc = func(name string) (api.PodInfo, error) { + if name == "goodpod" { return expected, nil } - return nil, fmt.Errorf("bad container") + return nil, fmt.Errorf("bad pod") } - resp, err := http.Get(fw.testHttpServer.URL + "/containerInfo?container=goodcontainer") + resp, err := http.Get(fw.testHttpServer.URL + "/podInfo?podID=goodpod") if err != nil { t.Errorf("Got error GETing: %v", err) } diff --git a/pkg/master/master.go b/pkg/master/master.go index 1c60eaba5a3..70dd71cec07 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -80,7 +80,7 @@ func New(etcdServers, minions []string, cloud cloudprovider.Interface, minionReg } func (m *Master) init(cloud cloudprovider.Interface) { - containerInfo := &client.HTTPContainerInfo{ + containerInfo := &client.HTTPPodInfoGetter{ Client: http.DefaultClient, Port: 10250, } diff --git a/pkg/master/pod_cache.go b/pkg/master/pod_cache.go index d3b20601b4b..d075e6cde57 100644 --- a/pkg/master/pod_cache.go +++ b/pkg/master/pod_cache.go @@ -21,53 +21,55 @@ import ( "sync" "time" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" - "github.com/fsouza/go-dockerclient" "github.com/golang/glog" ) // PodCache contains both a cache of container information, as well as the mechanism for keeping // that cache up to date. type PodCache struct { - containerInfo client.ContainerInfo + containerInfo client.PodInfoGetter pods registry.PodRegistry - podInfo map[string]docker.Container - period time.Duration - podLock sync.Mutex + // This is a map of pod id to a map of container name to the + podInfo map[string]api.PodInfo + period time.Duration + podLock sync.Mutex } -func NewPodCache(info client.ContainerInfo, pods registry.PodRegistry, period time.Duration) *PodCache { +func NewPodCache(info client.PodInfoGetter, pods registry.PodRegistry, period time.Duration) *PodCache { return &PodCache{ containerInfo: info, pods: pods, - podInfo: map[string]docker.Container{}, + podInfo: map[string]api.PodInfo{}, period: period, } } -// Implements the ContainerInfo interface. -func (p *PodCache) GetContainerInfo(host, id string) (*docker.Container, error) { +// Implements the PodInfoGetter interface. +// The returned value should be treated as read-only. +func (p *PodCache) GetPodInfo(host, podID string) (api.PodInfo, error) { p.podLock.Lock() defer p.podLock.Unlock() - value, ok := p.podInfo[id] + value, ok := p.podInfo[podID] if !ok { return nil, errors.New("No cached pod info") } else { - return &value, nil + return value, nil } } -func (p *PodCache) updateContainerInfo(host, id string) error { - info, err := p.containerInfo.GetContainerInfo(host, id) +func (p *PodCache) updatePodInfo(host, id string) error { + info, err := p.containerInfo.GetPodInfo(host, id) if err != nil { return err } p.podLock.Lock() defer p.podLock.Unlock() - p.podInfo[id] = *info + p.podInfo[id] = info return nil } @@ -79,7 +81,7 @@ func (p *PodCache) UpdateAllContainers() { return } for _, pod := range pods { - err := p.updateContainerInfo(pod.CurrentState.Host, pod.ID) + err := p.updatePodInfo(pod.CurrentState.Host, pod.ID) if err != nil { glog.Errorf("Error synchronizing container: %#v", err) } diff --git a/pkg/master/pod_cache_test.go b/pkg/master/pod_cache_test.go index 7465041b5e9..f2eb86c1d8e 100644 --- a/pkg/master/pod_cache_test.go +++ b/pkg/master/pod_cache_test.go @@ -26,14 +26,14 @@ import ( "github.com/fsouza/go-dockerclient" ) -type FakeContainerInfo struct { +type FakePodInfoGetter struct { host string id string - data *docker.Container + data api.PodInfo err error } -func (f *FakeContainerInfo) GetContainerInfo(host, id string) (*docker.Container, error) { +func (f *FakePodInfoGetter) GetPodInfo(host, id string) (api.PodInfo, error) { f.host = host f.id = id return f.data, f.err @@ -42,14 +42,14 @@ func (f *FakeContainerInfo) GetContainerInfo(host, id string) (*docker.Container func TestPodCacheGet(t *testing.T) { cache := NewPodCache(nil, nil, time.Second*1) - expected := docker.Container{ID: "foo"} + expected := api.PodInfo{"foo": docker.Container{ID: "foo"}} cache.podInfo["foo"] = expected - info, err := cache.GetContainerInfo("host", "foo") + info, err := cache.GetPodInfo("host", "foo") if err != nil { t.Errorf("Unexpected error: %#v", err) } - if !reflect.DeepEqual(info, &expected) { + if !reflect.DeepEqual(info, expected) { t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", &expected, info) } } @@ -57,7 +57,7 @@ func TestPodCacheGet(t *testing.T) { func TestPodCacheGetMissing(t *testing.T) { cache := NewPodCache(nil, nil, time.Second*1) - info, err := cache.GetContainerInfo("host", "foo") + info, err := cache.GetPodInfo("host", "foo") if err == nil { t.Errorf("Unexpected non-error: %#v", err) } @@ -66,24 +66,24 @@ func TestPodCacheGetMissing(t *testing.T) { } } -func TestPodGetContainerInfo(t *testing.T) { - expected := docker.Container{ID: "foo"} - fake := FakeContainerInfo{ - data: &expected, +func TestPodGetPodInfoGetter(t *testing.T) { + expected := api.PodInfo{"foo": docker.Container{ID: "foo"}} + fake := FakePodInfoGetter{ + data: expected, } cache := NewPodCache(&fake, nil, time.Second*1) - cache.updateContainerInfo("host", "foo") + cache.updatePodInfo("host", "foo") if fake.host != "host" || fake.id != "foo" { t.Errorf("Unexpected access: %#v", fake) } - info, err := cache.GetContainerInfo("host", "foo") + info, err := cache.GetPodInfo("host", "foo") if err != nil { t.Errorf("Unexpected error: %#v", err) } - if !reflect.DeepEqual(info, &expected) { + if !reflect.DeepEqual(info, expected) { t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", &expected, info) } } @@ -99,9 +99,9 @@ func TestPodUpdateAllContainers(t *testing.T) { pods := []api.Pod{pod} mockRegistry := registry.MakeMockPodRegistry(pods) - expected := docker.Container{ID: "foo"} - fake := FakeContainerInfo{ - data: &expected, + expected := api.PodInfo{"foo": docker.Container{ID: "foo"}} + fake := FakePodInfoGetter{ + data: expected, } cache := NewPodCache(&fake, mockRegistry, time.Second*1) @@ -111,11 +111,11 @@ func TestPodUpdateAllContainers(t *testing.T) { t.Errorf("Unexpected access: %#v", fake) } - info, err := cache.GetContainerInfo("machine", "foo") + info, err := cache.GetPodInfo("machine", "foo") if err != nil { t.Errorf("Unexpected error: %#v", err) } - if !reflect.DeepEqual(info, &expected) { + if !reflect.DeepEqual(info, expected) { t.Errorf("Unexpected mismatch. Expected: %#v, Got: #%v", &expected, info) } } diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index 495cee38719..bc6415bc294 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -27,15 +27,14 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/scheduler" - "github.com/fsouza/go-dockerclient" "github.com/golang/glog" ) // PodRegistryStorage implements the RESTStorage interface in terms of a PodRegistry type PodRegistryStorage struct { registry PodRegistry - containerInfo client.ContainerInfo - podCache client.ContainerInfo + podInfoGetter client.PodInfoGetter + podCache client.PodInfoGetter scheduler scheduler.Scheduler minionLister scheduler.MinionLister cloud cloudprovider.Interface @@ -45,20 +44,20 @@ type PodRegistryStorage struct { // MakePodRegistryStorage makes a RESTStorage object for a pod registry. // Parameters: // registry: The pod registry -// containerInfo: Source of fresh container info +// podInfoGetter: Source of fresh container info // scheduler: The scheduler for assigning pods to machines // minionLister: Object which can list available minions for the scheduler // cloud: Interface to a cloud provider (may be null) // podCache: Source of cached container info func MakePodRegistryStorage(registry PodRegistry, - containerInfo client.ContainerInfo, + podInfoGetter client.PodInfoGetter, scheduler scheduler.Scheduler, minionLister scheduler.MinionLister, cloud cloudprovider.Interface, - podCache client.ContainerInfo) apiserver.RESTStorage { + podCache client.PodInfoGetter) apiserver.RESTStorage { return &PodRegistryStorage{ registry: registry, - containerInfo: containerInfo, + podInfoGetter: podInfoGetter, scheduler: scheduler, minionLister: minionLister, cloud: cloud, @@ -84,18 +83,12 @@ func (storage *PodRegistryStorage) fillPodInfo(pod *api.Pod) { // Get cached info for the list currently. // TODO: Optionally use fresh info if storage.podCache != nil { - pod.CurrentState.Info = map[string]docker.Container{} - infoMap := pod.CurrentState.Info - - for _, container := range pod.DesiredState.Manifest.Containers { - // TODO: clearly need to pass both pod ID and container name here. - info, err := storage.podCache.GetContainerInfo(pod.CurrentState.Host, pod.ID) - if err != nil { - glog.Errorf("Error getting container info: %#v", err) - continue - } - infoMap[container.Name] = *info + info, err := storage.podCache.GetPodInfo(pod.CurrentState.Host, pod.ID) + if err != nil { + glog.Errorf("Error getting container info: %#v", err) + return } + pod.CurrentState.Info = info } } @@ -158,7 +151,7 @@ func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { if pod == nil { return pod, nil } - if storage.containerInfo != nil { + if storage.podCache != nil || storage.podInfoGetter != nil { storage.fillPodInfo(pod) pod.CurrentState.Status = makePodStatus(pod) } From 587fb75a7a40733f65af6782afe8b88855b2082c Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Tue, 1 Jul 2014 16:47:37 -0700 Subject: [PATCH 4/6] rearrange RunKubelet's parameters so that address and port are next to each other --- cmd/integration/integration.go | 4 ++-- cmd/kubelet/kubelet.go | 2 +- pkg/kubelet/kubelet.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index a90f7fb99af..76ed5abc216 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -64,7 +64,7 @@ func startComponents(manifestURL string) (apiServerURL string) { SyncFrequency: 5 * time.Second, HTTPCheckFrequency: 5 * time.Second, } - go myKubelet.RunKubelet("", manifestURL, servers[0], "localhost", "", 0) + go myKubelet.RunKubelet("", "", manifestURL, servers[0], "localhost", 10250) // Create a second kubelet so that the guestbook example's two redis slaves both // have a place they can schedule. @@ -76,7 +76,7 @@ func startComponents(manifestURL string) (apiServerURL string) { SyncFrequency: 5 * time.Second, HTTPCheckFrequency: 5 * time.Second, } - go otherKubelet.RunKubelet("", "", servers[0], "localhost", "", 0) + go otherKubelet.RunKubelet("", "", "", servers[0], "localhost", 10251) return apiserver.URL } diff --git a/cmd/kubelet/kubelet.go b/cmd/kubelet/kubelet.go index 55e880a3d39..865ea19ef11 100644 --- a/cmd/kubelet/kubelet.go +++ b/cmd/kubelet/kubelet.go @@ -89,5 +89,5 @@ func main() { SyncFrequency: *syncFrequency, HTTPCheckFrequency: *httpCheckFrequency, } - my_kubelet.RunKubelet(*config, *manifestUrl, *etcdServers, *address, *dockerEndpoint, *port) + my_kubelet.RunKubelet(*dockerEndpoint, *config, *manifestUrl, *etcdServers, *address, *port) } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index eac9c225241..a668d8f8083 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -101,9 +101,9 @@ const ( // Starts background goroutines. If config_path, manifest_url, or address are empty, // they are not watched. Never returns. -func (kl *Kubelet) RunKubelet(config_path, manifest_url, etcd_servers, address, endpoint string, port uint) { +func (kl *Kubelet) RunKubelet(dockerEndpoint, config_path, manifest_url, etcd_servers, address string, port uint) { if kl.DockerPuller == nil { - kl.DockerPuller = MakeDockerPuller(endpoint) + kl.DockerPuller = MakeDockerPuller(dockerEndpoint) } updateChannel := make(chan manifestUpdate) if config_path != "" { From bf3b34c2e9cf88b6a41a011ab61d5a5761af478e Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Tue, 1 Jul 2014 17:08:32 -0700 Subject: [PATCH 5/6] Allow master's pod info getter to be faked. Wire up in integration tests in futile attempt to make travis pass. --- cmd/apiserver/apiserver.go | 12 +++++++++-- cmd/integration/integration.go | 25 +++++++++++++++++++++- pkg/master/master.go | 38 +++++++++++++++------------------- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/cmd/apiserver/apiserver.go b/cmd/apiserver/apiserver.go index b38e761da59..584f3581488 100644 --- a/cmd/apiserver/apiserver.go +++ b/cmd/apiserver/apiserver.go @@ -21,8 +21,10 @@ package main import ( "flag" "net" + "net/http" "strconv" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/master" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -35,6 +37,7 @@ var ( apiPrefix = flag.String("api_prefix", "/api/v1beta1", "The prefix for API requests on the server. Default '/api/v1beta1'") cloudProvider = flag.String("cloud_provider", "", "The provider for cloud services. Empty string for no provider.") minionRegexp = flag.String("minion_regexp", "", "If non empty, and -cloud_provider is specified, a regular expression for matching minion VMs") + minionPort = flag.Uint("minion_port", 10250, "The port at which kubelet will be listening on the minions.") etcdServerList, machineList util.StringList ) @@ -68,11 +71,16 @@ func main() { } } + podInfoGetter := &client.HTTPPodInfoGetter{ + Client: http.DefaultClient, + Port: *minionPort, + } + var m *master.Master if len(etcdServerList) > 0 { - m = master.New(etcdServerList, machineList, cloud, *minionRegexp) + m = master.New(etcdServerList, machineList, podInfoGetter, cloud, *minionRegexp) } else { - m = master.NewMemoryServer(machineList, cloud) + m = master.NewMemoryServer(machineList, podInfoGetter, cloud) } glog.Fatal(m.Run(net.JoinHostPort(*address, strconv.Itoa(int(*port))), *apiPrefix)) diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 76ed5abc216..3368087270b 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -41,6 +41,29 @@ var ( fakeDocker1, fakeDocker2 kubelet.FakeDockerClient ) +type fakePodInfoGetter struct{} + +func (fakePodInfoGetter) GetPodInfo(host, podID string) (api.PodInfo, error) { + // This is a horrible hack to get around the fact that we can't provide + // different port numbers per kubelet... + var c client.PodInfoGetter + switch host { + case "localhost": + c = &client.HTTPPodInfoGetter{ + Client: http.DefaultClient, + Port: 10250, + } + case "machine": + c = &client.HTTPPodInfoGetter{ + Client: http.DefaultClient, + Port: 10251, + } + default: + glog.Fatalf("Can't get info for: %v, %v", host, podID) + } + return c.GetPodInfo("localhost", podID) +} + func startComponents(manifestURL string) (apiServerURL string) { // Setup servers := []string{"http://localhost:4001"} @@ -48,7 +71,7 @@ func startComponents(manifestURL string) (apiServerURL string) { machineList := []string{"localhost", "machine"} // Master - m := master.New(servers, machineList, nil, "") + m := master.New(servers, machineList, fakePodInfoGetter{}, nil, "") apiserver := httptest.NewServer(m.ConstructHandler("/api/v1beta1")) controllerManager := controller.MakeReplicationManager(etcd.NewClient(servers), client.New(apiserver.URL, nil)) diff --git a/pkg/master/master.go b/pkg/master/master.go index 70dd71cec07..5b17457fcb3 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -44,53 +44,49 @@ type Master struct { } // Returns a memory (not etcd) backed apiserver. -func NewMemoryServer(minions []string, cloud cloudprovider.Interface) *Master { +func NewMemoryServer(minions []string, podInfoGetter client.PodInfoGetter, cloud cloudprovider.Interface) *Master { m := &Master{ podRegistry: registry.MakeMemoryRegistry(), controllerRegistry: registry.MakeMemoryRegistry(), serviceRegistry: registry.MakeMemoryRegistry(), minionRegistry: registry.MakeMinionRegistry(minions), } - m.init(cloud) + m.init(cloud, podInfoGetter) return m } // Returns a new apiserver. -func New(etcdServers, minions []string, cloud cloudprovider.Interface, minionRegexp string) *Master { +func New(etcdServers, minions []string, podInfoGetter client.PodInfoGetter, cloud cloudprovider.Interface, minionRegexp string) *Master { etcdClient := etcd.NewClient(etcdServers) - var minionRegistry registry.MinionRegistry - if cloud != nil && len(minionRegexp) > 0 { - var err error - minionRegistry, err = registry.MakeCloudMinionRegistry(cloud, minionRegexp) - if err != nil { - glog.Errorf("Failed to initalize cloud minion registry reverting to static registry (%#v)", err) - minionRegistry = registry.MakeMinionRegistry(minions) - } - } else { - minionRegistry = registry.MakeMinionRegistry(minions) - } + minionRegistry := minionRegistryMaker(minions, cloud, minionRegexp) m := &Master{ podRegistry: registry.MakeEtcdRegistry(etcdClient, minionRegistry), controllerRegistry: registry.MakeEtcdRegistry(etcdClient, minionRegistry), serviceRegistry: registry.MakeEtcdRegistry(etcdClient, minionRegistry), minionRegistry: minionRegistry, } - m.init(cloud) + m.init(cloud, podInfoGetter) return m } -func (m *Master) init(cloud cloudprovider.Interface) { - containerInfo := &client.HTTPPodInfoGetter{ - Client: http.DefaultClient, - Port: 10250, +func minionRegistryMaker(minions []string, cloud cloudprovider.Interface, minionRegexp string) registry.MinionRegistry { + if cloud != nil && len(minionRegexp) > 0 { + minionRegistry, err := registry.MakeCloudMinionRegistry(cloud, minionRegexp) + if err != nil { + glog.Errorf("Failed to initalize cloud minion registry reverting to static registry (%#v)", err) + } + return minionRegistry } + return registry.MakeMinionRegistry(minions) +} +func (m *Master) init(cloud cloudprovider.Interface, podInfoGetter client.PodInfoGetter) { m.random = rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) - podCache := NewPodCache(containerInfo, m.podRegistry, time.Second*30) + podCache := NewPodCache(podInfoGetter, m.podRegistry, time.Second*30) go podCache.Loop() s := scheduler.MakeFirstFitScheduler(m.podRegistry, m.random) m.storage = map[string]apiserver.RESTStorage{ - "pods": registry.MakePodRegistryStorage(m.podRegistry, containerInfo, s, m.minionRegistry, cloud, podCache), + "pods": registry.MakePodRegistryStorage(m.podRegistry, podInfoGetter, s, m.minionRegistry, cloud, podCache), "replicationControllers": registry.MakeControllerRegistryStorage(m.controllerRegistry, m.podRegistry), "services": registry.MakeServiceRegistryStorage(m.serviceRegistry, cloud, m.minionRegistry), "minions": registry.MakeMinionRegistryStorage(m.minionRegistry), From 969586a21492b84f2a4fc48279abf8b96820bbf6 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Tue, 1 Jul 2014 17:24:17 -0700 Subject: [PATCH 6/6] Add logging, fix crash Crash was in kublet_server when fake docker client gives it nil pointer. --- cmd/integration/integration.go | 2 +- pkg/kubelet/kubelet.go | 7 ++++++- pkg/kubelet/kubelet_server.go | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 3368087270b..e6e9f183bed 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -173,7 +173,7 @@ func main() { if len(createdPods) != 7 { glog.Fatalf("Unexpected list of created pods:\n\n%#v\n\n%#v\n\n%#v\n\n", createdPods.List(), fakeDocker1.Created, fakeDocker2.Created) } - glog.Infof("OK") + glog.Infof("OK - found created pods: %#v", createdPods.List()) } // Serve a file for kubelet to read. diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index a668d8f8083..f33585183f1 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -801,7 +801,12 @@ func (kl *Kubelet) GetPodInfo(podID string) (api.PodInfo, error) { if err != nil { return nil, err } - info[containerName] = *inspectResult + if inspectResult == nil { + // Why did we not get an error? + info[containerName] = docker.Container{} + } else { + info[containerName] = *inspectResult + } } return info, nil } diff --git a/pkg/kubelet/kubelet_server.go b/pkg/kubelet/kubelet_server.go index b2ab0b8a362..0883dc61454 100644 --- a/pkg/kubelet/kubelet_server.go +++ b/pkg/kubelet/kubelet_server.go @@ -24,6 +24,7 @@ import ( "net/url" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "gopkg.in/v1/yaml" ) @@ -45,6 +46,10 @@ func (s *KubeletServer) error(w http.ResponseWriter, err error) { } func (s *KubeletServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { + logger := apiserver.MakeLogged(req, w) + w = logger + defer logger.Log() + u, err := url.ParseRequestURI(req.RequestURI) if err != nil { s.error(w, err)