Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

k8s API sets the default value of XR .spec.compositionUpdatePolicy field #4928

Merged
merged 1 commit into from
Nov 13, 2023
Merged

k8s API sets the default value of XR .spec.compositionUpdatePolicy field #4928

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 0 additions & 32 deletions internal/controller/apiextensions/composite/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,35 +399,3 @@ func (c *APINamingConfigurator) Configure(ctx context.Context, cp resource.Compo
meta.AddLabels(cp, map[string]string{xcrd.LabelKeyNamePrefixForComposed: cp.GetName()})
return errors.Wrap(c.client.Update(ctx, cp), errUpdateComposite)
}

// NewAPIDefaultCompositionUpdatePolicySelector returns a APIDefaultCompositionUpdatePolicySelector.
func NewAPIDefaultCompositionUpdatePolicySelector(c client.Client, ref corev1.ObjectReference, r event.Recorder) *APIDefaultCompositionUpdatePolicySelector {
return &APIDefaultCompositionUpdatePolicySelector{client: c, defRef: ref, recorder: r}
}

// APIDefaultCompositionUpdatePolicySelector selects the default composition update policy referenced in
// the definition of the resource if neither a reference nor selector is given
// in composite resource.
type APIDefaultCompositionUpdatePolicySelector struct {
client client.Client
defRef corev1.ObjectReference
recorder event.Recorder
}

// SelectCompositionUpdatePolicy selects the default composition update policy if neither a reference nor
// selector is given in composite resource.
func (s *APIDefaultCompositionUpdatePolicySelector) SelectCompositionUpdatePolicy(ctx context.Context, cp resource.Composite) error {
if cp.GetCompositionUpdatePolicy() != nil {
return nil
}
def := &v1.CompositeResourceDefinition{}
if err := s.client.Get(ctx, meta.NamespacedNameOf(&s.defRef), def); err != nil {
return errors.Wrap(err, errGetXRD)
}
if def.Spec.DefaultCompositionUpdatePolicy == nil {
return nil
}
cp.SetCompositionUpdatePolicy(def.Spec.DefaultCompositionUpdatePolicy)
s.recorder.Event(cp, event.Normal(reasonCompositionUpdatePolicy, "Default composition update policy has been selected"))
return nil
}
95 changes: 0 additions & 95 deletions internal/controller/apiextensions/composite/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,98 +998,3 @@ func TestAPINamingConfigurator(t *testing.T) {
})
}
}

func TestAPIDefaultCompositionUpdatePolicySelector(t *testing.T) {
errBoom := errors.New("boom")
manual := xpv1.UpdateManual
auto := xpv1.UpdateAutomatic
a, k := schema.EmptyObjectKind.GroupVersionKind().ToAPIVersionAndKind()
tref := v1.TypeReference{APIVersion: a, Kind: k}
comp := &v1.Composition{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: v1.CompositionSpec{
CompositeTypeRef: tref,
},
}
type args struct {
kube client.Client
defRef corev1.ObjectReference
cp resource.Composite
}
type want struct {
cp resource.Composite
err error
}
cases := map[string]struct {
reason string
args
want
}{
"AlreadyResolved": {
reason: "Should be no-op if a composition update policy is already specified",
args: args{
defRef: corev1.ObjectReference{},
cp: &fake.Composite{
CompositionUpdater: fake.CompositionUpdater{Policy: &manual},
},
},
want: want{
cp: &fake.Composite{
CompositionUpdater: fake.CompositionUpdater{Policy: &manual},
},
},
},
"GetDefinitionFailed": {
reason: "Should return error if XRD cannot be retrieved",
args: args{
kube: &test.MockClient{
MockGet: test.NewMockGetFn(errBoom),
},
cp: &fake.Composite{},
},
want: want{
err: errors.Wrap(errBoom, errGetXRD),
cp: &fake.Composite{},
},
},
"Success": {
reason: "Successfully set the default composition update policy",
args: args{
kube: &test.MockClient{
MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error {
switch cr := obj.(type) {
case *v1.CompositeResourceDefinition:
withRef := &v1.CompositeResourceDefinition{Spec: v1.CompositeResourceDefinitionSpec{DefaultCompositionUpdatePolicy: &auto}}
withRef.DeepCopyInto(cr)
return nil
case *v1.Composition:
comp.DeepCopyInto(cr)
return nil
}
return nil
},
},
cp: &fake.Composite{},
},
want: want{
cp: &fake.Composite{
CompositionUpdater: fake.CompositionUpdater{Policy: &auto},
},
},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
c := NewAPIDefaultCompositionUpdatePolicySelector(tc.args.kube, tc.args.defRef, event.NewNopRecorder())
err := c.SelectCompositionUpdatePolicy(context.Background(), tc.args.cp)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nSelectCompositionUpdatePolicy(...): -want, +got:\n%s", tc.reason, diff)
}
if diff := cmp.Diff(tc.want.cp, tc.args.cp); diff != "" {
t.Errorf("\n%s\nSelectCompositionUpdatePolicy(...): -want, +got:\n%s", tc.reason, diff)
}
})
}
}
29 changes: 0 additions & 29 deletions internal/controller/apiextensions/composite/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,6 @@ func (fn CompositionRevisionFetcherFn) Fetch(ctx context.Context, cr resource.Co
return fn(ctx, cr)
}

