Skip to content

Commit

Permalink
rename all mark-related terms to claim metadata
Browse files Browse the repository at this point in the history
Signed-off-by: zhzhuang-zju <[email protected]>
  • Loading branch information
zhzhuang-zju committed Oct 8, 2024
1 parent 2eb38a9 commit 380f12d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 50 deletions.
64 changes: 32 additions & 32 deletions pkg/detector/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ func (d *ResourceDetector) ReconcileClusterPropagationPolicy(key util.QueueKey)
}

// HandlePropagationPolicyDeletion handles PropagationPolicy delete event.
// After a policy is removed, the label and annotations marked on relevant resource template will be removed (which gives
// After a policy is removed, the label and annotations claimed on relevant resource template will be removed (which gives
// the resource template a change to match another policy).
//
// Note: The relevant ResourceBinding will continue to exist until the resource template is gone.
Expand All @@ -1059,20 +1059,20 @@ func (d *ResourceDetector) HandlePropagationPolicyDeletion(policyID string) erro

var errs []error
for index, binding := range rbs.Items {
// Must remove the marks, such as labels and annotations, from the resource template ahead of ResourceBinding,
// otherwise might lose the chance to do that in a retry loop (in particular, the marks was successfully removed
// Must remove the claim metadata, such as labels and annotations, from the resource template ahead of ResourceBinding,
// otherwise might lose the chance to do that in a retry loop (in particular, the claim metadata was successfully removed
// from ResourceBinding, but resource template not), since the ResourceBinding will not be listed again.
if err := d.CleanupResourceTemplateMarks(binding.Spec.Resource, CleanupPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up marks from resource(%s-%s/%s) when propagationPolicy removed, error: %v",
if err := d.CleanupResourceTemplateClaimMetadata(binding.Spec.Resource, CleanupPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up claim metadata from resource(%s-%s/%s) when propagationPolicy removed, error: %v",
binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, err)
errs = append(errs, err)
// Skip cleaning up policy labels and annotations from ResourceBinding, give a chance to do that in a retry loop.
continue
}

// Clean up the marks from the reference binding so that the karmada scheduler won't reschedule the binding.
if err := d.CleanupResourceBindingMarks(&rbs.Items[index], CleanupPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up marks from resource binding(%s/%s) when propagationPolicy removed, error: %v",
// Clean up the claim metadata from the reference binding so that the karmada scheduler won't reschedule the binding.
if err := d.CleanupResourceBindingClaimMetadata(&rbs.Items[index], CleanupPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up claim metadata from resource binding(%s/%s) when propagationPolicy removed, error: %v",
binding.Namespace, binding.Name, err)
errs = append(errs, err)
}
Expand All @@ -1081,7 +1081,7 @@ func (d *ResourceDetector) HandlePropagationPolicyDeletion(policyID string) erro
}

