Add an rc strategy, start delta validating updates

duda · Mar 13, 2015 · 6193608 · 6193608
1 parent df0838e
commit 6193608
Show file tree

Hide file tree

Showing 5 changed files with 192 additions and 145 deletions.
diff --git a/pkg/api/rest/create_test.go b/pkg/api/rest/create_test.go
@@ -17,12 +17,10 @@ limitations under the License.
 package rest
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 )
 
 func TestCheckGeneratedNameError(t *testing.T) {
@@ -41,75 +39,3 @@ func TestCheckGeneratedNameError(t *testing.T) {
 		t.Errorf("expected try again later error: %v", err)
 	}
 }
-
-func TestBeforeCreate(t *testing.T) {
-	failures := []runtime.Object{
-		&api.Service{},
-		&api.Service{
-			ObjectMeta: api.ObjectMeta{
-				Name:      "foo",
-				Namespace: "#$%%invalid",
-			},
-		},
-		&api.Service{
-			ObjectMeta: api.ObjectMeta{
-				Name:      "##&*(&invalid",
-				Namespace: api.NamespaceDefault,
-			},
-		},
-	}
-	for _, test := range failures {
-		ctx := api.NewDefaultContext()
-		err := BeforeCreate(Services, ctx, test)
-		if err == nil {
-			t.Errorf("unexpected non-error for %v", test)
-		}
-	}
-
-	obj := &api.ReplicationController{
-		ObjectMeta: api.ObjectMeta{
-			Name:      "foo",
-			Namespace: api.NamespaceDefault,
-		},
-		Spec: api.ReplicationControllerSpec{
-			Selector: map[string]string{"name": "foo"},
-			Template: &api.PodTemplateSpec{
-				ObjectMeta: api.ObjectMeta{
-					Labels: map[string]string{
-						"name": "foo",
-					},
-				},
-				Spec: api.PodSpec{
-					Containers: []api.Container{
-						{
-							Name:            "foo",
-							Image:           "foo",
-							ImagePullPolicy: api.PullAlways,
-						},
-					},
-					RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
-					DNSPolicy:     api.DNSDefault,
-				},
-			},
-		},
-		Status: api.ReplicationControllerStatus{
-			Replicas: 3,
-		},
-	}
-	ctx := api.NewDefaultContext()
-	err := BeforeCreate(ReplicationControllers, ctx, obj)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if !reflect.DeepEqual(obj.Status, api.ReplicationControllerStatus{}) {
-		t.Errorf("status was not cleared as expected.")
-	}
-	if obj.Name != "foo" || obj.Namespace != api.NamespaceDefault {
-		t.Errorf("unexpected object metadata: %v", obj.ObjectMeta)
-	}
-
-	obj.Spec.Replicas = -1
-	if err := BeforeCreate(ReplicationControllers, ctx, obj); err == nil {
-		t.Errorf("unexpected non-error for invalid replication controller.")
-	}
-}
diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go
@@ -44,34 +44,6 @@ func AllFuncs(fns ...ObjectFunc) ObjectFunc {
 	}
 }
 
-// rcStrategy implements behavior for Replication Controllers.
-// TODO: move to a replicationcontroller specific package.
-type rcStrategy struct {
-	runtime.ObjectTyper
-	api.NameGenerator
-}
-
-// ReplicationControllers is the default logic that applies when creating and updating Replication Controller
-// objects.
-var ReplicationControllers RESTCreateStrategy = rcStrategy{api.Scheme, api.SimpleNameGenerator}
-
-// NamespaceScoped is true for replication controllers.
-func (rcStrategy) NamespaceScoped() bool {
-	return true
-}
-
-// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation.
-func (rcStrategy) ResetBeforeCreate(obj runtime.Object) {
-	controller := obj.(*api.ReplicationController)
-	controller.Status = api.ReplicationControllerStatus{}
-}
-
-// Validate validates a new replication controller.
-func (rcStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
-	controller := obj.(*api.ReplicationController)
-	return validation.ValidateReplicationController(controller)
-}
-
 // svcStrategy implements behavior for Services
 // TODO: move to a service specific package.
 type svcStrategy struct {

diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go
@@ -29,6 +29,43 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
 )
 
