Merge pull request #46101 from sttts/sttts-crd-core-names

Automatic merge from submit-queue

apiextensions: add Established condition

This introduces a `Established` condition on `CustomResourceDefinition`s. `Established` means that the resource has become active. A resource is established when all names are accepted initially without a conflict. A resource stays established until deleted, even during a later NameConflict due to changed names. Note that not all names can be changed.

This change is necessary to allow deletion of once-active CRDs which might have still instances, but  have NameConflicts now. Before this PR the REST endpoint was not active anymore in this case, making deletion of the instances impossible.
This commit is contained in:
Kubernetes Submit Queue 2017-05-24 02:13:32 -07:00 committed by GitHub
commit 7c76e3994c
10 changed files with 566 additions and 160 deletions

View File

@ -70,8 +70,13 @@ const (
type CustomResourceDefinitionConditionType string
const (
// NameConflict means the names chosen for this CustomResourceDefinition conflict with others in the group.
NameConflict CustomResourceDefinitionConditionType = "NameConflict"
// Established means that the resource has become active. A resource is established when all names are
// accepted without a conflict for the first time. A resource stays established until deleted, even during
// a later NamesAccepted due to changed names. Note that not all names can be changed.
Established CustomResourceDefinitionConditionType = "Established"
// NamesAccepted means the names chosen for this CustomResourceDefinition do not conflict with others in
// the group and are therefore accepted.
NamesAccepted CustomResourceDefinitionConditionType = "NamesAccepted"
// Terminating means that the CustomResourceDefinition has been deleted and is cleaning up.
Terminating CustomResourceDefinitionConditionType = "Terminating"
)

View File

@ -70,8 +70,13 @@ const (
type CustomResourceDefinitionConditionType string
const (
// NameConflict means the names chosen for this CustomResourceDefinition conflict with others in the group.
NameConflict CustomResourceDefinitionConditionType = "NameConflict"
// Established means that the resource has become active. A resource is established when all names are
// accepted without a conflict for the first time. A resource stays established until deleted, even during
// a later NamesAccepted due to changed names. Note that not all names can be changed.
Established CustomResourceDefinitionConditionType = "Established"
// NamesAccepted means the names chosen for this CustomResourceDefinition do not conflict with others in
// the group and are therefore accepted.
NamesAccepted CustomResourceDefinitionConditionType = "NamesAccepted"
// Terminating means that the CustomResourceDefinition has been deleted and is cleaning up.
Terminating CustomResourceDefinitionConditionType = "Terminating"
)

View File

@ -46,7 +46,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
// ValidateCustomResourceDefinitionUpdate statically validates
func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition) field.ErrorList {
allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata"))
allErrs = append(allErrs, ValidateCustomResourceDefinitionSpecUpdate(&obj.Spec, &oldObj.Spec, field.NewPath("spec"))...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionSpecUpdate(&obj.Spec, &oldObj.Spec, apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established), field.NewPath("spec"))...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...)
return allErrs
}
@ -64,22 +64,21 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
if len(spec.Group) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("group"), ""))
}
if errs := validationutil.IsDNS1123Subdomain(spec.Group); len(errs) > 0 {
} else if errs := validationutil.IsDNS1123Subdomain(spec.Group); len(errs) > 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, strings.Join(errs, ",")))
}
if len(strings.Split(spec.Group, ".")) < 2 {
} else if len(strings.Split(spec.Group, ".")) < 2 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, "should be a domain with at least one dot"))
}
if len(spec.Version) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("version"), ""))
}
if errs := validationutil.IsDNS1035Label(spec.Version); len(errs) > 0 {
} else if errs := validationutil.IsDNS1035Label(spec.Version); len(errs) > 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("version"), spec.Version, strings.Join(errs, ",")))
}
switch spec.Scope {
case "":
allErrs = append(allErrs, field.Required(fldPath.Child("scope"), ""))
case apiextensions.ClusterScoped, apiextensions.NamespaceScoped:
default:
allErrs = append(allErrs, field.NotSupported(fldPath.Child("scope"), spec.Scope, []string{string(apiextensions.ClusterScoped), string(apiextensions.NamespaceScoped)}))
@ -105,17 +104,19 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
}
// ValidateCustomResourceDefinitionSpecUpdate statically validates
func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList {
func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, established bool, fldPath *field.Path) field.ErrorList {
allErrs := ValidateCustomResourceDefinitionSpec(spec, fldPath)
// these all affect the storage, so you can't change them
genericvalidation.ValidateImmutableField(spec.Group, oldSpec.Group, fldPath.Child("group"))
genericvalidation.ValidateImmutableField(spec.Version, oldSpec.Version, fldPath.Child("version"))
genericvalidation.ValidateImmutableField(spec.Scope, oldSpec.Scope, fldPath.Child("scope"))
genericvalidation.ValidateImmutableField(spec.Names.Kind, oldSpec.Names.Kind, fldPath.Child("names", "kind"))
if established {
// these effect the storage and cannot be changed therefore
allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Version, oldSpec.Version, fldPath.Child("version"))...)
allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Scope, oldSpec.Scope, fldPath.Child("scope"))...)
allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Names.Kind, oldSpec.Names.Kind, fldPath.Child("names", "kind"))...)
}
// this affects the expected resource name, so you can't change it either
genericvalidation.ValidateImmutableField(spec.Names.Plural, oldSpec.Names.Plural, fldPath.Child("names", "plural"))
// these affects the resource name, which is always immutable, so this can't be updated.
allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Group, oldSpec.Group, fldPath.Child("group"))...)
allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Names.Plural, oldSpec.Names.Plural, fldPath.Child("names", "plural"))...)
return allErrs
}

