Bug 1842742 - Ingress operator performs spurious updates in response to API's defaulting of nodeport service's session affinity
Summary: Ingress operator performs spurious updates in response to API's defaulting of...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Networking
Version: 4.5
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: 4.6.0
Assignee: Miciah Dashiel Butler Masters
QA Contact: Arvind iyengar
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-06-02 02:33 UTC by Miciah Dashiel Butler Masters
Modified: 2022-08-04 22:27 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: When the ingress operator reconciles an IngressController that uses the "NodePortService" endpoint publishing strategy type, the operator determines whether it needs to update the IngressController's "NodePort"-type Service by constructing an expected Service in memory, getting the actual Service from the API, and comparing the two. The operator leaves some values unspecified in its expected Service. When the API set default values for these unspecified values, the comparison would return a false positive. Consequence: The operator was repeatedly trying to update its "NodePort" Services in response to the API's setting default values. Fix: The operator now considers unspecified values and default values to be equal when comparing "NodePort" Services. Result: The operator should no longer update an IngressController's "NodePort"-type Service in response to API defaulting.
Clone Of:
Environment:
Last Closed: 2020-10-27 16:03:35 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github openshift cluster-ingress-operator pull 407 0 None closed Bug 1842742: nodePortServiceChanged: Fix for session affinity 2021-02-05 19:26:42 UTC
Red Hat Product Errata RHBA-2020:4196 0 None None None 2020-10-27 16:03:36 UTC

Description Miciah Dashiel Butler Masters 2020-06-02 02:33:21 UTC
Description of problem:

When the ingress operator reconciles an ingresscontroller that has the "NodePortService" endpoint publishing strategy type, the operator gets the ingresscontroller's nodeport service (if one exists) from the API to determine whether the operator needs to create or update the service.  If the service does not exist, the operator creates it, with an empty value for the spec.sessionAffinity field.  If the service does exist, the operator compares it with what the operator expects to get in order to determine whether an update is needed for that service.  In this comparison, if the API has set the default value ("None") for the service's spec.sessionAffinity field, the operator detects the update and tries to set the spec.sessionAffinity field back to the empty value.  The operator should not update the service in response to API defaulting.


Steps to Reproduce:

1. Launch a new cluster.

2. Create an ingresscontroller with the "NodePortService" endpoint publishing strategy:

    oc create -f - <<'EOF'
    apiVersion: operator.openshift.io/v1
    kind: IngressController
    metadata:
      name: nodeport
      namespace: openshift-ingress-operator
    spec:
      replicas: 1
      domain: example.com
      endpointPublishingStrategy:
        type: NodePortService
    EOF

3. Check the ingress operator's logs:

    oc -n openshift-ingress-operator logs deploy/ingress-operator -c ingress-operator


Actual results:

The ingress operator's logs have "updated NodePort service" repeated 6 or 7 times.


Expected results:

The ingress operator should ignore the default session affinity value that the API sets and should not log "updated NodePort service" unless the service is updated outside of API defaulting.

Comment 1 Miciah Dashiel Butler Masters 2020-06-18 19:44:34 UTC
A fix has been posted and is awaiting review.  We'll get it merged in the upcoming sprint.

