Add control over container entrypoint

contrib/for-tests/entrypoint-tester/Dockerfile

@@ -0,0 +1,20 @@
+# Copyright 2015 Google Inc. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+FROM scratch
+ADD ep ep
+ADD ep ep-2
+EXPOSE 8080
+ENTRYPOINT ["/ep"]
+CMD ["default", "arguments"]

contrib/for-tests/entrypoint-tester/Makefile

@@ -0,0 +1,15 @@
+all: push
+
+TAG = 0.1
+
+ep: ep.go
+	CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -ldflags '-w' ./ep.go
+
+image: ep
+	sudo docker build -t kubernetes/eptest:$(TAG) .
+
+push: image
+	sudo docker push kubernetes/eptest:$(TAG)
+
+clean:
+	rm -f ep

contrib/for-tests/entrypoint-tester/ep.go

@@ -0,0 +1,29 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"fmt"
+	"os"
+)
+
+// This program prints the arguments it's passed and exits.
+func main() {
+	args := os.Args
+	fmt.Printf("%v\n", args)
+	os.Exit(0)
+}

docs/containers.md

@@ -1,4 +1,46 @@
-# Container with Kubernetes
+# Containers with Kubernetes
+
+## Containers and commands
+
+So far the Pods we've seen have all used the `image` field to indicate what process Kubernetes
+should run in a container.  In this case, Kubernetes runs the image's default command.  If we want
+to run a particular command or override the image's defaults, there are two additional fields that
+we can use:
+
+1.  `Command`: Controls the actual command run by the image
+2.  `Args`: Controls the arguments passed to the command
+
+### How docker handles command and arguments
+
+Docker images have metadata associated with them that is used to store information about the image.
+The image author may use this to define defaults for the command and arguments to run a container
+when the user does not supply values.  Docker calls the fields for commands and arguments
+`Entrypoint` and `Cmd` respectively.  The full details for this feature are too complicated to 
+describe here, mostly due to the fact that the docker API allows users to specify both of these
+fields as either a string array or a string and there are subtle differences in how those cases are
+handled.  We encourage the curious to check out [docker's documentation]() for this feature.
+
+Kubernetes allows you to override both the image's default command (docker `Entrypoint`) and args
+(docker `Cmd`) with the `Command` and `Args` fields of `Container`.  The rules are:
+
+1.  If you do not supply a `Command` or `Args` for a container, the defaults defined by the image
+    will be used
+2.  If you supply a `Command` but no `Args` for a container, only the supplied `Command` will be
+    used; the image's default arguments are ignored
+3.  If you supply only `Args`, the image's default command will be used with the arguments you
+    supply
+4.  If you supply a `Command` **and** `Args`, the image's defaults will be ignored and the values
+    you supply will be used
+
+Here are examples for these rules in table format
+
+| Image `Entrypoint` |    Image `Cmd`   | Container `Command` |  Container `Args`  |    Command Run   |
+|--------------------|------------------|---------------------|--------------------|------------------|
+|     `[/ep-1]`      |   `[foo bar]`    |   <not set>   |   <not set>  | `[ep-1 foo bar]` | 
+|     `[/ep-1]`      |   `[foo bar]`    |      `[/ep-2]`      |   <not set>  |     `[ep-2]`     | 
+|     `[/ep-1]`      |   `[foo bar]`    |   <not set>   |     `[zoo boo]`    | `[ep-1 zoo boo]` | 
+|     `[/ep-1]`      |   `[foo bar]`    |      `[/ep-2]`      |     `[zoo boo]`    | `[ep-2 zoo boo]` | 
+
 
 ## Capabilities
 

pkg/api/types.go

@@ -505,8 +505,10 @@ type Container struct {
 	Name string `json:"name"`
 	// Required.
 	Image string `json:"image"`
-	// Optional: Defaults to whatever is defined in the image.
+	// Optional: The docker image's entrypoint is used if this is not provided; cannot be updated.
 	Command []string `json:"command,omitempty"`
+	// Optional: The docker image's cmd is used if this is not provided; cannot be updated.
+	Args []string `json:"args,omitempty"`
 	// Optional: Defaults to Docker's default.
 	WorkingDir string          `json:"workingDir,omitempty"`
 	Ports      []ContainerPort `json:"ports,omitempty"`

pkg/api/v1beta1/conversion.go

@@ -532,7 +532,10 @@ func init() {
 			if err := s.Convert(&in.Image, &out.Image, 0); err != nil {
 				return err
 			}
-			if err := s.Convert(&in.Command, &out.Command, 0); err != nil {
+			if err := s.Convert(&in.Command, &out.Entrypoint, 0); err != nil {
+				return err
+			}
+			if err := s.Convert(&in.Args, &out.Command, 0); err != nil {
 				return err
 			}
 			if err := s.Convert(&in.WorkingDir, &out.WorkingDir, 0); err != nil {
@@ -615,7 +618,10 @@ func init() {
 			if err := s.Convert(&in.Image, &out.Image, 0); err != nil {
 				return err
 			}
-			if err := s.Convert(&in.Command, &out.Command, 0); err != nil {
+			if err := s.Convert(&in.Command, &out.Args, 0); err != nil {
+				return err
+			}
+			if err := s.Convert(&in.Entrypoint, &out.Command, 0); err != nil {
 				return err
 			}
 			if err := s.Convert(&in.WorkingDir, &out.WorkingDir, 0); err != nil {

pkg/api/v1beta1/types.go

@@ -411,9 +411,11 @@ type Container struct {
 	Name string `json:"name" description:"name of the container; must be a DNS_LABEL and unique within the pod; cannot be updated"`
 	// Required.
 	Image string `json:"image" description:"Docker image name"`
-	// Optional: Defaults to whatever is defined in the image.
+	// Optional: The image's entrypoint is used if this is not provided; cannot be updated.
-	Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; defaults to entrypoint or command in the image; cannot be updated"`
+	Entrypoint []string `json:"entrypoint:omitempty" description:"entrypoint array; not executed within a shell; the image's entrypoint is used if this is not provided; cannot be updated"`
-	// Optional: Defaults to Docker's default.
+	// Optional: The image's cmd is used if this is not provided; cannot be updated.
+	Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; the image's cmd is used if this is not provided; cannot be updated"`
+	// Optional: Docker's default is used if this is not provided.
 	WorkingDir string               `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"`
 	Ports      []ContainerPort      `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated"`
 	Env        []EnvVar             `json:"env,omitempty" description:"list of environment variables to set in the container; cannot be updated"`

pkg/api/v1beta2/conversion.go

@@ -317,13 +317,15 @@ func init() {
 			if err := s.Convert(&in.Image, &out.Image, 0); err != nil {
 				return err
 			}
-			if err := s.Convert(&in.Command, &out.Command, 0); err != nil {
+			if err := s.Convert(&in.Command, &out.Entrypoint, 0); err != nil {
+				return err
+			}
+			if err := s.Convert(&in.Args, &out.Command, 0); err != nil {
 				return err
 			}
 			if err := s.Convert(&in.WorkingDir, &out.WorkingDir, 0); err != nil {
 				return err
 			}
-
 			if err := s.Convert(&in.Ports, &out.Ports, 0); err != nil {
 				return err
 			}
@@ -403,7 +405,10 @@ func init() {
 			if err := s.Convert(&in.Image, &out.Image, 0); err != nil {
 				return err
 			}
-			if err := s.Convert(&in.Command, &out.Command, 0); err != nil {
+			if err := s.Convert(&in.Command, &out.Args, 0); err != nil {
+				return err
+			}
+			if err := s.Convert(&in.Entrypoint, &out.Command, 0); err != nil {
 				return err
 			}
 			if err := s.Convert(&in.WorkingDir, &out.WorkingDir, 0); err != nil {

pkg/api/v1beta2/types.go

@@ -393,9 +393,11 @@ type Container struct {
 	Name string `json:"name" description:"name of the container; must be a DNS_LABEL and unique within the pod; cannot be updated"`
 	// Required.
 	Image string `json:"image" description:"Docker image name"`
-	// Optional: Defaults to whatever is defined in the image.
+	// Optional: The image's entrypoint is used if this is not provided; cannot be updated.
-	Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; defaults to entrypoint or command in the image; cannot be updated"`
+	Entrypoint []string `json:"entrypoint:omitempty" description:"entrypoint array; not executed within a shell; the image's entrypoint is used if this is not provided; cannot be updated"`
-	// Optional: Defaults to Docker's default.
+	// Optional: The image's cmd is used if this is not provided; cannot be updated.
+	Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; the image's cmd is used if this is not provided; cannot be updated"`
+	// Optional: Docker's default is used if this is not provided.
 	WorkingDir string               `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"`
 	Ports      []ContainerPort      `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated"`
 	Env        []EnvVar             `json:"env,omitempty" description:"list of environment variables to set in the container; cannot be updated"`

pkg/api/v1beta3/types.go

@@ -520,8 +520,10 @@ type Container struct {
 	Name string `json:"name" description:"name of the container; must be a DNS_LABEL and unique within the pod; cannot be updated"`
 	// Required.
 	Image string `json:"image" description:"Docker image name"`
-	// Optional: Defaults to whatever is defined in the image.
+	// Optional: The docker image's entrypoint is used if this is not provided; cannot be updated.
-	Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; defaults to entrypoint or command in the image; cannot be updated"`
+	Command []string `json:"command,omitempty" description:"entrypoint array; not executed within a shell; the docker image's entrypoint is used if this is not provided; cannot be updated"`
+	// Optional: The docker image's cmd is used if this is not provided; cannot be updated.
+	Args []string `json:"args,omitempty" description:"command array; the docker image's cmd is used if this is not provided; arguments to the entrypoint; cannot be updated"`
 	// Optional: Defaults to Docker's default.
 	WorkingDir     string               `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"`
 	Ports          []ContainerPort      `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated"`

pkg/kubelet/container/container_reference_manager.go

@@ -17,7 +17,6 @@ limitations under the License.
 package container
 
 import (
-	"fmt"
 	"sync"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -62,41 +61,3 @@ func (c *RefManager) GetRef(id string) (ref *api.ObjectReference, ok bool) {
 	ref, ok = c.containerIDToRef[id]
 	return ref, ok
 }
-
-// fieldPath returns a fieldPath locating container within pod.
-// Returns an error if the container isn't part of the pod.
-func fieldPath(pod *api.Pod, container *api.Container) (string, error) {
-	for i := range pod.Spec.Containers {
-		here := &pod.Spec.Containers[i]
-		if here.Name == container.Name {
-			if here.Name == "" {
-				return fmt.Sprintf("spec.containers[%d]", i), nil
-			} else {
-				return fmt.Sprintf("spec.containers{%s}", here.Name), nil
-			}
-		}
-	}
-	return "", fmt.Errorf("container %#v not found in pod %#v", container, pod)
-}
-
-// GenerateContainerRef returns an *api.ObjectReference which references the given container within the
-// given pod. Returns an error if the reference can't be constructed or the container doesn't
-// actually belong to the pod.
-// TODO: Pods that came to us by static config or over HTTP have no selfLink set, which makes
-// this fail and log an error. Figure out how we want to identify these pods to the rest of the
-// system.
-// TODO(yifan): Revisit this function later, for current case it does not need to use RefManager
-// as a receiver.
-func (c *RefManager) GenerateContainerRef(pod *api.Pod, container *api.Container) (*api.ObjectReference, error) {
-	fieldPath, err := fieldPath(pod, container)
-	if err != nil {
-		// TODO: figure out intelligent way to refer to containers that we implicitly
-		// start (like the pod infra container). This is not a good way, ugh.
-		fieldPath = "implicitly required container " + container.Name
-	}
-	ref, err := api.GetPartialReference(pod, fieldPath)
-	if err != nil {
-		return nil, err
-	}
-	return ref, nil
-}

pkg/kubelet/container/container_reference_manager_test.go

@@ -1,61 +0,0 @@
-/*
-Copyright 2015 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package container
-
-import (
-	"testing"
-
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-)
-
-func TestFieldPath(t *testing.T) {
-	pod := &api.Pod{Spec: api.PodSpec{Containers: []api.Container{
-		{Name: "foo"},
-		{Name: "bar"},
-		{Name: ""},
-		{Name: "baz"},
-	}}}
-	table := map[string]struct {
-		pod       *api.Pod
-		container *api.Container
-		path      string
-		success   bool
-	}{
-		"basic":            {pod, &api.Container{Name: "foo"}, "spec.containers{foo}", true},
-		"basic2":           {pod, &api.Container{Name: "baz"}, "spec.containers{baz}", true},
-		"emptyName":        {pod, &api.Container{Name: ""}, "spec.containers[2]", true},
-		"basicSamePointer": {pod, &pod.Spec.Containers[0], "spec.containers{foo}", true},
-		"missing":          {pod, &api.Container{Name: "qux"}, "", false},
-	}
-
-	for name, item := range table {
-		res, err := fieldPath(item.pod, item.container)
-		if item.success == false {
-			if err == nil {
-				t.Errorf("%v: unexpected non-error", name)
-			}
-			continue
-		}
-		if err != nil {
-			t.Errorf("%v: unexpected error: %v", name, err)
-			continue
-		}
-		if e, a := item.path, res; e != a {
-			t.Errorf("%v: wanted %v, got %v", name, e, a)
-		}
-	}
-}

pkg/kubelet/container/ref.go

@@ -0,0 +1,59 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package container
+
+import (
+	"fmt"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+)
+
+// GenerateContainerRef returns an *api.ObjectReference which references the given container
+// within the given pod. Returns an error if the reference can't be constructed or the
+// container doesn't actually belong to the pod.
+//
+// This function will return an error if the provided Pod does not have a selfLink,
+// but we expect selfLink to be populated at all call sites for the function.
+func GenerateContainerRef(pod *api.Pod, container *api.Container) (*api.ObjectReference, error) {
+	fieldPath, err := fieldPath(pod, container)
+	if err != nil {
+		// TODO: figure out intelligent way to refer to containers that we implicitly
+		// start (like the pod infra container). This is not a good way, ugh.
+		fieldPath = "implicitly required container " + container.Name
+	}
+	ref, err := api.GetPartialReference(pod, fieldPath)
+	if err != nil {
+		return nil, err
+	}
+	return ref, nil
+}
+
+// fieldPath returns a fieldPath locating container within pod.
+// Returns an error if the container isn't part of the pod.
+func fieldPath(pod *api.Pod, container *api.Container) (string, error) {
+	for i := range pod.Spec.Containers {
+		here := &pod.Spec.Containers[i]
+		if here.Name == container.Name {
+			if here.Name == "" {
+				return fmt.Sprintf("spec.containers[%d]", i), nil
+			} else {
+				return fmt.Sprintf("spec.containers{%s}", here.Name), nil
+			}
+		}
+	}
+	return "", fmt.Errorf("container %#v not found in pod %#v", container, pod)
+}

pkg/kubelet/container/ref_test.go

@@ -0,0 +1,208 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package container
+
+import (
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+)
+
+func TestFieldPath(t *testing.T) {
+	pod := &api.Pod{Spec: api.PodSpec{Containers: []api.Container{
+		{Name: "foo"},
+		{Name: "bar"},
+		{Name: ""},
+		{Name: "baz"},
+	}}}
+	table := map[string]struct {
+		pod       *api.Pod
+		container *api.Container
+		path      string
+		success   bool
+	}{
+		"basic":            {pod, &api.Container{Name: "foo"}, "spec.containers{foo}", true},
+		"basic2":           {pod, &api.Container{Name: "baz"}, "spec.containers{baz}", true},
+		"emptyName":        {pod, &api.Container{Name: ""}, "spec.containers[2]", true},
+		"basicSamePointer": {pod, &pod.Spec.Containers[0], "spec.containers{foo}", true},
+		"missing":          {pod, &api.Container{Name: "qux"}, "", false},
+	}
+
+	for name, item := range table {
+		res, err := fieldPath(item.pod, item.container)
+		if item.success == false {
+			if err == nil {
+				t.Errorf("%v: unexpected non-error", name)
+			}
+			continue
+		}
+		if err != nil {
+			t.Errorf("%v: unexpected error: %v", name, err)
+			continue
+		}
+		if e, a := item.path, res; e != a {
+			t.Errorf("%v: wanted %v, got %v", name, e, a)
+		}
+	}
+}
+
+func TestGenerateContainerRef(t *testing.T) {
+	var (
+		okPod = api.Pod{
+			TypeMeta: api.TypeMeta{
+				Kind:       "Pod",
+				APIVersion: "v1beta1",
+			},
+			ObjectMeta: api.ObjectMeta{
+				Name:            "ok",
+				Namespace:       "test-ns",
+				UID:             "bar",
+				ResourceVersion: "42",
+				SelfLink:        "/api/v1beta1/pods/foo",
+			},
+			Spec: api.PodSpec{
+				Containers: []api.Container{
+					{
+						Name: "by-name",
+					},
+					{},
+				},
+			},
+		}
+		noSelfLinkPod        = okPod
+		defaultedSelfLinkPod = okPod
+	)
+	noSelfLinkPod.ObjectMeta.SelfLink = ""
+	defaultedSelfLinkPod.ObjectMeta.SelfLink = "/api/v1beta1/pods/ok"
+
+	cases := []struct {
+		name      string
+		pod       *api.Pod
+		container *api.Container
+		expected  *api.ObjectReference
+		success   bool
+	}{
+		{
+			name: "by-name",
+			pod:  &okPod,
+			container: &api.Container{
+				Name: "by-name",
+			},
+			expected: &api.ObjectReference{
+				Kind:            "Pod",
+				APIVersion:      "v1beta1",
+				Name:            "ok",
+				Namespace:       "test-ns",
+				UID:             "bar",
+				ResourceVersion: "42",
+				FieldPath:       ".spec.containers{by-name}",
+			},
+			success: true,
+		},
+		{
+			name:      "no-name",
+			pod:       &okPod,
+			container: &api.Container{},
+			expected: &api.ObjectReference{
+				Kind:            "Pod",
+				APIVersion:      "v1beta1",
+				Name:            "ok",
+				Namespace:       "test-ns",
+				UID:             "bar",
+				ResourceVersion: "42",
+				FieldPath:       ".spec.containers[1]",
+			},
+			success: true,
+		},
+		{
+			name:      "no-selflink",
+			pod:       &noSelfLinkPod,
+			container: &api.Container{},
+			expected:  nil,
+			success:   false,
+		},
+		{
+			name: "defaulted-selflink",
+			pod:  &defaultedSelfLinkPod,
+			container: &api.Container{
+				Name: "by-name",
+			},
+			expected: &api.ObjectReference{
+				Kind:            "Pod",
+				APIVersion:      "v1beta1",
+				Name:            "ok",
+				Namespace:       "test-ns",
+				UID:             "bar",
+				ResourceVersion: "42",
+				FieldPath:       ".spec.containers{by-name}",
+			},
+			success: true,
+		},
+		{
+			name: "implicitly-required",
+			pod:  &okPod,
+			container: &api.Container{
+				Name: "net",
+			},
+			expected: &api.ObjectReference{
+				Kind:            "Pod",
+				APIVersion:      "v1beta1",
+				Name:            "ok",
+				Namespace:       "test-ns",
+				UID:             "bar",
+				ResourceVersion: "42",
+				FieldPath:       "implicitly required container net",
+			},
+			success: true,
+		},
+	}
+
+	for _, tc := range cases {
+		actual, err := GenerateContainerRef(tc.pod, tc.container)
+		if err != nil {
+			if tc.success {
+				t.Errorf("%v: unexpected error: %v", tc.name, err)
+			}
+
+			continue
+		}
+
+		if !tc.success {
+			t.Errorf("%v: unexpected success", tc.name)
+			continue
+		}
+
+		if e, a := tc.expected.Kind, actual.Kind; e != a {
+			t.Errorf("%v: kind: expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expected.APIVersion, actual.APIVersion; e != a {
+			t.Errorf("%v: apiVersion: expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expected.Name, actual.Name; e != a {
+			t.Errorf("%v: name: expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expected.Namespace, actual.Namespace; e != a {
+			t.Errorf("%v: namespace: expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expected.UID, actual.UID; e != a {
+			t.Errorf("%v: uid: expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expected.ResourceVersion, actual.ResourceVersion; e != a {
+			t.Errorf("%v: kind: expected %v, got %v", tc.name, e, a)
+		}
+	}
+}

pkg/kubelet/container/runtime.go

@@ -52,6 +52,14 @@ type Runtime interface {
 	// TODO(yifan): Pull/Remove images
 }
 
+// Container runner is a narrow interface to consume in the Kubelet
+// before there is a full implementation of Runtime.
+//
+// TODO: eventually include this interface in Runtime
+type ContainerRunner interface {
+	RunContainer(pod *api.Pod, container *api.Container, opts *RunContainerOptions) (string, error)
+}
+
 // Pod is a group of containers, with the status of the pod.
 type Pod struct {
 	// The ID of the pod, which can be used to retrieve a particular pod

pkg/kubelet/dockertools/docker.go

@@ -27,13 +27,10 @@ import (
 	"math/rand"
 	"os"
 	"os/exec"
-	"path"
 	"strconv"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/capabilities"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/record"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/credentialprovider"
 	kubecontainer "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/container"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/leaky"
@@ -945,104 +942,3 @@ func makeCapabilites(capAdd []api.CapabilityType, capDrop []api.CapabilityType)
 	}
 	return addCaps, dropCaps
 }
-
-// RunContainer creates and starts a docker container with the required RunContainerOptions.
-// On success it will return the container's ID with nil error. During the process, it will
-// use the reference and event recorder to report the state of the container (e.g. created,
-// started, failed, etc.).
-// TODO(yifan): To use a strong type for the returned container ID.
-func RunContainer(client DockerInterface, container *api.Container, pod *api.Pod, opts *kubecontainer.RunContainerOptions,
-	refManager *kubecontainer.RefManager, ref *api.ObjectReference, recorder record.EventRecorder) (string, error) {
-	dockerName := KubeletContainerName{
-		PodFullName:   kubecontainer.GetPodFullName(pod),
-		PodUID:        pod.UID,
-		ContainerName: container.Name,
-	}
-	exposedPorts, portBindings := makePortsAndBindings(container)
-	// TODO(vmarmol): Handle better.
-	// Cap hostname at 63 chars (specification is 64bytes which is 63 chars and the null terminating char).
-	const hostnameMaxLen = 63
-	containerHostname := pod.Name
-	if len(containerHostname) > hostnameMaxLen {
-		containerHostname = containerHostname[:hostnameMaxLen]
-	}
-	dockerOpts := docker.CreateContainerOptions{
-		Name: BuildDockerName(dockerName, container),
-		Config: &docker.Config{
-			Cmd:          container.Command,
-			Env:          opts.Envs,
-			ExposedPorts: exposedPorts,
-			Hostname:     containerHostname,
-			Image:        container.Image,
-			Memory:       container.Resources.Limits.Memory().Value(),
-			CPUShares:    milliCPUToShares(container.Resources.Limits.Cpu().MilliValue()),
-			WorkingDir:   container.WorkingDir,
-		},
-	}
-	dockerContainer, err := client.CreateContainer(dockerOpts)
-	if err != nil {
-		if ref != nil {
-			recorder.Eventf(ref, "failed", "Failed to create docker container with error: %v", err)
-		}
-		return "", err
-	}
-	// Remember this reference so we can report events about this container
-	if ref != nil {
-		refManager.SetRef(dockerContainer.ID, ref)
-		recorder.Eventf(ref, "created", "Created with docker id %v", dockerContainer.ID)
-	}
-
-	// The reason we create and mount the log file in here (not in kubelet) is because
-	// the file's location depends on the ID of the container, and we need to create and
-	// mount the file before actually starting the container.
-	// TODO(yifan): Consider to pull this logic out since we might need to reuse it in
-	// other container runtime.
-	if opts.PodContainerDir != "" && len(container.TerminationMessagePath) != 0 {
-		containerLogPath := path.Join(opts.PodContainerDir, dockerContainer.ID)
-		fs, err := os.Create(containerLogPath)
-		if err != nil {
-			// TODO: Clean up the previouly created dir? return the error?
-			glog.Errorf("Error on creating termination-log file %q: %v", containerLogPath, err)
-		} else {
-			fs.Close() // Close immediately; we're just doing a `touch` here
-			b := fmt.Sprintf("%s:%s", containerLogPath, container.TerminationMessagePath)
-			opts.Binds = append(opts.Binds, b)
-		}
-	}
-
-	privileged := false
-	if capabilities.Get().AllowPrivileged {
-		privileged = container.Privileged
-	} else if container.Privileged {
-		return "", fmt.Errorf("container requested privileged mode, but it is disallowed globally.")
-	}
-
-	capAdd, capDrop := makeCapabilites(container.Capabilities.Add, container.Capabilities.Drop)
-	hc := &docker.HostConfig{
-		PortBindings: portBindings,
-		Binds:        opts.Binds,
-		NetworkMode:  opts.NetMode,
-		IpcMode:      opts.IpcMode,
-		Privileged:   privileged,
-		CapAdd:       capAdd,
-		CapDrop:      capDrop,
-	}
-	if len(opts.DNS) > 0 {
-		hc.DNS = opts.DNS
-	}
-	if len(opts.DNSSearch) > 0 {
-		hc.DNSSearch = opts.DNSSearch
-	}
-
-	if err = client.StartContainer(dockerContainer.ID, hc); err != nil {
-		if ref != nil {
-			recorder.Eventf(ref, "failed",
-				"Failed to start with docker id %v with error: %v", dockerContainer.ID, err)
-		}
-		return "", err
-	}
-	if ref != nil {
-		recorder.Eventf(ref, "started", "Started with docker id %v", dockerContainer.ID)
-	}
-	return dockerContainer.ID, nil
-}

pkg/kubelet/dockertools/runner.go

@@ -0,0 +1,146 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dockertools
+
+import (
+	"fmt"
+	"os"
+	"path"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/capabilities"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/record"
+	kubecontainer "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/container"
+	"github.com/fsouza/go-dockerclient"
+	"github.com/golang/glog"
+)
+
+type DockerContainerRunner struct {
+	Client   DockerInterface
+	Recorder record.EventRecorder
+}
+
+func (r *DockerContainerRunner) RunContainer(pod *api.Pod, container *api.Container, opts *kubecontainer.RunContainerOptions) (string, error) {
+	ref, err := kubecontainer.GenerateContainerRef(pod, container)
+	if err != nil {
+		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
+	}
+
+	dockerName := KubeletContainerName{
+		PodFullName:   kubecontainer.GetPodFullName(pod),
+		PodUID:        pod.UID,
+		ContainerName: container.Name,
+	}
+	exposedPorts, portBindings := makePortsAndBindings(container)
+
+	// TODO(vmarmol): Handle better.
+	// Cap hostname at 63 chars (specification is 64bytes which is 63 chars and the null terminating char).
+	const hostnameMaxLen = 63
+	containerHostname := pod.Name
+	if len(containerHostname) > hostnameMaxLen {
+		containerHostname = containerHostname[:hostnameMaxLen]
+	}
+	dockerOpts := docker.CreateContainerOptions{
+		Name: BuildDockerName(dockerName, container),
+		Config: &docker.Config{
+			Env:          opts.Envs,
+			ExposedPorts: exposedPorts,
+			Hostname:     containerHostname,
+			Image:        container.Image,
+			Memory:       container.Resources.Limits.Memory().Value(),
+			CPUShares:    milliCPUToShares(container.Resources.Limits.Cpu().MilliValue()),
+			WorkingDir:   container.WorkingDir,
+		},
+	}
+
+	setEntrypointAndCommand(container, &dockerOpts)
+
+	dockerContainer, err := r.Client.CreateContainer(dockerOpts)
+	if err != nil {
+		if ref != nil {
+			r.Recorder.Eventf(ref, "failed", "Failed to create docker container with error: %v", err)
+		}
+		return "", err
+	}
+
+	if ref != nil {
+		r.Recorder.Eventf(ref, "created", "Created with docker id %v", dockerContainer.ID)
+	}
+
+	// The reason we create and mount the log file in here (not in kubelet) is because
+	// the file's location depends on the ID of the container, and we need to create and
+	// mount the file before actually starting the container.
+	// TODO(yifan): Consider to pull this logic out since we might need to reuse it in
+	// other container runtime.
+	if opts.PodContainerDir != "" && len(container.TerminationMessagePath) != 0 {
+		containerLogPath := path.Join(opts.PodContainerDir, dockerContainer.ID)
+		fs, err := os.Create(containerLogPath)
+		if err != nil {
+			// TODO: Clean up the previouly created dir? return the error?
+			glog.Errorf("Error on creating termination-log file %q: %v", containerLogPath, err)
+		} else {
+			fs.Close() // Close immediately; we're just doing a `touch` here
+			b := fmt.Sprintf("%s:%s", containerLogPath, container.TerminationMessagePath)
+			opts.Binds = append(opts.Binds, b)
+		}
+	}
+
+	privileged := false
+	if capabilities.Get().AllowPrivileged {
+		privileged = container.Privileged
+	} else if container.Privileged {
+		return "", fmt.Errorf("container requested privileged mode, but it is disallowed globally.")
+	}
+
+	capAdd, capDrop := makeCapabilites(container.Capabilities.Add, container.Capabilities.Drop)
+	hc := &docker.HostConfig{
+		PortBindings: portBindings,
+		Binds:        opts.Binds,
+		NetworkMode:  opts.NetMode,
+		IpcMode:      opts.IpcMode,
+		Privileged:   privileged,
+		CapAdd:       capAdd,
+		CapDrop:      capDrop,
+	}
+	if len(opts.DNS) > 0 {
+		hc.DNS = opts.DNS
+	}
+	if len(opts.DNSSearch) > 0 {
+		hc.DNSSearch = opts.DNSSearch
+	}
+
+	if err = r.Client.StartContainer(dockerContainer.ID, hc); err != nil {
+		if ref != nil {
+			r.Recorder.Eventf(ref, "failed",
+				"Failed to start with docker id %v with error: %v", dockerContainer.ID, err)
+		}
+		return "", err
+	}
+	if ref != nil {
+		r.Recorder.Eventf(ref, "started", "Started with docker id %v", dockerContainer.ID)
+	}
+	return dockerContainer.ID, nil
+}
+
+func setEntrypointAndCommand(container *api.Container, opts *docker.CreateContainerOptions) {
+	if len(container.Command) != 0 {
+		opts.Config.Entrypoint = container.Command
+	}
+	if len(container.Args) != 0 {
+		opts.Config.Cmd = container.Args
+	}
+}

pkg/kubelet/dockertools/runner_test.go

@@ -0,0 +1,89 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dockertools
+
+import (
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/fsouza/go-dockerclient"
+)
+
+func TestSetEntrypointAndCommand(t *testing.T) {
+	cases := []struct {
+		name      string
+		container *api.Container
+		expected  *docker.CreateContainerOptions
+	}{
+		{
+			name:      "none",
+			container: &api.Container{},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{},
+			},
+		},
+		{
+			name: "command",
+			container: &api.Container{
+				Command: []string{"foo", "bar"},
+			},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{
+					Entrypoint: []string{"foo", "bar"},
+				},
+			},
+		},
+		{
+			name: "args",
+			container: &api.Container{
+				Args: []string{"foo", "bar"},
+			},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{
+					Cmd: []string{"foo", "bar"},
+				},
+			},
+		},
+		{
+			name: "both",
+			container: &api.Container{
+				Command: []string{"foo"},
+				Args:    []string{"bar", "baz"},
+			},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{
+					Entrypoint: []string{"foo"},
+					Cmd:        []string{"bar", "baz"},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		actualOpts := &docker.CreateContainerOptions{
+			Config: &docker.Config{},
+		}
+		setEntrypointAndCommand(tc.container, actualOpts)
+
+		if e, a := tc.expected.Config.Entrypoint, actualOpts.Config.Entrypoint; !api.Semantic.DeepEqual(e, a) {
+			t.Errorf("%v: unexpected entrypoint: expected %v, got %v", tc.name, e, a)
+		}
+		if e, a := tc.expected.Config.Cmd, actualOpts.Config.Cmd; !api.Semantic.DeepEqual(e, a) {
+			t.Errorf("%v: unexpected command: expected %v, got %v", tc.name, e, a)
+		}
+	}
+}

pkg/kubelet/kubelet.go

@@ -214,6 +214,10 @@ func NewMainKubelet(
 		return nil, fmt.Errorf("failed to initialize image manager: %v", err)
 	}
 	statusManager := newStatusManager(kubeClient)
+	containerRunner := &dockertools.DockerContainerRunner{
+		Client:   dockerClient,
+		Recorder: recorder,
+	}
 
 	klet := &Kubelet{
 		hostname:               hostname,
@@ -243,6 +247,7 @@ func NewMainKubelet(
 		statusManager:                  statusManager,
 		cloud:                          cloud,
 		nodeRef:                        nodeRef,
+		containerRunner:                containerRunner,
 	}
 
 	klet.podManager = newBasicPodManager(klet.kubeClient)
@@ -359,6 +364,9 @@ type Kubelet struct {
 	// Syncs pods statuses with apiserver; also used as a cache of statuses.
 	statusManager *statusManager
 
+	// Knows how to run a container in a pod
+	containerRunner kubecontainer.ContainerRunner
+
 	//Cloud provider interface
 	cloud cloudprovider.Interface
 
@@ -649,7 +657,7 @@ func (kl *Kubelet) generateRunContainerOptions(pod *api.Pod, container *api.Cont
 
 // Run a single container from a pod. Returns the docker container ID
 func (kl *Kubelet) runContainer(pod *api.Pod, container *api.Container, podVolumes volumeMap, netMode, ipcMode string) (dockertools.DockerID, error) {
-	ref, err := kl.containerRefManager.GenerateContainerRef(pod, container)
+	ref, err := kubecontainer.GenerateContainerRef(pod, container)
 	if err != nil {
 		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
 	}
@@ -659,13 +667,16 @@ func (kl *Kubelet) runContainer(pod *api.Pod, container *api.Container, podVolum
 		return "", err
 	}
 
-	// TODO(yifan): Replace with RunContainerInPod, so we can eliminate 'netMode', 'ipcMode'
+	id, err := kl.containerRunner.RunContainer(pod, container, opts)
-	// by handling the pod infra container in the container runtime's implementation.
-	id, err := dockertools.RunContainer(kl.dockerClient, container, pod, opts, kl.containerRefManager, ref, kl.recorder)
 	if err != nil {
 		return "", err
 	}
 
+	// Remember this reference so we can report events about this container
+	if ref != nil {
+		kl.containerRefManager.SetRef(id, ref)
+	}
+
 	if container.Lifecycle != nil && container.Lifecycle.PostStart != nil {
 		handlerErr := kl.runHandler(kubecontainer.GetPodFullName(pod), pod.UID, container, container.Lifecycle.PostStart)
 		if handlerErr != nil {
@@ -885,7 +896,7 @@ func (kl *Kubelet) createPodInfraContainer(pod *api.Pod) (dockertools.DockerID,
 		Image: kl.podInfraContainerImage,
 		Ports: ports,
 	}
-	ref, err := kl.containerRefManager.GenerateContainerRef(pod, container)
+	ref, err := kubecontainer.GenerateContainerRef(pod, container)
 	if err != nil {
 		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
 	}
@@ -1036,7 +1047,7 @@ func (kl *Kubelet) getPodInfraContainer(podFullName string, uid types.UID,
 func (kl *Kubelet) pullImageAndRunContainer(pod *api.Pod, container *api.Container, podVolumes *volumeMap,
 	podInfraContainerID dockertools.DockerID) (dockertools.DockerID, error) {
 	podFullName := kubecontainer.GetPodFullName(pod)
-	ref, err := kl.containerRefManager.GenerateContainerRef(pod, container)
+	ref, err := kubecontainer.GenerateContainerRef(pod, container)
 	if err != nil {
 		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
 	}

pkg/kubelet/kubelet_test.go

@@ -112,6 +112,7 @@ func newTestKubelet(t *testing.T) *TestKubelet {
 	podManager, fakeMirrorClient := newFakePodManager()
 	kubelet.podManager = podManager
 	kubelet.containerRefManager = kubecontainer.NewRefManager()
+	kubelet.containerRunner = &dockertools.DockerContainerRunner{fakeDocker, fakeRecorder}
 	return &TestKubelet{kubelet, fakeDocker, mockCadvisor, fakeKubeClient, waitGroup, fakeMirrorClient}
 }
 

pkg/kubelet/runonce_test.go

@@ -140,6 +140,7 @@ func TestRunOnce(t *testing.T) {
 		t: t,
 	}
 	kb.dockerPuller = &dockertools.FakeDockerPuller{}
+	kb.containerRunner = &dockertools.DockerContainerRunner{kb.dockerClient, kb.recorder}
 	results, err := kb.runOnce([]api.Pod{
 		{
 			ObjectMeta: api.ObjectMeta{

pkg/util/runner_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2014 Google Inc. All rights reserved.
+Copyright 2015 Google Inc. All rights reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

test/e2e/docker_containers.go

@@ -0,0 +1,145 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+	"fmt"
+	"strings"
+	"time"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Docker Containers", func() {
+	var c *client.Client
+
+	BeforeEach(func() {
+		var err error
+		c, err = loadClient()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should use the image defaults if command and args are blank", func() {
+		runEntrypointTest("use defaults", c, entrypointTestPod(), []string{
+			"[/ep default arguments]",
+		})
+	})
+
+	It("should be able to override the image's default arguments (docker cmd)", func() {
+		pod := entrypointTestPod()
+		pod.Spec.Containers[0].Args = []string{"override", "arguments"}
+
+		runEntrypointTest("override arguments", c, pod, []string{
+			"[/ep override arguments]",
+		})
+	})
+
+	// Note: when you override the entrypoint, the image's arguments (docker cmd)
+	// are ignored.
+	It("should be able to override the image's default commmand (docker entrypoint)", func() {
+		pod := entrypointTestPod()
+		pod.Spec.Containers[0].Command = []string{"/ep-2"}
+
+		runEntrypointTest("override command", c, pod, []string{
+			"[/ep-2]",
+		})
+	})
+
+	It("should be able to override the image's default command and arguments", func() {
+		pod := entrypointTestPod()
+		pod.Spec.Containers[0].Command = []string{"/ep-2"}
+		pod.Spec.Containers[0].Args = []string{"override", "arguments"}
+
+		runEntrypointTest("override all", c, pod, []string{
+			"[/ep-2 override arguments]",
+		})
+	})
+})
+
+const testContainerName = "test-container"
+
+// Return a prototypical entrypoint test pod
+func entrypointTestPod() *api.Pod {
+	podName := "client-containers-" + string(util.NewUUID())
+
+	return &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name: podName,
+		},
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{
+					Name:  testContainerName,
+					Image: "kubernetes/eptest:0.1",
+				},
+			},
+			RestartPolicy: api.RestartPolicyNever,
+		},
+	}
+}
+
+// pod must have a container named 'test-container'
+func runEntrypointTest(scenarioName string, c *client.Client, pod *api.Pod, expectedOutput []string) {
+	ns := api.NamespaceDefault
+	By(fmt.Sprintf("Creating a pod to test %v", scenarioName))
+
+	defer c.Pods(ns).Delete(pod.Name)
+	if _, err := c.Pods(ns).Create(pod); err != nil {
+		Failf("Failed to create pod: %v", err)
+	}
+	// Wait for client pod to complete.
+	expectNoError(waitForPodSuccess(c, pod.Name, testContainerName))
+
+	// Grab its logs.  Get host first.
+	podStatus, err := c.Pods(ns).Get(pod.Name)
+	if err != nil {
+		Failf("Failed to get pod to know host: %v", err)
+	}
+	By(fmt.Sprintf("Trying to get logs from host %s pod %s container %s: %v",
+		podStatus.Status.Host, podStatus.Name, podStatus.Spec.Containers[0].Name, err))
+	var logs []byte
+	start := time.Now()
+
+	// Sometimes the actual containers take a second to get started, try to get logs for 60s
+	for time.Now().Sub(start) < (60 * time.Second) {
+		logs, err = c.Get().
+			Prefix("proxy").
+			Resource("minions").
+			Name(podStatus.Status.Host).
+			Suffix("containerLogs", ns, podStatus.Name, podStatus.Spec.Containers[0].Name).
+			Do().
+			Raw()
+		fmt.Sprintf("pod logs:%v\n", string(logs))
+		By(fmt.Sprintf("pod logs:%v\n", string(logs)))
+		if strings.Contains(string(logs), "Internal Error") {
+			By(fmt.Sprintf("Failed to get logs from host %s pod %s container %s: %v",
+				podStatus.Status.Host, podStatus.Name, podStatus.Spec.Containers[0].Name, string(logs)))
+			time.Sleep(5 * time.Second)
+			continue
+		}
+		break
+	}
+
+	for _, m := range expectedOutput {
+		Expect(string(logs)).To(ContainSubstring(m), "%q in container output", m)
+	}
+}