diff --git a/README.md b/README.md index 7fcbc69..5babb57 100644 --- a/README.md +++ b/README.md @@ -51,9 +51,9 @@ The resource reconciler 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 +- initialize conditions on the status by calling status.InitializeConditions() if defined (not available for Unstructured resources) +- normalizing the .status.conditions[].lastTransitionTime for status conditions that are metav1.Condition (the previous timestamp is preserved if the condition is otherwise unchanged) (not available for Unstructured resources) +- reflects the observed generation on the status (not available for Unstructured resources) - updates the resource status if it was modified - logging the reconcilers activities - records events for mutations and errors diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index 48e4798..6f0aaf4 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -20,6 +20,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -249,25 +250,24 @@ func (r *ResourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c r.initializeConditions(resource) result, err := r.reconcile(ctx, resource) - if r.hasStatus(originalResource) { - // restore last transition time for unchanged conditions - r.syncLastTransitionTime(r.conditions(resource), r.conditions(originalResource)) - - // check if status has changed before updating - resourceStatus, originalResourceStatus := r.status(resource), r.status(originalResource) - if !equality.Semantic.DeepEqual(resourceStatus, originalResourceStatus) && resource.GetDeletionTimestamp() == nil { - // update status - log.Info("updating status", "diff", cmp.Diff(originalResourceStatus, resourceStatus)) - if updateErr := c.Status().Update(ctx, resource); updateErr != nil { - log.Error(updateErr, "unable to update status") - c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusUpdateFailed", - "Failed to update status: %v", updateErr) - return ctrl.Result{}, updateErr - } - c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusUpdated", - "Updated status") + // attempt to restore last transition time for unchanged conditions + r.syncLastTransitionTime(r.conditions(resource), r.conditions(originalResource)) + + // check if status has changed before updating + resourceStatus, originalResourceStatus := r.status(resource), r.status(originalResource) + if !equality.Semantic.DeepEqual(resourceStatus, originalResourceStatus) && resource.GetDeletionTimestamp() == nil { + // update status + log.Info("updating status", "diff", cmp.Diff(originalResourceStatus, resourceStatus)) + if updateErr := c.Status().Update(ctx, resource); updateErr != nil { + log.Error(updateErr, "unable to update status") + c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusUpdateFailed", + "Failed to update status: %v", updateErr) + return ctrl.Result{}, updateErr } + c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusUpdated", + "Updated status") } + // return original reconcile result return result, err } @@ -308,7 +308,11 @@ func (r *ResourceReconciler) conditions(obj client.Object) []metav1.Condition { if status == nil { return nil } - statusValue := reflect.ValueOf(status).Elem() + statusValue := reflect.ValueOf(status) + if statusValue.Type().Kind() == reflect.Map { + return nil + } + statusValue = statusValue.Elem() conditionsValue := statusValue.FieldByName("Conditions") if !conditionsValue.IsValid() || conditionsValue.IsZero() { return nil @@ -326,7 +330,11 @@ func (r *ResourceReconciler) copyGeneration(obj client.Object) { if status == nil { return } - statusValue := reflect.ValueOf(status).Elem() + statusValue := reflect.ValueOf(status) + if statusValue.Type().Kind() == reflect.Map { + return + } + statusValue = statusValue.Elem() if !statusValue.IsValid() { return } @@ -347,6 +355,9 @@ func (r *ResourceReconciler) status(obj client.Object) interface{} { if obj == nil { return nil } + if u, ok := obj.(*unstructured.Unstructured); ok { + return u.UnstructuredContent()["status"] + } statusValue := reflect.ValueOf(obj).Elem().FieldByName("Status") if statusValue.Kind() == reflect.Ptr { statusValue = statusValue.Elem() diff --git a/reconcilers/reconcilers_test.go b/reconcilers/reconcilers_test.go index cfc3de4..c9e8a70 100644 --- a/reconcilers/reconcilers_test.go +++ b/reconcilers/reconcilers_test.go @@ -353,6 +353,88 @@ func TestResourceReconciler_NilableStatus(t *testing.T) { }) } +func TestResourceReconciler_Unstructured(t *testing.T) { + testNamespace := "test-namespace" + testName := "test-resource" + testRequest := controllerruntime.Request{ + NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: testName}, + } + + scheme := runtime.NewScheme() + _ = resources.AddToScheme(scheme) + + resource := dies.TestResourceBlank. + APIVersion(resources.GroupVersion.Identifier()). + Kind("TestResource"). + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.Namespace(testNamespace) + d.Name(testName) + d.Generation(1) + }). + StatusDie(func(d *dies.TestResourceStatusDie) { + d.ConditionsDie( + diemetav1.ConditionBlank.Type(apis.ConditionReady).Status(metav1.ConditionUnknown).Reason("Initializing"), + ) + }) + + rts := rtesting.ReconcilerTests{ + "in sync status": { + Request: testRequest, + 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, resource *unstructured.Unstructured) error { + return nil + }, + } + }, + }, + }, + "status update": { + Request: testRequest, + 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, resource *unstructured.Unstructured) error { + resource.Object["status"].(map[string]interface{})["fields"] = map[string]interface{}{ + "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") + }).DieReleaseUnstructured().(client.Object), + }, + }, + } + + rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler { + return &reconcilers.ResourceReconciler{ + Type: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": resources.GroupVersion.Identifier(), + "kind": "TestResource", + }, + }, + Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c), + Config: c, + } + }) +} + func TestResourceReconciler(t *testing.T) { testNamespace := "test-namespace" testName := "test-resource" diff --git a/testing/config.go b/testing/config.go index b87b5ee..c2f3d6b 100644 --- a/testing/config.go +++ b/testing/config.go @@ -360,7 +360,7 @@ func (c *ExpectConfig) compareActions(t *testing.T, actionName string, expectedA var ( IgnoreLastTransitionTime = cmp.FilterPath(func(p cmp.Path) bool { return strings.HasSuffix(p.String(), "LastTransitionTime") || - strings.HasSuffix(p.String(), `(map[string]interface {})["lastTransitionTime"]`) + strings.HasSuffix(p.GoString(), `(map[string]interface {})["lastTransitionTime"]`) }, cmp.Ignore()) IgnoreTypeMeta = cmp.FilterPath(func(p cmp.Path) bool { return strings.HasSuffix(p.String(), "TypeMeta.APIVersion") ||