Skip to content

Commit

Permalink
Handle if neither reference nor selector provided
Browse files Browse the repository at this point in the history 
Also:
- Resolve comments in Usage
- Remove composite check during deletion

Signed-off-by: Hasan Turken <turkenh@gmail.com>
  • Loading branch information
@turkenh
turkenh committed Sep 11, 2023
1 parent 4e7889a commit f7ac2fb
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 222 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ jobs:
test-suite:
- base
- composition-webhook-schema-validation
- environment-config
- environment-configs
- usage

steps:
Expand Down
15 changes: 7 additions & 8 deletions internal/controller/apiextensions/usage/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,11 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/ratelimiter"
xpresource "github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed"

"github.com/crossplane/crossplane/apis/apiextensions/v1alpha1"
apiextensionscontroller "github.com/crossplane/crossplane/internal/controller/apiextensions/controller"
"github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource"
"github.com/crossplane/crossplane/internal/usage"
"github.com/crossplane/crossplane/internal/xcrd"
)

const (
Expand Down Expand Up @@ -215,8 +214,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
of := u.Spec.Of
by := u.Spec.By

// Identify used xp resource as an unstructured object.
used := resource.New(resource.FromReference(v1.ObjectReference{
// Identify used xp composed as an unstructured object.
used := composed.New(composed.FromReference(v1.ObjectReference{
Kind: of.Kind,
Name: of.ResourceRef.Name,
APIVersion: of.APIVersion,
Expand All @@ -225,7 +224,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
if meta.WasDeleted(u) {
if by != nil {
// Identify using resource as an unstructured object.
using := resource.New(resource.FromReference(v1.ObjectReference{
using := composed.New(composed.FromReference(v1.ObjectReference{
Kind: by.Kind,
Name: by.ResourceRef.Name,
APIVersion: by.APIVersion,
Expand All @@ -239,8 +238,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
return reconcile.Result{}, err
}

if l := u.GetLabels()[xcrd.LabelKeyNamePrefixForComposed]; len(l) > 0 && l == using.GetLabels()[xcrd.LabelKeyNamePrefixForComposed] && err == nil {
// If the usage and using resource are part of the same composite resource, we need to wait for the using resource to be deleted
if err == nil {
// Using resource is still there, so we need to wait for it to be deleted.
msg := "Waiting for using resource to be deleted."
log.Debug(msg)
r.record.Event(u, event.Normal(reasonWaitUsing, msg))
Expand Down Expand Up @@ -341,7 +340,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco

if by != nil {
// Identify using resource as an unstructured object.
using := resource.New(resource.FromReference(v1.ObjectReference{
using := composed.New(composed.FromReference(v1.ObjectReference{
Kind: by.Kind,
Name: by.ResourceRef.Name,
APIVersion: by.APIVersion,
Expand Down
97 changes: 21 additions & 76 deletions internal/controller/apiextensions/usage/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/errors"
xpresource "github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed"
"github.com/crossplane/crossplane-runtime/pkg/test"

"github.com/crossplane/crossplane/apis/apiextensions/v1alpha1"
"github.com/crossplane/crossplane/internal/controller/apiextensions/usage/resource"
"github.com/crossplane/crossplane/internal/xcrd"
)

Expand Down Expand Up @@ -159,7 +159,7 @@ func TestReconcile(t *testing.T) {
switch o := obj.(type) {
case *v1alpha1.Usage:
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
case *resource.Unstructured:
case *composed.Unstructured:
return errBoom
}
return nil
Expand Down Expand Up @@ -194,13 +194,13 @@ func TestReconcile(t *testing.T) {
switch o := obj.(type) {
case *v1alpha1.Usage:
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
case *resource.Unstructured:
case *composed.Unstructured:
return nil
}
return nil
}),
MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error {
if _, ok := obj.(*resource.Unstructured); ok {
if _, ok := obj.(*composed.Unstructured); ok {
return errBoom
}
return nil
Expand Down Expand Up @@ -235,7 +235,7 @@ func TestReconcile(t *testing.T) {
o.Spec.By = &v1alpha1.Resource{
ResourceRef: &v1alpha1.ResourceRef{Name: "using"},
}
case *resource.Unstructured:
case *composed.Unstructured:
if o.GetName() == "using" {
return errBoom
}
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestReconcile(t *testing.T) {
}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetName() == "using" {
o.SetAPIVersion("v1")
o.SetKind("AnotherKind")
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestReconcile(t *testing.T) {
}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetName() == "using" {
o.SetAPIVersion("v1")
o.SetKind("AnotherKind")
Expand Down Expand Up @@ -380,13 +380,13 @@ func TestReconcile(t *testing.T) {
o.Spec.Reason = &reason
return nil
}
if _, ok := obj.(*resource.Unstructured); ok {
if _, ok := obj.(*composed.Unstructured); ok {
return nil
}
return errors.New("unexpected object type")
}),
MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error {
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetLabels()[inUseLabelKey] != "true" {
t.Fatalf("expected %s label to be true", inUseLabelKey)
}
Expand Down Expand Up @@ -429,7 +429,7 @@ func TestReconcile(t *testing.T) {
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
return nil
}
if _, ok := obj.(*resource.Unstructured); ok {
if _, ok := obj.(*composed.Unstructured); ok {
return kerrors.NewNotFound(schema.GroupResource{}, "")
}
return errors.New("unexpected object type")
Expand Down Expand Up @@ -463,7 +463,7 @@ func TestReconcile(t *testing.T) {
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
return nil
}
if _, ok := obj.(*resource.Unstructured); ok {
if _, ok := obj.(*composed.Unstructured); ok {
return errBoom
}
return errors.New("unexpected object type")
Expand Down Expand Up @@ -502,7 +502,7 @@ func TestReconcile(t *testing.T) {
}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetName() == "used" {
o.SetLabels(map[string]string{inUseLabelKey: "true"})
}
Expand Down Expand Up @@ -539,7 +539,7 @@ func TestReconcile(t *testing.T) {
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
o.SetLabels(map[string]string{inUseLabelKey: "true"})
return nil
}
Expand Down Expand Up @@ -577,7 +577,7 @@ func TestReconcile(t *testing.T) {
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
o.SetLabels(map[string]string{inUseLabelKey: "true"})
return nil
}
Expand Down Expand Up @@ -618,7 +618,7 @@ func TestReconcile(t *testing.T) {
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
return nil
}
if _, ok := obj.(*resource.Unstructured); ok {
if _, ok := obj.(*composed.Unstructured); ok {
return kerrors.NewNotFound(schema.GroupResource{}, "")
}
return errors.New("unexpected object type")
Expand Down Expand Up @@ -652,7 +652,7 @@ func TestReconcile(t *testing.T) {
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
o.SetLabels(map[string]string{inUseLabelKey: "true"})
return nil
}
Expand All @@ -662,7 +662,7 @@ func TestReconcile(t *testing.T) {
return nil
}),
MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error {
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetLabels()[inUseLabelKey] != "" {
t.Errorf("expected in use label to be removed")
}
Expand All @@ -686,63 +686,8 @@ func TestReconcile(t *testing.T) {
r: reconcile.Result{},
},
},
"SuccessfulDeleteWithUsedAndUsing": {
reason: "We should return no error once we have successfully deleted the usage resource with using resource defined.",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(xpresource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
if o, ok := obj.(*v1alpha1.Usage); ok {
o.SetDeletionTimestamp(&now)
o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "used"}
o.Spec.By = &v1alpha1.Resource{
APIVersion: "v1",
Kind: "AnotherKind",
ResourceRef: &v1alpha1.ResourceRef{Name: "using"},
}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o.GetName() == "used" {
o.SetLabels(map[string]string{inUseLabelKey: "true"})
return nil
}
return nil
}
return errors.New("unexpected object type")
}),
MockList: test.NewMockListFn(nil, func(obj client.ObjectList) error {
return nil
}),
MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error {
if o, ok := obj.(*resource.Unstructured); ok {
if o.GetLabels()[inUseLabelKey] != "" {
t.Errorf("expected in use label to be removed")
}
return nil
}
return errors.New("unexpected object type")
}),
},
}),
WithSelectorResolver(fakeSelectorResolver{
resourceSelectorFn: func(ctx context.Context, u *v1alpha1.Usage) error {
return nil
},
}),
WithFinalizer(xpresource.FinalizerFns{RemoveFinalizerFn: func(_ context.Context, _ xpresource.Object) error {
return nil
}}),
},
},
want: want{
r: reconcile.Result{},
},
},
"SuccessfulWaitWhenUsageAndUsingPartOfSameComposite": {
reason: "We should wait until the using resource is deleted when usage and using resources are part of same composite.",
"SuccessfulWaitWhenUsingStillThere": {
reason: "We should wait until the using resource is deleted.",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
Expand All @@ -760,7 +705,7 @@ func TestReconcile(t *testing.T) {
}
return nil
}
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetName() == "used" {
o.SetLabels(map[string]string{inUseLabelKey: "true"})
}
Expand All @@ -775,7 +720,7 @@ func TestReconcile(t *testing.T) {
return nil
}),
MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error {
if o, ok := obj.(*resource.Unstructured); ok {
if o, ok := obj.(*composed.Unstructured); ok {
if o.GetLabels()[inUseLabelKey] != "" {
t.Errorf("expected in use label to be removed")
}
Expand Down
Loading

0 comments on commit f7ac2fb

Please sign in to comment.