Introduce the simplest RestartPolicy and handling.

pkg/api/types.go

@@ -58,9 +58,10 @@ type ContainerManifest struct {
 	// TODO: UUID on Manifest is deprecated in the future once we are done
 	// with the API refactoring. It is required for now to determine the instance
 	// of a Pod.
-	UUID       string      `yaml:"uuid,omitempty" json:"uuid,omitempty"`
+	UUID          string        `yaml:"uuid,omitempty" json:"uuid,omitempty"`
-	Volumes    []Volume    `yaml:"volumes" json:"volumes"`
+	Volumes       []Volume      `yaml:"volumes" json:"volumes"`
-	Containers []Container `yaml:"containers" json:"containers"`
+	Containers    []Container   `yaml:"containers" json:"containers"`
+	RestartPolicy RestartPolicy `json:"restartPolicy,omitempty" yaml:"restartPolicy,omitempty"`
 }
 
 // ContainerManifestList is used to communicate container manifests to kubelet.
@@ -254,19 +255,21 @@ const (
 // PodInfo contains one entry for every container with available info.
 type PodInfo map[string]docker.Container
 
-// RestartPolicyType represents a restart policy for a pod.
+type RestartPolicyAlways struct{}
-type RestartPolicyType string
 
-// Valid restart policies defined for a PodState.RestartPolicy.
+// TODO(dchen1107): Define what kinds of failures should restart.
-const (
+// TODO(dchen1107): Decide whether to support policy knobs, and, if so, which ones.
-	RestartAlways    RestartPolicyType = "RestartAlways"
+type RestartPolicyOnFailure struct{}
-	RestartOnFailure RestartPolicyType = "RestartOnFailure"
+
-	RestartNever     RestartPolicyType = "RestartNever"
+type RestartPolicyNever struct{}
-)
 
 type RestartPolicy struct {
-	// Optional: Defaults to "RestartAlways".
+	// Only one of the following restart policies may be specified.
-	Type RestartPolicyType `yaml:"type,omitempty" json:"type,omitempty"`
+	// If none of the following policies is specified, the default one
+	// is RestartPolicyAlways.
+	Always    *RestartPolicyAlways    `json:"always,omitempty" yaml:"always,omitempty"`
+	OnFailure *RestartPolicyOnFailure `json:"onFailure,omitempty" yaml:"onFailure,omitempty"`
+	Never     *RestartPolicyNever     `json:"never,omitempty" yaml:"never,omitempty"`
 }
 
 // PodState is the state of a pod, used as either input (desired state) or output (current state).
@@ -283,8 +286,7 @@ type PodState struct {
 	// upon.
 	// TODO: Make real decisions about what our info should look like. Re-enable fuzz test
 	// when we have done this.
-	Info          PodInfo       `json:"info,omitempty" yaml:"info,omitempty"`
+	Info PodInfo `json:"info,omitempty" yaml:"info,omitempty"`
-	RestartPolicy RestartPolicy `json:"restartpolicy,omitempty" yaml:"restartpolicy,omitempty"`
 }
 
 // PodList is a list of Pods.

pkg/api/v1beta1/types.go

@@ -17,10 +17,10 @@ limitations under the License.
 package v1beta1
 
 import (
-	"github.com/fsouza/go-dockerclient"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
+	"github.com/fsouza/go-dockerclient"
 )
 
 // Common string formats
@@ -58,9 +58,10 @@ type ContainerManifest struct {
 	// TODO: UUID on Manifext is deprecated in the future once we are done
 	// with the API refactory. It is required for now to determine the instance
 	// of a Pod.
-	UUID       string      `yaml:"uuid,omitempty" json:"uuid,omitempty"`
+	UUID          string        `yaml:"uuid,omitempty" json:"uuid,omitempty"`
-	Volumes    []Volume    `yaml:"volumes" json:"volumes"`
+	Volumes       []Volume      `yaml:"volumes" json:"volumes"`
-	Containers []Container `yaml:"containers" json:"containers"`
+	Containers    []Container   `yaml:"containers" json:"containers"`
+	RestartPolicy RestartPolicy `json:"restartPolicy,omitempty" yaml:"restartPolicy,omitempty"`
 }
 
 // ContainerManifestList is used to communicate container manifests to kubelet.
@@ -267,19 +268,21 @@ const (
 // PodInfo contains one entry for every container with available info.
 type PodInfo map[string]docker.Container
 
-// RestartPolicyType represents a restart policy for a pod.
+type RestartPolicyAlways struct{}
-type RestartPolicyType string
 
-// Valid restart policies defined for a PodState.RestartPolicy.
+// TODO(dchen1107): Define what kinds of failures should restart
-const (
+// TODO(dchen1107): Decide whether to support policy knobs, and, if so, which ones.
-	RestartAlways    RestartPolicyType = "RestartAlways"
+type RestartPolicyOnFailure struct{}
-	RestartOnFailure RestartPolicyType = "RestartOnFailure"
+
-	RestartNever     RestartPolicyType = "RestartNever"
+type RestartPolicyNever struct{}
-)
 
 type RestartPolicy struct {
-	// Optional: Defaults to "RestartAlways".
+	// Only one of the following restart policy may be specified.
-	Type RestartPolicyType `yaml:"type,omitempty" json:"type,omitempty"`
+	// If none of the following policies is specified, the default one
+	// is RestartPolicyAlways.
+	Always    *RestartPolicyAlways    `json:"always,omitempty" yaml:"always,omitempty"`
+	OnFailure *RestartPolicyOnFailure `json:"onFailure,omitempty" yaml:"onFailure,omitempty"`
+	Never     *RestartPolicyNever     `json:"never,omitempty" yaml:"never,omitempty"`
 }
 
 // PodState is the state of a pod, used as either input (desired state) or output (current state).
@@ -296,8 +299,7 @@ type PodState struct {
 	// upon. To allow marshalling/unmarshalling, we copied the client's structs and added
 	// json/yaml tags.
 	// TODO: Make real decisions about what our info should look like.
-	Info          PodInfo       `json:"info,omitempty" yaml:"info,omitempty"`
+	Info PodInfo `json:"info,omitempty" yaml:"info,omitempty"`
-	RestartPolicy RestartPolicy `json:"restartpolicy,omitempty" yaml:"restartpolicy,omitempty"`
 }
 
 // PodList is a list of Pods.

pkg/api/validation/validation.go

@@ -231,19 +231,33 @@ func ValidateManifest(manifest *api.ContainerManifest) errs.ErrorList {
 	allVolumes, errs := validateVolumes(manifest.Volumes)
 	allErrs = append(allErrs, errs.Prefix("volumes")...)
 	allErrs = append(allErrs, validateContainers(manifest.Containers, allVolumes).Prefix("containers")...)
+	allErrs = append(allErrs, validateRestartPolicy(&manifest.RestartPolicy).Prefix("restartPolicy")...)
 	return allErrs
 }
 
+func validateRestartPolicy(restartPolicy *api.RestartPolicy) errs.ErrorList {
+	numPolicies := 0
+	allErrors := errs.ErrorList{}
+	if restartPolicy.Always != nil {
+		numPolicies++
+	}
+	if restartPolicy.OnFailure != nil {
+		numPolicies++
+	}
+	if restartPolicy.Never != nil {
+		numPolicies++
+	}
+	if numPolicies == 0 {
+		restartPolicy.Always = &api.RestartPolicyAlways{}
+	}
+	if numPolicies > 1 {
+		allErrors = append(allErrors, errs.NewFieldInvalid("", restartPolicy))
+	}
+	return allErrors
+}
+
 func ValidatePodState(podState *api.PodState) errs.ErrorList {
 	allErrs := errs.ErrorList(ValidateManifest(&podState.Manifest)).Prefix("manifest")
-	if podState.RestartPolicy.Type == "" {
-		podState.RestartPolicy.Type = api.RestartAlways
-	} else if podState.RestartPolicy.Type != api.RestartAlways &&
-		podState.RestartPolicy.Type != api.RestartOnFailure &&
-		podState.RestartPolicy.Type != api.RestartNever {
-		allErrs = append(allErrs, errs.NewFieldNotSupported("restartPolicy.type", podState.RestartPolicy.Type))
-	}
-
 	return allErrs
 }
 

pkg/api/validation/validation_test.go

@@ -216,6 +216,40 @@ func TestValidateContainers(t *testing.T) {
 	}
 }
 
+func TestValidateRestartPolicy(t *testing.T) {
+	successCases := []api.RestartPolicy{
+		{},
+		{Always: &api.RestartPolicyAlways{}},
+		{OnFailure: &api.RestartPolicyOnFailure{}},
+		{Never: &api.RestartPolicyNever{}},
+	}
+	for _, policy := range successCases {
+		if errs := validateRestartPolicy(&policy); len(errs) != 0 {
+			t.Errorf("expected success: %v", errs)
+		}
+	}
+
+	errorCases := []api.RestartPolicy{
+		{Always: &api.RestartPolicyAlways{}, Never: &api.RestartPolicyNever{}},
+		{Never: &api.RestartPolicyNever{}, OnFailure: &api.RestartPolicyOnFailure{}},
+	}
+	for k, policy := range errorCases {
+		if errs := validateRestartPolicy(&policy); len(errs) == 0 {
+			t.Errorf("expected failure for %s", k)
+		}
+	}
+
+	noPolicySpecified := api.RestartPolicy{}
+	errs := validateRestartPolicy(&noPolicySpecified)
+	if len(errs) != 0 {
+		t.Errorf("expected success: %v", errs)
+	}
+	if noPolicySpecified.Always == nil {
+		t.Errorf("expected Always policy specified")
+	}
+
+}
+
 func TestValidateManifest(t *testing.T) {
 	successCases := []api.ContainerManifest{
 		{Version: "v1beta1", ID: "abc"},
@@ -286,8 +320,13 @@ func TestValidatePod(t *testing.T) {
 			"foo": "bar",
 		},
 		DesiredState: api.PodState{
-			Manifest:      api.ContainerManifest{Version: "v1beta1", ID: "abc"},
+			Manifest: api.ContainerManifest{
-			RestartPolicy: api.RestartPolicy{Type: "RestartAlways"},
+				Version: "v1beta1",
+				ID:      "abc",
+				RestartPolicy: api.RestartPolicy{
+					Always: &api.RestartPolicyAlways{},
+				},
+			},
 		},
 	})
 	if len(errs) != 0 {
@@ -312,8 +351,12 @@ func TestValidatePod(t *testing.T) {
 			"foo": "bar",
 		},
 		DesiredState: api.PodState{
-			Manifest:      api.ContainerManifest{Version: "v1beta1", ID: "abc"},
+			Manifest: api.ContainerManifest{
-			RestartPolicy: api.RestartPolicy{Type: "WhatEver"},
+				Version: "v1beta1",
+				ID:      "abc",
+				RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{},
+					Never: &api.RestartPolicyNever{}},
+			},
 		},
 	})
 	if len(errs) != 1 {

pkg/kubelet/config/config_test.go

@@ -52,7 +52,8 @@ func CreateValidPod(name, namespace string) kubelet.Pod {
 		Name:      name,
 		Namespace: namespace,
 		Manifest: api.ContainerManifest{
-			Version: "v1beta1",
+			Version:       "v1beta1",
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
 		},
 	}
 }

