From fded23a777743a7fb9b78aac12d088a07f7bcb57 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Wed, 8 Apr 2015 22:13:59 -0700 Subject: [PATCH] Default replica controller nodeSelector using pod template. Add default labels from pod if not present. --- pkg/api/v1beta1/defaults.go | 8 ++ pkg/api/v1beta1/defaults_test.go | 96 +++++++++++++++ pkg/api/v1beta2/defaults.go | 8 ++ pkg/api/v1beta2/defaults_test.go | 96 +++++++++++++++ pkg/api/v1beta3/defaults.go | 15 +++ pkg/api/v1beta3/defaults_test.go | 109 ++++++++++++++++++ pkg/api/v1beta3/types.go | 6 +- pkg/controller/replication_controller_test.go | 4 + pkg/registry/controller/etcd/etcd_test.go | 4 +- 9 files changed, 341 insertions(+), 5 deletions(-) diff --git a/pkg/api/v1beta1/defaults.go b/pkg/api/v1beta1/defaults.go index f1c79530234..4d8a9b9cdfe 100644 --- a/pkg/api/v1beta1/defaults.go +++ b/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{ diff --git a/pkg/api/v1beta1/defaults_test.go b/pkg/api/v1beta1/defaults_test.go index ad79d0afd2a..3f304d1a512 100644 --- a/pkg/api/v1beta1/defaults_test.go +++ b/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: ¤t.ReplicationController{ + DesiredState: current.ReplicationControllerState{ + PodTemplate: current.PodTemplate{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + expectLabels: true, + expectSelector: true, + }, + { + rc: ¤t.ReplicationController{ + Labels: map[string]string{ + "bar": "foo", + }, + DesiredState: current.ReplicationControllerState{ + PodTemplate: current.PodTemplate{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + expectLabels: false, + expectSelector: true, + }, + { + rc: ¤t.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: ¤t.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 := ¤t.Service{} obj2 := roundTrip(t, runtime.Object(svc)) diff --git a/pkg/api/v1beta2/defaults.go b/pkg/api/v1beta2/defaults.go index 7b3846c72d9..27516e4b61a 100644 --- a/pkg/api/v1beta2/defaults.go +++ b/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) diff --git a/pkg/api/v1beta2/defaults_test.go b/pkg/api/v1beta2/defaults_test.go index 687aafcd9d2..112a362c9e2 100644 --- a/pkg/api/v1beta2/defaults_test.go +++ b/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: ¤t.ReplicationController{ + DesiredState: current.ReplicationControllerState{ + PodTemplate: current.PodTemplate{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + expectLabels: true, + expectSelector: true, + }, + { + rc: ¤t.ReplicationController{ + Labels: map[string]string{ + "bar": "foo", + }, + DesiredState: current.ReplicationControllerState{ + PodTemplate: current.PodTemplate{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + expectLabels: false, + expectSelector: true, + }, + { + rc: ¤t.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: ¤t.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 := ¤t.Service{} obj2 := roundTrip(t, runtime.Object(svc)) diff --git a/pkg/api/v1beta3/defaults.go b/pkg/api/v1beta3/defaults.go index 166e53b738c..a5a21b61a21 100644 --- a/pkg/api/v1beta3/defaults.go +++ b/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{ diff --git a/pkg/api/v1beta3/defaults_test.go b/pkg/api/v1beta3/defaults_test.go index ae99f3fe220..1d5e07b89fc 100644 --- a/pkg/api/v1beta3/defaults_test.go +++ b/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: ¤t.ReplicationController{ + Spec: current.ReplicationControllerSpec{ + Template: ¤t.PodTemplateSpec{ + ObjectMeta: current.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + expectLabels: true, + expectSelector: true, + }, + { + rc: ¤t.ReplicationController{ + ObjectMeta: current.ObjectMeta{ + Labels: map[string]string{ + "bar": "foo", + }, + }, + Spec: current.ReplicationControllerSpec{ + Template: ¤t.PodTemplateSpec{ + ObjectMeta: current.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + expectLabels: false, + expectSelector: true, + }, + { + rc: ¤t.ReplicationController{ + ObjectMeta: current.ObjectMeta{ + Labels: map[string]string{ + "bar": "foo", + }, + }, + Spec: current.ReplicationControllerSpec{ + Selector: map[string]string{ + "some": "other", + }, + Template: ¤t.PodTemplateSpec{ + ObjectMeta: current.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + expectLabels: false, + expectSelector: false, + }, + { + rc: ¤t.ReplicationController{ + Spec: current.ReplicationControllerSpec{ + Selector: map[string]string{ + "some": "other", + }, + Template: ¤t.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 := ¤t.Service{} obj2 := roundTrip(t, runtime.Object(svc)) diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 3acbbc17ce9..355b40731eb 100644 --- a/pkg/api/v1beta3/types.go +++ b/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. diff --git a/pkg/controller/replication_controller_test.go b/pkg/controller/replication_controller_test.go index ad07ef247b1..8adb7094ba5 100644 --- a/pkg/controller/replication_controller_test.go +++ b/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) diff --git a/pkg/registry/controller/etcd/etcd_test.go b/pkg/registry/controller/etcd/etcd_test.go index 57b6e9f79bb..f1f3b48bd30 100644 --- a/pkg/registry/controller/etcd/etcd_test.go +++ b/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 {