Bug 1499413

Summary: Should remove invalid ServiceClass immediately after update ServiceBroker
Product: OpenShift Container Platform Reporter: Zhang Cheng <chezhang>
Component: Service BrokerAssignee: Jeff Peeler <jpeeler>
Status: CLOSED ERRATA QA Contact: Zhang Cheng <chezhang>
Severity: medium Docs Contact:
Priority: high    
Version: 3.7.0CC: aos-bugs, eparis, jmatthew, pmorie, wsun, xtian
Target Milestone: ---   
Target Release: 3.9.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: No Doc Update
Doc Text:
undefined
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-12-13 19:26:48 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:

Comment 1 John Matthews 2017-10-11 11:34:06 UTC
Paul,

I think this BZ is referring to Service Catalog functionality.

Comment 2 Paul Morie 2017-10-18 15:05:43 UTC
This has been partially addressed and the partial fix is present in https://github.com/openshift/origin/pull/16908

This sets a status.removedFromCatalog field is set when a service or plan is removed from the catalog.  A user will not be able to create new instances of a service or plan with this field set.

Comment 3 Paul Morie 2017-10-25 18:14:28 UTC
Fix is present in https://github.com/openshift/origin/pull/17027

Comment 6 Paul Morie 2017-10-30 14:45:03 UTC
This will be delivered into origin with https://github.com/openshift/origin/pull/17075

For the record, when a service class/plan is removed from the broker's payload, they won't necessarily be immediately removed from the catalog.  Instead, a status field saying that the service class/plan has been removed is set.  A control loop is always running looking for service classes/plans with this field set.  These control loops delete the resource when there are no instances left on a particular removed service class or plan.

So, if service class X is removed from the broker's catalog, and there are still two instances left on this service class, the service class resource won't be deleted until those two instances are deprovisioned.

Comment 8 Zhang Cheng 2017-11-03 14:51:28 UTC
Paul, I did below trying with latest images v3.7.0-0.189.0.0, but still cannot get the expect result.

[Scenario 1]: 
As the original description of this bug, I want to know how to deal with the invalid clusterserviceclass while clusterservicebroker change to a invalid spec.url. In my testing, those invalid clusterserviceclass were not deleted and removedFromBrokerCatalog always "false".

Steps:
1. Deploy svc-catalog & asb env by ansible-installer.
[root@qe-chezhang-master-etcd-1 ~]# oc get clusterserviceclass | grep -v NAME | wc -l
17
[root@qe-chezhang-master-etcd-1 ~]# oc describe clusterserviceclass | grep "Removed"
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false

2. Update clusterservicebroker spec.url to a invalid url
url: https://asb.openshift-ansible-service-broker.svc:1338/ansible-service-broker
change to -->  url: https://invalid.asb.openshift-ansible-service-broker.svc:1338/ansible-service-broker

3. Wait about 15mins, those invalid clusterserviceclass were not deleted and removedFromBrokerCatalog always "false"
[root@qe-chezhang-master-etcd-1 ~]# oc get clusterserviceclass | grep -v NAME | wc -l
17
[root@qe-chezhang-master-etcd-1 ~]# oc describe clusterserviceclass | grep "Removed"
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false


[Scenario 2]: 
Follow your comment 6, I used my personal registry and removed some apb image to see invalid clusterserviceclass and removedFromBrokerCatalog if will be change. 

Steps:
1. Check original clusterserviceclass state
[root@qe-chezhang-master-etcd-1 ~]# oc get clusterserviceclass | grep -v NAME | wc -l
5
[root@qe-chezhang-master-etcd-1 ~]# oc describe clusterserviceclass | grep "Removed"
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false

2. Provision only one serviceinstance with mediawiki apb

3. Remove mediawiki apb and postgresql apb images from docker-hub

3. Wait about 15mins, the two invalid clusterserviceclass were not deleted and removedFromBrokerCatalog always "false".
root@qe-chezhang-master-etcd-1 ~]# oc get clusterserviceclass | grep -v NAME | wc -l
5
[root@qe-chezhang-master-etcd-1 ~]# oc describe clusterserviceclass | grep "Removed"
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false
  Removed From Broker Catalog:	false

Comment 9 Zhang Cheng 2017-11-05 16:49:17 UTC
Paul, Please help to confirm my comment 8. Especially for [Scenario 1], user still can use those invalid clusterserviceclass from web console at that time.

In my understanding, that not looks good to me. Please assign to QA again and help to provide test scenario you expect if I have a mistake. Thanks.

Comment 10 Paul Morie 2017-11-06 19:23:08 UTC
Zhang Cheng-

Let me address the scenarios one at a time:

Scenario 1:

This scenario doesn't correctly make the service-catalog aware that any services have been removed from the broker catalog.  If you change the broker URL to an invalid one, no changes at all will be made to the service-catalog resources for ClusterServiceClass and ClusterServicePlan, because the broker cannot be contacted.  We wouldn't want this to be the case because we expect that intermittent broker outages are inevitable in time.  We would not want to remove and services or plans from the service-catalog just because we had trouble contacting the broker.  So, I'm not sure this is a valid scenario.

I'll address scenario 2 in my next comment; need to check with Erik Nelson on a couple things.

Comment 11 openshift-github-bot 2017-11-08 16:35:20 UTC
Commit pushed to master at https://github.com/openshift/origin

https://github.com/openshift/origin/commit/db1fdcb9115c296926bd28c62e90eaa30ba6f7dc
Merge pull request #17205 from eriknelson/catalog-status-roles

Automatic merge from submit-queue.

Add missing catalog service account roles

* Adds the ability for the catalog controller service account to modify
clusterserviceclass and clusterserviceplan status

Bug 1499413

Comment 14 Jeff Peeler 2018-01-18 21:57:27 UTC
If I'm understanding scenario #2 in comment 8 correctly, removing an image from docker hub is going to have no affect on anything. Secondly, provisioning an instance using a serviceclass/serviceplan from the ASB, you're guaranteeing that no deletions will occur as Paul explained earlier.

The correct way to ensure that serviceclasses and serviceplans are being properly removed is to:
- ensure you don't deploy any instances at all
- have the broker return different serviceclasses/serviceplans after they have been created in kubernetes (I'm not sure if the broker would necessarily support removing a service on the fly or if you'd have to modify the broker)

I'm not sure if this should really be a bug or if it should be changed to verify deletions do occur in the correct scenario as described above.

Comment 16 Zhang Cheng 2018-01-19 08:08:58 UTC
Test version is service-catalog & ansible-service-broker v3.9.0-0.20.0

Comment 19 Zhang Cheng 2018-01-20 14:01:29 UTC
Verified and passed. Detail info refer to comment 15.

Comment 22 errata-xmlrpc 2018-12-13 19:26:48 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, 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-2018:3748