Revert "feat: add --wait flag to upgrade-service"

actor/v7action/service_instance.go

@@ -146,13 +146,10 @@ func (actor Actor) UpdateManagedServiceInstance(params UpdateManagedServiceInsta
 	return stream, Warnings(warnings), err
 }
 
-func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spaceGUID string) (chan PollJobEvent, Warnings, error) {
-	var (
-		serviceInstance resources.ServiceInstance
-		servicePlan     resources.ServicePlan
-		jobURL          ccv3.JobURL
-		stream          chan PollJobEvent
-	)
+func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spaceGUID string) (Warnings, error) {
+	var serviceInstance resources.ServiceInstance
+	var servicePlan resources.ServicePlan
+	var jobURL ccv3.JobURL
 
 	warnings, err := railway.Sequentially(
 		func() (warnings ccv3.Warnings, err error) {
@@ -176,12 +173,11 @@ func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spa
 			return
 		},
 		func() (warnings ccv3.Warnings, err error) {
-			stream = actor.PollJobToEventStream(jobURL)
-			return
+			return actor.CloudControllerClient.PollJobForState(jobURL, constant.JobPolling)
 		},
 	)
 
-	return stream, Warnings(warnings), err
+	return Warnings(warnings), err
 }
 
 func (actor Actor) RenameServiceInstance(currentServiceInstanceName, spaceGUID, newServiceInstanceName string) (Warnings, error) {

actor/v7action/service_instance_test.go

@@ -493,7 +493,7 @@ var _ = Describe("Service Instance Actions", func() {
 				})
 
 				It("returns warnings and an error", func() {
-					Expect(warnings).To(ContainElement("warning from get"))
+					//Expect(warnings).To(ContainElement("warning from get"))
 					Expect(executeErr).To(MatchError("bang"))
 				})
 			})
