From c487726c969e7dd27aaed33082306f9910337b3d Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Wed, 11 May 2022 21:52:45 -0400 Subject: [PATCH 1/4] Handle alternate forms of resource status We previously assumed that a resource either conformed to our notion of status or didn't have a status field at all. If a status field existed, but either didn't have the structure we expected or did have the structure but was a pointer, would err. Now we gracefully unpack the status as best we can, and safely ignore cases where the structure does not conform to our ideal structure. In those cases the functionality that would be applied is skipped. This includes: - calling `status.InitializeConditions()` - setting `status.ObservedGeneration` - normalizing the LastTransitionTime for conditions Signed-off-by: Scott Andrews --- internal/resources/dies/dies.go | 20 +- internal/resources/dies/zz_generated.die.go | 361 ++++++++++++++++++ .../resources/dies/zz_generated.die_test.go | 27 ++ .../resources/resource_with_empty_status.go | 48 +++ .../resources/resource_with_nilable_status.go | 62 +++ internal/resources/zz_generated.deepcopy.go | 137 +++++++ reconcilers/reconcilers.go | 87 +++-- reconcilers/reconcilers_test.go | 184 ++++++++- 8 files changed, 899 insertions(+), 27 deletions(-) create mode 100644 internal/resources/resource_with_empty_status.go create mode 100644 internal/resources/resource_with_nilable_status.go diff --git a/internal/resources/dies/dies.go b/internal/resources/dies/dies.go index 19e1f9d..826888e 100644 --- a/internal/resources/dies/dies.go +++ b/internal/resources/dies/dies.go @@ -56,5 +56,23 @@ func (d *TestResourceStatusDie) AddField(key, value string) *TestResourceStatusD }) } -// +die:object=true +// +die:object=true,spec=TestResourceSpec +type _ = resources.TestResourceEmptyStatus + +// +die +type _ = resources.TestResourceEmptyStatusStatus + +// +die:object=true,spec=TestResourceSpec type _ = resources.TestResourceNoStatus + +// +die:object=true,spec=TestResourceSpec +type _ = resources.TestResourceNilableStatus + +// StatusDie stamps the resource's status field with a mutable die. +func (d *TestResourceNilableStatusDie) StatusDie(fn func(d *TestResourceStatusDie)) *TestResourceNilableStatusDie { + return d.DieStamp(func(r *resources.TestResourceNilableStatus) { + d := TestResourceStatusBlank.DieImmutable(false).DieFeedPtr(r.Status) + fn(d) + r.Status = d.DieReleasePtr() + }) +} diff --git a/internal/resources/dies/zz_generated.die.go b/internal/resources/dies/zz_generated.die.go index a8b415c..bfa0113 100644 --- a/internal/resources/dies/zz_generated.die.go +++ b/internal/resources/dies/zz_generated.die.go @@ -339,6 +339,220 @@ func (d *TestResourceStatusDie) Fields(v map[string]string) *TestResourceStatusD }) } +var TestResourceEmptyStatusBlank = (&TestResourceEmptyStatusDie{}).DieFeed(resources.TestResourceEmptyStatus{}) + +type TestResourceEmptyStatusDie struct { + v1.FrozenObjectMeta + mutable bool + r resources.TestResourceEmptyStatus +} + +// DieImmutable returns a new die for the current die's state that is either mutable (`false`) or immutable (`true`). +func (d *TestResourceEmptyStatusDie) DieImmutable(immutable bool) *TestResourceEmptyStatusDie { + if d.mutable == !immutable { + return d + } + d = d.DeepCopy() + d.mutable = !immutable + return d +} + +// DieFeed returns a new die with the provided resource. +func (d *TestResourceEmptyStatusDie) DieFeed(r resources.TestResourceEmptyStatus) *TestResourceEmptyStatusDie { + if d.mutable { + d.FrozenObjectMeta = v1.FreezeObjectMeta(r.ObjectMeta) + d.r = r + return d + } + return &TestResourceEmptyStatusDie{ + FrozenObjectMeta: v1.FreezeObjectMeta(r.ObjectMeta), + mutable: d.mutable, + r: r, + } +} + +// DieFeedPtr returns a new die with the provided resource pointer. If the resource is nil, the empty value is used instead. +func (d *TestResourceEmptyStatusDie) DieFeedPtr(r *resources.TestResourceEmptyStatus) *TestResourceEmptyStatusDie { + if r == nil { + r = &resources.TestResourceEmptyStatus{} + } + return d.DieFeed(*r) +} + +// DieRelease returns the resource managed by the die. +func (d *TestResourceEmptyStatusDie) DieRelease() resources.TestResourceEmptyStatus { + if d.mutable { + return d.r + } + return *d.r.DeepCopy() +} + +// DieReleasePtr returns a pointer to the resource managed by the die. +func (d *TestResourceEmptyStatusDie) DieReleasePtr() *resources.TestResourceEmptyStatus { + r := d.DieRelease() + return &r +} + +// DieReleaseUnstructured returns the resource managed by the die as an unstructured object. +func (d *TestResourceEmptyStatusDie) DieReleaseUnstructured() runtime.Unstructured { + r := d.DieReleasePtr() + u, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(r) + return &unstructured.Unstructured{ + Object: u, + } +} + +// DieStamp returns a new die with the resource passed to the callback function. The resource is mutable. +func (d *TestResourceEmptyStatusDie) DieStamp(fn func(r *resources.TestResourceEmptyStatus)) *TestResourceEmptyStatusDie { + r := d.DieRelease() + fn(&r) + return d.DieFeed(r) +} + +// DeepCopy returns a new die with equivalent state. Useful for snapshotting a mutable die. +func (d *TestResourceEmptyStatusDie) DeepCopy() *TestResourceEmptyStatusDie { + r := *d.r.DeepCopy() + return &TestResourceEmptyStatusDie{ + FrozenObjectMeta: v1.FreezeObjectMeta(r.ObjectMeta), + mutable: d.mutable, + r: r, + } +} + +var _ runtime.Object = (*TestResourceEmptyStatusDie)(nil) + +func (d *TestResourceEmptyStatusDie) DeepCopyObject() runtime.Object { + return d.r.DeepCopy() +} + +func (d *TestResourceEmptyStatusDie) GetObjectKind() schema.ObjectKind { + r := d.DieRelease() + return r.GetObjectKind() +} + +func (d *TestResourceEmptyStatusDie) MarshalJSON() ([]byte, error) { + return json.Marshal(d.r) +} + +func (d *TestResourceEmptyStatusDie) UnmarshalJSON(b []byte) error { + if d == TestResourceEmptyStatusBlank { + return fmtx.Errorf("cannot unmarshal into the blank die, create a copy first") + } + if !d.mutable { + return fmtx.Errorf("cannot unmarshal into immutable dies, create a mutable version first") + } + r := &resources.TestResourceEmptyStatus{} + err := json.Unmarshal(b, r) + *d = *d.DieFeed(*r) + return err +} + +// MetadataDie stamps the resource's ObjectMeta field with a mutable die. +func (d *TestResourceEmptyStatusDie) MetadataDie(fn func(d *v1.ObjectMetaDie)) *TestResourceEmptyStatusDie { + return d.DieStamp(func(r *resources.TestResourceEmptyStatus) { + d := v1.ObjectMetaBlank.DieImmutable(false).DieFeed(r.ObjectMeta) + fn(d) + r.ObjectMeta = d.DieRelease() + }) +} + +// SpecDie stamps the resource's spec field with a mutable die. +func (d *TestResourceEmptyStatusDie) SpecDie(fn func(d *TestResourceSpecDie)) *TestResourceEmptyStatusDie { + return d.DieStamp(func(r *resources.TestResourceEmptyStatus) { + d := TestResourceSpecBlank.DieImmutable(false).DieFeed(r.Spec) + fn(d) + r.Spec = d.DieRelease() + }) +} + +// StatusDie stamps the resource's status field with a mutable die. +func (d *TestResourceEmptyStatusDie) StatusDie(fn func(d *TestResourceEmptyStatusStatusDie)) *TestResourceEmptyStatusDie { + return d.DieStamp(func(r *resources.TestResourceEmptyStatus) { + d := TestResourceEmptyStatusStatusBlank.DieImmutable(false).DieFeed(r.Status) + fn(d) + r.Status = d.DieRelease() + }) +} + +func (d *TestResourceEmptyStatusDie) Spec(v resources.TestResourceSpec) *TestResourceEmptyStatusDie { + return d.DieStamp(func(r *resources.TestResourceEmptyStatus) { + r.Spec = v + }) +} + +func (d *TestResourceEmptyStatusDie) Status(v resources.TestResourceEmptyStatusStatus) *TestResourceEmptyStatusDie { + return d.DieStamp(func(r *resources.TestResourceEmptyStatus) { + r.Status = v + }) +} + +var TestResourceEmptyStatusStatusBlank = (&TestResourceEmptyStatusStatusDie{}).DieFeed(resources.TestResourceEmptyStatusStatus{}) + +type TestResourceEmptyStatusStatusDie struct { + mutable bool + r resources.TestResourceEmptyStatusStatus +} + +// DieImmutable returns a new die for the current die's state that is either mutable (`false`) or immutable (`true`). +func (d *TestResourceEmptyStatusStatusDie) DieImmutable(immutable bool) *TestResourceEmptyStatusStatusDie { + if d.mutable == !immutable { + return d + } + d = d.DeepCopy() + d.mutable = !immutable + return d +} + +// DieFeed returns a new die with the provided resource. +func (d *TestResourceEmptyStatusStatusDie) DieFeed(r resources.TestResourceEmptyStatusStatus) *TestResourceEmptyStatusStatusDie { + if d.mutable { + d.r = r + return d + } + return &TestResourceEmptyStatusStatusDie{ + mutable: d.mutable, + r: r, + } +} + +// DieFeedPtr returns a new die with the provided resource pointer. If the resource is nil, the empty value is used instead. +func (d *TestResourceEmptyStatusStatusDie) DieFeedPtr(r *resources.TestResourceEmptyStatusStatus) *TestResourceEmptyStatusStatusDie { + if r == nil { + r = &resources.TestResourceEmptyStatusStatus{} + } + return d.DieFeed(*r) +} + +// DieRelease returns the resource managed by the die. +func (d *TestResourceEmptyStatusStatusDie) DieRelease() resources.TestResourceEmptyStatusStatus { + if d.mutable { + return d.r + } + return *d.r.DeepCopy() +} + +// DieReleasePtr returns a pointer to the resource managed by the die. +func (d *TestResourceEmptyStatusStatusDie) DieReleasePtr() *resources.TestResourceEmptyStatusStatus { + r := d.DieRelease() + return &r +} + +// DieStamp returns a new die with the resource passed to the callback function. The resource is mutable. +func (d *TestResourceEmptyStatusStatusDie) DieStamp(fn func(r *resources.TestResourceEmptyStatusStatus)) *TestResourceEmptyStatusStatusDie { + r := d.DieRelease() + fn(&r) + return d.DieFeed(r) +} + +// DeepCopy returns a new die with equivalent state. Useful for snapshotting a mutable die. +func (d *TestResourceEmptyStatusStatusDie) DeepCopy() *TestResourceEmptyStatusStatusDie { + r := *d.r.DeepCopy() + return &TestResourceEmptyStatusStatusDie{ + mutable: d.mutable, + r: r, + } +} + var TestResourceNoStatusBlank = (&TestResourceNoStatusDie{}).DieFeed(resources.TestResourceNoStatus{}) type TestResourceNoStatusDie struct { @@ -456,8 +670,155 @@ func (d *TestResourceNoStatusDie) MetadataDie(fn func(d *v1.ObjectMetaDie)) *Tes }) } +// SpecDie stamps the resource's spec field with a mutable die. +func (d *TestResourceNoStatusDie) SpecDie(fn func(d *TestResourceSpecDie)) *TestResourceNoStatusDie { + return d.DieStamp(func(r *resources.TestResourceNoStatus) { + d := TestResourceSpecBlank.DieImmutable(false).DieFeed(r.Spec) + fn(d) + r.Spec = d.DieRelease() + }) +} + func (d *TestResourceNoStatusDie) Spec(v resources.TestResourceSpec) *TestResourceNoStatusDie { return d.DieStamp(func(r *resources.TestResourceNoStatus) { r.Spec = v }) } + +var TestResourceNilableStatusBlank = (&TestResourceNilableStatusDie{}).DieFeed(resources.TestResourceNilableStatus{}) + +type TestResourceNilableStatusDie struct { + v1.FrozenObjectMeta + mutable bool + r resources.TestResourceNilableStatus +} + +// DieImmutable returns a new die for the current die's state that is either mutable (`false`) or immutable (`true`). +func (d *TestResourceNilableStatusDie) DieImmutable(immutable bool) *TestResourceNilableStatusDie { + if d.mutable == !immutable { + return d + } + d = d.DeepCopy() + d.mutable = !immutable + return d +} + +// DieFeed returns a new die with the provided resource. +func (d *TestResourceNilableStatusDie) DieFeed(r resources.TestResourceNilableStatus) *TestResourceNilableStatusDie { + if d.mutable { + d.FrozenObjectMeta = v1.FreezeObjectMeta(r.ObjectMeta) + d.r = r + return d + } + return &TestResourceNilableStatusDie{ + FrozenObjectMeta: v1.FreezeObjectMeta(r.ObjectMeta), + mutable: d.mutable, + r: r, + } +} + +// DieFeedPtr returns a new die with the provided resource pointer. If the resource is nil, the empty value is used instead. +func (d *TestResourceNilableStatusDie) DieFeedPtr(r *resources.TestResourceNilableStatus) *TestResourceNilableStatusDie { + if r == nil { + r = &resources.TestResourceNilableStatus{} + } + return d.DieFeed(*r) +} + +// DieRelease returns the resource managed by the die. +func (d *TestResourceNilableStatusDie) DieRelease() resources.TestResourceNilableStatus { + if d.mutable { + return d.r + } + return *d.r.DeepCopy() +} + +// DieReleasePtr returns a pointer to the resource managed by the die. +func (d *TestResourceNilableStatusDie) DieReleasePtr() *resources.TestResourceNilableStatus { + r := d.DieRelease() + return &r +} + +// DieReleaseUnstructured returns the resource managed by the die as an unstructured object. +func (d *TestResourceNilableStatusDie) DieReleaseUnstructured() runtime.Unstructured { + r := d.DieReleasePtr() + u, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(r) + return &unstructured.Unstructured{ + Object: u, + } +} + +// DieStamp returns a new die with the resource passed to the callback function. The resource is mutable. +func (d *TestResourceNilableStatusDie) DieStamp(fn func(r *resources.TestResourceNilableStatus)) *TestResourceNilableStatusDie { + r := d.DieRelease() + fn(&r) + return d.DieFeed(r) +} + +// DeepCopy returns a new die with equivalent state. Useful for snapshotting a mutable die. +func (d *TestResourceNilableStatusDie) DeepCopy() *TestResourceNilableStatusDie { + r := *d.r.DeepCopy() + return &TestResourceNilableStatusDie{ + FrozenObjectMeta: v1.FreezeObjectMeta(r.ObjectMeta), + mutable: d.mutable, + r: r, + } +} + +var _ runtime.Object = (*TestResourceNilableStatusDie)(nil) + +func (d *TestResourceNilableStatusDie) DeepCopyObject() runtime.Object { + return d.r.DeepCopy() +} + +func (d *TestResourceNilableStatusDie) GetObjectKind() schema.ObjectKind { + r := d.DieRelease() + return r.GetObjectKind() +} + +func (d *TestResourceNilableStatusDie) MarshalJSON() ([]byte, error) { + return json.Marshal(d.r) +} + +func (d *TestResourceNilableStatusDie) UnmarshalJSON(b []byte) error { + if d == TestResourceNilableStatusBlank { + return fmtx.Errorf("cannot unmarshal into the blank die, create a copy first") + } + if !d.mutable { + return fmtx.Errorf("cannot unmarshal into immutable dies, create a mutable version first") + } + r := &resources.TestResourceNilableStatus{} + err := json.Unmarshal(b, r) + *d = *d.DieFeed(*r) + return err +} + +// MetadataDie stamps the resource's ObjectMeta field with a mutable die. +func (d *TestResourceNilableStatusDie) MetadataDie(fn func(d *v1.ObjectMetaDie)) *TestResourceNilableStatusDie { + return d.DieStamp(func(r *resources.TestResourceNilableStatus) { + d := v1.ObjectMetaBlank.DieImmutable(false).DieFeed(r.ObjectMeta) + fn(d) + r.ObjectMeta = d.DieRelease() + }) +} + +// SpecDie stamps the resource's spec field with a mutable die. +func (d *TestResourceNilableStatusDie) SpecDie(fn func(d *TestResourceSpecDie)) *TestResourceNilableStatusDie { + return d.DieStamp(func(r *resources.TestResourceNilableStatus) { + d := TestResourceSpecBlank.DieImmutable(false).DieFeed(r.Spec) + fn(d) + r.Spec = d.DieRelease() + }) +} + +func (d *TestResourceNilableStatusDie) Spec(v resources.TestResourceSpec) *TestResourceNilableStatusDie { + return d.DieStamp(func(r *resources.TestResourceNilableStatus) { + r.Spec = v + }) +} + +func (d *TestResourceNilableStatusDie) Status(v *resources.TestResourceStatus) *TestResourceNilableStatusDie { + return d.DieStamp(func(r *resources.TestResourceNilableStatus) { + r.Status = v + }) +} diff --git a/internal/resources/dies/zz_generated.die_test.go b/internal/resources/dies/zz_generated.die_test.go index 746a419..3919c65 100644 --- a/internal/resources/dies/zz_generated.die_test.go +++ b/internal/resources/dies/zz_generated.die_test.go @@ -42,6 +42,24 @@ func TestTestResourceStatusDie_MissingMethods(t *testingx.T) { } } +func TestTestResourceEmptyStatusDie_MissingMethods(t *testingx.T) { + die := TestResourceEmptyStatusBlank + ignore := []string{"TypeMeta", "ObjectMeta"} + diff := testing.DieFieldDiff(die).Delete(ignore...) + if diff.Len() != 0 { + t.Errorf("found missing fields for TestResourceEmptyStatusDie: %s", diff.List()) + } +} + +func TestTestResourceEmptyStatusStatusDie_MissingMethods(t *testingx.T) { + die := TestResourceEmptyStatusStatusBlank + ignore := []string{} + diff := testing.DieFieldDiff(die).Delete(ignore...) + if diff.Len() != 0 { + t.Errorf("found missing fields for TestResourceEmptyStatusStatusDie: %s", diff.List()) + } +} + func TestTestResourceNoStatusDie_MissingMethods(t *testingx.T) { die := TestResourceNoStatusBlank ignore := []string{"TypeMeta", "ObjectMeta"} @@ -50,3 +68,12 @@ func TestTestResourceNoStatusDie_MissingMethods(t *testingx.T) { t.Errorf("found missing fields for TestResourceNoStatusDie: %s", diff.List()) } } + +func TestTestResourceNilableStatusDie_MissingMethods(t *testingx.T) { + die := TestResourceNilableStatusBlank + ignore := []string{"TypeMeta", "ObjectMeta"} + diff := testing.DieFieldDiff(die).Delete(ignore...) + if diff.Len() != 0 { + t.Errorf("found missing fields for TestResourceNilableStatusDie: %s", diff.List()) + } +} diff --git a/internal/resources/resource_with_empty_status.go b/internal/resources/resource_with_empty_status.go new file mode 100644 index 0000000..c27959b --- /dev/null +++ b/internal/resources/resource_with_empty_status.go @@ -0,0 +1,48 @@ +/* +Copyright 2022 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package resources + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +var _ webhook.Defaulter = &TestResourceEmptyStatus{} + +// +kubebuilder:object:root=true +// +genclient + +type TestResourceEmptyStatus struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec TestResourceSpec `json:"spec"` + Status TestResourceEmptyStatusStatus `json:"status"` +} + +func (r *TestResourceEmptyStatus) Default() { + if r.Spec.Fields == nil { + r.Spec.Fields = map[string]string{} + } + r.Spec.Fields["Defaulter"] = "ran" +} + +// +kubebuilder:object:generate=true +type TestResourceEmptyStatusStatus struct { +} + +// +kubebuilder:object:root=true + +type TestResourceEmptyStatusList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata"` + + Items []TestResourceNoStatus `json:"items"` +} + +func init() { + SchemeBuilder.Register(&TestResourceEmptyStatus{}, &TestResourceEmptyStatusList{}) +} diff --git a/internal/resources/resource_with_nilable_status.go b/internal/resources/resource_with_nilable_status.go new file mode 100644 index 0000000..3daa584 --- /dev/null +++ b/internal/resources/resource_with_nilable_status.go @@ -0,0 +1,62 @@ +/* +Copyright 2022 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package resources + +import ( + "github.com/vmware-labs/reconciler-runtime/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +var ( + _ webhook.Defaulter = &TestResourceNilableStatus{} + _ client.Object = &TestResourceNilableStatus{} + _ validation.FieldValidator = &TestResourceNilableStatus{} +) + +// +kubebuilder:object:root=true +// +genclient + +type TestResourceNilableStatus struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec TestResourceSpec `json:"spec"` + Status *TestResourceStatus `json:"status"` +} + +func (r *TestResourceNilableStatus) Default() { + if r.Spec.Fields == nil { + r.Spec.Fields = map[string]string{} + } + r.Spec.Fields["Defaulter"] = "ran" +} + +func (r *TestResourceNilableStatus) Validate() validation.FieldErrors { + errs := validation.FieldErrors{} + + if r.Spec.Fields != nil { + if _, ok := r.Spec.Fields["invalid"]; ok { + errs = errs.Also(validation.ErrInvalidValue(r.Spec.Fields["invalid"], "spec.fields.invalid")) + } + } + + return errs +} + +// +kubebuilder:object:root=true + +type TestResourceNilableStatusList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata"` + + Items []TestResourceNilableStatus `json:"items"` +} + +func init() { + SchemeBuilder.Register(&TestResourceNilableStatus{}, &TestResourceNilableStatusList{}) +} diff --git a/internal/resources/zz_generated.deepcopy.go b/internal/resources/zz_generated.deepcopy.go index 9536deb..0628fc4 100644 --- a/internal/resources/zz_generated.deepcopy.go +++ b/internal/resources/zz_generated.deepcopy.go @@ -41,6 +41,80 @@ func (in *TestResource) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TestResourceEmptyStatus) DeepCopyInto(out *TestResourceEmptyStatus) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TestResourceEmptyStatus. +func (in *TestResourceEmptyStatus) DeepCopy() *TestResourceEmptyStatus { + if in == nil { + return nil + } + out := new(TestResourceEmptyStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *TestResourceEmptyStatus) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TestResourceEmptyStatusList) DeepCopyInto(out *TestResourceEmptyStatusList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]TestResourceNoStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TestResourceEmptyStatusList. +func (in *TestResourceEmptyStatusList) DeepCopy() *TestResourceEmptyStatusList { + if in == nil { + return nil + } + out := new(TestResourceEmptyStatusList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *TestResourceEmptyStatusList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TestResourceEmptyStatusStatus) DeepCopyInto(out *TestResourceEmptyStatusStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TestResourceEmptyStatusStatus. +func (in *TestResourceEmptyStatusStatus) DeepCopy() *TestResourceEmptyStatusStatus { + if in == nil { + return nil + } + out := new(TestResourceEmptyStatusStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TestResourceList) DeepCopyInto(out *TestResourceList) { *out = *in @@ -73,6 +147,69 @@ func (in *TestResourceList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TestResourceNilableStatus) DeepCopyInto(out *TestResourceNilableStatus) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(TestResourceStatus) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TestResourceNilableStatus. +func (in *TestResourceNilableStatus) DeepCopy() *TestResourceNilableStatus { + if in == nil { + return nil + } + out := new(TestResourceNilableStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *TestResourceNilableStatus) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TestResourceNilableStatusList) DeepCopyInto(out *TestResourceNilableStatusList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]TestResourceNilableStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TestResourceNilableStatusList. +func (in *TestResourceNilableStatusList) DeepCopy() *TestResourceNilableStatusList { + if in == nil { + return nil + } + out := new(TestResourceNilableStatusList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *TestResourceNilableStatusList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TestResourceNoStatus) DeepCopyInto(out *TestResourceNoStatus) { *out = *in diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index c769171..f7e32e2 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -172,22 +172,18 @@ func (r *ParentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr defaulter.Default() } - if r.hasStatus(originalParent) { - if initializeConditions := reflect.ValueOf(parent).Elem().FieldByName("Status").Addr().MethodByName("InitializeConditions"); initializeConditions.Kind() == reflect.Func { - // parent.Status.InitializeConditions() - initializeConditions.Call([]reflect.Value{}) - } - } + r.initializeConditions(parent) result, err := r.reconcile(ctx, parent) if r.hasStatus(originalParent) { // restore last transition time for unchanged conditions - r.syncLastTransitionTime(r.statusConditions(parent), r.statusConditions(originalParent)) + r.syncLastTransitionTime(r.conditions(parent), r.conditions(originalParent)) // check if status has changed before updating - if !equality.Semantic.DeepEqual(r.status(parent), r.status(originalParent)) && parent.GetDeletionTimestamp() == nil { + parentStatus, originalParentStatus := r.status(parent), r.status(originalParent) + if !equality.Semantic.DeepEqual(parentStatus, originalParentStatus) && parent.GetDeletionTimestamp() == nil { // update status - log.Info("updating status", "diff", cmp.Diff(r.status(originalParent), r.status(parent))) + log.Info("updating status", "diff", cmp.Diff(originalParentStatus, parentStatus)) if updateErr := c.Status().Update(ctx, parent); updateErr != nil { log.Error(updateErr, "unable to update status") c.Recorder.Eventf(parent, corev1.EventTypeWarning, "StatusUpdateFailed", @@ -216,27 +212,31 @@ func (r *ParentReconciler) reconcile(ctx context.Context, parent client.Object) r.copyGeneration(parent) return result, nil } -func (r *ParentReconciler) hasStatus(obj client.Object) bool { - return reflect.ValueOf(obj).Elem().FieldByName("Status").IsValid() -} -func (r *ParentReconciler) copyGeneration(obj client.Object) { - if r.hasStatus(obj) { - // obj.Status.ObservedGeneration = obj.Generation - objVal := reflect.ValueOf(obj).Elem() - generation := objVal.FieldByName("Generation").Int() - objVal.FieldByName("Status").FieldByName("ObservedGeneration").SetInt(generation) +func (r *ParentReconciler) initializeConditions(obj client.Object) { + status := r.status(obj) + if status == nil { + return } + initializeConditions := reflect.ValueOf(status).MethodByName("InitializeConditions") + if !initializeConditions.IsValid() { + return + } + if t := initializeConditions.Type(); t.Kind() != reflect.Func || t.NumIn() != 0 || t.NumOut() != 0 { + return + } + initializeConditions.Call([]reflect.Value{}) } -func (r *ParentReconciler) status(obj client.Object) interface{} { - return reflect.ValueOf(obj).Elem().FieldByName("Status").Addr().Interface() -} - -func (r *ParentReconciler) statusConditions(obj client.Object) []metav1.Condition { - statusValue := reflect.ValueOf(r.status(obj)).Elem() +func (r *ParentReconciler) conditions(obj client.Object) []metav1.Condition { + // return obj.Status.Conditions + status := r.status(obj) + if status == nil { + return nil + } + statusValue := reflect.ValueOf(status).Elem() conditionsValue := statusValue.FieldByName("Conditions") - if conditionsValue.IsZero() { + if !conditionsValue.IsValid() || conditionsValue.IsZero() { return nil } conditions, ok := conditionsValue.Interface().([]metav1.Condition) @@ -246,6 +246,43 @@ func (r *ParentReconciler) statusConditions(obj client.Object) []metav1.Conditio return conditions } +func (r *ParentReconciler) copyGeneration(obj client.Object) { + // obj.Status.ObservedGeneration = obj.Generation + status := r.status(obj) + if status == nil { + return + } + statusValue := reflect.ValueOf(status).Elem() + if !statusValue.IsValid() { + return + } + observedGenerationValue := statusValue.FieldByName("ObservedGeneration") + if !observedGenerationValue.CanInt() || !observedGenerationValue.CanSet() { + return + } + generation := obj.GetGeneration() + observedGenerationValue.SetInt(generation) +} + +func (r *ParentReconciler) hasStatus(obj client.Object) bool { + status := r.status(obj) + return status != nil +} + +func (r *ParentReconciler) status(obj client.Object) interface{} { + if obj == nil { + return nil + } + statusValue := reflect.ValueOf(obj).Elem().FieldByName("Status") + if statusValue.Kind() == reflect.Pointer { + statusValue = statusValue.Elem() + } + if !statusValue.IsValid() || !statusValue.CanAddr() { + return nil + } + return statusValue.Addr().Interface() +} + // syncLastTransitionTime restores a condition's LastTransitionTime value for // each proposed condition that is otherwise equivlent to the original value. // This method is useful to prevent updating the status for a resource that is diff --git a/reconcilers/reconcilers_test.go b/reconcilers/reconcilers_test.go index 35fa5b8..64c8a63 100644 --- a/reconcilers/reconcilers_test.go +++ b/reconcilers/reconcilers_test.go @@ -95,7 +95,7 @@ func TestConfig_TrackAndGet(t *testing.T) { }) } -func TestParentReconcilerWithNoStatus(t *testing.T) { +func TestParentReconciler_NoStatus(t *testing.T) { testNamespace := "test-namespace" testName := "test-resource-no-status" testKey := types.NamespacedName{Namespace: testNamespace, Name: testName} @@ -136,6 +136,188 @@ func TestParentReconcilerWithNoStatus(t *testing.T) { }) } +func TestParentReconciler_EmptyStatus(t *testing.T) { + testNamespace := "test-namespace" + testName := "test-resource-empty-status" + testKey := types.NamespacedName{Namespace: testNamespace, Name: testName} + + scheme := runtime.NewScheme() + _ = resources.AddToScheme(scheme) + + resource := dies.TestResourceEmptyStatusBlank. + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.Namespace(testNamespace) + d.Name(testName) + d.CreationTimestamp(metav1.NewTime(time.UnixMilli(1000))) + d.AddAnnotation("blah", "blah") + }) + + rts := rtesting.ReconcilerTestSuite{{ + Name: "resource exists", + Key: testKey, + GivenObjects: []client.Object{ + resource, + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResourceEmptyStatus) error { + return nil + }, + } + }, + }, + }} + rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler { + return &reconcilers.ParentReconciler{ + Type: &resources.TestResourceEmptyStatus{}, + Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c), + Config: c, + } + }) +} + +func TestParentReconciler_NilableStatus(t *testing.T) { + testNamespace := "test-namespace" + testName := "test-resource" + testKey := types.NamespacedName{Namespace: testNamespace, Name: testName} + + scheme := runtime.NewScheme() + _ = resources.AddToScheme(scheme) + + resource := dies.TestResourceNilableStatusBlank. + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.Namespace(testNamespace) + d.Name(testName) + d.CreationTimestamp(metav1.NewTime(time.UnixMilli(1000))) + }). + StatusDie(func(d *dies.TestResourceStatusDie) { + d.ConditionsDie( + diemetav1.ConditionBlank.Type(apis.ConditionReady).Status(metav1.ConditionUnknown).Reason("Initializing"), + ) + }) + + rts := rtesting.ReconcilerTestSuite{{ + Name: "nil status", + Key: testKey, + GivenObjects: []client.Object{ + resource.Status(nil), + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResourceNilableStatus) error { + if parent.Status != nil { + t.Errorf("status expected to be nil") + } + return nil + }, + } + }, + }, + }, { + Name: "status conditions are initialized", + Key: testKey, + GivenObjects: []client.Object{ + resource.StatusDie(func(d *dies.TestResourceStatusDie) { + d.ConditionsDie() + }), + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResourceNilableStatus) error { + expected := []metav1.Condition{ + {Type: apis.ConditionReady, Status: metav1.ConditionUnknown, Reason: "Initializing"}, + } + if diff := cmp.Diff(expected, parent.Status.Conditions, rtesting.IgnoreLastTransitionTime); diff != "" { + t.Errorf("Unexpected condition (-expected, +actual): %s", diff) + } + return nil + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated", + `Updated status`), + }, + ExpectStatusUpdates: []client.Object{ + resource, + }, + }, { + Name: "reconciler mutated status", + Key: testKey, + GivenObjects: []client.Object{ + resource, + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResourceNilableStatus) error { + if parent.Status.Fields == nil { + parent.Status.Fields = map[string]string{} + } + parent.Status.Fields["Reconciler"] = "ran" + return nil + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated", + `Updated status`), + }, + ExpectStatusUpdates: []client.Object{ + resource.StatusDie(func(d *dies.TestResourceStatusDie) { + d.AddField("Reconciler", "ran") + }), + }, + }, { + Name: "status update failed", + Key: testKey, + GivenObjects: []client.Object{ + resource, + }, + WithReactors: []rtesting.ReactionFunc{ + rtesting.InduceFailure("update", "TestResourceNilableStatus", rtesting.InduceFailureOpts{ + SubResource: "status", + }), + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResourceNilableStatus) error { + if parent.Status.Fields == nil { + parent.Status.Fields = map[string]string{} + } + parent.Status.Fields["Reconciler"] = "ran" + return nil + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(resource, scheme, corev1.EventTypeWarning, "StatusUpdateFailed", + `Failed to update status: inducing failure for update TestResourceNilableStatus`), + }, + ExpectStatusUpdates: []client.Object{ + resource.StatusDie(func(d *dies.TestResourceStatusDie) { + d.AddField("Reconciler", "ran") + }), + }, + ShouldErr: true, + }} + + rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler { + return &reconcilers.ParentReconciler{ + Type: &resources.TestResourceNilableStatus{}, + Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c), + Config: c, + } + }) +} + func TestParentReconciler(t *testing.T) { testNamespace := "test-namespace" testName := "test-resource" From 17988621dcdc1bb202a69df177cce89147469a1c Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Wed, 11 May 2022 22:32:21 -0400 Subject: [PATCH 2/4] Preserve go 1.17 support Signed-off-by: Scott Andrews --- reconcilers/reconcilers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index f7e32e2..fe31956 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -257,7 +257,7 @@ func (r *ParentReconciler) copyGeneration(obj client.Object) { return } observedGenerationValue := statusValue.FieldByName("ObservedGeneration") - if !observedGenerationValue.CanInt() || !observedGenerationValue.CanSet() { + if observedGenerationValue.Kind() != reflect.Int64 || !observedGenerationValue.CanSet() { return } generation := obj.GetGeneration() @@ -274,7 +274,7 @@ func (r *ParentReconciler) status(obj client.Object) interface{} { return nil } statusValue := reflect.ValueOf(obj).Elem().FieldByName("Status") - if statusValue.Kind() == reflect.Pointer { + if statusValue.Kind() == reflect.Ptr { statusValue = statusValue.Elem() } if !statusValue.IsValid() || !statusValue.CanAddr() { From a259f8e31298b78518e751e21a33fbac9546eb97 Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Thu, 12 May 2022 23:28:31 -0400 Subject: [PATCH 3/4] Log warning when a parent resource doesn't follow best practices We log when a ParentReconciler is setup when: - there is no status - the status is a pointer - the status doesn't have a ObservedGeneration int64 field - the status doesn't have a Conditions []metav1.Condition field - the status doesn't have an InitializeConditions() method None of these will block the system at runtime, if a field/method is missing, the behavior is skipped. Signed-off-by: Scott Andrews --- reconcilers/reconcilers.go | 50 +++++++++++ reconcilers/reconcilers_validate_test.go | 110 +++++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index fe31956..3e38dd8 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -131,6 +131,10 @@ func (r *ParentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage ctx = StashParentType(ctx, r.Type) ctx = StashCastParentType(ctx, r.Type) + if err := r.validate(ctx); err != nil { + return err + } + bldr := ctrl.NewControllerManagedBy(mgr).For(r.Type) if err := r.Reconciler.SetupWithManager(ctx, mgr, bldr); err != nil { return err @@ -138,6 +142,52 @@ func (r *ParentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage return bldr.Complete(r) } +func (r *ParentReconciler) validate(ctx context.Context) error { + // validate Type value + if r.Type == nil { + return fmt.Errorf("ParentReconciler %q must define Type", r.Name) + } + + // validate Reconciler value + if r.Reconciler == nil { + return fmt.Errorf("ParentReconciler %q must define Reconciler", r.Name) + } + + // warn users of common pitfalls. These are not blockers. + + log := logr.FromContextOrDiscard(ctx) + + parentType := reflect.TypeOf(r.Type).Elem() + statusField, hasStatus := parentType.FieldByName("Status") + if !hasStatus { + log.Info("parent resource missing status field, operations related to status will be skipped") + return nil + } + + statusType := statusField.Type + if statusType.Kind() == reflect.Ptr { + log.Info("parent status is nilable, status is typically a struct") + statusType = statusType.Elem() + } + + observedGenerationField, hasObservedGeneration := statusType.FieldByName("ObservedGeneration") + if !hasObservedGeneration || observedGenerationField.Type.Kind() != reflect.Int64 { + log.Info("parent status missing ObservedGeneration field of type int64, generation will not be managed") + } + + initializeConditionsMethod, hasInitializeConditions := reflect.PtrTo(statusType).MethodByName("InitializeConditions") + if !hasInitializeConditions || initializeConditionsMethod.Type.NumIn() != 1 || initializeConditionsMethod.Type.NumOut() != 0 { + log.Info("parent status missing InitializeConditions() method, conditions will not be auto-initialized") + } + + conditionsField, hasConditions := statusType.FieldByName("Conditions") + if !hasConditions || !conditionsField.Type.AssignableTo(reflect.TypeOf([]metav1.Condition{})) { + log.Info("parent status is missing field Conditions of type []metav1.Condition, condition timestamps will not be managed") + } + + return nil +} + func (r *ParentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { ctx = WithStash(ctx) diff --git a/reconcilers/reconcilers_validate_test.go b/reconcilers/reconcilers_validate_test.go index 011f36a..cc3c9d5 100644 --- a/reconcilers/reconcilers_validate_test.go +++ b/reconcilers/reconcilers_validate_test.go @@ -9,12 +9,99 @@ import ( "context" "testing" + "github.com/go-logr/logr" + "github.com/google/go-cmp/cmp" + "github.com/vmware-labs/reconciler-runtime/internal/resources" "github.com/vmware-labs/reconciler-runtime/tracker" corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) +func TestParentReconciler_validate(t *testing.T) { + tests := []struct { + name string + reconciler *ParentReconciler + shouldErr string + expectedLogs []string + }{ + { + name: "empty", + reconciler: &ParentReconciler{}, + shouldErr: `ParentReconciler "" must define Type`, + }, + { + name: "valid", + reconciler: &ParentReconciler{ + Type: &resources.TestResource{}, + Reconciler: Sequence{}, + }, + }, + { + name: "missing type", + reconciler: &ParentReconciler{ + Name: "missing type", + Reconciler: Sequence{}, + }, + shouldErr: `ParentReconciler "missing type" must define Type`, + }, + { + name: "missing reconciler", + reconciler: &ParentReconciler{ + Name: "missing reconciler", + Type: &resources.TestResource{}, + }, + shouldErr: `ParentReconciler "missing reconciler" must define Reconciler`, + }, + { + name: "type has no status", + reconciler: &ParentReconciler{ + Type: &resources.TestResourceNoStatus{}, + Reconciler: Sequence{}, + }, + expectedLogs: []string{ + "parent resource missing status field, operations related to status will be skipped", + }, + }, + { + name: "type has empty status", + reconciler: &ParentReconciler{ + Type: &resources.TestResourceEmptyStatus{}, + Reconciler: Sequence{}, + }, + expectedLogs: []string{ + "parent status missing ObservedGeneration field of type int64, generation will not be managed", + "parent status missing InitializeConditions() method, conditions will not be auto-initialized", + "parent status is missing field Conditions of type []metav1.Condition, condition timestamps will not be managed", + }, + }, + { + name: "type has nilable status", + reconciler: &ParentReconciler{ + Type: &resources.TestResourceNilableStatus{}, + Reconciler: Sequence{}, + }, + expectedLogs: []string{ + "parent status is nilable, status is typically a struct", + }, + }, + } + + for _, c := range tests { + t.Run(c.name, func(t *testing.T) { + sink := &bufferedSink{} + ctx := logr.NewContext(context.TODO(), logr.New(sink)) + err := c.reconciler.validate(ctx) + if (err != nil) != (c.shouldErr != "") || (c.shouldErr != "" && c.shouldErr != err.Error()) { + t.Errorf("validate() error = %q, shouldErr %q", err, c.shouldErr) + } + if diff := cmp.Diff(c.expectedLogs, sink.Lines); diff != "" { + t.Errorf("%s: unexpected logs (-expected, +actual): %s", c.name, diff) + } + }) + } +} + func TestSyncReconciler_validate(t *testing.T) { tests := []struct { name string @@ -1000,3 +1087,26 @@ func TestWithFinalizer_validate(t *testing.T) { }) } } + +var _ logr.LogSink = &bufferedSink{} + +type bufferedSink struct { + Lines []string +} + +func (s *bufferedSink) Init(info logr.RuntimeInfo) {} +func (s *bufferedSink) Enabled(level int) bool { + return true +} +func (s *bufferedSink) Info(level int, msg string, keysAndValues ...interface{}) { + s.Lines = append(s.Lines, msg) +} +func (s *bufferedSink) Error(err error, msg string, keysAndValues ...interface{}) { + s.Lines = append(s.Lines, msg) +} +func (s *bufferedSink) WithValues(keysAndValues ...interface{}) logr.LogSink { + return s +} +func (s *bufferedSink) WithName(name string) logr.LogSink { + return s +} From d3f756020c710c6a334b032990a2d23ec0378c02 Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Fri, 13 May 2022 09:54:04 -0400 Subject: [PATCH 4/4] polish readme Signed-off-by: Scott Andrews --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 38b29a4..182dece 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,8 @@ The parent is responsible for: - fetching the resource being reconciled - creating a stash to pass state between sub reconcilers - passing the resource to each sub reconciler in turn +- initialize conditions on the status by calling status.InitializeConditions() if defined +- normalizing the .status.conditions[].lastTransitionTime for status conditions that are metav1.Condition (the previous timestamp is preserved if the condition is otherwise unchanged) - reflects the observed generation on the status - updates the resource status if it was modified - logging the reconcilers activities