diff --git a/pkg/reconciler/configmappropagation/configmappropagation.go b/pkg/reconciler/configmappropagation/configmappropagation.go index f762092554b..1f38c6c6c42 100644 --- a/pkg/reconciler/configmappropagation/configmappropagation.go +++ b/pkg/reconciler/configmappropagation/configmappropagation.go @@ -298,7 +298,8 @@ func (r *Reconciler) createOrUpdateConfigMaps(ctx context.Context, cmp *v1alpha1 // OwnerReference will be removed when the knative.dev/config-propagation:copy label is not set in copy configmap // so that this copy configmap will not be deleted if cmp is deleted. expected = current.DeepCopy() - expected.OwnerReferences = nil + // Delete the CMP owner reference, and keep all other owner references (if any). + expected.OwnerReferences = removeOwnerReference(expected.OwnerReferences, cmp.UID) // It will return false for the create/update action is not successful, due to removed copy label. // But it is not an error for ConfigMapPropagation for not propagating successfully. succeed = false @@ -317,7 +318,7 @@ func (r *Reconciler) createOrUpdateConfigMaps(ctx context.Context, cmp *v1alpha1 // deleteOrKeepConfigMap will return error and bool (represents whether a delete action is successful or not). func (r *Reconciler) deleteOrKeepConfigMap(ctx context.Context, cmp *v1alpha1.ConfigMapPropagation, copyConfigMap *corev1.ConfigMap, originalConfigMapName string, originalConfigMapList []*corev1.ConfigMap) (error, bool) { - originalConfigMap, contains := r.contains(originalConfigMapName, originalConfigMapList) + originalConfigMap, contains := contains(originalConfigMapName, originalConfigMapList) expectedSelector := resources.ExpectedOriginalSelector(cmp.Spec.Selector) if !contains || !expectedSelector.Matches(labels.Set(originalConfigMap.Labels)) { // If Original ConfigMap no longer exists or no longer has the required label, delete copy ConfigMap. @@ -333,7 +334,7 @@ func (r *Reconciler) deleteOrKeepConfigMap(ctx context.Context, cmp *v1alpha1.Co } // contains returns a configmap object if its name is in a configmaplist. -func (r *Reconciler) contains(name string, list []*corev1.ConfigMap) (*corev1.ConfigMap, bool) { +func contains(name string, list []*corev1.ConfigMap) (*corev1.ConfigMap, bool) { for _, configMap := range list { if configMap.Name == name { return configMap, true @@ -341,3 +342,14 @@ func (r *Reconciler) contains(name string, list []*corev1.ConfigMap) (*corev1.Co } return nil, false } + +// removeOwnerReference removes the target ownerReference and returns a new slice of ownerReferences. +func removeOwnerReference(ownerReferences []metav1.OwnerReference, uid types.UID) []metav1.OwnerReference { + var expected []metav1.OwnerReference + for _, owner := range ownerReferences { + if owner.UID != uid { + expected = append(expected, owner) + } + } + return expected +} diff --git a/pkg/reconciler/configmappropagation/configmappropagation_test.go b/pkg/reconciler/configmappropagation/configmappropagation_test.go index 1ea31203caf..708addde976 100644 --- a/pkg/reconciler/configmappropagation/configmappropagation_test.go +++ b/pkg/reconciler/configmappropagation/configmappropagation_test.go @@ -352,7 +352,7 @@ func TestAllCase(t *testing.T) { Eventf(corev1.EventTypeNormal, configMapPropagationReadinessChanged, "ConfigMapPropagation %q became ready", configMapPropagationName), }, }, { - Name: "Copy ConfigMap no longer has required labels", + Name: "Copy ConfigMap no longer has required labels, with only one owner reference", Key: testKey, Objects: []runtime.Object{ NewConfigMapPropagation(configMapPropagationName, currentNS, @@ -395,6 +395,56 @@ func TestAllCase(t *testing.T) { `Stop propagating ConfigMap: test-original-cm`), Eventf(corev1.EventTypeNormal, configMapPropagationReadinessChanged, "ConfigMapPropagation %q became ready", configMapPropagationName), }, + }, { + Name: "Copy ConfigMap no longer has required labels, with multiple owner references", + Key: testKey, + Objects: []runtime.Object{ + NewConfigMapPropagation(configMapPropagationName, currentNS, + WithInitConfigMapPropagationConditions, + WithConfigMapPropagationSelector(selector), + ), + NewConfigMapPropagation("default", currentNS, + WithInitConfigMapPropagationConditions, + WithConfigMapPropagationUID("1234"), + ), + NewConfigMap(originalConfigMapName, originalNS, + WithConfigMapLabels(originalSelector), + ), + NewConfigMap(copyConfigMapName, currentNS, + WithConfigMapLabels(metav1.LabelSelector{ + MatchLabels: map[string]string{}, + }), + WithConfigMapOwnerReference(NewConfigMapPropagation(configMapPropagationName, currentNS, + WithInitConfigMapPropagationConditions, + WithConfigMapPropagationSelector(selector), + )), + WithConfigMapOwnerReference(NewConfigMapPropagation("additional", currentNS, WithInitConfigMapPropagationConditions, WithConfigMapPropagationUID("1234"))), + ), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewConfigMap(copyConfigMapName, currentNS, + WithConfigMapLabels(metav1.LabelSelector{ + MatchLabels: map[string]string{}, + }), + WithConfigMapOwnerReference(NewConfigMapPropagation("additional", currentNS, WithInitConfigMapPropagationConditions, WithConfigMapPropagationUID("1234"))), + ), + }}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewConfigMapPropagation(configMapPropagationName, currentNS, + WithInitConfigMapPropagationConditions, + WithConfigMapPropagationSelector(selector), + WithConfigMapPropagationPropagated, + WithInitConfigMapStatus(), + WithCopyConfigMapStatus("test-cmp-test-original-cm", "knative-eventing/test-original-cm", + "Stop", "True", `copy ConfigMap doesn't have copy label, stop propagating this ConfigMap`), + ), + }}, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, configMapPropagationPropagateSingleConfigMapSucceed, + `Stop propagating ConfigMap: test-original-cm`), + Eventf(corev1.EventTypeNormal, configMapPropagationReadinessChanged, "ConfigMapPropagation %q became ready", configMapPropagationName), + }, }, { Name: "Create new ConfigMap failed", Key: testKey, diff --git a/pkg/reconciler/testing/configmap.go b/pkg/reconciler/testing/configmap.go index 8f412628a83..dfdd0aabd28 100644 --- a/pkg/reconciler/testing/configmap.go +++ b/pkg/reconciler/testing/configmap.go @@ -49,9 +49,10 @@ func WithConfigMapLabels(labels metav1.LabelSelector) ConfigMapOption { func WithConfigMapOwnerReference(ConfigMapPropagation *v1alpha1.ConfigMapPropagation) ConfigMapOption { return func(cm *v1.ConfigMap) { - cm.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ - *kmeta.NewControllerRef(ConfigMapPropagation), + if cm.ObjectMeta.OwnerReferences == nil { + cm.ObjectMeta.OwnerReferences = []metav1.OwnerReference{} } + cm.ObjectMeta.OwnerReferences = append(cm.ObjectMeta.OwnerReferences, *kmeta.NewControllerRef(ConfigMapPropagation)) } } diff --git a/pkg/reconciler/testing/configmappropagation.go b/pkg/reconciler/testing/configmappropagation.go index fa72f41cec5..97e9399db0c 100644 --- a/pkg/reconciler/testing/configmappropagation.go +++ b/pkg/reconciler/testing/configmappropagation.go @@ -21,6 +21,8 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "knative.dev/eventing/pkg/apis/configs/v1alpha1" ) @@ -89,6 +91,12 @@ func WithConfigMapPropagationStatusObservedGeneration(gen int64) ConfigMapPropag } } +func WithConfigMapPropagationUID(uid string) ConfigMapPropagationOption { + return func(cmp *v1alpha1.ConfigMapPropagation) { + cmp.UID = types.UID(uid) + } +} + // WithConfigMapPropagationPropagated calls .Status.MarkConfigMapPropagationPropagated on the ConfigMapPropagation. func WithConfigMapPropagationPropagated(cmp *v1alpha1.ConfigMapPropagation) { cmp.Status.MarkPropagated()