diff --git a/pkg/kubelet/fake_docker_client.go b/pkg/kubelet/fake_docker_client.go index d84496da92..8664ba64e5 100644 --- a/pkg/kubelet/fake_docker_client.go +++ b/pkg/kubelet/fake_docker_client.go @@ -22,7 +22,7 @@ import ( "github.com/fsouza/go-dockerclient" ) -// A simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup. +// FakeDockerClient is a simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup. type FakeDockerClient struct { containerList []docker.APIContainers container *docker.Container @@ -41,16 +41,22 @@ func (f *FakeDockerClient) appendCall(call string) { f.called = append(f.called, call) } +// ListContainers is a test-spy implementation of DockerInterface.ListContainers. +// It adds an entry "list" to the internal method call record. func (f *FakeDockerClient) ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) { f.appendCall("list") return f.containerList, f.err } +// InspectContainer is a test-spy implementation of DockerInterface.InspectContainer. +// It adds an entry "inspect" to the internal method call record. func (f *FakeDockerClient) InspectContainer(id string) (*docker.Container, error) { f.appendCall("inspect") return f.container, f.err } +// CreateContainer is a test-spy implementation of DockerInterface.CreateContainer. +// It adds an entry "create" to the internal method call record. func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*docker.Container, error) { f.appendCall("create") f.Created = append(f.Created, c.Name) @@ -61,11 +67,15 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do return &docker.Container{ID: name}, nil } +// StartContainer is a test-spy implementation of DockerInterface.StartContainer. +// It adds an entry "start" to the internal method call record. func (f *FakeDockerClient) StartContainer(id string, hostConfig *docker.HostConfig) error { f.appendCall("start") return f.err } +// StopContainer is a test-spy implementation of DockerInterface.StopContainer. +// It adds an entry "stop" to the internal method call record. func (f *FakeDockerClient) StopContainer(id string, timeout uint) error { f.appendCall("stop") f.stopped = append(f.stopped, id) @@ -79,12 +89,15 @@ func (f *FakeDockerClient) StopContainer(id string, timeout uint) error { return f.err } +// PullImage is a test-spy implementation of DockerInterface.StopContainer. +// It adds an entry "pull" to the internal method call record. func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error { f.appendCall("pull") f.pulled = append(f.pulled, fmt.Sprintf("%s/%s:%s", opts.Repository, opts.Registry, opts.Tag)) return f.err } +// FakeDockerPuller is a stub implementation of DockerPuller. type FakeDockerPuller struct { ImagesPulled []string @@ -93,7 +106,7 @@ type FakeDockerPuller struct { ErrorsToInject []error } -// Records the image pull attempt, and optionally injects an error. +// Pull records the image pull attempt, and optionally injects an error. func (f *FakeDockerPuller) Pull(image string) error { f.ImagesPulled = append(f.ImagesPulled, image) diff --git a/pkg/kubelet/health_check.go b/pkg/kubelet/health_check.go index 391d976efa..7bf9933edf 100644 --- a/pkg/kubelet/health_check.go +++ b/pkg/kubelet/health_check.go @@ -25,7 +25,9 @@ import ( "github.com/golang/glog" ) +// HealthChecker is an abstract interface for container health checker. type HealthChecker interface { + // IsHealthy checks if the container is healthy. IsHealthy(container api.Container) (bool, error) } @@ -33,6 +35,7 @@ type httpDoInterface interface { Get(string) (*http.Response, error) } +// MakeHealthChecker creates a new HealthChecker. func MakeHealthChecker() HealthChecker { return &MuxHealthChecker{ checkers: map[string]HealthChecker{ @@ -43,10 +46,12 @@ func MakeHealthChecker() HealthChecker { } } +// MuxHealthChecker bundles multiple implementations of HealthChecker of different types. type MuxHealthChecker struct { checkers map[string]HealthChecker } +// IsHealthy checks the health of the container by delegating to an appropriate HealthChecker according to container.LivenessProbe.Type. func (m *MuxHealthChecker) IsHealthy(container api.Container) (bool, error) { checker, ok := m.checkers[container.LivenessProbe.Type] if !ok || checker == nil { @@ -56,6 +61,7 @@ func (m *MuxHealthChecker) IsHealthy(container api.Container) (bool, error) { return checker.IsHealthy(container) } +// HTTPHealthChecker is an implementation of HealthChecker which checks container health by sending HTTP Get requests. type HTTPHealthChecker struct { client httpDoInterface } @@ -70,6 +76,7 @@ func (h *HTTPHealthChecker) findPort(container api.Container, portName string) i return -1 } +// IsHealthy checks if the container is healthy by trying sending HTTP Get requests to the container. func (h *HTTPHealthChecker) IsHealthy(container api.Container) (bool, error) { params := container.LivenessProbe.HTTPGet port := h.findPort(container, params.Port) diff --git a/pkg/kubelet/health_check_test.go b/pkg/kubelet/health_check_test.go index da25b590c2..6aa18dc042 100644 --- a/pkg/kubelet/health_check_test.go +++ b/pkg/kubelet/health_check_test.go @@ -23,19 +23,19 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) -type fakeHttpClient struct { +type fakeHTTPClient struct { req string res http.Response err error } -func (f *fakeHttpClient) Get(url string) (*http.Response, error) { +func (f *fakeHTTPClient) Get(url string) (*http.Response, error) { f.req = url return &f.res, f.err } func TestHttpHealth(t *testing.T) { - fakeClient := fakeHttpClient{ + fakeClient := fakeHTTPClient{ res: http.Response{ StatusCode: http.StatusOK, }, diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index c329f7f903..65f92da43b 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -44,17 +44,14 @@ import ( "gopkg.in/v1/yaml" ) -// State, sub object of the Docker JSON data -type State struct { - Running bool -} - -// The structured representation of the JSON object returned by Docker inspect +// DockerContainerData is the structured representation of the JSON object returned by Docker inspect type DockerContainerData struct { - state State + state struct { + Running bool + } } -// Interface for testability +// DockerInterface is an abstract interface for testability. It abstracts the interface of docker.Client. type DockerInterface interface { ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) InspectContainer(id string) (*docker.Container, error) @@ -64,24 +61,26 @@ type DockerInterface interface { PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error } -// Type to make it clear when we're working with docker container Ids +// DockerID is an ID of docker container. It is a type to make it clear when we're working with docker container Ids type DockerID string -//Interface for testability +// DockerPuller is an abstract interface for testability. It abstracts image pull operations. type DockerPuller interface { Pull(image string) error } +// CadvisorInterface is an abstract interface for testability. It abstracts the interface of "github.com/google/cadvisor/client".Client. type CadvisorInterface interface { ContainerInfo(name string) (*info.ContainerInfo, error) MachineInfo() (*info.MachineInfo, error) } +// New creates a new Kubelet. func New() *Kubelet { return &Kubelet{} } -// The main kubelet implementation +// Kubelet is the main kubelet implementation. type Kubelet struct { Hostname string EtcdClient tools.EtcdClient @@ -107,9 +106,9 @@ const ( httpServerSource = "http_server" ) -// Starts background goroutines. If config_path, manifest_url, or address are empty, +// RunKubelet starts background goroutines. If config_path, manifest_url, or address are empty, // they are not watched. Never returns. -func (kl *Kubelet) RunKubelet(dockerEndpoint, config_path, manifest_url, etcd_servers, address string, port uint) { +func (kl *Kubelet) RunKubelet(dockerEndpoint, configPath, manifestURL, etcdServers, address string, port uint) { if kl.CadvisorClient == nil { var err error kl.CadvisorClient, err = cadvisor.NewClient("http://127.0.0.1:5000") @@ -121,22 +120,22 @@ func (kl *Kubelet) RunKubelet(dockerEndpoint, config_path, manifest_url, etcd_se kl.DockerPuller = kl.MakeDockerPuller() } updateChannel := make(chan manifestUpdate) - if config_path != "" { - glog.Infof("Watching for file configs at %s", config_path) + if configPath != "" { + glog.Infof("Watching for file configs at %s", configPath) go util.Forever(func() { - kl.WatchFiles(config_path, updateChannel) + kl.WatchFiles(configPath, updateChannel) }, kl.FileCheckFrequency) } - if manifest_url != "" { - glog.Infof("Watching for HTTP configs at %s", manifest_url) + if manifestURL != "" { + glog.Infof("Watching for HTTP configs at %s", manifestURL) go util.Forever(func() { - if err := kl.extractFromHTTP(manifest_url, updateChannel); err != nil { + if err := kl.extractFromHTTP(manifestURL, updateChannel); err != nil { glog.Errorf("Error syncing http: %v", err) } }, kl.HTTPCheckFrequency) } - if etcd_servers != "" { - servers := []string{etcd_servers} + if etcdServers != "" { + servers := []string{etcdServers} glog.Infof("Watching for etcd configs at %v", servers) kl.EtcdClient = etcd.NewClient(servers) go util.Forever(func() { kl.SyncAndSetupEtcdWatch(updateChannel) }, 20*time.Second) @@ -160,15 +159,15 @@ func (kl *Kubelet) RunKubelet(dockerEndpoint, config_path, manifest_url, etcd_se kl.syncLoop(updateChannel, kl) } -// Interface implemented by Kubelet, for testability +// SyncHandler is an interface implemented by Kubelet, for testability type SyncHandler interface { SyncManifests([]api.ContainerManifest) error } -// Log an event to the etcd backend. +// LogEvent logs an event to the etcd backend. func (kl *Kubelet) LogEvent(event *api.Event) error { if kl.EtcdClient == nil { - return fmt.Errorf("no etcd client connection.") + return fmt.Errorf("no etcd client connection") } event.Timestamp = time.Now().Unix() data, err := json.Marshal(event) @@ -234,12 +233,14 @@ func (kl *Kubelet) getContainer(ID DockerID) (*docker.APIContainers, error) { return nil, nil } +// MakeDockerPuller creates a new instance of the default implementation of DockerPuller. func (kl *Kubelet) MakeDockerPuller() DockerPuller { return dockerPuller{ client: kl.DockerClient, } } +// dockerPuller is the default implementation of DockerPuller. type dockerPuller struct { client DockerInterface } @@ -304,7 +305,7 @@ func makeEnvironmentVariables(container *api.Container) []string { return result } -func makeVolumesAndBinds(manifestId string, container *api.Container) (map[string]struct{}, []string) { +func makeVolumesAndBinds(manifestID string, container *api.Container) (map[string]struct{}, []string) { volumes := map[string]struct{}{} binds := []string{} for _, volume := range container.VolumeMounts { @@ -314,7 +315,7 @@ func makeVolumesAndBinds(manifestId string, container *api.Container) (map[strin basePath = fmt.Sprintf("%s:%s", volume.MountPath, volume.MountPath) } else { volumes[volume.MountPath] = struct{}{} - basePath = fmt.Sprintf("/exports/%s/%s:%s", manifestId, volume.Name, volume.MountPath) + basePath = fmt.Sprintf("/exports/%s/%s:%s", manifestID, volume.Name, volume.MountPath) } if volume.ReadOnly { basePath += ":ro" @@ -465,12 +466,12 @@ func (kl *Kubelet) extractFromDir(name string) ([]api.ContainerManifest, error) return manifests, nil } -// Watch a file or direcory of files for changes to the set of pods that +// WatchFiles watches a file or direcory of files for changes to the set of pods that // should run on this Kubelet. -func (kl *Kubelet) WatchFiles(config_path string, updateChannel chan<- manifestUpdate) { +func (kl *Kubelet) WatchFiles(configPath string, updateChannel chan<- manifestUpdate) { var err error - statInfo, err := os.Stat(config_path) + statInfo, err := os.Stat(configPath) if err != nil { if !os.IsNotExist(err) { glog.Errorf("Error accessing path: %v", err) @@ -478,14 +479,14 @@ func (kl *Kubelet) WatchFiles(config_path string, updateChannel chan<- manifestU return } if statInfo.Mode().IsDir() { - manifests, err := kl.extractFromDir(config_path) + manifests, err := kl.extractFromDir(configPath) if err != nil { glog.Errorf("Error polling dir: %v", err) return } updateChannel <- manifestUpdate{fileSource, manifests} } else if statInfo.Mode().IsRegular() { - manifest, err := kl.extractFromFile(config_path) + manifest, err := kl.extractFromFile(configPath) if err != nil { glog.Errorf("Error polling file: %v", err) return @@ -548,8 +549,8 @@ func (kl *Kubelet) extractFromHTTP(url string, updateChannel chan<- manifestUpda url, string(data), singleErr, manifest, multiErr, manifests) } -// Take an etcd Response object, and turn it into a structured list of containers -// Return a list of containers, or an error if one occurs. +// ResponseToManifests takes an etcd Response object, and turns it into a structured list of containers. +// It returns a list of containers, or an error if one occurs. func (kl *Kubelet) ResponseToManifests(response *etcd.Response) ([]api.ContainerManifest, error) { if response.Node == nil || len(response.Node.Value) == 0 { return nil, fmt.Errorf("no nodes field: %v", response) @@ -578,7 +579,7 @@ func (kl *Kubelet) getKubeletStateFromEtcd(key string, updateChannel chan<- mani return nil } -// Sync with etcd, and set up an etcd watch for new configurations +// SyncAndSetupEtcdWatch synchronizes with etcd, and sets up an etcd watch for new configurations. // The channel to send new configurations across // This function loops forever and is intended to be run in a go routine. func (kl *Kubelet) SyncAndSetupEtcdWatch(updateChannel chan<- manifestUpdate) { @@ -610,7 +611,7 @@ func (kl *Kubelet) SyncAndSetupEtcdWatch(updateChannel chan<- manifestUpdate) { } } -// Timeout the watch after 30 seconds +// TimeoutWatch timeout the watch after 30 seconds. func (kl *Kubelet) TimeoutWatch(done chan bool) { t := time.Tick(30 * time.Second) for _ = range t { @@ -618,7 +619,7 @@ func (kl *Kubelet) TimeoutWatch(done chan bool) { } } -// Extract data from YAML file into a list of containers. +// ExtractYAMLData extracts data from YAML file into a list of containers. func (kl *Kubelet) ExtractYAMLData(buf []byte, output interface{}) error { err := yaml.Unmarshal(buf, output) if err != nil { @@ -637,7 +638,7 @@ func (kl *Kubelet) extractFromEtcd(response *etcd.Response) ([]api.ContainerMani return manifests, err } -// Watch etcd for changes, receives config objects from the etcd client watch. +// WatchEtcd watches etcd for changes, receives config objects from the etcd client watch. // This function loops until the watchChannel is closed, and is intended to be run as a goroutine. func (kl *Kubelet) WatchEtcd(watchChannel <-chan *etcd.Response, updateChannel chan<- manifestUpdate) { defer util.HandleCrash() @@ -751,7 +752,7 @@ func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, keepChannel cha type empty struct{} -// Sync the configured list of containers (desired state) with the host current state +// SyncManifests synchronizes 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 @@ -873,12 +874,12 @@ func (kl *Kubelet) syncLoop(updateChannel <-chan manifestUpdate, handler SyncHan } } -// getContainerIdFromName looks at the list of containers on the machine and returns the ID of the container whose name +// 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) (DockerID, bool, error) { +func (kl *Kubelet) getContainerIDFromName(name string) (DockerID, bool, error) { containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{}) if err != nil { return "", false, err @@ -891,7 +892,7 @@ func (kl *Kubelet) getContainerIdFromName(name string) (DockerID, bool, error) { return "", false, nil } -// Returns docker info for all containers in the pod/manifest +// GetPodInfo returns docker info for all containers in the pod/manifest. func (kl *Kubelet) GetPodInfo(podID string) (api.PodInfo, error) { info := api.PodInfo{} @@ -970,7 +971,7 @@ func (kl *Kubelet) statsFromContainerPath(containerPath string) (*api.ContainerS return ret, nil } -// Returns stats (from Cadvisor) for a container. +// GetContainerStats returns stats (from Cadvisor) for a container. func (kl *Kubelet) GetContainerStats(podID, containerName string) (*api.ContainerStats, error) { if kl.CadvisorClient == nil { return nil, nil @@ -982,7 +983,7 @@ func (kl *Kubelet) GetContainerStats(podID, containerName string) (*api.Containe return kl.statsFromContainerPath(fmt.Sprintf("/docker/%s", string(dockerID))) } -// Returns stats (from Cadvisor) of current machine. +// GetMachineStats returns stats (from Cadvisor) of current machine. func (kl *Kubelet) GetMachineStats() (*api.ContainerStats, error) { return kl.statsFromContainerPath("/") } diff --git a/pkg/kubelet/kubelet_server.go b/pkg/kubelet/kubelet_server.go index 01cae8498a..5154c7ddc9 100644 --- a/pkg/kubelet/kubelet_server.go +++ b/pkg/kubelet/kubelet_server.go @@ -31,6 +31,7 @@ import ( "gopkg.in/v1/yaml" ) +// KubeletServer is a http.Handler which exposes kubelet functionality over HTTP. type KubeletServer struct { Kubelet kubeletInterface UpdateChannel chan<- manifestUpdate diff --git a/pkg/kubelet/kubelet_server_test.go b/pkg/kubelet/kubelet_server_test.go index 4c550edaf7..bb00612ba9 100644 --- a/pkg/kubelet/kubelet_server_test.go +++ b/pkg/kubelet/kubelet_server_test.go @@ -54,7 +54,7 @@ type serverTestFramework struct { updateReader *channelReader serverUnderTest *KubeletServer fakeKubelet *fakeKubelet - testHttpServer *httptest.Server + testHTTPServer *httptest.Server } func makeServerTest() *serverTestFramework { @@ -67,7 +67,7 @@ func makeServerTest() *serverTestFramework { Kubelet: fw.fakeKubelet, UpdateChannel: fw.updateChan, } - fw.testHttpServer = httptest.NewServer(fw.serverUnderTest) + fw.testHTTPServer = httptest.NewServer(fw.serverUnderTest) return fw } @@ -83,7 +83,7 @@ func TestContainer(t *testing.T) { {ID: "test_manifest"}, } body := bytes.NewBuffer([]byte(util.MakeJSONString(expected[0]))) // Only send a single ContainerManifest - resp, err := http.Post(fw.testHttpServer.URL+"/container", "application/json", body) + resp, err := http.Post(fw.testHTTPServer.URL+"/container", "application/json", body) if err != nil { t.Errorf("Post returned: %v", err) } @@ -105,7 +105,7 @@ func TestContainers(t *testing.T) { {ID: "test_manifest_2"}, } body := bytes.NewBuffer([]byte(util.MakeJSONString(expected))) - resp, err := http.Post(fw.testHttpServer.URL+"/containers", "application/json", body) + resp, err := http.Post(fw.testHTTPServer.URL+"/containers", "application/json", body) if err != nil { t.Errorf("Post returned: %v", err) } @@ -129,7 +129,7 @@ func TestPodInfo(t *testing.T) { } return nil, fmt.Errorf("bad pod") } - resp, err := http.Get(fw.testHttpServer.URL + "/podInfo?podID=goodpod") + resp, err := http.Get(fw.testHTTPServer.URL + "/podInfo?podID=goodpod") if err != nil { t.Errorf("Got error GETing: %v", err) } @@ -170,7 +170,7 @@ func TestContainerStats(t *testing.T) { return expectedStats, nil } - resp, err := http.Get(fw.testHttpServer.URL + fmt.Sprintf("/stats/%v/%v", expectedPodID, expectedContainerName)) + resp, err := http.Get(fw.testHTTPServer.URL + fmt.Sprintf("/stats/%v/%v", expectedPodID, expectedContainerName)) if err != nil { t.Fatalf("Got error GETing: %v", err) } @@ -205,7 +205,7 @@ func TestMachineStats(t *testing.T) { return expectedStats, nil } - resp, err := http.Get(fw.testHttpServer.URL + "/stats") + resp, err := http.Get(fw.testHTTPServer.URL + "/stats") if err != nil { t.Fatalf("Got error GETing: %v", err) } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 0724bf3270..ce07145107 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -913,13 +913,15 @@ type mockCadvisorClient struct { mock.Mock } -func (self *mockCadvisorClient) ContainerInfo(name string) (*info.ContainerInfo, error) { - args := self.Called(name) +// ContainerInfo is a mock implementation of CadvisorInterface.ContainerInfo. +func (c *mockCadvisorClient) ContainerInfo(name string) (*info.ContainerInfo, error) { + args := c.Called(name) return args.Get(0).(*info.ContainerInfo), args.Error(1) } -func (self *mockCadvisorClient) MachineInfo() (*info.MachineInfo, error) { - args := self.Called() +// MachineInfo is a mock implementation of CadvisorInterface.MachineInfo. +func (c *mockCadvisorClient) MachineInfo() (*info.MachineInfo, error) { + args := c.Called() return args.Get(0).(*info.MachineInfo), args.Error(1) }