Merge pull request #6616 from brendandburns/default

pkg/api/v1beta1/defaults.go

@@ -28,6 +28,14 @@ import (
 
 func init() {
 	api.Scheme.AddDefaultingFuncs(
+		func(obj *ReplicationController) {
+			if len(obj.DesiredState.ReplicaSelector) == 0 {
+				obj.DesiredState.ReplicaSelector = obj.DesiredState.PodTemplate.Labels
+			}
+			if len(obj.Labels) == 0 {
+				obj.Labels = obj.DesiredState.PodTemplate.Labels
+			}
+		},
 		func(obj *Volume) {
 			if util.AllPtrFieldsNil(&obj.Source) {
 				obj.Source = VolumeSource{

pkg/api/v1beta1/defaults_test.go

@@ -46,6 +46,102 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
 	return obj3
 }
 
+func TestSetDefaultReplicationController(t *testing.T) {
+	tests := []struct {
+		rc             *current.ReplicationController
+		expectLabels   bool
+		expectSelector bool
+	}{
+		{
+			rc: &current.ReplicationController{
+				DesiredState: current.ReplicationControllerState{
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   true,
+			expectSelector: true,
+		},
+		{
+			rc: &current.ReplicationController{
+				Labels: map[string]string{
+					"bar": "foo",
+				},
+				DesiredState: current.ReplicationControllerState{
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   false,
+			expectSelector: true,
+		},
+		{
+			rc: &current.ReplicationController{
+				Labels: map[string]string{
+					"bar": "foo",
+				},
+				DesiredState: current.ReplicationControllerState{
+					ReplicaSelector: map[string]string{
+						"some": "other",
+					},
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   false,
+			expectSelector: false,
+		},
+		{
+			rc: &current.ReplicationController{
+				DesiredState: current.ReplicationControllerState{
+					ReplicaSelector: map[string]string{
+						"some": "other",
+					},
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   true,
+			expectSelector: false,
+		},
+	}
+	for _, test := range tests {
+		rc := test.rc
+		obj2 := roundTrip(t, runtime.Object(rc))
+		rc2, ok := obj2.(*current.ReplicationController)
+		if !ok {
+			t.Errorf("unexpected object: %v", rc2)
+			t.FailNow()
+		}
+		if test.expectSelector != reflect.DeepEqual(rc2.DesiredState.ReplicaSelector, rc2.DesiredState.PodTemplate.Labels) {
+			if test.expectSelector {
+				t.Errorf("expected: %v, got: %v", rc2.DesiredState.PodTemplate.Labels, rc2.DesiredState.ReplicaSelector)
+			} else {
+				t.Errorf("unexpected equality: %v", rc2.DesiredState.PodTemplate.Labels)
+			}
+		}
+		if test.expectLabels != reflect.DeepEqual(rc2.Labels, rc2.DesiredState.PodTemplate.Labels) {
+			if test.expectLabels {
+				t.Errorf("expected: %v, got: %v", rc2.DesiredState.PodTemplate.Labels, rc2.Labels)
+			} else {
+				t.Errorf("unexpected equality: %v", rc2.DesiredState.PodTemplate.Labels)
+			}
+		}
+	}
+}
+
 func TestSetDefaultService(t *testing.T) {
 	svc := &current.Service{}
 	obj2 := roundTrip(t, runtime.Object(svc))

pkg/api/v1beta2/defaults.go

@@ -28,6 +28,14 @@ import (
 
 func init() {
 	api.Scheme.AddDefaultingFuncs(
+		func(obj *ReplicationController) {
+			if len(obj.DesiredState.ReplicaSelector) == 0 {
+				obj.DesiredState.ReplicaSelector = obj.DesiredState.PodTemplate.Labels
+			}
+			if len(obj.Labels) == 0 {
+				obj.Labels = obj.DesiredState.PodTemplate.Labels
+			}
+		},
 		func(obj *Volume) {
 			if util.AllPtrFieldsNil(&obj.Source) {
 				glog.Errorf("Defaulting volume source for %v", obj)

pkg/api/v1beta2/defaults_test.go

@@ -46,6 +46,102 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
 	return obj3
 }
 
+func TestSetDefaultReplicationController(t *testing.T) {
+	tests := []struct {
+		rc             *current.ReplicationController
+		expectLabels   bool
+		expectSelector bool
+	}{
+		{
+			rc: &current.ReplicationController{
+				DesiredState: current.ReplicationControllerState{
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   true,
+			expectSelector: true,
+		},
+		{
+			rc: &current.ReplicationController{
+				Labels: map[string]string{
+					"bar": "foo",
+				},
+				DesiredState: current.ReplicationControllerState{
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   false,
+			expectSelector: true,
+		},
+		{
+			rc: &current.ReplicationController{
+				Labels: map[string]string{
+					"bar": "foo",
+				},
+				DesiredState: current.ReplicationControllerState{
+					ReplicaSelector: map[string]string{
+						"some": "other",
+					},
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   false,
+			expectSelector: false,
+		},
+		{
+			rc: &current.ReplicationController{
+				DesiredState: current.ReplicationControllerState{
+					ReplicaSelector: map[string]string{
+						"some": "other",
+					},
+					PodTemplate: current.PodTemplate{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+				},
+			},
+			expectLabels:   true,
+			expectSelector: false,
+		},
+	}
+	for _, test := range tests {
+		rc := test.rc
+		obj2 := roundTrip(t, runtime.Object(rc))
+		rc2, ok := obj2.(*current.ReplicationController)
+		if !ok {
+			t.Errorf("unexpected object: %v", rc2)
+			t.FailNow()
+		}
+		if test.expectSelector != reflect.DeepEqual(rc2.DesiredState.ReplicaSelector, rc2.DesiredState.PodTemplate.Labels) {
+			if test.expectSelector {
+				t.Errorf("expected: %v, got: %v", rc2.DesiredState.PodTemplate.Labels, rc2.DesiredState.ReplicaSelector)
+			} else {
+				t.Errorf("unexpected equality: %v", rc2.DesiredState.PodTemplate.Labels)
+			}
+		}
+		if test.expectLabels != reflect.DeepEqual(rc2.Labels, rc2.DesiredState.PodTemplate.Labels) {
+			if test.expectLabels {
+				t.Errorf("expected: %v, got: %v", rc2.DesiredState.PodTemplate.Labels, rc2.Labels)
+			} else {
+				t.Errorf("unexpected equality: %v", rc2.DesiredState.PodTemplate.Labels)
+			}
+		}
+	}
+}
+
 func TestSetDefaultService(t *testing.T) {
 	svc := &current.Service{}
 	obj2 := roundTrip(t, runtime.Object(svc))

pkg/api/v1beta3/defaults.go

@@ -25,6 +25,21 @@ import (
 
 func init() {
 	api.Scheme.AddDefaultingFuncs(
+		func(obj *ReplicationController) {
+			var labels map[string]string
+			if obj.Spec.Template != nil {
+				labels = obj.Spec.Template.Labels
+			}
+			// TODO: support templates defined elsewhere when we support them in the API
+			if labels != nil {
+				if len(obj.Spec.Selector) == 0 {
+					obj.Spec.Selector = labels
+				}
+				if len(obj.Labels) == 0 {
+					obj.Labels = labels
+				}
+			}
+		},
 		func(obj *Volume) {
 			if util.AllPtrFieldsNil(&obj.VolumeSource) {
 				obj.VolumeSource = VolumeSource{

pkg/api/v1beta3/defaults_test.go

@@ -46,6 +46,115 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
 	return obj3
 }
 
+func TestSetDefaultReplicationController(t *testing.T) {
+	tests := []struct {
+		rc             *current.ReplicationController
+		expectLabels   bool
+		expectSelector bool
+	}{
+		{
+			rc: &current.ReplicationController{
+				Spec: current.ReplicationControllerSpec{
+					Template: &current.PodTemplateSpec{
+						ObjectMeta: current.ObjectMeta{
+							Labels: map[string]string{
+								"foo": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectLabels:   true,
+			expectSelector: true,
+		},
+		{
+			rc: &current.ReplicationController{
+				ObjectMeta: current.ObjectMeta{
+					Labels: map[string]string{
+						"bar": "foo",
+					},
+				},
+				Spec: current.ReplicationControllerSpec{
+					Template: &current.PodTemplateSpec{
+						ObjectMeta: current.ObjectMeta{
+							Labels: map[string]string{
+								"foo": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectLabels:   false,
+			expectSelector: true,
+		},
+		{
+			rc: &current.ReplicationController{
+				ObjectMeta: current.ObjectMeta{
+					Labels: map[string]string{
+						"bar": "foo",
+					},
+				},
+				Spec: current.ReplicationControllerSpec{
+					Selector: map[string]string{
+						"some": "other",
+					},
+					Template: &current.PodTemplateSpec{
+						ObjectMeta: current.ObjectMeta{
+							Labels: map[string]string{
+								"foo": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectLabels:   false,
+			expectSelector: false,
+		},
+		{
+			rc: &current.ReplicationController{
+				Spec: current.ReplicationControllerSpec{
+					Selector: map[string]string{
+						"some": "other",
+					},
+					Template: &current.PodTemplateSpec{
+						ObjectMeta: current.ObjectMeta{
+							Labels: map[string]string{
+								"foo": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectLabels:   true,
+			expectSelector: false,
+		},
+	}
+
+	for _, test := range tests {
+		rc := test.rc
+		obj2 := roundTrip(t, runtime.Object(rc))
+		rc2, ok := obj2.(*current.ReplicationController)
+		if !ok {
+			t.Errorf("unexpected object: %v", rc2)
+			t.FailNow()
+		}
+		if test.expectSelector != reflect.DeepEqual(rc2.Spec.Selector, rc2.Spec.Template.Labels) {
+			if test.expectSelector {
+				t.Errorf("expected: %v, got: %v", rc2.Spec.Template.Labels, rc2.Spec.Selector)
+			} else {
+				t.Errorf("unexpected equality: %v", rc.Spec.Selector)
+			}
+		}
+		if test.expectLabels != reflect.DeepEqual(rc2.Labels, rc2.Spec.Template.Labels) {
+			if test.expectLabels {
+				t.Errorf("expected: %v, got: %v", rc2.Spec.Template.Labels, rc2.Labels)
+			} else {
+				t.Errorf("unexpected equality: %v", rc.Labels)
+			}
+		}
+	}
+}
+
 func TestSetDefaultService(t *testing.T) {
 	svc := &current.Service{}
 	obj2 := roundTrip(t, runtime.Object(svc))

pkg/api/v1beta3/types.go

@@ -833,7 +833,8 @@ type ReplicationControllerSpec struct {
 	Replicas int `json:"replicas" description:"number of replicas desired"`
 
 	// Selector is a label query over pods that should match the Replicas count.
-	Selector map[string]string `json:"selector,omitempty" description:"label keys and values that must match in order to be controlled by this replication controller"`
+	// If Selector is empty, it is defaulted to the labels present on the Pod template.
+	Selector map[string]string `json:"selector,omitempty" description:"label keys and values that must match in order to be controlled by this replication controller, if empty defaulted to labels on Pod template"`
 
 	// TemplateRef is a reference to an object that describes the pod that will be created if
 	// insufficient replicas are detected.
@@ -854,7 +855,8 @@ type ReplicationControllerStatus struct {
 
 // ReplicationController represents the configuration of a replication controller.
 type ReplicationController struct {
-	TypeMeta   `json:",inline"`
+	TypeMeta `json:",inline"`
+	// If the Labels of a ReplicationController are empty, they are defaulted to be the same as the Pod(s) that the replication controller manages.
 	ObjectMeta `json:"metadata,omitempty" description:"standard object metadata; see https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/api-conventions.md#metadata"`
 
 	// Spec defines the desired behavior of this replication controller.

pkg/controller/replication_controller_test.go

@@ -333,6 +333,10 @@ func TestControllerUpdateReplicas(t *testing.T) {
 
 	// Status.Replicas should go up from 2->4 even though we created 5-4=1 pod
 	rc.Status = api.ReplicationControllerStatus{Replicas: 4}
+	// These are set by default.
+	rc.Spec.Selector = rc.Spec.Template.Labels
+	rc.Labels = rc.Spec.Template.Labels
+
 	decRc := runtime.EncodeOrDie(testapi.Codec(), &rc)
 	fakeUpdateHandler.ValidateRequest(t, testapi.ResourcePathWithQueryParams(replicationControllerResourceName(), rc.Namespace, rc.Name), "PUT", &decRc)
 	validateSyncReplication(t, &fakePodControl, 1, 0)

pkg/registry/controller/etcd/etcd_test.go

@@ -139,9 +139,7 @@ func TestEtcdCreateControllerValidates(t *testing.T) {
 	storage, _ := newStorage(t)
 	emptyName := validController
 	emptyName.Name = ""
-	emptySelector := validController
-	emptySelector.Spec.Selector = map[string]string{}
-	failureCases := []api.ReplicationController{emptyName, emptySelector}
+	failureCases := []api.ReplicationController{emptyName}
 	for _, failureCase := range failureCases {
 		c, err := storage.Create(ctx, &failureCase)
 		if c != nil {