From 520631fe42e97cc82513d6ae2ca065ce2dc122c5 Mon Sep 17 00:00:00 2001 From: whitewindmills Date: Wed, 15 Nov 2023 11:34:20 +0800 Subject: [PATCH] ensure idempotence when deleting policy Signed-off-by: whitewindmills --- pkg/detector/detector.go | 53 ++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/pkg/detector/detector.go b/pkg/detector/detector.go index 62d40fd4f08e..08dfaa3bd53c 100644 --- a/pkg/detector/detector.go +++ b/pkg/detector/detector.go @@ -1039,20 +1039,20 @@ func (d *ResourceDetector) HandlePropagationPolicyDeletion(policyNS string, poli } for index, binding := range rbs.Items { - // Cleanup the labels from the reference binding so that the karmada scheduler won't reschedule the binding. - if err := d.CleanupResourceBindingLabels(&rbs.Items[index], policyv1alpha1.PropagationPolicyNamespaceLabel, policyv1alpha1.PropagationPolicyNameLabel); err != nil { - klog.Errorf("Failed to cleanup label from resource binding(%s/%s) when propagation policy(%s/%s) removing, error: %v", - binding.Namespace, binding.Name, policyNS, policyName, err) - return err - } - - // Cleanup the labels from the object referencing by binding. + // Clean up the labels from the object referencing by binding first to ensure idempotence. // In addition, this will give the object a chance to match another policy. if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.PropagationPolicyNamespaceLabel, policyv1alpha1.PropagationPolicyNameLabel); err != nil { - klog.Errorf("Failed to cleanup label from resource(%s-%s/%s) when propagation policy(%s/%s) removing, error: %v", + klog.Errorf("Failed to clean up label from resource(%s-%s/%s) when propagation policy(%s/%s) removing, error: %v", binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, policyNS, policyName, err) return err } + + // Clean up the labels from the reference binding so that the karmada scheduler won't reschedule the binding. + if err := d.CleanupResourceBindingLabels(&rbs.Items[index], policyv1alpha1.PropagationPolicyNamespaceLabel, policyv1alpha1.PropagationPolicyNameLabel); err != nil { + klog.Errorf("Failed to clean up label from resource binding(%s/%s) when propagation policy(%s/%s) removing, error: %v", + binding.Namespace, binding.Name, policyNS, policyName, err) + return err + } } return nil } @@ -1075,18 +1075,18 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyName str errs = append(errs, err) } else if len(crbs.Items) > 0 { for index, binding := range crbs.Items { - // Cleanup the labels from the reference binding so that the karmada scheduler won't reschedule the binding. - if err := d.CleanupClusterResourceBindingLabels(&crbs.Items[index], policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { - klog.Errorf("Failed to cleanup label from cluster resource binding(%s) when cluster propagation policy(%s) removing, error: %v", - binding.Name, policyName, err) + // Clean up the labels from the object referencing by binding first to ensure idempotence. + // In addition, this will give the object a chance to match another policy. + if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { + klog.Errorf("Failed to clean up label from resource(%s-%s) when cluster propagation policy(%s) removing, error: %v", + binding.Spec.Resource.Kind, binding.Spec.Resource.Name, policyName, err) errs = append(errs, err) } - // Cleanup the labels from the object referencing by binding. - // In addition, this will give the object a chance to match another policy. - if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { - klog.Errorf("Failed to cleanup label from resource(%s-%s/%s) when cluster resource binding(%s) removing, error: %v", - binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, binding.Name, err) + // Clean up the labels from the reference binding so that the karmada scheduler won't reschedule the binding. + if err := d.CleanupClusterResourceBindingLabels(&crbs.Items[index], policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { + klog.Errorf("Failed to clean up label from cluster resource binding(%s) when cluster propagation policy(%s) removing, error: %v", + binding.Name, policyName, err) errs = append(errs, err) } } @@ -1099,17 +1099,18 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyName str errs = append(errs, err) } else if len(rbs.Items) > 0 { for index, binding := range rbs.Items { - // Cleanup the labels from the reference binding so that the karmada scheduler won't reschedule the binding. - if err := d.CleanupResourceBindingLabels(&rbs.Items[index], policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { - klog.Errorf("Failed to cleanup label from resource binding(%s/%s) when cluster propagation policy(%s) removing, error: %v", - binding.Namespace, binding.Name, policyName, err) + // Clean up the labels from the object referencing by binding first to ensure idempotence. + // In addition, this will give the object a chance to match another policy. + if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { + klog.Errorf("Failed to clean up label from resource(%s-%s/%s) when cluster propagation policy(%s) removing, error: %v", + binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, policyName, err) errs = append(errs, err) } - // Cleanup the labels from the object referencing by binding. - // In addition, this will give the object a chance to match another policy. - if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { - klog.Errorf("Failed to cleanup label from resource binding(%s/%s), error: %v", binding.Namespace, binding.Name, err) + // Clean up the labels from the reference binding so that the karmada scheduler won't reschedule the binding. + if err := d.CleanupResourceBindingLabels(&rbs.Items[index], policyv1alpha1.ClusterPropagationPolicyLabel); err != nil { + klog.Errorf("Failed to clean up label from resource binding(%s/%s) when cluster propagation policy(%s) removing, error: %v", + binding.Namespace, binding.Name, policyName, err) errs = append(errs, err) } }