@@ -1282,13 +1282,12 @@ var _ = Describe("Service Instance Actions", func() {
 		)
 
 		var (
-			executeErr error
-			warnings   Warnings
-			stream     chan PollJobEvent
+			err      error
+			warnings Warnings
 		)
 
 		JustBeforeEach(func() {
-			stream, warnings, executeErr = actor.UpgradeManagedServiceInstance(fakeServiceInstanceName, fakeSpaceGUID)
+			warnings, err = actor.UpgradeManagedServiceInstance(fakeServiceInstanceName, fakeSpaceGUID)
 		})
 
 		It("makes a request to get the service instance", func() {
@@ -1314,9 +1313,8 @@ var _ = Describe("Service Instance Actions", func() {
 			})
 
 			It("returns the appropriate error", func() {
-				Expect(stream).To(BeNil())
 				Expect(warnings).To(ConsistOf("get SI warning"))
-				Expect(executeErr).To(MatchError(actionerror.ServiceInstanceNotFoundError{
+				Expect(err).To(MatchError(actionerror.ServiceInstanceNotFoundError{
 					Name: fakeServiceInstanceName,
 				}))
 			})
@@ -1344,9 +1342,8 @@ var _ = Describe("Service Instance Actions", func() {
 			})
 
 			It("returns the appropriate error", func() {
-				Expect(stream).To(BeNil())
 				Expect(warnings).To(ConsistOf("get SI warning"))
-				Expect(executeErr).To(MatchError(actionerror.ServiceInstanceUpgradeNotAvailableError{}))
+				Expect(err).To(MatchError(actionerror.ServiceInstanceUpgradeNotAvailableError{}))
 			})
 		})
 
@@ -1383,15 +1380,10 @@ var _ = Describe("Service Instance Actions", func() {
 					ccv3.Warnings{"warning from update"},
 					nil,
 				)
-
-				fakeStream := make(chan ccv3.PollJobEvent)
-				go func() {
-					fakeStream <- ccv3.PollJobEvent{
-						State:    constant.JobProcessing,
-						Warnings: ccv3.Warnings{"stream warning"},
-					}
-				}()
-				fakeCloudControllerClient.PollJobToEventStreamReturns(fakeStream)
+				fakeCloudControllerClient.PollJobForStateReturns(
+					ccv3.Warnings{"warning from poll"},
+					nil,
+				)
 			})
 
 			It("makes the right calls and returns all warnings", func() {
@@ -1411,18 +1403,15 @@ var _ = Describe("Service Instance Actions", func() {
 				})
 
 				By("polling the job", func() {
-					Expect(fakeCloudControllerClient.PollJobToEventStreamCallCount()).To(Equal(1))
-					Expect(fakeCloudControllerClient.PollJobToEventStreamArgsForCall(0)).To(Equal(jobURL))
+					Expect(fakeCloudControllerClient.PollJobForStateCallCount()).To(Equal(1))
+					actualURL, actualState := fakeCloudControllerClient.PollJobForStateArgsForCall(0)
+					Expect(actualURL).To(Equal(jobURL))
+					Expect(actualState).To(Equal(constant.JobPolling))
 				})
 
-				By("returning a stream, warnings and no error", func() {
-					Eventually(stream).Should(Receive(Equal(PollJobEvent{
-						State:    JobProcessing,
-						Warnings: Warnings{"stream warning"},
-					})))
-
-					Expect(executeErr).NotTo(HaveOccurred())
-					Expect(warnings).To(ConsistOf("warning from get", "warning from plan", "warning from update"))
+				By("returning warnings and no error", func() {
+					Expect(err).NotTo(HaveOccurred())
+					Expect(warnings).To(ConsistOf("warning from get", "warning from plan", "warning from update", "warning from poll"))
 				})
 			})
 		})

command/v7/actor.go

@@ -237,7 +237,7 @@ type Actor interface {
 	UpdateDestination(string, string, string) (v7action.Warnings, error)
 	UpdateDomainLabelsByDomainName(string, map[string]types.NullString) (v7action.Warnings, error)
 	UpdateManagedServiceInstance(params v7action.UpdateManagedServiceInstanceParams) (chan v7action.PollJobEvent, v7action.Warnings, error)
-	UpgradeManagedServiceInstance(serviceInstanceName, spaceGUID string) (chan v7action.PollJobEvent, v7action.Warnings, error)
+	UpgradeManagedServiceInstance(serviceInstanceName, spaceGUID string) (v7action.Warnings, error)
 	UpdateOrganizationLabelsByOrganizationName(string, map[string]types.NullString) (v7action.Warnings, error)
 	UpdateOrganizationQuota(quotaName string, newName string, limits v7action.QuotaLimits) (v7action.Warnings, error)
 	UpdateProcessByTypeAndApplication(processType string, appGUID string, updatedProcess resources.Process) (v7action.Warnings, error)

command/v7/upgrade_service_command.go

@@ -4,15 +4,13 @@ import (
 	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/command/flag"
 	"code.cloudfoundry.org/cli/command/translatableerror"
-	"code.cloudfoundry.org/cli/command/v7/shared"
 )
 
 type UpgradeServiceCommand struct {
 	BaseCommand
 
 	RequiredArgs flag.ServiceInstance `positional-args:"yes"`
 	Force        bool                 `short:"f" long:"force" description:"Force upgrade without asking for confirmation"`
-	Wait         bool                 `short:"w" long:"wait" description:"Wait for the operation to complete"`
 
 	relatedCommands interface{} `related_commands:"services, update-service, update-user-provided-service"`
 }
@@ -40,14 +38,16 @@ func (cmd UpgradeServiceCommand) Execute(args []string) error {
 
 	serviceInstanceName := string(cmd.RequiredArgs.ServiceInstance)
 
-	stream, warnings, actorError := cmd.Actor.UpgradeManagedServiceInstance(
+	warnings, actorError := cmd.Actor.UpgradeManagedServiceInstance(
 		serviceInstanceName,
 		cmd.Config.TargetedSpace().GUID,
 	)
 	cmd.UI.DisplayWarnings(warnings)
 
 	switch actorError.(type) {
 	case nil:
+		cmd.displayUpgradeInProgressMessage()
+		cmd.UI.DisplayOK()
 	case actionerror.ServiceInstanceUpgradeNotAvailableError:
 		cmd.UI.DisplayText(actorError.Error())
 		cmd.UI.DisplayOK()
@@ -57,17 +57,6 @@ func (cmd UpgradeServiceCommand) Execute(args []string) error {
 		return actorError
 	}
 
-	complete, err := shared.WaitForResult(stream, cmd.UI, cmd.Wait)
-	switch {
-	case err != nil:
-		return err
-	case complete:
-		cmd.UI.DisplayTextWithFlavor("Upgrade of service instance {{.ServiceInstanceName}} complete.", cmd.serviceInstanceName())
-	default:
-		cmd.UI.DisplayTextWithFlavor("Upgrade in progress. Use 'cf services' or 'cf service {{.ServiceInstanceName}}' to check operation status.", cmd.serviceInstanceName())
-	}
-
-	cmd.UI.DisplayOK()
 	return nil
 }
 
@@ -90,7 +79,6 @@ func (cmd UpgradeServiceCommand) displayEvent() error {
 			"Username":            user.Name,
 		},
 	)
-	cmd.UI.DisplayNewline()
 
 	return nil
 }
@@ -113,6 +101,13 @@ func (cmd UpgradeServiceCommand) displayPrompt() (bool, error) {
 	return upgrade, nil
 }
 
+func (cmd UpgradeServiceCommand) displayUpgradeInProgressMessage() {
+	cmd.UI.DisplayTextWithFlavor("Upgrade in progress. Use 'cf services' or 'cf service {{.ServiceInstance}}' to check operation status.",
+		map[string]interface{}{
+			"ServiceInstance": cmd.RequiredArgs.ServiceInstance,
+		})
+}
+
 func (cmd UpgradeServiceCommand) serviceInstanceName() map[string]interface{} {
 	return map[string]interface{}{
 		"ServiceInstanceName": cmd.RequiredArgs.ServiceInstance,

command/v7/upgrade_service_command_test.go

@@ -76,7 +76,6 @@ var _ = Describe("upgrade-service command", func() {
 		When("the service instance does not exist", func() {
 			BeforeEach(func() {
 				fakeActor.UpgradeManagedServiceInstanceReturns(
-					nil,
 					v7action.Warnings{"upgrade warning"},
 					actionerror.ServiceInstanceNotFoundError{Name: serviceInstanceName},
 				)
@@ -90,126 +89,37 @@ var _ = Describe("upgrade-service command", func() {
 			})
 		})
 
-		When("the actor returns an unexpected error", func() {
+		When("the service instance upgrade starts successfully", func() {
 			BeforeEach(func() {
 				fakeActor.UpgradeManagedServiceInstanceReturns(
-					make(chan v7action.PollJobEvent),
 					v7action.Warnings{"upgrade warning"},
-					errors.New("bang"),
-				)
-			})
-
-			It("fails with warnings", func() {
-				Expect(executeErr).To(MatchError("bang"))
-				Expect(testUI.Err).To(Say("upgrade warning"))
-				Expect(testUI.Out).NotTo(Say("OK"))
-			})
-		})
-
-		When("stream goes to polling", func() {
-			BeforeEach(func() {
-				fakeStream := make(chan v7action.PollJobEvent)
-				fakeActor.UpgradeManagedServiceInstanceReturns(
-					fakeStream,
-					v7action.Warnings{"actor warning"},
 					nil,
 				)
-
-				go func() {
-					fakeStream <- v7action.PollJobEvent{
-						State:    v7action.JobPolling,
-						Warnings: v7action.Warnings{"poll warning"},
-					}
-				}()
 			})
 
-			It("prints messages and warnings", func() {
+			It("succeeds with a message", func() {
+				Expect(executeErr).NotTo(HaveOccurred())
+				Expect(testUI.Err).To(Say("upgrade warning"))
 				Expect(testUI.Out).To(SatisfyAll(
-					Say(`Upgrading service instance %s in org %s / space %s as %s...\n`, serviceInstanceName, orgName, spaceName, username),
-					Say(`\n`),
-					Say(`Upgrade in progress. Use 'cf services' or 'cf service %s' to check operation status\.`, serviceInstanceName),
-					Say(`OK\n`),
-				))
-
-				Expect(testUI.Err).To(SatisfyAll(
-					Say("actor warning"),
-					Say("poll warning"),
+					Say("\n"),
+					Say(`Upgrade in progress. Use 'cf services' or 'cf service %s' to check operation status\.\n`, serviceInstanceName),
+					Say("OK\n"),
 				))
 			})
 		})
 
-		When("error in event stream", func() {
-			BeforeEach(func() {
-				setFlag(&cmd, "--wait")
-
-				fakeStream := make(chan v7action.PollJobEvent)
-				fakeActor.UpgradeManagedServiceInstanceReturns(
-					fakeStream,
-					v7action.Warnings{"a warning"},
-					nil,
-				)
-
-				go func() {
-					fakeStream <- v7action.PollJobEvent{
-						State:    v7action.JobPolling,
-						Warnings: v7action.Warnings{"poll warning"},
-					}
-					fakeStream <- v7action.PollJobEvent{
-						State:    v7action.JobFailed,
-						Warnings: v7action.Warnings{"failed warning"},
-						Err:      errors.New("boom"),
-					}
-				}()
-			})
-
-			It("returns the error and prints warnings", func() {
-				Expect(executeErr).To(MatchError("boom"))
-				Expect(testUI.Err).To(SatisfyAll(
-					Say("poll warning"),
-					Say("failed warning"),
-				))
-			})
-		})
-
-		When("--wait flag specified", func() {
+		When("the actor returns an unexpected error", func() {
 			BeforeEach(func() {
-				setFlag(&cmd, "--wait")
-
-				fakeStream := make(chan v7action.PollJobEvent)
 				fakeActor.UpgradeManagedServiceInstanceReturns(
-					fakeStream,
-					v7action.Warnings{"a warning"},
-					nil,
+					v7action.Warnings{"upgrade warning"},
+					errors.New("bang"),
 				)
-
-				go func() {
-					fakeStream <- v7action.PollJobEvent{
-						State:    v7action.JobPolling,
-						Warnings: v7action.Warnings{"poll warning"},
-					}
-					fakeStream <- v7action.PollJobEvent{
-						State:    v7action.JobComplete,
-						Warnings: v7action.Warnings{"failed warning"},
-					}
-					close(fakeStream)
-				}()
 			})
 
-			It("prints messages and warnings", func() {
-				Expect(testUI.Out).To(SatisfyAll(
-					Say(`Upgrading service instance %s in org %s / space %s as %s...\n`, serviceInstanceName, orgName, spaceName, username),
-					Say(`\n`),
-					Say(`Waiting for the operation to complete\.\.\n`),
-					Say(`\n`),
-					Say(`Upgrade of service instance %s complete\.\n`, serviceInstanceName),
-					Say(`OK\n`),
-				))
-
-				Expect(testUI.Err).To(SatisfyAll(
-					Say("a warning"),
-					Say("poll warning"),
-					Say("failed warning"),
-				))
+			It("fails with warnings", func() {
+				Expect(executeErr).To(MatchError("bang"))
+				Expect(testUI.Err).To(Say("upgrade warning"))
+				Expect(testUI.Out).NotTo(Say("OK"))
 			})
 		})
 	}