crossplane
/
crossplane
mirror of https://github.com/crossplane/crossplane.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Propagate XRC updates to their corresponding XR 

This commit also includes a handful of updates to prevent spurious reconciles
that were discovered as a byproduct of reconciling composite resources more
frequently.

Signed-off-by: Nic Cope <negz@rk0n.org>
This commit is contained in:
Nic Cope 2020-11-13 23:48:10 +00:00
parent dd775ab34f
commit 520a7f7b42
13 changed files with 911 additions and 259 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
pkg/controller/apiextensions/claim/api.go
View File

@ -34,65 +34,17 @@ import (
// Error strings.
const (
errCreateComposite = "cannot create composite resource"
errUpdateClaim = "cannot update composite resource claim"
errUpdateComposite = "cannot update composite resource"
errDeleteComposite = "cannot delete composite resource"
errBindConflict = "cannot bind composite resource that references a different claim"
errGetSecret = "cannot get composite resource's connection secret"
errSecretConflict = "cannot establish control of existing connection secret"
errCreateOrUpdateSecret = "cannot create or update connection secret"
errUpdateClaim = "cannot update composite resource claim"
errUpdateComposite = "cannot update composite resource"
errBindClaimConflict = "cannot bind claim that references a different composite resource"
errBindCompositeConflict = "cannot bind composite resource that references a different claim"
errGetSecret = "cannot get composite resource's connection secret"
errSecretConflict = "cannot establish control of existing connection secret"
errCreateOrUpdateSecret = "cannot create or update connection secret"
)
// An APICompositeCreator creates resources by submitting them to a Kubernetes
// API server.
type APICompositeCreator struct {
client client.Client
typer runtime.ObjectTyper
}
// NewAPICompositeCreator returns a new APICompositeCreator.
func NewAPICompositeCreator(c client.Client, t runtime.ObjectTyper) *APICompositeCreator {
return &APICompositeCreator{client: c, typer: t}
}
// TODO(negz): We should render and patch a composite resource on each
// reconcile, rather than just creating it once.
// Create the supplied composite using the supplied claim.
func (a *APICompositeCreator) Create(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
cp.SetClaimReference(meta.ReferenceTo(cm, resource.MustGetKind(cm, a.typer)))
if err := a.client.Create(ctx, cp); err != nil {
return errors.Wrap(err, errCreateComposite)
}
// Since we use GenerateName feature of ObjectMeta, final name of the
// resource is calculated during the creation of the resource. So, we
// can generate a complete reference only after the creation.
cpr := meta.ReferenceTo(cp, resource.MustGetKind(cp, a.typer))
cm.SetResourceReference(cpr)
return errors.Wrap(a.client.Update(ctx, cm), errUpdateClaim)
}
// An APICompositeDeleter deletes composite resources from the API server.
type APICompositeDeleter struct {
client client.Client
}
// NewAPICompositeDeleter returns a new APICompositeDeleter.
func NewAPICompositeDeleter(c client.Client) *APICompositeDeleter {
return &APICompositeDeleter{client: c}
}
// Delete the supplied composite resource from the API server.
func (a *APICompositeDeleter) Delete(ctx context.Context, _ resource.CompositeClaim, cp resource.Composite) error {
return errors.Wrap(resource.IgnoreNotFound(a.client.Delete(ctx, cp)), errDeleteComposite)
}
// An APIBinder binds claims to composites by updating them in a Kubernetes API
// server. Note that APIBinder does not support objects that do not use the
// status subresource; such objects should use APIBinder.
// server.
type APIBinder struct {
client client.Client
typer runtime.ObjectTyper
@ -105,25 +57,32 @@ func NewAPIBinder(c client.Client, t runtime.ObjectTyper) *APIBinder {
// Bind the supplied claim to the supplied composite.
func (a *APIBinder) Bind(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
existing := cp.GetClaimReference()
proposed := meta.ReferenceTo(cm, resource.MustGetKind(cm, a.typer))
existing := cm.GetResourceReference()
proposed := meta.ReferenceTo(cp, resource.MustGetKind(cp, a.typer))
if existing != nil && !cmp.Equal(existing, proposed, cmpopts.IgnoreFields(corev1.ObjectReference{}, "UID")) {
return errors.New(errBindClaimConflict)
}
if existing != nil && (existing.Namespace != proposed.Namespace || existing.Name != proposed.Name) {
return errors.New(errBindConflict)
// Propagate the actual external name back from the composite to the claim.
meta.SetExternalName(cm, meta.GetExternalName(cp))
// We set the claim's resource reference first in order to reduce the chance
// of leaking newly created composite resources. We want as few calls that
// could fail and trigger a requeue between composite creation and reference
// persistence as possible.
cm.SetResourceReference(proposed)
if err := a.client.Update(ctx, cm); err != nil {
return errors.Wrap(err, errUpdateClaim)
}
existing = cp.GetClaimReference()
proposed = meta.ReferenceTo(cm, resource.MustGetKind(cm, a.typer))
if existing != nil && !cmp.Equal(existing, proposed, cmpopts.IgnoreFields(corev1.ObjectReference{}, "UID")) {
return errors.New(errBindCompositeConflict)
}
cp.SetClaimReference(proposed)
if err := a.client.Update(ctx, cp); err != nil {
return errors.Wrap(err, errUpdateComposite)
}
if meta.GetExternalName(cp) == "" {
return nil
}
// Propagate back the final name of the composite resource to the claim.
meta.SetExternalName(cm, meta.GetExternalName(cp))
return errors.Wrap(a.client.Update(ctx, cm), errUpdateClaim)
return errors.Wrap(a.client.Update(ctx, cp), errUpdateComposite)
}
// An APIConnectionPropagator propagates connection details by reading

149
pkg/controller/apiextensions/claim/api_test.go
View File

@ -25,6 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/resource"
@ -32,7 +33,153 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/test"
)
var _ ConnectionPropagator = &APIConnectionPropagator{}
var (
_ Binder = &APIBinder{}
_ ConnectionPropagator = &APIConnectionPropagator{}
)
func TestBind(t *testing.T) {
errBoom := errors.New("boom")
type fields struct {
c client.Client
t runtime.ObjectTyper
}
type args struct {
ctx context.Context
cm resource.CompositeClaim
cp resource.Composite
}
cases := map[string]struct {
reason string
fields fields
args args
want error
}{
"CompositeRefConflict": {
reason: "An error should be returned if the claim is bound to another composite resource",
fields: fields{
t: fake.SchemeWith(&fake.Composite{}),
},
args: args{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
APIVersion: fake.GVK(&fake.Composite{}).GroupVersion().String(),
Kind: fake.GVK(&fake.Composite{}).Kind,
Name: "who",
},
},
},
cp: &fake.Composite{
ObjectMeta: metav1.ObjectMeta{
Name: "wat",
},
},
},
want: errors.New(errBindClaimConflict),
},
"UpdateClaimError": {
reason: "Errors updating the claim should be returned",
fields: fields{
c: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(errBoom),
},
t: fake.SchemeWith(&fake.Composite{}),
},
args: args{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
APIVersion: fake.GVK(&fake.Composite{}).GroupVersion().String(),
Kind: fake.GVK(&fake.Composite{}).Kind,
},
},
},
cp: &fake.Composite{},
},
want: errors.Wrap(errBoom, errUpdateClaim),
},
"ClaimRefConflict": {
reason: "An error should be returned if the composite resource is bound to another claim",
fields: fields{
c: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(nil),
},
t: fake.SchemeWith(&fake.Composite{}, &fake.CompositeClaim{}),
},
args: args{
cm: &fake.CompositeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "wat",
},
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
APIVersion: fake.GVK(&fake.Composite{}).GroupVersion().String(),
Kind: fake.GVK(&fake.Composite{}).Kind,
},
},
},
cp: &fake.Composite{
ClaimReferencer: fake.ClaimReferencer{
Ref: &corev1.ObjectReference{
APIVersion: fake.GVK(&fake.CompositeClaim{}).GroupVersion().String(),
Kind: fake.GVK(&fake.CompositeClaim{}).Kind,
Name: "who",
},
},
},
},
want: errors.New(errBindCompositeConflict),
},
"UpdateCompositeError": {
reason: "Errors updating the composite resource should be returned",
fields: fields{
c: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(nil, func(obj runtime.Object) error {
if _, ok := obj.(*fake.Composite); ok {
return errBoom
}
return nil
}),
},
t: fake.SchemeWith(&fake.Composite{}, &fake.CompositeClaim{}),
},
args: args{
cm: &fake.CompositeClaim{
CompositeResourceReferencer: fake.CompositeResourceReferencer{
Ref: &corev1.ObjectReference{
APIVersion: fake.GVK(&fake.Composite{}).GroupVersion().String(),
Kind: fake.GVK(&fake.Composite{}).Kind,
},
},
},
cp: &fake.Composite{
ClaimReferencer: fake.ClaimReferencer{
Ref: &corev1.ObjectReference{
APIVersion: fake.GVK(&fake.CompositeClaim{}).GroupVersion().String(),
Kind: fake.GVK(&fake.CompositeClaim{}).Kind,
},
},
},
},
want: errors.Wrap(errBoom, errUpdateComposite),
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
b := NewAPIBinder(tc.fields.c, tc.fields.t)
got := b.Bind(tc.args.ctx, tc.args.cm, tc.args.cp)
if diff := cmp.Diff(tc.want, got, test.EquateErrors()); diff != "" {
t.Errorf("b.Bind(...): %s\n-want, +got:\n%s\n", tc.reason, diff)
}
})
}
}
func TestPropagateConnection(t *testing.T) {
errBoom := errors.New("boom")

30
pkg/controller/apiextensions/claim/configurator.go
View File

@ -18,14 +18,16 @@ package claim
import (
"context"
"errors"
"fmt"
"github.com/pkg/errors"
"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
"github.com/crossplane/crossplane-runtime/pkg/meta"
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite"
"github.com/crossplane/crossplane/pkg/xcrd"
)
// Label keys.
@ -35,10 +37,20 @@ const (
LabelKeyClaimNamespace = "crossplane.io/claim-namespace"
)
const errUnsupportedClaimSpec = "composite resource claim spec was not an object"
// Configure the supplied composite resource. The composite resource name is
// derived from the supplied claim, as {name}-{random-string}. The claim's
// external name annotation, if any, is propagated to the composite resource.
func Configure(_ context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
cp.SetGenerateName(fmt.Sprintf("%s-", cm.GetName()))
meta.AddAnnotations(cp, cm.GetAnnotations())
meta.AddLabels(cp, cm.GetLabels())
meta.AddLabels(cp, map[string]string{
xcrd.LabelKeyClaimName: cm.GetName(),
xcrd.LabelKeyClaimNamespace: cm.GetNamespace(),
})
ucm, ok := cm.(*claim.Unstructured)
if !ok {
return nil
@ -57,21 +69,11 @@ func Configure(_ context.Context, cm resource.CompositeClaim, cp resource.Compos
i, _ := fieldpath.Pave(ucm.Object).GetValue("spec")
spec, ok := i.(map[string]interface{})
if !ok {
return errors.New("composite resource claim spec was not an object")
return errors.New(errUnsupportedClaimSpec)
}
// TODO(negz): Make these filtered keys constants in the xcrds package?
_ = fieldpath.Pave(ucp.Object).SetValue("spec", filter(spec, "resourceRef", "writeConnectionSecretToRef"))
meta.AddAnnotations(ucp, ucm.GetAnnotations())
meta.AddLabels(ucp, ucm.GetLabels())
ucp.SetGenerateName(fmt.Sprintf("%s-", cm.GetName()))
if meta.GetExternalName(cm) != "" {
meta.SetExternalName(ucp, meta.GetExternalName(cm))
}
meta.AddLabels(ucp, map[string]string{
LabelKeyClaimName: cm.GetName(),
LabelKeyClaimNamespace: cm.GetNamespace(),
})
_ = fieldpath.Pave(ucp.Object).SetValue("spec", filter(spec, xcrd.FilterClaimSpecProps...))
return nil
}

195
pkg/controller/apiextensions/claim/configurator_test.go Normal file
View File

@ -0,0 +1,195 @@
/*
Copyright 2020 The Crossplane Authors.
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 claim
import (
"context"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite"
"github.com/crossplane/crossplane-runtime/pkg/test"
"github.com/crossplane/crossplane/pkg/xcrd"
)
func TestConfigure(t *testing.T) {
// errBoom := errors.New("boom")
ns := "spacename"
name := "cool"
type args struct {
ctx context.Context
cm resource.CompositeClaim
cp resource.Composite
}
type want struct {
cp resource.Composite
err error
}
cases := map[string]struct {
reason string
args args
want want
}{
"ClaimNotUnstructured": {
reason: "We should return early if the claim is not unstructured",
args: args{
cm: &fake.CompositeClaim{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns,
Name: name,
},
},
cp: &fake.Composite{},
},
want: want{
cp: &fake.Composite{
ObjectMeta: metav1.ObjectMeta{
GenerateName: name + "-",
Labels: map[string]string{
xcrd.LabelKeyClaimNamespace: ns,
xcrd.LabelKeyClaimName: name,
},
},
},
},
},
"CompositeNotUnstructured": {
reason: "We should return early if the composite is not unstructured",
args: args{
cm: &claim.Unstructured{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": ns,
"name": name,
},
},
},
},
cp: &fake.Composite{},
},
want: want{
cp: &fake.Composite{
ObjectMeta: metav1.ObjectMeta{
GenerateName: name + "-",
Labels: map[string]string{
xcrd.LabelKeyClaimNamespace: ns,
xcrd.LabelKeyClaimName: name,
},
},
},
},
},
"UnsupportedSpecError": {
reason: "We should return early if the claim's spec is not an unstructured object",
args: args{
cm: &claim.Unstructured{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": ns,
"name": name,
},
"spec": "wat",
},
},
},
cp: &composite.Unstructured{},
},
want: want{
cp: &composite.Unstructured{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"generateName": name + "-",
"labels": map[string]interface{}{
xcrd.LabelKeyClaimNamespace: ns,
xcrd.LabelKeyClaimName: name,
},
},
},
},
},
err: errors.New(errUnsupportedClaimSpec),
},
},
"Configured": {
reason: "The composite resource should be configured according to the claim",
args: args{
cm: &claim.Unstructured{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": ns,
"name": name,
},
"spec": map[string]interface{}{
"coolness": 23,
// These should be filtered out.
"resourceRef": "ref",
"writeConnectionSecretToRef": "ref",
},
},
},
},
cp: &composite.Unstructured{},
},
want: want{
cp: &composite.Unstructured{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"generateName": name + "-",
"labels": map[string]interface{}{
xcrd.LabelKeyClaimNamespace: ns,
xcrd.LabelKeyClaimName: name,
},
},
"spec": map[string]interface{}{
"coolness": int64(23),
},
},
},
},
},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got := Configure(tc.args.ctx, tc.args.cm, tc.args.cp)
if diff := cmp.Diff(tc.want.err, got, test.EquateErrors()); diff != "" {
t.Errorf("b.Bind(...): %s\n-want error, +got error:\n%s\n", tc.reason, diff)
}
if diff := cmp.Diff(tc.want.cp, tc.args.cp); diff != "" {
t.Errorf("b.Bind(...): %s\n-want, +got:\n%s\n", tc.reason, diff)
}
})
}
}

206
pkg/controller/apiextensions/claim/reconciler.go
View File

@ -22,7 +22,6 @@ import (
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
@ -72,14 +71,16 @@ func ControllerName(name string) string {
return "claim/" + name
}
// A CompositeConfigurator configures a resource, typically by converting it to
// a known type and populating its spec.
// A CompositeConfigurator configures the supplied composite resource, typically
// by converting it to a known type and populating its spec to reflect the
// supplied composite resource claim.
type CompositeConfigurator interface {
Configure(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
}
// A CompositeConfiguratorFn is a function that satisfies the
// CompositeConfigurator interface.
// A CompositeConfiguratorFn configures the supplied composite resource,
// typically by converting it to a known type and populating its spec to reflect
// the supplied composite resource claim.
type CompositeConfiguratorFn func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
// Configure the supplied resource using the supplied claim.
@ -87,48 +88,18 @@ func (fn CompositeConfiguratorFn) Configure(ctx context.Context, cm resource.Com
return fn(ctx, cm, cp)
}
// A CompositeCreator creates a resource, typically by submitting it to an API
// server. CompositeCreators must not modify the supplied resource class, but are
// responsible for final modifications to the claim and resource, for example
// ensuring resource, claim, and owner references are set.
type CompositeCreator interface {
Create(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
}
// A CompositeCreatorFn is a function that satisfies the CompositeCreator interface.
type CompositeCreatorFn func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
// Create the supplied resource.
func (fn CompositeCreatorFn) Create(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
return fn(ctx, cm, cp)
}
// A CompositeDeleter deletes a composite resource.
type CompositeDeleter interface {
// Delete the supplied Claim to the supplied Composite resource.
Delete(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
}
// A Binder binds a composite resource claim to a composite resource.
type Binder interface {
// Bind the supplied Claim to the supplied Composite resource.
Bind(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
}
// BinderFns satisfy the Binder interface.
type BinderFns struct {
BindFn func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
UnbindFn func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
}
// A BinderFn binds a composite resource claim to a composite resource.
type BinderFn func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error
// Bind the supplied Claim to the supplied Composite resource.
func (b BinderFns) Bind(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
return b.BindFn(ctx, cm, cp)
}
// Unbind the supplied Claim from the supplied Composite resource.
func (b BinderFns) Unbind(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
return b.UnbindFn(ctx, cm, cp)
func (fn BinderFn) Bind(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error {
return fn(ctx, cm, cp)
}
// A ConnectionPropagator is responsible for propagating information required to
@ -137,6 +108,15 @@ type ConnectionPropagator interface {
PropagateConnection(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error)
}
// A ConnectionPropagatorFn is responsible for propagating information required
// to connect to a resource.
type ConnectionPropagatorFn func(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error)
// PropagateConnection details from one resource to the other.
func (fn ConnectionPropagatorFn) PropagateConnection(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error) {
return fn(ctx, to, from)
}
// A Reconciler reconciles composite resource claims by creating exactly one kind of
// concrete composite resource. Each composite resource claim kind should create an instance
// of this controller for each composite resource kind they can bind to, using
@ -144,7 +124,7 @@ type ConnectionPropagator interface {
// type of resource class provisioner. Each controller must watch its subset of
// composite resource claims and any composite resources they control.
type Reconciler struct {
client client.Client
client resource.ClientApplicator
newClaim func() resource.CompositeClaim
newComposite func() resource.Composite
@ -160,16 +140,12 @@ type Reconciler struct {
type crComposite struct {
CompositeConfigurator
CompositeCreator
CompositeDeleter
ConnectionPropagator
}
func defaultCRComposite(c client.Client, t runtime.ObjectTyper) crComposite {
return crComposite{
CompositeConfigurator: CompositeConfiguratorFn(Configure),
CompositeCreator: NewAPICompositeCreator(c, t),
CompositeDeleter: NewAPICompositeDeleter(c),
ConnectionPropagator: NewAPIConnectionPropagator(c, t),
}
}
@ -189,11 +165,19 @@ func defaultCRClaim(c client.Client, t runtime.ObjectTyper) crClaim {
// A ReconcilerOption configures a Reconciler.
type ReconcilerOption func(*Reconciler)
// WithCompositeCreator specifies which CompositeCreator should be used to create
// composite resources.
func WithCompositeCreator(c CompositeCreator) ReconcilerOption {
// WithClientApplicator specifies how the Reconciler should interact with the
// Kubernetes API.
func WithClientApplicator(ca resource.ClientApplicator) ReconcilerOption {
return func(r *Reconciler) {
r.composite.CompositeCreator = c
r.client = ca
}
}
// WithCompositeConfigurator specifies how the Reconciler should configure the bound
// composite resource.
func WithCompositeConfigurator(cf CompositeConfigurator) ReconcilerOption {
return func(r *Reconciler) {
r.composite.CompositeConfigurator = cf
}
}
@ -243,7 +227,10 @@ func WithRecorder(er event.Recorder) ReconcilerOption {
func NewReconciler(m manager.Manager, of resource.CompositeClaimKind, with resource.CompositeKind, o ...ReconcilerOption) *Reconciler {
c := unstructured.NewClient(m.GetClient())
r := &Reconciler{
client: c,
client: resource.ClientApplicator{
Client: c,
Applicator: resource.NewAPIPatchingApplicator(c),
},
newClaim: func() resource.CompositeClaim {
return claim.New(claim.WithGroupVersionKind(schema.GroupVersionKind(of)))
},
@ -294,44 +281,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
record = record.WithAnnotations("composite-name", cm.GetResourceReference().Name)
log = log.WithValues("composite-name", cm.GetResourceReference().Name)
err := r.client.Get(ctx, meta.NamespacedNameOf(ref), cp)
if kerrors.IsNotFound(err) {
// Our composite was not found, but we're being deleted too. There's
// nothing to finalize.
if meta.WasDeleted(cm) {
// TODO(negz): Can we refactor to avoid this deletion logic that
// is almost identical to the meta.WasDeleted block below?
log = log.WithValues("deletion-timestamp", cm.GetDeletionTimestamp())
if err := r.claim.RemoveFinalizer(ctx, cm); err != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error.
log.Debug("Cannot remove finalizer", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonDelete, err))
return reconcile.Result{RequeueAfter: aShortWait}, nil
}
// We've successfully deleted our claim and removed our finalizer. If we
// assume we were the only controller that added a finalizer to this
// claim then it should no longer exist and thus there is no point
// trying to update its status.
log.Debug("Successfully deleted composite resource claim")
return reconcile.Result{Requeue: false}, nil
}
// If the composite resource we explicitly reference doesn't exist yet
// we want to retry after a brief wait, in case it is created. We
// must explicitly requeue because our EnqueueRequestForClaim
// handler can only enqueue reconciles for composite resources that
// have their claim reference set, so we can't expect to be queued
// implicitly when the composite resource we want to bind to appears.
log.Debug("Referenced composite resource not found", "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonBind, err))
cm.SetConditions(Waiting())
return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
}
if err != nil {
if err := r.client.Get(ctx, meta.NamespacedNameOf(ref), cp); resource.IgnoreNotFound(err) != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error.
@ -344,7 +294,10 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
if meta.WasDeleted(cm) {
log = log.WithValues("deletion-timestamp", cm.GetDeletionTimestamp())
if err := r.composite.Delete(ctx, cm, cp); err != nil {
// TODO(negz): We should make sure the composite resource references the
// claim before we try to delete it.
if err := r.client.Delete(ctx, cp); resource.IgnoreNotFound(err) != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error.
@ -379,47 +332,44 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
// after a brief wait, in case this was a transient error.
log.Debug("Cannot add composite resource claim finalizer", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonBind, err))
cm.SetConditions(v1alpha1.Creating())
return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
return reconcile.Result{RequeueAfter: aShortWait}, nil
}
// Claim reconcilers (should) watch for either claims with a resource ref,
// claims with a class ref, or composite resources with a claim ref. In the
// first case the composite resource always exists by the time we get here. In
// the second case the class reference is set. The third case exposes us to
// a pathological scenario in which a composite resource references a claim
// that has no resource ref or class ref, so we can't assume the class ref
// is always set at this point.
if !meta.WasCreated(cp) {
if err := r.composite.Configure(ctx, cm, cp); err != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error or some
// issue with the resource class was resolved.
log.Debug("Cannot configure composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonConfigure, err))
return reconcile.Result{RequeueAfter: aShortWait}, nil
}
if err := r.composite.Configure(ctx, cm, cp); err != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error or some
// issue with the resource class was resolved.
log.Debug("Cannot configure composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonConfigure, err))
cm.SetConditions(v1alpha1.Creating())
return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
}
// We'll know our composite resource's name at this point because it was
// set by the above configure step.
record = record.WithAnnotations("composite-name", cp.GetName())
log = log.WithValues("composite-name", cp.GetName())
// We'll know our composite resource's name at this point because it was
// set by the above configure step.
record = record.WithAnnotations("composite-name", cp.GetName())
log = log.WithValues("composite-name", cp.GetName())
if err := r.client.Apply(ctx, cp); err != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error.
log.Debug("Cannot apply composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonConfigure, err))
return reconcile.Result{RequeueAfter: aShortWait}, nil
}
if err := r.composite.Create(ctx, cm, cp); err != nil {
// If we didn't hit this error last time we'll be requeued
// implicitly due to the status update. Otherwise we want to retry
// after a brief wait, in case this was a transient error.
log.Debug("Cannot create composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonConfigure, err))
cm.SetConditions(v1alpha1.Creating())
return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
}
log.Debug("Successfully applied composite resource")
record.Event(cm, event.Normal(reasonConfigure, "Successfully applied composite resource"))
log.Debug("Successfully created composite resource")
record.Event(cm, event.Normal(reasonConfigure, "Successfully configured composite resource"))
if err := r.claim.Bind(ctx, cm, cp); err != nil {
// If we didn't hit this error last time we'll be requeued implicitly
// due to the status update. Otherwise we want to retry after a brief
// wait, in case this was a transient error.
log.Debug("Cannot bind to composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonBind, err))
cm.SetConditions(v1alpha1.Unavailable().WithMessage(err.Error()))
return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
}
if !resource.IsConditionTrue(cp.GetCondition(v1alpha1.TypeReady)) {
@ -432,16 +382,6 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
}
if err := r.claim.Bind(ctx, cm, cp); err != nil {
// If we didn't hit this error last time we'll be requeued implicitly
// due to the status update. Otherwise we want to retry after a brief
// wait, in case this was a transient error.
log.Debug("Cannot bind to composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
record.Event(cm, event.Warning(reasonBind, err))
cm.SetConditions(v1alpha1.Unavailable().WithMessage(err.Error()))
return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
}
log.Debug("Successfully bound composite resource")
record.Event(cm, event.Normal(reasonBind, "Successfully bound composite resource"))

403
pkg/controller/apiextensions/claim/reconciler_test.go Normal file
View File

@ -0,0 +1,403 @@
/*
Copyright 2020 The Crossplane Authors.
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 claim
import (
"context"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite"
"github.com/crossplane/crossplane-runtime/pkg/test"
)
func TestReconcile(t *testing.T) {
errBoom := errors.New("boom")
type args struct {
mgr manager.Manager
of resource.CompositeClaimKind
with resource.CompositeKind
opts []ReconcilerOption
}
type want struct {
r reconcile.Result
err error
}
cases := map[string]struct {
reason string
args args
want want
}{
"ClaimNotFound": {
reason: "We should not return an error if the composite resource was not found.",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(kerrors.NewNotFound(schema.GroupResource{}, "")),
},
}),
},
},
want: want{
r: reconcile.Result{},
},
},
"GetCompositeError": {
reason: "We should requeue after a short wait if we encounter an error while getting the referenced composite resource",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
switch o := obj.(type) {
case *claim.Unstructured:
o.SetResourceReference(&corev1.ObjectReference{})
return nil
case *composite.Unstructured:
return errBoom
}
return nil
}),
},
}),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"DeleteCompositeError": {
reason: "We should requeue after a short wait if we encounter an error while deleting the referenced composite resource",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
now := metav1.Now()
o.SetDeletionTimestamp(&now)
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
MockDelete: test.NewMockDeleteFn(errBoom),
},
}),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"RemoveFinalizerError": {
reason: "We should requeue after a short wait if we encounter an error while removing the claim's finalizer",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
now := metav1.Now()
o.SetDeletionTimestamp(&now)
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
MockDelete: test.NewMockDeleteFn(nil),
},
}),
WithClaimFinalizer(resource.FinalizerFns{
RemoveFinalizerFn: func(ctx context.Context, obj resource.Object) error { return errBoom },
}),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"SuccessfulDelete": {
reason: "We should not requeue if we successfully delete the bound composite resource",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
now := metav1.Now()
o.SetDeletionTimestamp(&now)
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
MockDelete: test.NewMockDeleteFn(nil),
},
}),
WithClaimFinalizer(resource.FinalizerFns{
RemoveFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
},
},
want: want{
r: reconcile.Result{Requeue: false},
},
},
"AddFinalizerError": {
reason: "We should requeue after a short wait if we encounter an error while adding the claim's finalizer",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
},
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return errBoom },
}),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"ConfigureError": {
reason: "We should requeue after a short wait if we encounter an error configuring the composite resource",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
},
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
WithCompositeConfigurator(CompositeConfiguratorFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return errBoom })),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"ApplyError": {
reason: "We should requeue after a short wait if we encounter an error applying the composite resource",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
},
Applicator: resource.ApplyFn(func(c context.Context, r runtime.Object, ao ...resource.ApplyOption) error {
return errBoom
}),
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
WithCompositeConfigurator(CompositeConfiguratorFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"BindError": {
reason: "We should requeue after a short wait if we encounter an error binding the composite resource",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
Applicator: resource.ApplyFn(func(c context.Context, r runtime.Object, ao ...resource.ApplyOption) error {
return nil
}),
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
WithCompositeConfigurator(CompositeConfiguratorFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
WithBinder(BinderFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return errBoom })),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"CompositeNotReady": {
reason: "We should return early if the bound composite resource is not yet ready",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
if o, ok := obj.(*claim.Unstructured); ok {
o.SetResourceReference(&corev1.ObjectReference{})
}
return nil
}),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
Applicator: resource.ApplyFn(func(c context.Context, r runtime.Object, ao ...resource.ApplyOption) error {
return nil
}),
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
WithCompositeConfigurator(CompositeConfiguratorFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
WithBinder(BinderFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
},
},
want: want{
r: reconcile.Result{},
},
},
"PropagateConnectionError": {
reason: "We should requeue after a short wait if an error is encountered while propagating the bound composite's connection details",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
switch o := obj.(type) {
case *claim.Unstructured:
o.SetResourceReference(&corev1.ObjectReference{})
case *composite.Unstructured:
o.SetConditions(v1alpha1.Available())
}
return nil
}),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
Applicator: resource.ApplyFn(func(c context.Context, r runtime.Object, ao ...resource.ApplyOption) error {
return nil
}),
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
WithCompositeConfigurator(CompositeConfiguratorFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
WithBinder(BinderFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
WithConnectionPropagator(ConnectionPropagatorFn(func(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error) {
return false, errBoom
})),
},
},
want: want{
r: reconcile.Result{RequeueAfter: aShortWait},
},
},
"SuccessfulPropagate": {
reason: "We should not requeue if we successfully applied the composite resource and propagated its connection details",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClientApplicator(resource.ClientApplicator{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj runtime.Object) error {
switch o := obj.(type) {
case *claim.Unstructured:
o.SetResourceReference(&corev1.ObjectReference{})
case *composite.Unstructured:
o.SetConditions(v1alpha1.Available())
}
return nil
}),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
Applicator: resource.ApplyFn(func(c context.Context, r runtime.Object, ao ...resource.ApplyOption) error {
return nil
}),
}),
WithClaimFinalizer(resource.FinalizerFns{
AddFinalizerFn: func(ctx context.Context, obj resource.Object) error { return nil },
}),
WithCompositeConfigurator(CompositeConfiguratorFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
WithBinder(BinderFn(func(ctx context.Context, cm resource.CompositeClaim, cp resource.Composite) error { return nil })),
WithConnectionPropagator(ConnectionPropagatorFn(func(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error) {
return true, nil
})),
},
},
want: want{
r: reconcile.Result{Requeue: false},
},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
r := NewReconciler(tc.args.mgr, tc.args.of, tc.args.with, tc.args.opts...)
got, err := r.Reconcile(reconcile.Request{})
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nr.Reconcile(...): -want error, +got error:\n%s", tc.reason, diff)
}
if diff := cmp.Diff(tc.want.r, got, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nr.Reconcile(...): -want, +got:\n%s", tc.reason, diff)
}
})
}
}

5
pkg/controller/apiextensions/composite/api.go
View File

@ -35,6 +35,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane/apis/apiextensions/v1beta1"
"github.com/crossplane/crossplane/pkg/xcrd"
)
// Error strings.
@ -313,9 +314,9 @@ type APINamingConfigurator struct {
// Configure the supplied composite resource's root name prefix.
func (c *APINamingConfigurator) Configure(ctx context.Context, cp resource.Composite, _ *v1beta1.Composition) error {
if cp.GetLabels()[LabelKeyNamePrefixForComposed] != "" {
if cp.GetLabels()[xcrd.LabelKeyNamePrefixForComposed] != "" {
return nil
}
meta.AddLabels(cp, map[string]string{LabelKeyNamePrefixForComposed: cp.GetName()})
meta.AddLabels(cp, map[string]string{xcrd.LabelKeyNamePrefixForComposed: cp.GetName()})
return errors.Wrap(c.client.Update(ctx, cp), errUpdateComposite)
}

7
pkg/controller/apiextensions/composite/api_test.go
View File

@ -36,6 +36,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/test"
"github.com/crossplane/crossplane/apis/apiextensions/v1beta1"
"github.com/crossplane/crossplane/pkg/xcrd"
)
var errBoom = errors.New("boom")
@ -586,10 +587,10 @@ func TestAPINamingConfigurator(t *testing.T) {
"LabelAlreadyExists": {
reason: "No operation should be done if the name prefix is already given",
args: args{
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{LabelKeyNamePrefixForComposed: "given"}}},
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{xcrd.LabelKeyNamePrefixForComposed: "given"}}},
},
want: want{
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{LabelKeyNamePrefixForComposed: "given"}}},
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{xcrd.LabelKeyNamePrefixForComposed: "given"}}},
},
},
"AssignedName": {
@ -601,7 +602,7 @@ func TestAPINamingConfigurator(t *testing.T) {
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "cp"}},
},
want: want{
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "cp", Labels: map[string]string{LabelKeyNamePrefixForComposed: "cp"}}},
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Name: "cp", Labels: map[string]string{xcrd.LabelKeyNamePrefixForComposed: "cp"}}},
},
},
}

18
pkg/controller/apiextensions/composite/composed.go
View File

@ -34,6 +34,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed"
"github.com/crossplane/crossplane/apis/apiextensions/v1beta1"
"github.com/crossplane/crossplane/pkg/xcrd"
)
// Error strings
@ -48,13 +49,6 @@ const (
errName = "cannot use dry-run create to name composed resource"
)
// Label keys.
const (
LabelKeyNamePrefixForComposed = "crossplane.io/composite"
LabelKeyClaimName = "crossplane.io/claim-name"
LabelKeyClaimNamespace = "crossplane.io/claim-namespace"
)
// Observation is the result of composed reconciliation.
type Observation struct {
Ref corev1.ObjectReference
@ -95,19 +89,19 @@ func (r *APIDryRunRenderer) Render(ctx context.Context, cp resource.Composite, c
if err := json.Unmarshal(t.Base.Raw, cd); err != nil {
return errors.Wrap(err, errUnmarshal)
}
if cp.GetLabels()[LabelKeyNamePrefixForComposed] == "" {
if cp.GetLabels()[xcrd.LabelKeyNamePrefixForComposed] == "" {
return errors.New(errNamePrefix)
}
// This label will be used if composed resource is yet another composite.
meta.AddLabels(cd, map[string]string{
LabelKeyNamePrefixForComposed: cp.GetLabels()[LabelKeyNamePrefixForComposed],
LabelKeyClaimName: cp.GetLabels()[LabelKeyClaimName],
LabelKeyClaimNamespace: cp.GetLabels()[LabelKeyClaimNamespace],
xcrd.LabelKeyNamePrefixForComposed: cp.GetLabels()[xcrd.LabelKeyNamePrefixForComposed],
xcrd.LabelKeyClaimName: cp.GetLabels()[xcrd.LabelKeyClaimName],
xcrd.LabelKeyClaimNamespace: cp.GetLabels()[xcrd.LabelKeyClaimNamespace],
})
// Unmarshalling the template will overwrite any existing fields, so we must
// restore the existing name, if any. We also set generate name in case we
// haven't yet named this composed resource.
cd.SetGenerateName(cp.GetLabels()[LabelKeyNamePrefixForComposed] + "-")
cd.SetGenerateName(cp.GetLabels()[xcrd.LabelKeyNamePrefixForComposed] + "-")
cd.SetName(name)
cd.SetNamespace(namespace)
for i, p := range t.Patches {

25
pkg/controller/apiextensions/composite/composed_test.go
View File

@ -38,6 +38,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed"
"github.com/crossplane/crossplane-runtime/pkg/test"
"github.com/crossplane/crossplane/apis/apiextensions/v1beta1"
"github.com/crossplane/crossplane/pkg/xcrd"
)
func TestRender(t *testing.T) {
@ -88,9 +89,9 @@ func TestRender(t *testing.T) {
client: &test.MockClient{MockCreate: test.NewMockCreateFn(errBoom)},
args: args{
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
LabelKeyNamePrefixForComposed: "ola",
LabelKeyClaimName: "rola",
LabelKeyClaimNamespace: "rolans",
xcrd.LabelKeyNamePrefixForComposed: "ola",
xcrd.LabelKeyClaimName: "rola",
xcrd.LabelKeyClaimNamespace: "rolans",
}}},
cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{}},
t: v1beta1.ComposedTemplate{Base: runtime.RawExtension{Raw: tmpl}},
@ -99,9 +100,9 @@ func TestRender(t *testing.T) {
cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{
GenerateName: "ola-",
Labels: map[string]string{
LabelKeyNamePrefixForComposed: "ola",
LabelKeyClaimName: "rola",
LabelKeyClaimNamespace: "rolans",
xcrd.LabelKeyNamePrefixForComposed: "ola",
xcrd.LabelKeyClaimName: "rola",
xcrd.LabelKeyClaimNamespace: "rolans",
},
OwnerReferences: []metav1.OwnerReference{{Controller: &ctrl}},
}},
@ -113,9 +114,9 @@ func TestRender(t *testing.T) {
client: &test.MockClient{MockCreate: test.NewMockCreateFn(nil)},
args: args{
cp: &fake.Composite{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
LabelKeyNamePrefixForComposed: "ola",
LabelKeyClaimName: "rola",
LabelKeyClaimNamespace: "rolans",
xcrd.LabelKeyNamePrefixForComposed: "ola",
xcrd.LabelKeyClaimName: "rola",
xcrd.LabelKeyClaimNamespace: "rolans",
}}},
cd: &fake.Composed{ObjectMeta: metav1.ObjectMeta{Name: "cd"}},
t: v1beta1.ComposedTemplate{Base: runtime.RawExtension{Raw: tmpl}},
@ -125,9 +126,9 @@ func TestRender(t *testing.T) {
Name: "cd",
GenerateName: "ola-",
Labels: map[string]string{
LabelKeyNamePrefixForComposed: "ola",
LabelKeyClaimName: "rola",
LabelKeyClaimNamespace: "rolans",
xcrd.LabelKeyNamePrefixForComposed: "ola",
xcrd.LabelKeyClaimName: "rola",
xcrd.LabelKeyClaimNamespace: "rolans",
},
OwnerReferences: []metav1.OwnerReference{{Controller: &ctrl}},
}},

18
pkg/controller/apiextensions/composite/reconciler.go
View File

@ -418,6 +418,8 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
}
}
r.record.Event(cr, event.Normal(reasonCompose, "Successfully composed resources"))
published, err := r.composite.PublishConnection(ctx, cr, conn)
if err != nil {
log.Debug(errPublish, "error", err)
@ -430,17 +432,15 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
r.record.Event(cr, event.Normal(reasonPublish, "Successfully published connection details"))
}
// TODO(muvaf): Report which resources are not ready.
// TODO(muvaf): If a resource becomes Unavailable at some point, should we still
// report it as Creating?
wait := longWait
cr.SetConditions(runtimev1alpha1.Available())
// TODO(muvaf):
// * Report which resources are not ready.
// * If a resource becomes Unavailable at some point, should we still report
// it as Creating?
if ready != len(refs) {
cr.SetConditions(runtimev1alpha1.Creating())
wait = shortWait
return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
}
r.record.Event(cr, event.Normal(reasonCompose, "Successfully composed resources"))
return reconcile.Result{RequeueAfter: wait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
cr.SetConditions(runtimev1alpha1.Available())
return reconcile.Result{RequeueAfter: longWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
}

1
pkg/xcrd/crd_test.go
View File

@ -214,7 +214,6 @@ func TestForCompositeResource(t *testing.T) {
"apiVersion": {Type: "string"},
"name": {Type: "string"},
"kind": {Type: "string"},
"uid": {Type: "string"},
},
Required: []string{"apiVersion", "kind", "name"},
},

12
pkg/xcrd/schemas.go
View File

@ -18,6 +18,17 @@ package xcrd
import extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
// Label keys.
const (
LabelKeyNamePrefixForComposed = "crossplane.io/composite"
LabelKeyClaimName = "crossplane.io/claim-name"
LabelKeyClaimNamespace = "crossplane.io/claim-namespace"
)
// FilterClaimSpecProps is the list of XRC resource spec properties to filter
// out when translating an XRC into an XR.
var FilterClaimSpecProps = []string{"resourceRef", "writeConnectionSecretToRef"}
// TODO(negz): Add descriptions to schema fields.
// BaseProps is a partial OpenAPIV3Schema for the spec fields that Crossplane
@ -90,7 +101,6 @@ func CompositeResourceSpecProps() map[string]extv1.JSONSchemaProps {
"apiVersion": {Type: "string"},
"name": {Type: "string"},
"kind": {Type: "string"},
"uid": {Type: "string"},
},
Required: []string{"apiVersion", "kind", "name"},
},