From 0c125709ad950f494b9a8f311f9365373a052ba4 Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Wed, 11 May 2022 21:52:45 -0400 Subject: [PATCH] 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 | 81 ++-- reconcilers/reconcilers_test.go | 97 +++++ 8 files changed, 808 insertions(+), 25 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..0258d17 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,27 @@ 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 := reflect.ValueOf(obj).Elem().FieldByName("Status") + if !status.CanAddr() { + return } + initializeConditions := status.Addr().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 { +func (r *ParentReconciler) conditions(obj client.Object) []metav1.Condition { + // return obj.Status.Conditions statusValue := reflect.ValueOf(r.status(obj)).Elem() conditionsValue := statusValue.FieldByName("Conditions") - if conditionsValue.IsZero() { + if !conditionsValue.IsValid() || conditionsValue.IsZero() { return nil } conditions, ok := conditionsValue.Interface().([]metav1.Condition) @@ -246,6 +242,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..69b108e 100644 --- a/reconcilers/reconcilers_test.go +++ b/reconcilers/reconcilers_test.go @@ -136,6 +136,103 @@ func TestParentReconcilerWithNoStatus(t *testing.T) { }) } +func TestParentReconcilerWithEmptyStatus(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 TestParentReconcilerWithNilableStatus(t *testing.T) { + testNamespace := "test-namespace" + testName := "test-resource-nilable-status" + 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))) + d.AddAnnotation("blah", "blah") + }) + + rts := rtesting.ReconcilerTestSuite{{ + Name: "nil 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 { + return nil + }, + } + }, + }, + }, { + Name: "pointer status", + Key: testKey, + GivenObjects: []client.Object{ + resource.Status(&resources.TestResourceStatus{}), + }, + 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 { + return nil + }, + } + }, + }, + }} + 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"