Bug 2054200

Summary: Custom created services in openshift-ingress removed even though the services are not of type LoadBalancer
Product: OpenShift Container Platform Reporter: Simon Reber <sreber>
Component: NetworkingAssignee: Grant Spence <gspence>
Networking sub component: router QA Contact: Arvind iyengar <aiyengar>
Status: CLOSED ERRATA Docs Contact:
Severity: high    
Priority: high CC: aiyengar, aos-bugs, gspence, hongli, mmasters
Version: 4.9   
Target Milestone: ---   
Target Release: 4.11.0   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Cause: The logic in the ingress-operator didn't validate whether a kubernetes service object in the openshift-ingress namespace was actually created/owned by the ingress controller it was attempting to reconcile with. Consequence: The ingress-operator would modify/remove kubernetes services with the same name and namespace regardless of ownership which could cause unexpected behavior. This quite rare because the service has to have a very specific name and it also has to be in the openshift-ingress namespace. Fix: The ingress-operator now checks the ownership of existing kubernetes services it attempts to create/remove and if ownership doesn't match, the ingress-operator throws an error and does not take any action. Result: The ingress-operator won't modify/delete custom kubernetes services with the same name as one that it wants to modify/remove in the openshift-ingress namespace.
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-08-10 10:49:41 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:
Bug Depends On:    
Bug Blocks: 2094051    

Description Simon Reber 2022-02-14 12:24:54 UTC
Description of problem:

Having `IngressController` with `endpointPublishingStrategy` set to `Private` and a `kubernetes` service created with same naming convention `NodePort` in `openshift-ingress` namespace is being removed when the `ingress-operator` is restarted.

$ oc get ingresscontroller -n openshift-ingress-operator example-service-testing -o json
{
    "apiVersion": "operator.openshift.io/v1",
    "kind": "IngressController",
    "metadata": {
        "creationTimestamp": "2022-02-14T10:34:35Z",
        "finalizers": [
            "ingresscontroller.operator.openshift.io/finalizer-ingresscontroller"
        ],
        "generation": 2,
        "name": "example-service-testing",
        "namespace": "openshift-ingress-operator",
        "resourceVersion": "19329705",
        "uid": "ffc9f14d-63ad-43bb-8a56-5e590cda9b38"
    },
    "spec": {
        "clientTLS": {
            "clientCA": {
                "name": ""
            },
            "clientCertificatePolicy": ""
        },
        "domain": "apps.example.com",
        "endpointPublishingStrategy": {
            "type": "Private"
        },
        "httpEmptyRequestsPolicy": "Respond",
        "httpErrorCodePages": {
            "name": ""
        },
        "tuningOptions": {},
        "unsupportedConfigOverrides": null
    },
    "status": {
        "availableReplicas": 2,
        "conditions": [
            {
                "lastTransitionTime": "2022-02-14T10:34:35Z",
                "reason": "Valid",
                "status": "True",
                "type": "Admitted"
            },
            {
                "lastTransitionTime": "2022-02-14T10:34:35Z",
                "status": "True",
                "type": "PodsScheduled"
            },
            {
                "lastTransitionTime": "2022-02-14T10:35:10Z",
                "message": "The deployment has Available status condition set to True",
                "reason": "DeploymentAvailable",
                "status": "True",
                "type": "DeploymentAvailable"
            },
            {
                "lastTransitionTime": "2022-02-14T10:35:10Z",
                "message": "Minimum replicas requirement is met",
                "reason": "DeploymentMinimumReplicasMet",
                "status": "True",
                "type": "DeploymentReplicasMinAvailable"
            },
            {
                "lastTransitionTime": "2022-02-14T10:35:10Z",
                "message": "All replicas are available",
                "reason": "DeploymentReplicasAvailable",
                "status": "True",
                "type": "DeploymentReplicasAllAvailable"
            },
            {
                "lastTransitionTime": "2022-02-14T10:34:35Z",
                "message": "The configured endpoint publishing strategy does not include a managed load balancer",
                "reason": "EndpointPublishingStrategyExcludesManagedLoadBalancer",
                "status": "False",
                "type": "LoadBalancerManaged"
            },
            {
                "lastTransitionTime": "2022-02-14T10:34:35Z",
                "message": "The endpoint publishing strategy doesn't support DNS management.",
                "reason": "UnsupportedEndpointPublishingStrategy",
                "status": "False",
                "type": "DNSManaged"
            },
            {
                "lastTransitionTime": "2022-02-14T10:35:10Z",
                "status": "True",
                "type": "Available"
            },
            {
                "lastTransitionTime": "2022-02-14T10:35:10Z",
                "status": "False",
                "type": "Degraded"
            }
        ],
        "domain": "apps.example.com",
        "endpointPublishingStrategy": {
            "type": "Private"
        },
        "observedGeneration": 2,
        "selector": "ingresscontroller.operator.openshift.io/deployment-ingresscontroller=example-service-testing",
        "tlsProfile": {
            "ciphers": [
                "ECDHE-ECDSA-AES128-GCM-SHA256",
                "ECDHE-RSA-AES128-GCM-SHA256",
                "ECDHE-ECDSA-AES256-GCM-SHA384",
                "ECDHE-RSA-AES256-GCM-SHA384",
                "ECDHE-ECDSA-CHACHA20-POLY1305",
                "ECDHE-RSA-CHACHA20-POLY1305",
                "DHE-RSA-AES128-GCM-SHA256",
                "DHE-RSA-AES256-GCM-SHA384",
                "TLS_AES_128_GCM_SHA256",
                "TLS_AES_256_GCM_SHA384",
                "TLS_CHACHA20_POLY1305_SHA256"
            ],
            "minTLSVersion": "VersionTLS12"
        }
    }
}

