Set status.observedGeneration for federated resources

CHANGELOG.md

@@ -1,4 +1,8 @@
 # Unreleased
+-  [#1107](https://github.com/kubernetes-sigs/kubefed/pull/1107)
+  `status.observedGeneration` is now recorded by the sync controller
+   for federated resources to provide an indication of whether the
+   current state of a resource has been processed.
 -  [#1121](https://github.com/kubernetes-sigs/kubefed/pull/1121) Update
    `kubefedctl federate` shorthand option for `--enable-type` to `-t` instead
    of `-e` to avoid confusing error message when only one dash is accidentally

charts/kubefed/templates/crds.yaml

@@ -122,6 +122,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -251,6 +254,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -382,6 +388,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -511,6 +520,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -638,6 +650,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -767,6 +782,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -898,6 +916,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -1025,6 +1046,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -1154,6 +1178,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec
@@ -1283,6 +1310,9 @@ spec:
                 - status
                 type: object
               type: array
+            observedGeneration:
+              format: int64
+              type: integer
           type: object
       required:
       - spec

pkg/controller/sync/controller.go

@@ -289,13 +289,13 @@ func (s *KubeFedSyncController) syncToClusters(fedResource FederatedResource) ut
 	clusters, err := s.informer.GetClusters()
 	if err != nil {
 		fedResource.RecordError(string(status.ClusterRetrievalFailed), errors.Wrap(err, "Failed to retrieve list of clusters"))
-		return s.setPropagationStatus(fedResource, status.ClusterRetrievalFailed, nil)
+		return s.setFederatedStatus(fedResource, status.ClusterRetrievalFailed, nil)
 	}
 
 	selectedClusterNames, err := fedResource.ComputePlacement(clusters)
 	if err != nil {
 		fedResource.RecordError(string(status.ComputePlacementFailed), errors.Wrap(err, "Failed to compute placement"))
-		return s.setPropagationStatus(fedResource, status.ComputePlacementFailed, nil)
+		return s.setFederatedStatus(fedResource, status.ComputePlacementFailed, nil)
 	}
 
 	kind := fedResource.TargetKind()
@@ -379,10 +379,10 @@ func (s *KubeFedSyncController) syncToClusters(fedResource FederatedResource) ut
 	}
 
 	collectedStatus := dispatcher.CollectedStatus()
-	return s.setPropagationStatus(fedResource, status.AggregateSuccess, &collectedStatus)
+	return s.setFederatedStatus(fedResource, status.AggregateSuccess, &collectedStatus)
 }
 
