Add --abandon and --yes flags to svcat deprovision command

[taragu]

This PR is a

• Feature Implementation
• Bug Fix
• Documentation

What this PR does / why we need it: Add --abandon and --yes flags to svcat deprovision command

Which issue(s) this PR fixes

Fixes #2268

Please leave this checklist in the PR comment so that maintainers can ensure a good PR.

Merge Checklist:

• New feature
  • Tests
  • Documentation
• SVCat CLI flag
• Server Flag for config
  • Chart changes
  • removing a flag by marking deprecated and hiding to avoid
    breaking the chart release and existing clients who provide a
    flag that will get an error when they try to update

[taragu]

/cc @jberkhahn
/cc @jboyd01

[jberkhahn]

Are you sure you need to always print this?

[jberkhahn]

hmm, this doesn't seem like it works. When I try to get it to deprovision an instance to a broker that I've removed on the back end, the instance says the deprovision call failed and gets stuck like this:

apiVersion: servicecatalog.k8s.io/v1beta1
kind: ServiceInstance
metadata:
  creationTimestamp: 2019-04-05T23:15:32Z
  deletionGracePeriodSeconds: 0
  deletionTimestamp: 2019-04-05T23:18:29Z
  finalizers:
  - kubernetes-incubator/service-catalog
  generation: 2
  name: foobar
  namespace: default
  resourceVersion: "3321"
  selfLink: /apis/servicecatalog.k8s.io/v1beta1/namespaces/default/serviceinstances/foobar
  uid: b60f4476-57f8-11e9-a39a-0242ac110005
spec:
  clusterServiceClassExternalName: mysql
  clusterServiceClassRef:
    name: mysql
  clusterServicePlanExternalName: 5-7-14
  clusterServicePlanRef:
    name: mysql-5-7-14
  externalID: b60f43e7-57f8-11e9-a39a-0242ac110005
  parameters: {}
  updateRequests: 0
  userInfo:
    groups:
    - system:masters
    - system:authenticated
    uid: ""
    username: minikube-user
status:
  asyncOpInProgress: false
  conditions:
  - lastTransitionTime: 2019-04-05T23:18:52Z
    message: 'Error deprovisioning, ClusterServiceClass (K8S: "mysql" ExternalName:
      "mysql") at ClusterServiceBroker "minibroker": Delete http://minibroker-minibroker.minibroker.svc.cluster.local/v2/service_instances/b60f43e7-57f8-11e9-a39a-0242ac110005?accepts_incomplete=true&plan_id=mysql-5-7-14&service_id=mysql:
      net/http: request canceled while waiting for connection (Client.Timeout exceeded
      while awaiting headers)'
    reason: DeprovisionCallFailed
    status: Unknown
    type: Ready
  currentOperation: Deprovision
  deprovisionStatus: Required
  externalProperties:
    clusterServicePlanExternalID: mysql-5-7-14
    clusterServicePlanExternalName: 5-7-14
    userInfo:
      groups:
      - system:masters
      - system:authenticated
      uid: ""
      username: minikube-user
  inProgressProperties:
    clusterServicePlanExternalID: mysql-5-7-14
    clusterServicePlanExternalName: 5-7-14
    userInfo:
      groups:
      - system:masters
      - system:authenticated
      uid: ""
      username: minikube-user
  observedGeneration: 2
  operationStartTime: 2019-04-05T23:16:50Z
  orphanMitigationInProgress: false
  provisionStatus: Provisioned
  reconciledGeneration: 1

Is there some other thing we have to do to get it to delete the instance?

[jberkhahn]

Hmm, seems removing the finalizers gets us past this check: https://github.com/kubernetes-incubator/service-catalog/blob/master/pkg/controller/controller_instance.go#L825 but it will still try to reach out to the broker to deprovision. I'm not really sure what we want to do in this case. Will bring it up in the meeting on Monday.

[jboyd01]

Adding a work in progress label, please remove it once you are set on this and ready for review. Thanks @taragu

[jberkhahn]

so just for comparison, here's the reconcile for deleting a binding: https://github.com/kubernetes-incubator/service-catalog/blob/master/pkg/controller/controller_binding.go#L348

I'm not seeing offhand how this succeeds while the deprovision case doesn't. Hmm. Maybe I'm doing something wrong on my end? I tested this by deploying a minibroker, registering it with service catalog, provisioning an instance, and then deleting the minibroker deployment directly, and then trying to svcat deprovision --abandon the instance.

[jboyd01]

As we discussed in SIG meeting on Monday, this is the problem. We need to remove the finalizers from this instance. This call to RemoveBindingFinalizerByInstance is removing the finalizer from any bindings related to the instance.

[jberkhahn]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jberkhahn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [jberkhahn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jberkhahn]

Actually just noticed a problem right here: https://github.com/kubernetes-incubator/service-catalog/blob/645a642342ac55f0e7731518d8efd112d1575b75/cmd/svcat/binding/unbind_cmd.go#L116

This shouldn't just print the error. It should check if the error is nil or not, and return the err if it exists.

[taragu]

/test pull-service-catalog-integration

[jboyd01]

/lgtm