// A CompositionUpdatePolicySelector selects a composition update policy.
type CompositionUpdatePolicySelector interface {
SelectCompositionUpdatePolicy(ctx context.Context, cr resource.Composite) error
}

// A CompositionUpdatePolicySelectorFn selects a composition update policy.
type CompositionUpdatePolicySelectorFn func(ctx context.Context, cr resource.Composite) error

// SelectCompositionUpdatePolicy for the supplied composite resource.
func (fn CompositionUpdatePolicySelectorFn) SelectCompositionUpdatePolicy(ctx context.Context, cr resource.Composite) error {
return fn(ctx, cr)
}

// EnvironmentSelector selects environment references for a composition environment.
type EnvironmentSelector interface {
SelectEnvironment(ctx context.Context, cr resource.Composite, rev *v1.CompositionRevision) error
Expand Down Expand Up @@ -293,14 +280,6 @@ func WithCompositionSelector(s CompositionSelector) ReconcilerOption {
}
}

// WithCompositionUpdatePolicySelector specifies how the composition update policy to be used should be
// selected.
func WithCompositionUpdatePolicySelector(s CompositionUpdatePolicySelector) ReconcilerOption {
return func(r *Reconciler) {
r.composite.CompositionUpdatePolicySelector = s
}
}

// WithEnvironmentSelector specifies how the environment to be used should be
// selected.
func WithEnvironmentSelector(s EnvironmentSelector) ReconcilerOption {
Expand Down Expand Up @@ -374,7 +353,6 @@ type environment struct {
type compositeResource struct {
resource.Finalizer
CompositionSelector
CompositionUpdatePolicySelector
EnvironmentSelector
Configurator
managed.ConnectionPublisher
Expand Down Expand Up @@ -538,13 +516,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, xr), errUpdateStatus)
}

if err := r.composite.SelectCompositionUpdatePolicy(ctx, xr); err != nil {
err = errors.Wrap(err, errSelectCompUpdatePolicy)
r.record.Event(xr, event.Warning(reasonResolve, err))
xr.SetConditions(xpv1.ReconcileError(err))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, xr), errUpdateStatus)
}

