Create list of all ServiceCatalog api-server custom features

[mszostok]

Description

We need to remove the Service Catalog api-server and replace it with the CRD
functionality. To do that we need to check the api-server and find all customization. Create a list of them and prepare scenario how they can be addressed by the CRD concept.

Create follow-up tickets for all found customization, like replacing the api-server plugins with proper mutation/validation webhooks.

All restrictions should be documented (e.g. requirement of using the K8s in version 1.11 or higher) and discussed if it's acceptable or not.

Here we also should have the Service Catalog without api-server which is supporting basic scenarios.

AC

• all customization of Service Catalog api-server are detected
• follow-up tickets for implementing found customization with CRDs are defined
• Service Catalog is working with CRD with minimal scope

[mszostok]

Findings from simple scenario

✅ Ready to implement

• controller-manager is using the status-subresource on all ServiceCatalog resources.
  Issue: Define the CRDs for Service Catalog resources #2798

  Click for details

  Solution:

  Resolved by adding

  subresources:
    status: {}
  

  to do CRD definition.
  This feature is in beta. Current design status can be found in this issue.

  Concerns:

  • k8s at least in version 1.11

• api-server has bug in PreperForUpdate functions for statuses.
  Issue: Unify the way how finalizers are removed in controller-manager #2835

  Click for details

  Implemented logic does not allow modification under .Spec but all another field can be changed e.g. metadata entry. Thanks to that removing finalizers worked in controllers. Using the status sub-resource on CRD this action is not allowed. Only status entry can be changed.

  Solution:

  1. Remove the finalizers by updating the resource. See POC implementation.

  Concerns:

  • none

• api-server is using TableConvertor for printing custom columns on kubectl get.
  Issue: Replace TableConvertor with AdditionalPrinterColumns in SC #2790

  Click for details

  $ kc get serviceinstances
  NAME      CLASS                       PLAN      STATUS    AGE
  redis     ClusterServiceClass/redis   micro     Ready     4h
  

  Solution:

  1. Replaced with the additional-printer-columns defined on CRD.
     
     Problem: There can be filed which needs to define the function to be resolved for a particular value e.g. STATUS or CLASS columns.
     
     Some issues from community: here and here.

  Concerns:

  • k8s in version 1.11

• api-server uses plugins which are gathered and registered in api-server, see this.
  Issue: Replace Plugins with AdmissionWebhooks in SC #2791

  Click for details

  Solution:

  1. Implement this functionality using the MutatingAdmissionWebhook

  Concerns:

  • none

• storage for api-server is placed in registry pkg. There we can find functions which are used to prepare for CREATE/UPDATE (mutators) and also some validators
  Issue: Replace "registry" pkg with AdmissionWebhooks in SC #2792

  Click for details

  We should be able to reuse most of the code but we will need to adjust it to fulfill webhook API.

  Solution:

  1. Implement this functionality using the MutatingAdmissionWebhook and ValidatingAdmissionWebhook

  Concerns:

  • K8s at least in version 1.9

⚠️ Needs decision

• api-server uses the PrepareForCreate for adjusting newly created resource.
  Issue: Replace PrepareForCreate with AdmissionWebhooks in SC #2784

  Click for details

  Solution:

  1. Implement this functionality using the MutatingAdmissionWebhook

  Concerns:

  • K8s in version 1.9
  • Where to add webhook source code?
  • How to deploy the webhook servers?

• controller-manager is using the FieldSelector. This feature is not supported for CRD. You can query only by name and namespace. Here is the k8s tests documenting that contract.
  Issue: Replace FieldSelector with LabelSelector in SC #2794

  Click for details

  Highlights:

  Using a field selector to look up a single object in a list is extremely inefficient today. We'd likely need a way to build indexes before supporting that query pattern across all objects (especially across namespaces)

  source: Issue about generic fieldSelector

  Based on that comment, maybe the FieldSelector is not a good solution at all ;)

  Solution:

  1. Replace with proper indexer for particular informer. See POC implementation.
  2. Use the LabelSelector and decorate the resource with additional labels, e.g.:

  service-catalog.k8s.io/spec.clusterServiceBrokerName: helm-broker

  It's redundant because you have this information under spec and also in labels but e.g. Prow is using this approach.

  Concerns:

  • We need to choose one approach. Consult someone from k8s. The K8s Office Hours

• api-server provides the generic subresources on your CR. Service Catalog is using that to store the references.
  Issue: Replace Service Catalog references store #2836

  Click for details

  Unfortunately, this is not supported in CRDs, see CRD docs.
  
  NOTE: It looks like only the ServiceInstance is using that. Here is PR which introduced that.

  Solution:

  1. Replace sub-resource operation with patch. See POC implementation.

  Concerns:

  • check if this approach is acceptable, cause it's not replacing whole sub-resource functionality, e.g. the RBAC

❓Needs investigation

• api-server uses the defaults. Those defaults are generated by defaulter-gen. For now, I do not know how they are used. Maybe they will work as they are or maybe we will need to implement it in mutating webhook.
  Issue: Investigate the service catalog defaults and migrate to CRDs if needed #2837

Additional notes:

• api-server sets the Finalizers using the PrepareForCreate. Is this the right place to do that?

• Service Catalog still has the PodPreset functionality in the source code, should we migrate it, or just remove?

• implementation of the controllers is tightly coupled. IMO we should get rid of it and replace with the pattern introduced by the kubebuilder where each controller has it owns struct and constructor. Check this one to see the pattern.

Still to address:

• manual check api-server for customization
• check the features gates. Document those used by api-server and check if they are not deprecated.
• which packages can be removed after switching to CRDs?

[MHBauer]

I shouted some things out in a Svc-Cat meeting. Can't find the recording. Were there any other questions? @jberkhahn