Skip to content

Commit c84fe9a

Browse files
committed
simplify event msg
Signed-off-by: Kexin2000 <3299133282@qq.com>
1 parent d12893d commit c84fe9a

File tree

7 files changed

+114
-46
lines changed

7 files changed

+114
-46
lines changed

pkg/dependenciesdistributor/dependencies_distributor.go

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"context"
2121
"encoding/json"
2222
"fmt"
23-
"sort"
24-
"strings"
2523

2624
corev1 "k8s.io/api/core/v1"
2725
"k8s.io/apimachinery/pkg/api/equality"
@@ -593,28 +591,19 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
593591
oldRequiredByLen := len(existBinding.Spec.RequiredBy)
594592

595593
mergedRequiredBy := mergeBindingSnapshot(existBinding.Spec.RequiredBy, attachedBinding.Spec.RequiredBy)
596-
parentsForEvent := formatBindingParents(mergedRequiredBy)
597594
multiParentAfter := len(mergedRequiredBy) > 1
598595

599596
finalConflictResolution := effectiveConflictResolution(attachedBinding.Spec.ConflictResolution)
600597
finalPreserve := ptr.Deref(attachedBinding.Spec.PreserveResourcesOnDeletion, false)
601598

602-
var conflictDetails []string
603599
conflictDetected := false
604600
shouldEmitAggregationEvent := false
605601

606602
// If the spec.Placement is nil, this means that existBinding is generated by the dependency mechanism.
607603
// If the spec.Placement is not nil, then it must be generated by PropagationPolicy.
608604
if existBinding.Spec.Placement == nil {
609-
if oldRequiredByLen > 0 && oldConflictResolution != finalConflictResolution {
605+
if oldRequiredByLen > 0 && (oldConflictResolution != finalConflictResolution || oldPreserve != finalPreserve) {
610606
conflictDetected = true
611-
conflictDetails = append(conflictDetails, fmt.Sprintf("conflictResolution existing=%s incoming=%s",
612-
oldConflictResolution, finalConflictResolution))
613-
}
614-
if oldRequiredByLen > 0 && oldPreserve != finalPreserve {
615-
conflictDetected = true
616-
conflictDetails = append(conflictDetails, fmt.Sprintf("preserveResourcesOnDeletion existing=%t incoming=%t",
617-
oldPreserve, finalPreserve))
618607
}
619608
existBinding.Spec.ConflictResolution = attachedBinding.Spec.ConflictResolution
620609
shouldEmitAggregationEvent = multiParentAfter
@@ -626,8 +615,9 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
626615
existBinding.Spec.PreserveResourcesOnDeletion = attachedBinding.Spec.PreserveResourcesOnDeletion
627616

628617
if conflictDetected && d.EventRecorder != nil {
629-
message := fmt.Sprintf("[dep-agg] conflict on %s/%s from parents %s: %s",
630-
existBinding.Namespace, existBinding.Name, strings.Join(parentsForEvent, ","), strings.Join(conflictDetails, "; "))
618+
// Keep the event message concise and consistent with other events' style.
619+
// Detailed differences and parent lists are omitted intentionally.
620+
message := "Dependency policy conflict detected during aggregation."
631621
d.EventRecorder.Eventf(existBinding, corev1.EventTypeWarning, events.EventReasonDependencyPolicyConflict, message)
632622
}
633623

@@ -637,8 +627,8 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
637627
}
638628

639629
if shouldEmitAggregationEvent && d.EventRecorder != nil {
640-
message := fmt.Sprintf("[dep-agg] aggregated policy for %s/%s from parents %s: conflictResolution=%s preserveResourcesOnDeletion=%t",
641-
existBinding.Namespace, existBinding.Name, strings.Join(parentsForEvent, ","), finalConflictResolution, finalPreserve)
630+
// Emit a short success message aligned with other event messages' tone.
631+
message := "Aggregated dependency policy successfully."
642632
d.EventRecorder.Eventf(existBinding, corev1.EventTypeNormal, events.EventReasonDependencyPolicyAggregated, message)
643633
}
644634
return nil
@@ -734,23 +724,8 @@ func effectiveConflictResolution(value policyv1alpha1.ConflictResolution) policy
734724
return value
735725
}
736726

737-
func formatBindingParents(snapshots []workv1alpha2.BindingSnapshot) []string {
738-
if len(snapshots) == 0 {
739-
return nil
740-
}
741-
742-
parents := make([]string, 0, len(snapshots))
743-
for _, snapshot := range snapshots {
744-
if snapshot.Namespace != "" {
745-
parents = append(parents, fmt.Sprintf("%s/%s", snapshot.Namespace, snapshot.Name))
746-
continue
747-
}
748-
parents = append(parents, snapshot.Name)
749-
}
750-
751-
sort.Strings(parents)
752-
return parents
753-
}
727+
// formatBindingParents was used to generate detailed event messages.
728+
// It's removed to keep event messages concise and to avoid unused code warnings.
754729

