Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions pkg/reconciler/configmappropagation/configmappropagation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -333,11 +334,22 @@ 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
}
}
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
}
52 changes: 51 additions & 1 deletion pkg/reconciler/configmappropagation/configmappropagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions pkg/reconciler/testing/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/reconciler/testing/configmappropagation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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()
Expand Down