pkg/kubelet/config/http_test.go

@@ -105,8 +105,12 @@ func TestExtractFromHTTP(t *testing.T) {
 			manifests: api.ContainerManifest{Version: "v1beta1", ID: "foo"},
 			expected: CreatePodUpdate(kubelet.SET,
 				kubelet.Pod{
-					Name:     "foo",
+					Name: "foo",
-					Manifest: api.ContainerManifest{Version: "v1beta1", ID: "foo"},
+					Manifest: api.ContainerManifest{
+						Version:       "v1beta1",
+						ID:            "foo",
+						RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+					},
 				}),
 		},
 		{

pkg/kubelet/docker.go

@@ -153,6 +153,33 @@ func getKubeletDockerContainers(client DockerInterface) (DockerContainers, error
 	return result, nil
 }
 
+// getRecentDockerContainersWithName returns a list of dead docker containers which matches the name
+// and uuid given.
+func getRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullName, uuid, containerName string) ([]*docker.Container, error) {
+	var result []*docker.Container
+	containers, err := client.ListContainers(docker.ListContainersOptions{All: true})
+	if err != nil {
+		return nil, err
+	}
+	for _, dockerContainer := range containers {
+		dockerPodName, dockerUUID, dockerContainerName, _ := parseDockerName(dockerContainer.Names[0])
+		if dockerPodName != podFullName {
+			continue
+		}
+		if uuid != "" && dockerUUID != uuid {
+			continue
+		}
+		if dockerContainerName != containerName {
+			continue
+		}
+		inspectResult, _ := client.InspectContainer(dockerContainer.ID)
+		if inspectResult != nil && !inspectResult.State.Running && !inspectResult.State.Paused {
+			result = append(result, inspectResult)
+		}
+	}
+	return result, nil
+}
+
 // ErrNoContainersInPod is returned when there are no running containers for a given pod
 var ErrNoContainersInPod = errors.New("no containers exist for this pod")
 

