diff --git a/pkg/api/types.go b/pkg/api/types.go index 9bac8e1cfb1..cf5de924ad5 100644 --- a/pkg/api/types.go +++ b/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"` diff --git a/pkg/controller/replication_controller.go b/pkg/controller/replication_controller.go index 771d05d1595..dfdc0bd23d0 100644 --- a/pkg/controller/replication_controller.go +++ b/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, } diff --git a/pkg/controller/replication_controller_test.go b/pkg/controller/replication_controller_test.go index 9492e4fd658..4b147f83fdd 100644 --- a/pkg/controller/replication_controller_test.go +++ b/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) { diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index 296ce9c756a..6aa0dcbe99d 100644 --- a/pkg/registry/controller_registry.go +++ b/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) { diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index c63991dc15f..ebbccdc2689 100644 --- a/pkg/registry/pod_registry.go +++ b/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) { diff --git a/pkg/registry/pod_registry_test.go b/pkg/registry/pod_registry_test.go index 8d547d02f49..160bc374f5e 100644 --- a/pkg/registry/pod_registry_test.go +++ b/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"),