Actually wait for the project to be deleted..

Signed-off-by: Charlie Drage <charlie@charliedrage.com>

pkg/occlient/occlient.go

@@ -107,6 +107,9 @@ const (
 	// waitForPodTimeOut controls how long we should wait for a pod before giving up
 	waitForPodTimeOut = 240 * time.Second
 
+	// waitForProjectDeletionTimeOut
+	waitForProjectDeletionTimeOut = 240 * time.Second
+
 	// ComponentPortAnnotationName annotation is used on the secrets that are created for each exposed port of the component
 	ComponentPortAnnotationName = "component-port"
 
@@ -2006,67 +2009,102 @@ func (c *Client) DeleteServiceInstance(labels map[string]string) error {
 }
 
 // DeleteProject deletes given project
-func (c *Client) DeleteProject(name string) error {
-	err := c.projectClient.Projects().Delete(name, &metav1.DeleteOptions{})
-	if err != nil {
-		return errors.Wrap(err, "unable to delete project")
+//
+// NOTE:
+// There is a very specific edge case that may happen during project deletion when deleting a project and then immediately creating another.
+// Unfortunately, despite the watch interface, we cannot safely determine if the project is 100% deleted. See this link:
+// https://stackoverflow.com/questions/48208001/deleted-openshift-online-pro-project-has-left-a-trace-so-cannot-create-project-o
+// Will Gordon (Engineer @ RedHat) describes the issue:
+//
+// "Projects are deleted asynchronously after you send the delete command. So it's possible that the deletion just hasn't been reconciled yet. It should happen within a minute or so, so try again.
+// Also, please be aware that in a multitenant environment, like OpenShift Online, you are prevented from creating a project with the same name as any other project in the cluster, even if it's not your own. So if you can't create the project, it's possible that someone has already created a project with the same name."
+func (c *Client) DeleteProject(name string, wait bool) error {
+
+	// Instantiate watcher for our "wait" function
+	var watcher watch.Interface
+	var err error
+
+	// If --wait has been passed, we will wait for the project to fully be deleted
+	if wait {
+		watcher, err = c.projectClient.Projects().Watch(metav1.ListOptions{
+			FieldSelector: fields.Set{"metadata.name": name}.AsSelector().String(),
+		})
+		if err != nil {
+			return errors.Wrapf(err, "unable to watch project")
+		}
+		defer watcher.Stop()
 	}
 
-	// wait for delete to complete
-	w, err := c.projectClient.Projects().Watch(metav1.ListOptions{
-		FieldSelector: fields.Set{"metadata.name": name}.AsSelector().String(),
-	})
+	// Delete the project
+	err = c.projectClient.Projects().Delete(name, &metav1.DeleteOptions{})
 	if err != nil {
-		return errors.Wrapf(err, "unable to watch project")
+		return errors.Wrap(err, "unable to delete project")
 	}
 
-	defer w.Stop()
-	for {
-		val, ok := <-w.ResultChan()
-		// When marked for deletion... val looks like:
-		/*
-			val: {
-				Type:MODIFIED
-				Object:&Project{
-					ObjectMeta:k8s_io_apimachinery_pkg_apis_meta_v1.ObjectMeta{...},
-					Spec:ProjectSpec{...},
-					Status:ProjectStatus{
-						Phase:Terminating,
-					},
-				}
-			}
-		*/
-		// Post deletion val will look like:
-		/*
-			val: {
-				Type:DELETED
-				Object:&Project{
-					ObjectMeta:k8s_io_apimachinery_pkg_apis_meta_v1.ObjectMeta{...},
-					Spec:ProjectSpec{...},
-					Status:ProjectStatus{
-						Phase:,
-					},
-				}
-			}
-		*/
-		if !ok {
-			return fmt.Errorf("received unexpected signal %+v on project watch channel", val)
-		}
-		// So we depend on val.Type as val.Object.Status.Phase is just empty string and not a mapped value constant
-		if prj, ok := val.Object.(*projectv1.Project); ok {
-			glog.V(4).Infof("Status of delete of project %s is %s", name, prj.Status.Phase)
-			switch prj.Status.Phase {
-			//prj.Status.Phase can only be "Terminating" or "Active" or ""
-			case "":
-				if val.Type == watch.Deleted {
-					return nil
+	// If watcher has been created (wait was passed) we will create a go routine and actually **wait**
+	// until *EVERYTHING* is successfully deleted.
+	if watcher != nil {
+
+		// Project channel
+		// Watch error channel
+		projectChannel := make(chan *projectv1.Project)
+		watchErrorChannel := make(chan error)
+
+		// Create a go routine to run in the background
+		go func() {
+
+			for {
+
+				// If watch unexpected has been closed..
+				val, ok := <-watcher.ResultChan()
+				if !ok {
+					//return fmt.Errorf("received unexpected signal %+v on project watch channel", val)
+					watchErrorChannel <- errors.Errorf("watch channel was closed unexpectedly: %+v", val)
+					break
 				}
-				if val.Type == watch.Error {
-					return fmt.Errorf("failed watching the deletion of project %s", name)
+
+				// So we depend on val.Type as val.Object.Status.Phase is just empty string and not a mapped value constant
+				if projectStatus, ok := val.Object.(*projectv1.Project); ok {
+
+					glog.V(4).Infof("Status of delete of project %s is '%s'", name, projectStatus.Status.Phase)
+
+					switch projectStatus.Status.Phase {
+					//projectStatus.Status.Phase can only be "Terminating" or "Active" or ""
+					case "":
+						if val.Type == watch.Deleted {
+							projectChannel <- projectStatus
+							break
+						}
+						if val.Type == watch.Error {
+							watchErrorChannel <- errors.Errorf("failed watching the deletion of project %s", name)
+							break
+						}
+					}
+
+				} else {
+					watchErrorChannel <- errors.New("unable to convert event object to Project")
+					break
 				}
+
 			}
+			close(projectChannel)
+			close(watchErrorChannel)
+		}()
+
+		select {
+		case val := <-projectChannel:
+			glog.V(4).Infof("Deletion information for project: %+v", val)
+			return nil
+		case err := <-watchErrorChannel:
+			return err
+		case <-time.After(waitForProjectDeletionTimeOut):
+			return errors.Errorf("waited %s but couldn't delete project %s in time", waitForPodTimeOut, name)
 		}
+
 	}
+
+	// Return nil since we don't bother checking for the watcher..
+	return nil
 }
 
 // GetDeploymentConfigLabelValues get label values of given label from objects in project that are matching selector

pkg/odo/cli/project/delete.go

@@ -35,6 +35,10 @@ type ProjectDeleteOptions struct {
 
 	// generic context options common to all commands
 	*genericclioptions.Context
+
+	// wait is a boolean value to choose if we wait or not for
+	// our project to be deleted
+	wait bool
 }
 
 // NewProjectDeleteOptions creates a ProjectDeleteOptions instance
@@ -62,6 +66,9 @@ func (pdo *ProjectDeleteOptions) Validate() (err error) {
 // Run runs the project delete command
 func (pdo *ProjectDeleteOptions) Run() (err error) {
 
+	// Create the "spinner"
+	s := &log.Status{}
+
 	// This to set the project in the file and runtime
 	err = project.SetCurrent(pdo.Context.Client, pdo.projectName)
 	if err != nil {
@@ -77,10 +84,17 @@ func (pdo *ProjectDeleteOptions) Run() (err error) {
 	if log.IsJSON() || (pdo.projectForceDeleteFlag || ui.Proceed(fmt.Sprintf("Are you sure you want to delete project %v", pdo.projectName))) {
 		successMessage := fmt.Sprintf("Deleted project : %v", pdo.projectName)
 
-		err := project.Delete(pdo.Context.Client, pdo.projectName)
+		// If the --wait parameter has been passed, we add a spinner..
+		if pdo.wait {
+			s = log.Spinner("Waiting for project to be deleted")
+			defer s.End(false)
+		}
+
+		err := project.Delete(pdo.Context.Client, pdo.projectName, pdo.wait)
 		if err != nil {
 			return err
 		}
+		s.End(true)
 
 		if log.IsJSON() {
 			project.MachineReadableSuccessOutput(pdo.projectName, successMessage)
@@ -109,6 +123,7 @@ func NewCmdProjectDelete(name, fullName string) *cobra.Command {
 		},
 	}
 
+	projectDeleteCmd.Flags().BoolVarP(&o.wait, "wait", "w", false, "Wait until the project has been completely deleted")
 	projectDeleteCmd.Flags().BoolVarP(&o.projectForceDeleteFlag, "force", "f", false, "Delete project without prompting")
 
 	return projectDeleteCmd

pkg/project/project.go

@@ -4,7 +4,6 @@ import (
 	"github.com/openshift/odo/pkg/machineoutput"
 	"github.com/pkg/errors"
 
-	"github.com/openshift/odo/pkg/log"
 	"github.com/openshift/odo/pkg/occlient"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -37,18 +36,16 @@ func Create(client *occlient.Client, projectName string, wait bool) error {
 }
 
 // Delete deletes the project with name projectName and returns errors if any
-func Delete(client *occlient.Client, projectName string) error {
-	// Loading spinner
-	s := log.Spinnerf("Deleting project %s", projectName)
-	defer s.End(false)
+func Delete(client *occlient.Client, projectName string, wait bool) error {
+	if projectName == "" {
+		return errors.Errorf("no project name given")
+	}
 
 	// Delete the requested project
-	err := client.DeleteProject(projectName)
+	err := client.DeleteProject(projectName, wait)
 	if err != nil {
 		return errors.Wrap(err, "unable to delete project")
 	}
-
-	s.End(true)
 	return nil
 }
 

pkg/project/project_test.go

@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"testing"
 
+	projectv1 "github.com/openshift/api/project/v1"
 	v1 "github.com/openshift/api/project/v1"
 
 	"github.com/openshift/odo/pkg/occlient"
@@ -94,16 +95,25 @@ func TestDelete(t *testing.T) {
 	tests := []struct {
 		name        string
 		wantErr     bool
+		wait        bool
 		projectName string
 	}{
 		{
-			name:        "Test project delete for multiple projects",
+			name:        "Case 1: Test project delete for multiple projects",
 			wantErr:     false,
+			wait:        false,
 			projectName: "prj2",
 		},
 		{
-			name:        "Test delete the only remaining project",
+			name:        "Case 2: Test delete the only remaining project",
 			wantErr:     false,
+			wait:        false,
+			projectName: "testing",
+		},
+		{
+			name:        "Case 3: Test project delete with a wait passed in.",
+			wantErr:     false,
+			wait:        true,
 			projectName: "testing",
 		},
 	}
@@ -148,19 +158,24 @@ func TestDelete(t *testing.T) {
 				return true, nil, nil
 			})
 
-			go func() {
-				fkWatch.Delete(testingutil.FakeProjectStatus(corev1.NamespacePhase(""), tt.projectName))
-			}()
+			// We pass in the fakeProject in order to avoid race conditions with multiple go routines
+			fakeProject := testingutil.FakeProjectStatus(corev1.NamespacePhase(""), tt.projectName)
+			go func(project *projectv1.Project) {
+				fkWatch.Delete(project)
+			}(fakeProject)
+
 			fakeClientSet.ProjClientset.PrependWatchReactor("projects", func(action ktesting.Action) (handled bool, ret watch.Interface, err error) {
 				return true, fkWatch, nil
 			})
 
 			// The function we are testing
-			err := Delete(client, tt.projectName)
+			err := Delete(client, tt.projectName, tt.wait)
 
 			if err == nil && !tt.wantErr {
-				if len(fakeClientSet.ProjClientset.Actions()) != 2 {
+				if tt.wait && len(fakeClientSet.ProjClientset.Actions()) != 2 {
 					t.Errorf("expected 2 ProjClientSet.Actions() in Project Delete, got: %v", len(fakeClientSet.ProjClientset.Actions()))
+				} else if !tt.wait && len(fakeClientSet.ProjClientset.Actions()) != 1 {
+					t.Errorf("expected 1 ProjClientSet.Actions() in Project Delete, got: %v", len(fakeClientSet.ProjClientset.Actions()))
 				}
 			}
 

tests/integration/generic_test.go

@@ -102,22 +102,6 @@ var _ = Describe("odo generic", func() {
 		})
 	})
 
-	Context("Delete the project with flag -o json", func() {
-		var projectName string
-		JustBeforeEach(func() {
-			projectName = helper.RandString(6)
-		})
-
-		// odo project delete foobar -o json
-		It("should be able to delete project and show output in json format", func() {
-			helper.CmdShouldPass("odo", "project", "create", projectName, "-o", "json")
-
-			actual := helper.CmdShouldPass("odo", "project", "delete", projectName, "-o", "json")
-			desired := fmt.Sprintf(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"%s","namespace":"%s","creationTimestamp":null},"message":"Deleted project : %s"}`, projectName, projectName, projectName)
-			Expect(desired).Should(MatchJSON(actual))
-		})
-	})
-
 	// Uncomment once https://github.com/openshift/odo/issues/1708 is fixed
 	/*Context("odo machine readable output on empty project", func() {
 		JustBeforeEach(func() {

tests/integration/project/cmd_project_test.go

@@ -1,6 +1,7 @@
 package project
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"time"
@@ -47,6 +48,44 @@ var _ = Describe("odo project command tests", func() {
 		})
 	})
 
+	Context("Should be able to delete a project with --wait", func() {
+		var projectName string
+		JustBeforeEach(func() {
+			projectName = helper.RandString(6)
+		})
+
+		It("--wait should work with deleting a project", func() {
+
+			// Create the project
+			helper.CmdShouldPass("odo", "project", "create", projectName)
+
+			// Delete with --wait
+			output := helper.CmdShouldPass("odo", "project", "delete", projectName, "-f", "--wait")
+			Expect(output).To(ContainSubstring("Waiting for project to be deleted"))
+
+			// Check that it's NOT there..
+			output = helper.CmdShouldPass("odo", "project", "list")
+			Expect(output).ToNot(ContainSubstring(projectName))
+		})
+
+	})
+
+	Context("Delete the project with flag -o json", func() {
+		var projectName string
+		JustBeforeEach(func() {
+			projectName = helper.RandString(6)
+		})
+
+		// odo project delete foobar -o json
+		It("should be able to delete project and show output in json format", func() {
+			helper.CmdShouldPass("odo", "project", "create", projectName, "-o", "json")
+
+			actual := helper.CmdShouldPass("odo", "project", "delete", projectName, "-o", "json")
+			desired := fmt.Sprintf(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"%s","namespace":"%s","creationTimestamp":null},"message":"Deleted project : %s"}`, projectName, projectName, projectName)
+			Expect(desired).Should(MatchJSON(actual))
+		})
+	})
+
 	Context("when running project command app parameter in directory that doesn't contain .odo config directory", func() {
 		It("should successfully execute list along with machine readable output", func() {
 			time.Sleep(1 * time.Second)