Skip to content

Commit 9824e3d

Browse files
authored
Merge pull request #6512 from XiShanYongYe-Chang/remove-instruction-label
Delete the propagation.karmada.io/instruction label
2 parents 0b5fe5e + 027741b commit 9824e3d

File tree

10 files changed

+73
-321
lines changed

10 files changed

+73
-321
lines changed

cmd/agent/app/agent.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,6 @@ func startExecutionController(ctx controllerscontext.Context) (bool, error) {
348348
EventRecorder: ctx.Mgr.GetEventRecorderFor(execution.ControllerName),
349349
RESTMapper: ctx.Mgr.GetRESTMapper(),
350350
ObjectWatcher: ctx.ObjectWatcher,
351-
PredicateFunc: helper.NewExecutionPredicateOnAgent(),
352351
InformerManager: genericmanager.GetInstance(),
353352
RateLimiterOptions: ctx.Opts.RateLimiterOptions,
354353
}
@@ -366,7 +365,6 @@ func startWorkStatusController(ctx controllerscontext.Context) (bool, error) {
366365
InformerManager: genericmanager.GetInstance(),
367366
Context: ctx.Context,
368367
ObjectWatcher: ctx.ObjectWatcher,
369-
PredicateFunc: helper.NewExecutionPredicateOnAgent(),
370368
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
371369
ClusterCacheSyncTimeout: ctx.Opts.ClusterCacheSyncTimeout,
372370
ConcurrentWorkStatusSyncs: ctx.Opts.ConcurrentWorkSyncs,

cmd/controller-manager/app/controllermanager.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ func startExecutionController(ctx controllerscontext.Context) (enabled bool, err
454454
EventRecorder: ctx.Mgr.GetEventRecorderFor(execution.ControllerName),
455455
RESTMapper: ctx.Mgr.GetRESTMapper(),
456456
ObjectWatcher: ctx.ObjectWatcher,
457-
PredicateFunc: helper.NewExecutionPredicate(ctx.Mgr),
457+
WorkPredicateFunc: helper.WorkWithinPushClusterPredicate(ctx.Mgr),
458458
InformerManager: genericmanager.GetInstance(),
459459
RateLimiterOptions: ctx.Opts.RateLimiterOptions,
460460
}
@@ -473,7 +473,7 @@ func startWorkStatusController(ctx controllerscontext.Context) (enabled bool, er
473473
InformerManager: genericmanager.GetInstance(),
474474
Context: ctx.Context,
475475
ObjectWatcher: ctx.ObjectWatcher,
476-
PredicateFunc: helper.NewExecutionPredicate(ctx.Mgr),
476+
WorkPredicateFunc: helper.WorkWithinPushClusterPredicate(ctx.Mgr),
477477
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSet,
478478
ClusterClientOption: ctx.ClusterClientOption,
479479
ClusterCacheSyncTimeout: opts.ClusterCacheSyncTimeout,

pkg/controllers/ctrlutil/work.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,7 @@ func CreateOrUpdateWork(ctx context.Context, c client.Client, workMeta metav1.Ob
7676

7777
// Do the same thing as the mutating webhook does, add the permanent ID to workload if not exist,
7878
// This is to avoid unnecessary updates to the Work object, especially when controller starts.
79-
//nolint:staticcheck // SA1019 ignore deprecated util.PropagationInstruction
80-
if runtimeObject.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
81-
util.SetLabelsAndAnnotationsForWorkload(resource, runtimeObject)
82-
}
79+
util.SetLabelsAndAnnotationsForWorkload(resource, runtimeObject)
8380
workloadJSON, err := json.Marshal(resource)
8481
if err != nil {
8582
klog.Errorf("Failed to marshal workload(%s/%s), error: %v", resource.GetNamespace(), resource.GetName(), err)

pkg/controllers/ctrlutil/work_test.go

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030

3131
workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
3232
workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
33-
"github.com/karmada-io/karmada/pkg/util"
3433
)
3534

3635
func TestCreateOrUpdateWork(t *testing.T) {
@@ -70,50 +69,6 @@ func TestCreateOrUpdateWork(t *testing.T) {
7069
assert.Equal(t, 1, len(work.Spec.Workload.Manifests))
7170
},
7271
},
73-
{
74-
name: "create work with PropagationInstruction",
75-
workMeta: metav1.ObjectMeta{
76-
Namespace: "default",
77-
Name: "test-work",
78-
Labels: map[string]string{
79-
//nolint:staticcheck // SA1019 ignore deprecated util.PropagationInstruction
80-
util.PropagationInstruction: "some-value",
81-
},
82-
Annotations: map[string]string{
83-
workv1alpha2.ResourceConflictResolutionAnnotation: "overwrite",
84-
},
85-
},
86-
resource: &unstructured.Unstructured{
87-
Object: map[string]interface{}{
88-
"apiVersion": "apps/v1",
89-
"kind": "Deployment",
90-
"metadata": map[string]interface{}{
91-
"name": "test-deployment",
92-
"uid": "test-uid",
93-
},
94-
},
95-
},
96-
verify: func(t *testing.T, c client.Client) {
97-
work := &workv1alpha1.Work{}
98-
err := c.Get(context.TODO(), client.ObjectKey{Namespace: "default", Name: "test-work"}, work)
99-
assert.NoError(t, err)
100-
101-
// Get the resource from manifests
102-
manifest := &unstructured.Unstructured{}
103-
err = manifest.UnmarshalJSON(work.Spec.Workload.Manifests[0].Raw)
104-
assert.NoError(t, err)
105-
106-
// Verify labels and annotations were set
107-
labels := manifest.GetLabels()
108-
assert.Equal(t, util.ManagedByKarmadaLabelValue, labels[util.ManagedByKarmadaLabel])
109-
110-
annotations := manifest.GetAnnotations()
111-
assert.Equal(t, "test-uid", annotations[workv1alpha2.ResourceTemplateUIDAnnotation])
112-
assert.Equal(t, "test-work", annotations[workv1alpha2.WorkNameAnnotation])
113-
assert.Equal(t, "default", annotations[workv1alpha2.WorkNamespaceAnnotation])
114-
assert.Equal(t, "overwrite", annotations[workv1alpha2.ResourceConflictResolutionAnnotation])
115-
},
116-
},
11772
{
11873
name: "update existing work",
11974
existingWork: &workv1alpha1.Work{

pkg/controllers/execution/execution_controller.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ type Controller struct {
7171
EventRecorder record.EventRecorder
7272
RESTMapper meta.RESTMapper
7373
ObjectWatcher objectwatcher.ObjectWatcher
74-
PredicateFunc predicate.Predicate
74+
WorkPredicateFunc predicate.Predicate
7575
InformerManager genericmanager.MultiClusterInformerManager
7676
RateLimiterOptions ratelimiterflag.Options
7777
}
@@ -133,14 +133,19 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
133133

134134
// SetupWithManager creates a controller and register to controller manager.
135135
func (c *Controller) SetupWithManager(mgr controllerruntime.Manager) error {
136-
return controllerruntime.NewControllerManagedBy(mgr).
137-
Named(ControllerName).
138-
For(&workv1alpha1.Work{}, builder.WithPredicates(c.PredicateFunc)).
136+
ctrlBuilder := controllerruntime.NewControllerManagedBy(mgr).Named(ControllerName).
139137
WithEventFilter(predicate.GenerationChangedPredicate{}).
140138
WithOptions(controller.Options{
141139
RateLimiter: ratelimiterflag.DefaultControllerRateLimiter[controllerruntime.Request](c.RateLimiterOptions),
142-
}).
143-
Complete(c)
140+
})
141+
142+
if c.WorkPredicateFunc != nil {
143+
ctrlBuilder.For(&workv1alpha1.Work{}, builder.WithPredicates(c.WorkPredicateFunc))
144+
} else {
145+
ctrlBuilder.For(&workv1alpha1.Work{})
146+
}
147+
148+
return ctrlBuilder.Complete(c)
144149
}
145150

146151
func (c *Controller) syncWork(ctx context.Context, clusterName string, work *workv1alpha1.Work) (controllerruntime.Result, error) {

pkg/controllers/status/work_status_controller.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ type WorkStatusController struct {
7070
// ConcurrentWorkStatusSyncs is the number of Work status that are allowed to sync concurrently.
7171
ConcurrentWorkStatusSyncs int
7272
ObjectWatcher objectwatcher.ObjectWatcher
73-
PredicateFunc predicate.Predicate
73+
WorkPredicateFunc predicate.Predicate
7474
ClusterDynamicClientSetFunc util.NewClusterDynamicClientSetFunc
7575
ClusterClientOption *util.ClientOption
7676
ClusterCacheSyncTimeout metav1.Duration
@@ -296,11 +296,6 @@ func (c *WorkStatusController) handleDeleteEvent(ctx context.Context, key keys.F
296296
return nil
297297
}
298298

299-
//nolint:staticcheck // SA1019 ignore deprecated util.PropagationInstruction
300-
if util.GetLabelValue(work.Labels, util.PropagationInstruction) == util.PropagationInstructionSuppressed {
301-
return nil
302-
}
303-
304299
reCreateErr := c.recreateResourceIfNeeded(ctx, work, key)
305300
if reCreateErr != nil {
306301
c.updateAppliedCondition(ctx, work, metav1.ConditionFalse, "ReCreateFailed", reCreateErr.Error())
@@ -548,12 +543,18 @@ func (c *WorkStatusController) getSingleClusterManager(cluster *clusterv1alpha1.
548543

549544
// SetupWithManager creates a controller and register to controller manager.
550545
func (c *WorkStatusController) SetupWithManager(mgr controllerruntime.Manager) error {
551-
return controllerruntime.NewControllerManagedBy(mgr).
552-
Named(WorkStatusControllerName).
553-
For(&workv1alpha1.Work{}, builder.WithPredicates(c.PredicateFunc)).
546+
ctrlBuilder := controllerruntime.NewControllerManagedBy(mgr).Named(WorkStatusControllerName).
554547
WithOptions(controller.Options{
555548
RateLimiter: ratelimiterflag.DefaultControllerRateLimiter[controllerruntime.Request](c.RateLimiterOptions),
556-
}).Complete(c)
549+
})
550+
551+
if c.WorkPredicateFunc != nil {
552+
ctrlBuilder.For(&workv1alpha1.Work{}, builder.WithPredicates(c.WorkPredicateFunc))
553+
} else {
554+
ctrlBuilder.For(&workv1alpha1.Work{})
555+
}
556+
557+
return ctrlBuilder.Complete(c)
557558
}
558559

559560
func (c *WorkStatusController) eventf(object *unstructured.Unstructured, eventType, reason, messageFmt string, args ...interface{}) {

pkg/controllers/status/work_status_controller_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
107107
Data: map[string][]byte{clusterv1alpha1.SecretTokenKey: []byte("token"), clusterv1alpha1.SecretCADataKey: testCA},
108108
}).Build(),
109109
InformerManager: genericmanager.GetInstance(),
110-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
110+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
111111
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSet,
112112
ClusterCacheSyncTimeout: metav1.Duration{},
113113
RateLimiterOptions: ratelimiterflag.Options{},
@@ -135,7 +135,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
135135
c: WorkStatusController{
136136
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
137137
InformerManager: genericmanager.GetInstance(),
138-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
138+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
139139
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
140140
ClusterCacheSyncTimeout: metav1.Duration{},
141141
RateLimiterOptions: ratelimiterflag.Options{},
@@ -163,7 +163,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
163163
c: WorkStatusController{
164164
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
165165
InformerManager: genericmanager.GetInstance(),
166-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
166+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
167167
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
168168
ClusterCacheSyncTimeout: metav1.Duration{},
169169
RateLimiterOptions: ratelimiterflag.Options{},
@@ -192,7 +192,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
192192
c: WorkStatusController{
193193
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
194194
InformerManager: genericmanager.GetInstance(),
195-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
195+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
196196
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
197197
ClusterCacheSyncTimeout: metav1.Duration{},
198198
RateLimiterOptions: ratelimiterflag.Options{},
@@ -220,7 +220,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
220220
c: WorkStatusController{
221221
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
222222
InformerManager: genericmanager.GetInstance(),
223-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
223+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
224224
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
225225
ClusterCacheSyncTimeout: metav1.Duration{},
226226
RateLimiterOptions: ratelimiterflag.Options{},
@@ -248,7 +248,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
248248
c: WorkStatusController{
249249
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster1", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
250250
InformerManager: genericmanager.GetInstance(),
251-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
251+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
252252
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
253253
ClusterCacheSyncTimeout: metav1.Duration{},
254254
RateLimiterOptions: ratelimiterflag.Options{},
@@ -276,7 +276,7 @@ func TestWorkStatusController_Reconcile(t *testing.T) {
276276
c: WorkStatusController{
277277
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionFalse)).Build(),
278278
InformerManager: genericmanager.GetInstance(),
279-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
279+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
280280
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
281281
ClusterCacheSyncTimeout: metav1.Duration{},
282282
RateLimiterOptions: ratelimiterflag.Options{},
@@ -335,7 +335,7 @@ func TestWorkStatusController_getEventHandler(t *testing.T) {
335335
c := WorkStatusController{
336336
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionFalse)).Build(),
337337
InformerManager: genericmanager.GetInstance(),
338-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
338+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
339339
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
340340
ClusterCacheSyncTimeout: metav1.Duration{},
341341
RateLimiterOptions: ratelimiterflag.Options{},
@@ -351,7 +351,7 @@ func TestWorkStatusController_RunWorkQueue(_ *testing.T) {
351351
c := WorkStatusController{
352352
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionFalse)).Build(),
353353
InformerManager: genericmanager.GetInstance(),
354-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
354+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
355355
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
356356
ClusterCacheSyncTimeout: metav1.Duration{},
357357
RateLimiterOptions: ratelimiterflag.Options{},
@@ -740,7 +740,7 @@ func newWorkStatusController(cluster *clusterv1alpha1.Cluster, dynamicClientSets
740740
c := WorkStatusController{
741741
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(cluster).WithStatusSubresource().Build(),
742742
InformerManager: genericmanager.GetInstance(),
743-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
743+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
744744
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
745745
ClusterCacheSyncTimeout: metav1.Duration{},
746746
RateLimiterOptions: ratelimiterflag.Options{},
@@ -874,7 +874,7 @@ func TestWorkStatusController_recreateResourceIfNeeded(t *testing.T) {
874874
c := WorkStatusController{
875875
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
876876
InformerManager: genericmanager.GetInstance(),
877-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
877+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
878878
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
879879
ClusterCacheSyncTimeout: metav1.Duration{},
880880
RateLimiterOptions: ratelimiterflag.Options{},
@@ -921,7 +921,7 @@ func TestWorkStatusController_buildStatusIdentifier(t *testing.T) {
921921
c := WorkStatusController{
922922
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
923923
InformerManager: genericmanager.GetInstance(),
924-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
924+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
925925
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
926926
ClusterCacheSyncTimeout: metav1.Duration{},
927927
RateLimiterOptions: ratelimiterflag.Options{},
@@ -982,7 +982,7 @@ func TestWorkStatusController_mergeStatus(t *testing.T) {
982982
c := WorkStatusController{
983983
Client: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(newCluster("cluster", clusterv1alpha1.ClusterConditionReady, metav1.ConditionTrue)).Build(),
984984
InformerManager: genericmanager.GetInstance(),
985-
PredicateFunc: helper.NewClusterPredicateOnAgent("test"),
985+
WorkPredicateFunc: helper.NewClusterPredicateOnAgent("test"),
986986
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSetForAgent,
987987
ClusterCacheSyncTimeout: metav1.Duration{},
988988
RateLimiterOptions: ratelimiterflag.Options{},

pkg/util/constants.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,6 @@ const (
4040
// This label indicates the name.
4141
MultiClusterServiceNameLabel = "multiclusterservice.karmada.io/name"
4242

43-
// PropagationInstruction is used to mark a resource(like Work) propagation instruction.
44-
// Valid values includes:
45-
// - suppressed: indicates that the resource should not be propagated.
46-
//
47-
// Note: This instruction is intended to set on Work objects to indicate the Work should be ignored by
48-
// execution controller. The instruction maybe deprecated once we extend the Work API and no other scenario want this.
49-
//
50-
// Deprecated: This label has been deprecated since v1.14, and will be replaced by the filed .spec.suspendDispatching of
51-
// Work API. This label should only be used internally by Karmada, but for compatibility, the deletion will be postponed
52-
// to release 1.15.
53-
PropagationInstruction = "propagation.karmada.io/instruction"
54-
5543
// FederatedResourceQuotaNamespaceLabel is added to Work to specify associated FederatedResourceQuota's namespace.
5644
FederatedResourceQuotaNamespaceLabel = "federatedresourcequota.karmada.io/namespace"
5745

@@ -90,9 +78,6 @@ const (
9078

9179
// RetainReplicasValue is an optional value of RetainReplicasLabel, indicating retain
9280
RetainReplicasValue = "true"
93-
94-
// PropagationInstructionSuppressed indicates that the resource should not be propagated.
95-
PropagationInstructionSuppressed = "suppressed"
9681
)
9782

9883
// Define annotations used by karmada system.

0 commit comments

Comments
 (0)