755730
func generateBindingDependedLabels(bindingID, bindingNamespace, bindingName string) map[string]string {
756731
labelKey := generateBindingDependedLabelKey(bindingNamespace, bindingName)

pkg/dependenciesdistributor/dependencies_distributor_test.go

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3565,18 +3565,12 @@ func TestCreateOrUpdateAttachedBinding_RecordsConflictEvent(t *testing.T) {
35653565

35663566
conflictEvent := findEventByReason(t, eventList, events.EventReasonDependencyPolicyConflict)
35673567
assertEventMessageContains(t, conflictEvent,
3568-
"[dep-agg] conflict",
3569-
"conflictResolution existing=Overwrite incoming=Abort",
3570-
"preserveResourcesOnDeletion existing=true incoming=false",
3568+
"Dependency policy conflict",
35713569
)
35723570

35733571
aggregatedEvent := findEventByReason(t, eventList, events.EventReasonDependencyPolicyAggregated)
35743572
assertEventMessageContains(t, aggregatedEvent,
3575-
"[dep-agg] aggregated policy",
3576-
"conflictResolution=Abort",
3577-
"preserveResourcesOnDeletion=false",
3578-
"default/workload-a",
3579-
"default/workload-b",
3573+
"Aggregated dependency policy successfully",
35803574
)
35813575
}
35823576

@@ -3615,11 +3609,7 @@ func TestCreateOrUpdateAttachedBinding_RecordsAggregationEvent(t *testing.T) {
36153609

36163610
aggregatedEvent := findEventByReason(t, eventList, events.EventReasonDependencyPolicyAggregated)
36173611
assertEventMessageContains(t, aggregatedEvent,
3618-
"[dep-agg] aggregated policy",
3619-
"conflictResolution=Overwrite",
3620-
"preserveResourcesOnDeletion=true",
3621-
"default/workload-a",
3622-
"default/workload-b",
3612+
"Aggregated dependency policy successfully",
36233613
)
36243614

36253615
for _, evt := range eventList {

samples/nginx/configmap.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
apiVersion: v1
2+
kind: ConfigMap
3+
metadata:
4+
name: nginx-shared-config
5+
data:
6+
index.html: |
7+
<html>
8+
<head><title>Karmada Multi-Parent Dependency Demo</title></head>
9+
<body>
10+
<h1>Hello from shared ConfigMap</h1>
11+
<p>This page is served by nginx and mounted from a shared ConfigMap.</p>
12+
</body>
13+
</html>

samples/nginx/deployment-a.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: nginx-main
5+
labels:
6+
app: nginx-main
7+
spec:
8+
replicas: 1
9+
selector:
10+
matchLabels:
11+
app: nginx-main
12+
template:
13+
metadata:
14+
labels:
15+
app: nginx-main
16+
spec:
17+
containers:
18+
- name: nginx
19+
image: nginx:1.25-alpine
20+
volumeMounts:
21+
- name: web
22+
mountPath: /usr/share/nginx/html
23+
volumes:
24+
- name: web
25+
configMap:
26+
name: nginx-shared-config

samples/nginx/deployment-b.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: nginx-side
5+
labels:
6+
app: nginx-side
7+
spec:
8+
replicas: 1
9+
selector:
10+
matchLabels:
11+
app: nginx-side
12+
template:
13+
metadata:
14+
labels:
15+
app: nginx-side
16+
spec:
17+
containers:
18+
- name: nginx
19+
image: nginx:1.25-alpine
20+
volumeMounts:
21+
- name: web
22+
mountPath: /usr/share/nginx/html
23+
volumes:
24+
- name: web
25+
configMap:
26+
name: nginx-shared-config
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
apiVersion: policy.karmada.io/v1alpha1
2+
kind: PropagationPolicy
3+
metadata:
4+
name: nginx-shared-prop
5+
spec:
6+
propagateDeps: true
7+
resourceSelectors:
8+
- apiVersion: apps/v1
9+
kind: Deployment
10+
name: nginx-main
11+
placement:
12+
clusterAffinity:
13+
clusterNames:
14+
- member1
15+
- member2
16+
replicaScheduling:
17+
replicaSchedulingType: Duplicated
18+
conflictResolution: Abort
19+
preserveResourcesOnDeletion: false
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
apiVersion: policy.karmada.io/v1alpha1
2+
kind: PropagationPolicy
3+
metadata:
4+
name: nginx-side-prop
5+
spec:
6+
propagateDeps: true
7+
resourceSelectors:
8+
- apiVersion: apps/v1
9+
kind: Deployment
10+
name: nginx-side
11+
placement:
12+
clusterAffinity:
13+
clusterNames:
14+
- member1
15+
- member2
16+
replicaScheduling:
17+
replicaSchedulingType: Duplicated
18+
conflictResolution: Overwrite
19+
preserveResourcesOnDeletion: true

0 commit comments

Comments
 (0)