Merge pull request #633 from pedjak/add-reconcile-observation

Add ability to expose resource reconciliation progress

apis/common/v1/condition.go

@@ -54,6 +54,8 @@ const (
 	ReasonReconcilePaused  ConditionReason = "ReconcilePaused"
 )
 
+// See https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
+
 // A Condition that may apply to a resource.
 type Condition struct {
 	// Type of this condition. At most one of each condition type may apply to
@@ -74,10 +76,16 @@ type Condition struct {
 	// one status to another, if any.
 	// +optional
 	Message string `json:"message,omitempty"`
+
+	// ObservedGeneration represents the .metadata.generation that the condition was set based upon.
+	// For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
+	// with respect to the current state of the instance.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 // Equal returns true if the condition is identical to the supplied condition,
-// ignoring the LastTransitionTime.
+// ignoring the LastTransitionTime and ObservedGeneration.
 func (c Condition) Equal(other Condition) bool {
 	return c.Type == other.Type &&
 		c.Status == other.Status &&
@@ -92,6 +100,13 @@ func (c Condition) WithMessage(msg string) Condition {
 	return c
 }
 
+// WithObservedGeneration returns a condition by adding the provided observed generation
+// to existing condition.
+func (c Condition) WithObservedGeneration(gen int64) Condition {
+	c.ObservedGeneration = gen
+	return c
+}
+
 // NOTE(negz): Conditions are implemented as a slice rather than a map to comply
 // with Kubernetes API conventions. Ideally we'd comply by using a map that
 // marshalled to a JSON array, but doing so confuses the CRD schema generator.
@@ -131,6 +146,7 @@ func (s *ConditionedStatus) GetCondition(ct ConditionType) Condition {
 // SetConditions sets the supplied conditions, replacing any existing conditions
 // of the same type. This is a no-op if all supplied conditions are identical,
 // ignoring the last transition time, to those already set.
+// Observed generation is updated if higher than the existing one.
 func (s *ConditionedStatus) SetConditions(c ...Condition) {
 	for _, new := range c {
 		exists := false
@@ -141,6 +157,9 @@ func (s *ConditionedStatus) SetConditions(c ...Condition) {
 
 			if existing.Equal(new) {
 				exists = true
+				if existing.ObservedGeneration < new.ObservedGeneration {
+					existing.ObservedGeneration = new.ObservedGeneration
+				}
 				continue
 			}
 

apis/common/v1/condition_test.go

@@ -228,3 +228,32 @@ func TestConditionWithMessage(t *testing.T) {
 		})
 	}
 }
+
+func TestConditionWithObservedGeneration(t *testing.T) {
+	cases := map[string]struct {
+		c                  Condition
+		observedGeneration int64
+		want               Condition
+	}{
+		"Added": {
+			c:                  Condition{Type: TypeReady, Reason: ReasonUnavailable},
+			observedGeneration: 10,
+			want:               Condition{Type: TypeReady, Reason: ReasonUnavailable, ObservedGeneration: 10},
+		},
+		"Changed": {
+			c:                  Condition{Type: TypeReady, Reason: ReasonUnavailable, ObservedGeneration: 3},
+			observedGeneration: 10,
+			want:               Condition{Type: TypeReady, Reason: ReasonUnavailable, ObservedGeneration: 10},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := tc.c.WithObservedGeneration(tc.observedGeneration)
+
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("a.Equal(b): -want, +got:\n%s", diff)
+			}
+		})
+	}
+}

apis/common/v1/observation.go

@@ -0,0 +1,37 @@
+/*
+Copyright 2024 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+// ObservedStatus contains the recent reconciliation stats.
+type ObservedStatus struct {
+	// ObservedGeneration is the latest metadata.generation
+	// which resulted in either a ready state, or stalled due to error
+	// it can not recover from without human intervention.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+}
+
+// SetObservedGeneration sets the generation of the main resource
+// during the last reconciliation.
+func (s *ObservedStatus) SetObservedGeneration(generation int64) {
+	s.ObservedGeneration = generation
+}
+
+// GetObservedGeneration returns the last observed generation of the main resource.
+func (s *ObservedStatus) GetObservedGeneration() int64 {
+	return s.ObservedGeneration
+}

apis/common/v1/resource.go

@@ -226,6 +226,7 @@ type ResourceSpec struct {
 // ResourceStatus represents the observed state of a managed resource.
 type ResourceStatus struct {
 	ConditionedStatus `json:",inline"`
+	ObservedStatus    `json:",inline"`
 }
 
 // A CredentialsSource is a source from which provider credentials may be

pkg/resource/fake/mocks.go

@@ -364,7 +364,7 @@ type Composite struct {
 	ConnectionSecretWriterTo
 	ConnectionDetailsPublisherTo
 
-	xpv1.ConditionedStatus
+	xpv1.ResourceStatus
 	ConnectionDetailsLastPublishedTimer
 }
 
@@ -389,7 +389,7 @@ type Composed struct {
 	metav1.ObjectMeta
 	ConnectionSecretWriterTo
 	ConnectionDetailsPublisherTo
-	xpv1.ConditionedStatus
+	xpv1.ResourceStatus
 }
 
 // GetObjectKind returns schema.ObjectKind.
@@ -421,7 +421,7 @@ type CompositeClaim struct {
 	LocalConnectionSecretWriterTo
 	ConnectionDetailsPublisherTo
 
-	xpv1.ConditionedStatus
+	xpv1.ResourceStatus
 	ConnectionDetailsLastPublishedTimer
 }
 

pkg/resource/interfaces.go

@@ -176,6 +176,12 @@ type ConnectionDetailsPublishedTimer interface {
 	GetConnectionDetailsLastPublishedTime() *metav1.Time
 }
 
+// ReconciliationObserver can track data observed by resource reconciler.
+type ReconciliationObserver interface {
+	SetObservedGeneration(generation int64)
+	GetObservedGeneration() int64
+}
+
 // An Object is a Kubernetes object.
 type Object interface {
 	metav1.Object
@@ -245,6 +251,7 @@ type Composite interface { //nolint:interfacebloat // This interface has to be b
 
 	Conditioned
 	ConnectionDetailsPublishedTimer
+	ReconciliationObserver
 }
 
 // Composed resources can be a composed into a Composite resource.
@@ -254,6 +261,7 @@ type Composed interface {
 	Conditioned
 	ConnectionSecretWriterTo
 	ConnectionDetailsPublisherTo
+	ReconciliationObserver
 }
 
 // A CompositeClaim for a Composite resource.
@@ -272,4 +280,5 @@ type CompositeClaim interface { //nolint:interfacebloat // This interface has to
 
 	Conditioned
 	ConnectionDetailsPublishedTimer
+	ReconciliationObserver
 }

pkg/resource/interfaces_test.go

@@ -16,7 +16,12 @@ limitations under the License.
 
 package resource
 
-import "github.com/crossplane/crossplane-runtime/pkg/resource/fake"
+import (
+	"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
+	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
+	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed"
+	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite"
+)
 
 // We test that our fakes satisfy our interfaces here rather than in the fake
 // package to avoid a cyclic dependency.
@@ -29,4 +34,8 @@ var (
 	_ CompositeClaim = &fake.CompositeClaim{}
 	_ Composite      = &fake.Composite{}
 	_ Composed       = &fake.Composed{}
+
+	_ CompositeClaim = &claim.Unstructured{}
+	_ Composite      = &composite.Unstructured{}
+	_ Composed       = &composed.Unstructured{}
 )

pkg/resource/unstructured/claim/claim.go

@@ -253,3 +253,18 @@ func (c *Unstructured) GetConnectionDetailsLastPublishedTime() *metav1.Time {
 func (c *Unstructured) SetConnectionDetailsLastPublishedTime(t *metav1.Time) {
 	_ = fieldpath.Pave(c.Object).SetValue("status.connectionDetails.lastPublishedTime", t)
 }
+
+// SetObservedGeneration of this composite resource claim.
+func (c *Unstructured) SetObservedGeneration(generation int64) {
+	status := &xpv1.ObservedStatus{}
+	_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
+	status.SetObservedGeneration(generation)
+	_ = fieldpath.Pave(c.Object).SetValue("status.observedGeneration", status.ObservedGeneration)
+}
+
+// GetObservedGeneration of this composite resource claim.
+func (c *Unstructured) GetObservedGeneration() int64 {
+	status := &xpv1.ObservedStatus{}
+	_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
+	return status.GetObservedGeneration()
+}

pkg/resource/unstructured/claim/claim_test.go

@@ -368,3 +368,28 @@ func TestConnectionDetailsLastPublishedTime(t *testing.T) {
 		})
 	}
 }
+
+func TestObservedGeneration(t *testing.T) {
+	cases := map[string]struct {
+		u    *Unstructured
+		want int64
+	}{
+		"Set": {
+			u: New(func(u *Unstructured) {
+				u.SetObservedGeneration(123)
+			}),
+			want: 123,
+		},
+		"NotFound": {
+			u: New(),
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := tc.u.GetObservedGeneration()
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("\nu.GetObservedGeneration(): -want, +got:\n%s", diff)
+			}
+		})
+	}
+}

pkg/resource/unstructured/composed/composed.go

@@ -168,3 +168,18 @@ type UnstructuredList struct {
 func (cr *UnstructuredList) GetUnstructuredList() *unstructured.UnstructuredList {
 	return &cr.UnstructuredList
 }
+
+// SetObservedGeneration of this composite resource claim.
+func (cr *Unstructured) SetObservedGeneration(generation int64) {
+	status := &xpv1.ObservedStatus{}
+	_ = fieldpath.Pave(cr.Object).GetValueInto("status", status)
+	status.SetObservedGeneration(generation)
+	_ = fieldpath.Pave(cr.Object).SetValue("status.observedGeneration", status.ObservedGeneration)
+}
+
+// GetObservedGeneration of this composite resource claim.
+func (cr *Unstructured) GetObservedGeneration() int64 {
+	status := &xpv1.ObservedStatus{}
+	_ = fieldpath.Pave(cr.Object).GetValueInto("status", status)
+	return status.GetObservedGeneration()
+}

pkg/resource/unstructured/composed/composed_test.go

@@ -146,3 +146,28 @@ func TestWriteConnectionSecretToReference(t *testing.T) {
 		})
 	}
 }
+
+func TestObservedGeneration(t *testing.T) {
+	cases := map[string]struct {
+		u    *Unstructured
+		want int64
+	}{
+		"Set": {
+			u: New(func(u *Unstructured) {
+				u.SetObservedGeneration(123)
+			}),
+			want: 123,
+		},
+		"NotFound": {
+			u: New(),
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := tc.u.GetObservedGeneration()
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("\nu.GetObservedGeneration(): -want, +got:\n%s", diff)
+			}
+		})
+	}
+}

pkg/resource/unstructured/composite/composite.go

@@ -258,3 +258,18 @@ func (c *Unstructured) SetEnvironmentConfigReferences(refs []corev1.ObjectRefere
 	}
 	_ = fieldpath.Pave(c.Object).SetValue("spec.environmentConfigRefs", filtered)
 }
+
+// SetObservedGeneration of this composite resource claim.
+func (c *Unstructured) SetObservedGeneration(generation int64) {
+	status := &xpv1.ObservedStatus{}
+	_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
+	status.SetObservedGeneration(generation)
+	_ = fieldpath.Pave(c.Object).SetValue("status.observedGeneration", status.ObservedGeneration)
+}
+
+// GetObservedGeneration of this composite resource claim.
+func (c *Unstructured) GetObservedGeneration() int64 {
+	status := &xpv1.ObservedStatus{}
+	_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
+	return status.GetObservedGeneration()
+}

pkg/resource/unstructured/composite/composite_test.go

@@ -356,3 +356,28 @@ func TestConnectionDetailsLastPublishedTime(t *testing.T) {
 		})
 	}
 }
+
+func TestObservedGeneration(t *testing.T) {
+	cases := map[string]struct {
+		u    *Unstructured
+		want int64
+	}{
+		"Set": {
+			u: New(func(u *Unstructured) {
+				u.SetObservedGeneration(123)
+			}),
+			want: 123,
+		},
+		"NotFound": {
+			u: New(),
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := tc.u.GetObservedGeneration()
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("\nu.GetObservedGeneration(): -want, +got:\n%s", diff)
+			}
+		})
+	}
+}