Add --abandon and --yes flags to svcat deprovision command (#2589)

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

* Retrieve service instance in RemoveBindingFinalizerByInstance

cmd/svcat/binding/unbind_cmd.go

@@ -27,10 +27,8 @@ import (
 
 	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/command"
 	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/output"
-	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
 	"github.com/spf13/cobra"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -114,8 +112,9 @@ func (c *unbindCmd) Run() error {
 			s := bufio.NewScanner(os.Stdin)
 			s.Scan()
 
-			err = s.Err()
-			fmt.Fprintln(c.Output, err)
+			if err = s.Err(); err != nil {
+				return err
+			}
 
 			if strings.ToLower(s.Text()) != "y" {
 				err = fmt.Errorf("aborted abandon operation")
@@ -124,8 +123,7 @@ func (c *unbindCmd) Run() error {
 		}
 
 		if c.instanceName != "" {
-			si := &v1beta1.ServiceInstance{ObjectMeta: metav1.ObjectMeta{Name: c.instanceName, Namespace: c.Namespace}}
-			_, err = c.App.RemoveBindingFinalizerByInstance(si)
+			_, err = c.App.RemoveBindingFinalizerByInstance(c.Namespace, c.instanceName)
 		} else {
 			retrievedBindings := c.getBindingsToDelete()
 			_, err = c.App.RemoveFinalizerForBindings(retrievedBindings)

cmd/svcat/binding/unbind_cmd_test.go

@@ -166,7 +166,7 @@ func TestUnbindCommand(t *testing.T) {
 			instanceName: "myinstance",
 			abandon:      true,
 			userResponse: "y",
-			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \n<nil>\ndeleted mybinding",
+			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \ndeleted mybinding",
 		},
 		{
 			name:         "delete all finalizers with user skip prompt flag",
@@ -185,7 +185,7 @@ func TestUnbindCommand(t *testing.T) {
 			instanceName: "myinstance",
 			abandon:      true,
 			userResponse: "n",
-			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \n<nil>\naborted abandon operation",
+			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \naborted abandon operation",
 			wantError:    true,
 		},
 		{
@@ -195,7 +195,7 @@ func TestUnbindCommand(t *testing.T) {
 			instanceName: "myinstance",
 			abandon:      true,
 			userResponse: "foo",
-			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \n<nil>\naborted abandon operation",
+			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \naborted abandon operation",
 			wantError:    true,
 		},
 		{
@@ -206,7 +206,7 @@ func TestUnbindCommand(t *testing.T) {
 			instanceName: "",
 			abandon:      true,
 			userResponse: "y",
-			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \n<nil>\ndeleted mybinding",
+			wantOutput:   "This action is not reversible and may cause you to be charged for the broker resources that are abandoned.\nAre you sure? [y|n]: \ndeleted mybinding",
 			wantError:    false,
 		},
 	}

cmd/svcat/instance/deprovision_cmd.go

@@ -17,7 +17,10 @@ limitations under the License.
 package instance
 
 import (
+	"bufio"
 	"fmt"
+	"os"
+	"strings"
 
 	"github.com/kubernetes-incubator/service-catalog/cmd/svcat/output"
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
@@ -31,6 +34,8 @@ type deprovisonCmd struct {
 	*command.Waitable
 
 	instanceName string
+	abandon      bool
+	skipPrompt   bool
 }
 
 // NewDeprovisionCmd builds a "svcat deprovision" command
@@ -44,12 +49,26 @@ func NewDeprovisionCmd(cxt *command.Context) *cobra.Command {
 		Short: "Deletes an instance of a service",
 		Example: command.NormalizeExamples(`
   svcat deprovision wordpress-mysql-instance
+  svcat deprovision --abandon wordpress-mysql-instance
 `),
 		PreRunE: command.PreRunE(deprovisonCmd),
 		RunE:    command.RunE(deprovisonCmd),
 	}
 	deprovisonCmd.AddNamespaceFlags(cmd.Flags(), false)
 	deprovisonCmd.AddWaitFlags(cmd)
+	cmd.Flags().BoolVar(
+		&deprovisonCmd.abandon,
+		"abandon",
+		false,
+		"Forcefully and immediately delete the resource from Service Catalog ONLY, potentially abandoning any broker resources that you may continue to be charged for.",
+	)
+	cmd.Flags().BoolVarP(
+		&deprovisonCmd.skipPrompt,
+		"yes",
+		"y",
+		false,
+		`Automatic yes to prompts. Assume "yes" as answer to all prompts and run non-interactively.`,
+	)
 
 	return cmd
 }
@@ -68,7 +87,32 @@ func (c *deprovisonCmd) Run() error {
 }
 
 func (c *deprovisonCmd) deprovision() error {
-	err := c.App.Deprovision(c.Namespace, c.instanceName)
+	var err error
+	if c.abandon {
+		fmt.Fprintln(c.Output, "This action is not reversible and may cause you to be charged for the broker resources that are abandoned. If you have any bindings for this instance, please delete them manually with svcat unbind --abandon --name bindingName")
+		if !c.skipPrompt {
+			fmt.Fprintln(c.Output, "Are you sure? [y|n]: ")
+			s := bufio.NewScanner(os.Stdin)
+			s.Scan()
+
+			err = s.Err()
+			if err != nil {
+				return err
+			}
+
+			if strings.ToLower(s.Text()) != "y" {
+				err = fmt.Errorf("aborted abandon operation")
+				return err
+			}
+		}
+
+		// Only delete the instance finalizer here. The bindings will still exist for this instance.
+		if err = c.App.RemoveFinalizerForInstance(c.Namespace, c.instanceName); err != nil {
+			return err
+		}
+	}
+
+	err = c.App.Deprovision(c.Namespace, c.instanceName)
 	if err != nil {
 		return err
 	}

cmd/svcat/testdata/output/completion-bash.txt

@@ -373,6 +373,8 @@ _svcat_deprovision()
     flags_with_completion=()
     flags_completion=()
 
+    flags+=("--abandon")
+    local_nonpersistent_flags+=("--abandon")
     flags+=("--interval=")
     local_nonpersistent_flags+=("--interval=")
     flags+=("--namespace=")
@@ -382,6 +384,9 @@ _svcat_deprovision()
     local_nonpersistent_flags+=("--timeout=")
     flags+=("--wait")
     local_nonpersistent_flags+=("--wait")
+    flags+=("--yes")
+    flags+=("-y")
+    local_nonpersistent_flags+=("--yes")
     flags+=("--context=")
     flags+=("--kubeconfig=")
     flags+=("--logtostderr")

cmd/svcat/testdata/output/completion-zsh.txt

@@ -507,6 +507,8 @@ _svcat_deprovision()
     flags_with_completion=()
     flags_completion=()
 
+    flags+=("--abandon")
+    local_nonpersistent_flags+=("--abandon")
     flags+=("--interval=")
     local_nonpersistent_flags+=("--interval=")
     flags+=("--namespace=")
@@ -516,6 +518,9 @@ _svcat_deprovision()
     local_nonpersistent_flags+=("--timeout=")
     flags+=("--wait")
     local_nonpersistent_flags+=("--wait")
+    flags+=("--yes")
+    flags+=("-y")
+    local_nonpersistent_flags+=("--yes")
     flags+=("--context=")
     flags+=("--kubeconfig=")
     flags+=("--logtostderr")

cmd/svcat/testdata/plugin.yaml

@@ -79,8 +79,14 @@ tree:
     use: class [NAME] --from [EXISTING_NAME]
   use: create
 - command: ./svcat deprovision
-  example: '  svcat deprovision wordpress-mysql-instance'
+  example: |2-
+      svcat deprovision wordpress-mysql-instance
+      svcat deprovision --abandon wordpress-mysql-instance
   flags:
+  - desc: Forcefully and immediately delete the resource from Service Catalog ONLY,
+      potentially abandoning any broker resources that you may continue to be charged
+      for.
+    name: abandon
   - desc: 'Poll interval for --wait, specified in human readable format: 30s, 1m,
       1h'
     name: interval
@@ -89,6 +95,10 @@ tree:
     name: timeout
   - desc: Wait until the operation completes.
     name: wait
+  - desc: Automatic yes to prompts. Assume "yes" as answer to all prompts and run
+      non-interactively.
+    name: "yes"
+    shorthand: "y"
   name: deprovision
   shortDesc: Deletes an instance of a service
   use: deprovision NAME

pkg/controller/controller.go

@@ -714,7 +714,7 @@ func (c *controller) getAuthCredentialsFromClusterServiceBroker(broker *v1beta1.
 			BearerConfig: bearerConfig,
 		}, nil
 	}
-	return nil, fmt.Errorf("empty auth info or unsupported auth mode: %s", authInfo)
+	return nil, fmt.Errorf("empty auth info or unsupported auth mode: %v", authInfo)
 }
 
 // getAuthCredentialsFromServiceBroker returns the auth credentials, if any, or
@@ -752,7 +752,7 @@ func (c *controller) getAuthCredentialsFromServiceBroker(broker *v1beta1.Service
 			BearerConfig: bearerConfig,
 		}, nil
 	}
-	return nil, fmt.Errorf("empty auth info or unsupported auth mode: %s", authInfo)
+	return nil, fmt.Errorf("empty auth info or unsupported auth mode: %v", authInfo)
 }
 
 func getBasicAuthConfig(secret *corev1.Secret) (*osb.BasicAuthConfig, error) {

pkg/svcat/service-catalog/binding.go

@@ -320,7 +320,12 @@ func (sdk *SDK) RemoveFinalizerForBindings(bindings []types.NamespacedName) ([]t
 }
 
 // RemoveBindingFinalizerByInstance removes v1beta1.FinalizerServiceCatalog from all bindings for the specified instance.
-func (sdk *SDK) RemoveBindingFinalizerByInstance(instance *v1beta1.ServiceInstance) ([]types.NamespacedName, error) {
+func (sdk *SDK) RemoveBindingFinalizerByInstance(ns, name string) ([]types.NamespacedName, error) {
+	instance, err := sdk.RetrieveInstance(ns, name)
+	if err != nil {
+		return nil, err
+	}
+
 	instanceBindings, err := sdk.RetrieveBindingsByInstance(instance)
 	if err != nil {
 		return nil, err

pkg/svcat/service-catalog/binding_test.go

@@ -392,50 +392,38 @@ var _ = Describe("Binding", func() {
 	})
 
 	Describe("RemoveBindingFinalizerByInstance", func() {
-		It("Calls the generated v1beta1 List method on the provided instance's namespace, and calls Update method on all bindings of the instance", func() {
-			si := &v1beta1.ServiceInstance{ObjectMeta: metav1.ObjectMeta{Name: "apple_instance", Namespace: sb.Namespace}}
+		It("Deletes all bindings of the instance", func() {
+			si := &v1beta1.ServiceInstance{ObjectMeta: metav1.ObjectMeta{Name: "myinstance", Namespace: sb.Namespace}}
 			sb.Spec.InstanceRef.Name = si.Name
-			svcCatClient = fake.NewSimpleClientset(sb, sb2)
-			sdk = &SDK{
-				ServiceCatalogClient: svcCatClient,
-			}
+			sb2.Spec.InstanceRef.Name = si.Name
+			client := fake.NewSimpleClientset(sb, sb2, si)
+			sdk.ServiceCatalogClient = client
 
-			bindings, err := sdk.RemoveBindingFinalizerByInstance(si)
+			bindings, err := sdk.RemoveBindingFinalizerByInstance(si.Namespace, si.Name)
 			Expect(err).NotTo(HaveOccurred())
 
-			Expect(bindings).To(ConsistOf(types.NamespacedName{Namespace: sb.Namespace, Name: sb.Name}))
-			actions := svcCatClient.Actions()
-
-			Expect(actions[0].Matches("list", "servicebindings")).To(BeTrue())
-			Expect(actions[0].(testing.ListActionImpl).Namespace).To(Equal(si.Namespace))
-			Expect(actions[1].Matches("get", "servicebindings")).To(BeTrue())
-			Expect(actions[1].(testing.GetActionImpl).Name).To(Equal(sb.Name))
-			Expect(actions[1].(testing.GetActionImpl).Namespace).To(Equal(sb.Namespace))
-			Expect(actions[2].Matches("update", "servicebindings")).To(BeTrue())
-			Expect(actions[2].(testing.UpdateActionImpl).Object.(*v1beta1.ServiceBinding).ObjectMeta.Name).To(Equal(sb.ObjectMeta.Name))
-			Expect(actions[2].(testing.UpdateActionImpl).Object.(*v1beta1.ServiceBinding).ObjectMeta.Namespace).To(Equal(sb.ObjectMeta.Namespace))
+			Expect(bindings).To(ConsistOf(
+				types.NamespacedName{Namespace: sb.Namespace, Name: sb.Name},
+				types.NamespacedName{Namespace: sb2.Namespace, Name: sb2.Name},
+			))
 		})
 
 		It("Bubbles up errors", func() {
-			si := &v1beta1.ServiceInstance{ObjectMeta: metav1.ObjectMeta{Name: "apple_instance", Namespace: sb.Namespace}}
+			si := &v1beta1.ServiceInstance{ObjectMeta: metav1.ObjectMeta{Name: "myinstance", Namespace: sb.Namespace}}
 			sb.Spec.InstanceRef.Name = si.Name
-			svcCatClient = fake.NewSimpleClientset(sb)
+			sb2.Spec.InstanceRef.Name = si.Name
+			client := fake.NewSimpleClientset(sb, sb2, si)
 			errorMessage := "error updating bindings"
-			svcCatClient.PrependReactor("update", "servicebindings", func(action testing.Action) (bool, runtime.Object, error) {
+			client.PrependReactor("update", "servicebindings", func(action testing.Action) (bool, runtime.Object, error) {
 				return true, nil, fmt.Errorf(errorMessage)
 			})
-			sdk = &SDK{
-				ServiceCatalogClient: svcCatClient,
-			}
+			sdk.ServiceCatalogClient = client
 
-			bindings, err := sdk.RemoveBindingFinalizerByInstance(si)
+			bindings, err := sdk.RemoveBindingFinalizerByInstance(si.Namespace, si.Name)
 
 			Expect(bindings).To(BeNil())
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).Should(ContainSubstring(errorMessage))
-			Expect(svcCatClient.Actions()[0].Matches("list", "servicebindings")).To(BeTrue())
-			Expect(svcCatClient.Actions()[1].Matches("get", "servicebindings")).To(BeTrue())
-			Expect(svcCatClient.Actions()[2].Matches("update", "servicebindings")).To(BeTrue())
 		})
 	})
 })

pkg/svcat/service-catalog/instance.go

@@ -26,6 +26,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
@@ -291,3 +292,20 @@ func (sdk *SDK) InstanceHasStatus(instance *v1beta1.ServiceInstance, status v1be
 
 	return false
 }
+
+// RemoveFinalizerForInstance removes v1beta1.FinalizerServiceCatalog from the specified instance.
+func (sdk *SDK) RemoveFinalizerForInstance(ns, name string) error {
+	instance, err := sdk.RetrieveInstance(ns, name)
+	if err != nil {
+		return err
+	}
+
+	finalizers := sets.NewString(instance.Finalizers...)
+	finalizers.Delete(v1beta1.FinalizerServiceCatalog)
+	instance.Finalizers = finalizers.List()
+	_, err = sdk.ServiceCatalog().ServiceInstances(instance.Namespace).UpdateStatus(instance)
+	if err != nil {
+		return err
+	}
+	return nil
+}

pkg/svcat/service-catalog/instance_test.go

@@ -743,4 +743,37 @@ var _ = Describe("Instances", func() {
 			Expect(actions[0].(testing.GetActionImpl).Namespace).To(Equal(si.Namespace))
 		})
 	})
+
+	Describe("RemoveFinalizerForInstance", func() {
+		It("Calls the generated v1beta1 put method with the passed in instance", func() {
+			err := sdk.RemoveFinalizerForInstance(si.Namespace, si.Name)
+			Expect(err).NotTo(HaveOccurred())
+
+			actions := svcCatClient.Actions()
+			Expect(len(actions)).To(Equal(2))
+			Expect(actions[0].Matches("get", "serviceinstances")).To(BeTrue())
+			Expect(actions[0].(testing.GetActionImpl).Name).To(Equal(si.Name))
+			Expect(actions[0].(testing.GetActionImpl).Namespace).To(Equal(si.Namespace))
+			Expect(actions[1].Matches("update", "serviceinstances")).To(BeTrue())
+			Expect(actions[1].(testing.UpdateActionImpl).Object.(*v1beta1.ServiceInstance).ObjectMeta.Name).To(Equal(si.Name))
+			Expect(actions[1].(testing.UpdateActionImpl).Object.(*v1beta1.ServiceInstance).ObjectMeta.Namespace).To(Equal(si.Namespace))
+		})
+		It("Bubbles up errors", func() {
+			errorMessage := "instance not found"
+			badClient := &fake.Clientset{}
+			badClient.AddReactor("get", "serviceinstances", func(action testing.Action) (bool, runtime.Object, error) {
+				return true, nil, fmt.Errorf(errorMessage)
+			})
+			sdk.ServiceCatalogClient = badClient
+
+			err := sdk.RemoveFinalizerForInstance("foo", "bar")
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring(errorMessage))
+			actions := badClient.Actions()
+			Expect(len(actions)).To(Equal(1))
+			Expect(actions[0].Matches("get", "serviceinstances")).To(BeTrue())
+			Expect(actions[0].(testing.GetActionImpl).Name).To(Equal("bar"))
+			Expect(actions[0].(testing.GetActionImpl).Namespace).To(Equal("foo"))
+		})
+	})
 })

pkg/svcat/service-catalog/sdk.go

@@ -43,9 +43,10 @@ type SvcatClient interface {
 	RetrieveBindingsByInstance(*apiv1beta1.ServiceInstance) ([]apiv1beta1.ServiceBinding, error)
 	Unbind(string, string) ([]types.NamespacedName, error)
 	WaitForBinding(string, string, time.Duration, *time.Duration) (*apiv1beta1.ServiceBinding, error)
-	RemoveBindingFinalizerByInstance(*apiv1beta1.ServiceInstance) ([]types.NamespacedName, error)
+	RemoveBindingFinalizerByInstance(string, string) ([]types.NamespacedName, error)
 	RemoveFinalizerForBindings([]types.NamespacedName) ([]types.NamespacedName, error)
 	RemoveFinalizerForBinding(types.NamespacedName) error
+	RemoveFinalizerForInstance(string, string) error
 
 	Deregister(string, *ScopeOptions) error
 	RetrieveBrokers(opts ScopeOptions) ([]Broker, error)