diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go index 612c40e75..1442cc0a5 100644 --- a/pkg/util/helper/workstatus.go +++ b/pkg/util/helper/workstatus.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -124,7 +125,7 @@ func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1a func generateFullyAppliedCondition(spec workv1alpha2.ResourceBindingSpec, aggregatedStatuses []workv1alpha2.AggregatedStatusItem) metav1.Condition { clusterNames := GetBindingClusterNames(spec.Clusters, spec.RequiredBy) - if len(clusterNames) == len(aggregatedStatuses) && areWorksFullyApplied(aggregatedStatuses) { + if worksFullyApplied(aggregatedStatuses, clusterNames) { return util.NewCondition(workv1alpha2.FullyApplied, FullyAppliedSuccessReason, FullyAppliedSuccessMessage, metav1.ConditionTrue) } return util.NewCondition(workv1alpha2.FullyApplied, FullyAppliedFailedReason, FullyAppliedFailedMessage, metav1.ConditionFalse) @@ -232,13 +233,33 @@ func equalIdentifier(targetIdentifier *workv1alpha1.ResourceIdentifier, ordinal return false, nil } -// areWorksFullyApplied checks if all works are applied for a Binding -func areWorksFullyApplied(aggregatedStatuses []workv1alpha2.AggregatedStatusItem) bool { +// worksFullyApplied checks if all works are applied according the scheduled result and collected status. +func worksFullyApplied(aggregatedStatuses []workv1alpha2.AggregatedStatusItem, targetClusters []string) bool { + // short path: not scheduled + if len(targetClusters) == 0 { + return false + } + + // short path: lack of status + if len(targetClusters) != len(aggregatedStatuses) { + return false + } + + targetClusterSet := sets.String{} + for i := range targetClusters { + targetClusterSet.Insert(targetClusters[i]) + } + for _, aggregatedSatusItem := range aggregatedStatuses { if !aggregatedSatusItem.Applied { return false } + + if !targetClusterSet.Has(aggregatedSatusItem.ClusterName) { + return false + } } + return true } diff --git a/pkg/util/helper/workstatus_test.go b/pkg/util/helper/workstatus_test.go new file mode 100644 index 000000000..f991e3e4e --- /dev/null +++ b/pkg/util/helper/workstatus_test.go @@ -0,0 +1,108 @@ +package helper + +import ( + "testing" + + workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" +) + +func TestWorksFullyApplied(t *testing.T) { + type args struct { + aggregatedStatuses []workv1alpha2.AggregatedStatusItem + targetClusters []string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "no cluster", + args: args{ + aggregatedStatuses: []workv1alpha2.AggregatedStatusItem{ + { + ClusterName: "member1", + Applied: true, + }, + }, + targetClusters: nil, + }, + want: false, + }, + { + name: "no aggregatedStatuses", + args: args{ + aggregatedStatuses: nil, + targetClusters: []string{"member1"}, + }, + want: false, + }, + { + name: "cluster size is not equal to aggregatedStatuses", + args: args{ + aggregatedStatuses: []workv1alpha2.AggregatedStatusItem{ + { + ClusterName: "member1", + Applied: true, + }, + }, + targetClusters: []string{"member1", "member2"}, + }, + want: false, + }, + { + name: "aggregatedStatuses is equal to clusterNames and all applied", + args: args{ + aggregatedStatuses: []workv1alpha2.AggregatedStatusItem{ + { + ClusterName: "member1", + Applied: true, + }, + { + ClusterName: "member2", + Applied: true, + }, + }, + targetClusters: []string{"member1", "member2"}, + }, + want: true, + }, + { + name: "aggregatedStatuses is equal to clusterNames but not all applied", + args: args{ + aggregatedStatuses: []workv1alpha2.AggregatedStatusItem{ + { + ClusterName: "member1", + Applied: true, + }, + { + ClusterName: "member2", + Applied: false, + }, + }, + targetClusters: []string{"member1", "member2"}, + }, + want: false, + }, + { + name: "target clusters not match expected status", + args: args{ + aggregatedStatuses: []workv1alpha2.AggregatedStatusItem{ + { + ClusterName: "member1", + Applied: true, + }, + }, + targetClusters: []string{"member2"}, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := worksFullyApplied(tt.args.aggregatedStatuses, tt.args.targetClusters); got != tt.want { + t.Errorf("worksFullyApplied() = %v, want %v", got, tt.want) + } + }) + } +}