+// rcStrategy implements verification logic for Replication Controllers.
+type rcStrategy struct {
+	runtime.ObjectTyper
+	api.NameGenerator
+}
+
+// Strategy is the default logic that applies when creating and updating Replication Controller objects.
+var Strategy = rcStrategy{api.Scheme, api.SimpleNameGenerator}
+
+// NamespaceScoped returns true because all Replication Controllers need to be within a namespace.
+func (rcStrategy) NamespaceScoped() bool {
+	return true
+}
+
+// ResetBeforeCreate clears the status of a replication controller before creation.
+func (rcStrategy) ResetBeforeCreate(obj runtime.Object) {
+	controller := obj.(*api.ReplicationController)
+	controller.Status = api.ReplicationControllerStatus{}
+}
+
+// Validate validates a new replication controller.
+func (rcStrategy) Validate(obj runtime.Object) errors.ValidationErrorList {
+	controller := obj.(*api.ReplicationController)
+	return validation.ValidateReplicationController(controller)
+}
+
+// AllowCreateOnUpdate is false for replication controllers; this means a POST is
+// needed to create one.
+func (rcStrategy) AllowCreateOnUpdate() bool {
+	return false
+}
+
+// ValidateUpdate is the default update validation for an end user.
+func (rcStrategy) ValidateUpdate(obj, old runtime.Object) errors.ValidationErrorList {
+	return validation.ValidateReplicationControllerUpdate(old.(*api.ReplicationController), obj.(*api.ReplicationController))
+}
+
 // PodLister is anything that knows how to list pods.
 type PodLister interface {
 	ListPods(ctx api.Context, labels labels.Selector) (*api.PodList, error)
@@ -38,30 +75,33 @@ type PodLister interface {
 type REST struct {
 	registry  Registry
 	podLister PodLister
+	strategy  rcStrategy
 }
 
 // NewREST returns a new apiserver.RESTStorage for the given registry and PodLister.
 func NewREST(registry Registry, podLister PodLister) *REST {
 	return &REST{
 		registry:  registry,
 		podLister: podLister,
+		strategy:  Strategy,
 	}
 }
 
-// Create registers the given ReplicationController.
+// Create registers the given ReplicationController with the system,
+// which eventually leads to the controller manager acting on its behalf.
 func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, error) {
 	controller, ok := obj.(*api.ReplicationController)
 	if !ok {
 		return nil, fmt.Errorf("not a replication controller: %#v", obj)
 	}
 
-	if err := rest.BeforeCreate(rest.ReplicationControllers, ctx, obj); err != nil {
+	if err := rest.BeforeCreate(rs.strategy, ctx, obj); err != nil {
 		return nil, err
 	}
 
 	out, err := rs.registry.CreateController(ctx, controller)
 	if err != nil {
-		err = rest.CheckGeneratedNameError(rest.ReplicationControllers, err, controller)
+		err = rest.CheckGeneratedNameError(rs.strategy, err, controller)
 	}
 	return out, err
 }
@@ -115,11 +155,12 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, boo
 	if !ok {
 		return nil, false, fmt.Errorf("not a replication controller: %#v", obj)
 	}
-	if !api.ValidNamespace(ctx, &controller.ObjectMeta) {
-		return nil, false, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context"))
+	existingController, err := rs.registry.GetController(ctx, controller.Name)
+	if err != nil {
+		return nil, false, err
 	}
-	if errs := validation.ValidateReplicationController(controller); len(errs) > 0 {
-		return nil, false, errors.NewInvalid("replicationController", controller.Name, errs)
+	if err := rest.BeforeUpdate(rs.strategy, ctx, controller, existingController); err != nil {
+		return nil, false, err
 	}
 	out, err := rs.registry.UpdateController(ctx, controller)
 	return out, false, err