Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object #149

miheer · 2020-11-18T13:42:51Z

BZ 1860136: Fix for Annotation was not propagated to the route when changes made to exisiting ingress object

BZ https://bugzilla.redhat.com/show_bug.cgi?id=1860136

miheer · 2020-11-18T16:14:26Z

@Miciah PTAL

Miciah

Doesn't routeMatchesIngress need to check annotations?

Can you add a test case to TestController_sync?

Miciah · 2020-11-18T16:23:55Z

pkg/route/ingress/ingress.go

@@ -439,11 +439,21 @@ func (c *Controller) sync(key queueKey) error {
 		if err != nil {
 			return err
 		}
+		annotations, err_annotation := json.Marshal(&route.Annotations)
+		if err_annotation != nil {
+			return err_annotation


Could you just use err?

Miciah · 2020-11-18T16:23:55Z

pkg/route/ingress/ingress.go

+		annotations = []byte(fmt.Sprintf(`[{"op":"replace","path":"/metadata/annotations","value":%s}]`, annotations))
+		_, err_annotation = c.routeClient.Routes(route.Namespace).Patch(context.TODO(), route.Name, types.JSONPatchType, annotations, metav1.PatchOptions{})
+		if err_annotation != nil {
+			err_annotations = append(err_annotations, err)


You don't use err_annotations anywhere. Why not append the error value to errs?

miheer · 2020-11-19T14:14:34Z

@Miciah PTAL

miheer · 2020-11-19T14:15:43Z

Unit test have passed ->
[miheer@miheer openshift-controller-manager]$ GO111MODULE=on go test -v -run TestController_sync ./pkg/route/ingress/
=== RUN TestController_sync
=== RUN TestController_sync/no_changes
=== RUN TestController_sync/sync_namespace_-no_ingress
=== RUN TestController_sync/sync_namespace-two_ingress
=== RUN TestController_sync/ignores_incomplete_ingress-no_host
=== RUN TestController_sync/ignores_incomplete_ingress-no_service
=== RUN TestController_sync/ignores_incomplete_ingress-no_paths
=== RUN TestController_sync/ignores_incomplete_ingress-service_does_not_exist
=== RUN TestController_sync/create_route
=== RUN TestController_sync/create_route-_targetPort_string,service_port_with_name
=== RUN TestController_sync/create_route-blocked_by_expectation
=== RUN TestController_sync/update_route
=== RUN TestController_sync/no-op
=== RUN TestController_sync/no-op-ignore_partially_owned_resource
=== RUN TestController_sync/update_ingress_with_missing_secret_ref
=== RUN TestController_sync/update_ingress_to_not_reference_secret
=== RUN TestController_sync/update_route-tls_config_missing
=== RUN TestController_sync/update_route-termination_policy_changed_to_passthrough
=== RUN TestController_sync/update_route-termination_policy_changed_to_reencrypt
=== RUN TestController_sync/update_route-_termination_policy_changed_to_reencrypt_and_no_tls_secret
=== RUN TestController_sync/termination_policy_on_ingress_invalid,nothing_happens
=== RUN TestController_sync/termination_policy_on_ingress_invalid,disables_tls
=== RUN TestController_sync/Empty_tlsconfig_enables_edge_termination_without_explicit_cert
=== RUN TestController_sync/update_route-secret_values_changed
=== RUN TestController_sync/no-op-has_TLS
=== RUN TestController_sync/no-op-has_secret_with_empty_keys
=== RUN TestController_sync/no-op-termination_policy_has_been_changed_by_the_user
=== RUN TestController_sync/update_route-router_admitted_route
=== RUN TestController_sync/update_route-second_router_admitted_route
=== RUN TestController_sync/no-op-ingress_status_already_updated
=== RUN TestController_sync/no-op-router_rejected_route
=== RUN TestController_sync/delete_route_when_referenced_secret_is_not_TLS
=== RUN TestController_sync/delete_route_when_referenced_secret_is_not_valid
=== RUN TestController_sync/ignore_route_when_parent_ingress_no_longer_exists(gc_will_handle)
=== RUN TestController_sync/update_route-termination_policy_changed_to_passthrough_and_timeout_set
--- PASS: TestController_sync (0.00s)
--- PASS: TestController_sync/no_changes (0.00s)
--- PASS: TestController_sync/sync_namespace-no_ingress (0.00s)
--- PASS: TestController_sync/sync_namespace-two_ingress (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_host (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_service (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_paths (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-service_does_not_exist (0.00s)
--- PASS: TestController_sync/create_route (0.00s)
--- PASS: TestController_sync/create_route-_targetPort_string,service_port_with_name (0.00s)
--- PASS: TestController_sync/create_route-blocked_by_expectation (0.00s)
--- PASS: TestController_sync/update_route (0.00s)
--- PASS: TestController_sync/no-op (0.00s)
--- PASS: TestController_sync/no-op-ignore_partially_owned_resource (0.00s)
--- PASS: TestController_sync/update_ingress_with_missing_secret_ref (0.00s)
--- PASS: TestController_sync/update_ingress_to_not_reference_secret (0.00s)
--- PASS: TestController_sync/update_route-tls_config_missing (0.00s)
--- PASS: TestController_sync/update_route-termination_policy_changed_to_passthrough (0.00s)
--- PASS: TestController_sync/update_route-termination_policy_changed_to_reencrypt (0.00s)
--- PASS: TestController_sync/update_route-_termination_policy_changed_to_reencrypt_and_no_tls_secret (0.00s)
--- PASS: TestController_sync/termination_policy_on_ingress_invalid,nothing_happens (0.00s)
--- PASS: TestController_sync/termination_policy_on_ingress_invalid,disables_tls (0.00s)
--- PASS: TestController_sync/Empty_tlsconfig_enables_edge_termination_without_explicit_cert (0.00s)
--- PASS: TestController_sync/update_route-secret_values_changed (0.00s)
--- PASS: TestController_sync/no-op-has_TLS (0.00s)
--- PASS: TestController_sync/no-op-has_secret_with_empty_keys (0.00s)
--- PASS: TestController_sync/no-op-termination_policy_has_been_changed_by_the_user (0.00s)
--- PASS: TestController_sync/update_route-router_admitted_route (0.00s)
--- PASS: TestController_sync/update_route-second_router_admitted_route (0.00s)
--- PASS: TestController_sync/no-op-ingress_status_already_updated (0.00s)
--- PASS: TestController_sync/no-op-router_rejected_route (0.00s)
--- PASS: TestController_sync/delete_route_when_referenced_secret_is_not_TLS (0.00s)
--- PASS: TestController_sync/delete_route_when_referenced_secret_is_not_valid (0.00s)
--- PASS: TestController_sync/ignore_route_when_parent_ingress_no_longer_exists(gc_will_handle) (0.00s)
--- PASS: TestController_sync/update_route-_termination_policy_changed_to_passthrough_and_timeout_set (0.00s)
PASS
ok github.com/openshift/openshift-controller-manager/pkg/route/ingress (cached)
[miheer@miheer openshift-controller-manager]$

Miciah

/retitle Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object

Could you add a unit test where the route exists and has an annotation and the ingress has no annotations?

Miciah · 2020-11-21T05:28:49Z

pkg/route/ingress/ingress.go

@@ -15,7 +16,6 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/types"


Please put this back.

pkg/route/ingress/ingress.go

Miciah · 2020-11-21T05:31:28Z

pkg/route/ingress/ingress.go

@@ -565,6 +572,9 @@ func newRouteForIngress(

 func preserveRouteAttributesFromExisting(r, existing *routev1.Route) {
 	r.Name = existing.Name
+	if r.Annotations != nil && existing.Annotations != nil {


I think if r.Annotations != nil && existing.Annotations != nil should be deleted for the same reason I mentioned on the patch logic.

The unit test cases fails after making these changes ->

[miheer@miheer openshift-controller-manager]$ GO111MODULE=on go test -v -run TestController_sync ./pkg/route/ingress/
=== RUN TestController_sync
=== RUN TestController_sync/no_changes
=== RUN TestController_sync/sync_namespace_-no_ingress
=== RUN TestController_sync/sync_namespace-two_ingress
=== RUN TestController_sync/ignores_incomplete_ingress-no_host
=== RUN TestController_sync/ignores_incomplete_ingress-no_service
=== RUN TestController_sync/ignores_incomplete_ingress-no_paths
=== RUN TestController_sync/ignores_incomplete_ingress-service_does_not_exist
=== RUN TestController_sync/create_route
=== RUN TestController_sync/create_route-_targetPort_string,service_port_with_name
=== RUN TestController_sync/create_route-blocked_by_expectation
=== RUN TestController_sync/update_route
=== RUN TestController_sync/no-op
=== RUN TestController_sync/no-op-ignore_partially_owned_resource
=== RUN TestController_sync/update_ingress_with_missing_secret_ref
=== RUN TestController_sync/update_ingress_to_not_reference_secret
=== RUN TestController_sync/update_route-tls_config_missing
=== RUN TestController_sync/update_route-termination_policy_changed_to_passthrough
=== RUN TestController_sync/update_route-termination_policy_changed_to_reencrypt
=== RUN TestController_sync/update_route-_termination_policy_changed_to_reencrypt_and_no_tls_secret
=== RUN TestController_sync/termination_policy_on_ingress_invalid,nothing_happens
=== RUN TestController_sync/termination_policy_on_ingress_invalid,disables_tls
=== RUN TestController_sync/Empty_tlsconfig_enables_edge_termination_without_explicit_cert
=== RUN TestController_sync/update_route-secret_values_changed
=== RUN TestController_sync/no-op-has_TLS
=== RUN TestController_sync/no-op-has_secret_with_empty_keys
=== RUN TestController_sync/no-op-termination_policy_has_been_changed_by_the_user
=== RUN TestController_sync/update_route-router_admitted_route
=== RUN TestController_sync/update_route-second_router_admitted_route
=== RUN TestController_sync/no-op-ingress_status_already_updated
=== RUN TestController_sync/no-op-router_rejected_route
=== RUN TestController_sync/delete_route_when_referenced_secret_is_not_TLS
=== RUN TestController_sync/delete_route_when_referenced_secret_is_not_valid
=== RUN TestController_sync/ignore_route_when_parent_ingress_no_longer_exists(gc_will_handle)
=== RUN TestController_sync/update_route-termination_policy_changed_to_passthrough_and_timeout_set
--- FAIL: TestController_sync (0.00s)
--- PASS: TestController_sync/no_changes (0.00s)
--- PASS: TestController_sync/sync_namespace-no_ingress (0.00s)
--- PASS: TestController_sync/sync_namespace-two_ingress (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_host (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_service (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_paths (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-service_does_not_exist (0.00s)
--- PASS: TestController_sync/create_route (0.00s)
--- PASS: TestController_sync/create_route-_targetPort_string,service_port_with_name (0.00s)
--- PASS: TestController_sync/create_route-blocked_by_expectation (0.00s)
--- PASS: TestController_sync/update_route (0.00s)
--- PASS: TestController_sync/no-op (0.00s)
--- PASS: TestController_sync/no-op-ignore_partially_owned_resource (0.00s)
--- PASS: TestController_sync/update_ingress_with_missing_secret_ref (0.00s)
--- PASS: TestController_sync/update_ingress_to_not_reference_secret (0.00s)
--- PASS: TestController_sync/update_route-tls_config_missing (0.00s)
--- FAIL: TestController_sync/update_route-termination_policy_changed_to_passthrough (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- FAIL: TestController_sync/update_route-termination_policy_changed_to_reencrypt (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- FAIL: TestController_sync/update_route-_termination_policy_changed_to_reencrypt_and_no_tls_secret (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- FAIL: TestController_sync/termination_policy_on_ingress_invalid,nothing_happens (0.00s)
ingress_test.go:2623: Controller.sync() unexpected actions: []testing.Action{testing.PatchActionImpl{ActionImpl:testing.ActionImpl{Namespace:"test", Verb:"patch", Resource:schema.GroupVersionResource{Group:"route.openshift.io", Version:"v1", Resource:"routes"}, Subresource:""}, Name:"1-abcdef", PatchType:"application/json-patch+json", Patch:[]uint8{0x5b, 0x7b, 0x22, 0x6f, 0x70, 0x22, 0x3a, 0x22, 0x72, 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0x22, 0x2c, 0x22, 0x70, 0x61, 0x74, 0x68, 0x22, 0x3a, 0x22, 0x2f, 0x73, 0x70, 0x65, 0x63, 0x22, 0x2c, 0x22, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x7b, 0x22, 0x68, 0x6f, 0x73, 0x74, 0x22, 0x3a, 0x22, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x63, 0x6f, 0x6d, 0x22, 0x2c, 0x22, 0x70, 0x61, 0x74, 0x68, 0x22, 0x3a, 0x22, 0x2f, 0x22, 0x2c, 0x22, 0x74, 0x6f, 0x22, 0x3a, 0x7b, 0x22, 0x6b, 0x69, 0x6e, 0x64, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x6e, 0x61, 0x6d, 0x65, 0x22, 0x3a, 0x22, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x2d, 0x31, 0x22, 0x2c, 0x22, 0x77, 0x65, 0x69, 0x67, 0x68, 0x74, 0x22, 0x3a, 0x6e, 0x75, 0x6c, 0x6c, 0x7d, 0x2c, 0x22, 0x70, 0x6f, 0x72, 0x74, 0x22, 0x3a, 0x7b, 0x22, 0x74, 0x61, 0x72, 0x67, 0x65, 0x74, 0x50, 0x6f, 0x72, 0x74, 0x22, 0x3a, 0x22, 0x68, 0x74, 0x74, 0x70, 0x22, 0x7d, 0x2c, 0x22, 0x74, 0x6c, 0x73, 0x22, 0x3a, 0x7b, 0x22, 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x22, 0x3a, 0x22, 0x65, 0x64, 0x67, 0x65, 0x22, 0x2c, 0x22, 0x63, 0x65, 0x72, 0x74, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x65, 0x22, 0x3a, 0x22, 0x63, 0x65, 0x72, 0x74, 0x22, 0x2c, 0x22, 0x6b, 0x65, 0x79, 0x22, 0x3a, 0x22, 0x6b, 0x65, 0x79, 0x22, 0x2c, 0x22, 0x69, 0x6e, 0x73, 0x65, 0x63, 0x75, 0x72, 0x65, 0x45, 0x64, 0x67, 0x65, 0x54, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x22, 0x3a, 0x22, 0x52, 0x65, 0x64, 0x69, 0x72, 0x65, 0x63, 0x74, 0x22, 0x7d, 0x7d, 0x7d, 0x2c, 0x7b, 0x22, 0x6f, 0x70, 0x22, 0x3a, 0x22, 0x72, 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0x22, 0x2c, 0x22, 0x70, 0x61, 0x74, 0x68, 0x22, 0x3a, 0x22, 0x2f, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x2f, 0x61, 0x6e, 0x6e, 0x6f, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x22, 0x2c, 0x22, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x6e, 0x75, 0x6c, 0x6c, 0x7d, 0x5d}}}
--- FAIL: TestController_sync/termination_policy_on_ingress_invalid,disables_tls (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- PASS: TestController_sync/Empty_tlsconfig_enables_edge_termination_without_explicit_cert (0.00s)
--- PASS: TestController_sync/update_route-secret_values_changed (0.00s)
--- PASS: TestController_sync/no-op-has_TLS (0.00s)
--- PASS: TestController_sync/no-op-has_secret_with_empty_keys (0.00s)
--- PASS: TestController_sync/no-op-termination_policy_has_been_changed_by_the_user (0.00s)
--- PASS: TestController_sync/update_route-router_admitted_route (0.00s)
--- PASS: TestController_sync/update_route-second_router_admitted_route (0.00s)
--- PASS: TestController_sync/no-op-ingress_status_already_updated (0.00s)
--- PASS: TestController_sync/no-op-router_rejected_route (0.00s)
--- PASS: TestController_sync/delete_route_when_referenced_secret_is_not_TLS (0.00s)
--- PASS: TestController_sync/delete_route_when_referenced_secret_is_not_valid (0.00s)
--- PASS: TestController_sync/ignore_route_when_parent_ingress_no_longer_exists(gc_will_handle) (0.00s)
--- FAIL: TestController_sync/update_route-_termination_policy_changed_to_passthrough_and_timeout_set (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
FAIL
FAIL github.com/openshift/openshift-controller-manager/pkg/route/ingress 0.019s
FAIL

Ah, you're right— the assignment r.Annotations = existing.Annotations overrides the annotations that were inherited from the Ingress with whatever annotations are set on the Route. Why did you add the assignment?

Miciah · 2020-11-21T05:33:35Z

pkg/route/ingress/ingress.go

 		len(route.Spec.AlternateBackends) == 0
-	if !match {
-		return false
+	if route.Annotations != nil && ingress.Annotations != nil {
+		match_annotations = reflect.DeepEqual(route.Annotations, ingress.Annotations)
+		if !match && !match_annotations {
+			return false
+		} else if !match {
+			return false
+		}


Suggested change

len(route.Spec.AlternateBackends) == 0

if !match {

return false

if route.Annotations != nil && ingress.Annotations != nil {

match_annotations = reflect.DeepEqual(route.Annotations, ingress.Annotations)

if !match && !match_annotations {

return false

} else if !match {

return false

}

len(route.Spec.AlternateBackends) == 0 &&

reflect.DeepEqual(route.Annotations, ingress.Annotations)

The unit test cases fails after making these changes ->

[miheer@miheer openshift-controller-manager]$ GO111MODULE=on go test -v -run TestController_sync ./pkg/route/ingress/
=== RUN TestController_sync
=== RUN TestController_sync/no_changes
=== RUN TestController_sync/sync_namespace_-no_ingress
=== RUN TestController_sync/sync_namespace-two_ingress
=== RUN TestController_sync/ignores_incomplete_ingress-no_host
=== RUN TestController_sync/ignores_incomplete_ingress-no_service
=== RUN TestController_sync/ignores_incomplete_ingress-no_paths
=== RUN TestController_sync/ignores_incomplete_ingress-service_does_not_exist
=== RUN TestController_sync/create_route
=== RUN TestController_sync/create_route-_targetPort_string,service_port_with_name
=== RUN TestController_sync/create_route-blocked_by_expectation
=== RUN TestController_sync/update_route
=== RUN TestController_sync/no-op
=== RUN TestController_sync/no-op-ignore_partially_owned_resource
=== RUN TestController_sync/update_ingress_with_missing_secret_ref
=== RUN TestController_sync/update_ingress_to_not_reference_secret
=== RUN TestController_sync/update_route-tls_config_missing
=== RUN TestController_sync/update_route-termination_policy_changed_to_passthrough
=== RUN TestController_sync/update_route-termination_policy_changed_to_reencrypt
=== RUN TestController_sync/update_route-_termination_policy_changed_to_reencrypt_and_no_tls_secret
=== RUN TestController_sync/termination_policy_on_ingress_invalid,nothing_happens
=== RUN TestController_sync/termination_policy_on_ingress_invalid,disables_tls
=== RUN TestController_sync/Empty_tlsconfig_enables_edge_termination_without_explicit_cert
=== RUN TestController_sync/update_route-secret_values_changed
=== RUN TestController_sync/no-op-has_TLS
=== RUN TestController_sync/no-op-has_secret_with_empty_keys
=== RUN TestController_sync/no-op-termination_policy_has_been_changed_by_the_user
=== RUN TestController_sync/update_route-router_admitted_route
=== RUN TestController_sync/update_route-second_router_admitted_route
=== RUN TestController_sync/no-op-ingress_status_already_updated
=== RUN TestController_sync/no-op-router_rejected_route
=== RUN TestController_sync/delete_route_when_referenced_secret_is_not_TLS
=== RUN TestController_sync/delete_route_when_referenced_secret_is_not_valid
=== RUN TestController_sync/ignore_route_when_parent_ingress_no_longer_exists(gc_will_handle)
=== RUN TestController_sync/update_route-termination_policy_changed_to_passthrough_and_timeout_set
--- FAIL: TestController_sync (0.00s)
--- PASS: TestController_sync/no_changes (0.00s)
--- PASS: TestController_sync/sync_namespace-no_ingress (0.00s)
--- PASS: TestController_sync/sync_namespace-two_ingress (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_host (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_service (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-no_paths (0.00s)
--- PASS: TestController_sync/ignores_incomplete_ingress-service_does_not_exist (0.00s)
--- PASS: TestController_sync/create_route (0.00s)
--- PASS: TestController_sync/create_route-_targetPort_string,service_port_with_name (0.00s)
--- PASS: TestController_sync/create_route-blocked_by_expectation (0.00s)
--- PASS: TestController_sync/update_route (0.00s)
--- PASS: TestController_sync/no-op (0.00s)
--- PASS: TestController_sync/no-op-ignore_partially_owned_resource (0.00s)
--- PASS: TestController_sync/update_ingress_with_missing_secret_ref (0.00s)
--- PASS: TestController_sync/update_ingress_to_not_reference_secret (0.00s)
--- PASS: TestController_sync/update_route-tls_config_missing (0.00s)
--- FAIL: TestController_sync/update_route-termination_policy_changed_to_passthrough (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- FAIL: TestController_sync/update_route-termination_policy_changed_to_reencrypt (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- FAIL: TestController_sync/update_route-_termination_policy_changed_to_reencrypt_and_no_tls_secret (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- FAIL: TestController_sync/termination_policy_on_ingress_invalid,nothing_happens (0.00s)
ingress_test.go:2623: Controller.sync() unexpected actions: []testing.Action{testing.PatchActionImpl{ActionImpl:testing.ActionImpl{Namespace:"test", Verb:"patch", Resource:schema.GroupVersionResource{Group:"route.openshift.io", Version:"v1", Resource:"routes"}, Subresource:""}, Name:"1-abcdef", PatchType:"application/json-patch+json", Patch:[]uint8{0x5b, 0x7b, 0x22, 0x6f, 0x70, 0x22, 0x3a, 0x22, 0x72, 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0x22, 0x2c, 0x22, 0x70, 0x61, 0x74, 0x68, 0x22, 0x3a, 0x22, 0x2f, 0x73, 0x70, 0x65, 0x63, 0x22, 0x2c, 0x22, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x7b, 0x22, 0x68, 0x6f, 0x73, 0x74, 0x22, 0x3a, 0x22, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x63, 0x6f, 0x6d, 0x22, 0x2c, 0x22, 0x70, 0x61, 0x74, 0x68, 0x22, 0x3a, 0x22, 0x2f, 0x22, 0x2c, 0x22, 0x74, 0x6f, 0x22, 0x3a, 0x7b, 0x22, 0x6b, 0x69, 0x6e, 0x64, 0x22, 0x3a, 0x22, 0x22, 0x2c, 0x22, 0x6e, 0x61, 0x6d, 0x65, 0x22, 0x3a, 0x22, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x2d, 0x31, 0x22, 0x2c, 0x22, 0x77, 0x65, 0x69, 0x67, 0x68, 0x74, 0x22, 0x3a, 0x6e, 0x75, 0x6c, 0x6c, 0x7d, 0x2c, 0x22, 0x70, 0x6f, 0x72, 0x74, 0x22, 0x3a, 0x7b, 0x22, 0x74, 0x61, 0x72, 0x67, 0x65, 0x74, 0x50, 0x6f, 0x72, 0x74, 0x22, 0x3a, 0x22, 0x68, 0x74, 0x74, 0x70, 0x22, 0x7d, 0x2c, 0x22, 0x74, 0x6c, 0x73, 0x22, 0x3a, 0x7b, 0x22, 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x22, 0x3a, 0x22, 0x65, 0x64, 0x67, 0x65, 0x22, 0x2c, 0x22, 0x63, 0x65, 0x72, 0x74, 0x69, 0x66, 0x69, 0x63, 0x61, 0x74, 0x65, 0x22, 0x3a, 0x22, 0x63, 0x65, 0x72, 0x74, 0x22, 0x2c, 0x22, 0x6b, 0x65, 0x79, 0x22, 0x3a, 0x22, 0x6b, 0x65, 0x79, 0x22, 0x2c, 0x22, 0x69, 0x6e, 0x73, 0x65, 0x63, 0x75, 0x72, 0x65, 0x45, 0x64, 0x67, 0x65, 0x54, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x22, 0x3a, 0x22, 0x52, 0x65, 0x64, 0x69, 0x72, 0x65, 0x63, 0x74, 0x22, 0x7d, 0x7d, 0x7d, 0x2c, 0x7b, 0x22, 0x6f, 0x70, 0x22, 0x3a, 0x22, 0x72, 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0x22, 0x2c, 0x22, 0x70, 0x61, 0x74, 0x68, 0x22, 0x3a, 0x22, 0x2f, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x2f, 0x61, 0x6e, 0x6e, 0x6f, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x22, 0x2c, 0x22, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x6e, 0x75, 0x6c, 0x6c, 0x7d, 0x5d}}}
--- FAIL: TestController_sync/termination_policy_on_ingress_invalid,disables_tls (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
--- PASS: TestController_sync/Empty_tlsconfig_enables_edge_termination_without_explicit_cert (0.00s)
--- PASS: TestController_sync/update_route-secret_values_changed (0.00s)
--- PASS: TestController_sync/no-op-has_TLS (0.00s)
--- PASS: TestController_sync/no-op-has_secret_with_empty_keys (0.00s)
--- PASS: TestController_sync/no-op-termination_policy_has_been_changed_by_the_user (0.00s)
--- PASS: TestController_sync/update_route-router_admitted_route (0.00s)
--- PASS: TestController_sync/update_route-second_router_admitted_route (0.00s)
--- PASS: TestController_sync/no-op-ingress_status_already_updated (0.00s)
--- PASS: TestController_sync/no-op-router_rejected_route (0.00s)
--- PASS: TestController_sync/delete_route_when_referenced_secret_is_not_TLS (0.00s)
--- PASS: TestController_sync/delete_route_when_referenced_secret_is_not_valid (0.00s)
--- PASS: TestController_sync/ignore_route_when_parent_ingress_no_longer_exists(gc_will_handle) (0.00s)
--- FAIL: TestController_sync/update_route-_termination_policy_changed_to_passthrough_and_timeout_set (0.00s)
ingress_test.go:2603: unexpected action[0]: [{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]
FAIL
FAIL github.com/openshift/openshift-controller-manager/pkg/route/ingress 0.019s
FAIL

Now that #149 (comment) is resolved, I think you can fix the "termination policy on ingress invalid, nothing happens" test case as follows:

@@ -1452,6 +1452,9 @@ func TestController_sync(t *testing.T) { Name: "1-abcdef", Namespace: "test", OwnerReferences: []metav1.OwnerReference{{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, + Annotations: map[string]string{ + "route.openshift.io/termination": "Passthrough", + }, }, Spec: routev1.RouteSpec{ Host: "test.com",

And then this logic can be simplified as I suggested before:

match := route.Spec.Host == rule.Host && route.Spec.Path == path.Path && route.Spec.To.Name == path.Backend.ServiceName && route.Spec.WildcardPolicy == routev1.WildcardPolicyNone && len(route.Spec.AlternateBackends) == 0 && reflect.DeepEqual(route.Annotations, ingress.Annotations) if !match { return false }

Miciah · 2020-11-21T05:33:52Z

pkg/route/ingress/ingress_test.go

@@ -2415,6 +2415,79 @@ func TestController_sync(t *testing.T) {
 			},
 			args: queueKey{namespace: "test", name: "1"},
 		},
+


Delete the blank line.

Miciah · 2020-11-21T05:35:07Z

pkg/route/ingress/ingress_test.go

@@ -2526,6 +2599,7 @@ func TestController_sync(t *testing.T) {
 					t.Errorf("unexpected action[%d]: %#v", i, action)
 				}
 				if !reflect.DeepEqual(string(action.GetPatch()), string(tt.wantRoutePatches[i].Patch)) {
+					//if !(string(action.GetPatch()) == string(tt.wantRoutePatches[i].Patch)) {


Please delete this.

openshift-ci-robot · 2020-11-21T05:37:06Z

@miheer: This pull request references Bugzilla bug 1860136, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Miciah · 2020-11-24T03:35:34Z

pkg/route/ingress/ingress.go

@@ -565,6 +572,9 @@ func newRouteForIngress(

 func preserveRouteAttributesFromExisting(r, existing *routev1.Route) {
 	r.Name = existing.Name
+	if r.Annotations != nil && existing.Annotations != nil {


Ah, you're right— the assignment r.Annotations = existing.Annotations overrides the annotations that were inherited from the Ingress with whatever annotations are set on the Route. Why did you add the assignment?

Miciah · 2020-11-24T03:39:12Z

pkg/route/ingress/ingress_test.go

+			name: "update route - termination policy changed to passthrough and timeout set",
+			fields: fields{
+				i: &ingressLister{Items: []*networkingv1beta1.Ingress{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "1",
+							Namespace: "test",
+							Annotations: map[string]string{
+								"route.openshift.io/termination":      "passthrough",
+								"haproxy.router.openshift.io/timeout": "6m",
+							},
+						},
+						Spec: networkingv1beta1.IngressSpec{
+							TLS: []networkingv1beta1.IngressTLS{
+								{Hosts: []string{"test.com"}, SecretName: "secret-1"},
+							},
+							Rules: []networkingv1beta1.IngressRule{
+								{
+									Host: "test.com",
+									IngressRuleValue: networkingv1beta1.IngressRuleValue{
+										HTTP: &networkingv1beta1.HTTPIngressRuleValue{
+											Paths: []networkingv1beta1.HTTPIngressPath{
+												{
+													Path: "/", Backend: networkingv1beta1.IngressBackend{
+														ServiceName: "service-1",
+														ServicePort: intstr.FromString("http"),
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				}},
+				r: &routeLister{Items: []*routev1.Route{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:            "1-abcdef",
+							Namespace:       "test",
+							OwnerReferences: []metav1.OwnerReference{{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}},
+						},


This test case should verify that the annotations are updated even if the route already has annotations (see #149 (comment)):

{ name: "update route - existing annotations are updated", fields: fields{ i: &ingressLister{Items: []*networkingv1beta1.Ingress{ { ObjectMeta: metav1.ObjectMeta{ Name: "1", Namespace: "test", Annotations: map[string]string{ "route.openshift.io/termination": "passthrough", "haproxy.router.openshift.io/timeout": "6m", }, }, Spec: networkingv1beta1.IngressSpec{ TLS: []networkingv1beta1.IngressTLS{ {Hosts: []string{"test.com"}, SecretName: "secret-1"}, }, Rules: []networkingv1beta1.IngressRule{ { Host: "test.com", IngressRuleValue: networkingv1beta1.IngressRuleValue{ HTTP: &networkingv1beta1.HTTPIngressRuleValue{ Paths: []networkingv1beta1.HTTPIngressPath{ { Path: "/", Backend: networkingv1beta1.IngressBackend{ ServiceName: "service-1", ServicePort: intstr.FromString("http"), }, }, }, }, }, }, }, }, }, }}, r: &routeLister{Items: []*routev1.Route{ { ObjectMeta: metav1.ObjectMeta{ Name: "1-abcdef", Namespace: "test", OwnerReferences: []metav1.OwnerReference{{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}}, Annotations: map[string]string{ "route.openshift.io/termination": "passthrough", }, },

miheer · 2020-11-27T02:21:05Z

/test e2e-aws

quarterpin · 2020-11-30T09:53:51Z

/bugzilla cc-qa

openshift-ci-robot · 2020-11-30T09:53:55Z

@quarterpin: This pull request references Bugzilla bug 1860136, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @quarterpin

In response to this:

/bugzilla cc-qa

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

quarterpin · 2020-11-30T10:00:33Z

/lgtm

Verified via pre-merge verification workflow, more reference related to the test can be found in:
https://bugzilla.redhat.com/show_bug.cgi?id=1860136#c9

miheer · 2020-12-04T03:49:02Z

@Miciah PTAL

Miciah · 2020-12-04T04:30:07Z

Did you see #149 (comment)? The following patch works for me, and I think it makes the behavior more correct:

diff --git a/pkg/route/ingress/ingress.go b/pkg/route/ingress/ingress.go
index 6521a7e7..228c89b9 100644
--- a/pkg/route/ingress/ingress.go
+++ b/pkg/route/ingress/ingress.go
@@ -586,19 +586,14 @@ func routeMatchesIngress(
 	secretLister corelisters.SecretLister,
 	serviceLister corelisters.ServiceLister,
 ) bool {
-	var match_annotations bool
 	match := route.Spec.Host == rule.Host &&
 		route.Spec.Path == path.Path &&
 		route.Spec.To.Name == path.Backend.ServiceName &&
 		route.Spec.WildcardPolicy == routev1.WildcardPolicyNone &&
-		len(route.Spec.AlternateBackends) == 0
-	if route.Annotations != nil && ingress.Annotations != nil {
-		match_annotations = reflect.DeepEqual(route.Annotations, ingress.Annotations)
-		if !match && !match_annotations {
-			return false
-		} else if !match {
-			return false
-		}
+		len(route.Spec.AlternateBackends) == 0 &&
+		reflect.DeepEqual(route.Annotations, ingress.Annotations)
+	if !match {
+		return false
 	}
 	targetPort, err := targetPortForService(ingress.Namespace, path, serviceLister)
 	if err != nil {
diff --git a/pkg/route/ingress/ingress_test.go b/pkg/route/ingress/ingress_test.go
index 2355be80..823c57a8 100644
--- a/pkg/route/ingress/ingress_test.go
+++ b/pkg/route/ingress/ingress_test.go
@@ -1452,6 +1452,9 @@ func TestController_sync(t *testing.T) {
 							Name:            "1-abcdef",
 							Namespace:       "test",
 							OwnerReferences: []metav1.OwnerReference{{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}},
+							Annotations: map[string]string{
+								"route.openshift.io/termination": "Passthrough",
+							},
 						},
 						Spec: routev1.RouteSpec{
 							Host: "test.com",

miheer · 2020-12-04T07:42:36Z

Did you see #149 (comment)? The following patch works for me, and I think it makes the behavior more correct:

diff --git a/pkg/route/ingress/ingress.go b/pkg/route/ingress/ingress.go
index 6521a7e7..228c89b9 100644
--- a/pkg/route/ingress/ingress.go
+++ b/pkg/route/ingress/ingress.go
@@ -586,19 +586,14 @@ func routeMatchesIngress(
 	secretLister corelisters.SecretLister,
 	serviceLister corelisters.ServiceLister,
 ) bool {
-	var match_annotations bool
 	match := route.Spec.Host == rule.Host &&
 		route.Spec.Path == path.Path &&
 		route.Spec.To.Name == path.Backend.ServiceName &&
 		route.Spec.WildcardPolicy == routev1.WildcardPolicyNone &&
-		len(route.Spec.AlternateBackends) == 0
-	if route.Annotations != nil && ingress.Annotations != nil {
-		match_annotations = reflect.DeepEqual(route.Annotations, ingress.Annotations)
-		if !match && !match_annotations {
-			return false
-		} else if !match {
-			return false
-		}
+		len(route.Spec.AlternateBackends) == 0 &&
+		reflect.DeepEqual(route.Annotations, ingress.Annotations)
+	if !match {
+		return false
 	}
 	targetPort, err := targetPortForService(ingress.Namespace, path, serviceLister)
 	if err != nil {
diff --git a/pkg/route/ingress/ingress_test.go b/pkg/route/ingress/ingress_test.go
index 2355be80..823c57a8 100644
--- a/pkg/route/ingress/ingress_test.go
+++ b/pkg/route/ingress/ingress_test.go
@@ -1452,6 +1452,9 @@ func TestController_sync(t *testing.T) {
 							Name:            "1-abcdef",
 							Namespace:       "test",
 							OwnerReferences: []metav1.OwnerReference{{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}},
+							Annotations: map[string]string{
+								"route.openshift.io/termination": "Passthrough",
+							},
 						},
 						Spec: routev1.RouteSpec{
 							Host: "test.com",

ACK it will work but don't we need to check annotations ?

Miciah · 2020-12-04T07:47:24Z

ACK it will work but don't we need to check annotations ?

What do you mean? reflect.DeepEqual(route.Annotations, ingress.Annotations) checks annotations, and the change to routeMatchesIngress that I am suggesting would prevent routeMatchesIngress from returning a false positive if exactly one of route.Annotations or ingress.Annotations is nil.

…to exisiting ingress object BZ https://bugzilla.redhat.com/show_bug.cgi?id=1860136

Miciah · 2020-12-07T04:35:06Z

Looks great! It took some iterations to arrive at the end result, but it looks clean and correct. Thanks!
/lgtm

openshift-ci-robot · 2020-12-07T04:35:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah, miheer, quarterpin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/route/OWNERS~~ [Miciah]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2020-12-07T04:38:24Z

@miheer: All pull requests linked via external trackers have merged:

openshift/openshift-controller-manager#149

Bugzilla bug 1860136 has been moved to the MODIFIED state.

In response to this:

Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 18, 2020

openshift-ci-robot requested review from danehans and ironcladlou November 18, 2020 13:43

Miciah reviewed Nov 18, 2020

View reviewed changes

miheer force-pushed the route-timeout branch 9 times, most recently from bb6f0ed to ae39fd4 Compare November 19, 2020 14:10

miheer changed the title ~~[WIP] BZ 1860136: Fix for Annotation was not propagated to the route when changes made to exisiting ingress object~~ BZ 1860136: Fix for Annotation was not propagated to the route when changes made to exisiting ingress object Nov 19, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 19, 2020

Miciah reviewed Nov 21, 2020

View reviewed changes

openshift-ci-robot changed the title ~~BZ 1860136: Fix for Annotation was not propagated to the route when changes made to exisiting ingress object~~ Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object Nov 21, 2020

openshift-ci-robot added the bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. label Nov 21, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Nov 21, 2020

miheer force-pushed the route-timeout branch 2 times, most recently from 9a5b777 to b94c6b7 Compare November 23, 2020 09:02

Miciah reviewed Nov 24, 2020

View reviewed changes

miheer force-pushed the route-timeout branch 3 times, most recently from 533eccd to f0010b4 Compare November 24, 2020 08:23

openshift-ci-robot requested a review from quarterpin November 30, 2020 09:53

openshift-ci-robot assigned quarterpin Nov 30, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2020

Fix for Annotation was not propagated to the route when changes made …

cde189e

…to exisiting ingress object BZ https://bugzilla.redhat.com/show_bug.cgi?id=1860136

miheer force-pushed the route-timeout branch from f0010b4 to cde189e Compare December 7, 2020 01:56

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2020

openshift-ci-robot assigned Miciah Dec 7, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 7, 2020

openshift-merge-robot merged commit ff8eac9 into openshift:master Dec 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object #149

Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object #149

miheer commented Nov 18, 2020

miheer commented Nov 18, 2020

Miciah left a comment

Miciah Nov 18, 2020

Miciah Nov 18, 2020

miheer commented Nov 19, 2020

miheer commented Nov 19, 2020

Miciah left a comment

Miciah Nov 21, 2020

Miciah Nov 21, 2020

miheer Nov 23, 2020

Miciah Nov 24, 2020

Miciah Nov 21, 2020

miheer Nov 23, 2020 •

edited

Miciah Nov 24, 2020

Miciah Nov 21, 2020

Miciah Nov 21, 2020

openshift-ci-robot commented Nov 21, 2020

Miciah Nov 24, 2020

Miciah Nov 24, 2020 •

edited

miheer commented Nov 27, 2020

quarterpin commented Nov 30, 2020

openshift-ci-robot commented Nov 30, 2020

quarterpin commented Nov 30, 2020

miheer commented Dec 4, 2020

Miciah commented Dec 4, 2020 •

edited

miheer commented Dec 4, 2020

Miciah commented Dec 4, 2020

Miciah commented Dec 7, 2020

openshift-ci-robot commented Dec 7, 2020

openshift-ci-robot commented Dec 7, 2020

Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object #149

Bug 1860136: Fix for Annotation was not propagated to the route when changes made to existing ingress object #149

Conversation

miheer commented Nov 18, 2020

miheer commented Nov 18, 2020

Miciah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miheer commented Nov 19, 2020

miheer commented Nov 19, 2020

Miciah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miheer Nov 23, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Nov 21, 2020

Choose a reason for hiding this comment

Miciah Nov 24, 2020 • edited

Choose a reason for hiding this comment

miheer commented Nov 27, 2020

quarterpin commented Nov 30, 2020

openshift-ci-robot commented Nov 30, 2020

quarterpin commented Nov 30, 2020

miheer commented Dec 4, 2020

Miciah commented Dec 4, 2020 • edited

miheer commented Dec 4, 2020

Miciah commented Dec 4, 2020

Miciah commented Dec 7, 2020

openshift-ci-robot commented Dec 7, 2020

openshift-ci-robot commented Dec 7, 2020

miheer Nov 23, 2020 •

edited

Miciah Nov 24, 2020 •

edited

Miciah commented Dec 4, 2020 •

edited