$ oc get svc
NAME                                      TYPE           CLUSTER-IP       EXTERNAL-IP                                                               PORT(S)                                     AGE
router-default                            LoadBalancer   172.30.242.215   a777bc4ce4da740d99abdaa899bf8e88-1599963277.us-west-1.elb.amazonaws.com   80:30779/TCP,443:31713/TCP                  13d
router-internal-default                   ClusterIP      172.30.233.135   <none>                                                                    80/TCP,443/TCP,1936/TCP                     13d
router-internal-example-service-testing   ClusterIP      172.30.86.100    <none>                                                                    80/TCP,443/TCP,1936/TCP                     87m

After `IngressController` creation, it all looks as expected and for the Private `IngressController` we can see `router-internal-example-service-testing` Service.

$ oc create svc nodeport router-example-service-testing --tcp=80
service/router-example-service-testing created

$ oc get svc
NAME                                      TYPE           CLUSTER-IP       EXTERNAL-IP                                                               PORT(S)                                     AGE
router-default                            LoadBalancer   172.30.242.215   a777bc4ce4da740d99abdaa899bf8e88-1599963277.us-west-1.elb.amazonaws.com   80:30779/TCP,443:31713/TCP                  13d
router-internal-default                   ClusterIP      172.30.233.135   <none>                                                                    80/TCP,443/TCP,1936/TCP                     13d
router-internal-example-service-testing   ClusterIP      172.30.86.100    <none>                                                                    80/TCP,443/TCP,1936/TCP                     88m
router-example-service-testing            NodePort       172.30.2.39      <none>                                                                    80:31874/TCP                                3s

Now we are creating a `kubernetes` service of type NodePort with the same naming scheme like the one created by the `IngressController`. So far so good and also no impact or similar with regards to functionality.

$ oc get pod -n openshift-ingress-operator
NAME                                READY   STATUS    RESTARTS   AGE
ingress-operator-7d56fd784c-plwpj   2/2     Running   0          78m

$ oc delete pod ingress-operator-7d56fd784c-plwpj -n openshift-ingress-operator
pod "ingress-operator-7d56fd784c-plwpj" deleted

$ oc get svc
NAME                                      TYPE           CLUSTER-IP       EXTERNAL-IP                                                               PORT(S)                                     AGE
router-default                            LoadBalancer   172.30.242.215   a777bc4ce4da740d99abdaa899bf8e88-1599963277.us-west-1.elb.amazonaws.com   80:30779/TCP,443:31713/TCP                  13d
router-internal-default                   ClusterIP      172.30.233.135   <none>                                                                    80/TCP,443/TCP,1936/TCP                     13d
router-internal-example-service-testing   ClusterIP      172.30.86.100    <none>                                                                    80/TCP,443/TCP,1936/TCP                     88m
router-example-service-testing            NodePort       172.30.2.39      <none>                                                                    80:31874/TCP                                53s

