feat: add --wait flag to upgrade-service

Most of the service commands (e.g. "cf update-service") take a --wait
flag to wait for completion. For some reason "cf upgrade-service" did
not, and this commit resolves that inconsistency.

This is a redone version of cloudfoundry#2285 which was reverted in cloudfoundry#2317

actor/v7action/service_instance.go

@@ -146,10 +146,13 @@ func (actor Actor) UpdateManagedServiceInstance(params UpdateManagedServiceInsta
 	return stream, Warnings(warnings), err
 }
 
-func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spaceGUID string) (Warnings, error) {
-	var serviceInstance resources.ServiceInstance
-	var servicePlan resources.ServicePlan
-	var jobURL ccv3.JobURL
+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
+	)
 
 	warnings, err := railway.Sequentially(
 		func() (warnings ccv3.Warnings, err error) {
@@ -173,11 +176,12 @@ func (actor Actor) UpgradeManagedServiceInstance(serviceInstanceName string, spa
 			return
 		},
 		func() (warnings ccv3.Warnings, err error) {
-			return actor.CloudControllerClient.PollJobForState(jobURL, constant.JobPolling)
+			stream = actor.PollJobToEventStream(jobURL)
+			return
 		},
 	)
 
-	return Warnings(warnings), err
+	return stream, 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,12 +1282,13 @@ var _ = Describe("Service Instance Actions", func() {
 		)
 
 		var (
-			err      error
-			warnings Warnings
+			executeErr error
+			warnings   Warnings
+			stream     chan PollJobEvent
 		)
 
 		JustBeforeEach(func() {
-			warnings, err = actor.UpgradeManagedServiceInstance(fakeServiceInstanceName, fakeSpaceGUID)
+			stream, warnings, executeErr = actor.UpgradeManagedServiceInstance(fakeServiceInstanceName, fakeSpaceGUID)
 		})
 
 		It("makes a request to get the service instance", func() {
@@ -1313,8 +1314,9 @@ var _ = Describe("Service Instance Actions", func() {
 			})
 
 			It("returns the appropriate error", func() {
+				Expect(stream).To(BeNil())
 				Expect(warnings).To(ConsistOf("get SI warning"))
-				Expect(err).To(MatchError(actionerror.ServiceInstanceNotFoundError{
+				Expect(executeErr).To(MatchError(actionerror.ServiceInstanceNotFoundError{
 					Name: fakeServiceInstanceName,
 				}))
 			})
@@ -1342,8 +1344,9 @@ var _ = Describe("Service Instance Actions", func() {
 			})
 
 			It("returns the appropriate error", func() {
+				Expect(stream).To(BeNil())
 				Expect(warnings).To(ConsistOf("get SI warning"))
-				Expect(err).To(MatchError(actionerror.ServiceInstanceUpgradeNotAvailableError{}))
+				Expect(executeErr).To(MatchError(actionerror.ServiceInstanceUpgradeNotAvailableError{}))
 			})
 		})
 
@@ -1380,10 +1383,15 @@ var _ = Describe("Service Instance Actions", func() {
 					ccv3.Warnings{"warning from update"},
 					nil,
 				)
-				fakeCloudControllerClient.PollJobForStateReturns(
-					ccv3.Warnings{"warning from poll"},
-					nil,
-				)
+
+				fakeStream := make(chan ccv3.PollJobEvent)
+				go func() {
+					fakeStream <- ccv3.PollJobEvent{
+						State:    constant.JobProcessing,
+						Warnings: ccv3.Warnings{"stream warning"},
+					}
+				}()
+				fakeCloudControllerClient.PollJobToEventStreamReturns(fakeStream)
 			})
 
 			It("makes the right calls and returns all warnings", func() {
@@ -1403,15 +1411,18 @@ var _ = Describe("Service Instance Actions", func() {
 				})
 
 				By("polling the job", func() {
-					Expect(fakeCloudControllerClient.PollJobForStateCallCount()).To(Equal(1))
-					actualURL, actualState := fakeCloudControllerClient.PollJobForStateArgsForCall(0)
-					Expect(actualURL).To(Equal(jobURL))
-					Expect(actualState).To(Equal(constant.JobPolling))
+					Expect(fakeCloudControllerClient.PollJobToEventStreamCallCount()).To(Equal(1))
+					Expect(fakeCloudControllerClient.PollJobToEventStreamArgsForCall(0)).To(Equal(jobURL))
 				})
 
-				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"))
+				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"))
 				})
 			})
 		})

command/v7/actor.go

@@ -238,7 +238,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) (v7action.Warnings, error)
+	UpgradeManagedServiceInstance(serviceInstanceName, spaceGUID string) (chan v7action.PollJobEvent, 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,13 +4,15 @@ 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"`
 }
@@ -38,16 +40,14 @@ func (cmd UpgradeServiceCommand) Execute(args []string) error {
 
 	serviceInstanceName := string(cmd.RequiredArgs.ServiceInstance)
 
-	warnings, actorError := cmd.Actor.UpgradeManagedServiceInstance(
+	stream, 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,6 +57,17 @@ 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
 }
 
@@ -79,6 +90,7 @@ func (cmd UpgradeServiceCommand) displayEvent() error {
 			"Username":            user.Name,
 		},
 	)
+	cmd.UI.DisplayNewline()
 
 	return nil
 }
@@ -101,15 +113,8 @@ 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{}{
+func (cmd UpgradeServiceCommand) serviceInstanceName() map[string]any {
+	return map[string]any{
 		"ServiceInstanceName": cmd.RequiredArgs.ServiceInstance,
 	}
 }

command/v7/upgrade_service_command_test.go

@@ -76,6 +76,7 @@ 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},
 				)
@@ -89,37 +90,126 @@ var _ = Describe("upgrade-service command", func() {
 			})
 		})
 
-		When("the service instance upgrade starts successfully", func() {
+		When("the actor returns an unexpected error", func() {
 			BeforeEach(func() {
 				fakeActor.UpgradeManagedServiceInstanceReturns(
+					make(chan v7action.PollJobEvent),
 					v7action.Warnings{"upgrade warning"},
-					nil,
+					errors.New("bang"),
 				)
 			})
 
-			It("succeeds with a message", func() {
-				Expect(executeErr).NotTo(HaveOccurred())
+			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() {
 				Expect(testUI.Out).To(SatisfyAll(
-					Say("\n"),
-					Say(`Upgrade in progress. Use 'cf services' or 'cf service %s' to check operation status\.\n`, serviceInstanceName),
-					Say("OK\n"),
+					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"),
 				))
 			})
 		})
 
-		When("the actor returns an unexpected error", func() {
+		When("error in event stream", func() {
 			BeforeEach(func() {
+				setFlag(&cmd, "--wait")
+
+				fakeStream := make(chan v7action.PollJobEvent)
 				fakeActor.UpgradeManagedServiceInstanceReturns(
-					v7action.Warnings{"upgrade warning"},
-					errors.New("bang"),
+					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("fails with warnings", func() {
-				Expect(executeErr).To(MatchError("bang"))
-				Expect(testUI.Err).To(Say("upgrade warning"))
-				Expect(testUI.Out).NotTo(Say("OK"))
+			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() {
+			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.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"),
+				))
 			})
 		})
 	}