Skip to content

K
k3s
Commit 6193608e authored by Prashanth Balasubramanian's avatar Prashanth Balasubramanian
Browse files
Options

Add an rc strategy, start delta validating updates

parent df0838ed
Hide whitespace changes
Inline Side-by-side
Showing with 192 additions and 145 deletions
pkg/api/rest/create_test.go
View file @ 6193608e
......@@ -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.")
}
}
pkg/api/rest/types.go
View file @ 6193608e
......@@ -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 {
......
pkg/registry/controller/rest.go
View file @ 6193608e
......@@ -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,6 +75,7 @@ type PodLister interface {
type REST struct {
registry Registry
podLister PodLister
strategy rcStrategy
}
// NewREST returns a new apiserver.RESTStorage for the given registry and PodLister.
......@@ -45,23 +83,25 @@ 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
......
pkg/registry/controller/rest_test.go
View file @ 6193608e
......@@ -20,17 +20,20 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"reflect"
"strings"
"testing"
"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"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest"
_ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
)
func TestListControllersError(t *testing.T) {
......@@ -258,6 +261,7 @@ func TestCreateController(t *testing.T) {
storage := REST{
registry: &mockRegistry,
podLister: &mockPodRegistry,
strategy: Strategy,
}
controller := &api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "test"},
......@@ -287,6 +291,7 @@ func TestControllerStorageValidatesCreate(t *testing.T) {
storage := REST{
registry: &mockRegistry,
podLister: nil,
strategy: Strategy,
}
failureCases := map[string]api.ReplicationController{
"empty ID": {
......@@ -312,34 +317,71 @@ func TestControllerStorageValidatesCreate(t *testing.T) {
}
}
func TestControllerStorageValidatesUpdate(t *testing.T) {
mockRegistry := registrytest.ControllerRegistry{}
func TestControllerValidatesUpdate(t *testing.T) {
mockRegistry := registrytest.ControllerRegistry{
Controllers: &api.ReplicationControllerList{},
}
storage := REST{
registry: &mockRegistry,
podLister: nil,
strategy: Strategy,
}
failureCases := map[string]api.ReplicationController{
"empty ID": {
ObjectMeta: api.ObjectMeta{Name: ""},
Spec: api.ReplicationControllerSpec{
Selector: map[string]string{"bar": "baz"},
},
var validControllerSpec = api.ReplicationControllerSpec{
Selector: validPodTemplate.Spec.Labels,
Template: &validPodTemplate.Spec,
}
var validController = api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: validControllerSpec,
}
ctx := api.NewDefaultContext()
storage.Create(ctx, &validController)
ns := "newNamespace"
updaters := []func(rc api.ReplicationController) (runtime.Object, bool, error){
func(rc api.ReplicationController) (runtime.Object, bool, error) {
rc.UID = "newUID"
return storage.Update(ctx, &rc)
},
"empty selector": {
ObjectMeta: api.ObjectMeta{Name: "abc"},
Spec: api.ReplicationControllerSpec{},
func(rc api.ReplicationController) (runtime.Object, bool, error) {
rc.Name = ""
return storage.Update(ctx, &rc)
},
func(rc api.ReplicationController) (runtime.Object, bool, error) {
rc.Namespace = ns
return storage.Update(api.WithNamespace(ctx, ns), &rc)
},
func(rc api.ReplicationController) (runtime.Object, bool, error) {
rc.Spec.Selector = map[string]string{}
return storage.Update(ctx, &rc)
},
}
ctx := api.NewDefaultContext()
for _, failureCase := range failureCases {
c, created, err := storage.Update(ctx, &failureCase)
if c != nil || created {
for _, u := range updaters {
c, updated, err := u(validController)
if c != nil || updated {
t.Errorf("Expected nil object and not created")
}
if !errors.IsInvalid(err) {
t.Errorf("Expected to get an invalid resource error, got %v", err)
}
}
// The update should fail if the namespace on the controller is set to something
// other than the namespace on the given context, even if the namespace on the
// controller is valid.
c, updated, err := storage.Update(api.WithNamespace(ctx, ns), &validController)
if c != nil || updated {
t.Errorf("Expected nil object and not created")
}
errSubString := "namespace"
if err == nil {
t.Errorf("Expected an error, but we didn't get one")
} else if !errors.IsBadRequest(err) ||
strings.Index(err.Error(), errSubString) == -1 {
t.Errorf("Expected a Bad Request error with the sub string '%s', got %v", errSubString, err)
}
}
type fakePodLister struct {
......@@ -380,7 +422,7 @@ func TestCreateControllerWithGeneratedName(t *testing.T) {
// TODO: remove, covered by TestCreate
func TestCreateControllerWithConflictingNamespace(t *testing.T) {
storage := REST{}
storage := REST{strategy: Strategy}
controller := &api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "not-default"},
}
......@@ -390,28 +432,12 @@ func TestCreateControllerWithConflictingNamespace(t *testing.T) {
if channel != nil {
t.Error("Expected a nil channel, but we got a value")
}
errSubString := "namespace"
if err == nil {
t.Errorf("Expected an error, but we didn't get one")
} else if strings.Contains(err.Error(), "Controller.Namespace does not match the provided context") {
t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error())
}
}
func TestUpdateControllerWithConflictingNamespace(t *testing.T) {
storage := REST{}
controller := &api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "not-default"},
}
ctx := api.NewDefaultContext()
obj, created, err := storage.Update(ctx, controller)
if obj != nil || created {
t.Error("Expected a nil object, but we got a value or created was true")
}
if err == nil {
t.Errorf("Expected an error, but we didn't get one")
} else if strings.Index(err.Error(), "Controller.Namespace does not match the provided context") == -1 {
t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error())
} else if !errors.IsBadRequest(err) ||
strings.Index(err.Error(), errSubString) == -1 {
t.Errorf("Expected a Bad Request error with the sub string '%s', got %v", errSubString, err)
}
}
......@@ -437,3 +463,75 @@ func TestCreate(t *testing.T) {
},
)
}
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 := rest.BeforeCreate(rest.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 := rest.BeforeCreate(Strategy, 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 := rest.BeforeCreate(Strategy, ctx, obj); err == nil {
t.Errorf("unexpected non-error for invalid replication controller.")
}
}
pkg/registry/registrytest/controller.go
View file @ 6193608e
......@@ -47,12 +47,22 @@ func (r *ControllerRegistry) ListControllers(ctx api.Context) (*api.ReplicationC
func (r *ControllerRegistry) GetController(ctx api.Context, ID string) (*api.ReplicationController, error) {
r.Lock()
defer r.Unlock()
if r.Controllers != nil {
for _, rc := range r.Controllers.Items {
if ID == rc.Name {
return &r.Controllers.Items[0], r.Err
}
}
}
return &api.ReplicationController{}, r.Err
}
func (r *ControllerRegistry) CreateController(ctx api.Context, controller *api.ReplicationController) (*api.ReplicationController, error) {
r.Lock()
defer r.Unlock()
if r.Controllers != nil {
r.Controllers.Items = append(r.Controllers.Items, *controller)
}
return controller, r.Err
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment