Merge pull request #340 from thockin/cleanups

nit: s/Id/ID/ for go style
This commit is contained in:
brendandburns 2014-07-02 13:10:13 -07:00
commit 94e1d26992
2 changed files with 47 additions and 47 deletions

View File

@ -62,7 +62,7 @@ type DockerInterface interface {
} }
// Type to make it clear when we're working with docker container Ids // Type to make it clear when we're working with docker container Ids
type DockerId string type DockerID string
//Interface for testability //Interface for testability
type DockerPuller interface { type DockerPuller interface {
@ -177,8 +177,8 @@ func (kl *Kubelet) LogEvent(event *api.Event) error {
} }
// Return a map of docker containers that we manage. The map key is the docker container ID // Return a map of docker containers that we manage. The map key is the docker container ID
func (kl *Kubelet) getDockerContainers() (map[DockerId]docker.APIContainers, error) { func (kl *Kubelet) getDockerContainers() (map[DockerID]docker.APIContainers, error) {
result := map[DockerId]docker.APIContainers{} result := map[DockerID]docker.APIContainers{}
containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{}) containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{})
if err != nil { if err != nil {
return result, err return result, err
@ -189,21 +189,21 @@ func (kl *Kubelet) getDockerContainers() (map[DockerId]docker.APIContainers, err
if !strings.HasPrefix(value.Names[0], "/"+containerNamePrefix+"--") { if !strings.HasPrefix(value.Names[0], "/"+containerNamePrefix+"--") {
continue continue
} }
result[DockerId(value.ID)] = value result[DockerID(value.ID)] = value
} }
return result, err return result, err
} }
// Return Docker's container ID for a manifest's container. Returns an empty string if it doesn't exist. // 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) (DockerId, error) { func (kl *Kubelet) getContainerID(manifest *api.ContainerManifest, container *api.Container) (DockerID, error) {
dockerContainers, err := kl.getDockerContainers() dockerContainers, err := kl.getDockerContainers()
if err != nil { if err != nil {
return "", err return "", err
} }
for id, dockerContainer := range dockerContainers { for id, dockerContainer := range dockerContainers {
manifestId, containerName := parseDockerName(dockerContainer.Names[0]) manifestID, containerName := parseDockerName(dockerContainer.Names[0])
if manifestId == manifest.ID && containerName == container.Name { if manifestID == manifest.ID && containerName == container.Name {
return DockerId(id), nil return DockerID(id), nil
} }
} }
return "", nil return "", nil
@ -257,7 +257,7 @@ func buildDockerName(manifest *api.ContainerManifest, container *api.Container)
// Upacks a container name, returning the manifest id and container name we would have used to // Upacks a container name, returning the manifest id and container name we would have used to
// construct the docker name. If the docker name isn't one we created, we may return empty strings. // construct the docker name. If the docker name isn't one we created, we may return empty strings.
func parseDockerName(name string) (manifestId, containerName string) { func parseDockerName(name string) (manifestID, containerName string) {
// For some reason docker appears to be appending '/' to names. // For some reason docker appears to be appending '/' to names.
// If its there, strip it. // If its there, strip it.
if name[0] == '/' { if name[0] == '/' {
@ -271,7 +271,7 @@ func parseDockerName(name string) (manifestId, containerName string) {
containerName = unescapeDash(parts[1]) containerName = unescapeDash(parts[1])
} }
if len(parts) > 2 { if len(parts) > 2 {
manifestId = unescapeDash(parts[2]) manifestID = unescapeDash(parts[2])
} }
return return
} }
@ -336,7 +336,7 @@ func makePortsAndBindings(container *api.Container) (map[docker.Port]struct{}, m
} }
// Run a single container from a manifest. Returns the docker container ID // 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 DockerId, err error) { func (kl *Kubelet) runContainer(manifest *api.ContainerManifest, container *api.Container, netMode string) (id DockerID, err error) {
envVariables := makeEnvironmentVariables(container) envVariables := makeEnvironmentVariables(container)
volumes, binds := makeVolumesAndBinds(container) volumes, binds := makeVolumesAndBinds(container)
exposedPorts, portBindings := makePortsAndBindings(container) exposedPorts, portBindings := makePortsAndBindings(container)
@ -361,17 +361,17 @@ func (kl *Kubelet) runContainer(manifest *api.ContainerManifest, container *api.
Binds: binds, Binds: binds,
NetworkMode: netMode, NetworkMode: netMode,
}) })
return DockerId(dockerContainer.ID), err return DockerID(dockerContainer.ID), err
} }
// Kill a docker container // Kill a docker container
func (kl *Kubelet) killContainer(container docker.APIContainers) error { func (kl *Kubelet) killContainer(container docker.APIContainers) error {
err := kl.DockerClient.StopContainer(container.ID, 10) err := kl.DockerClient.StopContainer(container.ID, 10)
manifestId, containerName := parseDockerName(container.Names[0]) manifestID, containerName := parseDockerName(container.Names[0])
kl.LogEvent(&api.Event{ kl.LogEvent(&api.Event{
Event: "STOP", Event: "STOP",
Manifest: &api.ContainerManifest{ Manifest: &api.ContainerManifest{
ID: manifestId, ID: manifestID,
}, },
Container: &api.Container{ Container: &api.Container{
Name: containerName, Name: containerName,
@ -621,12 +621,12 @@ func (kl *Kubelet) WatchEtcd(watchChannel <-chan *etcd.Response, updateChannel c
const networkContainerName = "net" const networkContainerName = "net"
// Return the docker ID for a manifest's network container. Returns an empty string if it doesn't exist. // 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) (DockerId, error) { func (kl *Kubelet) getNetworkContainerID(manifest *api.ContainerManifest) (DockerID, error) {
return kl.getContainerId(manifest, &api.Container{Name: networkContainerName}) 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. // Create a network container for a manifest. Returns the docker container ID of the newly created container.
func (kl *Kubelet) createNetworkContainer(manifest *api.ContainerManifest) (DockerId, error) { func (kl *Kubelet) createNetworkContainer(manifest *api.ContainerManifest) (DockerID, error) {
var ports []api.Port var ports []api.Port
// Docker only exports ports from the network container. Let's // Docker only exports ports from the network container. Let's
// collect all of the relevant ports and export them. // collect all of the relevant ports and export them.
@ -643,45 +643,45 @@ func (kl *Kubelet) createNetworkContainer(manifest *api.ContainerManifest) (Dock
return kl.runContainer(manifest, container, "") return kl.runContainer(manifest, container, "")
} }
func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, keepChannel chan<- DockerId) error { func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, keepChannel chan<- DockerID) error {
// Make sure we have a network container // Make sure we have a network container
netId, err := kl.getNetworkContainerId(manifest) netID, err := kl.getNetworkContainerID(manifest)
if err != nil { if err != nil {
glog.Errorf("Failed to introspect network container. (%v) Skipping manifest %s", err, manifest.ID) glog.Errorf("Failed to introspect network container. (%v) Skipping manifest %s", err, manifest.ID)
return err return err
} }
if netId == "" { if netID == "" {
glog.Infof("Network container doesn't exist, creating") glog.Infof("Network container doesn't exist, creating")
netId, err = kl.createNetworkContainer(manifest) netID, err = kl.createNetworkContainer(manifest)
if err != nil { if err != nil {
glog.Errorf("Failed to introspect network container. (%v) Skipping manifest %s", err, manifest.ID) glog.Errorf("Failed to introspect network container. (%v) Skipping manifest %s", err, manifest.ID)
return err return err
} }
} }
keepChannel <- netId keepChannel <- netID
for _, container := range manifest.Containers { for _, container := range manifest.Containers {
containerId, err := kl.getContainerId(manifest, &container) containerID, err := kl.getContainerID(manifest, &container)
if err != nil { if err != nil {
glog.Errorf("Error finding container: %v skipping manifest %s container %s.", err, manifest.ID, container.Name) glog.Errorf("Error finding container: %v skipping manifest %s container %s.", err, manifest.ID, container.Name)
continue continue
} }
if containerId == "" { if containerID == "" {
glog.Infof("%+v doesn't exist, creating", container) glog.Infof("%+v doesn't exist, creating", container)
kl.DockerPuller.Pull(container.Image) kl.DockerPuller.Pull(container.Image)
if err != nil { if err != nil {
glog.Errorf("Failed to create container: %v skipping manifest %s container %s.", err, manifest.ID, container.Name) glog.Errorf("Failed to create container: %v skipping manifest %s container %s.", err, manifest.ID, container.Name)
continue continue
} }
containerId, err = kl.runContainer(manifest, &container, "container:"+string(netId)) containerID, err = kl.runContainer(manifest, &container, "container:"+string(netID))
if err != nil { if err != nil {
// TODO(bburns) : Perhaps blacklist a container after N failures? // TODO(bburns) : Perhaps blacklist a container after N failures?
glog.Errorf("Error running manifest %s container %s: %v", manifest.ID, container.Name, err) glog.Errorf("Error running manifest %s container %s: %v", manifest.ID, container.Name, err)
continue continue
} }
} else { } else {
glog.V(1).Infof("manifest %s container %s exists as %v", manifest.ID, container.Name, containerId) glog.V(1).Infof("manifest %s container %s exists as %v", manifest.ID, container.Name, containerID)
} }
keepChannel <- containerId keepChannel <- containerID
} }
return nil return nil
} }
@ -690,8 +690,8 @@ func (kl *Kubelet) syncManifest(manifest *api.ContainerManifest, keepChannel cha
func (kl *Kubelet) SyncManifests(config []api.ContainerManifest) error { func (kl *Kubelet) SyncManifests(config []api.ContainerManifest) error {
glog.Infof("Desired: %+v", config) glog.Infof("Desired: %+v", config)
var err error var err error
dockerIdsToKeep := map[DockerId]bool{} dockerIdsToKeep := map[DockerID]bool{}
keepChannel := make(chan DockerId) keepChannel := make(chan DockerID)
waitGroup := sync.WaitGroup{} waitGroup := sync.WaitGroup{}
// Check for any containers that need starting // Check for any containers that need starting
@ -774,14 +774,14 @@ func (kl *Kubelet) syncLoop(updateChannel <-chan manifestUpdate, handler SyncHan
// it returns true if the container is found, false otherwise, and any error that occurs. // it returns true if the container is found, false otherwise, and any error that occurs.
// TODO: This functions exists to support GetContainerInfo and GetContainerStats // TODO: This functions exists to support GetContainerInfo and GetContainerStats
// It should be removed once those two functions start taking proper pod.IDs // 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{}) containerList, err := kl.DockerClient.ListContainers(docker.ListContainersOptions{})
if err != nil { if err != nil {
return "", false, err return "", false, err
} }
for _, value := range containerList { for _, value := range containerList {
if strings.Contains(value.Names[0], name) { if strings.Contains(value.Names[0], name) {
return DockerId(value.ID), true, nil return DockerID(value.ID), true, nil
} }
} }
return "", false, nil return "", false, nil
@ -820,12 +820,12 @@ func (kl *Kubelet) GetContainerStats(name string) (*api.ContainerStats, error) {
if kl.CadvisorClient == nil { if kl.CadvisorClient == nil {
return nil, nil return nil, nil
} }
dockerId, found, err := kl.getContainerIdFromName(name) dockerID, found, err := kl.getContainerIdFromName(name)
if err != nil || !found { if err != nil || !found {
return nil, err return nil, err
} }
info, err := kl.CadvisorClient.ContainerInfo(fmt.Sprintf("/docker/%s", string(dockerId))) info, err := kl.CadvisorClient.ContainerInfo(fmt.Sprintf("/docker/%s", string(dockerID)))
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -115,14 +115,14 @@ func verifyStringArrayEquals(t *testing.T, actual, expected []string) {
} }
} }
func verifyPackUnpack(t *testing.T, manifestId, containerName string) { func verifyPackUnpack(t *testing.T, manifestID, containerName string) {
name := buildDockerName( name := buildDockerName(
&api.ContainerManifest{ID: manifestId}, &api.ContainerManifest{ID: manifestID},
&api.Container{Name: containerName}, &api.Container{Name: containerName},
) )
returnedManifestId, returnedContainerName := parseDockerName(name) returnedManifestID, returnedContainerName := parseDockerName(name)
if manifestId != returnedManifestId || containerName != returnedContainerName { if manifestID != returnedManifestID || containerName != returnedContainerName {
t.Errorf("For (%s, %s), unpacked (%s, %s)", manifestId, containerName, returnedManifestId, returnedContainerName) t.Errorf("For (%s, %s), unpacked (%s, %s)", manifestID, containerName, returnedManifestID, returnedContainerName)
} }
} }
@ -140,7 +140,7 @@ func TestContainerManifestNaming(t *testing.T) {
verifyPackUnpack(t, "_m___anifest", "-_-container") verifyPackUnpack(t, "_m___anifest", "-_-container")
} }
func TestGetContainerId(t *testing.T) { func TestGetContainerID(t *testing.T) {
kubelet, _, fakeDocker := makeTestKubelet(t) kubelet, _, fakeDocker := makeTestKubelet(t)
manifest := api.ContainerManifest{ manifest := api.ContainerManifest{
ID: "qux", ID: "qux",
@ -162,7 +162,7 @@ func TestGetContainerId(t *testing.T) {
ID: "foobar", ID: "foobar",
} }
id, err := kubelet.getContainerId(&manifest, &container) id, err := kubelet.getContainerID(&manifest, &container)
verifyCalls(t, fakeDocker, []string{"list"}) verifyCalls(t, fakeDocker, []string{"list"})
if id == "" { if id == "" {
t.Errorf("Failed to find container %#v", container) t.Errorf("Failed to find container %#v", container)
@ -173,7 +173,7 @@ func TestGetContainerId(t *testing.T) {
fakeDocker.clearCalls() fakeDocker.clearCalls()
missingManifest := api.ContainerManifest{ID: "foobar"} missingManifest := api.ContainerManifest{ID: "foobar"}
id, err = kubelet.getContainerId(&missingManifest, &container) id, err = kubelet.getContainerID(&missingManifest, &container)
verifyCalls(t, fakeDocker, []string{"list"}) verifyCalls(t, fakeDocker, []string{"list"})
if id != "" { if id != "" {
t.Errorf("Failed to not find container %#v", missingManifest) t.Errorf("Failed to not find container %#v", missingManifest)
@ -864,8 +864,8 @@ func areSamePercentiles(
} }
func TestGetContainerStats(t *testing.T) { func TestGetContainerStats(t *testing.T) {
containerId := "ab2cdf" containerID := "ab2cdf"
containerPath := fmt.Sprintf("/docker/%v", containerId) containerPath := fmt.Sprintf("/docker/%v", containerID)
containerInfo := &info.ContainerInfo{ containerInfo := &info.ContainerInfo{
ContainerReference: info.ContainerReference{ ContainerReference: info.ContainerReference{
Name: containerPath, Name: containerPath,
@ -893,7 +893,7 @@ func TestGetContainerStats(t *testing.T) {
fakeDocker.containerList = []docker.APIContainers{ fakeDocker.containerList = []docker.APIContainers{
{ {
Names: []string{"foo"}, Names: []string{"foo"},
ID: containerId, ID: containerID,
}, },
} }
@ -934,8 +934,8 @@ func TestGetContainerStatsWithoutCadvisor(t *testing.T) {
} }
func TestGetContainerStatsWhenCadvisorFailed(t *testing.T) { func TestGetContainerStatsWhenCadvisorFailed(t *testing.T) {
containerId := "ab2cdf" containerID := "ab2cdf"
containerPath := fmt.Sprintf("/docker/%v", containerId) containerPath := fmt.Sprintf("/docker/%v", containerID)
containerInfo := &info.ContainerInfo{} containerInfo := &info.ContainerInfo{}
mockCadvisor := &mockCadvisorClient{} mockCadvisor := &mockCadvisorClient{}
@ -947,7 +947,7 @@ func TestGetContainerStatsWhenCadvisorFailed(t *testing.T) {
fakeDocker.containerList = []docker.APIContainers{ fakeDocker.containerList = []docker.APIContainers{
{ {
Names: []string{"foo"}, Names: []string{"foo"},
ID: containerId, ID: containerID,
}, },
} }