Add a new policy about resolving references for every reconcile loop

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
This commit is contained in:
Sergen Yalçın 2022-04-16 15:58:24 +03:00
parent ac7cf2045e
commit ec82fef853
No known key found for this signature in database
GPG Key ID: 1EC8D6C7CC79DB6B
5 changed files with 115 additions and 20 deletions

View File

@ -85,19 +85,39 @@ const (
// When the ReferenceResolutionPolicy is set to ReferencePolicyOptional the // When the ReferenceResolutionPolicy is set to ReferencePolicyOptional the
// execution could continue even if the reference cannot be resolved. // execution could continue even if the reference cannot be resolved.
ReferencePolicyOptional ReferenceResolutionPolicy = "Optional" ReferencePolicyOptional ReferenceResolutionPolicy = "Optional"
// ReferencePolicyAlways is a resolution option.
// When the ReferenceResolutionPolicy is set to ReferencePolicyAlways the
// reference will be tried to resolve for every reconcile loop.
ReferencePolicyAlways ReferenceResolutionPolicy = "Always"
) )
// A Reference to a named object. // A Reference to a named object.
type Reference struct { type Reference struct {
// Name of the referenced object. // Name of the referenced object.
Name string `json:"name"` Name string `json:"name"`
// Policy of the referenced object. // Policies of the referenced object.
Policy ReferenceResolutionPolicy `json:"policy,omitempty"` Policies []ReferenceResolutionPolicy `json:"policies,omitempty"`
} }
// IsReferenceResolutionPolicyOptional checks whether the resolution policy of relevant reference is Optional. // IsReferenceResolutionPolicyOptional checks whether the resolution policy of relevant reference is Optional.
func (in *Reference) IsReferenceResolutionPolicyOptional() bool { func (in *Reference) IsReferenceResolutionPolicyOptional() bool {
return in.Policy == ReferencePolicyOptional for _, policy := range in.Policies {
if policy == ReferencePolicyOptional {
return true
}
}
return false
}
// IsReferenceResolutionPolicyAlways checks whether the resolution policy of relevant reference is Always.
func (in *Reference) IsReferenceResolutionPolicyAlways() bool {
for _, policy := range in.Policies {
if policy == ReferencePolicyAlways {
return true
}
}
return false
} }
// A TypedReference refers to an object by Name, Kind, and APIVersion. It is // A TypedReference refers to an object by Name, Kind, and APIVersion. It is

View File

@ -248,7 +248,7 @@ func (in *ProviderConfigStatus) DeepCopy() *ProviderConfigStatus {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *ProviderConfigUsage) DeepCopyInto(out *ProviderConfigUsage) { func (in *ProviderConfigUsage) DeepCopyInto(out *ProviderConfigUsage) {
*out = *in *out = *in
out.ProviderConfigReference = in.ProviderConfigReference in.ProviderConfigReference.DeepCopyInto(&out.ProviderConfigReference)
out.ResourceReference = in.ResourceReference out.ResourceReference = in.ResourceReference
} }
@ -273,7 +273,7 @@ func (in *PublishConnectionDetailsTo) DeepCopyInto(out *PublishConnectionDetails
if in.SecretStoreConfigRef != nil { if in.SecretStoreConfigRef != nil {
in, out := &in.SecretStoreConfigRef, &out.SecretStoreConfigRef in, out := &in.SecretStoreConfigRef, &out.SecretStoreConfigRef
*out = new(Reference) *out = new(Reference)
**out = **in (*in).DeepCopyInto(*out)
} }
} }
@ -290,6 +290,11 @@ func (in *PublishConnectionDetailsTo) DeepCopy() *PublishConnectionDetailsTo {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *Reference) DeepCopyInto(out *Reference) { func (in *Reference) DeepCopyInto(out *Reference) {
*out = *in *out = *in
if in.Policies != nil {
in, out := &in.Policies, &out.Policies
*out = make([]ReferenceResolutionPolicy, len(*in))
copy(*out, *in)
}
} }
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Reference. // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Reference.
@ -318,12 +323,12 @@ func (in *ResourceSpec) DeepCopyInto(out *ResourceSpec) {
if in.ProviderConfigReference != nil { if in.ProviderConfigReference != nil {
in, out := &in.ProviderConfigReference, &out.ProviderConfigReference in, out := &in.ProviderConfigReference, &out.ProviderConfigReference
*out = new(Reference) *out = new(Reference)
**out = **in (*in).DeepCopyInto(*out)
} }
if in.ProviderReference != nil { if in.ProviderReference != nil {
in, out := &in.ProviderReference, &out.ProviderReference in, out := &in.ProviderReference, &out.ProviderReference
*out = new(Reference) *out = new(Reference)
**out = **in (*in).DeepCopyInto(*out)
} }
} }

View File