View File

@ -29,11 +29,17 @@ type validationMatch struct {
errorType field.ErrorType
}
func required(path *field.Path) validationMatch {
return validationMatch{path: path, errorType: field.ErrorTypeRequired}
func required(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeRequired}
}
func invalid(path *field.Path) validationMatch {
return validationMatch{path: path, errorType: field.ErrorTypeInvalid}
func invalid(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid}
}
func unsupported(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeNotSupported}
}
func immutable(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid}
}
func (v validationMatch) matches(err *field.Error) bool {
@ -58,7 +64,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
},
},
errors: []validationMatch{
invalid(field.NewPath("metadata", "name")),
invalid("metadata", "name"),
required("spec", "version"),
required("spec", "scope"),
required("spec", "names", "singular"),
required("spec", "names", "kind"),
required("spec", "names", "listKind"),
},
},
{
@ -67,13 +78,14 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
},
errors: []validationMatch{
required(field.NewPath("spec", "group")),
required(field.NewPath("spec", "version")),
{path: field.NewPath("spec", "scope"), errorType: field.ErrorTypeNotSupported},
required(field.NewPath("spec", "names", "plural")),
required(field.NewPath("spec", "names", "singular")),
required(field.NewPath("spec", "names", "kind")),
required(field.NewPath("spec", "names", "listKind")),
invalid("metadata", "name"),
required("spec", "group"),
required("spec", "version"),
required("spec", "scope"),
required("spec", "names", "plural"),
required("spec", "names", "singular"),
required("spec", "names", "kind"),
required("spec", "names", "listKind"),
},
},
{
@ -101,17 +113,20 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
},
},
errors: []validationMatch{
invalid(field.NewPath("spec", "group")),
invalid(field.NewPath("spec", "version")),
{path: field.NewPath("spec", "scope"), errorType: field.ErrorTypeNotSupported},
invalid(field.NewPath("spec", "names", "plural")),
invalid(field.NewPath("spec", "names", "singular")),
invalid(field.NewPath("spec", "names", "kind")),
invalid(field.NewPath("spec", "names", "listKind")),
invalid(field.NewPath("status", "acceptedNames", "plural")),
invalid(field.NewPath("status", "acceptedNames", "singular")),
invalid(field.NewPath("status", "acceptedNames", "kind")),
invalid(field.NewPath("status", "acceptedNames", "listKind")),
invalid("metadata", "name"),
invalid("spec", "group"),
invalid("spec", "version"),
unsupported("spec", "scope"),
invalid("spec", "names", "plural"),
invalid("spec", "names", "singular"),
invalid("spec", "names", "kind"),
invalid("spec", "names", "listKind"), // invalid format
invalid("spec", "names", "listKind"), // kind == listKind
invalid("status", "acceptedNames", "plural"),
invalid("status", "acceptedNames", "singular"),
invalid("status", "acceptedNames", "kind"),
invalid("status", "acceptedNames", "listKind"), // invalid format
invalid("status", "acceptedNames", "listKind"), // kind == listKind
},
},
{
@ -138,21 +153,25 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
},
},
errors: []validationMatch{
invalid(field.NewPath("spec", "group")),
invalid(field.NewPath("spec", "names", "listKind")),
invalid(field.NewPath("status", "acceptedNames", "listKind")),
invalid("metadata", "name"),
invalid("spec", "group"),
required("spec", "scope"),
invalid("spec", "names", "listKind"),
invalid("status", "acceptedNames", "listKind"),
},
},
}
for _, tc := range tests {
errs := ValidateCustomResourceDefinition(tc.resource)
seenErrs := make([]bool, len(errs))
for _, expectedError := range tc.errors {
found := false
for _, err := range errs {
if expectedError.matches(err) {
for i, err := range errs {
if expectedError.matches(err) && !seenErrs[i] {
found = true
seenErrs[i] = true
break
}
}
@ -161,5 +180,281 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
t.Errorf("%s: expected %v at %v, got %v", tc.name, expectedError.errorType, expectedError.path.String(), errs)
}
}
for i, seen := range seenErrs {
if !seen {
t.Errorf("%s: unexpected error: %v", tc.name, errs[i])
}
}
}
}
func TestValidateCustomResourceDefinitionUpdate(t *testing.T) {
tests := []struct {
name string
old *apiextensions.CustomResourceDefinition
resource *apiextensions.CustomResourceDefinition
errors []validationMatch
}{
{
name: "unchanged",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
},
errors: []validationMatch{},
},
{
name: "unchanged-established",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
Conditions: []apiextensions.CustomResourceDefinitionCondition{
{Type: apiextensions.Established, Status: apiextensions.ConditionTrue},
},
},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
},
errors: []validationMatch{},
},
{
name: "changes",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
Conditions: []apiextensions.CustomResourceDefinitionCondition{
{Type: apiextensions.Established, Status: apiextensions.ConditionFalse},
},
},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "abc.com",
Version: "version2",
Scope: apiextensions.ResourceScope("Namespaced"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural2",
Singular: "singular2",
Kind: "kind2",
ListKind: "listkind2",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural2",
Singular: "singular2",
Kind: "kind2",
ListKind: "listkind2",
},
},
},
errors: []validationMatch{
immutable("spec", "group"),
immutable("spec", "names", "plural"),
},
},
{
name: "changes-established",
old: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Scope: apiextensions.ResourceScope("Cluster"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "kind",
ListKind: "listkind",
},
Conditions: []apiextensions.CustomResourceDefinitionCondition{
{Type: apiextensions.Established, Status: apiextensions.ConditionTrue},
},
},
},
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "plural.group.com",
ResourceVersion: "42",
},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "abc.com",
Version: "version2",
Scope: apiextensions.ResourceScope("Namespaced"),
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural2",
Singular: "singular2",
Kind: "kind2",
ListKind: "listkind2",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
AcceptedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "plural2",
Singular: "singular2",
Kind: "kind2",
ListKind: "listkind2",
},
},
},
errors: []validationMatch{
immutable("spec", "group"),
immutable("spec", "version"),
immutable("spec", "scope"),
immutable("spec", "names", "kind"),
immutable("spec", "names", "plural"),
},
},
}
for _, tc := range tests {
errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old)
seenErrs := make([]bool, len(errs))
for _, expectedError := range tc.errors {
found := false
for i, err := range errs {
if expectedError.matches(err) && !seenErrs[i] {
found = true
seenErrs[i] = true
break
}
}
if !found {
t.Errorf("%s: expected %v at %v, got %v", tc.name, expectedError.errorType, expectedError.path.String(), errs)
}
}
for i, seen := range seenErrs {
if !seen {
t.Errorf("%s: unexpected error: %v", tc.name, errs[i])
}
}
}
}

