Use name generation on pods via replication controllers

The generated name is '<controllerName>-%s', unless controllerName-
would be long enough to cause a validation error.

pkg/api/rest/resttest/resttest.go

@@ -24,6 +24,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 type Tester struct {
@@ -53,6 +54,28 @@ func (t *Tester) TestCreate(valid runtime.Object, invalid ...runtime.Object) {
 	t.TestCreateInvokesValidation(invalid...)
 }
 
+func (t *Tester) TestCreateResetsUserData(valid runtime.Object) {
+	objectMeta, err := api.ObjectMetaFor(valid)
+	if err != nil {
+		t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid)
+	}
+
+	now := util.Now()
+	objectMeta.UID = "bad-uid"
+	objectMeta.CreationTimestamp = now
+
+	channel, err := t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	if obj := <-channel; obj.Object == nil {
+		t.Fatalf("Unexpected object from channel: %#v", obj)
+	}
+	if objectMeta.UID == "bad-uid" || objectMeta.CreationTimestamp == now {
+		t.Errorf("ObjectMeta did not reset basic fields: %#v", objectMeta)
+	}
+}
+
 func (t *Tester) TestCreateHasMetadata(valid runtime.Object) {
 	objectMeta, err := api.ObjectMetaFor(valid)
 	if err != nil {

pkg/api/rest/types.go

@@ -45,3 +45,26 @@ func (rcStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
 	controller := obj.(*api.ReplicationController)
 	return validation.ValidateReplicationController(controller)
 }
+
+// podStrategy implements behavior for Pods
+// TODO: move to a pod specific package.
+type podStrategy struct {
+	runtime.ObjectTyper
+	api.NameGenerator
+}
+
+// Pods is the default logic that applies when creating and updating Pod
+// objects.
+var Pods RESTCreateStrategy = podStrategy{api.Scheme, api.SimpleNameGenerator}
+
+// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation.
+func (podStrategy) ResetBeforeCreate(obj runtime.Object) {
+	pod := obj.(*api.Pod)
+	pod.Status = api.PodStatus{}
+}
+
+// Validate validates a new pod.
+func (podStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
+	pod := obj.(*api.Pod)
+	return validation.ValidatePod(pod)
+}

pkg/api/testing/fuzzer.go

@@ -24,6 +24,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 
 	"github.com/fsouza/go-dockerclient"
@@ -57,6 +58,8 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
 			j.Name = c.RandString()
 			j.ResourceVersion = strconv.FormatUint(c.RandUint64(), 10)
 			j.SelfLink = c.RandString()
+			j.UID = types.UID(c.RandString())
+			j.GenerateName = c.RandString()
 
 			var sec, nsec int64
 			c.Fuzz(&sec)

pkg/api/v1beta1/conversion.go

@@ -79,6 +79,7 @@ func init() {
 		func(in *newer.ObjectMeta, out *TypeMeta, s conversion.Scope) error {
 			out.Namespace = in.Namespace
 			out.ID = in.Name
+			out.GenerateName = in.GenerateName
 			out.UID = in.UID
 			out.CreationTimestamp = in.CreationTimestamp
 			out.SelfLink = in.SelfLink
@@ -94,6 +95,7 @@ func init() {
 		func(in *TypeMeta, out *newer.ObjectMeta, s conversion.Scope) error {
 			out.Namespace = in.Namespace
 			out.Name = in.ID
+			out.GenerateName = in.GenerateName
 			out.UID = in.UID
 			out.CreationTimestamp = in.CreationTimestamp
 			out.SelfLink = in.SelfLink

pkg/api/v1beta2/conversion.go

@@ -79,6 +79,7 @@ func init() {
 		func(in *newer.ObjectMeta, out *TypeMeta, s conversion.Scope) error {
 			out.Namespace = in.Namespace
 			out.ID = in.Name
+			out.GenerateName = in.GenerateName
 			out.UID = in.UID
 			out.CreationTimestamp = in.CreationTimestamp
 			out.SelfLink = in.SelfLink
@@ -94,6 +95,7 @@ func init() {
 		func(in *TypeMeta, out *newer.ObjectMeta, s conversion.Scope) error {
 			out.Namespace = in.Namespace
 			out.Name = in.ID
+			out.GenerateName = in.GenerateName
 			out.UID = in.UID
 			out.CreationTimestamp = in.CreationTimestamp
 			out.SelfLink = in.SelfLink

pkg/api/validation/validation.go

@@ -66,6 +66,35 @@ func maskTrailingDash(name string) string {
 	return name
 }
 
+// ValidatePodName can be used to check whether the given pod name is valid.
+// Prefix indicates this name will be used as part of generation, in which case
+// trailing dashes are allowed.
+func ValidatePodName(name string, prefix bool) (bool, string) {
+	return nameIsDNSSubdomain(name, prefix)
+}
+
+// ValidateReplicationControllerName can be used to check whether the given replication
+// controller name is valid.
+// Prefix indicates this name will be used as part of generation, in which case
+// trailing dashes are allowed.
+func ValidateReplicationControllerName(name string, prefix bool) (bool, string) {
+	return nameIsDNSSubdomain(name, prefix)
+}
+
+// ValidateServiceName can be used to check whether the given service name is valid.
+// Prefix indicates this name will be used as part of generation, in which case
+// trailing dashes are allowed.
+func ValidateServiceName(name string, prefix bool) (bool, string) {
+	return nameIsDNS952Label(name, prefix)
+}
+
+// ValidateNodeName can be used to check whether the given node name is valid.
+// Prefix indicates this name will be used as part of generation, in which case
+// trailing dashes are allowed.
+func ValidateNodeName(name string, prefix bool) (bool, string) {
+	return nameIsDNSSubdomain(name, prefix)
+}
+
 // nameIsDNSSubdomain is a ValidateNameFunc for names that must be a DNS subdomain.
 func nameIsDNSSubdomain(name string, prefix bool) (bool, string) {
 	if prefix {
@@ -510,7 +539,7 @@ func validateDNSPolicy(dnsPolicy *api.DNSPolicy) errs.ValidationErrorList {
 // ValidatePod tests if required fields in the pod are set.
 func ValidatePod(pod *api.Pod) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
-	allErrs = append(allErrs, ValidateObjectMeta(&pod.ObjectMeta, true, nameIsDNSSubdomain).Prefix("metadata")...)
+	allErrs = append(allErrs, ValidateObjectMeta(&pod.ObjectMeta, true, ValidatePodName).Prefix("metadata")...)
 	allErrs = append(allErrs, ValidatePodSpec(&pod.Spec).Prefix("spec")...)
 
 	return allErrs
@@ -563,7 +592,7 @@ var supportedSessionAffinityType = util.NewStringSet(string(api.AffinityTypeClie
 // ValidateService tests if required fields in the service are set.
 func ValidateService(service *api.Service) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
-	allErrs = append(allErrs, ValidateObjectMeta(&service.ObjectMeta, true, nameIsDNS952Label).Prefix("metadata")...)
+	allErrs = append(allErrs, ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName).Prefix("metadata")...)
 
 	if !util.IsValidPortNum(service.Spec.Port) {
 		allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, ""))
@@ -604,7 +633,7 @@ func ValidateServiceUpdate(oldService, service *api.Service) errs.ValidationErro
 // ValidateReplicationController tests if required fields in the replication controller are set.
 func ValidateReplicationController(controller *api.ReplicationController) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
-	allErrs = append(allErrs, ValidateObjectMeta(&controller.ObjectMeta, true, nameIsDNSSubdomain).Prefix("metadata")...)
+	allErrs = append(allErrs, ValidateObjectMeta(&controller.ObjectMeta, true, ValidateReplicationControllerName).Prefix("metadata")...)
 	allErrs = append(allErrs, ValidateReplicationControllerSpec(&controller.Spec).Prefix("spec")...)
 
 	return allErrs
@@ -694,7 +723,7 @@ func ValidateBoundPod(pod *api.BoundPod) errs.ValidationErrorList {
 // ValidateMinion tests if required fields in the node are set.
 func ValidateMinion(node *api.Node) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
-	allErrs = append(allErrs, ValidateObjectMeta(&node.ObjectMeta, false, nameIsDNSSubdomain).Prefix("metadata")...)
+	allErrs = append(allErrs, ValidateObjectMeta(&node.ObjectMeta, false, ValidateNodeName).Prefix("metadata")...)
 	return allErrs
 }
 

pkg/controller/replication_controller.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -60,9 +61,17 @@ func (r RealPodControl) createReplica(namespace string, controller api.Replicati
 	for k, v := range controller.Spec.Template.Labels {
 		desiredLabels[k] = v
 	}
+
+	// use the dash (if the name isn't too long) to make the pod name a bit prettier
+	prefix := fmt.Sprintf("%s-", controller.Name)
+	if ok, _ := validation.ValidatePodName(prefix, true); !ok {
+		prefix = controller.Name
+	}
+
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{
-			Labels: desiredLabels,
+			Labels:       desiredLabels,
+			GenerateName: prefix,
 		},
 	}
 	if err := api.Scheme.Convert(&controller.Spec.Template.Spec, &pod.Spec); err != nil {

pkg/controller/replication_controller_test.go

@@ -229,7 +229,8 @@ func TestCreateReplica(t *testing.T) {
 
 	expectedPod := api.Pod{
 		ObjectMeta: api.ObjectMeta{
-			Labels: controllerSpec.Spec.Template.Labels,
+			Labels:       controllerSpec.Spec.Template.Labels,
+			GenerateName: fmt.Sprintf("%s-", controllerSpec.Name),
 		},
 		Spec: controllerSpec.Spec.Template.Spec,
 	}

pkg/registry/pod/rest.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
@@ -56,18 +57,11 @@ func NewREST(config *RESTConfig) *REST {
 
 func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) {
 	pod := obj.(*api.Pod)
-	if !api.ValidNamespace(ctx, &pod.ObjectMeta) {
-		return nil, errors.NewConflict("pod", pod.Namespace, fmt.Errorf("Pod.Namespace does not match the provided context"))
-	}
-	api.FillObjectMetaSystemFields(ctx, &pod.ObjectMeta)
-	if len(pod.Name) == 0 {
-		// TODO properly handle auto-generated names.
-		// See https://github.com/GoogleCloudPlatform/kubernetes/issues/148 170 & 1135
-		pod.Name = string(pod.UID)
-	}
-	if errs := validation.ValidatePod(pod); len(errs) > 0 {
-		return nil, errors.NewInvalid("pod", pod.Name, errs)
+
+	if err := rest.BeforeCreate(rest.Pods, ctx, obj); err != nil {
+		return nil, err
 	}
+
 	return apiserver.MakeAsync(func() (runtime.Object, error) {
 		if err := rs.registry.CreatePod(ctx, pod); err != nil {
 			return nil, err

pkg/registry/pod/rest_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@@ -83,11 +84,15 @@ func TestCreatePodRegistryError(t *testing.T) {
 		registry: podRegistry,
 		podCache: &fakeCache{statusToReturn: &api.PodStatus{}},
 	}
-	pod := &api.Pod{}
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name: "foo",
+		},
+	}
 	ctx := api.NewDefaultContext()
 	ch, err := storage.Create(ctx, pod)
 	if err != nil {
-		t.Errorf("Expected %#v, Got %#v", nil, err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 	expectApiStatusError(t, ch, podRegistry.Err.Error())
 }
@@ -99,11 +104,15 @@ func TestCreatePodSetsIds(t *testing.T) {
 		registry: podRegistry,
 		podCache: &fakeCache{statusToReturn: &api.PodStatus{}},
 	}
-	pod := &api.Pod{}
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name: "foo",
+		},
+	}
 	ctx := api.NewDefaultContext()
 	ch, err := storage.Create(ctx, pod)
 	if err != nil {
-		t.Errorf("Expected %#v, Got %#v", nil, err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 	expectApiStatusError(t, ch, podRegistry.Err.Error())
 
@@ -122,11 +131,15 @@ func TestCreatePodSetsUID(t *testing.T) {
 		registry: podRegistry,
 		podCache: &fakeCache{statusToReturn: &api.PodStatus{}},
 	}
-	pod := &api.Pod{}
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name: "foo",
+		},
+	}
 	ctx := api.NewDefaultContext()
 	ch, err := storage.Create(ctx, pod)
 	if err != nil {
-		t.Errorf("Expected %#v, Got %#v", nil, err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 	expectApiStatusError(t, ch, podRegistry.Err.Error())
 
@@ -190,7 +203,7 @@ func TestListEmptyPodList(t *testing.T) {
 	ctx := api.NewContext()
 	pods, err := storage.List(ctx, labels.Everything(), labels.Everything())
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	if len(pods.(*api.PodList).Items) != 0 {
@@ -225,7 +238,7 @@ func TestListPodList(t *testing.T) {
 	podsObj, err := storage.List(ctx, labels.Everything(), labels.Everything())
 	pods := podsObj.(*api.PodList)
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	if len(pods.Items) != 2 {
@@ -336,12 +349,12 @@ func TestPodDecode(t *testing.T) {
 	}
 	body, err := latest.Codec.Encode(expected)
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	actual := storage.New()
 	if err := latest.Codec.DecodeInto(body, actual); err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	if !reflect.DeepEqual(expected, actual) {
@@ -363,7 +376,7 @@ func TestGetPod(t *testing.T) {
 	obj, err := storage.Get(ctx, "foo")
 	pod := obj.(*api.Pod)
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	expect := *podRegistry.Pod
@@ -386,7 +399,7 @@ func TestGetPodCacheError(t *testing.T) {
 	obj, err := storage.Get(ctx, "foo")
 	pod := obj.(*api.Pod)
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	expect := *podRegistry.Pod
@@ -396,6 +409,7 @@ func TestGetPodCacheError(t *testing.T) {
 	}
 }
 
+// TODO: remove, this is covered by RESTTest.TestCreate
 func TestPodStorageValidatesCreate(t *testing.T) {
 	podRegistry := registrytest.NewPodRegistry(nil)
 	podRegistry.Err = fmt.Errorf("test error")
@@ -420,6 +434,7 @@ func TestPodStorageValidatesCreate(t *testing.T) {
 	}
 }
 
+// TODO: remove, this is covered by RESTTest.TestCreate
 func TestCreatePod(t *testing.T) {
 	podRegistry := registrytest.NewPodRegistry(nil)
 	podRegistry.Pod = &api.Pod{
@@ -437,7 +452,7 @@ func TestCreatePod(t *testing.T) {
 	ctx := api.NewDefaultContext()
 	channel, err := storage.Create(ctx, pod)
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 	select {
 	case <-channel:
@@ -450,6 +465,7 @@ func TestCreatePod(t *testing.T) {
 	}
 }
 
+// TODO: remove, this is covered by RESTTest.TestCreate
 func TestCreatePodWithConflictingNamespace(t *testing.T) {
 	storage := REST{}
 	pod := &api.Pod{
@@ -463,7 +479,7 @@ func TestCreatePodWithConflictingNamespace(t *testing.T) {
 	}
 	if err == nil {
 		t.Errorf("Expected an error, but we didn't get one")
-	} else if strings.Index(err.Error(), "Pod.Namespace does not match the provided context") == -1 {
+	} else if strings.Contains(err.Error(), "Controller.Namespace does not match the provided context") {
 		t.Errorf("Expected 'Pod.Namespace does not match the provided context' error, got '%v'", err.Error())
 	}
 }
@@ -605,7 +621,7 @@ func TestDeletePod(t *testing.T) {
 	ctx := api.NewDefaultContext()
 	channel, err := storage.Delete(ctx, "foo")
 	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 	var result apiserver.RESTResult
 	select {
@@ -618,3 +634,29 @@ func TestDeletePod(t *testing.T) {
 		t.Errorf("Unexpeceted cache delete: %s %s %#v", fakeCache.clearedName, fakeCache.clearedNamespace, result.Object)
 	}
 }
+
+func TestCreate(t *testing.T) {
+	test := resttest.New(t, &REST{
+		registry: registrytest.NewPodRegistry(nil),
+		podCache: &fakeCache{statusToReturn: &api.PodStatus{}},
+	})
+	test.TestCreate(
+		// valid
+		&api.Pod{
+			Spec: api.PodSpec{
+				Containers: []api.Container{
+					{
+						Name:  "test1",
+						Image: "foo",
+					},
+				},
+			},
+		},
+		// invalid
+		&api.Pod{
+			Spec: api.PodSpec{
+				Containers: []api.Container{},
+			},
+		},
+	)
+}

test/integration/client_test.go

@@ -87,6 +87,9 @@ func TestClient(t *testing.T) {
 
 		// get a validation error
 		pod := &api.Pod{
+			ObjectMeta: api.ObjectMeta{
+				GenerateName: "test",
+			},
 			Spec: api.PodSpec{
 				Containers: []api.Container{
 					{