-func (s *KubeFedSyncController) setPropagationStatus(fedResource FederatedResource,
+func (s *KubeFedSyncController) setFederatedStatus(fedResource FederatedResource,
 	reason status.AggregateReason, collectedStatus *status.CollectedPropagationStatus) util.ReconciliationStatus {
 
 	if collectedStatus == nil {
@@ -407,10 +407,10 @@ func (s *KubeFedSyncController) setPropagationStatus(fedResource FederatedResour
 	// If the underlying resource has changed, attempt to retrieve and
 	// update it repeatedly.
 	err := wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) {
-		if updateRequired, err := status.SetPropagationStatus(obj, reason, *collectedStatus); err != nil {
+		if updateRequired, err := status.SetFederatedStatus(obj, reason, *collectedStatus); err != nil {
 			return false, errors.Wrapf(err, "failed to set the status")
 		} else if !updateRequired {
-			klog.V(4).Infof("No update necessary for %s %q propagation status", kind, name)
+			klog.V(4).Infof("No status update necessary for %s %q", kind, name)
 			return true, nil
 		}
 

pkg/controller/sync/status/status.go

@@ -91,16 +91,17 @@ type GenericCondition struct {
 	Reason AggregateReason `json:"reason,omitempty"`
 }
 
-type GenericPropagationStatus struct {
-	Conditions []*GenericCondition    `json:"conditions,omitempty"`
-	Clusters   []GenericClusterStatus `json:"clusters,omitempty"`
+type GenericFederatedStatus struct {
+	ObservedGeneration int64                  `json:"observedGeneration,omitempty"`
+	Conditions         []*GenericCondition    `json:"conditions,omitempty"`
+	Clusters           []GenericClusterStatus `json:"clusters,omitempty"`
 }
 
-type GenericFederatedStatus struct {
+type GenericFederatedResource struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Status *GenericPropagationStatus `json:"status,omitempty"`
+	Status *GenericFederatedStatus `json:"status,omitempty"`
 }
 
 type PropagationStatusMap map[string]PropagationStatus
@@ -110,43 +111,49 @@ type CollectedPropagationStatus struct {
 	ResourcesUpdated bool
 }
 
-// SetPropagationStatus sets the conditions and clusters fields of the
-// federated resource's object map from the provided reason and collected
-// propagation status. Returns a boolean indication of whether status
-// should be written to the API.
-func SetPropagationStatus(fedObject *unstructured.Unstructured, reason AggregateReason, collectedStatus CollectedPropagationStatus) (bool, error) {
-	status := &GenericFederatedStatus{}
-	err := util.UnstructuredToInterface(fedObject, status)
+// SetFederatedStatus sets the conditions and clusters fields of the
+// federated resource's object map. Returns a boolean indication of
+// whether status should be written to the API.
+func SetFederatedStatus(fedObject *unstructured.Unstructured, reason AggregateReason, collectedStatus CollectedPropagationStatus) (bool, error) {
+	resource := &GenericFederatedResource{}
+	err := util.UnstructuredToInterface(fedObject, resource)
 	if err != nil {
-		return false, errors.Wrapf(err, "Failed to unmarshall to generic status")
+		return false, errors.Wrapf(err, "Failed to unmarshall to generic resource")
 	}
-	if status.Status == nil {
-		status.Status = &GenericPropagationStatus{}
+	if resource.Status == nil {
+		resource.Status = &GenericFederatedStatus{}
 	}
 
-	changed := status.Status.update(reason, collectedStatus)
+	changed := resource.Status.update(fedObject.GetGeneration(), reason, collectedStatus)
 	if !changed {
 		return false, nil
 	}
 
-	statusJSON, err := json.Marshal(status)
+	resourceJSON, err := json.Marshal(resource)
 	if err != nil {
 		return false, errors.Wrapf(err, "Failed to marshall generic status to json")
 	}
-	statusObj := &unstructured.Unstructured{}
-	err = statusObj.UnmarshalJSON(statusJSON)
+	resourceObj := &unstructured.Unstructured{}
+	err = resourceObj.UnmarshalJSON(resourceJSON)
 	if err != nil {
-		return false, errors.Wrapf(err, "Failed to marshall generic status json to unstructured")
+		return false, errors.Wrapf(err, "Failed to marshall generic resource json to unstructured")
 	}
-	fedObject.Object[util.StatusField] = statusObj.Object[util.StatusField]
+	fedObject.Object[util.StatusField] = resourceObj.Object[util.StatusField]
 
 	return true, nil
 }
 
-// update ensures that the status reflects the given reason and collected
-// status. Returns a boolean indication of whether the status has been
-// changed.
-func (s *GenericPropagationStatus) update(reason AggregateReason, collectedStatus CollectedPropagationStatus) bool {
+// update ensures that the status reflects the given generation, reason
+// and collected status. Returns a boolean indication of whether the
+// status has been changed.
+func (s *GenericFederatedStatus) update(generation int64, reason AggregateReason,
+	collectedStatus CollectedPropagationStatus) bool {
+
+	generationUpdated := s.ObservedGeneration != generation
+	if generationUpdated {
+		s.ObservedGeneration = generation
+	}
+
 	// Identify whether one or more clusters could not be reconciled
 	// successfully.
 	if reason == AggregateSuccess {
@@ -165,13 +172,16 @@ func (s *GenericPropagationStatus) update(reason AggregateReason, collectedStatu
 	// occur even if status.clusters was unchanged).
 	changesPropagated := clustersChanged || len(collectedStatus.StatusMap) > 0 && collectedStatus.ResourcesUpdated
 
-	return s.setPropagationCondition(reason, changesPropagated)
+	propStatusUpdated := s.setPropagationCondition(reason, changesPropagated)
+
+	statusUpdated := generationUpdated || propStatusUpdated
+	return statusUpdated
 }
 
 // setClusters sets the status.clusters slice from a propagation status
 // map. Returns a boolean indication of whether the status.clusters was
 // modified.
-func (s *GenericPropagationStatus) setClusters(statusMap PropagationStatusMap) bool {
+func (s *GenericFederatedStatus) setClusters(statusMap PropagationStatusMap) bool {
 	if !s.clustersDiffers(statusMap) {
 		return false
 	}
@@ -187,7 +197,7 @@ func (s *GenericPropagationStatus) setClusters(statusMap PropagationStatusMap) b
 
 // clustersDiffers checks whether `status.clusters` differs from the
 // given status map.
-func (s *GenericPropagationStatus) clustersDiffers(statusMap PropagationStatusMap) bool {
+func (s *GenericFederatedStatus) clustersDiffers(statusMap PropagationStatusMap) bool {
 	if len(s.Clusters) != len(statusMap) {
 		return true
 	}
@@ -202,7 +212,7 @@ func (s *GenericPropagationStatus) clustersDiffers(statusMap PropagationStatusMa
 // setPropagationCondition ensures that the Propagation condition is
 // updated to reflect the given reason.  The type of the condition is
 // derived from the reason (empty -> True, not empty -> False).
-func (s *GenericPropagationStatus) setPropagationCondition(reason AggregateReason, changesPropagated bool) bool {
+func (s *GenericFederatedStatus) setPropagationCondition(reason AggregateReason, changesPropagated bool) bool {
 	// Determine the appropriate status from the reason.
 	var newStatus apiv1.ConditionStatus
 	if reason == AggregateSuccess {

pkg/controller/sync/status/status_test.go

@@ -24,6 +24,7 @@ import (
 
 func TestGenericPropagationStatusUpdateChanged(t *testing.T) {
 	testCases := map[string]struct {
+		generation       int64
 		reason           AggregateReason
 		statusMap        PropagationStatusMap
 		resourcesUpdated bool
@@ -48,10 +49,14 @@ func TestGenericPropagationStatusUpdateChanged(t *testing.T) {
 			reason:          NamespaceNotFederated,
 			expectedChanged: true,
 		},
+		"Changed generation indicates changed": {
+			generation:      1,
+			expectedChanged: true,
+		},
 	}
 	for testName, tc := range testCases {
 		t.Run(testName, func(t *testing.T) {
-			propStatus := &GenericPropagationStatus{
+			propStatus := &GenericFederatedStatus{
 				Clusters: []GenericClusterStatus{
 					{
 						Name: "cluster1",
@@ -68,7 +73,7 @@ func TestGenericPropagationStatusUpdateChanged(t *testing.T) {
 				StatusMap:        tc.statusMap,
 				ResourcesUpdated: tc.resourcesUpdated,
 			}
-			changed := propStatus.update(tc.reason, collectedStatus)
+			changed := propStatus.update(tc.generation, tc.reason, collectedStatus)
 			if tc.expectedChanged != changed {
 				t.Fatalf("Expected changed to be %v, got %v", tc.expectedChanged, changed)
 			}

pkg/kubefedctl/enable/validation.go

@@ -240,6 +240,10 @@ func ValidationSchema(specProps v1beta1.JSONSchemaProps) *v1beta1.CustomResource
 								},
 							},
 						},
+						"observedGeneration": {
+							Format: "int64",
+							Type:   "integer",
+						},
 					},
 				},
 			},