pkg/kubelet/kubelet.go

@@ -516,11 +516,35 @@ func (kl *Kubelet) syncPod(pod *Pod, dockerContainers DockerContainers) error {
 			killedContainers[containerID] = empty{}
 		}
 
-		glog.Infof("Container doesn't exist, creating %#v", container)
+		// Check RestartPolicy for container
+		recentContainers, err := getRecentDockerContainersWithNameAndUUID(kl.dockerClient, podFullName, uuid, container.Name)
+		if err != nil {
+			glog.Errorf("Error listing recent containers with name and uuid:%s--%s--%s", podFullName, uuid, container.Name)
+			// TODO(dawnchen): error handling here?
+		}
+
+		if len(recentContainers) > 0 && pod.Manifest.RestartPolicy.Always == nil {
+			if pod.Manifest.RestartPolicy.Never != nil {
+				glog.Infof("Already ran container with name %s--%s--%s, do nothing",
+					podFullName, uuid, container.Name)
+				continue
+			}
+			if pod.Manifest.RestartPolicy.OnFailure != nil {
+				// Check the exit code of last run
+				if recentContainers[0].State.ExitCode == 0 {
+					glog.Infof("Already successfully ran container with name %s--%s--%s, do nothing",
+						podFullName, uuid, container.Name)
+					continue
+				}
+			}
+		}
+
+		glog.Infof("Container with name %s--%s--%s doesn't exist, creating %#v", podFullName, uuid, container.Name, container)
 		if err := kl.dockerPuller.Pull(container.Image); err != nil {
 			glog.Errorf("Failed to pull image %s: %v skipping pod %s container %s.", container.Image, err, podFullName, container.Name)
 			continue
 		}