$ oc get svc
NAME                                      TYPE           CLUSTER-IP       EXTERNAL-IP                                                               PORT(S)                                     AGE
router-default                            LoadBalancer   172.30.242.215   a777bc4ce4da740d99abdaa899bf8e88-1599963277.us-west-1.elb.amazonaws.com   80:30779/TCP,443:31713/TCP                  13d
router-internal-default                   ClusterIP      172.30.233.135   <none>                                                                    80/TCP,443/TCP,1936/TCP                     13d
router-internal-example-service-testing   ClusterIP      172.30.86.100    <none>                                                                    80/TCP,443/TCP,1936/TCP                     89m


$ oc logs ingress-operator-7d56fd784c-7g48r -n openshift-ingress-operator -c ingress-operator
2022-02-14T12:03:26.624Z	INFO	operator.main	ingress-operator/start.go:63	using operator namespace	{"namespace": "openshift-ingress-operator"}
I0214 12:03:27.675884       1 request.go:668] Waited for 1.02063447s due to client-side throttling, not priority and fairness, request: GET:https://172.30.0.1:443/apis/apps.openshift.io/v1?timeout=32s
2022-02-14T12:03:29.284Z	INFO	operator.main	ingress-operator/start.go:63	registering Prometheus metrics for canary_controller
2022-02-14T12:03:29.284Z	INFO	operator.main	ingress-operator/start.go:63	registering Prometheus metrics for ingress_controller
[...]
2022-02-14T12:03:33.119Z	INFO	operator.dns	dns/controller.go:535	using region from operator config	{"region name": "us-west-1"}
2022-02-14T12:03:33.417Z	INFO	operator.ingress_controller	controller/controller.go:298	reconciling	{"request": "openshift-ingress-operator/example-service-testing"}
2022-02-14T12:03:33.509Z	INFO	operator.ingress_controller	ingress/load_balancer_service.go:190	deleted load balancer service	{"namespace": "openshift-ingress", "name": "router-example-service-testing"}
[...]

When restarting the `ingress-operator` pod we can see that shortly after, the manual created `kubernetes` service of type NodePort is being removed. Looking through the code it looks related to https://bugzilla.redhat.com/show_bug.cgi?id=1914127 but that should only target/focus on `kubernetes` Service of type Loadbalancer. But we can clearly see that this is happening for all `kubernetes` Service type if they are matching the pre-defined `IngressController` naming scheme.

As this is not expected and also the `kubernetes` Services don't have any owner reference to the `IngressController` created services, it's unexpected that does are being removed and thus this should be fixed.

OpenShift release version:

 - OpenShift Container Platform 4.9.15

Cluster Platform:

 - AWS but likely on other platform as well

How reproducible:

 - Always 

Steps to Reproduce (in detail):
1. See the steps in the problem description

Actual results:

`kubernetes` services of any type and without owner reference to the `IngressController` are being removed by the `IngressController` if they have a specific naming scheme.

Expected results:

`kubernetes` services without `IngressController` reference should never be touched/modified/removed by the same as they may be required for 3rd party integration or similar.


Impact of the problem:

3rd party implementation broken after updating to OpenShift Container Platform 4.8 as some helper services were removed unexpected.

Additional info:

Check https://bugzilla.redhat.com/show_bug.cgi?id=1914127 as this seems the change that introduced that behavior. Although this seems specific for `kubernetes` type LoadBalancer and we are therefore wondering why other services are in scope as well.

Comment 1 Miciah Dashiel Butler Masters 2022-02-14 15:56:20 UTC
Setting blocker- as this behavior is in a shipped release and so the BZ shouldn't block 4.10.0 GA or z-stream releases.  

The issue is that the operator does not check the owner reference of resources in the operand namespace and relies on the name of the service alone to determine whether or not it is managed by the operator.  As a workaround, use a name that does not overlap with the operator's naming scheme.  

The resolve this issue, we can add checks on owner references.  Then the operator could do one or more of the following:

* Set the Upgradeable=False status condition on the ingress clusteroperator when the operator detects a conflict (which would annoy users and require a potentially disruptive configuration change to unblock upgrades).  

* Raise an alert (which would annoy users).  

* Silently ignore the conflict (which would confuse users).  

Do you have other suggestions?

