Bug 1860136

Summary:	default ingress does not propagate annotations to route object on update
Product:	OpenShift Container Platform	Reporter:	Ben Parees <bparees>
Component:	Networking	Assignee:	Miheer Salunke <misalunk>
Networking sub component:	router	QA Contact:	Arvind iyengar <aiyengar>
Status:	CLOSED ERRATA	Docs Contact:
Severity:	low
Priority:	medium	CC:	amcdermo, aos-bugs, bbennett, hgomes, hongli, misalunk
Version:	4.5	Keywords:	UpcomingSprint
Target Milestone:	---
Target Release:	4.7.0
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2021-02-24 15:13:58 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Ben Parees 2020-07-23 18:57:23 UTC

Description of problem:
We had an existing ingress object that was using the default router ingress controller.  The route was created.

We then added the timeout annotation to the ingress object.  The timeout annotation was not propagated to the route.

Deleting the route and allowing the ingress controller to create it resolved the issue (the newly created route had the annotation) but this is:

1) unexpected
2) undesirable (I shouldn't have to temporarily make my app unavailable to get annotations updated)


Version-Release number of selected component (if applicable):
4.5

How reproducible:
Always

Steps to Reproduce:
1. create an ingress using the default route ingress controller
2. add an annotation to that ingress


Actual results:
the annotation is not applied to the route

Expected results:
the annotation is applied to the route



Additional info:

discussion w/ network edge team here:
https://coreos.slack.com/archives/CCH60A77E/p1595434102167000

Comment 1 Andrew McDermott 2020-07-24 17:38:51 UTC

I’m adding UpcomingSprint, because I was occupied by fixing bugs with
higher priority/severity, developing new features with higher
priority, or developing new features to improve stability at a macro
level. I will revisit this bug next sprint.

Comment 2 mfisher 2020-08-18 20:01:39 UTC

Target reset from 4.6 to 4.7 while investigation is either ongoing or not yet started.  Will be considered for earlier release versions when diagnosed and resolved.

Comment 3 Andrew McDermott 2020-09-10 11:51:57 UTC

I’m adding UpcomingSprint, because I was occupied by fixing bugs with
higher priority/severity, developing new features with higher
priority, or developing new features to improve stability at a macro
level. I will revisit this bug next sprint.

Comment 4 Andrew McDermott 2020-10-02 17:39:33 UTC

Tagging with UpcomingSprint while investigation is either ongoing or
pending. Will be considered for earlier release versions when
diagnosed and resolved.

Comment 8 Miheer Salunke 2020-11-19 14:17:03 UTC

PR - https://github.com/openshift/openshift-controller-manager/pull/149

Units tests 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]$

Comment 9 Arvind iyengar 2020-11-30 09:59:45 UTC

Tested in "4.7.0-0.ci.test-2020-11-30-072352-ci-ln-wkrp2j2" CI image. With this payload, the ingress timeout annotation appears to be getting honored and gets applied on the corresponding route:
-----
Ingress definition used:
$ cat ingress-route-timout-example.yaml
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: ingress-route
  annotations:
    haproxy.router.openshift.io/timeout: 5m <---
spec:
  rules:
  - host: service-unsecure-test1.internalapps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com
    http:
      paths:
      - path: "/"
        pathType: "Prefix"
        backend:
          service:
            name: service-unsecure
            port:
              number: 27017

After creating the ingress object the corresponding route could be seen getting created with the defined timeout annotation :

$ oc create -f ingress-route-timout-example.yaml
ingress.networking.k8s.io/ingress-route created

$ oc get ingress
NAME            CLASS    HOSTS                                                                                       ADDRESS                                                                                                                         PORTS   AGE
ingress-route   <none>   service-unsecure-test1.internalapps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com   apps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com,internalapps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com   80      21s

$ oc get route
NAME                  HOST/PORT                                                                                              PATH   SERVICES           PORT   TERMINATION   WILDCARD
ingress-route-h57f6   service-unsecure-test1.internalapps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com ... 1 more   /      service-unsecure   http                 None

$ oc describe route ingress-route-h57f6
Name:			ingress-route-h57f6
Namespace:		test1
Created:		About a minute ago
Labels:			<none>
Annotations:		haproxy.router.openshift.io/timeout=5m  <-----
Requested Host:		service-unsecure-test1.internalapps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com
			  exposed on router internalapps (host internalapps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com) about a minute ago
			  exposed on router default (host apps.ci-ln-wkrp2j2-002ac.ci.azure.devcluster.openshift.com) about a minute ago
Path:			/
TLS Termination:	<none>
Insecure Policy:	<none>
Endpoint Port:		http

Service:	service-unsecure
Weight:		100 (100%)
Endpoints:	10.128.2.13:8080, 10.129.2.24:8080
-----

Comment 14 errata-xmlrpc 2021-02-24 15:13:58 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: OpenShift Container Platform 4.7.0 security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2020:5633

Comment 15 Red Hat Bugzilla 2023-09-15 01:30:06 UTC

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 365 days