Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apiextensions: update CRD strategy #50764

Merged
merged 1 commit into from
Aug 24, 2017
+40 −15
Merged

apiextensions: update CRD strategy #50764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ import (
genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
)

// rest implements a RESTStorage for API services against etcd
// REST implements a RESTStorage for API services against etcd
type REST struct {
*genericregistry.Store
}

// NewREST returns a RESTStorage object that will work against API services.
func NewREST(resource schema.GroupResource, listKind schema.GroupVersionKind, copier runtime.ObjectCopier, strategy CustomResourceDefinitionStorageStrategy, optsGetter generic.RESTOptionsGetter) *REST {
func NewREST(resource schema.GroupResource, listKind schema.GroupVersionKind, copier runtime.ObjectCopier, strategy customResourceDefinitionStorageStrategy, optsGetter generic.RESTOptionsGetter) *REST {
store := &genericregistry.Store{
Copier: copier,
NewFunc: func() runtime.Object { return &unstructured.Unstructured{} },
Expand Down
26 changes: 13 additions & 13 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ import (
"k8s.io/apiserver/pkg/storage/names"
)

type CustomResourceDefinitionStorageStrategy struct {
type customResourceDefinitionStorageStrategy struct {
runtime.ObjectTyper
names.NameGenerator

namespaceScoped bool
validator customResourceValidator
}

func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind) CustomResourceDefinitionStorageStrategy {
return CustomResourceDefinitionStorageStrategy{
func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind) customResourceDefinitionStorageStrategy {
return customResourceDefinitionStorageStrategy{
ObjectTyper: typer,
NameGenerator: names.SimpleNameGenerator,
namespaceScoped: namespaceScoped,
Expand All @@ -52,36 +52,36 @@ func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.Gr
}
}

func (a CustomResourceDefinitionStorageStrategy) NamespaceScoped() bool {
func (a customResourceDefinitionStorageStrategy) NamespaceScoped() bool {
return a.namespaceScoped
}

func (CustomResourceDefinitionStorageStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
func (customResourceDefinitionStorageStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
}

func (CustomResourceDefinitionStorageStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
func (customResourceDefinitionStorageStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
}

func (a CustomResourceDefinitionStorageStrategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList {
func (a customResourceDefinitionStorageStrategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList {
return a.validator.Validate(ctx, obj)
}

func (CustomResourceDefinitionStorageStrategy) AllowCreateOnUpdate() bool {
func (customResourceDefinitionStorageStrategy) AllowCreateOnUpdate() bool {
return false
}

func (CustomResourceDefinitionStorageStrategy) AllowUnconditionalUpdate() bool {
func (customResourceDefinitionStorageStrategy) AllowUnconditionalUpdate() bool {
return false
}

func (CustomResourceDefinitionStorageStrategy) Canonicalize(obj runtime.Object) {
func (customResourceDefinitionStorageStrategy) Canonicalize(obj runtime.Object) {
}

func (a CustomResourceDefinitionStorageStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList {
func (a customResourceDefinitionStorageStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList {
return a.validator.ValidateUpdate(ctx, obj, old)
}

func (a CustomResourceDefinitionStorageStrategy) GetAttrs(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
func (a customResourceDefinitionStorageStrategy) GetAttrs(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
accessor, err := meta.Accessor(obj)
if err != nil {
return nil, nil, false, err
Expand All @@ -102,7 +102,7 @@ func objectMetaFieldsSet(objectMeta metav1.Object, namespaceScoped bool) fields.
}
}

func (a CustomResourceDefinitionStorageStrategy) MatchCustomResourceDefinitionStorage(label labels.Selector, field fields.Selector) storage.SelectionPredicate {
func (a customResourceDefinitionStorageStrategy) MatchCustomResourceDefinitionStorage(label labels.Selector, field fields.Selector) storage.SelectionPredicate {
return storage.SelectionPredicate{
Label: label,
Field: field,
Expand Down
1 change: 1 addition & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ go_library(
deps = [
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/fields:go_default_library",
Expand Down
24 changes: 24 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package customresourcedefinition
import (
"fmt"

apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -46,9 +47,27 @@ func (strategy) NamespaceScoped() bool {
}

func (strategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
crd := obj.(*apiextensions.CustomResourceDefinition)
crd.Status = apiextensions.CustomResourceDefinitionStatus{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why reset the status?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't allow end users to set status on creation.

We do that for core objects too: https://github.com/kubernetes/kubernetes/blob/master/pkg/registry/core/replicationcontroller/strategy.go#L64.

crd.Generation = 1
}

func (strategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
newCRD := obj.(*apiextensions.CustomResourceDefinition)
oldCRD := old.(*apiextensions.CustomResourceDefinition)
newCRD.Status = oldCRD.Status
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this work without updating the controllers to use the status subresource? Do we have that? Or do we use that already? 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go#L170 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the controllers use /status already, all this looks fine.


// Any changes to the spec increment the generation number, any changes to the
// status should reflect the generation number of the corresponding object. We push
// the burden of managing the status onto the clients because we can't (in general)
// know here what version of spec the writer of the status has seen. It may seem like
// we can at first -- since obj contains spec -- but in the future we will probably make
// status its own object, and even if we don't, writes may be the result of a
// read-update-write loop, so the contents of spec may not actually be the spec that
// the controller has *seen*.
if !apiequality.Semantic.DeepEqual(oldCRD.Spec, newCRD.Spec) {
newCRD.Generation = oldCRD.Generation + 1
}
}

func (strategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList {
Expand Down Expand Up @@ -87,9 +106,14 @@ func (statusStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old r
newObj := obj.(*apiextensions.CustomResourceDefinition)
oldObj := old.(*apiextensions.CustomResourceDefinition)
newObj.Spec = oldObj.Spec

// Status updates are for only for updating status, not objectmeta.
// TODO: Update after ResetObjectMetaForStatus is added to meta/v1.
Copy link
Member Author

@nikhita nikhita Aug 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deletion won't work without updating finalizers (it will time out). However, this means we are touching some part of objectmeta while updating status...

Also, when ResetObjectMetaForStatus is added (#45552), it will not allow finalizers to be updated.

newObj.Labels = oldObj.Labels
newObj.Annotations = oldObj.Annotations
newObj.OwnerReferences = oldObj.OwnerReferences
newObj.Generation = oldObj.Generation
newObj.SelfLink = oldObj.SelfLink
}

func (statusStrategy) AllowCreateOnUpdate() bool {
Expand Down