Merge pull request #5165 from whitewindmills/ds-generation

Align federated DaemonSet's observedGeneration semantics with its native

pkg/resourceinterpreter/default/native/aggregatestatus.go

@@ -26,6 +26,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	policyv1 "k8s.io/api/policy/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -279,39 +280,44 @@ func aggregateDaemonSetStatus(object *unstructured.Unstructured, aggregatedStatu
 
 	oldStatus := &daemonSet.Status
 	newStatus := &appsv1.DaemonSetStatus{}
+	observedLatestResourceTemplateGenerationCount := 0
 	for _, item := range aggregatedStatusItems {
 		if item.Status == nil {
 			continue
 		}
-		temp := &appsv1.DaemonSetStatus{}
-		if err = json.Unmarshal(item.Status.Raw, temp); err != nil {
+		member := &WrappedDaemonSetStatus{}
+		if err = json.Unmarshal(item.Status.Raw, member); err != nil {
 			return nil, err
 		}
 		klog.V(3).Infof("Grab daemonSet(%s/%s) status from cluster(%s), currentNumberScheduled: %d, desiredNumberScheduled: %d, numberAvailable: %d, numberMisscheduled: %d, numberReady: %d, updatedNumberScheduled: %d, numberUnavailable: %d",
-			daemonSet.Namespace, daemonSet.Name, item.ClusterName, temp.CurrentNumberScheduled, temp.DesiredNumberScheduled, temp.NumberAvailable, temp.NumberMisscheduled, temp.NumberReady, temp.UpdatedNumberScheduled, temp.NumberUnavailable)
+			daemonSet.Namespace, daemonSet.Name, item.ClusterName, member.CurrentNumberScheduled, member.DesiredNumberScheduled, member.NumberAvailable, member.NumberMisscheduled, member.NumberReady, member.UpdatedNumberScheduled, member.NumberUnavailable)
 
-		// always set 'observedGeneration' with current generation(.metadata.generation)
-		// which is the generation Karmada 'observed'.
-		// The 'observedGeneration' is mainly used by GitOps tools(like 'Argo CD') to assess the health status.
-		// For more details, please refer to https://argo-cd.readthedocs.io/en/stable/operator-manual/health/.
+		// `memberStatus.ObservedGeneration >= memberStatus.Generation` means the member's status corresponds the latest spec revision of the member DaemonSet.
+		// `memberStatus.ResourceTemplateGeneration >= daemonSet.Generation` means the member DaemonSet has been aligned with the latest spec revision of federated DaemonSet.
+		// If both conditions are met, we consider the member's status corresponds the latest spec revision of federated DaemonSet.
+		if member.ObservedGeneration >= member.Generation &&
+			member.ResourceTemplateGeneration >= daemonSet.Generation {
+			observedLatestResourceTemplateGenerationCount++
+		}
+
+		newStatus.CurrentNumberScheduled += member.CurrentNumberScheduled
+		newStatus.DesiredNumberScheduled += member.DesiredNumberScheduled
+		newStatus.NumberAvailable += member.NumberAvailable
+		newStatus.NumberMisscheduled += member.NumberMisscheduled
+		newStatus.NumberReady += member.NumberReady
+		newStatus.UpdatedNumberScheduled += member.UpdatedNumberScheduled
+		newStatus.NumberUnavailable += member.NumberUnavailable
+	}
+
+	// The 'observedGeneration' is mainly used by GitOps tools(like 'Argo CD') to assess the health status.
+	// For more details, please refer to https://argo-cd.readthedocs.io/en/stable/operator-manual/health/.
+	if observedLatestResourceTemplateGenerationCount == len(aggregatedStatusItems) {
 		newStatus.ObservedGeneration = daemonSet.Generation
-		newStatus.CurrentNumberScheduled += temp.CurrentNumberScheduled
-		newStatus.DesiredNumberScheduled += temp.DesiredNumberScheduled
-		newStatus.NumberAvailable += temp.NumberAvailable
-		newStatus.NumberMisscheduled += temp.NumberMisscheduled
-		newStatus.NumberReady += temp.NumberReady
-		newStatus.UpdatedNumberScheduled += temp.UpdatedNumberScheduled
-		newStatus.NumberUnavailable += temp.NumberUnavailable
+	} else {
+		newStatus.ObservedGeneration = oldStatus.ObservedGeneration
 	}
 
