Resource template cannot be reconciled again due to cache modification

[whitewindmills]

What happened:
I deleted propagation policy default/test-pp to unbind from configmap default/test-cm, but find that configmap default/test-cm cannot be reconciled again.

that means configmap default/test-cm cannot be matched by new propagation policy default/test-pp-1.

What you expected to happen:
configmap default/test-cm can be matched by new propagation policy default/test-pp-1.

How to reproduce it (as minimally and precisely as possible):

1. create such configmap(keep labels non-nil):

apiVersion: v1
data:
  a: b
kind: ConfigMap
metadata:
  labels:
    a: b
  name: test-cm
  namespace: default

2. create propagation policy.
3. delete propagation policy after configmap is propagated.
4. check if configmap can be reconcile again.

Anything else we need to know?:
detector deletes directly policy labels from cached resource template.

karmada/pkg/detector/detector.go

Lines 1117 to 1131 in d916d87

	workload, err := helper.FetchResourceTemplate(d.DynamicClient, d.InformerManager, d.RESTMapper, objRef)
	if err != nil {
	// do nothing if resource template not exist, it might has been removed.
	if apierrors.IsNotFound(err) {
	return nil
	}
	klog.Errorf("Failed to fetch resource(kind=%s, %s/%s): %v", objRef.Kind, objRef.Namespace, objRef.Name, err)
	return err
	}
	workloadLabels := workload.GetLabels()
	for _, l := range labels {
	delete(workloadLabels, l)
	}
	workload.SetLabels(workloadLabels)

Environment:

• Karmada version:
• kubectl-karmada or karmadactl version (the result of kubectl-karmada version or karmadactl version):
• Others:

[whitewindmills]

/assign

[RainbowMango]

karmada/pkg/detector/detector.go

Line 1139 in a2dc2e8

newWorkload, err := d.DynamicClient.Resource(gvr).Namespace(workload.GetNamespace()).Update(context.TODO(), workload, metav1.UpdateOptions{})

detector deletes directly policy labels from cached resource template.

Do you mean here actually updates the cache?

[whitewindmills]

Do you mean here actually updates the cache?

here, workload is from the informer cache.

karmada/pkg/detector/detector.go

Line 1131 in d916d87

workload.SetLabels(workloadLabels)

[RainbowMango]

Yeah I get it, It mistakenly updated the cache, but I still don't understand why the update event would be ignored.

[whitewindmills]

it's ignored by the function SpecificationChanged.

karmada/pkg/detector/detector.go

Lines 306 to 309 in 946fc72

	if !SpecificationChanged(unstructuredOldObj, unstructuredNewObj) {
	klog.V(4).Infof("Ignore update event of object (kind=%s, %s/%s) as specification no change", unstructuredOldObj.GetKind(), unstructuredOldObj.GetNamespace(), unstructuredOldObj.GetName())
	return
	}

it removes uncessary fields and calls reflect.DeepEqual to compare if the remaining fields actually changed.

karmada/pkg/detector/eventfilter.go

Lines 9 to 23 in 946fc72

	// SpecificationChanged check if the specification of the given object change or not
	func SpecificationChanged(oldObj, newObj *unstructured.Unstructured) bool {
	oldBackup := oldObj.DeepCopy()
	newBackup := newObj.DeepCopy()
	// Remove the status and some system defined mutable fields in metadata, including managedFields and resourceVersion.
	// Refer to https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/object-meta/#ObjectMeta for more details.
	removedFields := [][]string{{"status"}, {"metadata", "managedFields"}, {"metadata", "resourceVersion"}}
	for _, r := range removedFields {
	unstructured.RemoveNestedField(oldBackup.Object, r...)
	unstructured.RemoveNestedField(newBackup.Object, r...)
	}
	return !reflect.DeepEqual(oldBackup, newBackup)
	}

[RainbowMango]

Do you mean oldObj comes from cache and newObj is the updated object, since the labels have already been removed from the cache, so the oldOjb is always equal to newOjb?

[whitewindmills]

since the labels have already been removed from the cache, so the oldOjb is always equal to newOjb?

yes

[RainbowMango]

Thanks. That makes sense now.