Comment 2 Simon Reber 2022-02-14 16:14:47 UTC
(In reply to Miciah Dashiel Butler Masters from comment #1)
> Setting blocker- as this behavior is in a shipped release and so the BZ
> shouldn't block 4.10.0 GA or z-stream releases.
Yes, definitely not as this is out there for some time and rather specific.
 
> The issue is that the operator does not check the owner reference of
> resources in the operand namespace and relies on the name of the service
> alone to determine whether or not it is managed by the operator.  As a
> workaround, use a name that does not overlap with the operator's naming
> scheme.
Yes, we already applied that work-around and are using the same moving forward. But we want to raise this as others may be affected and potentially see a change to improve and potentially have a owner reference in the managed services to prevent the removal of non managed services from happening.
 
> The resolve this issue, we can add checks on owner references.  Then the
> operator could do one or more of the following:
> 
> * Set the Upgradeable=False status condition on the ingress clusteroperator
> when the operator detects a conflict (which would annoy users and require a
> potentially disruptive configuration change to unblock upgrades).  
> 
> * Raise an alert (which would annoy users).  
> 
> * Silently ignore the conflict (which would confuse users).  
> 
> Do you have other suggestions?
To be honest, validating the owner reference of a service and only modifying/removing it when it's managed by the Operator should be enough. Services that are not managed by the IngressOperator should not be touched or modified. And if one custom service is conflicting with a Ingress Operator managed service, we should see it already in the Operator or service state as one or the other may not work as expected,

Comment 3 Miciah Dashiel Butler Masters 2022-02-15 23:39:13 UTC
(In reply to Simon Reber from comment #2)
[...]
> To be honest, validating the owner reference of a service and only
> modifying/removing it when it's managed by the Operator should be enough.
> Services that are not managed by the IngressOperator should not be touched
> or modified. And if one custom service is conflicting with a Ingress
> Operator managed service, we should see it already in the Operator or
> service state as one or the other may not work as expected,

For the example in comment 0 of an ingresscontroller with `spec.endpointPublishingStrategy.type: Private`, I would agree, it isn't very important to tell the user that the user-created service has the same name as the service that the operator *would* create if the ingresscontroller had `spec.endpointPublishingStrategy.type: LoadBalancerService`; in this case, it would suffice for the operator simply not to delete the user-created service.  However, if the user had created the service with that name and then created an ingresscontroller with `spec.endpointPublishingStrategy.type: LoadBalancerService`, the operator would be unable to create the service that the requested by specifying `spec.endpointPublishingStrategy.type: LoadBalancerService`, and in this case, it would make sense to warn the user somehow.

Comment 4 Simon Reber 2022-02-17 08:51:34 UTC
(In reply to Miciah Dashiel Butler Masters from comment #3)
> For the example in comment 0 of an ingresscontroller with
> `spec.endpointPublishingStrategy.type: Private`, I would agree, it isn't
> very important to tell the user that the user-created service has the same
> name as the service that the operator *would* create if the
> ingresscontroller had `spec.endpointPublishingStrategy.type:
> LoadBalancerService`; in this case, it would suffice for the operator simply
> not to delete the user-created service.  However, if the user had created
> the service with that name and then created an ingresscontroller with
> `spec.endpointPublishingStrategy.type: LoadBalancerService`, the operator
> would be unable to create the service that the requested by specifying
> `spec.endpointPublishingStrategy.type: LoadBalancerService`, and in this
> case, it would make sense to warn the user somehow.
It definitely would make sense to warn the user in such a case. I'm just wondering whether the `IngressController` respectively the `IngressOperator` would not already today report Degraded state when this service can be created? As generally the `IngressController` created would not become ready and thus the overall situation would be expected to be degraded. Maybe the condition would not immediately highlight the root cause and this is something we may be able to tackle, but I would expect that we already see a failure if the service can't be created (we can also test to see what is happening).

Comment 5 Grant Spence 2022-02-22 21:47:27 UTC
NOTE FOR QE:

Slight correction to the reproducer. Ensure you run the following command as the first step of the reproducer:
oc project openshift-ingress

Or add to the commandline for ALL oc svc command:
"-n openshift-ingress"

Otherwise this reproducer will always succeed regardless of the fix.

Comment 6 Arvind iyengar 2022-04-20 08:36:33 UTC
Verified in the latest 4.11 ci image with the patch. It is observed that the Kubernetes service without IngressController reference continues to remain available with the ingresscontroller operator pod is restarted. 
------
oc -n openshift-ingress-operator get ingresscontroller
NAME                      AGE
default                   146m
example-service-testing   5m45s  <------


apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
  creationTimestamp: "2022-04-20T08:16:06Z"
  finalizers:
  - ingresscontroller.operator.openshift.io/finalizer-ingresscontroller
  generation: 2
  name: example-service-testing
  namespace: openshift-ingress-operator
  resourceVersion: "72655"
  uid: 5b75d33e-9e6a-44e5-967c-c1e98b048a4b
spec:
  clientTLS:
    clientCA:
      name: ""
    clientCertificatePolicy: ""
  domain: example-service-testing.ci-ln-q3s4bjb-72292.origin-ci-int-gce.dev.rhcloud.com
  endpointPublishingStrategy:
    type: Private <-----


oc -n openshift-ingress get all            
NAME                                                  READY   STATUS    RESTARTS   AGE
pod/router-default-84977c6bdd-r4xx7                   1/1     Running   0          142m
pod/router-default-84977c6bdd-xtwnl                   1/1     Running   0          142m
pod/router-example-service-testing-5f7c96f79d-897zs   2/2     Running   0          102s

NAME                                              TYPE           CLUSTER-IP       EXTERNAL-IP     PORT(S)                      AGE
service/router-default                            LoadBalancer   172.30.232.26    34.68.177.226   80:31409/TCP,443:30420/TCP   142m
service/router-internal-default                   ClusterIP      172.30.215.200   <none>          80/TCP,443/TCP,1936/TCP      142m
service/router-internal-example-service-testing   ClusterIP      172.30.5.51      <none>          80/TCP,443/TCP,1936/TCP      102s


oc create svc nodeport router-example-service-testing --tcp=80 -n openshift-ingress
service/router-example-service-testing created


oc -n openshift-ingress get all             
NAME                                                  READY   STATUS    RESTARTS   AGE
pod/router-default-84977c6bdd-r4xx7                   1/1     Running   0          143m
pod/router-default-84977c6bdd-xtwnl                   1/1     Running   0          143m
pod/router-example-service-testing-5f7c96f79d-897zs   2/2     Running   0          2m47s

NAME                                              TYPE           CLUSTER-IP       EXTERNAL-IP     PORT(S)                      AGE
service/router-default                            LoadBalancer   172.30.232.26    34.68.177.226   80:31409/TCP,443:30420/TCP   143m
service/router-example-service-testing            NodePort       172.30.52.201    <none>          80:32205/TCP                 5s
service/router-internal-default                   ClusterIP      172.30.215.200   <none>          80/TCP,443/TCP,1936/TCP      143m
service/router-internal-example-service-testing   ClusterIP      172.30.5.51      <none>          80/TCP,443/TCP,1936/TCP      2m48s


Delete the operator pod:
oc -n openshift-ingress-operator get all  
NAME                                    READY   STATUS    RESTARTS       AGE
pod/ingress-operator-575f59d746-sgts7   2/2     Running   2 (140m ago)   151m

oc -n openshift-ingress-operator delete pod/ingress-operator-575f59d746-sgts7                          
pod "ingress-operator-575f59d746-sgts7" deleted


oc -n openshift-ingress get all            
pod/router-default-84977c6bdd-r4xx7                   1/1     Running   0          144m
pod/router-default-84977c6bdd-xtwnl                   1/1     Running   0          144m
pod/router-example-service-testing-5f7c96f79d-897zs   2/2     Running   0          4m28s


NAME                                              TYPE           CLUSTER-IP       EXTERNAL-IP     PORT(S)                      AGE
service/router-default                            LoadBalancer   172.30.232.26    34.68.177.226   80:31409/TCP,443:30420/TCP   144m
service/router-example-service-testing            NodePort       172.30.52.201    <none>          80:32205/TCP                 105s
service/router-internal-default                   ClusterIP      172.30.215.200   <none>          80/TCP,443/TCP,1936/TCP      144m
service/router-internal-example-service-testing   ClusterIP      172.30.5.51      <none>          80/TCP,443/TCP,1936/TCP      4m28s
------

Comment 14 errata-xmlrpc 2022-08-10 10:49:41 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 (Important: OpenShift Container Platform 4.11.0 bug fix and security 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-2022:5069