// HandleClusterPropagationPolicyDeletion handles ClusterPropagationPolicy delete event.
// After a policy is removed, the label and annotation marked on relevant resource template will be removed (which gives
// After a policy is removed, the label and annotation claimed on relevant resource template will be removed (which gives
// the resource template a change to match another policy).
//
// Note: The relevant ClusterResourceBinding or ResourceBinding will continue to exist until the resource template is gone.
Expand All @@ -1098,20 +1098,20 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyID strin
errs = append(errs, err)
} else if len(crbs.Items) > 0 {
for index, binding := range crbs.Items {
// Must remove the marks, such as labels and annotations, from the resource template ahead of
// Must remove the claim metadata, such as labels and annotations, from the resource template ahead of
// ClusterResourceBinding, otherwise might lose the chance to do that in a retry loop (in particular, the
// marks was successfully removed from ClusterResourceBinding, but resource template not), since the
// claim metadata was successfully removed from ClusterResourceBinding, but resource template not), since the
// ClusterResourceBinding will not be listed again.
if err := d.CleanupResourceTemplateMarks(binding.Spec.Resource, CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up marks from resource(%s-%s) when clusterPropagationPolicy removed, error: %v",
if err := d.CleanupResourceTemplateClaimMetadata(binding.Spec.Resource, CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up claim metadata from resource(%s-%s) when clusterPropagationPolicy removed, error: %v",
binding.Spec.Resource.Kind, binding.Spec.Resource.Name, err)
// Skip cleaning up policy labels and annotations from ClusterResourceBinding, give a chance to do that in a retry loop.
continue
}

// Clean up the marks from the reference binding so that the Karmada scheduler won't reschedule the binding.
if err := d.CleanupClusterResourceBindingMarks(&crbs.Items[index], CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up marks from clusterResourceBinding(%s) when clusterPropagationPolicy removed, error: %v",
// Clean up the claim metadata from the reference binding so that the Karmada scheduler won't reschedule the binding.
if err := d.CleanupClusterResourceBindingClaimMetadata(&crbs.Items[index], CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up claim metadata from clusterResourceBinding(%s) when clusterPropagationPolicy removed, error: %v",
binding.Name, err)
errs = append(errs, err)
}
Expand All @@ -1125,20 +1125,20 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyID strin
errs = append(errs, err)
} else if len(rbs.Items) > 0 {
for index, binding := range rbs.Items {
// Must remove the marks, such as labels and annotations, from the resource template ahead of ResourceBinding,
// Must remove the claim metadata, such as labels and annotations, from the resource template ahead of ResourceBinding,
// otherwise might lose the chance to do that in a retry loop (in particular, the label was successfully
// removed from ResourceBinding, but resource template not), since the ResourceBinding will not be listed again.
if err := d.CleanupResourceTemplateMarks(binding.Spec.Resource, CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up marks from resource(%s-%s/%s) when clusterPropagationPolicy removed, error: %v",
if err := d.CleanupResourceTemplateClaimMetadata(binding.Spec.Resource, CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up claim metadata from resource(%s-%s/%s) when clusterPropagationPolicy removed, error: %v",
binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, err)
errs = append(errs, err)
// Skip cleaning up policy labels and annotations from ResourceBinding, give a chance to do that in a retry loop.
continue
}

// Clean up the marks from the reference binding so that the Karmada scheduler won't reschedule the binding.
if err := d.CleanupResourceBindingMarks(&rbs.Items[index], CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up marks from resourceBinding(%s/%s) when clusterPropagationPolicy removed, error: %v",
// Clean up the claim metadata from the reference binding so that the Karmada scheduler won't reschedule the binding.
if err := d.CleanupResourceBindingClaimMetadata(&rbs.Items[index], CleanupCPPClaimMetadata); err != nil {
klog.Errorf("Failed to clean up claim metadata from resourceBinding(%s/%s) when clusterPropagationPolicy removed, error: %v",
binding.Namespace, binding.Name, err)
errs = append(errs, err)
}
Expand All @@ -1154,7 +1154,7 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyID strin
// from waiting list and throw the object to it's reconcile queue. If not, do nothing.
// Finally, handle the propagation policy preemption process if preemption is enabled.
func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *policyv1alpha1.PropagationPolicy) error {
// If the Policy's ResourceSelectors change, causing certain resources to no longer match the Policy, the label marked
// If the Policy's ResourceSelectors change, causing certain resources to no longer match the Policy, the label claimed
// on relevant resource template will be removed (which gives the resource template a change to match another policy).
policyID := policy.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel]
err := d.cleanPPUnmatchedRBs(policyID, policy.Namespace, policy.Name, policy.Spec.ResourceSelectors)
Expand Down Expand Up @@ -1196,7 +1196,7 @@ func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *polic
}

// If preemption is enabled, handle the preemption process.
// If this policy succeeds in preempting resource managed by other policy, the label marked on relevant resource
// If this policy succeeds in preempting resource managed by other policy, the label claimed on relevant resource
// will be replaced, which gives the resource template a change to match to this policy.
if preemptionEnabled(policy.Spec.Preemption) {
return d.handlePropagationPolicyPreemption(policy)
Expand All @@ -1212,7 +1212,7 @@ func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *polic
// from waiting list and throw the object to it's reconcile queue. If not, do nothing.
// Finally, handle the cluster propagation policy preemption process if preemption is enabled.
func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy *policyv1alpha1.ClusterPropagationPolicy) error {
// If the Policy's ResourceSelectors change, causing certain resources to no longer match the Policy, the label marked
// If the Policy's ResourceSelectors change, causing certain resources to no longer match the Policy, the label claimed
// on relevant resource template will be removed (which gives the resource template a change to match another policy).
policyID := policy.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel]
err := d.cleanCPPUnmatchedRBs(policyID, policy.Name, policy.Spec.ResourceSelectors)
Expand Down Expand Up @@ -1269,7 +1269,7 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy
}

// If preemption is enabled, handle the preemption process.
// If this policy succeeds in preempting resource managed by other policy, the label marked on relevant resource
// If this policy succeeds in preempting resource managed by other policy, the label claimed on relevant resource
// will be replaced, which gives the resource template a change to match to this policy.
if preemptionEnabled(policy.Spec.Preemption) {
return d.handleClusterPropagationPolicyPreemption(policy)
Expand All @@ -1278,8 +1278,8 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy
return nil
}

// CleanupResourceTemplateMarks removes marks, such as labels and annotations, from object referencing by objRef.
func (d *ResourceDetector) CleanupResourceTemplateMarks(objRef workv1alpha2.ObjectReference, cleanupFunc func(obj metav1.Object)) error {
// CleanupResourceTemplateClaimMetadata removes claim metadata, such as labels and annotations, from object referencing by objRef.
func (d *ResourceDetector) CleanupResourceTemplateClaimMetadata(objRef workv1alpha2.ObjectReference, cleanupFunc func(obj metav1.Object)) error {
gvr, err := restmapper.GetGroupVersionResource(d.RESTMapper, schema.FromAPIVersionAndKind(objRef.APIVersion, objRef.Kind))
if err != nil {
klog.Errorf("Failed to convert GVR from GVK(%s/%s), err: %v", objRef.APIVersion, objRef.Kind, err)
Expand Down Expand Up @@ -1309,8 +1309,8 @@ func (d *ResourceDetector) CleanupResourceTemplateMarks(objRef workv1alpha2.Obje
})
}

// CleanupResourceBindingMarks removes marks, such as labels and annotations, from resource binding.
func (d *ResourceDetector) CleanupResourceBindingMarks(rb *workv1alpha2.ResourceBinding, cleanupFunc func(obj metav1.Object)) error {
// CleanupResourceBindingClaimMetadata removes claim metadata, such as labels and annotations, from resource binding.
func (d *ResourceDetector) CleanupResourceBindingClaimMetadata(rb *workv1alpha2.ResourceBinding, cleanupFunc func(obj metav1.Object)) error {
return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
cleanupFunc(rb)
updateErr := d.Client.Update(context.TODO(), rb)
Expand All @@ -1328,8 +1328,8 @@ func (d *ResourceDetector) CleanupResourceBindingMarks(rb *workv1alpha2.Resource
})
}

// CleanupClusterResourceBindingMarks removes marks, such as labels and annotations, from cluster resource binding.
func (d *ResourceDetector) CleanupClusterResourceBindingMarks(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error {
// CleanupClusterResourceBindingClaimMetadata removes claim metadata, such as labels and annotations, from cluster resource binding.
func (d *ResourceDetector) CleanupClusterResourceBindingClaimMetadata(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error {
return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
cleanupFunc(crb)
updateErr := d.Client.Update(context.TODO(), crb)
Expand Down
16 changes: 8 additions & 8 deletions pkg/detector/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (d *ResourceDetector) cleanPPUnmatchedRBs(policyID, policyNamespace, policy
return err
}

return d.removeRBsMarks(bindings, selectors, propagationPolicyClaimLabels, propagationPolicyClaimAnnotations)
return d.removeRBsClaimMetadata(bindings, selectors, propagationPolicyClaimLabels, propagationPolicyClaimAnnotations)
}

func (d *ResourceDetector) cleanCPPUnmatchedRBs(policyID, policyName string, selectors []policyv1alpha1.ResourceSelector) error {
Expand All @@ -197,7 +197,7 @@ func (d *ResourceDetector) cleanCPPUnmatchedRBs(policyID, policyName string, sel
return err
}

return d.removeRBsMarks(bindings, selectors, clusterPropagationPolicyClaimLabels, clusterPropagationPolicyClaimAnnotations)
return d.removeRBsClaimMetadata(bindings, selectors, clusterPropagationPolicyClaimLabels, clusterPropagationPolicyClaimAnnotations)
}

func (d *ResourceDetector) cleanUnmatchedCRBs(policyID, policyName string, selectors []policyv1alpha1.ResourceSelector) error {
Expand All @@ -206,13 +206,13 @@ func (d *ResourceDetector) cleanUnmatchedCRBs(policyID, policyName string, selec
return err
}

return d.removeCRBsMarks(bindings, selectors, clusterPropagationPolicyClaimLabels, clusterPropagationPolicyClaimAnnotations)
return d.removeCRBsClaimMetadata(bindings, selectors, clusterPropagationPolicyClaimLabels, clusterPropagationPolicyClaimAnnotations)
}

func (d *ResourceDetector) removeRBsMarks(bindings *workv1alpha2.ResourceBindingList, selectors []policyv1alpha1.ResourceSelector, labels, annotations []string) error {
func (d *ResourceDetector) removeRBsClaimMetadata(bindings *workv1alpha2.ResourceBindingList, selectors []policyv1alpha1.ResourceSelector, labels, annotations []string) error {
var errs []error
for _, binding := range bindings.Items {
removed, err := d.removeResourceMarksIfNotMatched(binding.Spec.Resource, selectors, labels, annotations)
removed, err := d.removeResourceClaimMetadataIfNotMatched(binding.Spec.Resource, selectors, labels, annotations)
if err != nil {
klog.Errorf("Failed to remove resource labels and annotations when resource not match with policy selectors, err: %v", err)
errs = append(errs, err)
Expand All @@ -235,11 +235,11 @@ func (d *ResourceDetector) removeRBsMarks(bindings *workv1alpha2.ResourceBinding
return errors.NewAggregate(errs)
}

func (d *ResourceDetector) removeCRBsMarks(bindings *workv1alpha2.ClusterResourceBindingList,
func (d *ResourceDetector) removeCRBsClaimMetadata(bindings *workv1alpha2.ClusterResourceBindingList,
selectors []policyv1alpha1.ResourceSelector, removeLabels, removeAnnotations []string) error {
var errs []error
for _, binding := range bindings.Items {
removed, err := d.removeResourceMarksIfNotMatched(binding.Spec.Resource, selectors, removeLabels, removeAnnotations)
removed, err := d.removeResourceClaimMetadataIfNotMatched(binding.Spec.Resource, selectors, removeLabels, removeAnnotations)
if err != nil {
klog.Errorf("Failed to remove resource labels and annotations when resource not match with policy selectors, err: %v", err)
errs = append(errs, err)
Expand All @@ -262,7 +262,7 @@ func (d *ResourceDetector) removeCRBsMarks(bindings *workv1alpha2.ClusterResourc
return errors.NewAggregate(errs)
}

func (d *ResourceDetector) removeResourceMarksIfNotMatched(objectReference workv1alpha2.ObjectReference,
func (d *ResourceDetector) removeResourceClaimMetadataIfNotMatched(objectReference workv1alpha2.ObjectReference,
selectors []policyv1alpha1.ResourceSelector, labels, annotations []string) (bool, error) {
objectKey, err := helper.ConstructClusterWideKey(objectReference)
if err != nil {
Expand Down
20 changes: 10 additions & 10 deletions pkg/detector/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ func Test_cleanUnmatchedCRBs(t *testing.T) {
}
}

func Test_removeRBsMarks(t *testing.T) {
func Test_removeRBsClaimMetadata(t *testing.T) {
scheme := runtime.NewScheme()
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
utilruntime.Must(appsv1.AddToScheme(scheme))
Expand Down Expand Up @@ -664,15 +664,15 @@ func Test_removeRBsMarks(t *testing.T) {
RESTMapper: fakeClient.RESTMapper(),
InformerManager: genMgr,
}
err := resourceDetector.removeRBsMarks(tt.bindings, tt.selectors, tt.removeLabels, tt.removeAnnotations)
err := resourceDetector.removeRBsClaimMetadata(tt.bindings, tt.selectors, tt.removeLabels, tt.removeAnnotations)
if (err != nil) != tt.wantErr {
t.Errorf("removeRBsMarks() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("removeRBsClaimMetadata() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}

func Test_removeCRBsMarks(t *testing.T) {
func Test_removeCRBsClaimMetadata(t *testing.T) {
scheme := runtime.NewScheme()
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
utilruntime.Must(appsv1.AddToScheme(scheme))
Expand Down Expand Up @@ -889,15 +889,15 @@ func Test_removeCRBsMarks(t *testing.T) {
RESTMapper: fakeClient.RESTMapper(),
InformerManager: genMgr,
}
err := resourceDetector.removeCRBsMarks(tt.bindings, tt.selectors, tt.removeLabels, tt.removeAnnotations)
err := resourceDetector.removeCRBsClaimMetadata(tt.bindings, tt.selectors, tt.removeLabels, tt.removeAnnotations)
if (err != nil) != tt.wantErr {
t.Errorf("removeCRBsMarks() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("removeCRBsClaimMetadata() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}

func Test_removeResourceMarksIfNotMatched(t *testing.T) {
func Test_removeResourceClaimMetadataIfNotMatched(t *testing.T) {
scheme := runtime.NewScheme()
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
utilruntime.Must(appsv1.AddToScheme(scheme))
Expand Down Expand Up @@ -1113,13 +1113,13 @@ func Test_removeResourceMarksIfNotMatched(t *testing.T) {
InformerManager: genMgr,
}

updated, err := resourceDetector.removeResourceMarksIfNotMatched(tt.objectReference, tt.selectors, tt.labels, tt.annotations)
updated, err := resourceDetector.removeResourceClaimMetadataIfNotMatched(tt.objectReference, tt.selectors, tt.labels, tt.annotations)

if (err != nil) != tt.wantErr {
t.Errorf("removeResourceMarksIfNotMatched() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("removeResourceClaimMetadataIfNotMatched() error = %v, wantErr %v", err, tt.wantErr)
}
if updated != tt.wantUpdated {
t.Errorf("removeResourceMarksIfNotMatched() = %v, want %v", updated, tt.wantUpdated)
t.Errorf("removeResourceClaimMetadataIfNotMatched() = %v, want %v", updated, tt.wantUpdated)
}
})
}
Expand Down

0 comments on commit 380f12d

Please sign in to comment.