@ -113,10 +113,17 @@ type ResolutionRequest struct {
// IsNoOp returns true if the supplied ResolutionRequest cannot or should not be // IsNoOp returns true if the supplied ResolutionRequest cannot or should not be
// processed. // processed.
func (rr ResolutionRequest) IsNoOp() bool { func (rr ResolutionRequest) IsNoOp() bool {
// We don't resolve values that are already set; we effectively cache isAlways := false
// resolved values. The CR author can invalidate the cache and trigger a new if rr.Reference != nil {
// resolution by explicitly clearing the resolved value. if rr.Reference.IsReferenceResolutionPolicyAlways() {
if rr.CurrentValue != "" { isAlways = true
}
}
// We don't resolve values that are already set (if reference resolution policy
// is not set to Always); we effectively cache resolved values. The CR author
// can invalidate the cache and trigger a new resolution by explicitly clearing
// the resolved value.
if rr.CurrentValue != "" && !isAlways {
return true return true
} }
@ -154,12 +161,19 @@ type MultiResolutionRequest struct {
// IsNoOp returns true if the supplied MultiResolutionRequest cannot or should // IsNoOp returns true if the supplied MultiResolutionRequest cannot or should
// not be processed. // not be processed.
func (rr MultiResolutionRequest) IsNoOp() bool { func (rr MultiResolutionRequest) IsNoOp() bool {
// We don't resolve values that are already set; we effectively cache isAlways := false
// resolved values. The CR author can invalidate the cache and trigger a new for _, r := range rr.References {
// resolution by explicitly clearing the resolved values. This is a little if r.IsReferenceResolutionPolicyAlways() {
// unintuitive for the APIMultiResolver but mimics the UX of the APIResolver isAlways = true
// and simplifies the overall mental model. break
if len(rr.CurrentValues) > 0 { }
}
// We don't resolve values that are already set (if reference resolution policy
// is not set to Always); we effectively cache resolved values. The CR author
// can invalidate the cache and trigger a new resolution by explicitly clearing
// the resolved values. This is a little unintuitive for the APIMultiResolver
// but mimics the UX of the APIResolver and simplifies the overall mental model.
if len(rr.CurrentValues) > 0 && !isAlways {
return true return true
} }

View File

@ -91,7 +91,8 @@ func TestResolve(t *testing.T) {
now := metav1.Now() now := metav1.Now()
value := "coolv" value := "coolv"
ref := &xpv1.Reference{Name: "cool"} ref := &xpv1.Reference{Name: "cool"}
optionalRef := &xpv1.Reference{Name: "cool", Policy: xpv1.ReferencePolicyOptional} optionalRef := &xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyOptional}}
alwaysRef := &xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyAlways}}
controlled := &fake.Managed{} controlled := &fake.Managed{}
controlled.SetName(value) controlled.SetName(value)
@ -135,6 +136,32 @@ func TestResolve(t *testing.T) {
err: nil, err: nil,
}, },
}, },
"AlwaysResolve": {
reason: "Should not return early if the current value is non-zero, when the resolution policy is set to" +
"Always",
c: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
meta.SetExternalName(obj.(metav1.Object), value)
return nil
}),
},
from: &fake.Managed{},
args: args{
req: ResolutionRequest{
Reference: alwaysRef,
To: To{Managed: &fake.Managed{}},
Extract: ExternalName(),
CurrentValue: "oldValue",
},
},
want: want{
rsp: ResolutionResponse{
ResolvedValue: value,
ResolvedReference: alwaysRef,
},
err: nil,
},
},
"Unresolvable": { "Unresolvable": {
reason: "Should return early if neither a reference or selector were provided", reason: "Should return early if neither a reference or selector were provided",
from: &fake.Managed{}, from: &fake.Managed{},
@ -303,7 +330,8 @@ func TestResolveMultiple(t *testing.T) {
now := metav1.Now() now := metav1.Now()
value := "coolv" value := "coolv"
ref := xpv1.Reference{Name: "cool"} ref := xpv1.Reference{Name: "cool"}
optionalRef := xpv1.Reference{Name: "cool", Policy: xpv1.ReferencePolicyOptional} optionalRef := xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyOptional}}
alwaysRef := xpv1.Reference{Name: "cool", Policies: []xpv1.ReferenceResolutionPolicy{xpv1.ReferencePolicyAlways}}
controlled := &fake.Managed{} controlled := &fake.Managed{}
controlled.SetName(value) controlled.SetName(value)
@ -347,6 +375,32 @@ func TestResolveMultiple(t *testing.T) {
err: nil, err: nil,
}, },
}, },
"AlwaysResolve": {
reason: "Should not return early if the current value is non-zero, when the resolution policy is set to" +
"Always",
c: &test.MockClient{
MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
meta.SetExternalName(obj.(metav1.Object), value)
return nil
}),
},
from: &fake.Managed{},
args: args{
req: MultiResolutionRequest{
References: []xpv1.Reference{alwaysRef},
To: To{Managed: &fake.Managed{}},
Extract: ExternalName(),
CurrentValues: []string{"oldValue"},
},
},
want: want{
rsp: MultiResolutionResponse{
ResolvedValues: []string{value},
ResolvedReferences: []xpv1.Reference{alwaysRef},
},
err: nil,
},
},
"Unresolvable": { "Unresolvable": {
reason: "Should return early if neither a reference or selector were provided", reason: "Should return early if neither a reference or selector were provided",
from: &fake.Managed{}, from: &fake.Managed{},

View File

@ -20,6 +20,8 @@ import (
"context" "context"
"os" "os"
"github.com/google/go-cmp/cmp"
"github.com/spf13/afero" "github.com/spf13/afero"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -153,7 +155,7 @@ func (u *ProviderConfigUsageTracker) Track(ctx context.Context, mg Managed) erro
err := u.c.Apply(ctx, pcu, err := u.c.Apply(ctx, pcu,
MustBeControllableBy(mg.GetUID()), MustBeControllableBy(mg.GetUID()),
AllowUpdateIf(func(current, _ runtime.Object) bool { AllowUpdateIf(func(current, _ runtime.Object) bool {
return current.(ProviderConfigUsage).GetProviderConfigReference() != pcu.GetProviderConfigReference() return !cmp.Equal(current.(ProviderConfigUsage).GetProviderConfigReference(), pcu.GetProviderConfigReference())
}), }),
) )
return errors.Wrap(Ignore(IsNotAllowed, err), errApplyPCU) return errors.Wrap(Ignore(IsNotAllowed, err), errApplyPCU)