orig := xr.GetCompositionReference()
if err := r.composite.SelectComposition(ctx, xr); err != nil {
err = errors.Wrap(err, errSelectComp)
Expand Down
34 changes: 0 additions & 34 deletions internal/controller/apiextensions/composite/reconciler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,25 +209,6 @@ func TestReconcile(t *testing.T) {
r: reconcile.Result{Requeue: true},
},
},
"SelectCompositionUpdatePolicyError": {
reason: "We should return any error encountered while selecting a composition update policy.",
args: args{
mgr: &fake.Manager{},
opts: []ReconcilerOption{
WithClient(&test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockStatusUpdate: WantComposite(t, NewComposite(func(cr resource.Composite) {
cr.SetConditions(xpv1.ReconcileError(errors.Wrap(errBoom, errSelectCompUpdatePolicy)))
})),
}),
WithCompositeFinalizer(resource.NewNopFinalizer()),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return errBoom })),
},
},
want: want{
r: reconcile.Result{Requeue: true},
},
},
"SelectCompositionError": {
reason: "We should return any error encountered while selecting a composition.",
args: args{
Expand All @@ -243,7 +224,6 @@ func TestReconcile(t *testing.T) {
WithCompositionSelector(CompositionSelectorFn(func(_ context.Context, _ resource.Composite) error {
return errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand All @@ -270,7 +250,6 @@ func TestReconcile(t *testing.T) {
WithCompositionRevisionFetcher(CompositionRevisionFetcherFn(func(_ context.Context, _ resource.Composite) (*v1.CompositionRevision, error) {
return nil, errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -300,9 +279,6 @@ func TestReconcile(t *testing.T) {
WithCompositionRevisionValidator(CompositionRevisionValidatorFn(func(_ *v1.CompositionRevision) error {
return errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error {
return nil
})),
},
},
want: want{
Expand Down Expand Up @@ -333,7 +309,6 @@ func TestReconcile(t *testing.T) {
WithConfigurator(ConfiguratorFn(func(_ context.Context, _ resource.Composite, _ *v1.CompositionRevision) error {
return errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -362,7 +337,6 @@ func TestReconcile(t *testing.T) {
WithEnvironmentSelector(EnvironmentSelectorFn(func(ctx context.Context, cr resource.Composite, rev *v1.CompositionRevision) error {
return errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -391,7 +365,6 @@ func TestReconcile(t *testing.T) {
WithEnvironmentFetcher(EnvironmentFetcherFn(func(ctx context.Context, req EnvironmentFetcherRequest) (*Environment, error) {
return nil, errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -425,7 +398,6 @@ func TestReconcile(t *testing.T) {
WithComposer(ComposerFn(func(ctx context.Context, xr *composite.Unstructured, req CompositionRequest) (CompositionResult, error) {
return CompositionResult{}, errBoom
})),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -464,7 +436,6 @@ func TestReconcile(t *testing.T) {
return false, errBoom
},
}),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -508,7 +479,6 @@ func TestReconcile(t *testing.T) {
return false, nil
},
}),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -570,7 +540,6 @@ func TestReconcile(t *testing.T) {
return false, nil
},
}),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -617,7 +586,6 @@ func TestReconcile(t *testing.T) {
return true, nil
},
}),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -701,7 +669,6 @@ func TestReconcile(t *testing.T) {
return true, nil
},
}),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down Expand Up @@ -748,7 +715,6 @@ func TestReconcile(t *testing.T) {
return true, nil
},
}),
WithCompositionUpdatePolicySelector(CompositionUpdatePolicySelectorFn(func(ctx context.Context, cr resource.Composite) error { return nil })),
},
},
want: want{
Expand Down
1 change: 0 additions & 1 deletion internal/controller/apiextensions/definition/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,6 @@ func CompositeReconcilerOptions(co apiextensionscontroller.Options, d *v1.Compos
composite.NewAPIDefaultCompositionSelector(c, *meta.ReferenceTo(d, v1.CompositeResourceDefinitionGroupVersionKind), e),
composite.NewAPILabelSelectorResolver(c),
)),
composite.WithCompositionUpdatePolicySelector(composite.NewAPIDefaultCompositionUpdatePolicySelector(c, *meta.ReferenceTo(d, v1.CompositeResourceDefinitionGroupVersionKind), e)),
composite.WithLogger(l.WithValues("controller", composite.ControllerName(d.GetName()))),
composite.WithRecorder(e.WithAnnotations("controller", composite.ControllerName(d.GetName()))),
composite.WithPollInterval(co.PollInterval),
Expand Down
8 changes: 7 additions & 1 deletion internal/xcrd/crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,13 @@ func ForCompositeResource(xrd *v1.CompositeResourceDefinition) (*extv1.CustomRes
return nil, errors.Wrapf(err, errFmtGenCrd, "Composite Resource", xrd.Name)
}
crdv.AdditionalPrinterColumns = append(crdv.AdditionalPrinterColumns, CompositeResourcePrinterColumns()...)
for k, v := range CompositeResourceSpecProps() {
props := CompositeResourceSpecProps()
if xrd.Spec.DefaultCompositionUpdatePolicy != nil {
cup := props["compositionUpdatePolicy"]
cup.Default = &extv1.JSON{Raw: []byte(fmt.Sprintf("\"%s\"", *xrd.Spec.DefaultCompositionUpdatePolicy))}
props["compositionUpdatePolicy"] = cup
}
for k, v := range props {
crdv.Schema.OpenAPIV3Schema.Properties["spec"].Properties[k] = v
}
crd.Spec.Versions[i] = *crdv
Expand Down