Comment 2 Andrew McDermott 2020-07-09 12:15:41 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 5 Arvind iyengar 2020-07-17 10:33:30 UTC
The PR was merged and was available in "4.6.0-0.nightly-2020-07-16-005008" version as of during testing. With the patched version, it could be seen that the operator now does not try to update the controller nodeport service in response to API default. There is basically no reconciliation happening in regards to this when compared to a non-patched version:
----
$ oc -n openshift-ingress-operator logs deploy/ingress-operator -c ingress-operator | grep -i "updated NodePort service" 
$
$ oc -n openshift-ingress-operator logs deploy/ingress-operator -c ingress-operator | grep -i "NodePort service" 
2020-07-17T10:03:51.095Z	INFO	operator.ingress_controller	ingress/nodeport_service.go:50	created NodePort service	{"service": "&Service{ObjectMeta:{router-nodeport-nodeportapps  openshift-ingress /api/v1/namespaces/openshift-ingress/services/router-nodeport-nodeportapps f5ee3e22-91c5-4142-9fe3-c431d816c707 1204250 0 2020-07-17 10:03:51 +0000 UTC <nil> <nil> map[app:router ingresscontroller.operator.openshift.io/owning-ingresscontroller:nodeportapps router:router-nodeport-nodeportapps] map[] [{apps/v1 Deployment router-nodeportapps e53d1184-2481-43f0-bf2c-c38225cb90af 0xc000766128 <nil>}] []  [{ingress-operator Update v1 2020-07-17 10:03:51 +0000 UTC FieldsV1 FieldsV1{Raw:*[123 34 102 58 109 101 116 97 100 97 116 97 34 58 123 34 102 58 108 97 98 101 108 115 34 58 123 34 46 34 58 123 125 44 34 102 58 97 112 112 34 58 123 125 44 34 102 58 105 110 103 114 101 115 115 99 111 110 116 114 111 108 108 101 114 46 111 112 101 114 97 116 111 114 46 111 112 101 110 115 104 105 102 116 46 105 111 47 111 119 110 105 110 103 45 105 110 103 114 101 115 115 99 111 110 116 114 111 108 108 101 114 34 58 123 125 44 34 102 58 114 111 117 116 101 114 34 58 123 125 125 44 34 102 58 111 119 110 101 114 82 101 102 101 114 101 110 99 101 115 34 58 123 34 46 34 58 123 125 44 34 107 58 123 92 34 117 105 100 92 34 58 92 34 101 53 51 100 49 49 56 52 45 50 52 56 49 45 52 51 102 48 45 98 102 50 99 45 99 51 56 50 50 53 99 98 57 48 97 102 92 34 125 34 58 123 34 46 34 58 123 125 44 34 102 58 97 112 105 86 101 114 115 105 111 110 34 58 123 125 44 34 102 58 99 111 110 116 114 111 108 108 101 114 34 58 123 125 44 34 102 58 107 105 110 100 34 58 123 125 44 34 102 58 110 97 109 101 34 58 123 125 44 34 102 58 117 105 100 34 58 123 125 125 125 125 44 34 102 58 115 112 101 99 34 58 123 34 102 58 101 120 116 101 114 110 97 108 84 114 97 102 102 105 99 80 111 108 105 99 121 34 58 123 125 44 34 102 58 112 111 114 116 115 34 58 123 34 46 34 58 123 125 44 34 107 58 123 92 34 112 111 114 116 92 34 58 56 48 44 92 34 112 114 111 116 111 99 111 108 92 34 58 92 34 84 67 80 92 34 125 34 58 123 34 46 34 58 123 125 44 34 102 58 110 97 109 101 34 58 123 125 44 34 102 58 112 111 114 116 34 58 123 125 44 34 102 58 112 114 111 116 111 99 111 108 34 58 123 125 44 34 102 58 116 97 114 103 101 116 80 111 114 116 34 58 123 125 125 44 34 107 58 123 92 34 112 111 114 116 92 34 58 52 52 51 44 92 34 112 114 111 116 111 99 111 108 92 34 58 92 34 84 67 80 92 34 125 34 58 123 34 46 34 58 123 125 44 34 102 58 110 97 109 101 34 58 123 125 44 34 102 58 112 111 114 116 34 58 123 125 44 34 102 58 112 114 111 116 111 99 111 108 34 58 123 125 44 34 102 58 116 97 114 103 101 116 80 111 114 116 34 58 123 125 125 125 44 34 102 58 115 101 108 101 99 116 111 114 34 58 123 34 46 34 58 123 125 44 34 102 58 105 110 103 114 101 115 115 99 111 110 116 114 111 108 108 101 114 46 111 112 101 114 97 116 111 114 46 111 112 101 110 115 104 105 102 116 46 105 111 47 100 101 112 108 111 121 109 101 110 116 45 105 110 103 114 101 115 115 99 111 110 116 114 111 108 108 101 114 34 58 123 125 125 44 34 102 58 115 101 115 115 105 111 110 65 102 102 105 110 105 116 121 34 58 123 125 44 34 102 58 116 121 112 101 34 58 123 125 125 125],}}]},Spec:ServiceSpec{Ports:[]ServicePort{ServicePort{Name:http,Protocol:TCP,Port:80,TargetPort:{1 0 http},NodePort:31099,AppProtocol:nil,},ServicePort{Name:https,Protocol:TCP,Port:443,TargetPort:{1 0 https},NodePort:30381,AppProtocol:nil,},},Selector:map[string]string{ingresscontroller.operator.openshift.io/deployment-ingresscontroller: nodeportapps,},ClusterIP:172.30.31.129,Type:NodePort,ExternalIPs:[],SessionAffinity:None,LoadBalancerIP:,LoadBalancerSourceRanges:[],ExternalName:,ExternalTrafficPolicy:Local,HealthCheckNodePort:0,PublishNotReadyAddresses:false,SessionAffinityConfig:nil,IPFamily:nil,TopologyKeys:[],},Status:ServiceStatus{LoadBalancer:LoadBalancerStatus{Ingress:[]LoadBalancerIngress{},},},}"}
----

Comment 6 Arvind iyengar 2020-07-17 10:39:18 UTC
Reference from none patched version: 
-------
$ oc get clusterversion
NAME      VERSION   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.5.2     True        False         26h     Cluster version is 4.5.2

$ oc -n openshift-ingress-operator logs deploy/ingress-operator -c ingress-operator | grep -i "updated NodePort service" | wc -l
6
-------

Comment 8 errata-xmlrpc 2020-10-27 16:03:35 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 (OpenShift Container Platform 4.6 GA Images), 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/RHBA-2020:4196


Note You need to log in before you can comment on or make changes to this bug.