+		// TODO(dawnchen): Check RestartPolicy.DelaySeconds before restart a container
 		containerID, err := kl.runContainer(pod, &container, podVolumes, "container:"+string(netID))
 		if err != nil {
 			// TODO(bburns) : Perhaps blacklist a container after N failures?

pkg/kubelet/kubelet_test.go

@@ -300,7 +300,7 @@ func TestSyncPodsCreatesNetAndContainer(t *testing.T) {
 	kubelet.drainWorkers()
 
 	verifyCalls(t, fakeDocker, []string{
-		"list", "list", "create", "start", "list", "inspect", "create", "start"})
+		"list", "list", "create", "start", "list", "inspect", "list", "create", "start"})
 
 	fakeDocker.lock.Lock()
 	if len(fakeDocker.Created) != 2 ||
@@ -338,7 +338,7 @@ func TestSyncPodsWithNetCreatesContainer(t *testing.T) {
 	kubelet.drainWorkers()
 
 	verifyCalls(t, fakeDocker, []string{
-		"list", "list", "list", "inspect", "create", "start"})
+		"list", "list", "list", "inspect", "list", "create", "start"})
 
 	fakeDocker.lock.Lock()
 	if len(fakeDocker.Created) != 1 ||
@@ -388,7 +388,7 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
 	kubelet.drainWorkers()
 
 	verifyCalls(t, fakeDocker, []string{
-		"list", "list", "list", "inspect", "create", "start"})
+		"list", "list", "list", "inspect", "list", "create", "start"})
 
 	fakeDocker.lock.Lock()
 	if len(fakeDocker.Created) != 1 ||
@@ -428,7 +428,7 @@ func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) {
 	kubelet.drainWorkers()
 
 	verifyCalls(t, fakeDocker, []string{
-		"list", "list", "stop", "create", "start", "list", "list", "inspect", "create", "start"})
+		"list", "list", "stop", "create", "start", "list", "list", "inspect", "list", "create", "start"})
 
 	// A map iteration is used to delete containers, so must not depend on
 	// order here.
@@ -561,7 +561,7 @@ func TestSyncPodBadHash(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 
-	verifyCalls(t, fakeDocker, []string{"list", "stop", "create", "start"})
+	verifyCalls(t, fakeDocker, []string{"list", "stop", "list", "create", "start"})
 
 	// A map interation is used to delete containers, so must not depend on
 	// order here.
@@ -608,7 +608,7 @@ func TestSyncPodUnhealthy(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 
-	verifyCalls(t, fakeDocker, []string{"list", "stop", "create", "start"})
+	verifyCalls(t, fakeDocker, []string{"list", "stop", "list", "create", "start"})
 
 	// A map interation is used to delete containers, so must not depend on
 	// order here.
@@ -1329,7 +1329,7 @@ func TestSyncPodEventHandlerFails(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 
-	verifyCalls(t, fakeDocker, []string{"list", "create", "start", "stop"})
+	verifyCalls(t, fakeDocker, []string{"list", "list", "create", "start", "stop"})
 
 	if len(fakeDocker.stopped) != 1 {
 		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)