Merge pull request #611 from sgreene570/bz-1954330

Bug 1954330: ingress: Fix up openshift-ingress namespace reconciliation

pkg/operator/controller/ingress/namespace.go

@@ -86,7 +86,7 @@ func routerNamespaceChanged(current, expected *corev1.Namespace) (bool, *corev1.
 	}
 
 	knownLabels := []string{
-		"opensift.io/cluster-monitoring",
+		"openshift.io/cluster-monitoring",
 		"name",
 		"network.openshift.io/policy-group",
 		"policy-group.network.openshift.io/ingress",
@@ -104,14 +104,14 @@ func routerNamespaceChanged(current, expected *corev1.Namespace) (bool, *corev1.
 	}
 
 	for _, annotation := range knownAnnotations {
-		if current.Annotations[annotation] != expected.Annotations[annotation] {
+		if val, ok := current.Annotations[annotation]; !ok || val != expected.Annotations[annotation] {
 			updated.Annotations[annotation] = expected.Annotations[annotation]
 			changed = true
 		}
 	}
 
 	for _, label := range knownLabels {
-		if current.Labels[label] != expected.Labels[label] {
+		if val, ok := current.Labels[label]; !ok || val != expected.Labels[label] {
 			updated.Labels[label] = expected.Labels[label]
 			changed = true
 		}

pkg/operator/controller/ingress/namespace_test.go

@@ -41,16 +41,23 @@ func TestRouterNamespaceChanged(t *testing.T) {
 			},
 			expect: false,
 		},
+		{
+			description: "if a managed label with an empty string value is deleted",
+			mutate: func(ns *corev1.Namespace) {
+				delete(ns.Labels, "policy-group.network.openshift.io/ingress")
+			},
+			expect: true,
+		},
 	}
 
 	for _, tc := range testCases {
-		original := manifests.RouterNamespace()
-		mutated := original.DeepCopy()
+		desired := manifests.RouterNamespace()
+		mutated := desired.DeepCopy()
 		tc.mutate(mutated)
-		if changed, updated := routerNamespaceChanged(original, mutated); changed != tc.expect {
+		if changed, updated := routerNamespaceChanged(mutated, desired); changed != tc.expect {
 			t.Errorf("%s, expect routerNamespaceChanged to be %t, got %t", tc.description, tc.expect, changed)
 		} else if changed {
-			if changedAgain, _ := routerNamespaceChanged(mutated, updated); changedAgain {
+			if changedAgain, _ := routerNamespaceChanged(desired, updated); changedAgain {
 				t.Errorf("%s, routerNamespaceChanged does not behave as a fixed point function", tc.description)
 			}
 		}