-	if oldStatus.ObservedGeneration == newStatus.ObservedGeneration &&
-		oldStatus.CurrentNumberScheduled == newStatus.CurrentNumberScheduled &&
-		oldStatus.DesiredNumberScheduled == newStatus.DesiredNumberScheduled &&
-		oldStatus.NumberAvailable == newStatus.NumberAvailable &&
-		oldStatus.NumberMisscheduled == newStatus.NumberMisscheduled &&
-		oldStatus.NumberReady == newStatus.NumberReady &&
-		oldStatus.UpdatedNumberScheduled == newStatus.UpdatedNumberScheduled &&
-		oldStatus.NumberUnavailable == newStatus.NumberUnavailable {
+	if equality.Semantic.DeepEqual(oldStatus, newStatus) {
 		klog.V(3).Infof("Ignore update daemonSet(%s/%s) status as up to date", daemonSet.Namespace, daemonSet.Name)
 		return object, nil
 	}

pkg/resourceinterpreter/default/native/reflectstatus.go

@@ -78,8 +78,10 @@ func reflectDeploymentStatus(object *unstructured.Unstructured) (*runtime.RawExt
 	}
 
 	grabStatus := &WrappedDeploymentStatus{
-		Generation:                 object.GetGeneration(),
-		ResourceTemplateGeneration: resourceTemplateGenerationInt,
+		FederatedGeneration: FederatedGeneration{
+			Generation:                 object.GetGeneration(),
+			ResourceTemplateGeneration: resourceTemplateGenerationInt,
+		},
 		DeploymentStatus: appsv1.DeploymentStatus{
 			Replicas:            deploymentStatus.Replicas,
 			UpdatedReplicas:     deploymentStatus.UpdatedReplicas,
@@ -194,15 +196,31 @@ func reflectDaemonSetStatus(object *unstructured.Unstructured) (*runtime.RawExte
 		return nil, fmt.Errorf("failed to convert DaemonSetStatus from map[string]interface{}: %v", err)
 	}
 
-	grabStatus := appsv1.DaemonSetStatus{
-		CurrentNumberScheduled: daemonSetStatus.CurrentNumberScheduled,
-		DesiredNumberScheduled: daemonSetStatus.DesiredNumberScheduled,
-		NumberAvailable:        daemonSetStatus.NumberAvailable,
-		NumberMisscheduled:     daemonSetStatus.NumberMisscheduled,
-		NumberReady:            daemonSetStatus.NumberReady,
-		UpdatedNumberScheduled: daemonSetStatus.UpdatedNumberScheduled,
-		NumberUnavailable:      daemonSetStatus.NumberUnavailable,
+	resourceTemplateGenerationInt := int64(0)
+	resourceTemplateGenerationStr := util.GetAnnotationValue(object.GetAnnotations(), v1alpha2.ResourceTemplateGenerationAnnotationKey)
+	err = runtime.Convert_string_To_int64(&resourceTemplateGenerationStr, &resourceTemplateGenerationInt, nil)
+	if err != nil {
+		klog.Errorf("Failed to parse DaemonSet(%s/%s) generation from annotation(%s:%s): %v", object.GetNamespace(), object.GetName(), v1alpha2.ResourceTemplateGenerationAnnotationKey, resourceTemplateGenerationStr, err)
+		return nil, err
 	}
+
+	grabStatus := &WrappedDaemonSetStatus{
+		FederatedGeneration: FederatedGeneration{
+			Generation:                 object.GetGeneration(),
+			ResourceTemplateGeneration: resourceTemplateGenerationInt,
+		},
+		DaemonSetStatus: appsv1.DaemonSetStatus{
+			CurrentNumberScheduled: daemonSetStatus.CurrentNumberScheduled,
+			DesiredNumberScheduled: daemonSetStatus.DesiredNumberScheduled,
+			NumberAvailable:        daemonSetStatus.NumberAvailable,
+			NumberMisscheduled:     daemonSetStatus.NumberMisscheduled,
+			NumberReady:            daemonSetStatus.NumberReady,
+			UpdatedNumberScheduled: daemonSetStatus.UpdatedNumberScheduled,
+			NumberUnavailable:      daemonSetStatus.NumberUnavailable,
+			ObservedGeneration:     daemonSetStatus.ObservedGeneration,
+		},
+	}
+
 	return helper.BuildStatusRawExtension(grabStatus)
 }
 

pkg/resourceinterpreter/default/native/status_type.go

@@ -18,13 +18,24 @@ package native
 
 import appsv1 "k8s.io/api/apps/v1"
 
-// WrappedDeploymentStatus is a wrapper for appsv1.DeploymentStatus.
-type WrappedDeploymentStatus struct {
-	appsv1.DeploymentStatus `json:",inline"`
-
+// FederatedGeneration holds the generation of the same resource in the member cluster and the Karmada control plane.
+type FederatedGeneration struct {
 	// Generation holds the generation(.metadata.generation) of resource running on member cluster.
 	Generation int64 `json:"generation,omitempty"`
+
 	// ResourceTemplateGeneration holds the value of annotation resourcetemplate.karmada.io/generation grabbed
 	// from resource running on member cluster.
 	ResourceTemplateGeneration int64 `json:"resourceTemplateGeneration,omitempty"`
 }
+
+// WrappedDeploymentStatus is a wrapper for appsv1.DeploymentStatus.
+type WrappedDeploymentStatus struct {
+	FederatedGeneration     `json:",inline"`
+	appsv1.DeploymentStatus `json:",inline"`
+}
+
+// WrappedDaemonSetStatus is a wrapper for appsv1.DaemonSetStatus.
+type WrappedDaemonSetStatus struct {
+	FederatedGeneration    `json:",inline"`
+	appsv1.DaemonSetStatus `json:",inline"`
+}