diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 8e4908b7e83..dcd3610157b 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -73,7 +73,7 @@ func (fakeKubeletClient) GetPodStatus(host, podNamespace, podID string) (api.Pod Client: http.DefaultClient, Port: 10250, } - case "machine": + case "127.0.0.1": c = &client.HTTPKubeletClient{ Client: http.DefaultClient, Port: 10251, @@ -81,7 +81,18 @@ func (fakeKubeletClient) GetPodStatus(host, podNamespace, podID string) (api.Pod default: glog.Fatalf("Can't get info for: '%v', '%v - %v'", host, podNamespace, podID) } - return c.GetPodStatus("localhost", podNamespace, podID) + r, err := c.GetPodStatus("localhost", podNamespace, podID) + if err != nil { + return r, err + } + r.Status.PodIP = "1.2.3.4" + m := make(api.PodInfo) + for k, v := range r.Status.Info { + v.PodIP = "1.2.3.4" + m[k] = v + } + r.Status.Info = m + return r, nil } func (fakeKubeletClient) HealthCheck(host string) (probe.Status, error) { @@ -104,7 +115,7 @@ func startComponents(manifestURL string) (apiServerURL string) { // Setup servers := []string{"http://localhost:4001"} glog.Infof("Creating etcd client pointing to %v", servers) - machineList := []string{"localhost", "machine"} + machineList := []string{"localhost", "127.0.0.1"} handler := delegateHandler{} apiServer := httptest.NewServer(&handler) @@ -163,6 +174,7 @@ func startComponents(manifestURL string) (apiServerURL string) { ReadWritePort: portNumber, ReadOnlyPort: portNumber, PublicAddress: net.ParseIP(host), + CacheTimeout: 2 * time.Second, }) handler.delegate = m.Handler @@ -185,7 +197,7 @@ func startComponents(manifestURL string) (apiServerURL string) { nodeResources := &api.NodeResources{} nodeController := nodeControllerPkg.NewNodeController(nil, "", machineList, nodeResources, cl, fakeKubeletClient{}) - nodeController.Run(10*time.Second, 10) + nodeController.Run(5*time.Second, 10) // Kubelet (localhost) testRootDir := makeTempDirOrDie("kubelet_integ_1.") diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go index 930efe7dab2..e4345761ab1 100644 --- a/pkg/api/errors/errors.go +++ b/pkg/api/errors/errors.go @@ -174,6 +174,21 @@ func NewMethodNotSupported(kind, action string) error { }} } +// NewTryAgainLater returns an error indicating the requested action could not be completed due to a +// transient error, and the client should try again. +func NewTryAgainLater(kind, operation string) error { + return &StatusError{api.Status{ + Status: api.StatusFailure, + Code: http.StatusInternalServerError, + Reason: api.StatusReasonTryAgainLater, + Details: &api.StatusDetails{ + Kind: kind, + ID: operation, + }, + Message: fmt.Sprintf("The %s operation against %s could not be completed at this time, please try again.", operation, kind), + }} +} + // NewInternalError returns an error indicating the item is invalid and cannot be processed. func NewInternalError(err error) error { return &StatusError{api.Status{ @@ -218,6 +233,18 @@ func IsBadRequest(err error) bool { return reasonForError(err) == api.StatusReasonBadRequest } +// IsForbidden determines if err is an error which indicates that the request is forbidden and cannot +// be completed as requested. +func IsForbidden(err error) bool { + return reasonForError(err) == api.StatusReasonForbidden +} + +// IsTryAgainLater determines if err is an error which indicates that the request needs to be retried +// by the client. +func IsTryAgainLater(err error) bool { + return reasonForError(err) == api.StatusReasonTryAgainLater +} + func reasonForError(err error) api.StatusReason { switch t := err.(type) { case *StatusError: diff --git a/pkg/api/errors/errors_test.go b/pkg/api/errors/errors_test.go index d452b7cc7aa..a46583e25eb 100644 --- a/pkg/api/errors/errors_test.go +++ b/pkg/api/errors/errors_test.go @@ -43,6 +43,12 @@ func TestErrorNew(t *testing.T) { if IsBadRequest(err) { t.Errorf("expected to not be %s", api.StatusReasonBadRequest) } + if IsForbidden(err) { + t.Errorf("expected to not be %s", api.StatusReasonForbidden) + } + if IsTryAgainLater(err) { + t.Errorf("expected to not be %s", api.StatusReasonTryAgainLater) + } if IsMethodNotSupported(err) { t.Errorf("expected to not be %s", api.StatusReasonMethodNotAllowed) } @@ -59,6 +65,12 @@ func TestErrorNew(t *testing.T) { if !IsBadRequest(NewBadRequest("reason")) { t.Errorf("expected to be %s", api.StatusReasonBadRequest) } + if !IsForbidden(NewForbidden("test", "2", errors.New("reason"))) { + t.Errorf("expected to be %s", api.StatusReasonForbidden) + } + if !IsTryAgainLater(NewTryAgainLater("test", "reason")) { + t.Errorf("expected to be %s", api.StatusReasonTryAgainLater) + } if !IsMethodNotSupported(NewMethodNotSupported("foo", "delete")) { t.Errorf("expected to be %s", api.StatusReasonMethodNotAllowed) } diff --git a/pkg/api/generate.go b/pkg/api/generate.go new file mode 100644 index 00000000000..5447ff722d5 --- /dev/null +++ b/pkg/api/generate.go @@ -0,0 +1,74 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "fmt" + "math/rand" +) + +// NameGenerator generates names for objects. Some backends may have more information +// available to guide selection of new names and this interface hides those details. +type NameGenerator interface { + // GenerateName generates a valid name from the base name, adding a random suffix to the + // the base. If base is valid, the returned name must also be valid. The generator is + // responsible for knowing the maximum valid name length. + GenerateName(base string) string +} + +// GenerateName will resolve the object name of the provided ObjectMeta to a generated version if +// necessary. It expects that validation for ObjectMeta has already completed (that Base is a +// valid name) and that the NameGenerator generates a name that is also valid. +func GenerateName(u NameGenerator, meta *ObjectMeta) { + if len(meta.GenerateName) == 0 || len(meta.Name) != 0 { + return + } + meta.Name = u.GenerateName(meta.GenerateName) +} + +// simpleNameGenerator generates random names. +type simpleNameGenerator struct{} + +// SimpleNameGenerator is a generator that returns the name plus a random suffix of five alphanumerics +// when a name is requested. The string is guaranteed to not exceed the length of a standard Kubernetes +// name (63 characters) +var SimpleNameGenerator NameGenerator = simpleNameGenerator{} + +const ( + // TODO: make this flexible for non-core resources with alternate naming rules. + maxNameLength = 63 + randomLength = 5 + maxGeneratedNameLength = maxNameLength - randomLength +) + +func (simpleNameGenerator) GenerateName(base string) string { + if len(base) > maxGeneratedNameLength { + base = base[:maxGeneratedNameLength] + } + value := randSeq(randomLength) + return fmt.Sprintf("%s%s", base, value) +} + +var letters = []rune("abcdefghijklmnopqrstuvwxyz0123456789") + +func randSeq(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letters[rand.Intn(len(letters))] + } + return string(b) +} diff --git a/pkg/api/generate_test.go b/pkg/api/generate_test.go new file mode 100644 index 00000000000..7f943f21dac --- /dev/null +++ b/pkg/api/generate_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "strings" + "testing" +) + +type nameGeneratorFunc func(base string) string + +func (fn nameGeneratorFunc) GenerateName(base string) string { + return fn(base) +} + +func TestGenerateName(t *testing.T) { + testCases := []struct { + meta ObjectMeta + + base string + returned string + }{ + { + returned: "", + }, + { + meta: ObjectMeta{ + GenerateName: "test", + }, + base: "test", + returned: "test", + }, + { + meta: ObjectMeta{ + Name: "foo", + GenerateName: "test", + }, + base: "test", + returned: "foo", + }, + } + + for i, testCase := range testCases { + GenerateName(nameGeneratorFunc(func(base string) string { + if base != testCase.base { + t.Errorf("%d: unexpected call with base", i) + } + return "test" + }), &testCase.meta) + expect := testCase.returned + if expect != testCase.meta.Name { + t.Errorf("%d: unexpected name: %#v", i, testCase.meta) + } + } +} + +func TestSimpleNameGenerator(t *testing.T) { + meta := &ObjectMeta{ + GenerateName: "foo", + } + GenerateName(SimpleNameGenerator, meta) + if !strings.HasPrefix(meta.Name, "foo") || meta.Name == "foo" { + t.Errorf("unexpected name: %#v", meta) + } +} diff --git a/pkg/api/meta.go b/pkg/api/meta.go index 4bdfcdacd73..5782be6558f 100644 --- a/pkg/api/meta.go +++ b/pkg/api/meta.go @@ -17,6 +17,8 @@ limitations under the License. package api import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -24,6 +26,7 @@ import ( func FillObjectMetaSystemFields(ctx Context, meta *ObjectMeta) { meta.CreationTimestamp = util.Now() meta.UID = util.NewUUID() + meta.SelfLink = "" } // HasObjectMetaSystemFieldValues returns true if fields that are managed by the system on ObjectMeta have values. @@ -31,3 +34,17 @@ func HasObjectMetaSystemFieldValues(meta *ObjectMeta) bool { return !meta.CreationTimestamp.Time.IsZero() || len(meta.UID) != 0 } + +// GetObjectMetaPtr returns a pointer to a provided object's ObjectMeta. +// TODO: allow runtime.Unknown to extract this object +func ObjectMetaFor(obj runtime.Object) (*ObjectMeta, error) { + v, err := conversion.EnforcePtr(obj) + if err != nil { + return nil, err + } + var objectMeta *ObjectMeta + if err := runtime.FieldPtr(v, "ObjectMeta", &objectMeta); err != nil { + return nil, err + } + return objectMeta, nil +} diff --git a/pkg/api/meta/meta.go b/pkg/api/meta/meta.go index 1353083474d..ce11b272e39 100644 --- a/pkg/api/meta/meta.go +++ b/pkg/api/meta/meta.go @@ -373,36 +373,12 @@ func (a genericAccessor) SetAnnotations(annotations map[string]string) { *a.annotations = annotations } -// fieldPtr puts the address of fieldName, which must be a member of v, -// into dest, which must be an address of a variable to which this field's -// address can be assigned. -func fieldPtr(v reflect.Value, fieldName string, dest interface{}) error { - field := v.FieldByName(fieldName) - if !field.IsValid() { - return fmt.Errorf("couldn't find %v field in %#v", fieldName, v.Interface()) - } - v, err := conversion.EnforcePtr(dest) - if err != nil { - return err - } - field = field.Addr() - if field.Type().AssignableTo(v.Type()) { - v.Set(field) - return nil - } - if field.Type().ConvertibleTo(v.Type()) { - v.Set(field.Convert(v.Type())) - return nil - } - return fmt.Errorf("couldn't assign/convert %v to %v", field.Type(), v.Type()) -} - // extractFromTypeMeta extracts pointers to version and kind fields from an object func extractFromTypeMeta(v reflect.Value, a *genericAccessor) error { - if err := fieldPtr(v, "APIVersion", &a.apiVersion); err != nil { + if err := runtime.FieldPtr(v, "APIVersion", &a.apiVersion); err != nil { return err } - if err := fieldPtr(v, "Kind", &a.kind); err != nil { + if err := runtime.FieldPtr(v, "Kind", &a.kind); err != nil { return err } return nil @@ -410,25 +386,25 @@ func extractFromTypeMeta(v reflect.Value, a *genericAccessor) error { // extractFromObjectMeta extracts pointers to metadata fields from an object func extractFromObjectMeta(v reflect.Value, a *genericAccessor) error { - if err := fieldPtr(v, "Namespace", &a.namespace); err != nil { + if err := runtime.FieldPtr(v, "Namespace", &a.namespace); err != nil { return err } - if err := fieldPtr(v, "Name", &a.name); err != nil { + if err := runtime.FieldPtr(v, "Name", &a.name); err != nil { return err } - if err := fieldPtr(v, "UID", &a.uid); err != nil { + if err := runtime.FieldPtr(v, "UID", &a.uid); err != nil { return err } - if err := fieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { + if err := runtime.FieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { return err } - if err := fieldPtr(v, "SelfLink", &a.selfLink); err != nil { + if err := runtime.FieldPtr(v, "SelfLink", &a.selfLink); err != nil { return err } - if err := fieldPtr(v, "Labels", &a.labels); err != nil { + if err := runtime.FieldPtr(v, "Labels", &a.labels); err != nil { return err } - if err := fieldPtr(v, "Annotations", &a.annotations); err != nil { + if err := runtime.FieldPtr(v, "Annotations", &a.annotations); err != nil { return err } return nil @@ -436,10 +412,10 @@ func extractFromObjectMeta(v reflect.Value, a *genericAccessor) error { // extractFromObjectMeta extracts pointers to metadata fields from a list object func extractFromListMeta(v reflect.Value, a *genericAccessor) error { - if err := fieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { + if err := runtime.FieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { return err } - if err := fieldPtr(v, "SelfLink", &a.selfLink); err != nil { + if err := runtime.FieldPtr(v, "SelfLink", &a.selfLink); err != nil { return err } return nil diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go new file mode 100644 index 00000000000..36b931b95c5 --- /dev/null +++ b/pkg/api/rest/create.go @@ -0,0 +1,100 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rest + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" +) + +// RESTCreateStrategy defines the minimum validation, accepted input, and +// name generation behavior to create an object that follows Kubernetes +// API conventions. +type RESTCreateStrategy interface { + runtime.ObjectTyper + // The name generate is used when the standard GenerateName field is set. + // The NameGenerator will be invoked prior to validation. + api.NameGenerator + + // NamespaceScoped returns true if the object must be within a namespace. + NamespaceScoped() bool + // ResetBeforeCreate is invoked on create before validation to remove any fields + // that may not be persisted. + ResetBeforeCreate(obj runtime.Object) + // Validate is invoked after default fields in the object have been filled in before + // the object is persisted. + Validate(obj runtime.Object) errors.ValidationErrorList +} + +// BeforeCreate ensures that common operations for all resources are performed on creation. It only returns +// errors that can be converted to api.Status. It invokes ResetBeforeCreate, then GenerateName, then Validate. +// It returns nil if the object should be created. +func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Object) error { + objectMeta, kind, kerr := objectMetaAndKind(strategy, obj) + if kerr != nil { + return kerr + } + + if strategy.NamespaceScoped() { + if !api.ValidNamespace(ctx, objectMeta) { + return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request") + } + } else { + objectMeta.Namespace = api.NamespaceNone + } + strategy.ResetBeforeCreate(obj) + api.FillObjectMetaSystemFields(ctx, objectMeta) + api.GenerateName(strategy, objectMeta) + + if errs := strategy.Validate(obj); len(errs) > 0 { + return errors.NewInvalid(kind, objectMeta.Name, errs) + } + return nil +} + +// CheckGeneratedNameError checks whether an error that occured creating a resource is due +// to generation being unable to pick a valid name. +func CheckGeneratedNameError(strategy RESTCreateStrategy, err error, obj runtime.Object) error { + if !errors.IsAlreadyExists(err) { + return err + } + + objectMeta, kind, kerr := objectMetaAndKind(strategy, obj) + if kerr != nil { + return kerr + } + + if len(objectMeta.GenerateName) == 0 { + return err + } + + return errors.NewTryAgainLater(kind, "POST") +} + +// objectMetaAndKind retrieves kind and ObjectMeta from a runtime object, or returns an error. +func objectMetaAndKind(strategy RESTCreateStrategy, obj runtime.Object) (*api.ObjectMeta, string, error) { + objectMeta, err := api.ObjectMetaFor(obj) + if err != nil { + return nil, "", errors.NewInternalError(err) + } + _, kind, err := strategy.ObjectVersionAndKind(obj) + if err != nil { + return nil, "", errors.NewInternalError(err) + } + return objectMeta, kind, nil +} diff --git a/pkg/api/rest/create_test.go b/pkg/api/rest/create_test.go new file mode 100644 index 00000000000..c8ad1490225 --- /dev/null +++ b/pkg/api/rest/create_test.go @@ -0,0 +1,41 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rest + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" +) + +func TestCheckGeneratedNameError(t *testing.T) { + expect := errors.NewNotFound("foo", "bar") + if err := CheckGeneratedNameError(Pods, expect, &api.Pod{}); err != expect { + t.Errorf("NotFoundError should be ignored: %v", err) + } + + expect = errors.NewAlreadyExists("foo", "bar") + if err := CheckGeneratedNameError(Pods, expect, &api.Pod{}); err != expect { + t.Errorf("AlreadyExists should be returned when no GenerateName field: %v", err) + } + + expect = errors.NewAlreadyExists("foo", "bar") + if err := CheckGeneratedNameError(Pods, expect, &api.Pod{ObjectMeta: api.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsTryAgainLater(err) { + t.Errorf("expected try again later error: %v", err) + } +} diff --git a/pkg/api/rest/doc.go b/pkg/api/rest/doc.go new file mode 100644 index 00000000000..01d0e6ea553 --- /dev/null +++ b/pkg/api/rest/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package rest defines common logic around changes to Kubernetes resources. +package rest diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go new file mode 100644 index 00000000000..fa3fb7de03e --- /dev/null +++ b/pkg/api/rest/resttest/resttest.go @@ -0,0 +1,202 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resttest + +import ( + "strings" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "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 { + *testing.T + storage apiserver.RESTStorage + storageError injectErrorFunc + clusterScope bool +} + +type injectErrorFunc func(err error) + +func New(t *testing.T, storage apiserver.RESTStorage, storageError injectErrorFunc) *Tester { + return &Tester{ + T: t, + storage: storage, + storageError: storageError, + } +} + +func (t *Tester) withStorageError(err error, fn func()) { + t.storageError(err) + defer t.storageError(nil) + fn() +} + +func (t *Tester) ClusterScope() *Tester { + t.clusterScope = true + return t +} + +func copyOrDie(obj runtime.Object) runtime.Object { + out, err := api.Scheme.Copy(obj) + if err != nil { + panic(err) + } + return out +} + +func (t *Tester) TestCreate(valid runtime.Object, invalid ...runtime.Object) { + t.TestCreateHasMetadata(copyOrDie(valid)) + t.TestCreateGeneratesName(copyOrDie(valid)) + t.TestCreateGeneratesNameReturnsTryAgain(copyOrDie(valid)) + if t.clusterScope { + t.TestCreateRejectsNamespace(copyOrDie(valid)) + } else { + t.TestCreateRejectsMismatchedNamespace(copyOrDie(valid)) + } + 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 { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.Name = "test" + objectMeta.Namespace = api.NamespaceDefault + context := api.NewDefaultContext() + if t.clusterScope { + objectMeta.Namespace = api.NamespaceNone + context = api.NewContext() + } + + channel, err := t.storage.(apiserver.RESTCreater).Create(context, 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 !api.HasObjectMetaSystemFieldValues(objectMeta) { + t.Errorf("storage did not populate object meta field values") + } +} + +func (t *Tester) TestCreateGeneratesName(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.GenerateName = "test-" + + _, err = t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if objectMeta.Name == "test-" || !strings.HasPrefix(objectMeta.Name, "test-") { + t.Errorf("unexpected name: %#v", valid) + } +} + +func (t *Tester) TestCreateGeneratesNameReturnsTryAgain(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.GenerateName = "test-" + t.withStorageError(errors.NewAlreadyExists("kind", "thing"), func() { + ch, err := t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + res := <-ch + if err := errors.FromObject(res.Object); err == nil || !errors.IsTryAgainLater(err) { + t.Fatalf("Unexpected error: %v", err) + } + }) +} + +func (t *Tester) TestCreateInvokesValidation(invalid ...runtime.Object) { + for i, obj := range invalid { + ctx := api.NewDefaultContext() + _, err := t.storage.(apiserver.RESTCreater).Create(ctx, obj) + if !errors.IsInvalid(err) { + t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err) + } + } +} + +func (t *Tester) TestCreateRejectsMismatchedNamespace(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.Namespace = "not-default" + + _, err = t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + 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 (t *Tester) TestCreateRejectsNamespace(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.Namespace = "not-default" + + _, err = t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + 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()) + } +} diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go new file mode 100644 index 00000000000..a43458dd465 --- /dev/null +++ b/pkg/api/rest/types.go @@ -0,0 +1,138 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rest + +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/runtime" +) + +// 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) +} + +// 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} + +// NamespaceScoped is true for pods. +func (podStrategy) NamespaceScoped() bool { + return true +} + +// 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) +} + +// svcStrategy implements behavior for Services +// TODO: move to a service specific package. +type svcStrategy struct { + runtime.ObjectTyper + api.NameGenerator +} + +// Services is the default logic that applies when creating and updating Service +// objects. +var Services RESTCreateStrategy = svcStrategy{api.Scheme, api.SimpleNameGenerator} + +// NamespaceScoped is true for services. +func (svcStrategy) NamespaceScoped() bool { + return true +} + +// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. +func (svcStrategy) ResetBeforeCreate(obj runtime.Object) { + service := obj.(*api.Service) + // TODO: Get rid of ProxyPort. + service.Spec.ProxyPort = 0 + service.Status = api.ServiceStatus{} +} + +// Validate validates a new service. +func (svcStrategy) Validate(obj runtime.Object) errors.ValidationErrorList { + service := obj.(*api.Service) + return validation.ValidateService(service) +} + +// nodeStrategy implements behavior for nodes +// TODO: move to a node specific package. +type nodeStrategy struct { + runtime.ObjectTyper + api.NameGenerator +} + +// Nodes is the default logic that applies when creating and updating Node +// objects. +var Nodes RESTCreateStrategy = nodeStrategy{api.Scheme, api.SimpleNameGenerator} + +// NamespaceScoped is false for services. +func (nodeStrategy) NamespaceScoped() bool { + return false +} + +// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. +func (nodeStrategy) ResetBeforeCreate(obj runtime.Object) { + _ = obj.(*api.Node) + // Nodes allow *all* fields, including status, to be set. +} + +// Validate validates a new node. +func (nodeStrategy) Validate(obj runtime.Object) errors.ValidationErrorList { + node := obj.(*api.Node) + return validation.ValidateMinion(node) +} diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 93008d7aa58..a64b2a36ab5 100644 --- a/pkg/api/testing/fuzzer.go +++ b/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) diff --git a/pkg/api/types.go b/pkg/api/types.go index 3d72b98ed9e..05f1b0d0ac7 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -74,7 +74,7 @@ type ListMeta struct { } // ObjectMeta is metadata that all persisted resources must have, which includes all objects -// users must create. A resource may have only one of {ObjectMeta, ListMeta}. +// users must create. type ObjectMeta struct { // Name is unique within a namespace. Name is required when creating resources, although // some resources may allow a client to request the generation of an appropriate name @@ -82,6 +82,19 @@ type ObjectMeta struct { // definition. Name string `json:"name,omitempty"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty"` + // Namespace defines the space within which name must be unique. An empty namespace is // equivalent to the "default" namespace, but "default" is the canonical representation. // Not all objects are required to be scoped to a namespace - the value of this field for @@ -939,6 +952,17 @@ const ( // Status code 422 StatusReasonInvalid StatusReason = "Invalid" + // StatusReasonTryAgainLater means the server can be reached and understood the request, + // but cannot complete the action in a reasonable time. The client should retry the request. + // This is may be due to temporary server load or a transient communication issue with + // another server. Status code 500 is used because the HTTP spec provides no suitable + // server-requested client retry and the 5xx class represents actionable errors. + // Details (optional): + // "kind" string - the kind attribute of the resource being acted on. + // "id" string - the operation that is being attempted. + // Status code 500 + StatusReasonTryAgainLater StatusReason = "TryAgainLater" + // StatusReasonTimeout means that the request could not be completed within the given time. // Clients can get this response only when they specified a timeout param in the request. // The request might succeed with an increased value of timeout param. diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index 34432b08c3b..41e21263dcd 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/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 diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 652080d8bc4..a3e49a49c34 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -326,6 +326,19 @@ type TypeMeta struct { APIVersion string `json:"apiVersion,omitempty" description:"version of the schema the object should have"` Namespace string `json:"namespace,omitempty" description:"namespace to which the object belongs; must be a DNS_SUBDOMAIN; 'default' by default"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty" description:"an optional prefix to use to generate a unique name; has the same validation rules as name; optional, and is applied only name if is not specified"` + // Annotations are unstructured key value data stored with a resource that may be set by // external tooling. They are not queryable and should be preserved when modifying // objects. @@ -732,6 +745,17 @@ const ( // conflict. // Status code 409 StatusReasonConflict StatusReason = "Conflict" + + // StatusReasonTryAgainLater means the server can be reached and understood the request, + // but cannot complete the action in a reasonable time. The client should retry the request. + // This is may be due to temporary server load or a transient communication issue with + // another server. Status code 500 is used because the HTTP spec provides no suitable + // server-requested client retry and the 5xx class represents actionable errors. + // Details (optional): + // "kind" string - the kind attribute of the resource being acted on. + // "id" string - the operation that is being attempted. + // Status code 500 + StatusReasonTryAgainLater StatusReason = "TryAgainLater" ) // StatusCause provides more information about an api.Status failure, including diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index ccec2ad9e46..94c561f462c 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/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 diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index d2c52c4b3eb..79353bf3fd5 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -290,6 +290,19 @@ type TypeMeta struct { APIVersion string `json:"apiVersion,omitempty" description:"version of the schema the object should have"` Namespace string `json:"namespace,omitempty" description:"namespace to which the object belongs; must be a DNS_SUBDOMAIN; 'default' by default"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty" description:"an optional prefix to use to generate a unique name; has the same validation rules as name; optional, and is applied only name if is not specified"` + // Annotations are unstructured key value data stored with a resource that may be set by // external tooling. They are not queryable and should be preserved when modifying // objects. @@ -706,6 +719,17 @@ const ( // field attributes will be set. // Status code 422 StatusReasonInvalid StatusReason = "Invalid" + + // StatusReasonTryAgainLater means the server can be reached and understood the request, + // but cannot complete the action in a reasonable time. The client should retry the request. + // This is may be due to temporary server load or a transient communication issue with + // another server. Status code 500 is used because the HTTP spec provides no suitable + // server-requested client retry and the 5xx class represents actionable errors. + // Details (optional): + // "kind" string - the kind attribute of the resource being acted on. + // "id" string - the operation that is being attempted. + // Status code 500 + StatusReasonTryAgainLater StatusReason = "TryAgainLater" ) // StatusCause provides more information about an api.Status failure, including diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 1088d68a956..c8918aa6f69 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -82,6 +82,19 @@ type ObjectMeta struct { // definition. Name string `json:"name,omitempty"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty" description:"an optional prefix to use to generate a unique name; has the same validation rules as name; optional, and is applied only name if is not specified"` + // Namespace defines the space within which name must be unique. An empty namespace is // equivalent to the "default" namespace, but "default" is the canonical representation. // Not all objects are required to be scoped to a namespace - the value of this field for @@ -936,6 +949,17 @@ const ( // field attributes will be set. // Status code 422 StatusReasonInvalid StatusReason = "Invalid" + + // StatusReasonTryAgainLater means the server can be reached and understood the request, + // but cannot complete the action in a reasonable time. The client should retry the request. + // This is may be due to temporary server load or a transient communication issue with + // another server. Status code 500 is used because the HTTP spec provides no suitable + // server-requested client retry and the 5xx class represents actionable errors. + // Details (optional): + // "kind" string - the kind attribute of the resource being acted on. + // "id" string - the operation that is being attempted. + // Status code 500 + StatusReasonTryAgainLater StatusReason = "TryAgainLater" ) // StatusCause provides more information about an api.Status failure, including diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 93d62246ea7..3387d34a12f 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -53,11 +53,53 @@ func ValidateAnnotations(annotations map[string]string, field string) errs.Valid } // ValidateNameFunc validates that the provided name is valid for a given resource type. -// Not all resources have the same validation rules for names. -type ValidateNameFunc func(name string) (bool, string) +// Not all resources have the same validation rules for names. Prefix is true if the +// name will have a value appended to it. +type ValidateNameFunc func(name string, prefix bool) (bool, string) + +// maskTrailingDash replaces the final character of a string with a subdomain safe +// value if is a dash. +func maskTrailingDash(name string) string { + if strings.HasSuffix(name, "-") { + return name[:len(name)-2] + "a" + } + 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) (bool, string) { +func nameIsDNSSubdomain(name string, prefix bool) (bool, string) { + if prefix { + name = maskTrailingDash(name) + } if util.IsDNSSubdomain(name) { return true, "" } @@ -65,23 +107,36 @@ func nameIsDNSSubdomain(name string) (bool, string) { } // nameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label. -func nameIsDNS952Label(name string) (bool, string) { +func nameIsDNS952Label(name string, prefix bool) (bool, string) { + if prefix { + name = maskTrailingDash(name) + } if util.IsDNS952Label(name) { return true, "" } return false, "name must be lowercase letters, numbers, and dashes" } -// ValidateObjectMeta validates an object's metadata. +// ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already +// been performed. func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn ValidateNameFunc) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} + + if len(meta.GenerateName) != 0 { + if ok, qualifier := nameFn(meta.GenerateName, true); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("generateName", meta.GenerateName, qualifier)) + } + } + // if the generated name validates, but the calculated value does not, it's a problem with generation, and we + // report it here. This may confuse users, but indicates a programming bug and still must be validated. if len(meta.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", meta.Name)) } else { - if ok, qualifier := nameFn(meta.Name); !ok { + if ok, qualifier := nameFn(meta.Name, false); !ok { allErrs = append(allErrs, errs.NewFieldInvalid("name", meta.Name, qualifier)) } } + if requiresNamespace { if len(meta.Namespace) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("namespace", meta.Namespace)) @@ -96,10 +151,6 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val allErrs = append(allErrs, ValidateLabels(meta.Labels, "labels")...) allErrs = append(allErrs, ValidateAnnotations(meta.Annotations, "annotations")...) - // Clear self link internally - // TODO: move to its own area - meta.SelfLink = "" - return allErrs } @@ -131,10 +182,6 @@ func ValidateObjectMetaUpdate(old, meta *api.ObjectMeta) errs.ValidationErrorLis allErrs = append(allErrs, ValidateLabels(meta.Labels, "labels")...) allErrs = append(allErrs, ValidateAnnotations(meta.Annotations, "annotations")...) - // Clear self link internally - // TODO: move to its own area - meta.SelfLink = "" - return allErrs } @@ -492,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 @@ -545,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, "")) @@ -586,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 @@ -660,7 +707,7 @@ func ValidateBoundPod(pod *api.BoundPod) errs.ValidationErrorList { if len(pod.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", pod.Name)) } else { - if ok, qualifier := nameIsDNSSubdomain(pod.Name); !ok { + if ok, qualifier := nameIsDNSSubdomain(pod.Name, false); !ok { allErrs = append(allErrs, errs.NewFieldInvalid("name", pod.Name, qualifier)) } } @@ -676,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 } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 745e653ddea..210f4c72963 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -37,6 +37,30 @@ func expectPrefix(t *testing.T, prefix string, errs errors.ValidationErrorList) } } +// Ensure custom name functions are allowed +func TestValidateObjectMetaCustomName(t *testing.T) { + errs := ValidateObjectMeta(&api.ObjectMeta{Name: "test", GenerateName: "foo"}, false, func(s string, prefix bool) (bool, string) { + if s == "test" { + return true, "" + } + return false, "name-gen" + }) + if len(errs) != 1 { + t.Fatalf("unexpected errors: %v", errs) + } + if !strings.Contains(errs[0].Error(), "name-gen") { + t.Errorf("unexpected error message: %v", errs) + } +} + +// Ensure trailing slash is allowed in generate name +func TestValidateObjectMetaTrimsTrailingSlash(t *testing.T) { + errs := ValidateObjectMeta(&api.ObjectMeta{Name: "test", GenerateName: "foo-"}, false, nameIsDNSSubdomain) + if len(errs) != 0 { + t.Fatalf("unexpected errors: %v", errs) + } +} + func TestValidateLabels(t *testing.T) { successCases := []map[string]string{ {"simple": "bar"}, @@ -953,7 +977,7 @@ func TestValidateService(t *testing.T) { { name: "invalid id", svc: api.Service{ - ObjectMeta: api.ObjectMeta{Name: "123abc", Namespace: api.NamespaceDefault}, + ObjectMeta: api.ObjectMeta{Name: "-123abc", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ Port: 8675, Selector: map[string]string{"foo": "bar"}, @@ -962,6 +986,38 @@ func TestValidateService(t *testing.T) { // Should fail because the ID is invalid. numErrs: 1, }, + { + name: "invalid generate.base", + svc: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "valid", + GenerateName: "-123abc", + Namespace: api.NamespaceDefault, + }, + Spec: api.ServiceSpec{ + Port: 8675, + Selector: map[string]string{"foo": "bar"}, + }, + }, + // Should fail because the Base value for generation is invalid + numErrs: 1, + }, + { + name: "invalid generateName", + svc: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "valid", + GenerateName: "abc1234567abc1234567abc1234567abc1234567abc1234567abc1234567", + Namespace: api.NamespaceDefault, + }, + Spec: api.ServiceSpec{ + Port: 8675, + Selector: map[string]string{"foo": "bar"}, + }, + }, + // Should fail because the generate name type is invalid. + numErrs: 1, + }, { name: "missing port", svc: api.Service{ @@ -1426,7 +1482,9 @@ func TestValidateMinion(t *testing.T) { }, }, { - ObjectMeta: api.ObjectMeta{Name: "abc"}, + ObjectMeta: api.ObjectMeta{ + Name: "abc", + }, Status: api.NodeStatus{ HostIP: "something", }, diff --git a/pkg/controller/replication_controller.go b/pkg/controller/replication_controller.go index 704384edd1b..ef948b57e90 100644 --- a/pkg/controller/replication_controller.go +++ b/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 { diff --git a/pkg/controller/replication_controller_test.go b/pkg/controller/replication_controller_test.go index a2df341b8f8..58c88e5f186 100644 --- a/pkg/controller/replication_controller_test.go +++ b/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, } diff --git a/pkg/master/master.go b/pkg/master/master.go index 872c29ac45c..62b67830b57 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -100,6 +100,10 @@ type Config struct { // If nil, the first result from net.InterfaceAddrs will be used. PublicAddress net.IP + + // Control the interval that pod, node IP, and node heath status caches + // expire. + CacheTimeout time.Duration } // Master contains state for a Kubernetes cluster master/api server. @@ -117,6 +121,7 @@ type Master struct { storage map[string]apiserver.RESTStorage client *client.Client portalNet *net.IPNet + cacheTimeout time.Duration mux apiserver.Mux muxHelper *apiserver.MuxHelper @@ -179,6 +184,9 @@ func setDefaults(c *Config) { if c.ReadOnlyPort == 0 { c.ReadOnlyPort = 7080 } + if c.CacheTimeout == 0 { + c.CacheTimeout = 5 * time.Second + } if c.ReadWritePort == 0 { c.ReadWritePort = 443 } @@ -283,7 +291,9 @@ func New(c *Config) *Master { authorizer: c.Authorizer, admissionControl: c.AdmissionControl, v1beta3: c.EnableV1Beta3, - nodeIPCache: NewIPCache(c.Cloud, util.RealClock{}, 30*time.Second), + nodeIPCache: NewIPCache(c.Cloud, util.RealClock{}, c.CacheTimeout), + + cacheTimeout: c.CacheTimeout, masterCount: c.MasterCount, publicIP: c.PublicAddress, @@ -365,7 +375,7 @@ func (m *Master) init(c *Config) { RESTStorageToNodes(nodeRESTStorage).Nodes(), m.podRegistry, ) - go util.Forever(func() { podCache.UpdateAllContainers() }, time.Second*5) + go util.Forever(func() { podCache.UpdateAllContainers() }, m.cacheTimeout) go util.Forever(func() { podCache.GarbageCollectPodStatus() }, time.Minute*30) // TODO: Factor out the core API registration diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index 5171914af59..94c13664ef1 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -21,12 +21,12 @@ 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/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" rc "github.com/GoogleCloudPlatform/kubernetes/pkg/controller" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" ) @@ -55,23 +55,15 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE if !ok { return nil, fmt.Errorf("not a replication controller: %#v", obj) } - if !api.ValidNamespace(ctx, &controller.ObjectMeta) { - return nil, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context")) - } - if len(controller.Name) == 0 { - controller.Name = string(util.NewUUID()) + if err := rest.BeforeCreate(rest.ReplicationControllers, ctx, obj); err != nil { + return nil, err } - if errs := validation.ValidateReplicationController(controller); len(errs) > 0 { - return nil, errors.NewInvalid("replicationController", controller.Name, errs) - } - - api.FillObjectMetaSystemFields(ctx, &controller.ObjectMeta) return apiserver.MakeAsync(func() (runtime.Object, error) { - err := rs.registry.CreateController(ctx, controller) - if err != nil { - return nil, err + if err := rs.registry.CreateController(ctx, controller); err != nil { + err = rest.CheckGeneratedNameError(rest.ReplicationControllers, err, controller) + return apiserver.RESTResult{}, err } return rs.registry.GetController(ctx, controller.Name) }), nil diff --git a/pkg/registry/controller/rest_test.go b/pkg/registry/controller/rest_test.go index dc5455959b1..aaaea3e9460 100644 --- a/pkg/registry/controller/rest_test.go +++ b/pkg/registry/controller/rest_test.go @@ -27,6 +27,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/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" @@ -50,7 +51,9 @@ func TestListControllersError(t *testing.T) { } func TestListEmptyControllerList(t *testing.T) { - mockRegistry := registrytest.ControllerRegistry{nil, &api.ReplicationControllerList{ListMeta: api.ListMeta{ResourceVersion: "1"}}} + mockRegistry := registrytest.ControllerRegistry{ + Controllers: &api.ReplicationControllerList{ListMeta: api.ListMeta{ResourceVersion: "1"}}, + } storage := REST{ registry: &mockRegistry, } @@ -106,6 +109,7 @@ func TestListControllerList(t *testing.T) { } } +// TODO: remove, this is sufficiently covered by other tests func TestControllerDecode(t *testing.T) { mockRegistry := registrytest.ControllerRegistry{} storage := REST{ @@ -140,6 +144,7 @@ func TestControllerDecode(t *testing.T) { } } +// TODO: this is sufficiently covered by other tetss func TestControllerParsing(t *testing.T) { expectedController := api.ReplicationController{ ObjectMeta: api.ObjectMeta{ @@ -228,6 +233,7 @@ var validPodTemplate = api.PodTemplate{ }, } +// TODO: remove, this is sufficiently covered by other tests func TestCreateController(t *testing.T) { mockRegistry := registrytest.ControllerRegistry{} mockPodRegistry := registrytest.PodRegistry{ @@ -274,6 +280,7 @@ func TestCreateController(t *testing.T) { } } +// TODO: remove, covered by TestCreate func TestControllerStorageValidatesCreate(t *testing.T) { mockRegistry := registrytest.ControllerRegistry{} storage := REST{ @@ -376,6 +383,32 @@ func TestFillCurrentState(t *testing.T) { } } +// TODO: remove, covered by TestCreate +func TestCreateControllerWithGeneratedName(t *testing.T) { + storage := NewREST(®istrytest.ControllerRegistry{}, nil) + controller := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Namespace: api.NamespaceDefault, + GenerateName: "rc-", + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"a": "b"}, + Template: &validPodTemplate.Spec, + }, + } + + ctx := api.NewDefaultContext() + _, err := storage.Create(ctx, controller) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if controller.Name == "rc-" || !strings.HasPrefix(controller.Name, "rc-") { + t.Errorf("unexpected name: %#v", controller) + } +} + +// TODO: remove, covered by TestCreate func TestCreateControllerWithConflictingNamespace(t *testing.T) { storage := REST{} controller := &api.ReplicationController{ @@ -389,7 +422,7 @@ func TestCreateControllerWithConflictingNamespace(t *testing.T) { } 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 { + } 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()) } } @@ -411,3 +444,26 @@ func TestUpdateControllerWithConflictingNamespace(t *testing.T) { t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error()) } } + +func TestCreate(t *testing.T) { + registry := ®istrytest.ControllerRegistry{} + test := resttest.New(t, NewREST(registry, nil), registry.SetError) + test.TestCreate( + // valid + &api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"a": "b"}, + Template: &validPodTemplate.Spec, + }, + }, + // invalid + &api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{}, + Template: &validPodTemplate.Spec, + }, + }, + ) +} diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index 58f1450cfc1..5c0ab637383 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -24,6 +24,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" @@ -54,15 +55,13 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE return nil, fmt.Errorf("not a minion: %#v", obj) } - if errs := validation.ValidateMinion(minion); len(errs) > 0 { - return nil, kerrors.NewInvalid("minion", minion.Name, errs) + if err := rest.BeforeCreate(rest.Nodes, ctx, obj); err != nil { + return nil, err } - api.FillObjectMetaSystemFields(ctx, &minion.ObjectMeta) - return apiserver.MakeAsync(func() (runtime.Object, error) { - err := rs.registry.CreateMinion(ctx, minion) - if err != nil { + if err := rs.registry.CreateMinion(ctx, minion); err != nil { + err = rest.CheckGeneratedNameError(rest.Nodes, err, minion) return nil, err } return minion, nil diff --git a/pkg/registry/minion/rest_test.go b/pkg/registry/minion/rest_test.go index a18eade0518..0e3e2c87b83 100644 --- a/pkg/registry/minion/rest_test.go +++ b/pkg/registry/minion/rest_test.go @@ -21,6 +21,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" ) @@ -107,11 +108,14 @@ func TestMinionRegistryValidUpdate(t *testing.T) { } } +var ( + validSelector = map[string]string{"a": "b"} + invalidSelector = map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} +) + func TestMinionRegistryValidatesCreate(t *testing.T) { storage := NewREST(registrytest.NewMinionRegistry([]string{"foo", "bar"}, api.NodeResources{})) ctx := api.NewContext() - validSelector := map[string]string{"a": "b"} - invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} failureCases := map[string]api.Node{ "zero-length Name": { ObjectMeta: api.ObjectMeta{ @@ -148,3 +152,21 @@ func contains(nodes *api.NodeList, nodeID string) bool { } return false } + +func TestCreate(t *testing.T) { + registry := registrytest.NewMinionRegistry([]string{"foo", "bar"}, api.NodeResources{}) + test := resttest.New(t, NewREST(registry), registry.SetError).ClusterScope() + test.TestCreate( + // valid + &api.Node{ + Status: api.NodeStatus{ + HostIP: "something", + }, + }, + // invalid + &api.Node{ + ObjectMeta: api.ObjectMeta{ + Labels: invalidSelector, + }, + }) +} diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index 2693865bbef..b27290b1fdb 100644 --- a/pkg/registry/pod/rest.go +++ b/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,20 +57,14 @@ 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 { + err = rest.CheckGeneratedNameError(rest.Pods, err, pod) return nil, err } return rs.registry.GetPod(ctx, pod.Name) diff --git a/pkg/registry/pod/rest_test.go b/pkg/registry/pod/rest_test.go index 50e0ccd3049..aefbb509fcb 100644 --- a/pkg/registry/pod/rest_test.go +++ b/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,30 @@ func TestDeletePod(t *testing.T) { t.Errorf("Unexpeceted cache delete: %s %s %#v", fakeCache.clearedName, fakeCache.clearedNamespace, result.Object) } } + +func TestCreate(t *testing.T) { + registry := registrytest.NewPodRegistry(nil) + test := resttest.New(t, &REST{ + registry: registry, + podCache: &fakeCache{statusToReturn: &api.PodStatus{}}, + }, registry.SetError) + test.TestCreate( + // valid + &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "test1", + Image: "foo", + }, + }, + }, + }, + // invalid + &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{}, + }, + }, + ) +} diff --git a/pkg/registry/registrytest/controller.go b/pkg/registry/registrytest/controller.go index e0476574042..9e83ff86668 100644 --- a/pkg/registry/registrytest/controller.go +++ b/pkg/registry/registrytest/controller.go @@ -17,6 +17,8 @@ limitations under the License. package registrytest import ( + "sync" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" @@ -26,6 +28,13 @@ import ( type ControllerRegistry struct { Err error Controllers *api.ReplicationControllerList + sync.Mutex +} + +func (r *ControllerRegistry) SetError(err error) { + r.Lock() + defer r.Unlock() + r.Err = err } func (r *ControllerRegistry) ListControllers(ctx api.Context) (*api.ReplicationControllerList, error) { @@ -37,6 +46,8 @@ func (r *ControllerRegistry) GetController(ctx api.Context, ID string) (*api.Rep } func (r *ControllerRegistry) CreateController(ctx api.Context, controller *api.ReplicationController) error { + r.Lock() + defer r.Unlock() return r.Err } diff --git a/pkg/registry/registrytest/minion.go b/pkg/registry/registrytest/minion.go index 13db8ff2592..c44b2802e2e 100644 --- a/pkg/registry/registrytest/minion.go +++ b/pkg/registry/registrytest/minion.go @@ -52,6 +52,12 @@ func NewMinionRegistry(minions []string, nodeResources api.NodeResources) *Minio } } +func (r *MinionRegistry) SetError(err error) { + r.Lock() + defer r.Unlock() + r.Err = err +} + func (r *MinionRegistry) ListMinions(ctx api.Context) (*api.NodeList, error) { r.Lock() defer r.Unlock() diff --git a/pkg/registry/registrytest/pod.go b/pkg/registry/registrytest/pod.go index 571b61074b2..6bfa0eed471 100644 --- a/pkg/registry/registrytest/pod.go +++ b/pkg/registry/registrytest/pod.go @@ -40,6 +40,12 @@ func NewPodRegistry(pods *api.PodList) *PodRegistry { } } +func (r *PodRegistry) SetError(err error) { + r.Lock() + defer r.Unlock() + r.Err = err +} + func (r *PodRegistry) ListPodsPredicate(ctx api.Context, filter func(*api.Pod) bool) (*api.PodList, error) { r.Lock() defer r.Unlock() diff --git a/pkg/registry/registrytest/service.go b/pkg/registry/registrytest/service.go index ec0d393b3eb..d1cb2ff63a0 100644 --- a/pkg/registry/registrytest/service.go +++ b/pkg/registry/registrytest/service.go @@ -41,6 +41,12 @@ type ServiceRegistry struct { UpdatedID string } +func (r *ServiceRegistry) SetError(err error) { + r.mu.Lock() + defer r.mu.Unlock() + r.Err = err +} + func (r *ServiceRegistry) ListServices(ctx api.Context) (*api.ServiceList, error) { r.mu.Lock() defer r.mu.Unlock() diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 718b34b798e..70ed8ca69ef 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.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/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" @@ -81,35 +82,29 @@ func reloadIPsFromStorage(ipa *ipAllocator, registry Registry) { func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RESTResult, error) { service := obj.(*api.Service) - if !api.ValidNamespace(ctx, &service.ObjectMeta) { - return nil, errors.NewConflict("service", service.Namespace, fmt.Errorf("Service.Namespace does not match the provided context")) - } - if errs := validation.ValidateService(service); len(errs) > 0 { - return nil, errors.NewInvalid("service", service.Name, errs) + + if err := rest.BeforeCreate(rest.Services, ctx, obj); err != nil { + return nil, err } - api.FillObjectMetaSystemFields(ctx, &service.ObjectMeta) - - if service.Spec.PortalIP == "" { + if len(service.Spec.PortalIP) == 0 { // Allocate next available. - if ip, err := rs.portalMgr.AllocateNext(); err != nil { + ip, err := rs.portalMgr.AllocateNext() + if err != nil { return nil, err - } else { - service.Spec.PortalIP = ip.String() } + service.Spec.PortalIP = ip.String() } else { // Try to respect the requested IP. if err := rs.portalMgr.Allocate(net.ParseIP(service.Spec.PortalIP)); err != nil { el := errors.ValidationErrorList{errors.NewFieldInvalid("spec.portalIP", service.Spec.PortalIP, err.Error())} - return nil, errors.NewInvalid("service", service.Name, el) + return nil, errors.NewInvalid("Service", service.Name, el) } } return apiserver.MakeAsync(func() (runtime.Object, error) { - // TODO: Consider moving this to a rectification loop, so that we make/remove external load balancers + // TODO: Move this to post-creation rectification loop, so that we make/remove external load balancers // correctly no matter what http operations happen. - // TODO: Get rid of ProxyPort. - service.Spec.ProxyPort = 0 if service.Spec.CreateExternalLoadBalancer { if rs.cloud == nil { return nil, fmt.Errorf("requested an external service, but no cloud provider supplied.") @@ -155,8 +150,9 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE service.Spec.PublicIPs = []string{ip.String()} } } - err := rs.registry.CreateService(ctx, service) - if err != nil { + + if err := rs.registry.CreateService(ctx, service); err != nil { + err = rest.CheckGeneratedNameError(rest.Services, err, service) return nil, err } return rs.registry.GetService(ctx, service.Name) diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index eec35f117be..b18d7de0ad8 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -24,6 +24,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" cloud "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" @@ -621,6 +622,7 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) { } } +// TODO: remove, covered by TestCreate func TestCreateServiceWithConflictingNamespace(t *testing.T) { storage := REST{} service := &api.Service{ @@ -634,7 +636,7 @@ func TestCreateServiceWithConflictingNamespace(t *testing.T) { } if err == nil { t.Errorf("Expected an error, but we didn't get one") - } else if strings.Index(err.Error(), "Service.Namespace does not match the provided context") == -1 { + } else if strings.Contains(err.Error(), "Service.Namespace does not match the provided context") { t.Errorf("Expected 'Service.Namespace does not match the provided context' error, got '%s'", err.Error()) } } @@ -656,3 +658,26 @@ func TestUpdateServiceWithConflictingNamespace(t *testing.T) { t.Errorf("Expected 'Service.Namespace does not match the provided context' error, got '%s'", err.Error()) } } + +func TestCreate(t *testing.T) { + registry := registrytest.NewServiceRegistry() + fakeCloud := &cloud.FakeCloud{} + machines := []string{"foo", "bar", "baz"} + rest := NewREST(registry, fakeCloud, registrytest.NewMinionRegistry(machines, api.NodeResources{}), makeIPNet(t)) + rest.portalMgr.randomAttempts = 0 + + test := resttest.New(t, rest, registry.SetError) + test.TestCreate( + // valid + &api.Service{ + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + Port: 6502, + }, + }, + // invalid + &api.Service{ + Spec: api.ServiceSpec{}, + }, + ) +} diff --git a/pkg/runtime/helper.go b/pkg/runtime/helper.go index 8681912a869..aba69f3cb53 100644 --- a/pkg/runtime/helper.go +++ b/pkg/runtime/helper.go @@ -114,3 +114,27 @@ func SetList(list Object, objects []Object) error { items.Set(slice) return nil } + +// fieldPtr puts the address of fieldName, which must be a member of v, +// into dest, which must be an address of a variable to which this field's +// address can be assigned. +func FieldPtr(v reflect.Value, fieldName string, dest interface{}) error { + field := v.FieldByName(fieldName) + if !field.IsValid() { + return fmt.Errorf("couldn't find %v field in %#v", fieldName, v.Interface()) + } + v, err := conversion.EnforcePtr(dest) + if err != nil { + return err + } + field = field.Addr() + if field.Type().AssignableTo(v.Type()) { + v.Set(field) + return nil + } + if field.Type().ConvertibleTo(v.Type()) { + v.Set(field.Convert(v.Type())) + return nil + } + return fmt.Errorf("couldn't assign/convert %v to %v", field.Type(), v.Type()) +} diff --git a/pkg/runtime/types.go b/pkg/runtime/types.go index fec13030206..8f8a857c1a2 100644 --- a/pkg/runtime/types.go +++ b/pkg/runtime/types.go @@ -104,8 +104,9 @@ type RawExtension struct { // Unknown allows api objects with unknown types to be passed-through. This can be used // to deal with the API objects from a plug-in. Unknown objects still have functioning -// TypeMeta features-- kind, version, resourceVersion, etc. -// TODO: Not implemented yet! +// TypeMeta features-- kind, version, etc. +// TODO: Make this object have easy access to field based accessors and settors for +// metadata and field mutatation. type Unknown struct { TypeMeta `json:",inline"` // RawJSON will hold the complete JSON of the object which couldn't be matched diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 9ad1bb83c7d..3bb073e60c3 100644 --- a/test/integration/client_test.go +++ b/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{ {