Ensure pod and manifest always have a UUID

* Fixes implication #2 in docs/identifiers.md
* Replication controller lets the apiserver set the pod ID

pkg/api/types.go

@@ -50,6 +50,7 @@ type ContainerManifest struct {
 	// Required: This must be a supported version string, such as "v1beta1".
 	Version string `yaml:"version" json:"version"`
 	// Required: This must be a DNS_SUBDOMAIN.
+	// TODO: ID on Manifest is deprecated and will be removed in the future.
 	ID         string      `yaml:"id" json:"id"`
 	Volumes    []Volume    `yaml:"volumes" json:"volumes"`
 	Containers []Container `yaml:"containers" json:"containers"`

pkg/controller/replication_controller.go

@@ -19,7 +19,6 @@ package controller
 import (
 	"encoding/json"
 	"fmt"
-	"math/rand"
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -64,9 +63,6 @@ func (r RealPodControl) createReplica(controllerSpec api.ReplicationController)
 		labels["replicationController"] = controllerSpec.ID
 	}
 	pod := api.Pod{
-		JSONBase: api.JSONBase{
-			ID: fmt.Sprintf("%08x", rand.Uint32()),
-		},
 		DesiredState: controllerSpec.DesiredState.PodTemplate.DesiredState,
 		Labels:       controllerSpec.DesiredState.PodTemplate.Labels,
 	}

pkg/controller/replication_controller_test.go

@@ -200,12 +200,22 @@ func TestCreateReplica(t *testing.T) {
 
 	podControl.createReplica(controllerSpec)
 
-	//expectedPod := Pod{
-	//	Labels:       controllerSpec.DesiredState.PodTemplate.Labels,
-	//	DesiredState: controllerSpec.DesiredState.PodTemplate.DesiredState,
-	//}
-	// TODO: fix this so that it validates the body.
+	expectedPod := api.Pod{
+		JSONBase: api.JSONBase{
+			Kind: "Pod",
+		},
+		Labels:       controllerSpec.DesiredState.PodTemplate.Labels,
+		DesiredState: controllerSpec.DesiredState.PodTemplate.DesiredState,
+	}
 	fakeHandler.ValidateRequest(t, makeURL("/pods"), "POST", nil)
+	actualPod := api.Pod{}
+	if err := json.Unmarshal([]byte(fakeHandler.RequestBody), &actualPod); err != nil {
+		t.Errorf("Unexpected error: %#v", err)
+	}
+	if !reflect.DeepEqual(expectedPod, actualPod) {
+		t.Logf("Body: %s", fakeHandler.RequestBody)
+		t.Errorf("Unexpected mismatch.  Expected %#v, Got: %#v", expectedPod, actualPod)
+	}
 }
 
 func TestHandleWatchResponseNotSet(t *testing.T) {

pkg/registry/controller_registry.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"time"
 
+	"code.google.com/p/go-uuid/uuid"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@@ -79,9 +80,12 @@ func (storage *ControllerRegistryStorage) Create(obj interface{}) (<-chan interf
 	if !ok {
 		return nil, fmt.Errorf("not a replication controller: %#v", obj)
 	}
-	if controller.ID == "" {
-		return nil, fmt.Errorf("ID should not be empty: %#v", controller)
+	if len(controller.ID) == 0 {
+		controller.ID = uuid.NewUUID().String()
 	}
+	// Pod Manifest ID should be assigned by the pod API
+	controller.DesiredState.PodTemplate.DesiredState.Manifest.ID = ""
+
 	return apiserver.MakeAsync(func() (interface{}, error) {
 		err := storage.registry.CreateController(controller)
 		if err != nil {
@@ -96,7 +100,7 @@ func (storage *ControllerRegistryStorage) Update(obj interface{}) (<-chan interf
 	if !ok {
 		return nil, fmt.Errorf("not a replication controller: %#v", obj)
 	}
-	if controller.ID == "" {
+	if len(controller.ID) == 0 {
 		return nil, fmt.Errorf("ID should not be empty: %#v", controller)
 	}
 	return apiserver.MakeAsync(func() (interface{}, error) {

pkg/registry/pod_registry.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 	"time"
 
+	"code.google.com/p/go-uuid/uuid"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
@@ -185,8 +186,9 @@ func (storage *PodRegistryStorage) Extract(body []byte) (interface{}, error) {
 func (storage *PodRegistryStorage) Create(obj interface{}) (<-chan interface{}, error) {
 	pod := obj.(api.Pod)
 	if len(pod.ID) == 0 {
-		return nil, fmt.Errorf("id is unspecified: %#v", pod)
+		pod.ID = uuid.NewUUID().String()
 	}
+	pod.DesiredState.Manifest.ID = pod.ID
 
 	return apiserver.MakeAsync(func() (interface{}, error) {
 		// TODO(lavalamp): Separate scheduler more cleanly.
@@ -205,7 +207,7 @@ func (storage *PodRegistryStorage) Create(obj interface{}) (<-chan interface{},
 func (storage *PodRegistryStorage) Update(obj interface{}) (<-chan interface{}, error) {
 	pod := obj.(api.Pod)
 	if len(pod.ID) == 0 {
-		return nil, fmt.Errorf("id is unspecified: %#v", pod)
+		return nil, fmt.Errorf("ID should not be empty: %#v", pod)
 	}
 
 	return apiserver.MakeAsync(func() (interface{}, error) {

pkg/registry/pod_registry_test.go

@@ -35,6 +35,104 @@ func expectNoError(t *testing.T, err error) {
 	}
 }
 
+func expectApiStatusError(t *testing.T, ch <-chan interface{}, msg string) {
+	out := <-ch
+	status, ok := out.(*api.Status)
+	if !ok {
+		t.Errorf("Expected an api.Status object, was %#v", out)
+		return
+	}
+	if msg != status.Details {
+		t.Errorf("Expected %#v, was %s", msg, status.Details)
+	}
+}
+
+func expectPod(t *testing.T, ch <-chan interface{}) (*api.Pod, bool) {
+	out := <-ch
+	pod, ok := out.(*api.Pod)
+	if !ok || pod == nil {
+		t.Errorf("Expected an api.Pod object, was %#v", out)
+		return nil, false
+	}
+	return pod, true
+}
+
+func TestCreatePodRegistryError(t *testing.T) {
+	mockRegistry := &MockPodRegistry{
+		err: fmt.Errorf("test error"),
+	}
+	storage := PodRegistryStorage{
+		scheduler: &MockScheduler{},
+		registry:  mockRegistry,
+	}
+	pod := api.Pod{}
+	ch, err := storage.Create(pod)
+	if err != nil {
+		t.Errorf("Expected %#v, Got %#v", nil, err)
+	}
+	expectApiStatusError(t, ch, mockRegistry.err.Error())
+}
+
+type MockScheduler struct {
+	err     error
+	pod     api.Pod
+	machine string
+}
+
+func (m *MockScheduler) Schedule(pod api.Pod, lister scheduler.MinionLister) (string, error) {
+	m.pod = pod
+	return m.machine, m.err
+}
+
+func TestCreatePodSchedulerError(t *testing.T) {
+	mockScheduler := MockScheduler{
+		err: fmt.Errorf("test error"),
+	}
+	storage := PodRegistryStorage{
+		scheduler: &mockScheduler,
+	}
+	pod := api.Pod{}
+	ch, err := storage.Create(pod)
+	if err != nil {
+		t.Errorf("Expected %#v, Got %#v", nil, err)
+	}
+	expectApiStatusError(t, ch, mockScheduler.err.Error())
+}
+
+type MockPodStorageRegistry struct {
+	MockPodRegistry
+	machine string
+}
+
+func (r *MockPodStorageRegistry) CreatePod(machine string, pod api.Pod) error {
+	r.MockPodRegistry.pod = &pod
+	r.machine = machine
+	return r.MockPodRegistry.err
+}
+
+func TestCreatePodSetsIds(t *testing.T) {
+	mockRegistry := &MockPodStorageRegistry{
+		MockPodRegistry: MockPodRegistry{err: fmt.Errorf("test error")},
+	}
+	storage := PodRegistryStorage{
+		scheduler: &MockScheduler{machine: "test"},
+		registry:  mockRegistry,
+	}
+	pod := api.Pod{}
+	ch, err := storage.Create(pod)
+	if err != nil {
+		t.Errorf("Expected %#v, Got %#v", nil, err)
+	}
+	expectApiStatusError(t, ch, mockRegistry.err.Error())
+
+	if len(mockRegistry.MockPodRegistry.pod.ID) == 0 {
+		t.Errorf("Expected pod ID to be set, Got %#v", pod)
+	}
+	if mockRegistry.MockPodRegistry.pod.DesiredState.Manifest.ID != mockRegistry.MockPodRegistry.pod.ID {
+		t.Errorf("Expected manifest ID to be equal to pod ID, Got %#v", pod)
+	}
+}
+
 func TestListPodsError(t *testing.T) {
 	mockRegistry := MockPodRegistry{
 		err: fmt.Errorf("test error"),