View File

@ -85,8 +85,7 @@ func (c *DiscoveryController) sync(version schema.GroupVersion) error {
foundVersion := false
foundGroup := false
for _, crd := range crds {
// if we can't definitively determine that our names are good, don't serve it
if !apiextensions.IsCRDConditionFalse(crd, apiextensions.NameConflict) {
if !apiextensions.IsCRDConditionTrue(crd, apiextensions.Established) {
continue
}

View File

@ -143,8 +143,7 @@ func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
r.delegate.ServeHTTP(w, req)
return
}
// if we can't definitively determine that our names are good, delegate
if !apiextensions.IsCRDConditionFalse(crd, apiextensions.NameConflict) {
if !apiextensions.IsCRDConditionTrue(crd, apiextensions.Established) {
r.delegate.ServeHTTP(w, req)
}
if len(requestInfo.Subresource) > 0 {

View File

@ -127,38 +127,55 @@ func (c *CRDFinalizer) sync(key string) error {
return err
}
// It's possible for a naming conflict to have removed this resource from the API after instances were created.
// For now we will cowardly stop finalizing. If we don't go through the REST API, weird things may happen:
// no audit trail, no admission checks or side effects, finalization would probably still work but defaulting
// would be missed. It would be a mess.
// This requires human intervention to solve, update status so they have a reason.
// TODO split coreNamesAccepted from extendedNamesAccepted. If coreNames were accepted, then we have something to cleanup
// and the endpoint is serviceable. if they aren't, then there's nothing to cleanup.
if !apiextensions.IsCRDConditionFalse(crd, apiextensions.NameConflict) {
// Now we can start deleting items. We should use the REST API to ensure that all normal admission runs.
// Since we control the endpoints, we know that delete collection works. No need to delete if not established.
if apiextensions.IsCRDConditionTrue(crd, apiextensions.Established) {
cond, deleteErr := c.deleteInstances(crd)
apiextensions.SetCRDCondition(crd, *cond)
if deleteErr != nil {
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {
utilruntime.HandleError(err)
}
return deleteErr
}
} else {
apiextensions.SetCRDCondition(crd, apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Terminating,
Status: apiextensions.ConditionTrue,
Reason: "InstanceDeletionStuck",
Message: fmt.Sprintf("cannot proceed with deletion because of %v condition", apiextensions.NameConflict),
Status: apiextensions.ConditionFalse,
Reason: "NeverEstablished",
Message: "resource was never established",
})
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {
return err
}
return fmt.Errorf("cannot proceed with deletion because of %v condition", apiextensions.NameConflict)
}
apiextensions.CRDRemoveFinalizer(crd, apiextensions.CustomResourceCleanupFinalizer)
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {
return err
}
// and now issue another delete, which should clean it all up if no finalizers remain or no-op if they do
return c.crdClient.CustomResourceDefinitions().Delete(crd.Name, nil)
}
func (c *CRDFinalizer) deleteInstances(crd *apiextensions.CustomResourceDefinition) (*apiextensions.CustomResourceDefinitionCondition, error) {
// Now we can start deleting items. While it would be ideal to use a REST API client, doing so
// could incorrectly delete a ThirdPartyResource with the same URL as the CustomResource, so we go
// directly to the storage instead. Since we control the storage, we know that delete collection works.
crClient := c.crClientGetter.GetCustomResourceListerCollectionDeleter(crd.UID)
if crClient == nil {
return fmt.Errorf("unable to find a custom resource client for %s.%s", crd.Status.AcceptedNames.Plural, crd.Spec.Group)
return nil, fmt.Errorf("unable to find a custom resource client for %s.%s", crd.Status.AcceptedNames.Plural, crd.Spec.Group)
}
ctx := genericapirequest.NewContext()
allResources, err := crClient.List(ctx, nil)
if err != nil {
return err
return &apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Terminating,
Status: apiextensions.ConditionTrue,
Reason: "InstanceDeletionFailed",
Message: fmt.Sprintf("could not list instances: %v", err),
}, err
}
deletedNamespaces := sets.String{}
@ -181,23 +198,18 @@ func (c *CRDFinalizer) sync(key string) error {
}
}
if deleteError := utilerrors.NewAggregate(deleteErrors); deleteError != nil {
apiextensions.SetCRDCondition(crd, apiextensions.CustomResourceDefinitionCondition{
return &apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Terminating,
Status: apiextensions.ConditionTrue,
Reason: "InstanceDeletionFailed",
Message: fmt.Sprintf("could not issue all deletes: %v", deleteError),
})
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {
utilruntime.HandleError(err)
}
return deleteError
}, deleteError
}
// now we need to wait until all the resources are deleted. Start with a simple poll before we do anything fancy.
// TODO not all servers are synchronized on caches. It is possible for a stale one to still be creating things.
// Once we have a mechanism for servers to indicate their states, we should check that for concurrence.
listErr := wait.PollImmediate(5*time.Second, 1*time.Minute, func() (bool, error) {
err = wait.PollImmediate(5*time.Second, 1*time.Minute, func() (bool, error) {
listObj, err := crClient.List(ctx, nil)
if err != nil {
return false, err
@ -208,34 +220,20 @@ func (c *CRDFinalizer) sync(key string) error {
glog.V(2).Infof("%s.%s waiting for %d items to be removed", crd.Status.AcceptedNames.Plural, crd.Spec.Group, len(listObj.(*unstructured.UnstructuredList).Items))
return false, nil
})
if listErr != nil {
apiextensions.SetCRDCondition(crd, apiextensions.CustomResourceDefinitionCondition{
if err != nil {
return &apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Terminating,
Status: apiextensions.ConditionTrue,
Reason: "InstanceDeletionCheck",
Message: fmt.Sprintf("could not confirm zero CustomResources remaining: %v", listErr),
})
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {
utilruntime.HandleError(err)
}
return listErr
Message: fmt.Sprintf("could not confirm zero CustomResources remaining: %v", err),
}, err
}
apiextensions.SetCRDCondition(crd, apiextensions.CustomResourceDefinitionCondition{
return &apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Terminating,
Status: apiextensions.ConditionFalse,
Reason: "InstanceDeletionCompleted",
Message: "removed all instances",
})
apiextensions.CRDRemoveFinalizer(crd, apiextensions.CustomResourceCleanupFinalizer)
crd, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {
return err
}
// and now issue another delete, which should clean it all up if no finalizers remain or no-op if they do
return c.crdClient.CustomResourceDefinitions().Delete(crd.Name, nil)
}, nil
}
func (c *CRDFinalizer) Run(workers int, stopCh <-chan struct{}) {

View File

@ -28,6 +28,7 @@ go_library(
deps = [
"//vendor/github.com/golang/glog: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/conversion:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",

View File

@ -24,6 +24,7 @@ import (
"github.com/golang/glog"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/labels"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
@ -118,12 +119,12 @@ func (c *NamingConditionController) getAcceptedNamesForGroup(group string) (allR
return allResources, allKinds
}
func (c *NamingConditionController) calculateNames(in *apiextensions.CustomResourceDefinition) (apiextensions.CustomResourceDefinitionNames, apiextensions.CustomResourceDefinitionCondition) {
func (c *NamingConditionController) calculateNamesAndConditions(in *apiextensions.CustomResourceDefinition) (apiextensions.CustomResourceDefinitionNames, apiextensions.CustomResourceDefinitionCondition, apiextensions.CustomResourceDefinitionCondition) {
// Get the names that have already been claimed
allResources, allKinds := c.getAcceptedNamesForGroup(in.Spec.Group)
condition := apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.NameConflict,
namesAcceptedCondition := apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.NamesAccepted,
Status: apiextensions.ConditionUnknown,
}
@ -134,16 +135,16 @@ func (c *NamingConditionController) calculateNames(in *apiextensions.CustomResou
// Check each name for mismatches. If there's a mismatch between spec and status, then try to deconflict.
// Continue on errors so that the status is the best match possible
if err := equalToAcceptedOrFresh(requestedNames.Plural, acceptedNames.Plural, allResources); err != nil {
condition.Status = apiextensions.ConditionTrue
condition.Reason = "Plural"
condition.Message = err.Error()
namesAcceptedCondition.Status = apiextensions.ConditionFalse
namesAcceptedCondition.Reason = "PluralConflict"
namesAcceptedCondition.Message = err.Error()
} else {
newNames.Plural = requestedNames.Plural
}
if err := equalToAcceptedOrFresh(requestedNames.Singular, acceptedNames.Singular, allResources); err != nil {
condition.Status = apiextensions.ConditionTrue
condition.Reason = "Singular"
condition.Message = err.Error()
namesAcceptedCondition.Status = apiextensions.ConditionFalse
namesAcceptedCondition.Reason = "SingularConflict"
namesAcceptedCondition.Message = err.Error()
} else {
newNames.Singular = requestedNames.Singular
}
@ -161,37 +162,58 @@ func (c *NamingConditionController) calculateNames(in *apiextensions.CustomResou
}
if err := utilerrors.NewAggregate(errs); err != nil {
condition.Status = apiextensions.ConditionTrue
condition.Reason = "ShortNames"
condition.Message = err.Error()
namesAcceptedCondition.Status = apiextensions.ConditionFalse
namesAcceptedCondition.Reason = "ShortNamesConflict"
namesAcceptedCondition.Message = err.Error()
} else {
newNames.ShortNames = requestedNames.ShortNames
}
}
if err := equalToAcceptedOrFresh(requestedNames.Kind, acceptedNames.Kind, allKinds); err != nil {
condition.Status = apiextensions.ConditionTrue
condition.Reason = "Kind"
condition.Message = err.Error()
namesAcceptedCondition.Status = apiextensions.ConditionFalse
namesAcceptedCondition.Reason = "KindConflict"
namesAcceptedCondition.Message = err.Error()
} else {
newNames.Kind = requestedNames.Kind
}
if err := equalToAcceptedOrFresh(requestedNames.ListKind, acceptedNames.ListKind, allKinds); err != nil {
condition.Status = apiextensions.ConditionTrue
condition.Reason = "ListKind"
condition.Message = err.Error()
namesAcceptedCondition.Status = apiextensions.ConditionFalse
namesAcceptedCondition.Reason = "ListKindConflict"
namesAcceptedCondition.Message = err.Error()
} else {
newNames.ListKind = requestedNames.ListKind
}
// if we haven't changed the condition, then our names must be good.
if condition.Status == apiextensions.ConditionUnknown {
condition.Status = apiextensions.ConditionFalse
condition.Reason = "NoConflicts"
condition.Message = "no conflicts found"
if namesAcceptedCondition.Status == apiextensions.ConditionUnknown {
namesAcceptedCondition.Status = apiextensions.ConditionTrue
namesAcceptedCondition.Reason = "NoConflicts"
namesAcceptedCondition.Message = "no conflicts found"
}
return newNames, condition
// set EstablishedCondition to true if all names are accepted. Never set it back to false.
establishedCondition := apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Established,
Status: apiextensions.ConditionFalse,
Reason: "NotAccepted",
Message: "not all names are accepted",
LastTransitionTime: metav1.NewTime(time.Now()),
}
if old := apiextensions.FindCRDCondition(in, apiextensions.Established); old != nil {
establishedCondition = *old
}
if establishedCondition.Status != apiextensions.ConditionTrue && namesAcceptedCondition.Status == apiextensions.ConditionTrue {
establishedCondition = apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Established,
Status: apiextensions.ConditionTrue,
Reason: "InitialNamesAccepted",
Message: "the initial names have been accepted",
LastTransitionTime: metav1.NewTime(time.Now()),
}
}
return newNames, namesAcceptedCondition, establishedCondition
}
func equalToAcceptedOrFresh(requestedName, acceptedName string, usedNames sets.String) error {
@ -214,12 +236,12 @@ func (c *NamingConditionController) sync(key string) error {
return err
}
acceptedNames, namingCondition := c.calculateNames(inCustomResourceDefinition)
// nothing to do if accepted names and NameConflict condition didn't change
acceptedNames, namingCondition, establishedCondition := c.calculateNamesAndConditions(inCustomResourceDefinition)
// nothing to do if accepted names and NamesAccepted condition didn't change
if reflect.DeepEqual(inCustomResourceDefinition.Status.AcceptedNames, acceptedNames) &&
apiextensions.IsCRDConditionEquivalent(
&namingCondition,
apiextensions.FindCRDCondition(inCustomResourceDefinition, apiextensions.NameConflict)) {
apiextensions.IsCRDConditionEquivalent(&namingCondition, apiextensions.FindCRDCondition(inCustomResourceDefinition, apiextensions.NamesAccepted)) &&
apiextensions.IsCRDConditionEquivalent(&establishedCondition, apiextensions.FindCRDCondition(inCustomResourceDefinition, apiextensions.Established)) {
return nil
}
@ -230,6 +252,7 @@ func (c *NamingConditionController) sync(key string) error {
crd.Status.AcceptedNames = acceptedNames
apiextensions.SetCRDCondition(crd, namingCondition)
apiextensions.SetCRDCondition(crd, establishedCondition)
updatedObj, err := c.crdClient.CustomResourceDefinitions().UpdateStatus(crd)
if err != nil {

View File

@ -67,6 +67,12 @@ func (b *crdBuilder) StatusNames(plural, singular, kind, listKind string, shortN
return b
}
func (b *crdBuilder) Condition(c apiextensions.CustomResourceDefinitionCondition) *crdBuilder {
b.curr.Status.Conditions = append(b.curr.Status.Conditions, c)
return b
}
func names(plural, singular, kind, listKind string, shortNames ...string) apiextensions.CustomResourceDefinitionNames {
ret := apiextensions.CustomResourceDefinitionNames{
Plural: plural,
@ -82,30 +88,45 @@ func (b *crdBuilder) NewOrDie() *apiextensions.CustomResourceDefinition {
return &b.curr
}
var goodCondition = apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.NameConflict,
Status: apiextensions.ConditionFalse,
var acceptedCondition = apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.NamesAccepted,
Status: apiextensions.ConditionTrue,
Reason: "NoConflicts",
Message: "no conflicts found",
}
func badCondition(reason, message string) apiextensions.CustomResourceDefinitionCondition {
func nameConflictCondition(reason, message string) apiextensions.CustomResourceDefinitionCondition {
return apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.NameConflict,
Status: apiextensions.ConditionTrue,
Type: apiextensions.NamesAccepted,
Status: apiextensions.ConditionFalse,
Reason: reason,
Message: message,
}
}
var establishedCondition = apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Established,
Status: apiextensions.ConditionTrue,
Reason: "InitialNamesAccepted",
Message: "the initial names have been accepted",
}
var notEstablishedCondition = apiextensions.CustomResourceDefinitionCondition{
Type: apiextensions.Established,
Status: apiextensions.ConditionFalse,
Reason: "NotAccepted",
Message: "not all names are accepted",
}
func TestSync(t *testing.T) {
tests := []struct {
name string
in *apiextensions.CustomResourceDefinition
existing []*apiextensions.CustomResourceDefinition
expectedNames apiextensions.CustomResourceDefinitionNames
expectedCondition apiextensions.CustomResourceDefinitionCondition
in *apiextensions.CustomResourceDefinition
existing []*apiextensions.CustomResourceDefinition
expectedNames apiextensions.CustomResourceDefinitionNames
expectedNameConflictCondition apiextensions.CustomResourceDefinitionCondition
expectedEstablishedCondition apiextensions.CustomResourceDefinitionCondition
}{
{
name: "first resource",
@ -114,7 +135,8 @@ func TestSync(t *testing.T) {
expectedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "alfa",
},
expectedCondition: goodCondition,
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "different groups",
@ -122,8 +144,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("alfa.charlie.com").StatusNames("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2").NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: goodCondition,
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "conflict plural to singular",
@ -131,8 +154,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "alfa", "", "").NewOrDie(),
},
expectedNames: names("", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: badCondition("Plural", `"alfa" is already in use`),
expectedNames: names("", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("PluralConflict", `"alfa" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "conflict singular to shortName",
@ -140,8 +164,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "indias", "", "", "delta-singular").NewOrDie(),
},
expectedNames: names("alfa", "", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: badCondition("Singular", `"delta-singular" is already in use`),
expectedNames: names("alfa", "", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("SingularConflict", `"delta-singular" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "conflict on shortName to shortName",
@ -149,8 +174,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "indias", "", "", "hotel-shortname-2").NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind"),
expectedCondition: badCondition("ShortNames", `"hotel-shortname-2" is already in use`),
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind"),
expectedNameConflictCondition: nameConflictCondition("ShortNamesConflict", `"hotel-shortname-2" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "conflict on kind to listkind",
@ -158,8 +184,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "indias", "", "echo-kind").NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: badCondition("Kind", `"echo-kind" is already in use`),
expectedNames: names("alfa", "delta-singular", "", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("KindConflict", `"echo-kind" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "conflict on listkind to kind",
@ -167,8 +194,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "indias", "foxtrot-listkind", "").NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "echo-kind", "", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: badCondition("ListKind", `"foxtrot-listkind" is already in use`),
expectedNames: names("alfa", "delta-singular", "echo-kind", "", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("ListKindConflict", `"foxtrot-listkind" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "no conflict on resource and kind",
@ -176,8 +204,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "echo-kind", "", "").NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: goodCondition,
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "merge on conflicts",
@ -188,8 +217,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "indias", "foxtrot-listkind", "", "delta-singular").NewOrDie(),
},
expectedNames: names("alfa", "yankee-singular", "echo-kind", "whiskey-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: badCondition("ListKind", `"foxtrot-listkind" is already in use`),
expectedNames: names("alfa", "yankee-singular", "echo-kind", "whiskey-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("ListKindConflict", `"foxtrot-listkind" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "merge on conflicts shortNames as one",
@ -200,8 +230,9 @@ func TestSync(t *testing.T) {
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "indias", "foxtrot-listkind", "", "delta-singular", "golf-shortname-1").NewOrDie(),
},
expectedNames: names("alfa", "yankee-singular", "echo-kind", "whiskey-listkind", "victor-shortname-1", "uniform-shortname-2"),
expectedCondition: badCondition("ListKind", `"foxtrot-listkind" is already in use`),
expectedNames: names("alfa", "yankee-singular", "echo-kind", "whiskey-listkind", "victor-shortname-1", "uniform-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("ListKindConflict", `"foxtrot-listkind" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
{
name: "no conflicts on self",
@ -215,8 +246,9 @@ func TestSync(t *testing.T) {
StatusNames("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2").
NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedCondition: goodCondition,
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "no conflicts on self, remove shortname",
@ -230,8 +262,53 @@ func TestSync(t *testing.T) {
StatusNames("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2").
NewOrDie(),
},
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1"),
expectedCondition: goodCondition,
expectedNames: names("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1"),
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "established before with true condition",
in: newCRD("alfa.bravo.com").Condition(establishedCondition).NewOrDie(),
existing: []*apiextensions.CustomResourceDefinition{},
expectedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "alfa",
},
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "not established before with false condition",
in: newCRD("alfa.bravo.com").Condition(notEstablishedCondition).NewOrDie(),
existing: []*apiextensions.CustomResourceDefinition{},
expectedNames: apiextensions.CustomResourceDefinitionNames{
Plural: "alfa",
},
expectedNameConflictCondition: acceptedCondition,
expectedEstablishedCondition: establishedCondition,
},
{
name: "conflicting, established before with true condition",
in: newCRD("alfa.bravo.com").SpecNames("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2").
Condition(establishedCondition).
NewOrDie(),
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "alfa", "", "").NewOrDie(),
},
expectedNames: names("", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("PluralConflict", `"alfa" is already in use`),
expectedEstablishedCondition: establishedCondition,
},
{
name: "conflicting, not established before with false condition",
in: newCRD("alfa.bravo.com").SpecNames("alfa", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2").
Condition(notEstablishedCondition).
NewOrDie(),
existing: []*apiextensions.CustomResourceDefinition{
newCRD("india.bravo.com").StatusNames("india", "alfa", "", "").NewOrDie(),
},
expectedNames: names("", "delta-singular", "echo-kind", "foxtrot-listkind", "golf-shortname-1", "hotel-shortname-2"),
expectedNameConflictCondition: nameConflictCondition("PluralConflict", `"alfa" is already in use`),
expectedEstablishedCondition: notEstablishedCondition,
},
}
@ -245,12 +322,15 @@ func TestSync(t *testing.T) {
crdLister: listers.NewCustomResourceDefinitionLister(crdIndexer),
crdMutationCache: cache.NewIntegerResourceVersionMutationCache(crdIndexer, crdIndexer, 60*time.Second, false),
}
actualNames, actualCondition := c.calculateNames(tc.in)
actualNames, actualNameConflictCondition, actualEstablishedCondition := c.calculateNamesAndConditions(tc.in)
if e, a := tc.expectedNames, actualNames; !reflect.DeepEqual(e, a) {
t.Errorf("%v expected %v, got %#v", tc.name, e, a)
}
if e, a := tc.expectedCondition, actualCondition; !apiextensions.IsCRDConditionEquivalent(&e, &a) {
if e, a := tc.expectedNameConflictCondition, actualNameConflictCondition; !apiextensions.IsCRDConditionEquivalent(&e, &a) {
t.Errorf("%v expected %v, got %v", tc.name, e, a)
}
if e, a := tc.expectedEstablishedCondition, actualEstablishedCondition; !apiextensions.IsCRDConditionEquivalent(&e, &a) {
t.Errorf("%v expected %v, got %v", tc.name, e, a)
}
}